feat: common policy-based authorization cedarling module #14521#14522
Open
yurem wants to merge 100 commits into
Open
feat: common policy-based authorization cedarling module #14521#14522yurem wants to merge 100 commits into
yurem wants to merge 100 commits into
Conversation
Signed-off-by: pujavs <pujas.works@gmail.com>
…ns-config-10757
Signed-off-by: pujavs <pujas.works@gmail.com>
…ns-config-10757
Signed-off-by: pujavs <pujas.works@gmail.com>
…ns-config-10757
Signed-off-by: pujavs <pujas.works@gmail.com>
…ns-config-10757
Signed-off-by: pujavs <pujas.works@gmail.com>
…ns-config-10757
…ns-config-10757
…ns-config-10757
Signed-off-by: pujavs <pujas.works@gmail.com>
Signed-off-by: pujavs <pujas.works@gmail.com>
Signed-off-by: pujavs <pujas.works@gmail.com>
Signed-off-by: pujavs <pujas.works@gmail.com>
…ns-config-10757
…ns-config-10757
…ns-config-10757
…ns-config-10757
…ns-config-10757
Signed-off-by: pujavs <pujas.works@gmail.com>
…ns-config-10757
Signed-off-by: pujavs <pujas.works@gmail.com>
Signed-off-by: pujavs <pujas.works@gmail.com>
…ns-config-10757
…ns-config-10757
Signed-off-by: pujavs <pujas.works@gmail.com>
…ns-config-10757
Signed-off-by: pujavs <pujas.works@gmail.com>
Signed-off-by: pujavs <pujas.works@gmail.com>
pujavs
had a problem deploying
to
integration-tests
July 14, 2026 17:16 — with 
GitHub Actions
Failure
GitHub Actions
Failure
pujavs
had a problem deploying
to
integration-tests
July 14, 2026 17:16 — with
GitHub Actions
Failure
GitHub Actions
Failure
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
jans-config-api/server/src/main/java/io/jans/configapi/filters/AuthorizationFilter.java (1)
71-77: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove remaining verbose
INFOlogs.The
log.infostatements on lines 71 and 76 were left behind from previous cleanups. They contain excessive formatting (\n\n\nand======) and generate unnecessary noise on every unauthorized request. Please downgrade them toDEBUGor remove them entirely.♻️ Proposed fix
- log.info("\n\n\n AuthorizationFilter::filter() - Config Api OAuth Valdation Enabled"); if (!isTokenBasedAuthentication(authorizationHeader)) { log.warn("AuthorizationFilter - token-based authorization required for {} {}", context.getMethod(), info.getPath()); abortWithUnauthorized(context, "ONLY TOKEN BASED AUTHORIZATION IS SUPPORTED!"); - log.info("======ONLY TOKEN BASED AUTHORIZATION IS SUPPORTED======================"); return; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@jans-config-api/server/src/main/java/io/jans/configapi/filters/AuthorizationFilter.java` around lines 71 - 77, Remove the two verbose INFO statements in AuthorizationFilter.filter(), or downgrade them to DEBUG; preserve the existing warning and unauthorized response behavior.jans-core/cedarling/src/main/java/io/jans/core/cedarling/service/policy/PolicyDownloadService.java (1)
221-223: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winCatch
RuntimeExceptionto prevent a ZIP bomb exception from breaking the reload process.
validateZip()throws anIllegalStateException(aRuntimeException) when the entry count is exceeded. Since this block only catchesIOException, theIllegalStateExceptionwill propagate up, abort thereloadPolicies()loop (skipping any remaining policy sources), and potentially crash the application if triggered during startup initialization (initTime()).Catch
Exceptionor explicitly handleRuntimeExceptionhere to gracefully log the failure and continue processing other policy sources.🛡️ Proposed fix
- } catch (IOException ex) { + } catch (Exception ex) { log.error("Failed to execute load policies list from URI {}", policyStoreUri, ex); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@jans-core/cedarling/src/main/java/io/jans/core/cedarling/service/policy/PolicyDownloadService.java` around lines 221 - 223, Update the exception handling in the policy-loading block of PolicyDownloadService so RuntimeException from validateZip(), including IllegalStateException, is caught alongside IOException. Log the failure through the existing logger and allow reloadPolicies() to continue processing remaining policy sources.
♻️ Duplicate comments (3)
jans-config-api/server/src/main/java/io/jans/configapi/security/service/CedarAuthorizationService.java (1)
54-56: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winWrong argument passed as
methodtocedarlingService.authorize.Line 55 passes
resourceInfo.toString()as the 4th argument, butCedarlingService.authorizeexpects the HTTP method string there. Themethodparameter is already available inprocessAuthorization's signature (line 44) but is never used. This means Cedarling receives theResourceInfoobject'stoString()representation instead of the actual HTTP method (e.g.,"GET","POST"), causing authorization decisions to be evaluated against incorrect input.🐛 Proposed fix
- boolean isAuthorized = cedarlingService.authorize(token, issuer, resourceInfo, resourceInfo.toString(), path); + boolean isAuthorized = cedarlingService.authorize(token, issuer, resourceInfo, method, path);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@jans-config-api/server/src/main/java/io/jans/configapi/security/service/CedarAuthorizationService.java` around lines 54 - 56, Update the authorize call in processAuthorization to pass its existing method parameter as the fourth argument instead of resourceInfo.toString(), preserving the remaining arguments and authorization flow.jans-core/cedarling/src/test/java/io/jans/core/cedarling/service/CedarlingProtectionServiceTest.java (1)
531-546: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not expose internal exception messages in HTTP responses.
This test locks in disclosure of arbitrary adapter, parser, or infrastructure errors to unauthenticated clients. Assert a generic error body instead and keep the detailed exception in server logs. Ensure the corresponding
processAuthorizationerror handling returns a generic response to clients.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@jans-core/cedarling/src/test/java/io/jans/core/cedarling/service/CedarlingProtectionServiceTest.java` around lines 531 - 546, Update unexpectedException_returns500 and the corresponding processAuthorization error handling so internal exception messages are not returned in HTTP responses. Assert a generic error body in the test while preserving detailed exception logging on the server.jans-config-api/server/src/main/java/io/jans/configapi/configuration/AppInitializer.java (1)
209-211: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winConflicting CDI scopes on producer method.
The
getCedarlingConfiguration()producer method is annotated with both@Dependentand@ApplicationScoped. A bean or producer must define exactly one scope. To ensure the nullable configuration does not cause CDI proxy creation errors (which was the reason for adding@Dependent), remove the@ApplicationScopedannotation.♻️ Proposed fix
`@Produces` `@Dependent` - `@ApplicationScoped` public CedarlingConfiguration getCedarlingConfiguration() {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@jans-config-api/server/src/main/java/io/jans/configapi/configuration/AppInitializer.java` around lines 209 - 211, Remove the conflicting `@ApplicationScoped` annotation from the getCedarlingConfiguration() producer method, retaining `@Dependent` so nullable configuration values do not require CDI proxy creation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@jans-config-api/server/src/main/java/io/jans/configapi/filters/AuthorizationFilter.java`:
- Around line 71-77: Remove the two verbose INFO statements in
AuthorizationFilter.filter(), or downgrade them to DEBUG; preserve the existing
warning and unauthorized response behavior.
In
`@jans-core/cedarling/src/main/java/io/jans/core/cedarling/service/policy/PolicyDownloadService.java`:
- Around line 221-223: Update the exception handling in the policy-loading block
of PolicyDownloadService so RuntimeException from validateZip(), including
IllegalStateException, is caught alongside IOException. Log the failure through
the existing logger and allow reloadPolicies() to continue processing remaining
policy sources.
---
Duplicate comments:
In
`@jans-config-api/server/src/main/java/io/jans/configapi/configuration/AppInitializer.java`:
- Around line 209-211: Remove the conflicting `@ApplicationScoped` annotation from
the getCedarlingConfiguration() producer method, retaining `@Dependent` so
nullable configuration values do not require CDI proxy creation.
In
`@jans-config-api/server/src/main/java/io/jans/configapi/security/service/CedarAuthorizationService.java`:
- Around line 54-56: Update the authorize call in processAuthorization to pass
its existing method parameter as the fourth argument instead of
resourceInfo.toString(), preserving the remaining arguments and authorization
flow.
In
`@jans-core/cedarling/src/test/java/io/jans/core/cedarling/service/CedarlingProtectionServiceTest.java`:
- Around line 531-546: Update unexpectedException_returns500 and the
corresponding processAuthorization error handling so internal exception messages
are not returned in HTTP responses. Assert a generic error body in the test
while preserving detailed exception logging on the server.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cb73af84-16b3-4aa5-8d8f-c722bf98a90d
📒 Files selected for processing (15)
jans-config-api/docs/jans-config-api-swagger.yamljans-config-api/plugins/docs/fido2-plugin-swagger.yamljans-config-api/profiles/default/config-api-test.propertiesjans-config-api/profiles/jans-ui.jans.io/test.propertiesjans-config-api/profiles/local/test.propertiesjans-config-api/server/src/main/java/io/jans/configapi/configuration/AppInitializer.javajans-config-api/server/src/main/java/io/jans/configapi/filters/AuthorizationFilter.javajans-config-api/server/src/main/java/io/jans/configapi/security/service/CedarAuthorizationService.javajans-core/cedarling/src/main/java/io/jans/core/cedarling/model/CedarlingConfiguration.javajans-core/cedarling/src/main/java/io/jans/core/cedarling/model/PolicySource.javajans-core/cedarling/src/main/java/io/jans/core/cedarling/service/filter/CedarlingAuthorizationProcessingFilter.javajans-core/cedarling/src/main/java/io/jans/core/cedarling/service/policy/PolicyDownloadService.javajans-core/cedarling/src/test/java/io/jans/core/cedarling/service/CedarlingAuthorizationServiceIntegrationTest.javajans-core/cedarling/src/test/java/io/jans/core/cedarling/service/CedarlingProtectionServiceTest.javajans-linux-setup/jans_setup/templates/jans-config-api/dynamic-conf.json
💤 Files with no reviewable changes (1)
- jans-core/cedarling/src/main/java/io/jans/core/cedarling/service/filter/CedarlingAuthorizationProcessingFilter.java
…ns-config-10757
pujavs
had a problem deploying
to
integration-tests
July 15, 2026 10:49 — with
GitHub Actions
Failure
GitHub Actions
Failure
pujavs
had a problem deploying
to
integration-tests
July 15, 2026 10:49 — with
GitHub Actions
Failure
GitHub Actions
Failure
Signed-off-by: pujavs <pujas.works@gmail.com>
coderabbitai
Bot
added
comp-jans-fido2
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@jans-config-api/server/src/main/java/io/jans/configapi/service/cedar/CedarlingService.java`:
- Around line 31-32: Update the context construction in CedarlingService to
avoid Map.of for potentially null method or issuer values by using a
null-tolerant mutable map, and convert
resourceInfo.getResourceMethod().getAnnotations() into a serializable list of
annotation string representations such as simple class names before storing it
under the existing mapping keys.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5a204654-4b08-4388-907d-aafea347bcfb
📒 Files selected for processing (4)
jans-config-api/docs/jans-config-api-swagger.yamljans-config-api/plugins/docs/fido2-plugin-swagger.yamljans-config-api/server/src/main/java/io/jans/configapi/security/service/CedarAuthorizationService.javajans-config-api/server/src/main/java/io/jans/configapi/service/cedar/CedarlingService.java
Contributor
Signed-off-by: pujavs <pujas.works@gmail.com>
pujavs
had a problem deploying
to
integration-tests
July 15, 2026 12:37 — with
GitHub Actions
Failure
GitHub Actions
Failure
pujavs
had a problem deploying
to
integration-tests
July 15, 2026 12:37 — with
GitHub Actions
Failure
GitHub Actions
Failure
Contributor
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
coderabbitai
Bot
removed
comp-jans-orm
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@jans-config-api/server/src/main/java/io/jans/configapi/service/cedar/CedarlingService.java`:
- Around line 32-42: Move the cedar_entity_mapping entry from context into the
resource payload used by CedarlingService, while keeping the existing method,
action, and issuer mapping unchanged. Leave the context argument empty when
invoking CedarlingAuthorizationService so its expected payload shape is
preserved.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 56d3ec24-6b30-4c11-830a-e37da5d621fe
📒 Files selected for processing (3)
jans-config-api/docs/jans-config-api-swagger.yamljans-config-api/plugins/docs/fido2-plugin-swagger.yamljans-config-api/server/src/main/java/io/jans/configapi/service/cedar/CedarlingService.java
Signed-off-by: pujavs <pujas.works@gmail.com>
pujavs
had a problem deploying
to
integration-tests
July 15, 2026 13:01 — with
GitHub Actions
Failure
GitHub Actions
Failure
pujavs
had a problem deploying
to
integration-tests
July 15, 2026 13:01 — with
GitHub Actions
Failure
GitHub Actions
Failure
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@jans-config-api/server/src/main/java/io/jans/configapi/service/cedar/CedarlingService.java`:
- Around line 27-28: Update the tokens map in CedarlingService to use
CedarlingAuthorizationService.CEDARLING_JANS_ACCESS_TOKEN instead of the literal
"ACCESS_TOKEN", while preserving the existing token value and authorization
flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e6efa8d6-c2ac-4240-9c4e-4d5c793f6a13
📒 Files selected for processing (3)
jans-config-api/docs/jans-config-api-swagger.yamljans-config-api/plugins/docs/fido2-plugin-swagger.yamljans-config-api/server/src/main/java/io/jans/configapi/service/cedar/CedarlingService.java
Signed-off-by: pujavs <pujas.works@gmail.com>
pujavs
had a problem deploying
to
integration-tests
July 18, 2026 11:32 — with
GitHub Actions
Failure
GitHub Actions
Failure
pujavs
had a problem deploying
to
integration-tests
July 18, 2026 11:32 — with
GitHub Actions
Failure
GitHub Actions
Failure
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Prepare
Description
Added common cedar code in jans-core.
Target issue
closes #14521
A new cedarling module that can be integrated into backend Java services for policy-based authorization
Implementation Details
Test and Document the changes
Please check the below before submitting your PR. The PR will not be merged if there are no commits that start with
docs:to indicate documentation changes or if the below checklist is not selected.Closes #14523,
Summary by CodeRabbit
protectionModesupporting Cedarling and configurablecedarlingConfiguration.