Skip to content

feat(jans-cedarling): implement mapping JWT payload to cedar-policy entity #10169

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

Merged
merged 22 commits into from
Nov 18, 2024

Conversation

olehbozhok
Copy link
Contributor

@olehbozhok olehbozhok commented Nov 18, 2024

Prepare


Description

Add mapping custom types from cedar-policy schema

Target issue

link

closes #10169

Implementation Details

Implemented mapping custom types from cedar-policy schema
Mapping can be used only if JWT token is related to trusted issuer and has field iss

On using regex mapping if in group found empty string is used default value instead.

Mapping types can be only:
String
Number
Boolean

also added parsing userinfo_token entity. (it was missed)


Test and Document the changes

  • Static code analysis has been run locally and issues have been fixed
  • Relevant unit and integration tests have been added/updated
  • Relevant documentation has been updated if any (i.e. user guides, installation and configuration guides, technical design docs etc)

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.

  • I confirm that there is no impact on the docs due to the code changes in this PR.

Closes #10170,

…tityMetadata

Signed-off-by: Oleh Bohzok <olehbozhok@gmail.com>
…n deserializing

Signed-off-by: Oleh Bohzok <olehbozhok@gmail.com>
…schema

Signed-off-by: Oleh Bohzok <olehbozhok@gmail.com>
…hema

and prepare to `TokenPayload` mapping

Signed-off-by: Oleh Bohzok <olehbozhok@gmail.com>
…sion` and `get_record_expression`

Signed-off-by: Oleh Bohzok <olehbozhok@gmail.com>
Signed-off-by: Oleh Bohzok <olehbozhok@gmail.com>
Signed-off-by: Oleh Bohzok <olehbozhok@gmail.com>
…` method

Signed-off-by: Oleh Bohzok <olehbozhok@gmail.com>
Signed-off-by: Oleh Bohzok <olehbozhok@gmail.com>
… entities

Signed-off-by: Oleh Bohzok <olehbozhok@gmail.com>
Signed-off-by: Oleh Bohzok <olehbozhok@gmail.com>
…es to policies in PolicyStore

to be more consistent with documentation

Signed-off-by: Oleh Bohzok <olehbozhok@gmail.com>
test case `success_test_user_data_in_id_token`

Signed-off-by: Oleh Bohzok <olehbozhok@gmail.com>
…r-policy` types in schema

also this commit consist fixes

Signed-off-by: Oleh Bohzok <olehbozhok@gmail.com>
…aling-issue-10151

Signed-off-by: Oleh Bohzok <olehbozhok@gmail.com>
@olehbozhok olehbozhok self-assigned this Nov 18, 2024
Copy link

dryrunsecurity bot commented Nov 18, 2024

DryRun Security Summary

The pull request covers a wide range of updates to the Cedarling application, focusing on improving the security and flexibility of the authorization and policy management system, including enhancements to token handling, claim mapping, error handling, and policy management.

Expand for full summary

Summary:

The code changes in this pull request cover a wide range of updates to the Cedarling application, focusing on various aspects of the authorization and policy management system. The changes include improvements to error handling, token metadata handling, claim mapping, and the introduction of new entity types, such as the Userinfo_token. These changes aim to enhance the security and flexibility of the authorization mechanisms within the Cedarling application.

From an application security perspective, the key points to highlight are:

  1. Token Handling: The changes demonstrate a strong focus on securely handling and validating various types of tokens, including access tokens, ID tokens, and userinfo tokens. This is crucial for ensuring the integrity of the authentication and authorization processes.

  2. Claim Mapping and Entity Creation: The improvements to claim mapping and the creation of entities based on token metadata indicate a robust approach to mapping user and role information from the tokens to the application's internal data structures. This helps to enforce appropriate access controls and authorization decisions.

  3. Error Handling and Logging: The code changes include enhancements to error handling and logging, which is an important aspect of maintaining the overall security and reliability of the application.

  4. Policy Management: The updates to the policy store configuration files show that the Cedarling application is using a policy-driven approach to authorization, which can provide a more flexible and maintainable security architecture.

Files Changed:

  • jans-cedarling/bindings/cedarling_python/tests/test_authorize.py: The changes update the error messages in the unit tests to better reflect the underlying issue, which is an important security-related improvement.
  • jans-cedarling/cedarling/Cargo.toml: The addition of the regex dependency should be reviewed to ensure that the regular expressions are implemented securely and are not vulnerable to ReDoS attacks.
  • jans-cedarling/bindings/cedarling_python/src/authorize/errors.rs: The new error types and the centralized error handling mechanism demonstrate a security-conscious approach to exception management.
  • jans-cedarling/cedarling/src/authz/mod.rs: The changes to the token entity creation and the use of trusted issuer information are crucial for the overall security of the authorization process.
  • jans-cedarling/cedarling/src/authz/entities/create.rs: The input validation and the handling of nested types in the entity creation process are important security considerations.
  • jans-cedarling/cedarling/src/authz/entities/test_create.rs: The test cases cover various error scenarios, which is a positive sign for the application's security posture.
  • jans-cedarling/cedarling/src/common/cedar_schema/cedar_json.rs: The improvements to the schema handling and type lookup functionality can enhance the overall security of the policy management system.
  • jans-cedarling/cedarling/src/authz/token_data.rs: The changes to the TokenPayload struct and the associated methods demonstrate a focus on secure token data handling.
  • jans-cedarling/cedarling/src/common/cedar_schema/test_files/test_data_cedar.json: The addition of the Url and TrustedIssuer entities suggests the application is handling URLs and trusted issuers, which are important security considerations.
  • jans-cedarling/cedarling/src/common/cedar_schema/cedar_json/entity_types.rs: The custom deserialization implementation for the entity types and attributes is a security-conscious approach.
  • jans-cedarling/cedarling/src/common/policy_store.rs: The improvements to the TrustedIssuer struct and the associated metadata handling are crucial for the overall security of the authorization process.
  • jans-cedarling/cedarling/src/common/policy_store/claim_mapping.rs: The claim mapping functionality and the handling of regular expressions are important security features.
  • jans-cedarling/cedarling/src/common/policy_store/token_entity_metadata.rs: The simplification of the claim_mapping field is a positive change that can improve the security and maintainability of the code.
  • jans-cedarling/cedarling/src/lib.rs: The addition of the authorize_entities_data method suggests the application is using a complex authorization mechanism, which requires careful security review.
  • `jans

