-
Notifications
You must be signed in to change notification settings - Fork 136
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
Update to Spring Boot 2.4.0 #214
Conversation
@arvindkrishnakumar-okta can you restart that failing CI job, it looks like a network error on Travis (while downloading maven artifacts) |
@@ -67,4 +67,5 @@ scenarios: | |||
- --okta.oauth2.scopes=offline_access | |||
# - --okta.oauth2.redirectUri=/authorization-code/callback # not implemented until Spring Sec 5.2 | |||
- --server.servlet.session.tracking-modes=cookie | |||
- --spring.security.oauth2.client.registration.okta.client-authentication-method=none |
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.
@mraible looks like this is one of the things that changed in the latest version of Spring Security (I've got a question on the Spring Sec Gitter to confirm this)
This prop needs to be set in order for PKCE (for clients without secrets) to work. Assuming this is correct, any thoughts on pushing this down to the user like this? or should we re-map it to okta.oauth2.client-authentication-method=none
, or... should we just detect this condition and configure it automatically (to insure the previous functionality)
(PKCE still isn't used for clients with secrets, that that's another thread...)
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 would configure it automatically, then document it. That way, it's a seamless upgrade for folks. As far as I know, you can configure Spring Boot as a resource server with only an issuer. I'm not sure folks will know that an issuer and a client ID means it'll use PKCE. A lot of our JS SDKs have a pkce
flag that you can turn off. Does it make sense to add an okta.oauth2.pkce
property?
done |
http.authorizeRequests((requests) -> requests.anyRequest().authenticated()); | ||
http.oauth2Login(Customizer.withDefaults()); | ||
http.oauth2Client(); | ||
http.oauth2ResourceServer(OAuth2ResourceServerConfigurer::jwt); |
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.
Nice!
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.
In this example, I noticed they use withDefaults()
for login, client, and resource server. Not sure what that does, but thought I'd mention it.
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 just checked withDefaults()
, just adds a no-op "customizer"
@@ -47,7 +47,6 @@ scenarios: | |||
- --okta.oauth2.issuer=https://localhost:${mockHttpsPort}/oauth2/default | |||
- --okta.oauth2.clientId=OOICU812 | |||
- --okta.oauth2.clientSecret=VERY_SECRET | |||
- --okta.oauth2.scopes=profile,email,openid |
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 added a quick note in the commit, but this ensures, that the default scopes ARE picked up (as the ITs will validate these scopes are present)
Spring Sec 5.4 allows for this logic to just be a bean!
* lots of cleanup * removed explicit setting of scopes in default scopes testRunner.yml, to ensure that path is covered by tests
A few things change in Spring Security 5.4 (used by Spring Boot 2.4)
openid, profile, email