Skip to content

Spring Security 5 compatability with FixedAuthoritiesExtractor #5625

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
ddewaele opened this issue Aug 1, 2018 · 1 comment
Closed

Spring Security 5 compatability with FixedAuthoritiesExtractor #5625

ddewaele opened this issue Aug 1, 2018 · 1 comment
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose)

Comments

@ddewaele
Copy link

ddewaele commented Aug 1, 2018

In the Spring Security 4 based OAuth2 library, a FixedAuthoritiesExtractor was used by default that looked for an authorities field in the userInfo to extract authorities (and defaulted to ROLE_USER if none was found).

In the Spring Security 5 OAuth2 library, there doesn't seem to be a default AuthoritiesExtractor.
Instead, an OAuth2UserAuthority is used that always seems to default to ROLE_USER. It doesn't bother to look for an authorities field anymore.

I realise a custom GrantedAuthoritiesMapper can be provided like the one below, but this results in code like this that isn't too clean ...

private GrantedAuthoritiesMapper userAuthoritiesMapper() {
	return (authorities) -> {
		Set<GrantedAuthority> mappedAuthorities = new HashSet<>();
		authorities.forEach(authority -> {
			if (OAuth2UserAuthority.class.isInstance(authority)) {
				OAuth2UserAuthority oAuth2UserAuthority = (OAuth2UserAuthority)authority;
				List<String> groups = (List<String>)oAuth2UserAuthority.getAttributes().get("authorities");
				mappedAuthorities.addAll(groups.stream().map(group -> new SimpleGrantedAuthority(group)).collect(Collectors.toSet()));
			}
		});
		return mappedAuthorities;
	};
}

Is this by design (due to the fact that loading authorities isn't standarised in oauth2 / openid connect ) and/or is there a better way of dealing with this ?

Thanks a lot.

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

jgrandja commented Aug 1, 2018

@ddewaele Thanks for the feedback!

loading authorities isn't standarised in oauth2 / openid connect

Correct, this is not standardized and I'm not sure that it can be standardized either. Authority information can come from any source and possibly from multiple sources. It really depends how access is configured within the app and/or organization. So all we can really provide is hook(s) that will allow the user to map the authorities from whatever source.

Also, even though FixedAuthoritiesExtractor attempted to extract authority information from the UserInfo resource, the UserInfo resource might not contain any authority information and the same condition could apply to the ID Token as well. The bottom line is that depending on the Provider and the application/organization, authority information can come from any source so we just ensure you have the hooks to implement your custom mapping.

See the reference on the recommended way to map custom authorities:

Using a GrantedAuthoritiesMapper

Delegation-based strategy with OAuth2UserService

Here is a related ticket #5349 but this will not make it into 5.1.

I'm going to close this issue as I feel I have answered your question.
If I haven't than I'll re-open to discuss further.

@jgrandja jgrandja closed this as completed Aug 1, 2018
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

2 participants