Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix issue 3083: check user_name claim type #3084

Merged
merged 2 commits into from
Oct 15, 2024
Merged

fix issue 3083: check user_name claim type #3084

merged 2 commits into from
Oct 15, 2024

Conversation

strehle
Copy link
Member

@strehle strehle commented Oct 12, 2024

solve #3083

@strehle strehle linked an issue Oct 12, 2024 that may be closed by this pull request
@strehle strehle self-assigned this Oct 12, 2024
@strehle
Copy link
Member Author

strehle commented Oct 12, 2024

after first comming

ClassCastException: class java.util.ArrayList cannot be cast to class java.lang.String (java.util.ArrayList and java.lang.String are in module java.base of loader 'bootstrap')

@strehle strehle changed the title Add test for issue 3083 fix issue 3083: check user_name claim type Oct 12, 2024
@strehle strehle requested a review from a team October 13, 2024 07:28
@@ -424,7 +424,7 @@ private String getMappedClaim(String externalName, String internalName, Map<Stri
return (String) claimObject;
}
if (claimObject instanceof Collection) {
Set<String> entry = ((Collection<?>) claimObject).stream().map(String.class::cast).collect(Collectors.toSet());
Set<String> entry = ((Collection<?>) claimObject).stream().filter(String.class::isInstance).map(String.class::cast).collect(Collectors.toSet());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will collection items always going to be a string? Should we consider handle it in more generic way:

Set entry = ((Collection<?>) claimObject).stream()
.map(claim -> String.valueOf(claim))
.collect(Collectors.toSet());

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is not wanted to have such a generic approach, but simple check if the entries inside of the token are strings. With your approach we allow all but then we are incompatible with now. I mean if claims are type of Boolean or int we would allow them now. But that is not wanted

@strehle
Copy link
Member Author

strehle commented Oct 15, 2024

Wanted to solve is that there is a human readable error and not a technical class cast error. The error should show what mapping is wrong, eg. email , first name , user_name etc.

@strehle strehle merged commit 4d2f8e0 into develop Oct 15, 2024
22 checks passed
@strehle strehle deleted the fix/issue/3083 branch October 15, 2024 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Again error from IdP if mapped claim is no string
3 participants