Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(jans-cedarling): handle recoverable errors gracefully in authorization #10589

Open
rmarinn opened this issue Jan 10, 2025 · 1 comment · May be fixed by #10721
Open

refactor(jans-cedarling): handle recoverable errors gracefully in authorization #10589

rmarinn opened this issue Jan 10, 2025 · 1 comment · May be fixed by #10721
Assignees
Labels
comp-jans-cedarling Touching folder /jans-cedarling enhancement kind-enhancement Issue or PR is an enhancement to an existing functionality
Milestone

Comments

@rmarinn
Copy link
Contributor

rmarinn commented Jan 10, 2025

Is your feature request related to a problem? Please describe.

Currently, Cedarling returns an error for scenarios like failing to create certain entities or parsing a request, even if these errors are recoverable. While this approach provides clarity in Rust (via explicit error handling), it complicates error management in bindings for other languages where error handling may not be as robust or idiomatic.

Describe the solution you'd like

Update the authorize function to handle recoverable errors gracefully. Instead of returning an error, the function should:

  1. Return a DENY decision in cases where recoverable errors occur.
  2. Provide detailed feedback about the decision through structured fields:
    • Principal Name: Indicates the entity being authorized.
    • Decision Result: ALLOW or DENY.
    • Warnings: A list of messages for recoverable issues, e.g., failure to create a secondary entity like a Role that doesn’t block the primary authorization.
    • Errors: A list of critical issues that directly result in a DENY decision, e.g., failing to create the primary User entity.

An example for a serialized ALLOW result might be:

{
    "results": [{"principal": "Jans::Workload", "decision": "ALLOW"}],
    "decision": "ALLOW",
}

Then for a DENY result:

{
    "results": [{"principal": "Jans::User", "decision": "DENY"}],
    "decision": "DENY",
    "errors":  ["could not create User entity: could not get attribute value from payload: type mismatch for key 'email' expected: 'string' but found: 'number'"],
}

And for an ALLOW but with a warning:

{
    "results": [{"principal": "Jans::User", "decision": "DENY"}],
    "decision": "DENY",
    "warnings":  ["could not create Role entity: could not get attribute value from payload: type mismatch for key 'role' expected: 'string' but found: 'number'"],    
}

Handling Principal Combinations

To be able, to handle returning a decision result that is based on the combination of the authz result of different principals, we also return each of their results.

{
    "decisions": [
        {"principal": "Jans::User", "decision": "DENY"}, 
        {"principal": "Jans::Workload", "decision": "ALLOW"}
    ],
    "decision": "DENY",
    "principal_operator": { /* JSON logic here */ },
    "warnings":  ["could not create Role entity: could not get attribute value from payload: type mismatch for key 'role' expected: 'string' but found: 'number'"],    
}

Describe alternatives you've considered

An alternative approach would be to retain error returns for critical failures while ensuring that recoverable issues generate structured warnings or messages. However, Cedarling currently does not encounter critical errors that justify halting entirely.

EDIT: We now identified a potentially non-recoverable error (the logger failing).

Additional context

Here’s the updated function signature stays the same in Rust but the Error should now only be non-recoverable errors.

impl Cedarling {
    pub fn authorize(&self, request: Request) -> Result<AuthorizeResult, Error> {
        // ...
    }
} 

See #10590 for the proposed improvement on the "principal_operator".

@rmarinn rmarinn assigned moabu and unassigned moabu Jan 10, 2025
@mo-auto mo-auto added comp-jans-cedarling Touching folder /jans-cedarling kind-enhancement Issue or PR is an enhancement to an existing functionality labels Jan 10, 2025
@nynymike
Copy link
Contributor

Completely agree...

@rmarinn rmarinn changed the title refactor(jans-cedarling): handle reoverable errors gracefully in authorization refactor(jans-cedarling): handle recoverable errors gracefully in authorization Jan 10, 2025
@moabu moabu added this to the next release milestone Jan 13, 2025
@moabu moabu modified the milestones: 1.3.0, next-release Jan 21, 2025
@rmarinn rmarinn self-assigned this Jan 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp-jans-cedarling Touching folder /jans-cedarling enhancement kind-enhancement Issue or PR is an enhancement to an existing functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants