Skip to content

Provide XML namespace support for OAuth2Login #4557

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
tiwatsuka opened this issue Sep 20, 2017 · 22 comments
Closed

Provide XML namespace support for OAuth2Login #4557

tiwatsuka opened this issue Sep 20, 2017 · 22 comments
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: enhancement A general enhancement
Milestone

Comments

@tiwatsuka
Copy link

tiwatsuka commented Sep 20, 2017

Summary

Does it have any plan to add the way to configure OAuth2Login with XML namespace?
I think there are some projects still using XML configuration for security settings.
If we can define the beans related OAuth2Login just by setting about ClientRegistration with XML, it's very useful.

Related #5184

@jgrandja jgrandja added the in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) label Sep 20, 2017
@jgrandja
Copy link
Contributor

@tiwatsuka We haven't decided whether to add XML namespace support. It definitely won't make it into 5.0 GA. We may consider it in a subsequent release.

@tiwatsuka
Copy link
Author

@jgrandja I see. I'll expect that it is finally added. Thank you for your consideration.

@rwinch rwinch added this to the 5.1.0.M1 milestone Nov 16, 2017
@rwinch rwinch modified the milestones: 5.1.0.M1, 5.1.0.RC1 Dec 19, 2017
@jgrandja
Copy link
Contributor

@tiwatsuka Just to confirm, we will be adding XML namespace support. We'll do our best to get this in 5.1.

@jgrandja jgrandja removed this from the 5.1.0.M2 milestone Jul 24, 2018
@pthorson
Copy link

pthorson commented Nov 6, 2018

So 5.2 then or not at all? I have a large existing Spring MVC project with XML configuration that I need to add OAuth to today. Can I mix the namespace and Java config?

@jgrandja
Copy link
Contributor

jgrandja commented Nov 6, 2018

@pthorson This won't get into 5.2. Unless you'd be interested in submitting a PR? There are other higher priority items at the moment and there hasn't been much demand to add this support as of yet.

That being said, you should be able to mix namespace and Java config - see Importing XML Configuration

@pthorson
Copy link

Understood. Not sure of appetite here for a PR. That being said, is OAuth 2.0 and Open ID Connect possible now without Spring Boot being used? Based on the matrix it looks like Spring Boot 1.5x was previously required.

@rwinch
Copy link
Member

rwinch commented Nov 14, 2018

@pthorson The new support does not require Spring Boot (however, it is certainly easier to use with it)

@bendem
Copy link

bendem commented Apr 26, 2019

Hi, I'd like to request once again that feature. I just had to migrate our xml config for security to java config because there was no equivalent to org.springframework.security.config.annotation.web.builders.HttpSecurity#oauth2Login in xml. The Java config is less clear and more error prone. It's really annoying that there is no parity between the capabilities of org.springframework.security.config.annotation.web.builders.HttpSecurity and the spring-security xml namespace.

@jgrandja
Copy link
Contributor

@bendem Unfortunately, this support won't be added in the upcoming 5.2 release. There are other higher priority items that we're targeting for 5.2. As an FYI, we prioritize tasks based on user demand and given that there are only 2 upvotes on this issue there isn't much demand for it. This doesn't mean we won't add the support though, it just means it's on the lower priority list.

@rwinch rwinch added type: enhancement A general enhancement and removed in: config An issue in spring-security-config Namespace labels May 3, 2019
@veskop86
Copy link

Here is one more request :)

@rwinch
Copy link
Member

rwinch commented May 20, 2019

Please just use 👍 on the first comment (issue description) to indicate votes. It makes it much easier for us to add that up than counting comments. Thanks :)

@Rahman14354
Copy link

One more request.

@ghost
Copy link

ghost commented Jul 23, 2019

So here's the dilemma. I use XML configuration in a non-Spring boot application. I converted it to Java Configuration and then ran into a Spring Session bug Unable to obtain SessionCookieConfig using Java based config #22319.

Mixing XML with Java Configuration is doable, but not great and since both pieces here relate to the html configuration, I suspect it will break big time.

So, please provide XML configuration or make sure the Java configuration is 100% compatible.

@jgrandja
Copy link
Contributor

@JimRichards22 Looks like there is demand for this feature so I've scheduled it for the 5.3 release. There is no date for it yet but likely about 9-12 months from now.

@rwinch rwinch added the status: ideal-for-contribution An issue that we actively are looking for someone to help us with label Jul 31, 2019
@rwinch
Copy link
Member

rwinch commented Jul 31, 2019

If anyone wants to contribute the support for this, we'd be happy to add it sooner. Just ping the issue asking if you can have the issue and we will confirm. If you have questions on how to do it, let us know.

@jgrandja jgrandja changed the title OAuth2Login configuration with XML namespace Provide XML namespace support for OAuth2Login Nov 18, 2019
@rh-id
Copy link
Contributor

rh-id commented Dec 6, 2019

can I have this issue ? I'm a first timer in open source though, could you guide me on how to do it?

so far I had explore the codes and i came up with the following checklist on what should be modified:

  1. update Elements class to add "oauth2-login"
  2. Create new parser "OAuth2LoginBeanDefinitionParser" that extends AbstractSingleBeanDefinitionParser in org.springframework.security.config.oauth2 package
  3. add new entry at SecurityFilters for OAuth2LoginAuthenticationFilter
  4. update AuthenticationConfigBuilder to create OAuth2LoginAuthenticationFilter filter and update the getFilter to return it
  5. add unit test "OAuth2LoginBeanDefinitionParserTest" and the XML test files to test XML parser

if the above checklist more or less ok I will try to submit PR

@jgrandja
Copy link
Contributor

Thanks for the offer @rh-id. We typically label status: first-timers-only for first time issues. This one is a bit more involved. However, if you're keen on working on this than I would propose we both work on it that way I can guide you through the process and help with the implementation. Alternatively, I can find another issue for you to work on. Let me know your preference.

@rh-id
Copy link
Contributor

rh-id commented Dec 11, 2019

hi @jgrandja . i'm fine with either options if there are more priority issues that you think more suitable then I could help on that

@jgrandja
Copy link
Contributor

@rh-id This is an important feature and needs to get into 5.3.0. So let's work on this together. I think the best approach would be for me to put together the initial "Draft PR" and we can work from there. I'll think of an approach that will make it easy for you to implement the necessary artifacts. I'll submit the Draft PR end of week or early next week and we can go from there. Sound good?

@rh-id
Copy link
Contributor

rh-id commented Dec 11, 2019

@jgrandja sounds good 👍

@jgrandja
Copy link
Contributor

@rh-id If you haven't already, please get familiar with the contributing guidelines.

@jgrandja jgrandja self-assigned this Dec 11, 2019
@jgrandja jgrandja removed the status: ideal-for-contribution An issue that we actively are looking for someone to help us with label Dec 11, 2019
@jgrandja jgrandja modified the milestones: 5.3.x, 5.3.0.RC1 Jan 17, 2020
@eleftherias eleftherias modified the milestones: 5.3.0.RC1, 5.3.0 Feb 5, 2020
jgrandja added a commit that referenced this issue Feb 12, 2020
jgrandja added a commit that referenced this issue Feb 12, 2020
@jgrandja
Copy link
Contributor

For anyone who would like to try out the XML config in the snapshot build, you can find sample test configurations for the <oauth2-login> and <client-registrations> elements here:

  • <oauth2-login> - all files with this pattern OAuth2LoginBeanDefinitionParserTests-*.xml under this directory
  • <client-registrations> - all files in this directory

Feedback would be greatly appreciated!

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) type: enhancement A general enhancement
Projects
None yet
9 participants