letsencrypt
/
boulder
mirror of https://github.com/letsencrypt/boulder.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Move MergeUpdate out of core (#2114) 

Fixes #2022
This commit is contained in:
Ben Irving 2016-08-08 17:12:52 -07:00 committed by Roland Bracewell Shoemaker
parent fc39781274
commit 8b622c805a
4 changed files with 147 additions and 147 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

64
core/objects.go
View File

@ -159,70 +159,6 @@ type Registration struct {
CreatedAt time.Time `json:"createdAt"`
}
func (r *Registration) contactsEqual(other Registration) bool {
// If there is no existing contact slice, or the contact slice lengths
// differ, then the other contact is not equal
if r.Contact == nil || len(*other.Contact) != len(*r.Contact) {
return false
}
// If there is an existing contact slice and it has the same length as the
// new contact slice we need to look at each AcmeURL to determine if there
// is a change being made
//
// TODO(cpu): After #1966 is merged and the `AcmeURL`s are just `String`s we
// should use `sort.Strings` here to ensure a consistent
// comparison.
a := *other.Contact
b := *r.Contact
for i := 0; i < len(a); i++ {
// In order to call .String() we need to make sure the AcmeURL pointers
// aren't nil. In practice `a[i]` should never be nil but `b[i]` might be
if a[i] == nil || b[i] == nil {
// We return false, allowing MergeUpdate to use `other` to overwrite. The
// nil value in the Contact slice will be turned into a user-facing error
// when the RA validates the contacts post-merge.
return false
}
// If the contact's string representation differs at any index they aren't
// equal
contactA := (*a[i]).String()
contactB := (*b[i]).String()
if contactA != contactB {
return false
}
}
// They are equal!
return true
}
// MergeUpdate copies a subset of information from the input Registration
// into this one. It returns true if an update was performed and the base object
// was changed, and false if no change was made.
func (r *Registration) MergeUpdate(input Registration) bool {
var changed bool
// Note: we allow input.Contact to overwrite r.Contact even if the former is
// empty in order to allow users to remove the contact associated with
// a registration. Since the field type is a pointer to slice of pointers we
// can perform a nil check to differentiate between an empty value and a nil
// (e.g. not provided) value
if input.Contact != nil && !r.contactsEqual(input) {
r.Contact = input.Contact
changed = true
}
// If there is an agreement in the input and it's not the same as the base,
// then we update the base
if len(input.Agreement) > 0 && input.Agreement != r.Agreement {
r.Agreement = input.Agreement
changed = true
}
return changed
}
// ValidationRecord represents a validation attempt against a specific URL/hostname
// and the IP addresses that were resolved and used
type ValidationRecord struct {

81
core/objects_test.go
View File

@ -12,87 +12,6 @@ import (
"github.com/letsencrypt/boulder/test"
)
func TestRegistrationUpdate(t *testing.T) {
oldURL, _ := ParseAcmeURL("http://old.invalid")
newURL, _ := ParseAcmeURL("http://new.invalid")
reg := Registration{
ID: 1,
Contact: &[]*AcmeURL{oldURL},
Agreement: "",
}
update := Registration{
Contact: &[]*AcmeURL{newURL},
Agreement: "totally!",
}
changed := reg.MergeUpdate(update)
test.AssertEquals(t, changed, true)
test.Assert(t, len(*reg.Contact) == 1 && (*reg.Contact)[0] == (*update.Contact)[0], "Contact was not updated %v != %v")
test.Assert(t, reg.Agreement == update.Agreement, "Agreement was not updated")
// Make sure that a `MergeUpdate` call with a nil entry doesn't produce an
// error and results in a change to the base reg.
nilUpdate := Registration{
Contact: &[]*AcmeURL{nil},
Agreement: "totally!",
}
changed = reg.MergeUpdate(nilUpdate)
test.AssertEquals(t, changed, true)
}
func TestRegistrationContactUpdate(t *testing.T) {
contactURL, _ := ParseAcmeURL("mailto://example@example.com")
fullReg := Registration{
ID: 1,
Contact: &[]*AcmeURL{contactURL},
Agreement: "totally!",
}
// Test that a registration contact can be removed by updating with an empty
// Contact slice.
reg := fullReg
var contactRemoveUpdate Registration
contactRemoveJSON := []byte(`
{
"key": {
"e": "AQAB",
"kty": "RSA",
"n": "tSwgy3ORGvc7YJI9B2qqkelZRUC6F1S5NwXFvM4w5-M0TsxbFsH5UH6adigV0jzsDJ5imAechcSoOhAh9POceCbPN1sTNwLpNbOLiQQ7RD5mY_"
},
"id": 1,
"contact": [],
"agreement": "totally!"
}
`)
err := json.Unmarshal(contactRemoveJSON, &contactRemoveUpdate)
test.AssertNotError(t, err, "Failed to unmarshal contactRemoveJSON")
changed := reg.MergeUpdate(contactRemoveUpdate)
test.AssertEquals(t, changed, true)
test.Assert(t, len(*reg.Contact) == 0, "Contact was not deleted in update")
// Test that a registration contact isn't changed when an update is performed
// with no Contact field
reg = fullReg
var contactSameUpdate Registration
contactSameJSON := []byte(`
{
"key": {
"e": "AQAB",
"kty": "RSA",
"n": "tSwgy3ORGvc7YJI9B2qqkelZRUC6F1S5NwXFvM4w5-M0TsxbFsH5UH6adigV0jzsDJ5imAechcSoOhAh9POceCbPN1sTNwLpNbOLiQQ7RD5mY_"
},
"id": 1,
"agreement": "totally!"
}
`)
err = json.Unmarshal(contactSameJSON, &contactSameUpdate)
test.AssertNotError(t, err, "Failed to unmarshal contactSameJSON")
changed = reg.MergeUpdate(contactSameUpdate)
test.AssertEquals(t, changed, false)
test.Assert(t, len(*reg.Contact) == 1, "len(Contact) was updated unexpectedly")
test.Assert(t, (*reg.Contact)[0].String() == "mailto://example@example.com", "Contact was changed unexpectedly")
}
func TestExpectedKeyAuthorization(t *testing.T) {
ch := Challenge{Token: "hi"}
jwk1 := &jose.JsonWebKey{Key: &rsa.PublicKey{N: big.NewInt(1234), E: 1234}}

68
ra/ra.go
View File

@ -260,7 +260,7 @@ func (ra *RegistrationAuthorityImpl) NewRegistration(ctx context.Context, init c
reg = core.Registration{
Key: init.Key,
}
_ = reg.MergeUpdate(init)
_ = mergeUpdate(&reg, init)
// This field isn't updatable by the end user, so it isn't copied by
// MergeUpdate. But we need to fill it in for new registrations.
@ -772,7 +772,7 @@ func (ra *RegistrationAuthorityImpl) checkLimits(ctx context.Context, names []st
// UpdateRegistration updates an existing Registration with new values.
func (ra *RegistrationAuthorityImpl) UpdateRegistration(ctx context.Context, base core.Registration, update core.Registration) (core.Registration, error) {
if changed := base.MergeUpdate(update); !changed {
if changed := mergeUpdate(&base, update); !changed {
// If merging the update didn't actually change the base then our work is
// done, we can return before calling ra.SA.UpdateRegistration since theres
// nothing for the SA to do
@ -796,6 +796,70 @@ func (ra *RegistrationAuthorityImpl) UpdateRegistration(ctx context.Context, bas
return base, nil
}
func contactsEqual(r *core.Registration, other core.Registration) bool {
// If there is no existing contact slice, or the contact slice lengths
// differ, then the other contact is not equal
if r.Contact == nil || len(*other.Contact) != len(*r.Contact) {
return false
}
// If there is an existing contact slice and it has the same length as the
// new contact slice we need to look at each AcmeURL to determine if there
// is a change being made
//
// TODO(cpu): After #1966 is merged and the `AcmeURL`s are just `String`s we
// should use `sort.Strings` here to ensure a consistent
// comparison.
a := *other.Contact
b := *r.Contact
for i := 0; i < len(a); i++ {
// In order to call .String() we need to make sure the AcmeURL pointers
// aren't nil. In practice `a[i]` should never be nil but `b[i]` might be
if a[i] == nil || b[i] == nil {
// We return false, allowing MergeUpdate to use `other` to overwrite. The
// nil value in the Contact slice will be turned into a user-facing error
// when the RA validates the contacts post-merge.
return false
}
// If the contact's string representation differs at any index they aren't
// equal
contactA := (*a[i]).String()
contactB := (*b[i]).String()
if contactA != contactB {
return false
}
}
// They are equal!
return true
}
// MergeUpdate copies a subset of information from the input Registration
// into the Registration r. It returns true if an update was performed and the base object
// was changed, and false if no change was made.
func mergeUpdate(r *core.Registration, input core.Registration) bool {
var changed bool
// Note: we allow input.Contact to overwrite r.Contact even if the former is
// empty in order to allow users to remove the contact associated with
// a registration. Since the field type is a pointer to slice of pointers we
// can perform a nil check to differentiate between an empty value and a nil
// (e.g. not provided) value
if input.Contact != nil && !contactsEqual(r, input) {
r.Contact = input.Contact
changed = true
}
// If there is an agreement in the input and it's not the same as the base,
// then we update the base
if len(input.Agreement) > 0 && input.Agreement != r.Agreement {
r.Agreement = input.Agreement
changed = true
}
return changed
}
// UpdateAuthorization updates an authorization with new values.
func (ra *RegistrationAuthorityImpl) UpdateAuthorization(ctx context.Context, base core.Authorization, challengeIndex int, response core.Challenge) (authz core.Authorization, err error) {
// Refuse to update expired authorizations

81
ra/ra_test.go
View File

@ -1087,6 +1087,87 @@ func TestCheckCertificatesPerNameLimit(t *testing.T) {
}
}
func TestRegistrationUpdate(t *testing.T) {
oldURL, _ := core.ParseAcmeURL("http://old.invalid")
newURL, _ := core.ParseAcmeURL("http://new.invalid")
reg := core.Registration{
ID: 1,
Contact: &[]*core.AcmeURL{oldURL},
Agreement: "",
}
update := core.Registration{
Contact: &[]*core.AcmeURL{newURL},
Agreement: "totally!",
}
changed := mergeUpdate(&reg, update)
test.AssertEquals(t, changed, true)
test.Assert(t, len(*reg.Contact) == 1 && (*reg.Contact)[0] == (*update.Contact)[0], "Contact was not updated %v != %v")
test.Assert(t, reg.Agreement == update.Agreement, "Agreement was not updated")
// Make sure that a `MergeUpdate` call with a nil entry doesn't produce an
// error and results in a change to the base reg.
nilUpdate := core.Registration{
Contact: &[]*core.AcmeURL{nil},
Agreement: "totally!",
}
changed = mergeUpdate(&reg, nilUpdate)
test.AssertEquals(t, changed, true)
}
func TestRegistrationContactUpdate(t *testing.T) {
contactURL, _ := core.ParseAcmeURL("mailto://example@example.com")
fullReg := core.Registration{
ID: 1,
Contact: &[]*core.AcmeURL{contactURL},
Agreement: "totally!",
}
// Test that a registration contact can be removed by updating with an empty
// Contact slice.
reg := fullReg
var contactRemoveUpdate core.Registration
contactRemoveJSON := []byte(`
{
"key": {
"e": "AQAB",
"kty": "RSA",
"n": "tSwgy3ORGvc7YJI9B2qqkelZRUC6F1S5NwXFvM4w5-M0TsxbFsH5UH6adigV0jzsDJ5imAechcSoOhAh9POceCbPN1sTNwLpNbOLiQQ7RD5mY_"
},
"id": 1,
"contact": [],
"agreement": "totally!"
}
`)
err := json.Unmarshal(contactRemoveJSON, &contactRemoveUpdate)
test.AssertNotError(t, err, "Failed to unmarshal contactRemoveJSON")
changed := mergeUpdate(&reg, contactRemoveUpdate)
test.AssertEquals(t, changed, true)
test.Assert(t, len(*reg.Contact) == 0, "Contact was not deleted in update")
// Test that a registration contact isn't changed when an update is performed
// with no Contact field
reg = fullReg
var contactSameUpdate core.Registration
contactSameJSON := []byte(`
{
"key": {
"e": "AQAB",
"kty": "RSA",
"n": "tSwgy3ORGvc7YJI9B2qqkelZRUC6F1S5NwXFvM4w5-M0TsxbFsH5UH6adigV0jzsDJ5imAechcSoOhAh9POceCbPN1sTNwLpNbOLiQQ7RD5mY_"
},
"id": 1,
"agreement": "totally!"
}
`)
err = json.Unmarshal(contactSameJSON, &contactSameUpdate)
test.AssertNotError(t, err, "Failed to unmarshal contactSameJSON")
changed = mergeUpdate(&reg, contactSameUpdate)
test.AssertEquals(t, changed, false)
test.Assert(t, len(*reg.Contact) == 1, "len(Contact) was updated unexpectedly")
test.Assert(t, (*reg.Contact)[0].String() == "mailto://example@example.com", "Contact was changed unexpectedly")
}
// A mockSAWithFQDNSet is a mock StorageAuthority that supports
// CountCertificatesByName as well as FQDNSetExists. This allows testing
// checkCertificatesPerNameRateLimit's FQDN exemption logic.