diff --git a/mocks/mocks.go b/mocks/mocks.go index 09170653f..84f783d9a 100644 --- a/mocks/mocks.go +++ b/mocks/mocks.go @@ -256,6 +256,11 @@ func (sa *StorageAuthority) GetAuthorization(_ context.Context, id string) (core authz.Expires = &exp authz.Challenges[0].URI = "http://localhost:4300/acme/challenge/valid/23" return authz, nil + } else if id == "pending" { + exp := sa.clk.Now().AddDate(100, 0, 0) + authz.Expires = &exp + authz.Status = core.StatusPending + return authz, nil } else if id == "expired" { exp := sa.clk.Now().AddDate(0, -1, 0) authz.Expires = &exp diff --git a/wfe/wfe_test.go b/wfe/wfe_test.go index bb3df92b6..b475a045a 100644 --- a/wfe/wfe_test.go +++ b/wfe/wfe_test.go @@ -27,6 +27,7 @@ import ( corepb "github.com/letsencrypt/boulder/core/proto" "github.com/letsencrypt/boulder/ctpolicy" berrors "github.com/letsencrypt/boulder/errors" + "github.com/letsencrypt/boulder/features" "github.com/letsencrypt/boulder/goodkey" blog "github.com/letsencrypt/boulder/log" "github.com/letsencrypt/boulder/metrics" @@ -1171,7 +1172,8 @@ func TestChallenge(t *testing.T) { } -// MockRAUpdateAuthorizationError is a mock RA that just returns an error on UpdateAuthorization +// MockRAUpdateAuthorizationError is a mock RA that just returns an error on +// UpdateAuthorization or PerformValidation. type MockRAUpdateAuthorizationError struct { MockRegistrationAuthority } @@ -1180,6 +1182,10 @@ func (ra *MockRAUpdateAuthorizationError) UpdateAuthorization(_ context.Context, return core.Authorization{}, errors.New("broken on purpose") } +func (ra *MockRAUpdateAuthorizationError) PerformValidation(_ context.Context, _ *rapb.PerformValidationRequest) (*corepb.Authorization, error) { + return nil, errors.New("broken on purpose") +} + // TestUpdateChallengeFinalizedAuthz tests that POSTing a challenge associated // with an already valid authorization just returns the challenge without calling // the RA. @@ -1200,6 +1206,49 @@ func TestUpdateChallengeFinalizedAuthz(t *testing.T) { }`) } +// TestUpdateChallengeRAError tests that when the RA returns an error from +// UpdateAuthorization (or PerformValidation) that the WFE returns an internal +// server error as expected and does not panic or otherwise bug out. +func TestUpdateChallengeRAError(t *testing.T) { + wfe, _ := setupWFE(t) + // Mock the RA to always fail UpdateAuthorization/PerformValidation + wfe.RA = &MockRAUpdateAuthorizationError{} + + // Update a pending challenge + path := "pending/23" + responseWriter := httptest.NewRecorder() + wfe.Challenge(ctx, newRequestEvent(), responseWriter, + makePostRequestWithPath(path, + signRequest(t, `{"resource":"challenge"}`, wfe.nonceService))) + + // The result should be an internal server error problem. + body := responseWriter.Body.String() + test.AssertUnmarshaledEquals(t, body, `{ + "type": "urn:acme:error:serverInternal", + "detail": "Unable to update challenge", + "status": 500 + }`) + + // Doing the same with the PerformValidationRPC feature flag enabled should + // have the same result + if err := features.Set(map[string]bool{"PerformValidationRPC": true}); err != nil { + t.Fatalf("Failed to enable feature: %v", err) + } + defer features.Reset() + + responseWriter = httptest.NewRecorder() + wfe.Challenge(ctx, newRequestEvent(), responseWriter, + makePostRequestWithPath(path, + signRequest(t, `{"resource":"challenge"}`, wfe.nonceService))) + + body = responseWriter.Body.String() + test.AssertUnmarshaledEquals(t, body, `{ + "type": "urn:acme:error:serverInternal", + "detail": "Unable to update challenge", + "status": 500 + }`) +} + func TestBadNonce(t *testing.T) { wfe, _ := setupWFE(t) diff --git a/wfe2/wfe_test.go b/wfe2/wfe_test.go index 95e971915..306ddcb59 100644 --- a/wfe2/wfe_test.go +++ b/wfe2/wfe_test.go @@ -1102,7 +1102,8 @@ func TestChallenge(t *testing.T) { } } -// MockRAUpdateAuthorizationError is a mock RA that just returns an error on UpdateAuthorization +// MockRAUpdateAuthorizationError is a mock RA that just returns an error on +// UpdateAuthorization or PerformValidation. type MockRAUpdateAuthorizationError struct { MockRegistrationAuthority } @@ -1111,6 +1112,10 @@ func (ra *MockRAUpdateAuthorizationError) UpdateAuthorization(_ context.Context, return core.Authorization{}, errors.New("broken on purpose") } +func (ra *MockRAUpdateAuthorizationError) PerformValidation(_ context.Context, _ *rapb.PerformValidationRequest) (*corepb.Authorization, error) { + return nil, errors.New("broken on purpose") +} + // TestUpdateChallengeFinalizedAuthz tests that POSTing a challenge associated // with an already valid authorization just returns the challenge without calling // the RA. @@ -1131,6 +1136,50 @@ func TestUpdateChallengeFinalizedAuthz(t *testing.T) { }`) } +// TestUpdateChallengeRAError tests that when the RA returns an error from +// UpdateAuthorization (or PerformValidation) that the WFE returns an internal +// server error as expected and does not panic or otherwise bug out. +func TestUpdateChallengeRAError(t *testing.T) { + wfe, _ := setupWFE(t) + // Mock the RA to always fail UpdateAuthorization/PerformValidation + wfe.RA = &MockRAUpdateAuthorizationError{} + + // Update a pending challenge + signedURL := "http://localhost/pending/23" + _, _, jwsBody := signRequestKeyID(t, 1, nil, signedURL, `{}`, wfe.nonceService) + responseWriter := httptest.NewRecorder() + request := makePostRequestWithPath("pending/23", jwsBody) + + wfe.Challenge(ctx, newRequestEvent(), responseWriter, request) + + // The result should be an internal server error problem. + body := responseWriter.Body.String() + test.AssertUnmarshaledEquals(t, body, `{ + "type": "urn:ietf:params:acme:error:serverInternal", + "detail": "Unable to update challenge", + "status": 500 + }`) + + // Doing the same with the PerformValidationRPC feature flag enabled should + // have the same result + if err := features.Set(map[string]bool{"PerformValidationRPC": true}); err != nil { + t.Fatalf("Failed to enable feature: %v", err) + } + defer features.Reset() + + responseWriter = httptest.NewRecorder() + _, _, jwsBody = signRequestKeyID(t, 1, nil, signedURL, `{}`, wfe.nonceService) + request = makePostRequestWithPath("pending/23", jwsBody) + wfe.Challenge(ctx, newRequestEvent(), responseWriter, request) + + body = responseWriter.Body.String() + test.AssertUnmarshaledEquals(t, body, `{ + "type": "urn:ietf:params:acme:error:serverInternal", + "detail": "Unable to update challenge", + "status": 500 + }`) +} + func TestBadNonce(t *testing.T) { wfe, _ := setupWFE(t)