OIDC provider #330
Merged
OIDC provider #330
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- First oidc services and classes beans
…into oidc_provider # Conflicts: # src/modules/rest/impl/src/main/java/it/geosolutions/geostore/services/rest/impl/RESTResourceServiceImpl.java
|
@afabiani can you please fix conflicts so that we can proceed finally with the review? |
…into oidc_provider # Conflicts: # src/modules/rest/impl/pom.xml # src/modules/rest/impl/src/main/java/it/geosolutions/geostore/services/rest/impl/RESTResourceServiceImpl.java # src/modules/rest/impl/src/test/java/it/geosolutions/geostore/rest/service/impl/RESTResourceServiceImplTest.java # src/pom.xml
offtherailz
requested changes
Mar 12, 2024
There was a problem hiding this comment.
At the end of the review I found that only 9 files have significant changes.
Not any particular breaking change, a generic OIDC connect has been added to the current configuration.
So also this PR is safe to merge and backport. 👍 nice
It doesn't introduce the possibility of configure multiple ones (now we can have one google, one keycloak, one generic oidc. I think that anyway this is enough).
Is not clear to me how to configure this to test. Looking at various superclasses I found this list of properties:
- IdPConfiguration
- protected String
beanName; - protected boolean
enabled= false; - protected boolean
autoCreateUser; - protected String
internalRedirectUri; - protected String
redirectUri; - protected Role
authenticatedDefaultRole;
- protected String
- Oauth2Configuration
- protected String
clientId; - protected String
clientSecret; - protected String
accessTokenUri; - protected String
authorizationUri; - protected String
checkTokenEndpointUrl; - protected String
logoutUri; - protected String
scopes; - protected String
idTokenUri; - protected String
discoveryUrl; - protected String
revokeEndpoint; - protected boolean
enableRedirectEntryPoint= false; - protected String
principalKey; - protected String
rolesClaim; - protected String
groupsClaim;
- protected String
- OpenIdConnectConfiguration
jwkURIresponseModepostLogoutRedirectUrisendClientSecretallowBearerTokenusePKCE
Here my requests:
-
sendClientSecret,usePKCEare not clear, please document them in the JSDoc. -
responseModeandallowBearerTokendon't look to be used. Please remove them -
jwkURItoo doesn't seems to be used, only used to set thestoreproperty ofGeoStoreOauthRestTemplatebut never used again. Also this store is private and the PR introduces it without adding any access to the private property or propertyAccessor. If it is not needed, please remove it. - Please provide a sample configuration and instructions to test it.
|
@offtherailz please see below:
|
|
@afabiani is this ready for a new review or are you still working on it? |
|
Nope, no other additions on my side |
|
trying to set it up in mapstore I received the following error: @afabiani , do you know how to fix it? |
…into oidc_provider
|
@offtherailz try to update the <security:http auto-config="true" create-session="never" >
<security:http-basic entry-point-ref="restAuthenticationEntryPoint"/>
<security:csrf disabled="true"/>
<security:custom-filter ref="authenticationTokenProcessingFilter" before="FORM_LOGIN_FILTER"/>
<security:custom-filter ref="sessionTokenProcessingFilter" after="FORM_LOGIN_FILTER"/>
<security:custom-filter ref="keycloakFilter" before="BASIC_AUTH_FILTER"/>
<security:custom-filter ref="googleOpenIdFilter" after="BASIC_AUTH_FILTER"/>
<security:custom-filter ref="oidcOpenIdFilter" before="OPENID_FILTER"/>
<security:anonymous />
</security:http> |
offtherailz
reviewed
Mar 19, 2024
| @@ -27,6 +27,7 @@ | |||
| <security:custom-filter ref="sessionTokenProcessingFilter" after="FORM_LOGIN_FILTER"/> | |||
| <security:custom-filter ref="keycloakFilter" before="BASIC_AUTH_FILTER"/> | |||
| <security:custom-filter ref="googleOpenIdFilter" after="BASIC_AUTH_FILTER"/> | |||
| <security:custom-filter ref="oidcOpenIdFilter" after="BASIC_AUTH_FILTER"/> | |||
There was a problem hiding this comment.
Suggested change
| <security:custom-filter ref="oidcOpenIdFilter" after="BASIC_AUTH_FILTER"/> | |
| <security:custom-filter ref="oidcOpenIdFilter" before="OPENID_FILTER"/> |
- Allowing Auth Code flow only
|
@afabiani notes for testing keycloakconfigure keycloak keycloak new realm (named mapstore) Save Create a new client in the mapstore realm
Configure and run mapstore
Integrate new oidcConfigure MapStore front-end for Google OpenID {
"authenticationProviders": [
{
"type": "openID",
"provider": "oidc"
},
{
"type": "basic",
"provider": "geostore"
}
]
}Add to
(This allows to use proper jars for 2.2-SNAPSHOT from mapstore master )
# Client Secret xBRWNDXiAC4fPp8l89hGCBY6QGDPDDqa
# enables the keycloak OpenID Connect filter
oidcOAuth2Config.enabled=true
# note: this is the client id you have created in Keycloak
oidcOAuth2Config.clientId=mapstore
oidcOAuth2Config.clientSecret=xBRWNDXiAC4fPp8l89hGCBY6QGDPDDqa
oidcOAuth2Config.discoveryUrl=http://localhost:8180/realms/mapstore/.well-known/openid-configuration
oidcOAuth2Config.sendClientSecret=true
oidcOAuth2Config.redirectUri=http://localhost:8081/rest/geostore/openid/oidc/callback
# Internal redirect URI (you can set it to relative path like this `../../..` to make this config work across domain)
oidcOAuth2Config.internalRedirectUri=https://localhost:8081/Run and debug mapstoreedit # product/pom.xml
<containerId>tomcat8x</containerId>
+ <systemProperties>
+ <datadir.location>${env.DATADIR}</datadir.location>
+ </systemProperties>open 2 consoles
# MapStore front-end
# export MAPSTORE_BACKEND_PORT=8082
npm run fe:start
# export MAPSTORE_BACKEND_PORT=8082
## Change with path to your data dir.
export DATADIR=/home/offtherailz/work/projects/MapStore2/datadir
# debug (opt)
# export MAVEN_OPTS="-Xdebug -Xnoagent -Djava.compiler=NONE -Xrunjdwp:transport=dt_socket,address=4000,server=y,suspend=n"
# run backend.
npm run be:startContainer logs
|
|
@offtherailz see here |
offtherailz
approved these changes
Apr 3, 2024
|
@offtherailz @afabiani, once tested and accepted, this need to be backported to 2.1.x branch for MS 2024.01.01 |
17 tasks
|
@tdipisa this was assigned to 2024.02.00 as far as I see |
milestone changed according to #330 (comment) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This Pull Request introduces the OpenID Connect Provider Filter for GeoStore
Partially Fix geosolutions-it/MapStore2#10151