Introduce separate UpdateRegistrationContact & UpdateRegistrationKey methods in RA & SA

ra/proto/ra.proto

@@ -10,6 +10,8 @@ import "google/protobuf/empty.proto";
 service RegistrationAuthority {
   rpc NewRegistration(core.Registration) returns (core.Registration) {}
   rpc UpdateRegistration(UpdateRegistrationRequest) returns (core.Registration) {}
+  rpc UpdateRegistrationContact(UpdateRegistrationContactRequest) returns (core.Registration) {}
+  rpc UpdateRegistrationKey(UpdateRegistrationKeyRequest) returns (core.Registration) {}
   rpc PerformValidation(PerformValidationRequest) returns (core.Authorization) {}
   rpc DeactivateRegistration(core.Registration) returns (google.protobuf.Empty) {}
   rpc DeactivateAuthorization(core.Authorization) returns (google.protobuf.Empty) {}
@@ -33,6 +35,16 @@ message UpdateRegistrationRequest {
   core.Registration update = 2;
 }
 
+message UpdateRegistrationContactRequest {
+  int64 registrationID = 1;
+  repeated string contacts = 2;
+}
+
+message UpdateRegistrationKeyRequest {
+  int64 registrationID = 1;
+  bytes jwk = 2;
+}
+
 message UpdateAuthorizationRequest {
   core.Authorization authz = 1;
   int64 challengeIndex = 2;

ra/ra.go

@@ -1620,7 +1620,8 @@ func (ra *RegistrationAuthorityImpl) checkNewOrderLimits(ctx context.Context, na
 // UpdateRegistration updates an existing Registration with new values. Caller
 // is responsible for making sure that update.Key is only different from base.Key
 // if it is being called from the WFE key change endpoint.
-// TODO(#5554): Split this into separate methods for updating Contacts vs Key.
+//
+// Deprecated: Use UpdateRegistrationContact or UpdateRegistrationKey instead.
 func (ra *RegistrationAuthorityImpl) UpdateRegistration(ctx context.Context, req *rapb.UpdateRegistrationRequest) (*corepb.Registration, error) {
 	// Error if the request is nil, there is no account key or IP address
 	if req.Base == nil || len(req.Base.Key) == 0 || len(req.Base.InitialIP) == 0 || req.Base.Id == 0 {
@@ -1659,6 +1660,48 @@ func (ra *RegistrationAuthorityImpl) UpdateRegistration(ctx context.Context, req
 	return update, nil
 }
 
+// UpdateRegistrationContact updates an existing Registration's contact.
+// The updated contacts field may be empty.
+func (ra *RegistrationAuthorityImpl) UpdateRegistrationContact(ctx context.Context, req *rapb.UpdateRegistrationContactRequest) (*corepb.Registration, error) {
+	if core.IsAnyNilOrZero(req.RegistrationID) {
+		return nil, errIncompleteGRPCRequest
+	}
+
+	err := ra.validateContacts(req.Contacts)
+	if err != nil {
+		return nil, err
+	}
+
+	update, err := ra.SA.UpdateRegistrationContact(ctx, &sapb.UpdateRegistrationContactRequest{
+		RegistrationID: req.RegistrationID,
+		Contacts:       req.Contacts,
+	})
+	if err != nil {
+		err = fmt.Errorf("failed to update registration contact: %w", err)
+		return nil, err
+	}
+
+	return update, nil
+}
+
+// UpdateRegistrationKey updates an existing Registration's key.
+func (ra *RegistrationAuthorityImpl) UpdateRegistrationKey(ctx context.Context, req *rapb.UpdateRegistrationKeyRequest) (*corepb.Registration, error) {
+	if core.IsAnyNilOrZero(req.RegistrationID, req.Jwk) {
+		return nil, errIncompleteGRPCRequest
+	}
+
+	update, err := ra.SA.UpdateRegistrationKey(ctx, &sapb.UpdateRegistrationKeyRequest{
+		RegistrationID: req.RegistrationID,
+		Jwk:            req.Jwk,
+	})
+	if err != nil {
+		err = fmt.Errorf("failed to update registration key: %w", err)
+		return nil, err
+	}
+
+	return update, nil
+}
+
 func contactsEqual(a []string, b []string) bool {
 	if len(a) != len(b) {
 		return false

ra/ra_test.go

@@ -18,6 +18,7 @@ import (
 	mrand "math/rand/v2"
 	"net"
 	"os"
+	"reflect"
 	"regexp"
 	"strconv"
 	"strings"
@@ -4483,3 +4484,120 @@ func TestGetAuthorization(t *testing.T) {
 	test.AssertNotError(t, err, "should not fail")
 	test.AssertEquals(t, len(authz.Challenges), 0)
 }
+
+// An authority for testing UpdateRegistrationContact and UpdateRegistrationKey.
+type mockSAWithRegistration struct {
+	sapb.StorageAuthorityClient
+	expectRegistrationID int64
+	expectContacts       []string
+	expectJwk            []byte
+}
+
+// Mocked UpdateRegistrationContact returns an error if the "RegistrationID"
+// field of the request is not as expected, and returns updated contacts if they
+// are provided.
+func (sa *mockSAWithRegistration) UpdateRegistrationContact(ctx context.Context, req *sapb.UpdateRegistrationContactRequest, _ ...grpc.CallOption) (*corepb.Registration, error) {
+	if req.RegistrationID != sa.expectRegistrationID {
+		return nil, status.Error(codes.InvalidArgument, "RegistrationID didn't match expected value")
+	}
+	if !reflect.DeepEqual(req.Contacts, sa.expectContacts) {
+		return nil, status.Error(codes.InvalidArgument, "Contacts didn't match expected value")
+	}
+	if len(req.Contacts) != 0 {
+		return &corepb.Registration{
+			Id:      req.RegistrationID,
+			Contact: req.Contacts,
+		}, nil
+	} else {
+		return &corepb.Registration{
+			Id: req.RegistrationID,
+		}, nil
+	}
+}
+
+// Mocked UpdateRegistrationKey returns an error if the "RegistrationID" and/or
+// "Jwk" field of the request is not as expected, and returns the updated key.
+func (sa *mockSAWithRegistration) UpdateRegistrationKey(ctx context.Context, req *sapb.UpdateRegistrationKeyRequest, _ ...grpc.CallOption) (*corepb.Registration, error) {
+	if req.RegistrationID != sa.expectRegistrationID {
+		return nil, status.Error(codes.InvalidArgument, "RegistrationID didn't match expected value")
+	}
+	if !reflect.DeepEqual(req.Jwk, sa.expectJwk) {
+		return nil, status.Error(codes.InvalidArgument, "JWK didn't match expected value")
+	}
+	return &corepb.Registration{
+		Id:  req.RegistrationID,
+		Key: req.Jwk,
+	}, nil
+}
+
+// TestUpdateRegistrationContactBlank tests that the RA's
+// UpdateRegistrationContact method correctly: requires a registration ID; does
+// not require a contact; passes the requested registration ID to the SA; and
+// passes the updated Registration back to the caller.
+func TestUpdateRegistrationContactBlank(t *testing.T) {
+	_, _, ra, _, cleanUp := initAuthorities(t)
+	defer cleanUp()
+
+	expectRegID := int64(1)
+	mockSA := mockSAWithRegistration{expectRegistrationID: expectRegID}
+	ra.SA = &mockSA
+
+	_, err := ra.UpdateRegistrationContact(context.Background(), &rapb.UpdateRegistrationContactRequest{})
+	test.AssertError(t, err, "Should not have been able to update registration contact without a registration ID")
+
+	res, err := ra.UpdateRegistrationContact(context.Background(), &rapb.UpdateRegistrationContactRequest{
+		RegistrationID: 1,
+	})
+	test.AssertNotError(t, err, "Should have been able to update registration with a blank contact")
+	test.AssertEquals(t, res.Id, expectRegID)
+}
+
+// TestUpdateRegistrationContactPopulated tests that the RA's
+// UpdateRegistrationContact method correctly passes an optional contact to the
+// SA, and passes the updated Registration back to the caller.
+func TestUpdateRegistrationContactPopulated(t *testing.T) {
+	_, _, ra, _, cleanUp := initAuthorities(t)
+	defer cleanUp()
+
+	expectRegID := int64(1)
+	expectContacts := []string{"mailto:[email protected]"}
+	mockSA := mockSAWithRegistration{expectRegistrationID: expectRegID, expectContacts: expectContacts}
+	ra.SA = &mockSA
+
+	res, err := ra.UpdateRegistrationContact(context.Background(), &rapb.UpdateRegistrationContactRequest{
+		RegistrationID: 1,
+		Contacts:       []string{"mailto:[email protected]"},
+	})
+	test.AssertNotError(t, err, "Should have been able to update registration with a populated contact")
+	test.AssertEquals(t, res.Id, expectRegID)
+	test.AssertDeepEquals(t, res.Contact, expectContacts)
+}
+
+// TestUpdateRegistrationKey tests that the RA's UpdateRegistrationKey method
+// correctly requires a registration ID and key, passes them to the SA, and
+// passes the updated Registration back to the caller.
+func TestUpdateRegistrationKey(t *testing.T) {
+	_, _, ra, _, cleanUp := initAuthorities(t)
+	defer cleanUp()
+
+	expectRegID := int64(1)
+	expectJwk := AccountKeyJSONA
+	mockSA := mockSAWithRegistration{expectRegistrationID: expectRegID, expectJwk: expectJwk}
+	ra.SA = &mockSA
+
+	_, err := ra.UpdateRegistrationKey(context.Background(), &rapb.UpdateRegistrationKeyRequest{})
+	test.AssertError(t, err, "Should not have been able to update registration key without a registration ID or key")
+
+	_, err = ra.UpdateRegistrationKey(context.Background(), &rapb.UpdateRegistrationKeyRequest{RegistrationID: 1})
+	test.AssertError(t, err, "Should not have been able to update registration key without a key")
+
+	_, err = ra.UpdateRegistrationKey(context.Background(), &rapb.UpdateRegistrationKeyRequest{Jwk: AccountKeyJSONA})
+	test.AssertError(t, err, "Should not have been able to update registration key without a registration ID")
+
+	res, err := ra.UpdateRegistrationKey(context.Background(), &rapb.UpdateRegistrationKeyRequest{
+		RegistrationID: 1,
+		Jwk:            AccountKeyJSONA,
+	})
+	test.AssertNotError(t, err, "Should have been able to update registration key")
+	test.AssertDeepEquals(t, res.Key, expectJwk)
+}