Restrict contact info commands to owning registrars

Superuser can also execute contact info commands. AuthInfo is no longer checked in the input and always displayed in the output as the only ones who can get a response are the sponsoring registrar and super user.

Also corrected a Javadoc in which '@' should have been escaped (see https://reflectoring.io/howto-format-code-snippets-in-javadoc/)

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=199521153
This commit is contained in:
jianglai 2018-06-06 13:40:24 -07:00 committed by Ben McIlwain
parent 19f58f5557
commit 27fce55654
3 changed files with 49 additions and 23 deletions

View file

@ -70,6 +70,8 @@ registrar that owns the contact or to a registrar that already supplied it.
### Errors
* 2201
* The specified resource belongs to another client.
* 2303
* Resource with this id does not exist.

View file

@ -16,7 +16,7 @@ package google.registry.flows.contact;
import static google.registry.flows.FlowUtils.validateClientIsLoggedIn;
import static google.registry.flows.ResourceFlowUtils.loadAndVerifyExistence;
import static google.registry.flows.ResourceFlowUtils.verifyOptionalAuthInfo;
import static google.registry.flows.ResourceFlowUtils.verifyResourceOwnership;
import static google.registry.model.EppResourceUtils.isLinked;
import com.google.common.collect.ImmutableSet;
@ -25,6 +25,7 @@ import google.registry.flows.EppException;
import google.registry.flows.ExtensionManager;
import google.registry.flows.Flow;
import google.registry.flows.FlowModule.ClientId;
import google.registry.flows.FlowModule.Superuser;
import google.registry.flows.FlowModule.TargetId;
import google.registry.flows.annotations.ReportingSpec;
import google.registry.model.contact.ContactInfoData;
@ -47,6 +48,7 @@ import org.joda.time.DateTime;
* visible to the registrar that owns the contact or to a registrar that already supplied it.
*
* @error {@link google.registry.flows.ResourceFlowUtils.ResourceDoesNotExistException}
* @error {@link google.registry.flows.ResourceFlowUtils.ResourceNotOwnedException}
*/
@ReportingSpec(ActivityReportField.CONTACT_INFO)
public final class ContactInfoFlow implements Flow {
@ -56,8 +58,11 @@ public final class ContactInfoFlow implements Flow {
@Inject @ClientId String clientId;
@Inject @TargetId String targetId;
@Inject Optional<AuthInfo> authInfo;
@Inject @Superuser boolean isSuperuser;
@Inject EppResponse.Builder responseBuilder;
@Inject ContactInfoFlow() {}
@Inject
ContactInfoFlow() {}
@Override
public final EppResponse run() throws EppException {
@ -65,7 +70,9 @@ public final class ContactInfoFlow implements Flow {
extensionManager.validate(); // There are no legal extensions for this flow.
validateClientIsLoggedIn(clientId);
ContactResource contact = loadAndVerifyExistence(ContactResource.class, targetId, now);
verifyOptionalAuthInfo(authInfo, contact);
if (!isSuperuser) {
verifyResourceOwnership(clientId, contact);
}
boolean includeAuthInfo =
clientId.equals(contact.getCurrentSponsorClientId()) || authInfo.isPresent();
ImmutableSet.Builder<StatusValue> statusValues = new ImmutableSet.Builder<>();
@ -74,7 +81,8 @@ public final class ContactInfoFlow implements Flow {
statusValues.add(StatusValue.LINKED);
}
return responseBuilder
.setResData(ContactInfoData.newBuilder()
.setResData(
ContactInfoData.newBuilder()
.setContactId(contact.getContactId())
.setRepoId(contact.getRepoId())
.setStatusValues(statusValues.build())

View file

@ -27,6 +27,7 @@ import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import google.registry.flows.ResourceFlowTestCase;
import google.registry.flows.ResourceFlowUtils.ResourceDoesNotExistException;
import google.registry.flows.ResourceFlowUtils.ResourceNotOwnedException;
import google.registry.model.contact.ContactAddress;
import google.registry.model.contact.ContactAuthInfo;
import google.registry.model.contact.ContactPhoneNumber;
@ -135,13 +136,26 @@ public class ContactInfoFlowTest extends ResourceFlowTestCase<ContactInfoFlow, C
}
@Test
public void testSuccess_otherRegistrarWithoutAuthInfo_doesNotSeeAuthInfo() throws Exception {
public void testFailure_otherRegistrar_notAuthorized() throws Exception {
setClientIdForFlow("NewRegistrar");
persistContactResource(true);
// Check that the persisted contact info was returned.
assertTransactionalFlow(false);
ResourceNotOwnedException thrown = assertThrows(ResourceNotOwnedException.class, this::runFlow);
assertAboutEppExceptions().that(thrown).marshalsToXml();
}
@Test
public void testSuccess_otherRegistrarWithoutAuthInfoAsSuperuser_doesNotSeeAuthInfo()
throws Exception {
setClientIdForFlow("NewRegistrar");
setEppInput("contact_info_no_authinfo.xml");
persistContactResource(true);
// Check that the persisted contact info was returned.
assertTransactionalFlow(false);
runFlowAssertResponse(
CommitMode.LIVE,
UserPrivileges.SUPERUSER,
loadFile("contact_info_response_no_authinfo.xml"),
// We use a different roid scheme than the samples so ignore it.
"epp.response.resData.infData.roid");
@ -150,12 +164,14 @@ public class ContactInfoFlowTest extends ResourceFlowTestCase<ContactInfoFlow, C
}
@Test
public void testSuccess_otherRegistrarWithAuthInfo_seesAuthInfo() throws Exception {
public void testSuccess_otherRegistrarWithAuthInfoAsSuperuser_seesAuthInfo() throws Exception {
setClientIdForFlow("NewRegistrar");
persistContactResource(true);
// Check that the persisted contact info was returned.
assertTransactionalFlow(false);
runFlowAssertResponse(
CommitMode.LIVE,
UserPrivileges.SUPERUSER,
loadFile("contact_info_response.xml"),
// We use a different roid scheme than the samples so ignore it.
"epp.response.resData.infData.roid");