Decouple superuser from SessionMetadata

Superuser should only be settable via the tool (see []
which is merged in here but not diffbased, and which removes
the implicit superuser for CharlestonRoad). It is a property
of the request, not of the session (there are no sessions in the tool).
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=125204707
This commit is contained in:
cgoldfeder 2016-06-17 14:48:46 -07:00 committed by Ben McIlwain
parent e359ab5f52
commit fd6c4888db
44 changed files with 80 additions and 136 deletions

View file

@ -17,7 +17,7 @@ package google.registry.flows;
import static com.google.common.truth.Truth.assertThat;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.mockito.Mockito.eq;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
@ -43,33 +43,25 @@ public class EppConsoleActionTest extends ShardableTestCase {
.withUserService(UserInfo.create("person@example.com", "12345"))
.build();
private void doTest(boolean superuser) {
@Test
public void testPassesArgumentsThrough() {
EppConsoleAction action = new EppConsoleAction();
action.inputXmlBytes = INPUT_XML_BYTES;
action.session = new BasicHttpSession();
action.session.setAttribute("CLIENT_ID", "ClientIdentifier");
action.session.setAttribute("SUPERUSER", superuser);
action.eppRequestHandler = mock(EppRequestHandler.class);
action.run();
ArgumentCaptor<TransportCredentials> credentialsCaptor =
ArgumentCaptor.forClass(TransportCredentials.class);
ArgumentCaptor<SessionMetadata> metadataCaptor = ArgumentCaptor.forClass(SessionMetadata.class);
verify(action.eppRequestHandler).executeEpp(
metadataCaptor.capture(), credentialsCaptor.capture(), eq(false), eq(INPUT_XML_BYTES));
metadataCaptor.capture(),
credentialsCaptor.capture(),
eq(false),
eq(false),
eq(INPUT_XML_BYTES));
assertThat(((GaeUserCredentials) credentialsCaptor.getValue()).gaeUser.getEmail())
.isEqualTo("person@example.com");
SessionMetadata sessionMetadata = metadataCaptor.getValue();
assertThat(sessionMetadata.getClientId()).isEqualTo("ClientIdentifier");
assertThat(sessionMetadata.isSuperuser()).isEqualTo(superuser);
}
@Test
public void testSuperuser() throws Exception {
doTest(true);
}
@Test
public void testNotSuperuser() throws Exception {
doTest(false);
assertThat(metadataCaptor.getValue().getClientId()).isEqualTo("ClientIdentifier");
}
}

View file

@ -104,12 +104,12 @@ public class EppLifecycleDomainApplicationTest extends EppTestCase {
"domain_allocate_testvalidate.xml",
"domain_allocate_response_testvalidate_only_superuser.xml",
START_OF_GA.plusDays(1));
setSuperuser(true);
setIsSuperuser(true);
assertCommandAndResponse(
"domain_allocate_testvalidate.xml",
"domain_allocate_response_testvalidate.xml",
START_OF_GA.plusDays(1).plusMinutes(1));
setSuperuser(false);
setIsSuperuser(false);
assertCommandAndResponse(
"domain_info_testvalidate.xml",
"domain_info_response_testvalidate_ok.xml",

View file

@ -70,7 +70,7 @@ public class EppTestCase extends ShardableTestCase {
this.credentials = credentials;
}
protected void setSuperuser(boolean isSuperuser) {
protected void setIsSuperuser(boolean isSuperuser) {
this.isSuperuser = isSuperuser;
}
@ -96,7 +96,6 @@ public class EppTestCase extends ShardableTestCase {
if (sessionMetadata == null) {
sessionMetadata = new TestSessionMetadata();
}
sessionMetadata.setSuperuser(isSuperuser);
String actualOutput = executeXmlCommand(input);
if (!sessionMetadata.isValid()) {
sessionMetadata = null;
@ -118,7 +117,7 @@ public class EppTestCase extends ShardableTestCase {
handler.eppController = new EppController();
handler.eppController.clock = clock;
handler.eppController.metrics = mock(EppMetrics.class);
handler.executeEpp(sessionMetadata, credentials, false, inputXml.getBytes(UTF_8));
handler.executeEpp(sessionMetadata, credentials, false, isSuperuser, inputXml.getBytes(UTF_8));
assertThat(response.getStatus()).isEqualTo(SC_OK);
assertThat(response.getContentType()).isEqualTo(APPLICATION_EPP_XML_UTF8);
String result = response.getPayload();

View file

@ -37,31 +37,23 @@ public class EppTlsActionTest extends ShardableTestCase {
private static final byte[] INPUT_XML_BYTES = "<xml>".getBytes(UTF_8);
private void doTest(boolean superuser) {
@Test
public void testPassesArgumentsThrough() {
EppTlsAction action = new EppTlsAction();
action.inputXmlBytes = INPUT_XML_BYTES;
action.tlsCredentials = mock(TlsCredentials.class);
when(action.tlsCredentials.hasSni()).thenReturn(true);
action.session = new BasicHttpSession();
action.session.setAttribute("CLIENT_ID", "ClientIdentifier");
action.session.setAttribute("SUPERUSER", superuser);
action.eppRequestHandler = mock(EppRequestHandler.class);
action.run();
ArgumentCaptor<SessionMetadata> captor = ArgumentCaptor.forClass(SessionMetadata.class);
verify(action.eppRequestHandler)
.executeEpp(captor.capture(), same(action.tlsCredentials), eq(false), eq(INPUT_XML_BYTES));
SessionMetadata sessionMetadata = captor.getValue();
assertThat(sessionMetadata.getClientId()).isEqualTo("ClientIdentifier");
assertThat(sessionMetadata.isSuperuser()).isEqualTo(superuser);
}
@Test
public void testSuperuser() throws Exception {
doTest(true);
}
@Test
public void testNotSuperuser() throws Exception {
doTest(false);
verify(action.eppRequestHandler).executeEpp(
captor.capture(),
same(action.tlsCredentials),
eq(false),
eq(false),
eq(INPUT_XML_BYTES));
assertThat(captor.getValue().getClientId()).isEqualTo("ClientIdentifier");
}
}

View file

@ -16,8 +16,8 @@ package google.registry.flows;
import static com.google.common.truth.Truth.assertThat;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.mockito.Matchers.eq;
import static org.mockito.Matchers.isA;
import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.isA;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
@ -30,11 +30,11 @@ import org.mockito.ArgumentCaptor;
@RunWith(JUnit4.class)
public class EppToolActionTest {
private void doTest(boolean dryRun, boolean superuser) {
private void doTest(boolean isDryRun, boolean isSuperuser) {
EppToolAction action = new EppToolAction();
action.clientIdentifier = "ClientIdentifier";
action.dryRun = dryRun;
action.superuser = superuser;
action.isDryRun = isDryRun;
action.isSuperuser = isSuperuser;
action.eppRequestHandler = mock(EppRequestHandler.class);
action.xml = "<xml>";
action.run();
@ -42,11 +42,10 @@ public class EppToolActionTest {
verify(action.eppRequestHandler).executeEpp(
captor.capture(),
isA(PasswordOnlyTransportCredentials.class),
eq(dryRun),
eq(isDryRun),
eq(isSuperuser),
eq(action.xml.getBytes(UTF_8)));
SessionMetadata sessionMetadata = captor.getValue();
assertThat(sessionMetadata.getClientId()).isEqualTo("ClientIdentifier");
assertThat(sessionMetadata.isSuperuser()).isEqualTo(superuser);
assertThat(captor.getValue().getClientId()).isEqualTo("ClientIdentifier");
}
@Test

View file

@ -89,7 +89,7 @@ public abstract class FlowTestCase<F extends Flow> {
protected EppLoader eppLoader;
protected Class<? extends Flow> flowClass;
protected TestSessionMetadata sessionMetadata;
protected SessionMetadata sessionMetadata;
protected FakeClock clock = new FakeClock(DateTime.now(UTC));
protected TransportCredentials credentials = new PasswordOnlyTransportCredentials();
@ -121,7 +121,8 @@ public abstract class FlowTestCase<F extends Flow> {
}
/** Load a flow from an epp object. */
private FlowRunner getFlowRunner(CommitMode commitMode) throws Exception {
private FlowRunner getFlowRunner(CommitMode commitMode, UserPrivileges userPrivileges)
throws Exception {
EppInput eppInput = eppLoader.getEpp();
flowClass = firstNonNull(flowClass, FlowPicker.getFlowClass(eppInput));
Class<?> expectedFlowClass = new TypeInstantiator<F>(getClass()){}.getExactType();
@ -133,6 +134,7 @@ public abstract class FlowTestCase<F extends Flow> {
sessionMetadata,
credentials,
commitMode.equals(CommitMode.DRY_RUN),
userPrivileges.equals(UserPrivileges.SUPERUSER),
"<xml></xml>".getBytes(),
null,
clock);
@ -153,7 +155,8 @@ public abstract class FlowTestCase<F extends Flow> {
}
public void assertTransactionalFlow(boolean isTransactional) throws Exception {
assertThat(getFlowRunner(CommitMode.LIVE).isTransactional()).isEqualTo(isTransactional);
assertThat(getFlowRunner(CommitMode.LIVE, UserPrivileges.NORMAL).isTransactional())
.isEqualTo(isTransactional);
}
public void assertNoHistory() throws Exception {
@ -271,8 +274,7 @@ public abstract class FlowTestCase<F extends Flow> {
/** Run a flow, and attempt to marshal the result to EPP or throw if it doesn't validate. */
public EppOutput runFlow(CommitMode commitMode, UserPrivileges userPrivileges) throws Exception {
sessionMetadata.setSuperuser(userPrivileges.equals(UserPrivileges.SUPERUSER));
EppOutput output = getFlowRunner(commitMode).run();
EppOutput output = getFlowRunner(commitMode, userPrivileges).run();
marshal(output, ValidationMode.STRICT);
return output;
}
@ -284,8 +286,7 @@ public abstract class FlowTestCase<F extends Flow> {
public void runFlowAssertResponse(
CommitMode commitMode, UserPrivileges userPrivileges, String xml, String... ignoredPaths)
throws Exception {
sessionMetadata.setSuperuser(userPrivileges.equals(UserPrivileges.SUPERUSER));
EppOutput eppOutput = getFlowRunner(commitMode).run();
EppOutput eppOutput = getFlowRunner(commitMode, userPrivileges).run();
if (eppOutput.isResponse()) {
assertThat(eppOutput.isSuccess()).isTrue();
}

View file

@ -142,7 +142,6 @@ public class ContactDeleteFlowTest
@Test
public void testSuccess_superuserUnauthorizedClient() throws Exception {
sessionMetadata.setSuperuser(true);
sessionMetadata.setClientId("NewRegistrar");
persistActiveContact(getUniqueIdFromCommand());
clock.advanceOneMilli();

View file

@ -195,7 +195,6 @@ public class ContactUpdateFlowTest
@Test
public void testSuccess_superuserUnauthorizedClient() throws Exception {
sessionMetadata.setSuperuser(true);
sessionMetadata.setClientId("NewRegistrar");
persistActiveContact(getUniqueIdFromCommand());
clock.advanceOneMilli();

View file

@ -164,7 +164,6 @@ public class DomainApplicationDeleteFlowTest
@Test
public void testSuccess_superuserUnauthorizedClient() throws Exception {
sessionMetadata.setSuperuser(true);
sessionMetadata.setClientId("NewRegistrar");
persistResource(
newDomainApplication("example.tld").asBuilder().setRepoId("1-TLD").build());

View file

@ -565,7 +565,6 @@ public class DomainApplicationUpdateFlowTest
@Test
public void testSuccess_superuserUnauthorizedClient() throws Exception {
sessionMetadata.setSuperuser(true);
sessionMetadata.setClientId("NewRegistrar");
persistReferencedEntities();
persistApplication();

View file

@ -546,7 +546,6 @@ public class DomainDeleteFlowTest extends ResourceFlowTestCase<DomainDeleteFlow,
@Test
public void testSuccess_superuserUnauthorizedClient() throws Exception {
sessionMetadata.setSuperuser(true);
sessionMetadata.setClientId("NewRegistrar");
setupSuccessfulTest();
clock.advanceOneMilli();

View file

@ -425,7 +425,6 @@ public class DomainRenewFlowTest extends ResourceFlowTestCase<DomainRenewFlow, D
@Test
public void testSuccess_superuserUnauthorizedClient() throws Exception {
sessionMetadata.setSuperuser(true);
sessionMetadata.setClientId("NewRegistrar");
persistDomain();
runFlowAssertResponse(

View file

@ -369,7 +369,6 @@ public class DomainRestoreRequestFlowTest extends
@Test
public void testSuccess_superuserUnauthorizedClient() throws Exception {
thrown.expect(ResourceNotOwnedException.class);
sessionMetadata.setSuperuser(true);
sessionMetadata.setClientId("NewRegistrar");
persistPendingDeleteDomain();
runFlowAssertResponse(readFile("domain_update_response.xml"));

View file

@ -969,7 +969,6 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
@Test
public void testSuccess_superuserUnauthorizedClient() throws Exception {
sessionMetadata.setSuperuser(true);
sessionMetadata.setClientId("NewRegistrar");
persistReferencedEntities();
persistDomain();

View file

@ -145,7 +145,6 @@ public class HostDeleteFlowTest extends ResourceFlowTestCase<HostDeleteFlow, Hos
@Test
public void testSuccess_superuserUnauthorizedClient() throws Exception {
sessionMetadata.setSuperuser(true);
sessionMetadata.setClientId("NewRegistrar");
persistActiveHost(getUniqueIdFromCommand());
clock.advanceOneMilli();

View file

@ -833,7 +833,6 @@ public class HostUpdateFlowTest extends ResourceFlowTestCase<HostUpdateFlow, Hos
@Test
public void testSuccess_superuserUnauthorizedClient() throws Exception {
sessionMetadata.setSuperuser(true);
sessionMetadata.setClientId("NewRegistrar");
persistActiveHost(oldHostName());

View file

@ -14,7 +14,6 @@
package google.registry.flows.session;
import static com.google.common.truth.Truth.assertThat;
import static google.registry.testing.DatastoreHelper.deleteResource;
import static google.registry.testing.DatastoreHelper.persistResource;
@ -76,21 +75,6 @@ public abstract class LoginFlowTestCase extends FlowTestCase<LoginFlow> {
@Test
public void testSuccess() throws Exception {
doSuccessfulTest("login_valid.xml");
assertThat(sessionMetadata.isSuperuser()).isFalse();
}
@Test
public void testSuccess_superuser() throws Exception {
persistResource(getRegistrarBuilder().setIanaIdentifier(9999L).build());
doSuccessfulTest("login_valid.xml");
assertThat(sessionMetadata.isSuperuser()).isTrue();
}
@Test
public void testSuccess_notSuperuser() throws Exception {
persistResource(getRegistrarBuilder().setIanaIdentifier(15L).build());
doSuccessfulTest("login_valid.xml");
assertThat(sessionMetadata.isSuperuser()).isFalse();
}
@Test

View file

@ -87,6 +87,7 @@ public class EppResourceUtilsTest {
sessionMetadata,
new PasswordOnlyTransportCredentials(),
false,
false,
"<xml></xml>".getBytes(),
null,
clock)