Ensure `LockCol` is set correctly on reg update. (#3113)

In 2fb247488f we consolidated the
`regModelV2` and `regModelv1` structs to one `regModel` type. In the
process we accidentally lost the explicit assignment of the
to-be-updated registration model's `LockCol` with the value of the
existing registration's `LockCol`. This meant that the Update was
occurring with a where clause `LockCol=0` (the default value).

In practice this meant that the first reg update would succeed (since
the reg row starts with LockCol=0) but any regs that had already been
updated once before would modify 0 rows in the update (because the where
clause on `LockCol` failed) and this in turn was translated into
a ServerInternal error since we knew the reg being updated did exist.

This commit updates the SA's `UpdateRegistration` function to properly
set the `LockCol` on the to-be-updated row.

This commit additionally adds an integration test for registration
contact information updating to ensure we don't fall into this trap in
the future.

sa/sa.go

@@ -617,7 +617,7 @@ func (ssa *SQLStorageAuthority) MarkCertificateRevoked(ctx context.Context, seri
 // UpdateRegistration stores an updated Registration
 func (ssa *SQLStorageAuthority) UpdateRegistration(ctx context.Context, reg core.Registration) error {
 	const query = "WHERE id = ?"
-	_, err := selectRegistration(ssa.dbMap, query, reg.ID)
+	model, err := selectRegistration(ssa.dbMap, query, reg.ID)
 	if err == sql.ErrNoRows {
 		return berrors.NotFoundError("registration with ID '%d' not found", reg.ID)
 	}
@@ -627,6 +627,9 @@ func (ssa *SQLStorageAuthority) UpdateRegistration(ctx context.Context, reg core
 		return err
 	}
 
+	// Copy the existing registration model's LockCol to the new updated
+	// registration model's LockCol
+	updatedRegModel.LockCol = model.LockCol
 	n, err := ssa.dbMap.Update(updatedRegModel)
 	if err != nil {
 		return err

test/chisel.py

@@ -43,13 +43,41 @@ def make_client(email=None):
     client = acme_client.Client(DIRECTORY, key=key, net=net)
     account = client.register(messages.NewRegistration.from_data(email=email))
     client.agree_to_tos(account)
+    client.account = account
     return client
 
+class NoClientError(ValueError):
+    """
+    An error that occurs when no acme.Client is provided to a function that
+    requires one.
+    """
+    pass
+
+class EmailRequiredError(ValueError):
+    """
+    An error that occurs when a None email is provided to update_email.
+    """
+
+def update_email(client, email):
+    """
+    Use a provided acme.Client to update the client's account to the specified
+    email.
+    """
+    if client is None:
+        raise NoClientError("update_email requires a valid acme.Client argument")
+    if email is None:
+        raise EmailRequiredError("update_email requires an email argument")
+    if not email.startswith("mailto:"):
+        email = "mailto:"+ email
+    acct = client.account
+    updatedAcct = acct.update(body=acct.body.update(contact=(email,)))
+    return client.update_registration(updatedAcct)
+
 def get_chall(authz, typ):
     for chall_body in authz.body.challenges:
         if isinstance(chall_body.chall, typ):
             return chall_body
-    raise "No %s challenge found" % typ
+    raise Exception("No %s challenge found" % typ)
 
 class ValidationError(Exception):
     """An error that occurs during challenge validation."""

test/integration-test.py

@@ -252,6 +252,24 @@ def test_caa():
     chisel.expect_problem("urn:acme:error:caa",
         lambda: auth_and_issue(["bad-caa-reserved.com"]))
 
+def test_account_update():
+    """
+    Create a new ACME client/account with one contact email. Then update the
+    account to a different contact emails.
+    """
+    emails=("initial-email@example.com", "updated-email@example.com", "another-update@example.com")
+    client = chisel.make_client(email=emails[0])
+
+    for email in emails[1:]:
+        result = chisel.update_email(client, email=email)
+        # We expect one contact in the result
+        if len(result.body.contact) != 1:
+            raise Exception("\nUpdate account failed: expected one contact in result, got 0")
+        # We expect it to be the email we just updated to
+        actual = result.body.contact[0]
+        if actual != "mailto:"+email:
+            raise Exception("\nUpdate account failed: expected contact %s, got %s" % (email, actual))
+
 def run(cmd, **kwargs):
     return subprocess.check_output(cmd, shell=True, stderr=subprocess.STDOUT, **kwargs)
 
@@ -464,6 +482,7 @@ def run_chisel():
     test_dns_challenge()
     test_renewal_exemption()
     test_expired_authzs_404()
+    test_account_update()
 
 if __name__ == "__main__":
     try: