wfe: Handle empty JSON to /acme/acct like POST-as-GET (#7844)

Early drafts of the ACME spec said that clients should retrieve their existing account information by POSTing the empty JSON object `{}` to their account URL. This instruction was removed in the final version of ACME, replaced by the concept of POST-as-GET, which uses a wholly empty body to accomplish the same goal. However, Boulder has continued to incidentally support this behavior: when we receive an empty JSON object, our `updateAccount` code in the RA applies their desired diff (none) on top of their current account, writes it back to the database, and returns the updated account object...which hasn't actually changed. This behavior is also half-tested by `TestEmptyAccount`, but that test is actually testing that the MockRA implements the same behavior as the real RA; it's not truly testing the WFE's behavior.

This PR changes the WFE to explicitly treat receiving the empty JSON object as a request to retrieve the account data unchanged, rather than implicitly relying on internal details of the RA's account-update logic, which are expected to change in #7827.

---------

Co-authored-by: Jacob Hoffman-Andrews <jsha+github@letsencrypt.org>
This commit is contained in:
James Renken 2024-12-06 16:45:43 -08:00 committed by GitHub
parent 7e8b3fa10f
commit 071b8c5b35
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 59 additions and 23 deletions

View File

@ -1414,8 +1414,10 @@ func (wfe *WebFrontEndImpl) Account(
return
}
// If the body was not empty, then this is an account update request.
if string(body) != "" {
// An empty string means POST-as-GET (i.e. no update). A body of "{}" means
// an update of zero fields, returning the unchanged object. This was the
// recommended way to fetch the account object in ACMEv1.
if string(body) != "" && string(body) != "{}" {
currAcct, prob = wfe.updateAccount(ctx, body, currAcct)
if prob != nil {
wfe.sendError(response, logEvent, prob, nil)

View File

@ -1569,7 +1569,6 @@ func TestNewECDSAAccount(t *testing.T) {
// a populated acct object will be returned.
func TestEmptyAccount(t *testing.T) {
wfe, _, signer := setupWFE(t)
responseWriter := httptest.NewRecorder()
// Test Key 1 is mocked in the mock StorageAuthority used in setupWFE to
// return a populated account for GetRegistrationByKey when test key 1 is
@ -1578,31 +1577,66 @@ func TestEmptyAccount(t *testing.T) {
_, ok := key.(*rsa.PrivateKey)
test.Assert(t, ok, "Couldn't load RSA key")
payload := `{}`
path := "1"
signedURL := "http://localhost/1"
_, _, body := signer.byKeyID(1, key, signedURL, payload)
request := makePostRequestWithPath(path, body)
// Send an account update with the trivial body
wfe.Account(
ctx,
newRequestEvent(),
responseWriter,
request)
testCases := []struct {
Name string
Payload string
ExpectedStatus int
}{
{
Name: "POST empty string to acct",
Payload: "",
ExpectedStatus: http.StatusOK,
},
{
Name: "POST empty JSON object to acct",
Payload: "{}",
ExpectedStatus: http.StatusOK,
},
{
Name: "POST invalid empty JSON string to acct",
Payload: "\"\"",
ExpectedStatus: http.StatusBadRequest,
},
{
Name: "POST invalid empty JSON array to acct",
Payload: "[]",
ExpectedStatus: http.StatusBadRequest,
},
}
responseBody := responseWriter.Body.String()
// There should be no error
test.AssertNotContains(t, responseBody, probs.ErrorNS)
for _, tc := range testCases {
t.Run(tc.Name, func(t *testing.T) {
responseWriter := httptest.NewRecorder()
// We should get back a populated Account
var acct core.Registration
err := json.Unmarshal([]byte(responseBody), &acct)
test.AssertNotError(t, err, "Couldn't unmarshal returned account object")
test.Assert(t, len(*acct.Contact) >= 1, "No contact field in account")
test.AssertEquals(t, (*acct.Contact)[0], "mailto:person@mail.com")
test.AssertEquals(t, acct.Agreement, "")
responseWriter.Body.Reset()
_, _, body := signer.byKeyID(1, key, signedURL, tc.Payload)
request := makePostRequestWithPath(path, body)
// Send an account update with the trivial body
wfe.Account(
ctx,
newRequestEvent(),
responseWriter,
request)
responseBody := responseWriter.Body.String()
test.AssertEquals(t, responseWriter.Code, tc.ExpectedStatus)
// If success is expected, we should get back a populated Account
if tc.ExpectedStatus == http.StatusOK {
var acct core.Registration
err := json.Unmarshal([]byte(responseBody), &acct)
test.AssertNotError(t, err, "Couldn't unmarshal returned account object")
test.Assert(t, len(*acct.Contact) >= 1, "No contact field in account")
test.AssertEquals(t, (*acct.Contact)[0], "mailto:person@mail.com")
test.AssertEquals(t, acct.Agreement, "")
}
responseWriter.Body.Reset()
})
}
}
func TestNewAccount(t *testing.T) {