From 5b2212bbb6361126f03350e90844cde401e18fcc Mon Sep 17 00:00:00 2001 From: Andre Blanke Date: Sun, 8 Dec 2024 10:41:16 +0100 Subject: [PATCH] Add copy constructor for DefaultOAuth2User Accepts null name for consistency with DefaultOAuth2AuthenticatedPrincipal. The private static method DefaultOAuth2User.getNameFromAttributes was added in order to keep assertions about nameAttributeKey and the looked up name for the deprecated constructor. --- .../ROOT/pages/reactive/test/web/oauth2.adoc | 4 +- .../pages/servlet/test/mockmvc/oauth2.adoc | 4 +- .../jackson2/DefaultOAuth2UserMixin.java | 8 +++- .../userinfo/DefaultOAuth2UserService.java | 2 +- .../DefaultReactiveOAuth2UserService.java | 2 +- .../oauth2/core/user/DefaultOAuth2User.java | 40 +++++++++++++------ .../server/SecurityMockServerConfigurers.java | 3 +- .../SecurityMockMvcRequestPostProcessors.java | 3 +- 8 files changed, 44 insertions(+), 22 deletions(-) diff --git a/docs/modules/ROOT/pages/reactive/test/web/oauth2.adoc b/docs/modules/ROOT/pages/reactive/test/web/oauth2.adoc index 2548d96cf50..0f20a955cdc 100644 --- a/docs/modules/ROOT/pages/reactive/test/web/oauth2.adoc +++ b/docs/modules/ROOT/pages/reactive/test/web/oauth2.adoc @@ -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)) diff --git a/docs/modules/ROOT/pages/servlet/test/mockmvc/oauth2.adoc b/docs/modules/ROOT/pages/servlet/test/mockmvc/oauth2.adoc index 581f49adefa..696fac7688c 100644 --- a/docs/modules/ROOT/pages/servlet/test/mockmvc/oauth2.adoc +++ b/docs/modules/ROOT/pages/servlet/test/mockmvc/oauth2.adoc @@ -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") diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/jackson2/DefaultOAuth2UserMixin.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/jackson2/DefaultOAuth2UserMixin.java index 917062c905c..c5133e541fe 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/jackson2/DefaultOAuth2UserMixin.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/jackson2/DefaultOAuth2UserMixin.java @@ -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. @@ -42,10 +42,16 @@ @JsonIgnoreProperties(ignoreUnknown = true) abstract class DefaultOAuth2UserMixin { + @Deprecated @JsonCreator DefaultOAuth2UserMixin(@JsonProperty("authorities") Collection authorities, @JsonProperty("attributes") Map attributes, @JsonProperty("nameAttributeKey") String nameAttributeKey) { } + @JsonCreator + DefaultOAuth2UserMixin(@JsonProperty("name") String name, + @JsonProperty("attributes") Map attributes, + @JsonProperty("authorities") Collection authorities) { + } } diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/userinfo/DefaultOAuth2UserService.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/userinfo/DefaultOAuth2UserService.java index 02930047b16..5bc9595d8fd 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/userinfo/DefaultOAuth2UserService.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/userinfo/DefaultOAuth2UserService.java @@ -96,7 +96,7 @@ public OAuth2User loadUser(OAuth2UserRequest userRequest) throws OAuth2Authentic OAuth2AccessToken token = userRequest.getAccessToken(); Map attributes = this.attributesConverter.convert(userRequest).convert(response.getBody()); Collection authorities = getAuthorities(token, attributes, userNameAttributeName); - return new DefaultOAuth2User(authorities, attributes, userNameAttributeName); + return new DefaultOAuth2User(attributes.get(userNameAttributeName).toString(), attributes, authorities); } /** diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/userinfo/DefaultReactiveOAuth2UserService.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/userinfo/DefaultReactiveOAuth2UserService.java index ae3a65b52c0..e934b34ecab 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/userinfo/DefaultReactiveOAuth2UserService.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/userinfo/DefaultReactiveOAuth2UserService.java @@ -138,7 +138,7 @@ public Mono loadUser(OAuth2UserRequest userRequest) throws OAuth2Aut authorities.add(new SimpleGrantedAuthority("SCOPE_" + scope)); } - return new DefaultOAuth2User(authorities, attrs, userNameAttributeName); + return new DefaultOAuth2User(attrs.get(userNameAttributeName).toString(), attrs, authorities); }) .onErrorMap((ex) -> (ex instanceof UnsupportedMediaTypeException || ex.getCause() instanceof UnsupportedMediaTypeException), (ex) -> { diff --git a/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/user/DefaultOAuth2User.java b/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/user/DefaultOAuth2User.java index 6c80d7b64a2..ace741fd8e3 100644 --- a/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/user/DefaultOAuth2User.java +++ b/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/user/DefaultOAuth2User.java @@ -36,11 +36,9 @@ * The default implementation of an {@link OAuth2User}. * *

