Conversation
Code Coverage OverviewLanguages: Java Java / code-coverage/jacocoThe overall line coverage in commit 5d895a8 in the Show a line coverage summary of the most impacted files.
Updated |
|
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved authorization bypasses can grant service accounts unintended privileges, including platform-admin access.
Review effort: Balanced
Findings: 3
Open (7)
Service name claim alone grants service-account write privileges · New Service account can self-promote via ingestion mapping and connector updates · New Protected principal template creation bypasses authorization · New Missing identifier claim causes server error instead of 401 · New Authorization denials omit standard API error response fields · New Mock authentication fails when configured identifier claim differs · New Local profile overrides environment-configured identifier claim · New
What changed in this PR
Introduces Phase 1 global authorization for IDP-Core’s catalog APIs, replacing permissive baseline access with principal-based policies.
Changes:
- Adds authorization policies, controller resource metadata, and security-chain enforcement.
- Configures human identifiers and provisions principals without admin privileges.
- Updates security tests, integration fixtures, and authentication documentation.
| File | Description |
|---|---|
src/test/resources/db/test/R__6_insert_principal_user_for_test.sql |
Adds principal fixtures for authorization tests. |
src/test/resources/db/test/R__1_Insert_test_data.sql |
Adds an optional access-gate property. |
src/test/resources/application-test.yml |
Removes wildcard baseline configuration. |
src/test/java/com/decathlon/idp_core/SecurityConfigurationTest.java |
Checks removal of universal authority. |
src/test/java/com/decathlon/idp_core/infrastructure/adapters/api/handler/ApiExceptionHandlerTest.java |
Tests authorization exception mapping. |
src/test/java/com/decathlon/idp_core/infrastructure/adapters/api/controller/PrincipalControllerTest.java |
Tests standard human read access. |
src/test/java/com/decathlon/idp_core/infrastructure/adapters/api/controller/InboundWebhookManagementControllerTest.java |
Tests denied human webhook creation. |
src/test/java/com/decathlon/idp_core/infrastructure/adapters/api/controller/EntityTemplateControllerTest.java |
Seeds authorized principals for integration tests. |
src/test/java/com/decathlon/idp_core/infrastructure/adapters/api/controller/EntityDynamicMappingControllerTest.java |
Adds principal fixtures and adjusts payload escaping. |
src/test/java/com/decathlon/idp_core/infrastructure/adapters/api/controller/AuditControllerTest.java |
Uses admin principals for lifecycle tests. |
src/test/java/com/decathlon/idp_core/infrastructure/adapters/api/configuration/SecurityRolePropertiesTest.java |
Deletes obsolete baseline-role tests. |
src/test/java/com/decathlon/idp_core/infrastructure/adapters/api/configuration/security/chains/PublicFilterChainConfigTest.java |
Updates authentication configuration construction. |
src/test/java/com/decathlon/idp_core/infrastructure/adapters/api/configuration/security/chains/OAuth2LoginFilterChainConfigTest.java |
Checks authorization filter placement. |
src/test/java/com/decathlon/idp_core/infrastructure/adapters/api/configuration/security/chains/MockFilterChainConfigTest.java |
Checks mock-chain authorization wiring. |
src/test/java/com/decathlon/idp_core/infrastructure/adapters/api/configuration/security/chains/JwtFilterChainConfigTest.java |
Checks JWT-chain authorization wiring. |
src/test/java/com/decathlon/idp_core/infrastructure/adapters/api/configuration/AuthorizationConfigurationTest.java |
Tests policy binding and filter registration. |
src/test/java/com/decathlon/idp_core/infrastructure/adapters/api/configuration/AuthenticationPropertiesTest.java |
Tests identifier-claim configuration. |
src/test/java/com/decathlon/idp_core/infrastructure/adapters/api/auth/UnifiedUserProviderTest.java |
Tests shared identity extraction. |
src/test/java/com/decathlon/idp_core/infrastructure/adapters/api/auth/TestHandlerMappings.java |
Supplies controller mappings for authorization tests. |
src/test/java/com/decathlon/idp_core/infrastructure/adapters/api/auth/JitProvisioningFilterTest.java |
Checks request-scoped principal sharing. |
src/test/java/com/decathlon/idp_core/infrastructure/adapters/api/auth/GlobalAuthorizationFilterTest.java |
Tests authorization filtering and denial behavior. |
src/test/java/com/decathlon/idp_core/infrastructure/adapters/api/auth/AuthorizationRequestFactoryTest.java |
Tests route-derived actions and resources. |
src/test/java/com/decathlon/idp_core/domain/service/principal/PrincipalProvisioningServiceTest.java |
Checks non-admin provisioning defaults. |
src/test/java/com/decathlon/idp_core/domain/service/principal/PrincipalExtractorTest.java |
Tests configured human identifier extraction. |
src/test/java/com/decathlon/idp_core/domain/service/authorization/GlobalAuthorizationServiceTest.java |
Tests the global authorization matrix. |
src/main/resources/application.yml |
Adds authorization and identifier settings. |
src/main/resources/application-local.yml |
Selects GitHub’s identifier claim locally. |
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/principal/strategies/OAuth2UserPrincipalExtractionStrategy.java |
Uses configured OAuth2/OIDC identifiers. |
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/principal/strategies/JwtPrincipalExtractionStrategy.java |
Uses configured human JWT identifiers. |
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/handler/ApiExceptionHandler.java |
Maps authorization exceptions to 403. |
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/controller/PrincipalController.java |
Declares the principal resource. |
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/controller/InboundWebhookConfigurationController.java |
Declares webhook configuration resources. |
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/controller/EntityTemplateController.java |
Declares template resources. |
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/controller/EntityGraphController.java |
Declares entity graph resources. |
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/controller/EntityDynamicMappingController.java |
Marks mapping resources and read-only dry runs. |
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/controller/EntityController.java |
Marks entity resources and read-only searches. |
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/controller/AuditController.java |
Declares audit resources. |
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/configuration/SecurityRoleProperties.java |
Deletes baseline-role configuration. |
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/configuration/SecurityConfiguration.java |
Removes wildcard authority assignment. |
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/configuration/security/chains/OAuth2LoginFilterChainConfig.java |
Enforces authorization after provisioning. |
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/configuration/security/chains/MockFilterChainConfig.java |
Adds mock-chain authorization enforcement. |
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/configuration/security/chains/JwtFilterChainConfig.java |
Adds JWT-chain authorization enforcement. |
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/configuration/AuthorizationProperties.java |
Binds external authorization settings. |
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/configuration/AuthorizationConfiguration.java |
Builds the domain policy and controls registration. |
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/configuration/AuthenticationProperties.java |
Adds the human identifier-claim setting. |
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/auth/UnifiedUserProvider.java |
Aligns audit identities with principal extraction. |
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/auth/ProvisionedPrincipalContext.java |
Shares provisioned principals within requests. |
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/auth/JitProvisioningFilter.java |
Exposes provisioning results to authorization. |
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/auth/GlobalAuthorizationFilter.java |
Enforces the global policy on authenticated requests. |
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/auth/AuthorizedResource.java |
Defines controller resource metadata. |
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/auth/AuthorizationRequestFactory.java |
Derives authorization context from MVC mappings. |
src/main/java/com/decathlon/idp_core/domain/service/principal/PrincipalProvisioningService.java |
Defaults new principals to non-admin. |
src/main/java/com/decathlon/idp_core/domain/service/authorization/GlobalAuthorizationService.java |
Implements global access decisions. |
src/main/java/com/decathlon/idp_core/domain/model/authorization/AuthorizationResource.java |
Models resource identifiers and parent context. |
src/main/java/com/decathlon/idp_core/domain/model/authorization/AuthorizationRequest.java |
Models principal, action, and resource input. |
src/main/java/com/decathlon/idp_core/domain/model/authorization/AuthorizationPolicy.java |
Stores immutable authorization policy settings. |
src/main/java/com/decathlon/idp_core/domain/model/authorization/AuthorizationMode.java |
Defines current and future policy modes. |
src/main/java/com/decathlon/idp_core/domain/model/authorization/AuthorizationAction.java |
Defines supported authorization actions. |
src/main/java/com/decathlon/idp_core/domain/exception/authorization/PrincipalNotAuthorizedException.java |
Represents denied domain operations. |
docs/src/concepts/authentication.md |
Documents global policies and identifier configuration. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| throw new PrincipalNotAuthorizedException(identifier); | ||
| } | ||
|
|
||
| if (request.principal().kind() == PrincipalKind.SERVICE_ACCOUNT) { |
| if (!isPlatformAdminOnlyCreation(request)) { | ||
| return; |
| || (ENTITY_TEMPLATE_RESOURCE.equals(resource.type()) | ||
| && resource.identifier().filter(PRINCIPAL_TEMPLATE_IDENTIFIER::equals).isPresent()); |
| var principal = principalContext != null | ||
| ? principalContext.principal() | ||
| : principalExtractor.extractPrincipalInfo(authentication); |
| } catch (PrincipalNotAuthorizedException _) { | ||
| log.warn("Authorization denied for principal {} on {} {}", principal.identifier(), | ||
| request.getMethod(), request.getRequestURI()); | ||
| response.sendError(HttpServletResponse.SC_FORBIDDEN); |
| .addFilterAfter(jitProvisioningFilter, MockJwtAuthenticationFilter.class); | ||
| .addFilterAfter(jitProvisioningFilter, MockJwtAuthenticationFilter.class) | ||
| // 3. Apply the global authorization policy after the principal is provisioned | ||
| .addFilterAfter(globalAuthorizationFilter, JitProvisioningFilter.class); |
| enabled: true | ||
| mock: | ||
| enabled: false | ||
| principal-identifier-claim: id |





PR Description
Title:
feat(security)!: enforce global authorization for catalog APIsWhat this PR Provides
GLOBALauthorization to the JWT, OAuth2 login, and mock API security chains.principalentity hasis_admin: true.truefor access to protected API endpoints, including reads. When unset, the gate is disabled.principaltemplate through the regular API.is_adminand the configured human-access property.is_admin: false; JIT does not grant admin or access-gate status from token attributes, and it does not overwrite existing principal data.IDP_PRINCIPAL_IDENTIFIER_CLAIMdefaults tosub; service accounts continue to useclient_id, thenazp, thensub.@AuthorizedResourceand resolved from Spring MVC's matched handler and path variables, so the security layer does not parse request URLs. A request that matches no annotated handler gets the resource typeunknown.Fixes
N/A — no issue number was provided.
Review
The reviewer must double-check these points:
!after the type/scope to identify the breaking change in the release note and ensure we will release a major version.How to test
Initial state
principaltemplate with a Booleanis_adminproperty.sub; to use a different claim, setIDP_PRINCIPAL_IDENTIFIER_CLAIMto its name, such asuuid.IDP_PLATFORM_ADMIN_IDENTIFIERorIDP_SUPER_ADMIN_IDENTIFIERwith the exact value of the selected human identifier claim for a break-glass admin. These identifiers are checked even when JIT has not created a principal entity; use immutable, globally unique values.IDP_REQUIRED_PRINCIPAL_PROPERTYto a property name, such asis_idp_user. Add that optional Boolean property to theprincipaltemplate and configure a trusted ingestion mapping to populate it. If the variable is unset, the additional gate is disabled.Below is an example using
is_idp_useras a chosen property name, not a default. The gate is disabled unlessIDP_REQUIRED_PRINCIPAL_PROPERTYis set.1. Configure the gate
Base configuration:
To enable it, before starting the app:
Before enabling the gate, add
is_idp_useras an optional Boolean property on theprincipaltemplate, then configure the trusted ingestion webhook mapping to populate it. There is no production migration for this optional property.To disable the gate, leave
IDP_REQUIRED_PRINCIPAL_PROPERTYunset or set it to an empty value.2. Test access with curl
Set a human token:
Assuming the
web-servicetemplate exists, request its entities:200 OKfor the normal human read-only policyis_idp_user: true200 OKfalse, or invalid403 Forbiddenis_admin: trueor is on the break-glass allow-list200 OK, regardless of the gate propertyThe principal’s access property comes from the catalog entity, so after ingestion updates it, use a newly authenticated request to observe the change.
With the gate enabled, a regular human still cannot write, even when the property is
true:Expected result:
403 Forbidden. A platform admin or break-glass principal can perform writes.3. Update a principal for a local test
For manual testing, an authorized platform admin can set the property using the entity API, provided it has been added to the
principaltemplate:Expected result:
200 OK. ThisPUTreplaces the entity’s properties and relations, so preserve any existing values you need. In production, use the trusted ingestion mapping as the source of this property rather than relying on a manual update.Authorization scenarios
is_admin: trueon another principal entity. Authenticate as that principal and confirm it receives platform-wide CRUD access.IDP_REQUIRED_PRINCIPAL_PROPERTYunset. Confirm reads are allowed and catalog mutations are denied.IDP_REQUIRED_PRINCIPAL_PROPERTYto a Boolean property name. Confirm a human with the property set totruecan read, while a missing orfalseproperty results in denial, including for read endpoints. Confirm anis_adminprincipal and a break-glass principal bypass the gate.POST /api/v1/inbound_webhooks. Confirm webhook delivery continues to use connector-level security independently and trusted mappings can update principal properties.IDP_PRINCIPAL_IDENTIFIER_CLAIM=uuidand authenticate with a token containing a uniqueuuidclaim. Confirm the principal is provisioned under that value. Confirm authentication does not proceed successfully when the configured claim is absent.Automated validation
The focused authorization, configuration, filter, and principal-provisioning tests pass (34 tests).
AuthorizationRequestFactoryTestandGlobalAuthorizationFilterTestexercise request mapping against the annotated controllers, including read-only POST endpoints, named path variables, and unmatched routes. The Testcontainers-backed integration tests could not start in the previous environment because Docker was unavailable.Markdown lint passes. The strict documentation build is blocked by an existing broken link in
docs/src/contributing/adrs/_template.mdto0005-example.md.Breaking changes
IDP_REQUIRED_PRINCIPAL_PROPERTYis set, humans need the named principal property set totrueto access protected API endpoints. When unset, this extra condition is disabled.is_adminproperty is not carried over automatically.IDP_SUPER_ADMIN_IDENTIFIERandIDP_PLATFORM_ADMIN_IDENTIFIER; deployments must provide values matching the resulting principal identifiers.Context of the Breaking Change
The previous permissive baseline did not enforce the Phase 1 global authorization matrix. Human identifier extraction also depended on implicit claim behavior and included optional historical-principal migration support. This PR removes those behaviors and makes the chosen human identifier claim explicit.
Result of the Breaking Change
Humans can read catalog data but cannot mutate it unless their principal entity has
is_admin: trueor their identifier is configured for break-glass access. If an administrator configures the optional human-access gate, a missing or false configured property also blocks access to protected APIs. Applications must configure the identifier claim and break-glass values to match their IdP tokens. Existing principals are not automatically migrated when the identifier changes.