Don't write "null" to DB for missing contacts (#6090)

Instead write `[]`, a better representation of an empty contact set,
and avoid having literal JSON `null`s in our database.

As part of doing so, add some extra code to //sa/model.go that
bypasses the need for //sa/type-converter.go to do any magic
JSON-to-string-slice conversions for us.

Fixes #6074
This commit is contained in:
Aaron Gable 2022-05-10 09:25:41 -07:00 committed by GitHub
parent f6978f396f
commit f29f63a317
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 53 additions and 22 deletions

View File

@ -212,11 +212,11 @@ type issuedNameModel struct {
// regModel is the description of a core.Registration in the database before
type regModel struct {
ID int64 `db:"id"`
Key []byte `db:"jwk"`
KeySHA256 string `db:"jwk_sha256"`
Contact []string `db:"contact"`
Agreement string `db:"agreement"`
ID int64 `db:"id"`
Key []byte `db:"jwk"`
KeySHA256 string `db:"jwk_sha256"`
Contact string `db:"contact"`
Agreement string `db:"agreement"`
// InitialIP is stored as sixteen binary bytes, regardless of whether it
// represents a v4 or v6 IP address.
InitialIP []byte `db:"initialIp"`
@ -239,6 +239,18 @@ func registrationPbToModel(reg *corepb.Registration) (*regModel, error) {
return nil, err
}
// We don't want to write literal JSON "null" strings into the database if the
// list of contact addresses is empty. Replace any possibly-`nil` slice with
// an empty JSON array. We don't need to check reg.ContactPresent, because
// we're going to write the whole object to the database anyway.
jsonContact := []byte("[]")
if len(reg.Contact) != 0 {
jsonContact, err = json.Marshal(reg.Contact)
if err != nil {
return nil, err
}
}
// For some reason we use different serialization formats for InitialIP
// in database models and in protobufs, despite the fact that both formats
// are just []byte.
@ -260,7 +272,7 @@ func registrationPbToModel(reg *corepb.Registration) (*regModel, error) {
ID: reg.Id,
Key: reg.Key,
KeySHA256: sha,
Contact: reg.Contact,
Contact: string(jsonContact),
Agreement: reg.Agreement,
InitialIP: []byte(initialIP.To16()),
CreatedAt: createdAt,
@ -273,11 +285,16 @@ func registrationModelToPb(reg *regModel) (*corepb.Registration, error) {
return nil, errors.New("incomplete Registration retrieved from DB")
}
var contact []string
contact := []string{}
contactsPresent := false
if len(reg.Contact) != 0 {
contact = reg.Contact
contactsPresent = true
if len(reg.Contact) > 0 {
err := json.Unmarshal([]byte(reg.Contact), &contact)
if err != nil {
return nil, err
}
if len(contact) > 0 {
contactsPresent = true
}
}
// For some reason we use different serialization formats for InitialIP

View File

@ -1542,23 +1542,37 @@ def test_caa_extensions():
chisel2.expect_problem("urn:ietf:params:acme:error:caa", lambda: chisel2.auth_and_issue(["accounturi.good-caa-reserved.com"]))
chisel2.auth_and_issue(["accounturi.good-caa-reserved.com"], client=client)
def test_new_account():
"""
Test creating new accounts with no email, empty email, one email, and a
tuple of multiple emails.
"""
for contact in (None, (), ("mailto:single@chisel.com",), ("mailto:one@chisel.com", "mailto:two@chisel.com")):
# We don't use `chisel2.make_client` or `messages.NewRegistration.from_data`
# here because they do too much client-side processing to make the
# contact addresses look "nice".
client = chisel2.uninitialized_client()
result = client.new_account(messages.NewRegistration(contact=contact, terms_of_service_agreed=True))
actual = result.body.contact
if contact is not None and contact != actual:
raise(Exception("New Account failed: expected contact %s, got %s" % (contact, actual)))
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@not-example.com", "updated-email@not-example.com", "another-update@not-example.com")
client = chisel2.make_client(email=emails[0])
for email in emails[1:]:
result = chisel2.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)))
for contact in (None, (), ("mailto:single@chisel.com",), ("mailto:one@chisel.com", "mailto:two@chisel.com")):
# We don't use `chisel2.update_email` or `messages.NewRegistration.from_data`
# here because they do too much client-side processing to make the
# contact addresses look "nice".
print()
client = chisel2.make_client()
update = client.net.account.update(body=client.net.account.body.update(contact=contact))
result = client.update_registration(update)
actual = result.body.contact
if contact is not None and contact != actual:
raise(Exception("New Account failed: expected contact %s, got %s" % (contact, actual)))
def test_renewal_exemption():
"""