diff --git a/wfe2/wfe.go b/wfe2/wfe.go index 331fbf522..9b79cd290 100644 --- a/wfe2/wfe.go +++ b/wfe2/wfe.go @@ -353,8 +353,8 @@ func (wfe *WebFrontEndImpl) Handler(stats prometheus.Registerer) http.Handler { // TODO(@cpu): After November 1st, 2019 support for "GET" to the following // endpoints will be removed, leaving only POST-as-GET support. wfe.HandleFunc(m, orderPath, wfe.GetOrder, "GET", "POST") - wfe.HandleFunc(m, authzv2Path, wfe.AuthorizationV2, "GET", "POST") - wfe.HandleFunc(m, challengev2Path, wfe.ChallengeV2, "GET", "POST") + wfe.HandleFunc(m, authzv2Path, wfe.Authorization, "GET", "POST") + wfe.HandleFunc(m, challengev2Path, wfe.Challenge, "GET", "POST") wfe.HandleFunc(m, certPath, wfe.Certificate, "GET", "POST") // We don't use our special HandleFunc for "/" because it matches everything, @@ -980,10 +980,10 @@ func (wfe *WebFrontEndImpl) logCsr(request *http.Request, cr core.CertificateReq wfe.log.AuditObject("Certificate request", csrLog) } -// ChallengeV2 handles POST requests to challenge URLs belonging to +// Challenge handles POST requests to challenge URLs belonging to // authzv2-style authorizations. Such requests are clients' // responses to the server's challenges. -func (wfe *WebFrontEndImpl) ChallengeV2( +func (wfe *WebFrontEndImpl) Challenge( ctx context.Context, logEvent *web.RequestEvent, response http.ResponseWriter, @@ -1390,29 +1390,12 @@ func (wfe *WebFrontEndImpl) deactivateAuthorization( return true } -// authzLookupFunc is used by handleAuthorization to look up either an authzv1 -// or an authzv2, as appropriate. -type authzLookupFunc func() (*core.Authorization, error) +func (wfe *WebFrontEndImpl) Authorization( + ctx context.Context, + logEvent *web.RequestEvent, + response http.ResponseWriter, + request *http.Request) { -func (wfe *WebFrontEndImpl) AuthorizationV2(ctx context.Context, logEvent *web.RequestEvent, response http.ResponseWriter, request *http.Request) { - wfe.handleAuthorization(ctx, logEvent, response, request, func() (*core.Authorization, error) { - authzID, err := strconv.ParseInt(request.URL.Path, 10, 64) - if err != nil { - return nil, berrors.MalformedError("Invalid authorization ID") - } - authzPB, err := wfe.SA.GetAuthorization2(ctx, &sapb.AuthorizationID2{Id: &authzID}) - if err != nil { - return nil, err - } - authz, err := bgrpc.PBToAuthz(authzPB) - if err != nil { - return nil, err - } - return &authz, nil - }) -} - -func (wfe *WebFrontEndImpl) handleAuthorization(ctx context.Context, logEvent *web.RequestEvent, response http.ResponseWriter, request *http.Request, lookupFunc authzLookupFunc) { if features.Enabled(features.MandatoryPOSTAsGET) && request.Method != http.MethodPost { wfe.sendError(response, logEvent, probs.MethodNotAllowed(), nil) return @@ -1435,7 +1418,13 @@ func (wfe *WebFrontEndImpl) handleAuthorization(ctx context.Context, logEvent *w requestBody = body } - authz, err := lookupFunc() + authzID, err := strconv.ParseInt(request.URL.Path, 10, 64) + if err != nil { + wfe.sendError(response, logEvent, probs.Malformed("Invalid authorization ID"), nil) + return + } + + authzPB, err := wfe.SA.GetAuthorization2(ctx, &sapb.AuthorizationID2{Id: &authzID}) if berrors.Is(err, berrors.NotFound) { wfe.sendError(response, logEvent, probs.NotFound("No such authorization"), nil) return @@ -1446,6 +1435,13 @@ func (wfe *WebFrontEndImpl) handleAuthorization(ctx context.Context, logEvent *w wfe.sendError(response, logEvent, probs.ServerInternal("Problem getting authorization"), err) return } + + authz, err := bgrpc.PBToAuthz(authzPB) + if err != nil { + wfe.sendError(response, logEvent, probs.ServerInternal("Problem getting authorization"), err) + return + } + if authz.Identifier.Type == identifier.DNS { logEvent.DNSName = authz.Identifier.Value } @@ -1472,12 +1468,12 @@ func (wfe *WebFrontEndImpl) handleAuthorization(ctx context.Context, logEvent *w // If the deactivation fails return early as errors and return codes // have already been set. Otherwise continue so that the user gets // sent the deactivated authorization. - if !wfe.deactivateAuthorization(ctx, authz, logEvent, response, requestBody) { + if !wfe.deactivateAuthorization(ctx, &authz, logEvent, response, requestBody) { return } } - wfe.prepAuthorizationForDisplay(request, authz) + wfe.prepAuthorizationForDisplay(request, &authz) err = wfe.writeJsonResponse(response, logEvent, http.StatusOK, authz) if err != nil { diff --git a/wfe2/wfe_test.go b/wfe2/wfe_test.go index c274bed5b..35fca2d51 100644 --- a/wfe2/wfe_test.go +++ b/wfe2/wfe_test.go @@ -1105,7 +1105,7 @@ func TestGetChallenge(t *testing.T) { req.URL.Path = "1/-ZfxEw" test.AssertNotError(t, err, "Could not make NewRequest") - wfe.ChallengeV2(ctx, newRequestEvent(), resp, req) + wfe.Challenge(ctx, newRequestEvent(), resp, req) test.AssertEquals(t, resp.Code, http.StatusOK) @@ -1196,7 +1196,7 @@ func TestChallenge(t *testing.T) { for _, tc := range testCases { t.Run(tc.Name, func(t *testing.T) { responseWriter := httptest.NewRecorder() - wfe.ChallengeV2(ctx, newRequestEvent(), responseWriter, tc.Request) + wfe.Challenge(ctx, newRequestEvent(), responseWriter, tc.Request) // Check the response code, headers and body match expected headers := responseWriter.Header() body := responseWriter.Body.String() @@ -1230,7 +1230,7 @@ func TestUpdateChallengeFinalizedAuthz(t *testing.T) { signedURL := "http://localhost/1/-ZfxEw" _, _, jwsBody := signRequestKeyID(t, 1, nil, signedURL, `{}`, wfe.nonceService) request := makePostRequestWithPath("1/-ZfxEw", jwsBody) - wfe.ChallengeV2(ctx, newRequestEvent(), responseWriter, request) + wfe.Challenge(ctx, newRequestEvent(), responseWriter, request) body := responseWriter.Body.String() test.AssertUnmarshaledEquals(t, body, `{ @@ -1254,7 +1254,7 @@ func TestUpdateChallengeRAError(t *testing.T) { responseWriter := httptest.NewRecorder() request := makePostRequestWithPath("2/-ZfxEw", jwsBody) - wfe.ChallengeV2(ctx, newRequestEvent(), responseWriter, request) + wfe.Challenge(ctx, newRequestEvent(), responseWriter, request) // The result should be an internal server error problem. body := responseWriter.Body.String() @@ -1559,7 +1559,7 @@ func TestGetAuthorization(t *testing.T) { // Expired authorizations should be inaccessible authzURL := "3" responseWriter := httptest.NewRecorder() - wfe.AuthorizationV2(ctx, newRequestEvent(), responseWriter, &http.Request{ + wfe.Authorization(ctx, newRequestEvent(), responseWriter, &http.Request{ Method: "GET", URL: mustParseURL(authzURL), }) @@ -1569,7 +1569,7 @@ func TestGetAuthorization(t *testing.T) { responseWriter.Body.Reset() // Ensure that a valid authorization can't be reached with an invalid URL - wfe.AuthorizationV2(ctx, newRequestEvent(), responseWriter, &http.Request{ + wfe.Authorization(ctx, newRequestEvent(), responseWriter, &http.Request{ URL: mustParseURL("1d"), Method: "GET", }) @@ -1581,7 +1581,7 @@ func TestGetAuthorization(t *testing.T) { responseWriter = httptest.NewRecorder() // Ensure that a POST-as-GET to an authorization works - wfe.AuthorizationV2(ctx, newRequestEvent(), responseWriter, postAsGet) + wfe.Authorization(ctx, newRequestEvent(), responseWriter, postAsGet) test.AssertEquals(t, responseWriter.Code, http.StatusOK) body := responseWriter.Body.String() test.AssertUnmarshaledEquals(t, body, ` @@ -1618,7 +1618,7 @@ func TestAuthorization500(t *testing.T) { wfe, _ := setupWFE(t) responseWriter := httptest.NewRecorder() - wfe.AuthorizationV2(ctx, newRequestEvent(), responseWriter, &http.Request{ + wfe.Authorization(ctx, newRequestEvent(), responseWriter, &http.Request{ Method: "GET", URL: mustParseURL("4"), }) @@ -1641,7 +1641,7 @@ func TestAuthorizationChallengeNamespace(t *testing.T) { // For "oldNS" the SA mock returns an authorization with a failed challenge // that has an error with the type already prefixed by the v1 error NS responseWriter := httptest.NewRecorder() - wfe.AuthorizationV2(ctx, newRequestEvent(), responseWriter, &http.Request{ + wfe.Authorization(ctx, newRequestEvent(), responseWriter, &http.Request{ Method: "GET", URL: mustParseURL("55"), }) @@ -1656,7 +1656,7 @@ func TestAuthorizationChallengeNamespace(t *testing.T) { // For "failed" the SA mock returns an authorization with a failed challenge // that has an error with the type not prefixed by an error namespace. responseWriter = httptest.NewRecorder() - wfe.AuthorizationV2(ctx, newRequestEvent(), responseWriter, &http.Request{ + wfe.Authorization(ctx, newRequestEvent(), responseWriter, &http.Request{ Method: "GET", URL: mustParseURL("56"), }) @@ -2001,7 +2001,7 @@ func TestDeactivateAuthorization(t *testing.T) { _, _, body := signRequestKeyID(t, 1, nil, "http://localhost/1", payload, wfe.nonceService) request := makePostRequestWithPath("1", body) - wfe.AuthorizationV2(ctx, newRequestEvent(), responseWriter, request) + wfe.Authorization(ctx, newRequestEvent(), responseWriter, request) test.AssertUnmarshaledEquals(t, responseWriter.Body.String(), `{"type": "`+probs.V2ErrorNS+`malformed","detail": "Invalid status value","status": 400}`) @@ -2011,7 +2011,7 @@ func TestDeactivateAuthorization(t *testing.T) { _, _, body = signRequestKeyID(t, 1, nil, "http://localhost/1", payload, wfe.nonceService) request = makePostRequestWithPath("1", body) - wfe.AuthorizationV2(ctx, newRequestEvent(), responseWriter, request) + wfe.Authorization(ctx, newRequestEvent(), responseWriter, request) test.AssertUnmarshaledEquals(t, responseWriter.Body.String(), `{ @@ -3071,13 +3071,13 @@ func TestMandatoryPOSTAsGET(t *testing.T) { // GET requests to a mocked authorization path should return an error name: "GET Authz", path: "1", - handler: wfe.AuthorizationV2, + handler: wfe.Authorization, }, { // GET requests to a mocked challenge path should return an error name: "GET Chall", path: "1/-ZfxEw", - handler: wfe.ChallengeV2, + handler: wfe.Challenge, }, { // GET requests to a mocked certificate serial path should return an error @@ -3097,7 +3097,7 @@ func TestMandatoryPOSTAsGET(t *testing.T) { } } -func TestGetChallengeV2UpRel(t *testing.T) { +func TestGetChallengeUpRel(t *testing.T) { if !strings.HasSuffix(os.Getenv("BOULDER_CONFIG_DIR"), "config-next") { return } @@ -3111,7 +3111,7 @@ func TestGetChallengeV2UpRel(t *testing.T) { test.AssertNotError(t, err, "Could not make NewRequest") req.URL.Path = "1/-ZfxEw" - wfe.ChallengeV2(ctx, newRequestEvent(), resp, req) + wfe.Challenge(ctx, newRequestEvent(), resp, req) test.AssertEquals(t, resp.Code, http.StatusOK)