wfe: Use separate UpdateRegistrationContact & UpdateRegistrationKey methods (#7827)

Fixes #7716
Part of #5554

test/integration/errors_test.go

@@ -48,7 +48,7 @@ func TestAccountEmailError(t *testing.T) {
 	longStringBuf.WriteString("@a.com")
 
 	createErrorPrefix := "Error creating new account :: "
-	updateErrorPrefix := "Unable to update account :: "
+	updateErrorPrefix := "Unable to update account :: invalid contact: "
 
 	testCases := []struct {
 		name               string

wfe2/wfe.go

@@ -1442,8 +1442,8 @@ func (wfe *WebFrontEndImpl) Account(
 
 // updateAccount unmarshals an account update request from the provided
 // requestBody to update the given registration. Important: It is assumed the
-// request has already been authenticated by the caller. If the request is
+// request has already been authenticated by the caller. If the request is a
-// a valid update the resulting updated account is returned, otherwise a problem
+// valid update the resulting updated account is returned, otherwise a problem
 // is returned.
 func (wfe *WebFrontEndImpl) updateAccount(
 	ctx context.Context,
@@ -1461,56 +1461,29 @@ func (wfe *WebFrontEndImpl) updateAccount(
 		return nil, probs.Malformed("Error unmarshaling account")
 	}
 
-	// Convert existing account to corepb.Registration
+	// If a user tries to send both a deactivation request and an update to
-	basePb, err := bgrpc.RegistrationToPB(*currAcct)
+	// their contacts, the deactivation will take place and return before an
-	if err != nil {
+	// update would be performed. Deactivation deletes the contacts field.
-		return nil, probs.ServerInternal("Error updating account")
+	if accountUpdateRequest.Status == core.StatusDeactivated {
-	}
+		_, err = wfe.ra.DeactivateRegistration(ctx, &corepb.Registration{Id: currAcct.ID, Status: string(core.StatusDeactivated)})
-
-	var contacts []string
-	var contactsPresent bool
-	if accountUpdateRequest.Contact != nil {
-		contactsPresent = true
-		contacts = *accountUpdateRequest.Contact
-	}
-
-	// Copy over the fields from the request to the registration object used for
-	// the RA updates.
-	// Create corepb.Registration from provided account information
-	updatePb := &corepb.Registration{
-		Contact:         contacts,
-		ContactsPresent: contactsPresent,
-		Status:          string(accountUpdateRequest.Status),
-	}
-
-	// People *will* POST their full accounts to this endpoint, including
-	// the 'valid' status, to avoid always failing out when that happens only
-	// attempt to deactivate if the provided status is different from their current
-	// status.
-	//
-	// If a user tries to send both a deactivation request and an update to their
-	// contacts or subscriber agreement URL the deactivation will take place and
-	// return before an update would be performed.
-	if updatePb.Status != "" && updatePb.Status != basePb.Status {
-		if updatePb.Status != string(core.StatusDeactivated) {
-			return nil, probs.Malformed("Invalid value provided for status field")
-		}
-		_, err := wfe.ra.DeactivateRegistration(ctx, basePb)
 		if err != nil {
 			return nil, web.ProblemDetailsForError(err, "Unable to deactivate account")
 		}
+
 		currAcct.Status = core.StatusDeactivated
 		return currAcct, nil
 	}
 
-	// Account objects contain a JWK object which are merged in UpdateRegistration
+	if accountUpdateRequest.Status != core.StatusValid && accountUpdateRequest.Status != "" {
-	// if it is different from the existing account key. Since this isn't how you
+		return nil, probs.Malformed("Invalid value provided for status field")
-	// update the key we just copy the existing one into the update object here. This
+	}
-	// ensures the key isn't changed and that we can cleanly serialize the update as
-	// JSON to send via RPC to the RA.
-	updatePb.Key = basePb.Key
 
-	updatedAcct, err := wfe.ra.UpdateRegistration(ctx, &rapb.UpdateRegistrationRequest{Base: basePb, Update: updatePb})
+	var contacts []string
+	if accountUpdateRequest.Contact != nil {
+		contacts = *accountUpdateRequest.Contact
+	}
+
+	updatedAcct, err := wfe.ra.UpdateRegistrationContact(ctx, &rapb.UpdateRegistrationContactRequest{RegistrationID: currAcct.ID, Contacts: contacts})
 	if err != nil {
 		return nil, web.ProblemDetailsForError(err, "Unable to update account")
 	}
@@ -1993,18 +1966,9 @@ func (wfe *WebFrontEndImpl) KeyRollover(
 		wfe.sendError(response, logEvent, web.ProblemDetailsForError(err, "Failed to lookup existing keys"), err)
 		return
 	}
-	// Convert account to proto for grpc
-	regPb, err := bgrpc.RegistrationToPB(*acct)
-	if err != nil {
-		wfe.sendError(response, logEvent, probs.ServerInternal("Error marshaling Registration to proto"), err)
-		return
-	}
-
-	// Copy new key into an empty registration to provide as the update
-	updatePb := &corepb.Registration{Key: newKeyBytes}
 
 	// Update the account key to the new key
-	updatedAcctPb, err := wfe.ra.UpdateRegistration(ctx, &rapb.UpdateRegistrationRequest{Base: regPb, Update: updatePb})
+	updatedAcctPb, err := wfe.ra.UpdateRegistrationKey(ctx, &rapb.UpdateRegistrationKeyRequest{RegistrationID: acct.ID, Jwk: newKeyBytes})
 	if err != nil {
 		if errors.Is(err, berrors.Duplicate) {
 			// It is possible that between checking for the existing key, and performing the update

wfe2/wfe_test.go

@@ -194,11 +194,15 @@ func (ra *MockRegistrationAuthority) NewRegistration(ctx context.Context, in *co
 	return in, nil
 }
 
-func (ra *MockRegistrationAuthority) UpdateRegistration(ctx context.Context, in *rapb.UpdateRegistrationRequest, _ ...grpc.CallOption) (*corepb.Registration, error) {
+func (ra *MockRegistrationAuthority) UpdateRegistrationContact(ctx context.Context, in *rapb.UpdateRegistrationContactRequest, _ ...grpc.CallOption) (*corepb.Registration, error) {
-	if !bytes.Equal(in.Base.Key, in.Update.Key) {
+	return &corepb.Registration{
-		in.Base.Key = in.Update.Key
+		Contact: in.Contacts,
-	}
+		Key:     []byte(test1KeyPublicJSON),
-	return in.Base, nil
+	}, nil
+}
+
+func (ra *MockRegistrationAuthority) UpdateRegistrationKey(ctx context.Context, in *rapb.UpdateRegistrationKeyRequest, _ ...grpc.CallOption) (*corepb.Registration, error) {
+	return &corepb.Registration{Key: in.Jwk}, nil
 }
 
 func (ra *MockRegistrationAuthority) PerformValidation(context.Context, *rapb.PerformValidationRequest, ...grpc.CallOption) (*corepb.Authorization, error) {
@@ -3211,10 +3215,7 @@ func TestKeyRollover(t *testing.T) {
 			Payload: `{"oldKey":` + test1KeyPublicJSON + `,"account":"http://localhost/acme/acct/1"}`,
 			ExpectedResponse: `{
 		     "key": ` + string(newJWKJSON) + `,
-		     "contact": [
+		     "status": ""
-		       "mailto:person@mail.com"
-		     ],
-		     "status": "valid"
 		   }`,
 			NewKey: newKeyPriv,
 		},