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

feat(jans-cedarling): ensure that all cedarling test fixture files are human-readable. #10036

Merged
merged 30 commits into from
Nov 7, 2024

Conversation

djellemah
Copy link
Contributor

@djellemah djellemah commented Nov 4, 2024

Prepare


Description

Target issue

10021

closes #10021

Implementation Details

  • convert most json files in jans-cedarling/test_files. Some tests using the remaning json files rely on the base64 encoded values to achieve their assertions.

  • update all rust tests making use of changed files.

  • update all python tests making use of changed files.

  • document all files in jans-cedarling/test_files and in jans-cedarling/test_files/README.md


Test and Document the changes

  • clippy 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.

@djellemah djellemah added the comp-jans-cedarling Touching folder /jans-cedarling label Nov 4, 2024
@djellemah djellemah self-assigned this Nov 4, 2024
@djellemah djellemah changed the title Ensure that all cedarling test fixture files are human-readable. DRAFT Ensure that all cedarling test fixture files are human-readable. Nov 4, 2024
@mo-auto mo-auto added area-documentation Documentation needs to change as part of issue or PR comp-docs Touching folder /docs labels Nov 4, 2024
Copy link

dryrunsecurity bot commented Nov 4, 2024

DryRun Security Summary

The pull request enhances the policy management and access control capabilities of the Cedarling application by adding support for YAML policy stores, improving policy deserialization, enhancing the policy store structure, and providing detailed policy and schema definitions.

Expand for full summary

Summary:

The code changes in this pull request focus on enhancing the policy management and access control capabilities of the Cedarling application. The key changes include:

  1. Support for YAML Policy Stores: The application now supports loading policy stores from both JSON and YAML formats, providing more flexibility in how policies are defined and managed.
  2. Improved Policy Deserialization: The code includes extensive error handling and validation for the deserialization of policy and schema data, ensuring that malformed or invalid input is properly identified and handled.
  3. Enhanced Policy Store Structure: The policy store configuration has been updated to a more structured format, with the introduction of the AgamaPolicyStore and the ability to manage multiple policy stores within a single application.
  4. Detailed Policy and Schema Definitions: The code includes comprehensive policy and schema definitions, covering a wide range of entity types, actions, and conditions, which demonstrates a strong focus on fine-grained access control.

From an application security perspective, these changes are generally positive, as they improve the overall security posture of the Cedarling application. The focus on input validation, error handling, and the implementation of a robust policy management system are all important security considerations.

However, it's crucial to ensure that the policy definitions and their implementation are thoroughly reviewed to avoid any unintended security vulnerabilities. Additionally, the handling of sensitive information, such as identity source metadata and token data, should be carefully examined to prevent potential security issues like information disclosure or unauthorized access.

Files Changed:

  1. jans-cedarling/bindings/cedarling_python/src/config/policy_store_source.rs: Adds support for loading policy stores from YAML format in addition to JSON.
  2. jans-cedarling/bindings/cedarling_python/cedarling_python.pyi: Updates the PolicyStoreSource class to accept a yaml parameter for YAML-based policy stores.
  3. docs/cedarling/cedarling-policy-store.md: Describes the changes to the Cedarling Policy Store, including the new JSON schema, policy content format, and identity source schema.
  4. jans-cedarling/bindings/cedarling_python/example_files/policy-store.json: Updates the example policy store configuration to use the new v4.0.0 schema.
  5. jans-cedarling/bindings/cedarling_python/tests/config.py: Modifies the test fixture to use a YAML-based policy store configuration.
  6. jans-cedarling/cedarling/examples/authorize_without_jwt_validation.rs: Updates the policy store configuration to use a YAML file.
  7. jans-cedarling/cedarling/examples/authorize_with_jwt_validation.rs: Updates the policy store configuration to use a YAML file.
  8. jans-cedarling/cedarling/src/common/policy_store.rs: Introduces the AgamaPolicyStore and updates the PolicyStore struct to include new fields and functionality.
  9. jans-cedarling/cedarling/src/common/cedar_schema.rs: Enhances the deserialization logic to handle both JSON and YAML formats for the Cedar schema.
  10. jans-cedarling/cedarling/src/tests/mod.rs: Adds a new module called "agama_policy_test" for testing the policy enforcement functionality.
  11. jans-cedarling/cedarling/src/tests/agama_policy_test.rs: Includes a test case for loading and evaluating a policy store in JSON format.
  12. jans-cedarling/cedarling/src/tests/success_test_json.rs: Updates the test case to use a YAML-based policy store configuration.
  13. jans-cedarling/cedarling/src/jwt/decoding_strategy/key_service.rs: Includes a minor change to the HttpClient implementation.
  14. jans-cedarling/test_files/agama-store.yaml: Provides a sample YAML-based policy store configuration.
  15. jans-cedarling/test_files/README.md: Describes the various test fixtures for policy store files.
  16. jans-cedarling/test_files/policy-store_blobby.json: Updates the policy store configuration to use the new v4.0.0

Code Analysis

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

Riskiness

🟢 Risk threshold not exceeded.

View PR in the DryRun Dashboard.

@djellemah djellemah force-pushed the jans-cedarling-issue-10021 branch from 62240ff to ffdefa1 Compare November 4, 2024 21:58
@djellemah djellemah marked this pull request as ready for review November 4, 2024 22:08
@djellemah djellemah changed the title DRAFT Ensure that all cedarling test fixture files are human-readable. Ensure that all cedarling test fixture files are human-readable. Nov 4, 2024
rmarinn
rmarinn previously approved these changes Nov 5, 2024
@SafinWasi SafinWasi changed the title Ensure that all cedarling test fixture files are human-readable. feat(jans-cedarling): ensure that all cedarling test fixture files are human-readable. Nov 5, 2024
@mo-auto mo-auto added the kind-feature Issue or PR is a new feature request label Nov 5, 2024
abaghinyan
abaghinyan previously approved these changes Nov 5, 2024
Copy link
Contributor

@olehbozhok olehbozhok left a comment

Choose a reason for hiding this comment

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

I feel OK to merge but we have changes in the schema
in PR #10039
so probably better to wait some time

@moabu moabu dismissed stale reviews from abaghinyan and rmarinn November 6, 2024 15:30

The merge-base changed after approval.

@djellemah djellemah force-pushed the jans-cedarling-issue-10021 branch from ffdefa1 to 319146a Compare November 6, 2024 17:02
moabu
moabu previously approved these changes Nov 7, 2024
olehbozhok
olehbozhok previously approved these changes Nov 7, 2024
@djellemah djellemah dismissed stale reviews from olehbozhok and moabu via caa7e24 November 7, 2024 14:52
olehbozhok
olehbozhok previously approved these changes Nov 7, 2024
olehbozhok
olehbozhok previously approved these changes Nov 7, 2024
duttarnab
duttarnab previously approved these changes Nov 7, 2024
olehbozhok
olehbozhok previously approved these changes Nov 7, 2024
Copy link
Contributor

@olehbozhok olehbozhok left a comment

Choose a reason for hiding this comment

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

Looks great, but how about every notice of "cedar_version" : "v2.4.7", replace to "cedar_version": "v4.0.0", ?

@djellemah djellemah dismissed stale reviews from olehbozhok and duttarnab via fb81db6 November 7, 2024 15:25
@olehbozhok olehbozhok requested a review from duttarnab November 7, 2024 15:39
@duttarnab duttarnab merged commit 535520d into main Nov 7, 2024
11 checks passed
@duttarnab duttarnab deleted the jans-cedarling-issue-10021 branch November 7, 2024 15:51
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.

ok

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.

ensure that all test fixtures are human-readable
8 participants