Prioritize groups path over groups claim

[oemel09]

Fixes #766
Potentially existing group claims will be overwritten by the claim specified in smallrye.jwt.path.groups.

[sberyozkin]

@oemel09 I believe it has to be done somehow differently, something along this line: 1) if the path is set - start with checking it, if it produces nothing, check the groups, and finally fallback to the default role

It also should be tested

[oemel09]

Question is if the groups should be checked at all when the claim specified by smallrye.jwt.path.groups is empty or if it should use the default groups (specified via smallrye.jwt.claims.groups) without checking the actual groups claim in that case.
I'd vote for the latter.

[sberyozkin]

Question is if the groups should be checked at all when the claim specified by smallrye.jwt.path.groups is empty or if it should use the default groups (specified via smallrye.jwt.claims.groups) without checking the actual groups claim in that case.

So, if the path is configured, check it, otherwise check groups, and in both cases, fallback to the default role if it is set.
It makes sense, but I don't want to break someone's code where they may have the path configured right now which may be pointing to some non-existing claim, which has no impact anyway because the groups is always producing roles, and now, since the order is changed, and we skip the roles, their applications are broken

[oemel09]

but I don't want to break someone's code

When the groups claim is present in the token and the user has set smallrye.jwt.claims.groups to another claim which exists, the current implementation will still use the roles from the groups claim, but we want it to use the roles from the custom claim. So this is a breaking change already.

With the flow you described it is not possible to use the default role (via smallrye.jwt.claims.groups) in combination with the smallrye.jwt.claims.groups setting when the token also has a groups claim.

[sberyozkin]

@oemel09 Can I ask for the clarification. Do you have tokens with the custom groups claim whose value has nothing to do with roles ? Or do you have tokens with the groups claim containing roles but you want some other claim be used as a source of roles ?

[oemel09]

It's the former, in the groups claim in the token there are integers. But what I'm interested in is stored in a role claim.

[oemel09]

I changed the implementation in the way you suggested and also added tests to it.
As explained I'm still in favor of the other flow. But anyway, for my use case both flows would work.
What's the way forward here? And, in case you know it, how long will it take for this change to end up in quarkus? 🙈

[sberyozkin]

@oemel09 I've been thinking about it for a while, and came to the conclusion that I was over-thinking about the side-effects.
If someone does set the custom role claims path then it should simply be used.

So lets get back to what I believe you were suggesting:

• if the custom path is set - use it
• otherwise check groups
• If the roles is null and the default role is set - use it

Please do the final update per the above and resolve this issue

Thanks for the patience

[oemel09]

I did the changes and adapted the tests. I think I added all the relevant cases, please have a look at them and check if you are okay with all the expected results for each combination.

[sberyozkin]

@oemel09 Can you please squash commits to only have a single one, with the same name as this PR one ?

[sberyozkin]

You may have to build with Maven if the build fails with formatting errors

[oemel09]

All done 👍