Add claimMapping enforcement

[Happy2C0de]

Overview

Fixes #2162
This PR introduces an enforce option to specify if a user would like to enforce the values set in claimMapping. For example, the token:

{
"email": "[email protected]",
"preferred_email": "[email protected]"
}

Currently, the user is not able to use the preferred_email claim because the token contains the default email claim.

This change is very handy and assures backwards compatibility by introducing an enforce option. Configuration would look like:

claimMapping:
  enforce: true
  email: "preferred_email"

Same holds for preferred_username and groups options within claimMapping.

What this PR does / why we need it

Closes #2162

Special notes for your reviewer

OIDC connector tests were adjusted and expanded. Restructured the claimMapping struct to a type in order to simplify the configuration.

Does this PR introduce a user-facing change?

Adds the enforcement of claimMappings

[sagikazarmark]

The proper long-term solution for this is using middleware. I have two reservations with this PR:

• the claimMapping options should remain what it is: a map. The enforce option needs to live outside of it. It's not necessarily clean from a config structure perspective, but keeps the mapping config easier to comprehend IMHO.
• Enforce is not necessarily a good word here. Override, overwrite sound better.

[Happy2C0de]

@sagikazarmark Thanks for checking this PR. 👍

the claimMapping options should remain what it is: a map. The enforce option needs to live outside of it. It's not necessarily clean from a config structure perspective, but keeps the mapping config easier to comprehend IMHO.

Sounds reasonable. I could restructure the PR but will you merge and accept it? I would:

• Rename claimMapping.enforce tooverrideClaimMapping bool
• Take this option out of the ClaimMapping type

I see this PR as a bridge between the current limitations and your suggestion with the middleware. I assume the latter implementation will require some time to be implemented.

[sagikazarmark]

@Happy2C0de yes, I'd be inclined to merge it until we have a better solution in place.

[Happy2C0de]

@sagikazarmark I did the expected changes and rerun the test. Looks good from my point of view.

[sagikazarmark]

Thanks @Happy2C0de !

There are a couple things that needs to be cleared before we can merge it, but overall, it looks good to me.

[Happy2C0de]

@sagikazarmark
I did the requested changes and I simplified the merge in order to push this PR further. Mainly, I agree to your inputs and tried to make everything clear.
Thanks for checking it!

[sagikazarmark]

@Happy2C0de I think you misunderstood one of my comments. I tried to clarify in a follow-up comment.

[sagikazarmark]

This should result in an error accordingly

[Happy2C0de]

@sagikazarmark
Thanks for the review. I apologise the late reply.

I removed the test. I first thought that I could try to test against the error message but the current test cases are not handling errors either. This would require a bigger restructuring of the testing.

I hope the current changes meet your expectations.

Thanks again for checking!

[sagikazarmark]

@Happy2C0de please rebase your PR so that CI can be triggered. Thanks!

[Happy2C0de]

@sagikazarmark done :)

[sagikazarmark]

Perfect, thank you!