sirene by f-necas · Pull Request #269 · georchestra/georchestra-gateway

f-necas · 2026-04-10T13:54:11Z

No description provided.

…DC user provisioning

… record for organization name resolution

…nd dependency management

Copilot

Pull request overview

Adds a new pluggable “organization name resolver” SPI and integrates a SIRENE (INSEE) implementation to resolve organization names (and short names) from identifiers (e.g., SIRET) during OIDC/LDAP account provisioning.

Changes:

Introduces georchestra-gateway-org-resolvers (SPI) and georchestra-gateway-sirene (SIRENE resolver + auto-configuration) modules.
Extends OIDC provider config and LDAP account management to optionally resolve/override organization names via a configurable resolver chain.
Updates build/dependency wiring (parent, modules list, gateway deps, Makefile docker targets) and adds example configuration.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
pom.xml	Bumps Spring Cloud parent and adds new internal modules to dependency management.
modules/pom.xml	Registers `org-resolvers` and `sirene` modules in the build.
modules/org-resolvers/pom.xml	Adds new SPI module packaging.
modules/org-resolvers/src/main/java/.../OrganizationNameResolver.java	Defines the resolver SPI for organization name resolution.
modules/org-resolvers/src/main/java/.../ResolvedOrganization.java	Adds resolved org DTO (name + shortName).
modules/sirene/pom.xml	Adds new SIRENE integration module and dependencies.
modules/sirene/.../AutoConfiguration.imports	Registers SIRENE auto-configuration for Spring Boot.
modules/sirene/.../SireneApiConfigProperties.java	Adds config properties for SIRENE base URL/key and enablement.
modules/sirene/.../SireneAutoConfiguration.java	Wires `RestClient` + `OrganizationNameResolver` beans behind a property flag.
modules/sirene/.../SireneOrganizationNameResolver.java	Implements SIRENE API lookup + short-name derivation + caching.
gateway/pom.xml	Adds SPI dependency and optional SIRENE module dependency to the gateway.
gateway/src/main/java/.../OpenIdConnectCustomConfig.java	Adds provider-level flags and resolver-chain configuration accessors.
gateway/src/main/java/.../LdapAccountsManager.java	Uses resolver chain to set/update org names during org creation/linking.
gateway/src/main/java/.../GeorchestraLdapAccountManagementConfiguration.java	Injects optional `OrganizationNameResolver` into LDAP account manager.
gateway/src/test/java/.../LdapAccountsManagerTest.java	Adjusts tests for constructor signature + updated verification method signature.
datadir/gateway/security.yaml	Documents example SIRENE + OIDC resolver-chain configuration.
Makefile	Ensures new modules are built/installed before docker packaging.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-10T14:13:55Z

+    private Optional<ResolvedOrganization> resolveOrgNameFromChain(String identifier, @NonNull String oAuth2Provider) {
+        List<String> resolvers = providersConfig.orgNameResolvers(oAuth2Provider);
+        if (resolvers.isEmpty()) {
+            return Optional.empty();
+        }
+
+        for (String resolverEntry : resolvers) {
+            Optional<ResolvedOrganization> result = tryResolver(resolverEntry, identifier);
+            if (result.isPresent()) {
+                return result;
+            }
+        }
+
+        log.debug("No organization name resolver returned a result for identifier '{}' (provider: {})", identifier,
+                oAuth2Provider);
+        return Optional.empty();
+    }
+
+    /**
+     * Tries a single resolver entry from the fallback chain.
+     */
+    private Optional<ResolvedOrganization> tryResolver(String resolverEntry, String identifier) {
+        if ("sirene".equalsIgnoreCase(resolverEntry)) {
+            return orgNameResolver.flatMap(resolver -> resolver.resolve(identifier));
+        }
+
+        if ("identifier".equalsIgnoreCase(resolverEntry)) {
+            if (StringUtils.isNotEmpty(identifier)) {
+                String shortName = identifier.length() > 32 ? identifier.substring(0, 32) : identifier;
+                return Optional.of(new ResolvedOrganization(identifier, shortName));
+            }
+            return Optional.empty();
+        }
+
+        if (resolverEntry.toLowerCase().startsWith("static:")) {
+            String staticValue = resolverEntry.substring("static:".length()).trim();
+            if (!staticValue.isEmpty()) {
+                return Optional.of(new ResolvedOrganization(staticValue, staticValue));
+            }
+        }
+
+        log.warn("Unknown organization name resolver type: '{}'", resolverEntry);
+        return Optional.empty();


The new organization-name resolution chain (resolveOrgNameFromChain/tryResolver) and the overrideExistingOrgName behavior introduce non-trivial branching (provider config, resolver order, unknown resolver handling), but there are no tests covering these paths. Please add focused unit tests for at least: resolver ordering, static: parsing, identifier truncation, and override behavior when enabled/disabled.

…me resolver configuration

…or clarity

pmauduit

interesting so far, here are some early comments reading the diff

pmauduit · 2026-04-13T15:07:32Z

    private final @NonNull OpenIdConnectCustomConfig providersConfig;

+    /** Optional organization name resolver. */
+    private final @NonNull Optional<OrganizationNameResolver> orgNameResolver;


@NonNull and Optional<T> ? Isn't it redundant ?

pmauduit · 2026-04-13T15:08:29Z

            OrgsDao orgsDao, DemultiplexingUsersApi demultiplexingUsersApi,
            GeorchestraGatewaySecurityConfigProperties georchestraGatewaySecurityConfigProperties,
-            OpenIdConnectCustomConfig providersConfig) {
+            OpenIdConnectCustomConfig providersConfig, Optional<OrganizationNameResolver> orgNameResolver) {


I did not know that Spring was able to pass Optionals as dependencies, makes sense though.

pmauduit · 2026-04-13T15:12:16Z

+        Org org = new Org();
+        org.setId(orgId);
        org.setOrgUniqueId(Optional.ofNullable(orgUniqueId).orElse(""));
+        org.setOrgType("Other");


I am wondering if this should be configureable, and on the other hand, it could be up to the platform admin to manage them in the console once created, "other" sounding as a good enough default option.

pmauduit · 2026-04-13T15:17:51Z

+ * to resolve organization names from national registries.
+ * </p>
+ */
+public interface OrganizationNameResolver {


I think that since the interface is mentioned in the georchestra spring configuration, maybe it should go into the core (under gateway/), not into a module ? and keep the implementation (sirene) into modules/

pmauduit · 2026-04-13T15:19:39Z

+ * @param name      the full organization name (e.g. "Ministère de
+ *                  l'Environnement")
+ * @param shortName a shortened/normalized version of the name (max 32
+ *                  characters)


not sure if such a thing exist, but are there any annotation which could enforce this ? (like @MaxLength(32) or so ?). I guess we should be good with the current comment anyway, in term of documentation.

pmauduit · 2026-04-13T15:22:38Z

+    }
+
+    /**
+     * Creates a {@link RestClient} pre-configured for the INSEE SIRENE API.


I remember the issues we had with Gabriel working on the gateway in an environment where the http clients had to go through a proxy. But not prio here, it can probably be revisited later on if needed.

pmauduit · 2026-04-13T15:25:49Z

+    @SuppressWarnings("unchecked")
+    private Optional<ResolvedOrganization> doResolve(String siret) {
+        try {
+            log.info("Resolving organization name from SIRENE API for SIRET: {}", siret);


debug or trace instead of info ? (not sure though)

pmauduit · 2026-04-13T15:27:14Z

+            }
+
+            String orgName = extractOrgName(uniteLegale);
+            if (orgName == null || orgName.isBlank()) {


I think you have a StringUtils.isEmpty(), either via apache commons, or via spring.

pmauduit · 2026-04-13T15:28:02Z

+            }
+
+            String shortName = deriveShortName(orgName);
+            log.info("Resolved SIRET {} to organization: '{}' (short: '{}')", siret, orgName, shortName);


same, not sure if info is the right level, given the log trace being issued

f-necas added 4 commits April 10, 2026 13:23

feat: integrate SIRENE API for organization name resolution during OI…

eb3939b

…DC user provisioning

feat: add OrganizationNameResolver interface and ResolvedOrganization…

1b6394e

… record for organization name resolution

chore: update Makefile and pom.xml for improved module installation a…

386c107

…nd dependency management

chore: update documentation on how to setup sirene api

e4005f5

f-necas requested a review from Copilot April 10, 2026 14:04

Copilot started reviewing on behalf of f-necas April 10, 2026 14:05 View session

Copilot AI reviewed Apr 10, 2026

View reviewed changes

f-necas added 2 commits April 11, 2026 09:02

refactor: remove specific references to SIRENE API in organization na…

f04e080

…me resolver configuration

refactor: update organization name resolver logic and documentation f…

09b8f66

…or clarity

pmauduit reviewed Apr 13, 2026

View reviewed changes

Conversation

f-necas commented Apr 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pmauduit left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants