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

Refactor: query minimal user information everywhere authorities not needed #1322

Merged
merged 7 commits into from
Jul 1, 2021
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package org.cloudfoundry.identity.uaa.account;

import org.cloudfoundry.identity.uaa.authentication.UaaPrincipal;
import org.cloudfoundry.identity.uaa.user.UaaUser;
import org.cloudfoundry.identity.uaa.user.UaaUserDatabase;
import org.cloudfoundry.identity.uaa.user.UaaUserPrototype;
import org.cloudfoundry.identity.uaa.user.UserInfo;
import org.springframework.beans.factory.InitializingBean;
import org.springframework.security.oauth2.provider.OAuth2Authentication;
Expand Down Expand Up @@ -53,7 +53,7 @@ protected UaaPrincipal extractUaaPrincipal(OAuth2Authentication authentication)
}

protected UserInfoResponse getResponse(UaaPrincipal principal, boolean addCustomAttributes, boolean addRoles) {
UaaUser user = userDatabase.retrieveUserById(principal.getId());
UaaUserPrototype user = userDatabase.retrieveUserPrototypeById(principal.getId());
UserInfoResponse response = new UserInfoResponse();
response.setUserId(user.getId());
response.setUserName(user.getUsername());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@

package org.cloudfoundry.identity.uaa.authentication;

import org.cloudfoundry.identity.uaa.user.UaaUserPrototype;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.cloudfoundry.identity.uaa.constants.OriginKeys;
import org.cloudfoundry.identity.uaa.user.UaaUser;
import org.cloudfoundry.identity.uaa.user.UaaUserDatabase;
import org.springframework.security.core.context.SecurityContext;
import org.springframework.security.core.context.SecurityContextHolder;
Expand Down Expand Up @@ -65,7 +65,7 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
String userId = authentication.getPrincipal().getId();
try {
logger.debug("Evaluating user-id for session reset:"+userId);
UaaUser user = userDatabase.retrieveUserById(userId);
UaaUserPrototype user = userDatabase.retrieveUserPrototypeById(userId);
Date lastModified;
if ((lastModified = user.getPasswordLastModified()) != null) {
long lastAuthTime = authentication.getAuthenticatedTime();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import org.cloudfoundry.identity.uaa.user.UaaUser;
import org.cloudfoundry.identity.uaa.user.UaaUserPrototype;

/**
* The principal object which should end up as the representation of an
Expand Down Expand Up @@ -45,6 +46,16 @@ public UaaPrincipal(UaaUser user) {
);
}

public UaaPrincipal(UaaUserPrototype userPrototype) {
this(
userPrototype.getId(),
userPrototype.getUsername(),
userPrototype.getEmail(),
userPrototype.getOrigin(),
userPrototype.getExternalId(),
userPrototype.getZoneId()
);
}
@JsonCreator
public UaaPrincipal(
@JsonProperty("id") String id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.cloudfoundry.identity.uaa.user.UaaAuthority;
import org.cloudfoundry.identity.uaa.user.UaaUser;
import org.cloudfoundry.identity.uaa.user.UaaUserDatabase;
import org.cloudfoundry.identity.uaa.user.UaaUserPrototype;
import org.cloudfoundry.identity.uaa.user.UserInfo;
import org.cloudfoundry.identity.uaa.util.JsonUtils;
import org.cloudfoundry.identity.uaa.util.TimeService;
Expand Down Expand Up @@ -265,7 +266,7 @@ public OAuth2AccessToken refreshAccessToken(String refreshTokenValue, TokenReque

boolean isRevocable = isOpaque || (revocableClaim == null ? false : revocableClaim);

UaaUser user = userDatabase.retrieveUserById(userId);
UaaUser user = new UaaUser(userDatabase.retrieveUserPrototypeById(userId));
BaseClientDetails client = (BaseClientDetails) clientDetailsService.loadClientByClientId(clientId);

long refreshTokenExpireMillis = refreshTokenExpirySeconds.longValue() * 1000L;
Expand Down Expand Up @@ -823,7 +824,7 @@ public OAuth2Authentication loadAuthentication(String accessToken) throws Authen
Authentication userAuthentication = null;
// Is this a user token - minimum info is user_id
if (claims.containsKey(USER_ID)) {
UaaUser user = userDatabase.retrieveUserById((String)claims.get(USER_ID));
UaaUserPrototype user = userDatabase.retrieveUserPrototypeById((String)claims.get(USER_ID));
UaaPrincipal principal = new UaaPrincipal(user);
userAuthentication = new UaaAuthentication(principal, UaaAuthority.USER_AUTHORITIES, null);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -406,14 +406,14 @@ protected UaaUser userAuthenticated(Authentication request, UaaUser userFromRequ
if (is_invitation_acceptance) {
String invitedUserId = (String) RequestContextHolder.currentRequestAttributes().getAttribute("user_id", RequestAttributes.SCOPE_SESSION);
logger.debug("ExternalOAuth user accepted invitation, user_id:"+invitedUserId);
userFromDb = getUserDatabase().retrieveUserById(invitedUserId);
userFromDb = new UaaUser(getUserDatabase().retrieveUserPrototypeById(invitedUserId));
if (email != null) {
if (!email.equalsIgnoreCase(userFromDb.getEmail())) {
throw new BadCredentialsException("OAuth User email mismatch. Authenticated email doesn't match invited email.");
}
}
publish(new InvitedUserAuthenticatedEvent(userFromDb));
userFromDb = getUserDatabase().retrieveUserById(invitedUserId);
userFromDb = new UaaUser(getUserDatabase().retrieveUserPrototypeById(invitedUserId));
}

//we must check and see if the email address has changed between authentications
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.cloudfoundry.identity.uaa.scim.ScimGroupExternalMembershipManager;
import org.cloudfoundry.identity.uaa.user.UaaUser;
import org.cloudfoundry.identity.uaa.user.UaaUserDatabase;
import org.cloudfoundry.identity.uaa.user.UaaUserPrototype;
import org.cloudfoundry.identity.uaa.user.UserInfo;
import org.cloudfoundry.identity.uaa.util.UaaUrlUtils;
import org.cloudfoundry.identity.uaa.web.UaaSavedRequestAwareAuthenticationSuccessHandler;
Expand Down Expand Up @@ -358,18 +359,18 @@ protected UaaUser createIfMissing(UaaPrincipal samlPrincipal, boolean addNew, Co
user = userDatabase.retrieveUserByName(samlPrincipal.getName(), samlPrincipal.getOrigin());
}
} catch (UsernameNotFoundException e) {
UaaUser uaaUser = userDatabase.retrieveUserByEmail(userWithSamlAttributes.getEmail(), samlPrincipal.getOrigin());
UaaUserPrototype uaaUser = userDatabase.retrieveUserPrototypeByEmail(userWithSamlAttributes.getEmail(), samlPrincipal.getOrigin());
if (uaaUser != null) {
userModified = true;
user = uaaUser.modifyUsername(samlPrincipal.getName());
user = new UaaUser(uaaUser.withUsername(samlPrincipal.getName()));
} else {
if (!addNew) {
throw new LoginSAMLException("SAML user does not exist. "
+ "You can correct this by creating a shadow user for the SAML user.", e);
}
publish(new NewUserAuthenticatedEvent(userWithSamlAttributes));
try {
user = userDatabase.retrieveUserByName(samlPrincipal.getName(), samlPrincipal.getOrigin());
user = new UaaUser(userDatabase.retrieveUserPrototypeByName(samlPrincipal.getName(), samlPrincipal.getOrigin()));
} catch (UsernameNotFoundException ex) {
throw new BadCredentialsException("Unable to establish shadow user for SAML user:" + samlPrincipal.getName());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ public class JdbcUaaUserDatabase implements UaaUserDatabase {
private int maxSqlParameters;

private final RowMapper<UaaUser> mapper = new UaaUserRowMapper();
private final RowMapper<UaaUserPrototype> minimalMapper = new UaaUserPrototypeRowMapper();
private final RowMapper<UserInfo> userInfoMapper = new UserInfoRowMapper();

RowMapper<UaaUser> getMapper() {
Expand Down Expand Up @@ -84,6 +85,16 @@ public UaaUser retrieveUserByName(String username, String origin) throws Usernam
}
}

@Override
public UaaUserPrototype retrieveUserPrototypeByName(String username, String origin) throws UsernameNotFoundException {
try {
String sql = caseInsensitive ? DEFAULT_CASE_INSENSITIVE_USER_BY_USERNAME_QUERY : DEFAULT_CASE_SENSITIVE_USER_BY_USERNAME_QUERY;
return jdbcTemplate.queryForObject(sql, minimalMapper, username.toLowerCase(Locale.US), true, origin, identityZoneManager.getCurrentIdentityZoneId());
} catch (EmptyResultDataAccessException e) {
throw new UsernameNotFoundException(username);
}
}

@Override
public UaaUser retrieveUserById(String id) throws UsernameNotFoundException {
try {
Expand All @@ -93,6 +104,15 @@ public UaaUser retrieveUserById(String id) throws UsernameNotFoundException {
}
}

@Override
public UaaUserPrototype retrieveUserPrototypeById(String id) throws UsernameNotFoundException {
try {
return jdbcTemplate.queryForObject(DEFAULT_USER_BY_ID_QUERY, minimalMapper, id, true, identityZoneManager.getCurrentIdentityZoneId());
} catch (EmptyResultDataAccessException e) {
throw new UsernameNotFoundException(id);
}
}

@Override
public UaaUser retrieveUserByEmail(String email, String origin) throws UsernameNotFoundException {
String sql = caseInsensitive ? DEFAULT_CASE_INSENSITIVE_USER_BY_EMAIL_AND_ORIGIN_QUERY : DEFAULT_CASE_SENSITIVE_USER_BY_EMAIL_AND_ORIGIN_QUERY;
Expand All @@ -106,6 +126,19 @@ public UaaUser retrieveUserByEmail(String email, String origin) throws UsernameN
}
}

@Override
public UaaUserPrototype retrieveUserPrototypeByEmail(String email, String origin) throws UsernameNotFoundException {
String sql = caseInsensitive ? DEFAULT_CASE_INSENSITIVE_USER_BY_EMAIL_AND_ORIGIN_QUERY : DEFAULT_CASE_SENSITIVE_USER_BY_EMAIL_AND_ORIGIN_QUERY;
List<UaaUserPrototype> results = jdbcTemplate.query(sql, minimalMapper, email.toLowerCase(Locale.US), true, origin, identityZoneManager.getCurrentIdentityZoneId());
if (results.size() == 0) {
return null;
} else if (results.size() == 1) {
return results.get(0);
} else {
throw new IncorrectResultSizeDataAccessException(String.format("Multiple users match email=%s origin=%s", email, origin), 1, results.size());
}
}

@Override
public UserInfo getUserInfo(String id) {
try {
Expand Down Expand Up @@ -139,6 +172,39 @@ public void updateLastLogonTime(String userId) {
jdbcTemplate.update(DEFAULT_UPDATE_USER_LAST_LOGON, timeService.getCurrentTimeMillis(), userId, identityZoneManager.getCurrentIdentityZoneId());
}

private UaaUserPrototype getUaaUserPrototype(ResultSet rs) throws SQLException {
String id = rs.getString("id");
UaaUserPrototype prototype = new UaaUserPrototype().withId(id)
.withUsername(rs.getString("username"))
.withPassword(rs.getString("password"))
.withEmail(rs.getString("email"))
.withGivenName(rs.getString("givenName"))
.withFamilyName(rs.getString("familyName"))
.withCreated(rs.getTimestamp("created"))
.withModified(rs.getTimestamp("lastModified"))
.withOrigin(rs.getString("origin"))
.withExternalId(rs.getString("external_id"))
.withVerified(rs.getBoolean("verified"))
.withZoneId(rs.getString("identity_zone_id"))
.withSalt(rs.getString("salt"))
.withPasswordLastModified(rs.getTimestamp("passwd_lastmodified"))
.withPhoneNumber(rs.getString("phoneNumber"))
.withLegacyVerificationBehavior(rs.getBoolean("legacy_verification_behavior"))
.withPasswordChangeRequired(rs.getBoolean("passwd_change_required"));

Long lastLogon = rs.getLong("last_logon_success_time");
if (rs.wasNull()) {
lastLogon = null;
}
Long previousLogon = rs.getLong("previous_logon_success_time");
if (rs.wasNull()) {
previousLogon = null;
}
prototype.withLastLogonSuccess(lastLogon)
.withPreviousLogonSuccess(previousLogon);
return prototype;
}

private final class UserInfoRowMapper implements RowMapper<UserInfo> {
@Override
public UserInfo mapRow(ResultSet rs, int rowNum) throws SQLException {
Expand All @@ -147,45 +213,21 @@ public UserInfo mapRow(ResultSet rs, int rowNum) throws SQLException {
}
}

private final class UaaUserRowMapper implements RowMapper<UaaUser> {
private final class UaaUserPrototypeRowMapper implements RowMapper<UaaUserPrototype> {
@Override
public UaaUserPrototype mapRow(ResultSet rs, int rowNum) throws SQLException {
return getUaaUserPrototype(rs);
}
}

public UaaUser mapRow(ResultSet rs, int rowNum) throws SQLException {
String id = rs.getString("id");
UaaUserPrototype prototype = new UaaUserPrototype().withId(id)
.withUsername(rs.getString("username"))
.withPassword(rs.getString("password"))
.withEmail(rs.getString("email"))
.withGivenName(rs.getString("givenName"))
.withFamilyName(rs.getString("familyName"))
.withCreated(rs.getTimestamp("created"))
.withModified(rs.getTimestamp("lastModified"))
.withOrigin(rs.getString("origin"))
.withExternalId(rs.getString("external_id"))
.withVerified(rs.getBoolean("verified"))
.withZoneId(rs.getString("identity_zone_id"))
.withSalt(rs.getString("salt"))
.withPasswordLastModified(rs.getTimestamp("passwd_lastmodified"))
.withPhoneNumber(rs.getString("phoneNumber"))
.withLegacyVerificationBehavior(rs.getBoolean("legacy_verification_behavior"))
.withPasswordChangeRequired(rs.getBoolean("passwd_change_required"));

Long lastLogon = rs.getLong("last_logon_success_time");
if (rs.wasNull()) {
lastLogon = null;
private final class UaaUserRowMapper implements RowMapper<UaaUser> {
@Override
public UaaUser mapRow(ResultSet rs, int rowNum) throws SQLException {
UaaUserPrototype prototype = getUaaUserPrototype(rs);
List<GrantedAuthority> authorities =
AuthorityUtils.commaSeparatedStringToAuthorityList(getAuthorities(prototype.getId()));
return new UaaUser(prototype.withAuthorities(authorities));
}
Long previousLogon = rs.getLong("previous_logon_success_time");
if (rs.wasNull()) {
previousLogon = null;
}
prototype.withLastLogonSuccess(lastLogon)
.withPreviousLogonSuccess(previousLogon);


List<GrantedAuthority> authorities =
AuthorityUtils.commaSeparatedStringToAuthorityList(getAuthorities(id));
return new UaaUser(prototype.withAuthorities(authorities));
}

private String getAuthorities(final String userId) {
Set<String> authorities = new HashSet<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import java.util.Collection;
import java.util.Date;
import java.util.List;
import java.util.Optional;
import java.util.function.Consumer;

/**
Expand Down Expand Up @@ -208,7 +209,7 @@ public String getSalt() {
}

public List<? extends GrantedAuthority> getAuthorities() {
return authorities;
return Optional.ofNullable(authorities).orElseThrow();
}

public UaaUser id(String id) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,16 @@
public interface UaaUserDatabase {
UaaUser retrieveUserByName(String username, String origin) throws UsernameNotFoundException;

UaaUserPrototype retrieveUserPrototypeByName(String username, String origin) throws UsernameNotFoundException;

UaaUser retrieveUserById(String id) throws UsernameNotFoundException;

UaaUserPrototype retrieveUserPrototypeById(String id) throws UsernameNotFoundException;

UaaUser retrieveUserByEmail(String email, String origin) throws UsernameNotFoundException;

UaaUserPrototype retrieveUserPrototypeByEmail(String email, String origin) throws UsernameNotFoundException;

UserInfo getUserInfo(String id);

UserInfo storeUserInfo(String id, UserInfo info);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.cloudfoundry.identity.uaa.user.UaaAuthority;
import org.cloudfoundry.identity.uaa.user.UaaUser;
import org.cloudfoundry.identity.uaa.user.UaaUserDatabase;
import org.cloudfoundry.identity.uaa.user.UaaUserPrototype;
import org.cloudfoundry.identity.uaa.util.JsonUtils;
import org.cloudfoundry.identity.uaa.util.TimeService;
import org.cloudfoundry.identity.uaa.zone.IdentityZone;
Expand Down Expand Up @@ -103,6 +104,7 @@ public class CheckTokenEndpointTests {
private boolean useOpaque;

private AuthorizationRequest authorizationRequest = null;
private UaaUserPrototype uaaUserPrototype;
private UaaUser user;
private BaseClientDetails defaultClient;
private Map<String, BaseClientDetails> clientDetailsStore;
Expand Down Expand Up @@ -241,6 +243,7 @@ public void setUp(boolean opaque) throws Exception {
IdentityZoneHolder.get().getId(),
"salt",
new Date(nowMillis - 2000));
uaaUserPrototype = new UaaUserPrototype(user).withAuthorities(null);
authorizationRequest = new AuthorizationRequest("client", Collections.singleton("read"));
authorizationRequest.setResourceIds(new HashSet<>(Arrays.asList("client", "scim")));
Map<String, String> requestParameters = new HashMap<>();
Expand Down Expand Up @@ -335,6 +338,8 @@ private void resetAndMockUserDatabase(String userId, UaaUser user) {
reset(userDatabase);
when(userDatabase.retrieveUserById(eq(userId))).thenReturn(user);
when(userDatabase.retrieveUserById(not(eq(userId)))).thenThrow(new UsernameNotFoundException("mock"));
when(userDatabase.retrieveUserPrototypeById(eq(userId))).thenReturn(uaaUserPrototype);
when(userDatabase.retrieveUserPrototypeById(not(eq(userId)))).thenThrow(new UsernameNotFoundException("mock"));
}

@Test
Expand Down