ONYX-13640 - Nested annotations integration #2404
Conversation
| claim_name_for_error(annotation_name, claim_name) | ||
| end | ||
| claim_value = @decoded_token.dig(*parsed_claim_path(claim_name)) | ||
| raise Errors::Authentication::AuthnJwt::JwtTokenClaimIsMissing, |
| raise Errors::Authentication::AuthnJwt::JwtTokenClaimIsMissing, | ||
| claim_name_for_error(annotation_name, claim_name) | ||
| end | ||
| claim_value = @decoded_token.dig(*parsed_claim_path(claim_name)) |
There was a problem hiding this comment.
Move right side to function called claim_value
There was a problem hiding this comment.
To seperate responsibilities between function. One wrapping parsed_claim_path and one doing the compare. Do what you think its not critical
There was a problem hiding this comment.
It's replace one function dig by other.
The class is stateless from single restriction perspective so memorization is impossible.
There was a problem hiding this comment.
Without memoriaztion jsut
def claim_value(claim_name)
@decoded_token.dig(*parsed_claim_path(claim_name))
end
So the comprare
restriction_value == claim_value
will be the only call for the function.
There was a problem hiding this comment.
there are multiple usages by claim value in the main function
I don't want to dig for value each time it needed
It's enough to dig once.
![codeclimate[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
| @@ -23,12 +25,13 @@ def valid_restriction?(restriction) | |||
| raise Errors::Authentication::ResourceRestrictions::EmptyAnnotationGiven, annotation_name | |||
| end | |||
|
|
|||
| unless @decoded_token.key?(claim_name) | |||
| claim_value = @decoded_token.dig(*parsed_claim_path(claim_name)) | |||
| if claim_value.nil? | |||
There was a problem hiding this comment.
Authentication::AuthnJwt::RestrictionValidation::ValidateRestrictionsOneToOne#valid_restriction? performs a nil-check
|
Code Climate has analyzed commit ed1428b and detected 1 issue on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 100.0% (50% is the threshold). This pull request will bring the total coverage in the repository to 90.9% (0.0% change). View more on Code Climate. |
Desired Outcome
authn-jwt related host annotations supports nested claims.
Implemented Changes
ValidateRestrictionsOneToOneis using Hash.dig function instead of direct key access to json object.Connected Issue/Story
Follows design #2394
ONYX-13640
Definition of Done
Changelog
CHANGELOG update
Test coverage
changes, or
Documentation
READMEs) were updated in this PRBehavior
Security