Skip to content

Commit

Permalink
wfe: Handle empty JSON to /acme/acct like POST-as-GET (#7844)
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
jprenken and jsha authored Dec 7, 2024
1 parent 7e8b3fa commit 071b8c5
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 23 deletions.
6 changes: 4 additions & 2 deletions wfe2/wfe.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
76 changes: 55 additions & 21 deletions wfe2/wfe_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down

0 comments on commit 071b8c5

Please sign in to comment.