feat(core): add jit principal generation - #105
Conversation
There was a problem hiding this comment.
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/mecontroller endpoint. - Introduced
principal/teamtemplate 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
emailclaim as a service account (|| !claims.containsKey(CLAIM_EMAIL)). Human tokens may legitimately omitemail, which would misclassify them and persist the wrongkindproperty.
boolean isServiceAccount = claims.containsKey(CLAIM_CLIENT_ID) || claims.containsKey(CLAIM_AZP)
|| claims.containsKey(CLAIM_SERVICE_NAME) || !claims.containsKey(CLAIM_EMAIL);
Code Coverage OverviewLanguages: Java Java / code-coverage/jacocoThe overall line coverage in commit cb6b2aa in the Show a line coverage summary of the most impacted files.
Updated |
There was a problem hiding this comment.
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 usespreferred_usernamewhen present (as verified by thealicetest earlier in this class). Withpreferred_username = "david", the expected identifier should bedavid.
// 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
emailclaim as a service account (|| !claims.containsKey("email")). This misclassifies human tokens that omitemail(and contradicts the extractor contract that relies primarily onclient_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:
Unkown→Unknown.
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 themember_ofrelation.
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_coreschema (INSERT INTO idp_core...). Other migrations create/use unqualified table names (relying on Flywaydefault-schema/ connectionsearch_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
DataIntegrityViolationExceptionis 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;
There was a problem hiding this comment.
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()andinstanceofprevents 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 (nothrows), this is likely to break compilation. Either declarethrows Exceptionon 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
DataIntegrityViolationExceptionis 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 configureddefault-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.javabut now containsSecurityConfigurationTest, 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:
Unkown→Unknown.
void shouldCreateNewPrincipalOnFirstAuthenticationFromUnkownGroups() {
There was a problem hiding this comment.
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 OAuth2Useris missing a space beforeinstanceof, 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
PrincipalCreationExceptionformats 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
DataIntegrityViolationExceptionhandler 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:
Unkownshould beUnknownfor readability and searchability.
void shouldCreateNewPrincipalOnFirstAuthenticationFromUnkownGroups() {
src/main/resources/db/migration/V7_1__create_principal_entity_template.sql:7
- This migration hard-codes the
idp_coreschema (e.g.,INSERT INTO idp_core.entity_template ...). Other migrations indb/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');
There was a problem hiding this comment.
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()andinstanceofmakes 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:
Unkown→Unknown.
void shouldCreateNewPrincipalOnFirstAuthenticationFromUnkownGroups() {
src/main/resources/db/migration/V7_1__create_principal_entity_template.sql:7
- This migration hardcodes the
idp_core.schema prefix and usesgen_random_uuid(). Most existing migrations rely on Flyway'sdefault-schema/search_path(unqualified table names), andgen_random_uuid()requires thepgcryptoextension (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 genericExceptionhandler 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(
There was a problem hiding this comment.
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
instanceofmakes 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
DataIntegrityViolationExceptionhandler 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
isServiceAccountcurrently treats any JWT without anemailclaim 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-orderglobally 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');
There was a problem hiding this comment.
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
instanceofpattern match invalid Java syntax, so this file will not compile. Add a space betweengetPrincipal()andinstanceof.
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 aregex. InPropertyDefinitionValidationServicethese 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 leaveregexNULL.
-- 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: truein 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
DataIntegrityViolationExceptionhandler appears to have been removed, but the class still importsDataIntegrityViolationException. 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)
|
/run-owasp-zap |
🛡️ OWASP ZAP Scan Findings
📥 Download the complete HTML report from the Workflow Run Artifacts. |
|
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 🙏 |
Signed-off-by: renny vandomber <renny.vandomber@decathlon.com>
|



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
What This PR Provides
Configuration
Default JWT configuration:
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:
These integration tests require Docker because the test suite uses Testcontainers.
Run the complete verification:
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:
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:
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:
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.