-
Notifications
You must be signed in to change notification settings - Fork 740
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
Removed oauth2 token handling, populated User in security context. #188
Conversation
@@ -103,57 +103,58 @@ class OAuth2SecurityConfig implements WebSecurityAugmentor { | |||
@Override | |||
void configure(AuthenticationManagerBuilder authenticationManagerBuilder) { | |||
authenticationManagerBuilder.authenticationProvider( | |||
googleAuthenticationProvider(googleResourceServerTokenServices()) | |||
// googleAuthenticationProvider(googleResourceServerTokenServices()) |
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.
No dead code - remove it if it's not usable.
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.
Done.
@Autowired | ||
AnonymousAccountsService anonymousAccountsService | ||
|
||
@Value('${services.deck.baseUrl:http://localhost/9000}') |
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 think you want localhost:9000
Also, does this mean that it will always redirect to the homepage? Doesn't the dance in deck encode the requested link?
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.
Good catch. Yes, this would redirect to deck's homepage each time, which then makes the call to '/oauth/info' again, which is successful the second time (returns the User object).
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.
Fixed the default url string for deck.
This PR completes the proof of concept authentication implementation using Google as an OAuth2 provider. The user authenticates with Google, then we use the resulting token to make a user info call, and populate the session-based SecurityContext with that information.
OP-5567 : Added new request param for AP test-verification endpoint
This PR removes the propagation of OAuth2 tokens between Deck and Gate, and populates the User object returned from the authEndpoint call ('/oauth/info') with the email of the Google User associated with the OAuth2 token granted in the redirect flow. It also factors out Netflix's OAuth2 configuration from the generic configuration.