Skip to content

feat(core): add jit principal generation - #105

Open
RVANDO12 wants to merge 28 commits into
mainfrom
feat/right-management
Open

feat(core): add jit principal generation#105
RVANDO12 wants to merge 28 commits into
mainfrom
feat/right-management

Conversation

@RVANDO12

@RVANDO12 RVANDO12 commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

PR Description

feat(auth): implement just-in-time principal provisioning and self-service API

Summary

This PR adds automatic provisioning of authenticated actors into the IDP-Core catalog and exposes a self-service endpoint for retrieving the current principal.

Execution and Data Flow

Incoming HTTP request
          |
          v
Spring Security authentication
(JWT resource server or OAuth2/OIDC login)
          |
          v
JitProvisioningFilter
          |
          +--> PrincipalExtractor
          |        +--> JWT strategy
          |        +--> OAuth2/OIDC strategy
          |        +--> fallback strategy
          |
          +--> PrincipalProvisioningService
                    +--> Existing principal: return it unchanged
                    +--> New principal: create entity, properties and relations
          |
          v
Controller execution

What This PR Provides

  1. JIT provisioning of principal entities after successful authentication.
  2. Support for:
    • JWT resource-server authentication.
    • OAuth2/OIDC users through the local OAuth2 login flow.
    • Human principals and service accounts.
  3. Configurable identity-provider claim mappings through: app.security.authentication.user-claim-mappings.
  4. Configurable service-account detection:
    • strict: uses one definitive claim.
    • legacy: checks grant_type/gty, service_name, and client_id/azp.
  5. Human principal attributes:
    • kind
    • email, when available
  6. Service-account attributes:
    • kind
    • client_id
    • origin, when available
  7. Group extraction from JWT/OAuth2 claims.
  8. Defensive member_of relation creation:
    • Groups are resolved using one batch database query.
    • Unknown groups are ignored.
    • Missing teams do not fail principal provisioning.
  9. New endpoint: GET /api/v1/entities/principals/me
  10. Flyway migration creating:
    • principal entity template.
    • team entity template.
    • Principal properties.
    • member_of relation definition.
  11. Public-path exclusions for JIT provisioning.
  12. JIT provisioning failure is fail-open: the authenticated request continues, while the failure is logged.
  13. Existing principals are returned unchanged; this PR does not synchronize or update existing principal metadata.

Configuration

Default JWT configuration:

app:
  security:
     authentication:
        jwt:
          enabled: true
        oauth2-login:
          enabled: false
        mock:
          enabled: false
        user-claim-mappings:
          sub: sub
          preferred_username: preferred_username
          name: name
          email: email
          groups: groups
          client_id: client_id
          azp: azp
          grant_type: grant_type
          gty: gty
          service_name: service_name
        service-account-detection:
          enabled: true
          mode: legacy
          legacy-fallback-claims: grant_type,service_name,client_id

For production deployments, strict service-account detection is recommended with a definitive claim configured by the identity provider.

How to Test

Automated tests

Run the principal and authentication unit tests:

./mvnw -Dtest=PrincipalExtractorTest,PrincipalProvisioningServiceTest,JitProvisioningFilterTest,JwtFilterChainConfigTest,OAuth2LoginFilterChainConfigTest test
Run the controller integration tests:
./mvnw -Dtest=PrincipalControllerTest test

These integration tests require Docker because the test suite uses Testcontainers.

Run the complete verification:

./mvnw clean verify

Manual prerequisites
• IDP-Core is running.
• PostgreSQL is available and migrations have completed.
• Docker is running for integration tests.
• A valid JWT issuer and JWKS endpoint are configured.
• The token contains a valid sub claim.
• For group testing, create at least one team entity, for example: identifier: "backend-devs".

JWT human principal
Obtain a valid JWT containing claims such as:

{
  "sub": "jdoe",
  "preferred_username": "jdoe",
  "name": "Jane Doe",
  "email": "jane.doe@example.com",
  "groups": ["backend-devs"]
}

Call the endpoint:

curl http://localhost:8080/api/v1/entities/principals/me \
  -H "Authorization: Bearer <JWT_TOKEN>"

Expected result:

• 200 OK.
• template_identifier is principal.
• identifier is derived from preferred_username, falling back to sub.
• properties.kind is HUMAN.
• The email is included when present.
• Existing matching teams appear in the member_of relation.

JWT service account

Use a token that satisfies the configured service-account detection mode.
For legacy mode, verify one of:
• grant_type=client_credentials.
• gty=client_credentials.
• A service_name claim is present.
• The mapped client_id or azp equals sub.

Expected result:
• properties.kind is SERVICE_ACCOUNT.
• The principal identifier is the client ID, falling back to sub.
• client_id is stored as a principal property.
• origin is stored when present.

Unknown groups
Repeat the request with a group that does not match any team entity:

{
  "groups": ["team-that-does-not-exist"]
}

Expected result:
• Principal provisioning succeeds.
• The request does not fail because of the missing team.
• No relation is created for the unknown group.

Idempotency

Call /api/v1/entities/principals/me multiple times with the same identity.

Expected result:
• The same principal is returned.
• No duplicate entity is created.
• Existing principal data is not updated by subsequent authentication requests.

Authentication and access checks

Verify:
• No credentials result in 401 Unauthorized.
• Invalid or expired JWTs are rejected by Spring Security.
• Authenticated requests to protected /api/v1/ endpoints trigger JIT provisioning.
• Public paths such as /, /actuator/, /swagger-ui/, and /v3/api-docs/ do not trigger provisioning.
• A provisioning/database failure is logged, but the authenticated request continues.

OAuth2/OIDC local login

With the local profile:

SPRING_PROFILE=local
GITHUB_CLIENT_ID=<client-id>
GITHUB_CLIENT_SECRET=<client-secret>
./mvnw spring-boot:run

Configure the OAuth application callback:
http://localhost:8084/login/oauth2/code/github
Start login through:
http://localhost:8084/oauth2/authorization/github
After login, call a protected /api/v1/ endpoint and verify that the OAuth2/OIDC user is provisioned using the configured GitHub claim mappings.

Review Checklist

☐ Automated unit tests pass.
☐ Integration tests pass with Docker/Testcontainers available.
☐ JWT human-principal flow has been tested.
☐ JWT service-account flow has been tested.
☐ OAuth2/OIDC login flow has been tested.
☐ Existing and unknown group scenarios have been tested.
☐ Repeated requests do not create duplicate principals.
☐ Public paths remain excluded from JIT provisioning.
☐ Provisioning failure behavior has been verified.
☐ Flyway migration has been verified against PostgreSQL.
☐ Authentication documentation has been reviewed.
☐ Implementation follows the applicable technical documentation and ADRs.
☐ PR title follows the repository release-note convention.

Breaking Changes

No intentional breaking API change is introduced. The feature changes authenticated-request behavior by automatically attempting principal provisioning.

Copilot AI review requested due to automatic review settings July 30, 2026 07:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Implements just-in-time (JIT) provisioning for authenticated actors by introducing a principal catalog template, extracting PrincipalInfo from the Spring Security context, provisioning principals on authenticated requests via a servlet filter, and exposing a self-service GET /api/v1/entities/principals/me endpoint to retrieve the current principal.

Changes:

  • Added principal extraction + JIT provisioning pipeline (filter + provisioning service) and a new /principals/me controller endpoint.
  • Introduced principal/team template metadata via Flyway and updated local/test seed data accordingly.
  • Added unit/integration tests and extended the entity repository port to support batch lookups for defensive relation building.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/test/resources/db/test/R__1_Insert_test_data.sql Adds principal/team template metadata and rules to test seed data.
src/test/java/com/decathlon/idp_core/infrastructure/adapters/api/controller/PrincipalControllerTest.java New integration tests for /principals/me and JIT behavior.
src/test/java/com/decathlon/idp_core/infrastructure/adapters/api/controller/EntityTemplateControllerTest.java Updates expected template counts after adding principal.
src/test/java/com/decathlon/idp_core/infrastructure/adapters/api/auth/JitProvisioningFilterTest.java Unit tests for JIT filter behavior and path exclusions.
src/test/java/com/decathlon/idp_core/domain/service/principal/PrincipalProvisioningServiceTest.java Unit tests for principal provisioning service behavior.
src/test/java/com/decathlon/idp_core/domain/service/principal/PrincipalExtractorTest.java Unit tests for extracting principal info from auth tokens.
src/main/resources/db/migration/V7_1__create_principal_entity_template.sql Flyway migration to register principal/team templates + properties/relations.
src/main/resources/db/local/R__1_Insert_sample_data.sql Adds principal/team template metadata to local sample seed data.
src/main/resources/application.yml Adds configurable baseline role property for authenticated principals.
src/main/java/com/decathlon/idp_core/infrastructure/adapters/persistence/PostgresEntityAdapter.java Implements new batch lookup method for entity identifiers.
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/principal/PrincipalExtractor.java Extracts PrincipalInfo from Spring Security Authentication (JWT/OIDC/OAuth2/fallback).
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/handler/ApiExceptionHandler.java Adds HTTP mapping for PrincipalNotFoundException.
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/controller/PrincipalController.java New /api/v1/entities/principals/me endpoint returning current principal.
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/configuration/SecurityRoleProperties.java Binds baseline-role security configuration.
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/configuration/SecurityConfiguration.java Adds JIT filter + baseline authority assignment via JWT converter.
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/auth/JitProvisioningFilter.java New servlet filter triggering provisioning on authenticated requests (fail-open).
src/main/java/com/decathlon/idp_core/domain/service/principal/PrincipalProvisioningService.java Domain service to create/retrieve principals and build member_of relations defensively.
src/main/java/com/decathlon/idp_core/domain/port/EntityRepositoryPort.java Adds batch lookup method for defensive group/team relation building.
src/main/java/com/decathlon/idp_core/domain/model/principal/PrincipalKind.java Defines principal kinds (HUMAN, SERVICE_ACCOUNT).
src/main/java/com/decathlon/idp_core/domain/model/principal/PrincipalInfo.java Domain record representing extracted principal identity + attributes/groups.
src/main/java/com/decathlon/idp_core/domain/exception/principal/PrincipalNotFoundException.java New domain exception when principal lookup fails.
Comments suppressed due to low confidence (1)

src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/principal/PrincipalExtractor.java:70

  • Service-account detection treats any JWT without an email claim as a service account (|| !claims.containsKey(CLAIM_EMAIL)). Human tokens may legitimately omit email, which would misclassify them and persist the wrong kind property.
    boolean isServiceAccount = claims.containsKey(CLAIM_CLIENT_ID) || claims.containsKey(CLAIM_AZP)
        || claims.containsKey(CLAIM_SERVICE_NAME) || !claims.containsKey(CLAIM_EMAIL);

Comment thread src/main/resources/db/migration/V7_1__create_principal_entity_template.sql Outdated
@github-code-quality

github-code-quality Bot commented Jul 30, 2026

Copy link
Copy Markdown

Code Coverage Overview

Languages: Java

Java / code-coverage/jacoco

The overall line coverage in commit cb6b2aa in the feat/right-managemen... branch remains at 91%, unchanged from commit 6976334 in the main branch.

Show a line coverage summary of the most impacted files.
File main 6976334 feat/right-managemen... cb6b2aa +/-
com/decathlon/i...ingService.java 0% 80% +80%
com/decathlon/i...hainConfig.java 0% 82% +82%
com/decathlon/i...hainConfig.java 0% 96% +96%
com/decathlon/i...onStrategy.java 0% 97% +97%
com/decathlon/i...onStrategy.java 0% 100% +100%
com/decathlon/i...ningFilter.java 0% 100% +100%
com/decathlon/i...hainConfig.java 0% 100% +100%
com/decathlon/i...hainConfig.java 0% 100% +100%
com/decathlon/i...lExtractor.java 0% 100% +100%
com/decathlon/i...ncipalInfo.java 0% 100% +100%

Updated September 03, 2026 07:15 UTC

Copilot AI review requested due to automatic review settings July 30, 2026 09:42
Comment thread src/test/java/com/decathlon/idp_core/TestSecurityConfiguration.java Fixed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (7)

src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/principal/PrincipalExtractor.java:55

  • Missing whitespace in authentication.getPrincipal()instanceof ... makes this statement invalid Java syntax (won't compile).
    if (authentication.getPrincipal()instanceof OAuth2User oauth2User) {

src/test/java/com/decathlon/idp_core/domain/service/principal/PrincipalExtractorTest.java:158

  • This test asserts the identifier is david-sub, but the extractor uses preferred_username when present (as verified by the alice test earlier in this class). With preferred_username = "david", the expected identifier should be david.
    // Given: JWT token without optional claims (email, groups, etc.)
    Map<String, Object> claims = Map.of("sub", "david-sub", "preferred_username", "david");

    Jwt jwt = createJwt(claims);
    Authentication authentication = new JwtAuthenticationToken(jwt);

    // When: Extract principal info
    PrincipalInfo principalInfo = principalExtractor.extractPrincipalInfo(authentication);

    // Then: Optional fields are empty/null
    assertThat(principalInfo.identifier()).isEqualTo("david-sub");
    assertThat(principalInfo.attributes()).doesNotContainKey("email");
    assertThat(principalInfo.groups()).isEmpty();

src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/principal/PrincipalExtractor.java:70

  • Service-account detection treats any JWT without an email claim as a service account (|| !claims.containsKey("email")). This misclassifies human tokens that omit email (and contradicts the extractor contract that relies primarily on client_id/azp/service_name).
    boolean isServiceAccount = claims.containsKey(CLAIM_CLIENT_ID) || claims.containsKey(CLAIM_AZP)
        || claims.containsKey(CLAIM_SERVICE_NAME) || !claims.containsKey(CLAIM_EMAIL);

src/test/java/com/decathlon/idp_core/domain/service/principal/PrincipalProvisioningServiceTest.java:151

  • Typo in test method name: UnkownUnknown.
  void shouldCreateNewPrincipalOnFirstAuthenticationFromUnkownGroups() {

src/main/java/com/decathlon/idp_core/domain/service/principal/PrincipalProvisioningService.java:144

  • principalInfo.groups() can contain duplicates (depending on IdP claim content). As-is, duplicates can propagate into relation targets and may cause duplicate rows / constraint issues when persisting relations. Deduplicate the identifiers before building the member_of relation.
    List<String> validGroups = entityRepository
        .findAllByTemplateIdentifierAndIdentifierIn("team", principalInfo.groups()).stream()
        .map(Entity::identifier).toList();

src/main/resources/db/migration/V7_1__create_principal_entity_template.sql:7

  • This migration hard-codes the idp_core schema (INSERT INTO idp_core...). Other migrations create/use unqualified table names (relying on Flyway default-schema / connection search_path). Hard-coding the schema here can break environments that configure a different schema.
INSERT INTO idp_core.entity_template (id, identifier, name, description)
SELECT gen_random_uuid(),
       'principal',
       'Principal',
       'Unified identity representing authenticated actors (humans or service accounts) in the IDP-Core catalog' WHERE NOT EXISTS (SELECT 1 FROM idp_core.entity_template WHERE identifier = 'principal');

src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/handler/ApiExceptionHandler.java:31

  • DataIntegrityViolationException is still imported (line 13) but no longer used after the handler removal. Unused imports are compilation errors in Java.
import com.decathlon.idp_core.domain.exception.principal.PrincipalNotFoundException;

Copilot AI review requested due to automatic review settings July 30, 2026 11:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (6)

src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/principal/PrincipalExtractor.java:56

  • Syntax error: missing whitespace between getPrincipal() and instanceof prevents compilation of the OAuth2 principal extraction branch.
    if (authentication.getPrincipal()instanceof OAuth2User oauth2User) {

src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/configuration/SecurityConfiguration.java:61

  • HttpSecurity#build() can throw a checked exception; with the current method signature (no throws), this is likely to break compilation. Either declare throws Exception on the bean method or wrap the build call in a try/catch.
    return http.build();

src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/handler/ApiExceptionHandler.java:545

  • DataIntegrityViolationException is still imported but is no longer handled. This means DB constraint violations will fall back to the generic exception handler (500) instead of returning a conflict (409) with a safe message.

  @ExceptionHandler(EntityDynamicMappingAlreadyInUseException.class)
  public ResponseEntity<ErrorResponse> handleEntityDynamicMappingAlreadyInUseException(

src/main/resources/db/migration/V7_1__create_principal_entity_template.sql:7

  • This migration hardcodes the idp_core. schema in every statement, while existing migrations rely on Flyway’s configured default-schema (unqualified table names). Hardcoding the schema reduces portability across environments where the schema name is configured differently.
INSERT INTO idp_core.entity_template (id, identifier, name, description)
SELECT gen_random_uuid(),
       'principal',
       'Principal',
       'Unified identity representing authenticated actors (humans or service accounts) in the IDP-Core catalog' WHERE NOT EXISTS (SELECT 1 FROM idp_core.entity_template WHERE identifier = 'principal');

src/test/java/com/decathlon/idp_core/TestSecurityConfiguration.java:33

  • The file is named TestSecurityConfiguration.java but now contains SecurityConfigurationTest, which is confusing when navigating tests (and makes grepping for the test harder).
@ExtendWith(MockitoExtension.class)
class SecurityConfigurationTest {

src/test/java/com/decathlon/idp_core/domain/service/principal/PrincipalProvisioningServiceTest.java:147

  • Typo in test name: UnkownUnknown.
  void shouldCreateNewPrincipalOnFirstAuthenticationFromUnkownGroups() {

Copilot AI review requested due to automatic review settings July 30, 2026 12:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (5)

src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/principal/PrincipalExtractor.java:56

  • authentication.getPrincipal()instanceof OAuth2User is missing a space before instanceof, which makes this file fail to compile.
    if (authentication.getPrincipal()instanceof OAuth2User oauth2User) {

src/main/java/com/decathlon/idp_core/domain/service/principal/PrincipalProvisioningService.java:101

  • PrincipalCreationException formats its message using a principal identifier, but this call passes a full sentence. This produces a confusing error message (and hides the real identifier) when the fallback read fails after a concurrent create.
          .orElseThrow(() -> new PrincipalCreationException(
              "Principal concurrent creation detected but subsequent read failed for identifier: "
                  + principalInfo.identifier()));

src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/handler/ApiExceptionHandler.java:544

  • The dedicated DataIntegrityViolationException handler was removed, but the exception is still imported. Without this handler, low-level constraint violations are likely to bubble up as generic 500s (and the import becomes unused). Consider restoring the 409 mapping here.

  @ExceptionHandler(EntityDynamicMappingAlreadyInUseException.class)

src/test/java/com/decathlon/idp_core/domain/service/principal/PrincipalProvisioningServiceTest.java:147

  • Typo in test name: Unkown should be Unknown for readability and searchability.
  void shouldCreateNewPrincipalOnFirstAuthenticationFromUnkownGroups() {

src/main/resources/db/migration/V7_1__create_principal_entity_template.sql:7

  • This migration hard-codes the idp_core schema (e.g., INSERT INTO idp_core.entity_template ...). Other migrations in db/migration/ create/use tables without a schema prefix, and the project guidelines indicate the schema should be provided by configuration. Hard-coding the schema here can break installations that run in a different schema/search_path.
INSERT INTO idp_core.entity_template (id, identifier, name, description)
SELECT gen_random_uuid(),
       'principal',
       'Principal',
       'Unified identity representing authenticated actors (humans or service accounts) in the IDP-Core catalog' WHERE NOT EXISTS (SELECT 1 FROM idp_core.entity_template WHERE identifier = 'principal');

Copilot AI review requested due to automatic review settings July 30, 2026 13:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/principal/PrincipalExtractor.java:56

  • Missing whitespace between getPrincipal() and instanceof makes this line a syntax error, so the project will not compile.
    if (authentication.getPrincipal()instanceof OAuth2User oauth2User) {

src/test/java/com/decathlon/idp_core/domain/service/principal/PrincipalProvisioningServiceTest.java:147

  • Typo in test name: UnkownUnknown.
  void shouldCreateNewPrincipalOnFirstAuthenticationFromUnkownGroups() {

src/main/resources/db/migration/V7_1__create_principal_entity_template.sql:7

  • This migration hardcodes the idp_core. schema prefix and uses gen_random_uuid(). Most existing migrations rely on Flyway's default-schema/search_path (unqualified table names), and gen_random_uuid() requires the pgcrypto extension (no other migration in this repo uses it). This combination makes the migration less portable across environments and more likely to fail if schema/extension setup differs.
INSERT INTO idp_core.entity_template (id, identifier, name, description)
SELECT gen_random_uuid(),
       'principal',
       'Principal',
       'Unified identity representing authenticated actors (humans or service accounts) in the IDP-Core catalog' WHERE NOT EXISTS (SELECT 1 FROM idp_core.entity_template WHERE identifier = 'principal');

src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/handler/ApiExceptionHandler.java:543

  • The explicit DataIntegrityViolationException → 409 mapping was removed, so low-level DB constraint violations will now fall through to the generic Exception handler and return 500. That changes API behavior and makes conflict errors harder for clients to handle.
    return ResponseEntity.status(HttpStatus.CONFLICT).body(errorResponse);
  }

  @ExceptionHandler(EntityDynamicMappingAlreadyInUseException.class)
  public ResponseEntity<ErrorResponse> handleEntityDynamicMappingAlreadyInUseException(

Copilot AI review requested due to automatic review settings July 31, 2026 06:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated 1 comment.

Suppressed comments (5)

src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/principal/PrincipalExtractor.java:58

  • Missing whitespace before instanceof makes this line invalid Java syntax and prevents compilation.
    if (authentication.getPrincipal()instanceof OAuth2User oauth2User) {
      return extractFromOAuth2User(oauth2User);
    }

src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/handler/ApiExceptionHandler.java:545

  • The DataIntegrityViolationException handler was removed but the import remains, which will cause an unused-import compilation error. If the handler removal was unintentional, restoring it here also preserves the previous 409 mapping for DB constraint violations.
  }

  @ExceptionHandler(EntityDynamicMappingAlreadyInUseException.class)
  public ResponseEntity<ErrorResponse> handleEntityDynamicMappingAlreadyInUseException(

src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/principal/PrincipalExtractor.java:71

  • isServiceAccount currently treats any JWT without an email claim as a service account, which can misclassify human tokens that omit email (common when scopes/claims are minimal). Prefer detecting service accounts via explicit client claims (client_id/azp/service_name).
    // Detect if this is a service account token
    boolean isServiceAccount = claims.containsKey(CLAIM_CLIENT_ID) || claims.containsKey(CLAIM_AZP)
        || claims.containsKey(CLAIM_SERVICE_NAME) || !claims.containsKey(CLAIM_EMAIL);

src/main/resources/application.yml:54

  • Enabling Flyway out-of-order globally can lead to unexpected migration ordering in long-lived environments (for example, a late-added Vx_y migration being applied after newer versions). Consider defaulting this to false and enabling it only via env var/profile when needed.
    # Allows migrations to run even if they're detected out of order (common in ephemeral DBs).
    out-of-order: true
    

src/main/resources/db/migration/V7_1__create_principal_entity_template.sql:7

  • This migration hardcodes the idp_core. schema qualifier, while earlier migrations use unqualified table names (relying on the configured default schema/search_path). Hardcoding the schema reduces configurability and can break if the schema name changes.
INSERT INTO idp_core.entity_template (id, identifier, name, description)
SELECT gen_random_uuid(),
       'principal',
       'Principal',
       'Unified identity representing authenticated actors (humans or service accounts) in the IDP-Core catalog' WHERE NOT EXISTS (SELECT 1 FROM idp_core.entity_template WHERE identifier = 'principal');

Copilot AI review requested due to automatic review settings July 31, 2026 06:43
Copilot AI review requested due to automatic review settings July 31, 2026 12:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/principal/PrincipalExtractor.java:59

  • Missing whitespace makes the instanceof pattern match invalid Java syntax, so this file will not compile. Add a space between getPrincipal() and instanceof.
    if (authentication.getPrincipal()instanceof OAuth2User oauth2User) {

src/test/resources/db/test/R__1_Insert_test_data.sql:54

  • This property rule sets both format = 'EMAIL' and a regex. In PropertyDefinitionValidationService these are mutually exclusive, so this seed data can put the DB into an invalid state relative to the domain rules. Prefer using the built-in EMAIL format alone and leave regex NULL.
-- Email validation rule (for principal email)
('550e8400-e29b-41d4-a716-446655440015', 'EMAIL', NULL, '^[A-Za-z0-9+_.-]+@(.+)$', 100, NULL, NULL, NULL);

src/main/resources/application.yml:53

  • spring.flyway.out-of-order: true in the default config can mask accidental migration ordering issues in long-lived environments (and makes it easier to end up with different schema histories across instances). Consider defaulting this to false and enabling it only via an env var (or only in test/local profiles).
    # Allows migrations to run even if they're detected out of order (common in ephemeral DBs).
    out-of-order: true

src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/handler/ApiExceptionHandler.java:352

  • The DataIntegrityViolationException handler appears to have been removed, but the class still imports DataIntegrityViolationException. This will not compile due to an unused import, and it also changes the API behavior (likely turning DB constraint violations into 500s via the generic handler). Either remove the import and accept the behavior change, or reintroduce the 409-mapping handler.
  /// Handles principal not found exception when JIT provisioning fails.
  ///
  /// **HTTP mapping:** Maps PrincipalNotFoundException to HTTP 404 status
  /// indicating the authenticated principal does not have a catalog entry.
  @ExceptionHandler(PrincipalNotFoundException.class)

@RVANDO12

RVANDO12 commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator Author

/run-owasp-zap

@Decathlon Decathlon deleted a comment from github-actions Bot Aug 4, 2026
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

🛡️ OWASP ZAP Scan Findings

  • High Risk: 0
  • Medium Risk: 1
  • Low Risk: 4

📥 Download the complete HTML report from the Workflow Run Artifacts.

Comment thread src/main/resources/application.yml Outdated
Comment thread src/main/resources/application.yml
Comment thread docs/src/concepts/authentication.md Outdated
Comment thread docs/src/concepts/authentication.md
Comment thread docs/src/concepts/authentication.md Outdated
Comment thread docs/src/concepts/authentication.md Outdated
Comment thread docs/src/concepts/authentication.md Outdated
Comment thread docs/src/concepts/authentication.md
Comment thread docs/src/concepts/authentication.md
Comment thread docs/src/concepts/authentication.md Outdated
Comment thread src/main/resources/application.yml Outdated
@etiennej70

Copy link
Copy Markdown
Collaborator

Didn't have the time to fully test the feature. The implementation sounds good to me as a first implementation 👍 I let other team members have a look 🙏

@sonarqubecloud

sonarqubecloud Bot commented Sep 3, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants