Allow UserAuthInfo to contain either old GAE Users or new console Users (#1744)

This means that LegacyAuthenticationMechanism or a to-be-created
OAuth2AuthenticationMechanism) can return a UserAuthInfo object that
contains either the GAE User or the console User as appropriate. The
goal is that the non-auth flows shouldn't have to know about which user
type it is. Note: the registry lock flow (for now) needs to know about
the separate types of auth because it is a separate level of auth from
the standard AuthenticatedRegistrarAccessor.

The AuthenticatedRegistrarAccessor code is a bit odd because the new
role system doesn't quite fit neatly into the old registrar ->
OWNER,ADMIN system but this is a fine approximation. Basically, any
new registrar role will map to the old OWNER role.
This commit is contained in:
gbrodman
2022-08-24 14:18:32 -04:00
committed by GitHub
parent db6329a070
commit 3412f4417f
13 changed files with 416 additions and 167 deletions
@@ -108,7 +108,7 @@ public final class RdapModule {
if (userAuthInfo.isUserAdmin()) {
return RdapAuthorization.ADMINISTRATOR_AUTHORIZATION;
}
ImmutableSet<String> clientIds = registrarAccessor.getAllClientIdWithRoles().keySet();
ImmutableSet<String> clientIds = registrarAccessor.getAllRegistrarIdsWithRoles().keySet();
if (clientIds.isEmpty()) {
logger.atWarning().log("Couldn't find registrar for User %s.", authResult.userIdForLogging());
return RdapAuthorization.PUBLIC_AUTHORIZATION;
@@ -42,7 +42,7 @@ public abstract class AuthResult {
userAuthInfo ->
String.format(
"%s %s",
userAuthInfo.isUserAdmin() ? "admin" : "user", userAuthInfo.user().getEmail()))
userAuthInfo.isUserAdmin() ? "admin" : "user", userAuthInfo.getEmailAddress()))
.orElse("<logged-out user>");
}
@@ -40,8 +40,8 @@ import javax.inject.Inject;
* <p>A user has OWNER role on a Registrar if there exists a {@link RegistrarPoc} with that user's
* gaeId and the registrar as a parent.
*
* <p>An "admin" has in addition OWNER role on {@code #registryAdminClientId} and to all non-{@code
* REAL} registrars (see {@link Registrar#getType}).
* <p>An "admin" has in addition OWNER role on {@code #registryAdminRegistrarId} and to all
* non-{@code REAL} registrars (see {@link Registrar#getType}).
*
* <p>An "admin" also has ADMIN role on ALL registrars.
*
@@ -76,7 +76,7 @@ public class AuthenticatedRegistrarAccessor {
private final boolean isAdmin;
/**
* Gives all roles a user has for a given clientId.
* Gives all roles a user has for a given registrar ID.
*
* <p>The order is significant, with "more specific to this user" coming first.
*
@@ -107,13 +107,13 @@ public class AuthenticatedRegistrarAccessor {
@Inject
public AuthenticatedRegistrarAccessor(
AuthResult authResult,
@Config("registryAdminClientId") String registryAdminClientId,
@Config("registryAdminClientId") String registryAdminRegistrarId,
@Config("gSuiteSupportGroupEmailAddress") Optional<String> gSuiteSupportGroupEmailAddress,
Lazy<GroupsConnection> lazyGroupsConnection) {
this.isAdmin = userIsAdmin(authResult, gSuiteSupportGroupEmailAddress, lazyGroupsConnection);
this.userIdForLogging = authResult.userIdForLogging();
this.roleMap = createRoleMap(authResult, this.isAdmin, registryAdminClientId);
this.roleMap = createRoleMap(authResult, this.isAdmin, registryAdminRegistrarId);
logger.atInfo().log("%s has the following roles: %s", userIdForLogging(), roleMap);
}
@@ -129,7 +129,7 @@ public class AuthenticatedRegistrarAccessor {
* Creates a "logged-in user" accessor with a given role map, used for tests.
*
* <p>The user will be allowed to create Registrars (and hence do OT&amp;E setup) iff they have
* the role of ADMIN for at least one clientId.
* the role of ADMIN for at least one registrar ID.
*
* <p>The user's "name" in logs and exception messages is "TestUserId".
*/
@@ -148,59 +148,62 @@ public class AuthenticatedRegistrarAccessor {
}
/**
* A map that gives all roles a user has for a given clientId.
* A map that gives all roles a user has for a given registrar ID.
*
* <p>Throws a {@link RegistrarAccessDeniedException} if the user is not logged in.
*
* <p>The result is ordered starting from "most specific to this user".
*
* <p>If you want to load the {@link Registrar} object from these (or any other) {@code clientId},
* in order to perform actions on behalf of a user, you must use {@link #getRegistrar} which makes
* sure the user has permissions.
* <p>If you want to load the {@link Registrar} object from these (or any other) {@code
* registrarId}, in order to perform actions on behalf of a user, you must use {@link
* #getRegistrar} which makes sure the user has permissions.
*
* <p>Note that this is an OPTIONAL step in the authentication - only used if we don't have any
* other clue as to the requested {@code clientId}. It is perfectly OK to get a {@code clientId}
* from any other source, as long as the registrar is then loaded using {@link #getRegistrar}.
* other clue as to the requested {@code registrarId}. It is perfectly OK to get a {@code
* registrarId} from any other source, as long as the registrar is then loaded using {@link
* #getRegistrar}.
*/
public ImmutableSetMultimap<String, Role> getAllClientIdWithRoles() {
public ImmutableSetMultimap<String, Role> getAllRegistrarIdsWithRoles() {
return roleMap;
}
/**
* Returns all the roles the current user has on the given registrar.
*
* <p>This is syntactic sugar for {@code getAllClientIdWithRoles().get(clientId)}.
* <p>This is syntactic sugar for {@code getAllRegistrarIdsWithRoles().get(registrarId)}.
*/
public ImmutableSet<Role> getRolesForRegistrar(String clientId) {
return getAllClientIdWithRoles().get(clientId);
public ImmutableSet<Role> getRolesForRegistrar(String registrarId) {
return getAllRegistrarIdsWithRoles().get(registrarId);
}
/**
* Checks if we have a given role for a given registrar.
*
* <p>This is syntactic sugar for {@code getAllClientIdWithRoles().containsEntry(clientId, role)}.
* <p>This is syntactic sugar for {@code getAllRegistrarIdsWithRoles().containsEntry(registrarId,
* role)}.
*/
public boolean hasRoleOnRegistrar(Role role, String clientId) {
return getAllClientIdWithRoles().containsEntry(clientId, role);
public boolean hasRoleOnRegistrar(Role role, String registrarId) {
return getAllRegistrarIdsWithRoles().containsEntry(registrarId, role);
}
/**
* "Guesses" which client ID the user wants from all those they have access to.
*
* <p>If no such ClientIds exist, throws a RegistrarAccessDeniedException.
* <p>If no such registrar IDs exist, throws a RegistrarAccessDeniedException.
*
* <p>This should be the ClientId "most likely wanted by the user".
* <p>This should be the registrar ID "most likely wanted by the user".
*
* <p>If you want to load the {@link Registrar} object from this (or any other) {@code clientId},
* in order to perform actions on behalf of a user, you must use {@link #getRegistrar} which makes
* sure the user has permissions.
* <p>If you want to load the {@link Registrar} object from this (or any other) {@code
* registrarId}, in order to perform actions on behalf of a user, you must use {@link
* #getRegistrar} which makes sure the user has permissions.
*
* <p>Note that this is an OPTIONAL step in the authentication - only used if we don't have any
* other clue as to the requested {@code clientId}. It is perfectly OK to get a {@code clientId}
* from any other source, as long as the registrar is then loaded using {@link #getRegistrar}.
* other clue as to the requested {@code registrarId}. It is perfectly OK to get a {@code
* registrarId} from any other source, as long as the registrar is then loaded using {@link
* #getRegistrar}.
*/
public String guessClientId() throws RegistrarAccessDeniedException {
return getAllClientIdWithRoles().keySet().stream()
public String guessRegistrarId() throws RegistrarAccessDeniedException {
return getAllRegistrarIdsWithRoles().keySet().stream()
.findFirst()
.orElseThrow(
() ->
@@ -227,7 +230,7 @@ public class AuthenticatedRegistrarAccessor {
if (!registrarId.equals(registrar.getRegistrarId())) {
logger.atSevere().log(
"registrarLoader.apply(clientId) returned a Registrar with a different clientId. "
"registrarLoader.apply(registrarId) returned a Registrar with a different registrarId. "
+ "Requested: %s, returned: %s.",
registrarId, registrar.getRegistrarId());
throw new RegistrarAccessDeniedException("Internal error - please check logs");
@@ -237,7 +240,7 @@ public class AuthenticatedRegistrarAccessor {
}
public void verifyAccess(String registrarId) throws RegistrarAccessDeniedException {
ImmutableSet<Role> roles = getAllClientIdWithRoles().get(registrarId);
ImmutableSet<Role> roles = getAllRegistrarIdsWithRoles().get(registrarId);
if (roles.isEmpty()) {
throw new RegistrarAccessDeniedException(
@@ -279,53 +282,55 @@ public class AuthenticatedRegistrarAccessor {
AuthResult authResult,
Optional<String> gSuiteSupportGroupEmailAddress,
Lazy<GroupsConnection> lazyGroupsConnection) {
if (!authResult.userAuthInfo().isPresent()) {
return false;
}
UserAuthInfo userAuthInfo = authResult.userAuthInfo().get();
User user = userAuthInfo.user();
// both GAE project admin and members of the gSuiteSupportGroupEmailAddress are considered
// admins for the RegistrarConsole.
return !bypassAdminCheck
&& (userAuthInfo.isUserAdmin()
|| checkIsSupport(
lazyGroupsConnection, user.getEmail(), gSuiteSupportGroupEmailAddress));
lazyGroupsConnection,
userAuthInfo.getEmailAddress(),
gSuiteSupportGroupEmailAddress));
}
/**
* Returns a map of registrar client IDs to roles for all registrars that the user has access to.
*/
/** Returns a map of registrar IDs to roles for all registrars that the user has access to. */
private static ImmutableSetMultimap<String, Role> createRoleMap(
AuthResult authResult,
boolean isAdmin,
String registryAdminClientId) {
AuthResult authResult, boolean isAdmin, String registryAdminRegistrarId) {
if (!authResult.userAuthInfo().isPresent()) {
return ImmutableSetMultimap.of();
}
UserAuthInfo userAuthInfo = authResult.userAuthInfo().get();
User user = userAuthInfo.user();
ImmutableSetMultimap.Builder<String, Role> builder = new ImmutableSetMultimap.Builder<>();
logger.atInfo().log("Checking registrar contacts for user ID %s.", user.getUserId());
UserAuthInfo userAuthInfo = authResult.userAuthInfo().get();
if (userAuthInfo.appEngineUser().isPresent()) {
User user = userAuthInfo.appEngineUser().get();
logger.atInfo().log("Checking registrar contacts for user ID %s.", user.getUserId());
// Find all registrars that have a registrar contact with this user's ID.
jpaTm()
.transact(
() ->
jpaTm()
.query(
"SELECT r FROM Registrar r INNER JOIN RegistrarPoc rp ON "
+ "r.clientIdentifier = rp.registrarId WHERE rp.gaeUserId = "
+ ":gaeUserId AND r.state != :state",
Registrar.class)
.setParameter("gaeUserId", user.getUserId())
.setParameter("state", State.DISABLED)
.getResultStream()
.forEach(registrar -> builder.put(registrar.getRegistrarId(), Role.OWNER)));
// Find all registrars that have a registrar contact with this user's ID.
jpaTm()
.transact(
() ->
jpaTm()
.query(
"SELECT r FROM Registrar r INNER JOIN RegistrarPoc rp ON "
+ "r.clientIdentifier = rp.registrarId WHERE rp.gaeUserId = "
+ ":gaeUserId AND r.state != :state",
Registrar.class)
.setParameter("gaeUserId", user.getUserId())
.setParameter("state", State.DISABLED)
.getResultStream()
.forEach(registrar -> builder.put(registrar.getRegistrarId(), Role.OWNER)));
} else {
userAuthInfo
.consoleUser()
.get()
.getUserRoles()
.getRegistrarRoles()
.forEach((k, v) -> builder.put(k, Role.OWNER));
}
// Admins have ADMIN access to all registrars, and also OWNER access to the registry registrar
// and all non-REAL or non-live registrars.
@@ -337,7 +342,7 @@ public class AuthenticatedRegistrarAccessor {
registrar -> {
if (registrar.getType() != Registrar.Type.REAL
|| !registrar.isLive()
|| registrar.getRegistrarId().equals(registryAdminClientId)) {
|| registrar.getRegistrarId().equals(registryAdminRegistrarId)) {
builder.put(registrar.getRegistrarId(), Role.OWNER);
}
builder.put(registrar.getRegistrarId(), Role.ADMIN);
@@ -23,7 +23,7 @@ import java.util.Optional;
public abstract class UserAuthInfo {
/** User object from the AppEngine Users API. */
public abstract User user();
public abstract Optional<User> appEngineUser();
/**
* Whether the user is an admin.
@@ -34,16 +34,37 @@ public abstract class UserAuthInfo {
*/
public abstract boolean isUserAdmin();
public abstract Optional<google.registry.model.console.User> consoleUser();
/** Used by the OAuth authentication mechanism (only) to return information about the session. */
public abstract Optional<OAuthTokenInfo> oauthTokenInfo();
public String getEmailAddress() {
return appEngineUser()
.map(User::getEmail)
.orElseGet(() -> consoleUser().get().getEmailAddress());
}
public String getUsername() {
return appEngineUser()
.map(User::getNickname)
.orElseGet(() -> consoleUser().get().getEmailAddress());
}
public static UserAuthInfo create(
User user, boolean isUserAdmin) {
return new AutoValue_UserAuthInfo(user, isUserAdmin, Optional.empty());
return new AutoValue_UserAuthInfo(
Optional.of(user), isUserAdmin, Optional.empty(), Optional.empty());
}
public static UserAuthInfo create(
User user, boolean isUserAdmin, OAuthTokenInfo oauthTokenInfo) {
return new AutoValue_UserAuthInfo(user, isUserAdmin, Optional.of(oauthTokenInfo));
return new AutoValue_UserAuthInfo(
Optional.of(user), isUserAdmin, Optional.empty(), Optional.of(oauthTokenInfo));
}
public static UserAuthInfo create(google.registry.model.console.User user) {
return new AutoValue_UserAuthInfo(
Optional.empty(), user.getUserRoles().isAdmin(), Optional.of(user), Optional.empty());
}
}
@@ -109,13 +109,13 @@ public final class ConsoleUiAction extends HtmlAction {
.render());
return;
}
ImmutableSetMultimap<String, Role> roleMap = registrarAccessor.getAllClientIdWithRoles();
ImmutableSetMultimap<String, Role> roleMap = registrarAccessor.getAllRegistrarIdsWithRoles();
soyMapData.put("allClientIds", roleMap.keySet());
soyMapData.put("environment", RegistryEnvironment.get().toString());
// We set the initial value to the value that will show if guessClientId throws.
String clientId = "<null>";
try {
clientId = paramClientId.orElse(registrarAccessor.guessClientId());
clientId = paramClientId.orElse(registrarAccessor.guessRegistrarId());
soyMapData.put("clientId", clientId);
soyMapData.put("isOwner", roleMap.containsEntry(clientId, OWNER));
soyMapData.put("isAdmin", roleMap.containsEntry(clientId, ADMIN));
@@ -18,7 +18,6 @@ import static com.google.common.net.HttpHeaders.LOCATION;
import static com.google.common.net.HttpHeaders.X_FRAME_OPTIONS;
import static javax.servlet.http.HttpServletResponse.SC_MOVED_TEMPORARILY;
import com.google.appengine.api.users.User;
import com.google.appengine.api.users.UserService;
import com.google.common.flogger.FluentLogger;
import com.google.common.net.MediaType;
@@ -27,6 +26,7 @@ import google.registry.request.Action;
import google.registry.request.RequestMethod;
import google.registry.request.Response;
import google.registry.request.auth.AuthResult;
import google.registry.request.auth.UserAuthInfo;
import google.registry.security.XsrfTokenManager;
import java.util.HashMap;
import java.util.Map;
@@ -86,16 +86,15 @@ public abstract class HtmlAction implements Runnable {
}
response.setContentType(MediaType.HTML_UTF_8);
User user = authResult.userAuthInfo().get().user();
UserAuthInfo authInfo = authResult.userAuthInfo().get();
// Using HashMap to allow null values
HashMap<String, Object> data = new HashMap<>();
data.put("logoFilename", logoFilename);
data.put("productName", productName);
data.put("username", user.getNickname());
data.put("username", authInfo.getUsername());
data.put("logoutUrl", userService.createLogoutURL(getPath()));
data.put("analyticsConfig", analyticsConfig);
data.put("xsrfToken", xsrfTokenManager.generateToken(user.getEmail()));
data.put("xsrfToken", xsrfTokenManager.generateToken(authInfo.getEmailAddress()));
logger.atInfo().log(
"User %s is accessing %s with method %s.",
@@ -29,6 +29,7 @@ import com.google.common.collect.ImmutableMap;
import com.google.common.flogger.FluentLogger;
import com.google.common.net.MediaType;
import com.google.gson.Gson;
import google.registry.model.console.ConsolePermission;
import google.registry.model.domain.RegistryLock;
import google.registry.model.registrar.Registrar;
import google.registry.model.registrar.RegistrarPoc;
@@ -42,6 +43,7 @@ import google.registry.request.auth.Auth;
import google.registry.request.auth.AuthResult;
import google.registry.request.auth.AuthenticatedRegistrarAccessor;
import google.registry.request.auth.AuthenticatedRegistrarAccessor.RegistrarAccessDeniedException;
import google.registry.request.auth.UserAuthInfo;
import google.registry.security.JsonResponseHelper;
import java.util.Objects;
import java.util.Optional;
@@ -153,24 +155,35 @@ public final class RegistryLockGetAction implements JsonGetAction {
boolean isAdmin = registrarAccessor.isAdmin();
Registrar registrar = getRegistrarAndVerifyLockAccess(registrarAccessor, registrarId, isAdmin);
User user = authResult.userAuthInfo().get().user();
Optional<RegistrarPoc> contactOptional = getContactMatchingLogin(user, registrar);
boolean isRegistryLockAllowed =
isAdmin || contactOptional.map(RegistrarPoc::isRegistryLockAllowed).orElse(false);
UserAuthInfo userAuthInfo = authResult.userAuthInfo().get();
// Split logic depending on whether we are using the old auth system or the new one
boolean isRegistryLockAllowed;
String relevantEmail;
if (userAuthInfo.appEngineUser().isPresent()) {
User user = userAuthInfo.appEngineUser().get();
Optional<RegistrarPoc> contactOptional = getContactMatchingLogin(user, registrar);
isRegistryLockAllowed =
isAdmin || contactOptional.map(RegistrarPoc::isRegistryLockAllowed).orElse(false);
relevantEmail =
isAdmin
? user.getEmail()
// if the contact isn't present, we shouldn't display the email anyway
: contactOptional.flatMap(RegistrarPoc::getRegistryLockEmailAddress).orElse("");
} else {
google.registry.model.console.User user = userAuthInfo.consoleUser().get();
isRegistryLockAllowed =
user.getUserRoles().hasPermission(registrarId, ConsolePermission.REGISTRY_LOCK);
relevantEmail = user.getEmailAddress();
}
// Use the contact's registry lock email if it's present, else use the login email (for admins)
String relevantEmail =
isAdmin
? user.getEmail()
// if the contact isn't present, we shouldn't display the email anyway so empty is fine
: contactOptional.flatMap(RegistrarPoc::getRegistryLockEmailAddress).orElse("");
return ImmutableMap.of(
LOCK_ENABLED_FOR_CONTACT_PARAM,
isRegistryLockAllowed,
EMAIL_PARAM,
relevantEmail,
PARAM_CLIENT_ID,
registrar.getRegistrarId(),
registrarId,
LOCKS_PARAM,
getLockedDomains(registrarId, isAdmin));
}
@@ -184,10 +184,29 @@ public class RegistryLockPostAction implements Runnable, JsonActionRunner.JsonAc
private String verifyPasswordAndGetEmail(
UserAuthInfo userAuthInfo, RegistryLockPostInput postInput)
throws RegistrarAccessDeniedException {
User user = userAuthInfo.user();
if (registrarAccessor.isAdmin()) {
return user.getEmail();
return userAuthInfo.getEmailAddress();
}
if (userAuthInfo.appEngineUser().isPresent()) {
return verifyPasswordAndGetEmailLegacyUser(userAuthInfo.appEngineUser().get(), postInput);
} else {
return verifyPasswordAndGetEmailConsoleUser(userAuthInfo.consoleUser().get(), postInput);
}
}
private String verifyPasswordAndGetEmailConsoleUser(
google.registry.model.console.User user, RegistryLockPostInput postInput)
throws RegistrarAccessDeniedException {
// Verify that the registrar has locking enabled
getRegistrarAndVerifyLockAccess(registrarAccessor, postInput.registrarId, false);
checkArgument(
user.verifyRegistryLockPassword(postInput.password),
"Incorrect registry lock password for user");
return user.getEmailAddress();
}
private String verifyPasswordAndGetEmailLegacyUser(User user, RegistryLockPostInput postInput)
throws RegistrarAccessDeniedException {
// Verify that the user can access the registrar, that the user has
// registry lock enabled, and that the user provided a correct password
Registrar registrar =