Skip to content
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

Expose getter for nameAttributeKey in OAuth2AuthenticatedPrincipal #16003

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
4 changes: 2 additions & 2 deletions docs/modules/ROOT/pages/reactive/test/web/oauth2.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -509,9 +509,9 @@ Java::
[source,java,role="primary"]
----
OAuth2User oauth2User = new DefaultOAuth2User(
AuthorityUtils.createAuthorityList("SCOPE_message:read"),
"foo_user",
Collections.singletonMap("user_name", "foo_user"),
"user_name");
AuthorityUtils.createAuthorityList("SCOPE_message:read"));

client
.mutateWith(mockOAuth2Login().oauth2User(oauth2User))
Expand Down
4 changes: 2 additions & 2 deletions docs/modules/ROOT/pages/servlet/test/mockmvc/oauth2.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -514,9 +514,9 @@ Java::
[source,java,role="primary"]
----
OAuth2User oauth2User = new DefaultOAuth2User(
AuthorityUtils.createAuthorityList("SCOPE_message:read"),
"foo_user",
Collections.singletonMap("user_name", "foo_user"),
"user_name");
AuthorityUtils.createAuthorityList("SCOPE_message:read"));

mvc
.perform(get("/endpoint")
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2020 the original author or authors.
* Copyright 2002-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -42,10 +42,16 @@
@JsonIgnoreProperties(ignoreUnknown = true)
abstract class DefaultOAuth2UserMixin {

@Deprecated
@JsonCreator
DefaultOAuth2UserMixin(@JsonProperty("authorities") Collection<? extends GrantedAuthority> authorities,
@JsonProperty("attributes") Map<String, Object> attributes,
@JsonProperty("nameAttributeKey") String nameAttributeKey) {
}

@JsonCreator
DefaultOAuth2UserMixin(@JsonProperty("name") String name,
@JsonProperty("attributes") Map<String, Object> attributes,
@JsonProperty("authorities") Collection<? extends GrantedAuthority> authorities) {
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,17 @@
@JsonIgnoreProperties(value = { "attributes" }, ignoreUnknown = true)
abstract class DefaultOidcUserMixin {

@Deprecated
@JsonCreator
DefaultOidcUserMixin(@JsonProperty("authorities") Collection<? extends GrantedAuthority> authorities,
@JsonProperty("idToken") OidcIdToken idToken, @JsonProperty("userInfo") OidcUserInfo userInfo,
@JsonProperty("nameAttributeKey") String nameAttributeKey) {
}

@JsonCreator
DefaultOidcUserMixin(@JsonProperty("name") String name,
Copy link
Contributor

Choose a reason for hiding this comment

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

Will you please add DefaultOAuth2UserMixinTests and DefaultOidcUserMixinTests to ensure that serializations of either arrangement can correctly deserialize?

Copy link
Author

Choose a reason for hiding this comment

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

It seems that Jackson only supports a single @JsonCreator for each class, meaning the backwards compatibility would be a bit more involved (perhaps using a custom deserializer?).

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, the next evolution of that mixin would be to use a deserializer.

@JsonProperty("idToken") OidcIdToken idToken, @JsonProperty("userInfo") OidcUserInfo userInfo,
@JsonProperty("authorities") Collection<? extends GrantedAuthority> authorities) {
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.springframework.security.oauth2.client.oidc.userinfo;

import java.util.LinkedHashSet;
import java.util.Map;
import java.util.Set;

import org.springframework.security.core.GrantedAuthority;
Expand All @@ -28,6 +29,7 @@
import org.springframework.security.oauth2.core.oidc.user.DefaultOidcUser;
import org.springframework.security.oauth2.core.oidc.user.OidcUser;
import org.springframework.security.oauth2.core.oidc.user.OidcUserAuthority;
import org.springframework.security.oauth2.core.user.DefaultOAuth2User;
import org.springframework.util.CollectionUtils;
import org.springframework.util.StringUtils;

Expand Down Expand Up @@ -90,10 +92,15 @@ static OidcUser getUser(OidcUserRequest userRequest, OidcUserInfo userInfo) {
for (String scope : token.getScopes()) {
authorities.add(new SimpleGrantedAuthority("SCOPE_" + scope));
}
DefaultOidcUser.Builder userBuilder = new DefaultOidcUser.Builder();
if (StringUtils.hasText(userNameAttributeName)) {
return new DefaultOidcUser(authorities, userRequest.getIdToken(), userInfo, userNameAttributeName);
userBuilder.nameAttributeKey(userNameAttributeName);
}
return new DefaultOidcUser(authorities, userRequest.getIdToken(), userInfo);
return userBuilder
.idToken(userRequest.getIdToken())
.userInfo(userInfo)
.authorities(authorities)
.build();
}

private OidcUserRequestUtils() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,11 @@ public OAuth2User loadUser(OAuth2UserRequest userRequest) throws OAuth2Authentic
OAuth2AccessToken token = userRequest.getAccessToken();
Map<String, Object> attributes = this.attributesConverter.convert(userRequest).convert(response.getBody());
Collection<GrantedAuthority> authorities = getAuthorities(token, attributes, userNameAttributeName);
return new DefaultOAuth2User(authorities, attributes, userNameAttributeName);
return new DefaultOAuth2User.Builder()
.nameAttributeKey(userNameAttributeName)
.attributes(attributes)
.authorities(authorities)
.build();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,11 @@ public Mono<OAuth2User> loadUser(OAuth2UserRequest userRequest) throws OAuth2Aut
authorities.add(new SimpleGrantedAuthority("SCOPE_" + scope));
}

return new DefaultOAuth2User(authorities, attrs, userNameAttributeName);
return new DefaultOAuth2User.Builder()
.nameAttributeKey(userNameAttributeName)
.attributes(attrs)
.authorities(authorities)
.build();
})
.onErrorMap((ex) -> (ex instanceof UnsupportedMediaTypeException ||
ex.getCause() instanceof UnsupportedMediaTypeException), (ex) -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ public class DefaultOidcUser extends DefaultOAuth2User implements OidcUser {
* @param authorities the authorities granted to the user
* @param idToken the {@link OidcIdToken ID Token} containing claims about the user
*/
@Deprecated
public DefaultOidcUser(Collection<? extends GrantedAuthority> authorities, OidcIdToken idToken) {
this(authorities, idToken, IdTokenClaimNames.SUB);
}
Expand All @@ -62,6 +63,7 @@ public DefaultOidcUser(Collection<? extends GrantedAuthority> authorities, OidcI
* @param nameAttributeKey the key used to access the user's &quot;name&quot; from
* {@link #getAttributes()}
*/
@Deprecated
public DefaultOidcUser(Collection<? extends GrantedAuthority> authorities, OidcIdToken idToken,
String nameAttributeKey) {
this(authorities, idToken, null, nameAttributeKey);
Expand All @@ -74,6 +76,7 @@ public DefaultOidcUser(Collection<? extends GrantedAuthority> authorities, OidcI
* @param userInfo the {@link OidcUserInfo UserInfo} containing claims about the user,
* may be {@code null}
*/
@Deprecated
public DefaultOidcUser(Collection<? extends GrantedAuthority> authorities, OidcIdToken idToken,
OidcUserInfo userInfo) {
this(authorities, idToken, userInfo, IdTokenClaimNames.SUB);
Expand All @@ -88,13 +91,29 @@ public DefaultOidcUser(Collection<? extends GrantedAuthority> authorities, OidcI
* @param nameAttributeKey the key used to access the user's &quot;name&quot; from
* {@link #getAttributes()}
*/
@Deprecated
public DefaultOidcUser(Collection<? extends GrantedAuthority> authorities, OidcIdToken idToken,
OidcUserInfo userInfo, String nameAttributeKey) {
super(authorities, OidcUserAuthority.collectClaims(idToken, userInfo), nameAttributeKey);
this.idToken = idToken;
this.userInfo = userInfo;
}

/**
* Constructs a {@code DefaultOidcUser} using the provided parameters.
* @param name the name of the user
* @param idToken the {@link OidcIdToken ID Token} containing claims about the user
* @param userInfo the {@link OidcUserInfo UserInfo} containing claims about the user,
* may be {@code null}
* @param authorities the authorities granted to the user
*/
public DefaultOidcUser(String name, OidcIdToken idToken, OidcUserInfo userInfo,
Collection<? extends GrantedAuthority> authorities) {
super(name, OidcUserAuthority.collectClaims(idToken, userInfo), authorities);
this.idToken = idToken;
this.userInfo = userInfo;
}

@Override
public Map<String, Object> getClaims() {
return this.getAttributes();
Expand All @@ -110,4 +129,82 @@ public OidcUserInfo getUserInfo() {
return this.userInfo;
}

public static class Builder {

private String name;

private String nameAttributeKey;

private OidcIdToken idToken;

private OidcUserInfo userInfo;

private Collection<? extends GrantedAuthority> authorities;

/**
* Sets the name of the user.
* @param name the name of the user
* @return the {@link Builder}
*/
public Builder name(String name) {
this.name = name;
return this;
}

/**
* Sets the key used to access the user's &quot;name&quot; from the user attributes if no &quot;name&quot; is
* provided.
* @param nameAttributeKey the key used to access the user's &quot;name&quot; from the user attributes.
* @return the {@link Builder}
*/
public Builder nameAttributeKey(String nameAttributeKey) {
this.nameAttributeKey = nameAttributeKey;
return this;
}

/**
* Sets the {@link OidcIdToken ID Token} containing claims about the user.
* @param idToken the {@link OidcIdToken ID Token} containing claims about the user.
* @return the {@link Builder}
*/
public Builder idToken(OidcIdToken idToken) {
this.idToken = idToken;
return this;
}

/**
* Sets the {@link OidcUserInfo UserInfo} containing claims about the user.
* @param userInfo the {@link OidcUserInfo UserInfo} containing claims about the user.
* @return the {@link Builder}
*/
public Builder userInfo(OidcUserInfo userInfo) {
this.userInfo = userInfo;
return this;
}

/**
* Sets the authorities granted to the user.
* @param authorities the authorities granted to the user
* @return the {@link Builder}
*/
public Builder authorities(Collection<? extends GrantedAuthority> authorities) {
this.authorities = authorities;
return this;
}

/**
* Builds a new {@link DefaultOidcUser}.
* @return a {@link DefaultOidcUser}
*/
public DefaultOidcUser build() {
String name = this.name;
if (name == null) {
Map<String, Object> attributes = OidcUserAuthority.collectClaims(this.idToken, userInfo);
name = getNameFromAttributes(attributes, this.nameAttributeKey);
}
return new DefaultOidcUser(name, idToken, userInfo, authorities);
}

}

}
Loading