- * User attribute names are not standardized between providers and therefore it is - * required to supply the key for the user's "name" attribute to one of - * the constructors. The key will be used for accessing the "name" of the - * {@code Principal} (user) via {@link #getAttributes()} and returning it from - * {@link #getName()}. + * User attribute names are not standardized between providers, and therefore it is + * required to supply the user's "name" or "name" attribute to one of + * the constructors. * * @author Joe Grandja * @author Eddú Meléndez @@ -56,7 +54,7 @@ public class DefaultOAuth2User implements OAuth2User, Serializable { private final Map attributes; - private final String nameAttributeKey; + private final String name; /** * Constructs a {@code DefaultOAuth2User} using the provided parameters. @@ -65,23 +63,32 @@ public class DefaultOAuth2User implements OAuth2User, Serializable { * @param nameAttributeKey the key used to access the user's "name" from * {@link #getAttributes()} */ + @Deprecated public DefaultOAuth2User(Collection authorities, Map attributes, String nameAttributeKey) { - Assert.notEmpty(attributes, "attributes cannot be empty"); - Assert.hasText(nameAttributeKey, "nameAttributeKey cannot be empty"); - Assert.notNull(attributes.get(nameAttributeKey), - "Attribute value for '" + nameAttributeKey + "' cannot be null"); + this(getNameFromAttributes(attributes, nameAttributeKey), attributes, authorities); + } + /** + * Constructs a {@code DefaultOAuth2User} using the provided parameters. + * @param name the name of the user + * @param authorities the authorities granted to the user + * @param attributes the attributes about the user + */ + public DefaultOAuth2User(String name, Map attributes, + Collection authorities) { + Assert.notNull(name, "name cannot be null"); + Assert.notEmpty(attributes, "attributes cannot be empty"); + this.attributes = Collections.unmodifiableMap(new LinkedHashMap<>(attributes)); this.authorities = (authorities != null) ? Collections.unmodifiableSet(new LinkedHashSet<>(this.sortAuthorities(authorities))) : Collections.unmodifiableSet(new LinkedHashSet<>(AuthorityUtils.NO_AUTHORITIES)); - this.attributes = Collections.unmodifiableMap(new LinkedHashMap<>(attributes)); - this.nameAttributeKey = nameAttributeKey; + this.name = (name != null) ? name : (String) this.attributes.get("sub"); } @Override public String getName() { - return this.getAttribute(this.nameAttributeKey).toString(); + return this.name; } @Override @@ -140,4 +147,11 @@ public String toString() { return sb.toString(); } + private static String getNameFromAttributes(Map attributes, String nameAttributeKey) { + Assert.hasText(nameAttributeKey, "nameAttributeKey cannot be empty"); + Assert.notNull(attributes.get(nameAttributeKey), + "Attribute value for '" + nameAttributeKey + "' cannot be null"); + return attributes.get(nameAttributeKey).toString(); + } + } diff --git a/test/src/main/java/org/springframework/security/test/web/reactive/server/SecurityMockServerConfigurers.java b/test/src/main/java/org/springframework/security/test/web/reactive/server/SecurityMockServerConfigurers.java index 360b7a22982..0cce15b39d1 100644 --- a/test/src/main/java/org/springframework/security/test/web/reactive/server/SecurityMockServerConfigurers.java +++ b/test/src/main/java/org/springframework/security/test/web/reactive/server/SecurityMockServerConfigurers.java @@ -848,7 +848,8 @@ private Map defaultAttributes() { } private OAuth2User defaultPrincipal() { - return new DefaultOAuth2User(this.authorities.get(), this.attributes.get(), this.nameAttributeKey); + String name = this.attributes.get().get(this.nameAttributeKey).toString(); + return new DefaultOAuth2User(name, this.attributes.get(), this.authorities.get()); } } diff --git a/test/src/main/java/org/springframework/security/test/web/servlet/request/SecurityMockMvcRequestPostProcessors.java b/test/src/main/java/org/springframework/security/test/web/servlet/request/SecurityMockMvcRequestPostProcessors.java index 8d0aac3e696..2bba7a50f97 100644 --- a/test/src/main/java/org/springframework/security/test/web/servlet/request/SecurityMockMvcRequestPostProcessors.java +++ b/test/src/main/java/org/springframework/security/test/web/servlet/request/SecurityMockMvcRequestPostProcessors.java @@ -1390,7 +1390,8 @@ private Map defaultAttributes() { } private OAuth2User defaultPrincipal() { - return new DefaultOAuth2User(this.authorities.get(), this.attributes.get(), this.nameAttributeKey); + String name = this.attributes.get().get(this.nameAttributeKey).toString(); + return new DefaultOAuth2User(name, this.attributes.get(), this.authorities.get()); } }