Skip to content

Conversation

@alisonnRB
Copy link

His pull request introduces two significant improvements: the modernization of the TrustChainBuilder class to use a recursive approach and the correction of a critical logic flaw in the authorization request validation.

1. TrustChainBuilder Refactoring (spid_cie_oidc/entity/trust_chain.py)

The apply_metadata_policy method in the TrustChainBuilder class has been completely refactored. The previous logic, based on a complex while loop, has been replaced with a cleaner, more modern recursive approach (Depth-First Search).

Benefits:

  • Improved Readability: The recursive logic, broken down into helper functions, makes the trust path discovery flow much easier to understand.

  • Maintainability: Smaller functions with single responsibilities are easier to maintain and debug.

  • Reduced Complexity: Eliminates the need for control flag variables and complex state management within the loop.

2. Logic Fix in validate_authz Validation (spid_cie_oidc/provider/views/authz_request_view.py)

An inverted logic flaw was identified and corrected in the validation that checks whether the redirect_uri belongs to the client_id's domain.

  • Old Behavior (Incorrect): The code threw a ValidationException if the client_id was present in the redirect_uri, effectively punishing valid requests.

  • New Behavior (Correct): The if condition has been negated (if not ...). Now, the exception is correctly thrown only if the client_id is not present in the redirect_uri.

This fix is crucial for security, as it ensures the expected protection against open redirect attacks.

old:
if-redirect

@alisonnRB
Copy link
Author

As a suggestion, the max pax length attribute of the TrustChainBuilder class could be passed as a parameter for greater customization of the use of the entities.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant