Issue #6618 - azp claim should not be required for single value aud array #6620
Conversation
…rray Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
| @@ -105,14 +106,14 @@ public void redeemAuthCode(OpenIdConfiguration configuration) throws Exception | |||
| } | |||
| } | |||
|
|
|||
| private void validateClaims(OpenIdConfiguration configuration) throws Exception | |||
| static void validateClaims(Map<String, Object> claims, OpenIdConfiguration configuration) throws Exception | |||
There was a problem hiding this comment.
It's a little strange to have this as a static method and then need to pass in claims. Why not just make it a normal method?
| @@ -126,7 +127,7 @@ private void validateClaims(OpenIdConfiguration configuration) throws Exception | |||
| throw new AuthenticationException("ID Token has expired"); | |||
| } | |||
|
|
|||
| private void validateAudience(OpenIdConfiguration configuration) throws AuthenticationException | |||
| private static void validateAudience(Map<String, Object> claims, OpenIdConfiguration configuration) throws AuthenticationException | |||
There was a problem hiding this comment.
why static and passing in claims? why not keep as a non-static? If the answer is for testing.... I'd rather keep the design correct... perhaps with a special constructor.
There was a problem hiding this comment.
I don't really think it makes the design incorrect, it makes sense to have a method which validates claims given a configuration. And this doesn't impact anything because these methods are not public. I could add a constructor for OpenIdCredentials(Map<String, Object> claims) but that makes less sense to me.
I'll give it a try and see if it makes it any better.
…ic method. Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
…6618-OpenID-audArray
Issue #6618
It currently incorrectly assumed that any array for the
audclaim will contain more than 1 value.We should only enforce an
azpclaim if the array foraudis longer than 1.