mirror of
https://github.com/google/nomulus.git
synced 2025-07-09 12:43:24 +02:00
Handle null GAE user IDs gracefully (for non-admins) (#531)
Unfortunately in our testing environments, we're all admins so it's easy to miss things like this.
This commit is contained in:
parent
f1c46b8030
commit
2ff1026cfd
4 changed files with 11 additions and 3 deletions
|
@ -43,6 +43,7 @@ import google.registry.request.auth.AuthenticatedRegistrarAccessor;
|
||||||
import google.registry.request.auth.AuthenticatedRegistrarAccessor.RegistrarAccessDeniedException;
|
import google.registry.request.auth.AuthenticatedRegistrarAccessor.RegistrarAccessDeniedException;
|
||||||
import google.registry.schema.domain.RegistryLock;
|
import google.registry.schema.domain.RegistryLock;
|
||||||
import google.registry.security.JsonResponseHelper;
|
import google.registry.security.JsonResponseHelper;
|
||||||
|
import java.util.Objects;
|
||||||
import java.util.Optional;
|
import java.util.Optional;
|
||||||
import javax.inject.Inject;
|
import javax.inject.Inject;
|
||||||
import org.joda.time.DateTime;
|
import org.joda.time.DateTime;
|
||||||
|
@ -119,7 +120,8 @@ public final class RegistryLockGetAction implements JsonGetAction {
|
||||||
static Optional<RegistrarContact> getContactMatchingLogin(User user, Registrar registrar) {
|
static Optional<RegistrarContact> getContactMatchingLogin(User user, Registrar registrar) {
|
||||||
ImmutableList<RegistrarContact> matchingContacts =
|
ImmutableList<RegistrarContact> matchingContacts =
|
||||||
registrar.getContacts().stream()
|
registrar.getContacts().stream()
|
||||||
.filter(contact -> contact.getGaeUserId().equals(user.getUserId()))
|
.filter(contact -> contact.getGaeUserId() != null)
|
||||||
|
.filter(contact -> Objects.equals(contact.getGaeUserId(), user.getUserId()))
|
||||||
.collect(toImmutableList());
|
.collect(toImmutableList());
|
||||||
if (matchingContacts.size() > 1) {
|
if (matchingContacts.size() > 1) {
|
||||||
ImmutableList<String> matchingEmails =
|
ImmutableList<String> matchingEmails =
|
||||||
|
|
|
@ -224,7 +224,6 @@ public final class AppEngineRule extends ExternalResource {
|
||||||
.setEmailAddress("janedoe@theregistrar.com")
|
.setEmailAddress("janedoe@theregistrar.com")
|
||||||
.setPhoneNumber("+1.1234567890")
|
.setPhoneNumber("+1.1234567890")
|
||||||
.setTypes(ImmutableSet.of(RegistrarContact.Type.ADMIN))
|
.setTypes(ImmutableSet.of(RegistrarContact.Type.ADMIN))
|
||||||
.setGaeUserId(NEW_REGISTRAR_GAE_USER_ID)
|
|
||||||
.build();
|
.build();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -127,7 +127,11 @@ public class RegistrarContactCommandTest extends CommandTestCase<RegistrarContac
|
||||||
"--email=jane.doe@example.com",
|
"--email=jane.doe@example.com",
|
||||||
"--allow_console_access=true",
|
"--allow_console_access=true",
|
||||||
"NewRegistrar");
|
"NewRegistrar");
|
||||||
RegistrarContact registrarContact = loadRegistrar("NewRegistrar").getContacts().asList().get(1);
|
RegistrarContact registrarContact =
|
||||||
|
loadRegistrar("NewRegistrar").getContacts().stream()
|
||||||
|
.filter(rc -> rc.getEmailAddress().equals("jane.doe@example.com"))
|
||||||
|
.findFirst()
|
||||||
|
.get();
|
||||||
assertThat(registrarContact.getGaeUserId()).matches("-?[0-9]+");
|
assertThat(registrarContact.getGaeUserId()).matches("-?[0-9]+");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat;
|
||||||
import static google.registry.request.auth.AuthenticatedRegistrarAccessor.Role.ADMIN;
|
import static google.registry.request.auth.AuthenticatedRegistrarAccessor.Role.ADMIN;
|
||||||
import static google.registry.request.auth.AuthenticatedRegistrarAccessor.Role.OWNER;
|
import static google.registry.request.auth.AuthenticatedRegistrarAccessor.Role.OWNER;
|
||||||
import static google.registry.testing.AppEngineRule.makeRegistrar2;
|
import static google.registry.testing.AppEngineRule.makeRegistrar2;
|
||||||
|
import static google.registry.testing.AppEngineRule.makeRegistrarContact2;
|
||||||
import static google.registry.testing.AppEngineRule.makeRegistrarContact3;
|
import static google.registry.testing.AppEngineRule.makeRegistrarContact3;
|
||||||
import static google.registry.testing.DatastoreHelper.persistResource;
|
import static google.registry.testing.DatastoreHelper.persistResource;
|
||||||
import static google.registry.testing.SqlHelper.saveRegistryLock;
|
import static google.registry.testing.SqlHelper.saveRegistryLock;
|
||||||
|
@ -287,6 +288,8 @@ public final class RegistryLockGetActionTest {
|
||||||
public void testSuccess_lockAllowedForAdmin() {
|
public void testSuccess_lockAllowedForAdmin() {
|
||||||
// Locks are allowed for admins even when they're not enabled for the registrar
|
// Locks are allowed for admins even when they're not enabled for the registrar
|
||||||
persistResource(makeRegistrar2().asBuilder().setRegistryLockAllowed(false).build());
|
persistResource(makeRegistrar2().asBuilder().setRegistryLockAllowed(false).build());
|
||||||
|
// disallow the other user
|
||||||
|
persistResource(makeRegistrarContact2().asBuilder().setGaeUserId(null).build());
|
||||||
authResult = AuthResult.create(AuthLevel.USER, UserAuthInfo.create(user, true));
|
authResult = AuthResult.create(AuthLevel.USER, UserAuthInfo.create(user, true));
|
||||||
accessor =
|
accessor =
|
||||||
AuthenticatedRegistrarAccessor.createForTesting(
|
AuthenticatedRegistrarAccessor.createForTesting(
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue