-
Notifications
You must be signed in to change notification settings - Fork 96
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
Support for OIDC scopes #357
Conversation
…configuration (cherry picked from commit 59b7b63)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can not test.
- If I do not set scopes, it rises an exception (see inline comment)
- If I set scopes,
oidcOAuth2Config.scopes=openid,profile,groups
, I have aNo access token found on callback request.
without any additional logging.
Scope should be optional, and when we have errors, we should have a proper logging to be able to identify the problem and possibly be able to fix the configuration issues, without the needing to inspect the code everytime.
This issue without any logging happened a lot of times, and findi out the reason is extremely expensive in terms of time and resources,
|
||
// Get existing scopes from conf and split into a Set | ||
Set<String> mergedScopes = new HashSet<>(); | ||
mergedScopes.addAll(Arrays.asList(conf.getScopes().split(","))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a NullPointerException
when I do not set scopes.
... 30 more
Caused by: java.lang.NullPointerException
at it.geosolutions.geostore.services.rest.security.oauth2.DiscoveryClient.lambda$autofill$6(DiscoveryClient.java:114) ~[geostore-rest-impl-2.2-SNAPSHOT.jar:?]
at java.util.Optional.ifPresent(Optional.java:183) ~[?:?]
at it.geosolutions.geostore.services.rest.security.oauth2.DiscoveryClient.autofill(DiscoveryClient.java:107) ~[geostore-rest-impl-2.2-SNAPSHOT.jar:?]
at it.geosolutions.geostore.services.rest.security.oauth2.openid_connect.OpenIdConnectFilter.<init>(OpenIdConnectFilter.java:71) ~[geostore-rest-impl-2.2-SNAPSHOT.jar:?]
at it.geosolutions.geostore.services.rest.security.oauth2.openid_connect.OpenIdConnectSecurityConfiguration.oidcOpenIdFilter(OpenIdConnectSecurityConfiguration.java:145) ~[geostore-rest-impl-2.2-SNAPSHOT.jar:?]
at it.geosolutions.geostore.services.rest.security.oauth2.openid_connect.OpenIdConnectSecurityConfiguration$$EnhancerBySpringCGLIB$$9ded83d5.CGLIB$oidcOpenIdFilter$3(<generated>) ~[geostore-rest-impl-2.2-SNAPSHOT.jar:?]
at it.geosolutions.geostore.services.rest.security.oauth2.openid_connect.OpenIdConnectSecurityConfiguration$$EnhancerBySpringCGLIB$$9ded83d5$$FastClassBySpringCGLIB$$e4b2836b.invoke(<generated>) ~[geostore-rest-impl-2.2-SNAPSHOT.jar:?]
at org.springframework.cglib.proxy.MethodProxy.invokeSuper(MethodProxy.java:244) ~[spring-core-5.3.18.jar:5.3.18]
at org.springframework.context.annotation.ConfigurationClassEnhancer$BeanMethodInterceptor.intercept(ConfigurationClassEnhancer.java:331) ~[spring-context-5.3.18.jar:5.3.18]
at it.geosolutions.geostore.services.rest.security.oauth2.openid_connect.OpenIdConnectSecurityConfiguration$$EnhancerBySpringCGLIB$$9ded83d5.oidcOpenIdFilter(<generated>) ~[geostore-rest-impl-2.2-SNAPSHOT.jar:?]
at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:?]
at jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[?:?]
at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:?]
at java.lang.reflect.Method.invoke(Method.java:566) ~[?:?]
…into oidc_scopes_patch
@offtherailz NPE fix pushed, for the second statement, rise the log level to DEBUG or check the Keycloack service log. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Logging for callback error (and errorDescription) in
/callback
endpoint - Support for
groupMappings
,rolesMappings
and `dropUnmapped' as for keycloak
This reverts commit 9cf2c27.
…configuration
(cherry picked from commit 59b7b63)
Fix geosolutions-it/MapStore2#10151