Skip to content

Consider moving CommonOAuth2Provider to Spring Security #4597

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

Closed
mbhave opened this issue Oct 2, 2017 · 6 comments
Closed

Consider moving CommonOAuth2Provider to Spring Security #4597

mbhave opened this issue Oct 2, 2017 · 6 comments
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose)
Milestone

Comments

@mbhave
Copy link
Contributor

mbhave commented Oct 2, 2017

Instead of using spring-security-oauth2-client-templates.properties for common provider defaults, we updated the auto-config in Boot to use this. This seems like it could live in Spring Security instead of the properties file.

@philwebb
Copy link
Member

philwebb commented Oct 3, 2017

Even if CommonOAuth2Provider doesn't move I think that spring-security-oauth2-client-templates.properties should be deleted. It feels odd that there is a property file in Spring Security that defines keys in Spring Boot form.

@jgrandja
Copy link
Contributor

jgrandja commented Oct 3, 2017

@philwebb I added spring-security-oauth2-client-templates.properties to Spring Security based on my PR review comment and this comment.

However, given that a different approach was taken in the recent PR, spring-security-oauth2-client-templates.properties will be removed from Spring Security.

@philwebb
Copy link
Member

philwebb commented Oct 3, 2017

@jgrandja Yeah, and still think it makes sense to have those default in Spring Security in case users want them without Spring Boot. I'm just don't think we should use the property name format from Boot (in case we want to evolve it).

@jgrandja
Copy link
Contributor

jgrandja commented Oct 3, 2017

Related #4598

@jgrandja
Copy link
Contributor

jgrandja commented Oct 3, 2017

@philwebb I'll take care of this at some point next week.

@jgrandja jgrandja self-assigned this Oct 6, 2017
@jgrandja jgrandja added the in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) label Oct 6, 2017
@jgrandja jgrandja added this to the 5.0.0.M5 milestone Oct 6, 2017
@jgrandja
Copy link
Contributor

jgrandja commented Oct 6, 2017

Thanks for the contribution @philwebb ! Much nicer approach vs. properties file. This is now in master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose)
Projects
None yet
Development

No branches or pull requests

3 participants