Code Analysis

We ran 9 analyzers against 28 files and 0 analyzers had findings. 9 analyzers had no findings.

Riskiness

🟢 Risk threshold not exceeded.

View PR in the DryRun Dashboard.

@mo-auto
Copy link
Member

mo-auto commented Nov 18, 2024

Error: Hi @olehbozhok, You did not reference an open issue in your PR. I attempted to create an issue for you.
Please update that issues' title and body and make sure I correctly referenced it in the above PRs body.

@mo-auto mo-auto added the comp-jans-cedarling Touching folder /jans-cedarling label Nov 18, 2024
@olehbozhok olehbozhok changed the title Jans cedaling issue 10151 feat(jans-cedarling): implement mapping JWT payload to cedar-policy entity Nov 18, 2024
@mo-auto mo-auto added the kind-feature Issue or PR is a new feature request label Nov 18, 2024
Signed-off-by: Oleh Bohzok <olehbozhok@gmail.com>
@olehbozhok olehbozhok force-pushed the jans-cedaling-issue-10151 branch from 693bd19 to be4ce78 Compare November 18, 2024 00:40
Signed-off-by: Oleh Bohzok <olehbozhok@gmail.com>
…okens_data`

Signed-off-by: Oleh Bohzok <olehbozhok@gmail.com>
Signed-off-by: Oleh Bohzok <olehbozhok@gmail.com>
@olehbozhok olehbozhok requested a review from nynymike November 18, 2024 02:40
…dar-policy` types

Signed-off-by: Oleh Bohzok <olehbozhok@gmail.com>
@mo-auto mo-auto added area-documentation Documentation needs to change as part of issue or PR comp-docs Touching folder /docs labels Nov 18, 2024
Copy link
Contributor

@nynymike nynymike left a comment

Choose a reason for hiding this comment

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

This looks pretty good to me. I think we need to test it to see.


**Note**: You can include a `role_mapping` in each token but only the first one that get parsed will be recognized by Cedarling. Cedarling parses the `role_mapping`s for each token in this order:
You can include a `role_mapping` in each token but only the first one that get parsed will be recognized by Cedarling. Cedarling parses the `role_mapping`s for each token in this order:
Copy link
Contributor

Choose a reason for hiding this comment

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

Why? If the id_token says roles are contained in the role claim, and the access_token says in the member_of claim, why can't the cedarling instantiate add'l roles?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We never discussed that we should get roles from different tokens. And it looks little strange if different tokens generated for same person have different roles set...

Copy link
Contributor

@djellemah djellemah left a comment

Choose a reason for hiding this comment

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

Looks ok to me.

@olehbozhok olehbozhok merged commit df02b94 into main Nov 18, 2024
10 of 11 checks passed
@olehbozhok olehbozhok deleted the jans-cedaling-issue-10151 branch November 18, 2024 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-documentation Documentation needs to change as part of issue or PR comp-docs Touching folder /docs comp-jans-cedarling Touching folder /jans-cedarling kind-feature Issue or PR is a new feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fix: Jans cedaling issue 10151 -autocreated feat(jans-cedarling): implement mapping JWT payload to cedar-policy entity
5 participants