Skip to content

OAuth 2.0 Resource Server XML Support #7775

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Mar 2, 2020
Merged

OAuth 2.0 Resource Server XML Support #7775

merged 4 commits into from
Mar 2, 2020

Conversation

jzheaux
Copy link
Contributor

@jzheaux jzheaux commented Dec 28, 2019

Fixes: gh-5185

@jzheaux jzheaux requested a review from jgrandja December 28, 2019 01:26
@jzheaux jzheaux self-assigned this Dec 28, 2019
@jzheaux jzheaux added in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: enhancement A general enhancement labels Dec 28, 2019
@jzheaux jzheaux added this to the 5.3.0.M1 milestone Dec 28, 2019
@eleftherias eleftherias modified the milestones: 5.3.0.M1, 5.3.0.RC1 Jan 8, 2020
@eleftherias eleftherias modified the milestones: 5.3.0.RC1, 5.3.0 Feb 5, 2020
jgrandja
jgrandja requested changes Feb 13, 2020
View reviewed changes
Copy link
Contributor

@jgrandja jgrandja left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jzheaux Please see my comments below. In addition to those:

  • Ensure Copyright is 2020 - there are some out-of-date
  • Make sure you have all the tests covered as in OAuth2ResourceServerConfigurerTests - there seems to be a lot more in Java config vs. XML config
  • Update namespace.adoc and spring-security-5.3.rnc
  • OAuth2ResourceServerConfigurer exposes accessDeniedHandler() and authenticationEntryPoint(), however, I don't see access-denied-handler-ref or authentication-entry-point-ref in the xsd. Was this missed or intentional?

@jzheaux
Copy link
Contributor Author

jzheaux commented Feb 27, 2020 •
edited
Loading

@jgrandja This is ready for another look.

I added entry-point-ref to align with <http-basic> and based on your feedback. Note that <http-basic> calls the attribute entry-point-ref.

Yes, it was intentional to leave out access-denied-handler-ref. The user can simply supply it at the top level.

The reason for leaving it out, for now, is that the request matching in the XML config support isn't as mature as in the DSL. I did bring some of it that support over, but the request matchers that are used on the DSL side are more sophisticated, and I wasn't sure about bringing that into parity in this PR.

For example, the way to determine whether or not the request is HTTP basic is quite involved. Let me know if you think we should take a look at that in this PR. Personally, I think it's okay for the XML to be a subset, so long as the same feature is still possible, which it is in this case without adding equivalent support.

@jzheaux jzheaux added the status: duplicate A duplicate of another issue label Mar 2, 2020
jgrandja
jgrandja requested changes Mar 2, 2020
View reviewed changes
Copy link
Contributor

@jgrandja jgrandja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates @jzheaux.

In addition to the one comment I left, please update the copyright year in all test *.xml files.

config/src/main/resources/org/springframework/security/config/spring-security-5.3.rnc Outdated
@@ -572,6 +572,48 @@ provider.attlist &=
## The URI used to discover the configuration information for an OAuth 2.0 or OpenID Connect 1.0 Provider.
attribute issuer-uri {xsd:token}?

oauth2-resource-server =
## Configures authentication support as an OAuth 2.0 Resource Server.
element oauth2-resource-server {oauth2-resource-server.attlist}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused how <jwt> and <opaque-token> are <xs:choice>'s in the xsd, however, both elements are not defined as choices in the rnc file? It looks like both those elements do not have <oauth2-resource-server> as their parent. Maybe I'm missing something?

It should be similar to websocket-message-broker:

websocket-message-broker =
	element websocket-message-broker { websocket-message-broker.attrlist, (intercept-message* & expression-handler?) }

(intercept-message* & expression-handler?) are choices

@jzheaux jzheaux merged commit e97396b into spring-projects:master Mar 2, 2020
@jzheaux jzheaux deleted the gh-5185 branch March 2, 2020 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jgrandja jgrandja jgrandja approved these changes

Assignees

@jzheaux jzheaux

Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: duplicate A duplicate of another issue type: enhancement A general enhancement
Projects
None yet
Milestone
5.3.0
Development

Successfully merging this pull request may close these issues.

Provide XML namespace support for OAuth 2.0 Resource Server
3 participants
@jzheaux @jgrandja @eleftherias