Skip to content

OAuth2 Resource Server docs not in sync - authorityPrefix can't be set to "" #8421

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
judomu opened this issue Apr 21, 2020 · 4 comments · Fixed by #8424
Closed

OAuth2 Resource Server docs not in sync - authorityPrefix can't be set to "" #8421

judomu opened this issue Apr 21, 2020 · 4 comments · Fixed by #8424
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: backported An issue that has been backported to maintenance branches type: bug A general bug
Milestone

Comments

@judomu
Copy link
Contributor

judomu commented Apr 21, 2020

In spring-security documentation (https://docs.spring.io/spring-security/site/docs/current/reference/html5/):

Or, you can remove the prefix altogether by calling JwtGrantedAuthoritiesConverter#setAuthorityPrefix("").

But that will not work

	public void setAuthorityPrefix(String authorityPrefix) {
		Assert.hasText(authorityPrefix, "authorityPrefix cannot be empty");
		this.authorityPrefix = authorityPrefix;
	}

So the documentation is wrong or the implementation, imho the option to set authorityPrefix to "" makes sense to me.

@judomu judomu added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Apr 21, 2020
@rwinch
Copy link
Member

rwinch commented Apr 21, 2020

Thanks for the report. I think it is the implementation that is broken. Would you like to submit a PR to fix it?

@rwinch rwinch added status: ideal-for-contribution An issue that we actively are looking for someone to help us with in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) and removed status: waiting-for-triage An issue we've not yet triaged labels Apr 21, 2020
@judomu
Copy link
Contributor Author

judomu commented Apr 21, 2020

Sure, I will provide the fix

@rwinch
Copy link
Member

rwinch commented Apr 21, 2020

Thanks @judomu! The issue is yours

judomu added a commit to judomu/spring-security that referenced this issue Apr 21, 2020
- docs stated that empty authorityPrefix are allowed but implementation denied to use `""`
- commit removes the `hasText`-limitation but restricts to `notNull`

Fixes spring-projectsgh-8421
@rwinch rwinch added this to the 5.4.0.M1 milestone Apr 22, 2020
rwinch pushed a commit that referenced this issue Apr 22, 2020
- docs stated that empty authorityPrefix are allowed but implementation denied to use `""`
- commit removes the `hasText`-limitation but restricts to `notNull`

Fixes gh-8421
@rwinch rwinch removed the status: ideal-for-contribution An issue that we actively are looking for someone to help us with label Apr 22, 2020
rwinch pushed a commit that referenced this issue Apr 22, 2020
- docs stated that empty authorityPrefix are allowed but implementation denied to use `""`
- commit removes the `hasText`-limitation but restricts to `notNull`

Fixes gh-8421
@spring-projects-issues spring-projects-issues added the status: backported An issue that has been backported to maintenance branches label Apr 22, 2020
@rwinch
Copy link
Member

rwinch commented Apr 22, 2020

This is now merged into master and 5.2.x

eleftherias pushed a commit that referenced this issue Jul 7, 2020
- docs stated that empty authorityPrefix are allowed but implementation denied to use `""`
- commit removes the `hasText`-limitation but restricts to `notNull`

Fixes gh-8421
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) status: backported An issue that has been backported to maintenance branches type: bug A general bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants