diff --git a/sa/sa.go b/sa/sa.go index a565139c77b..747029d6d39 100644 --- a/sa/sa.go +++ b/sa/sa.go @@ -25,6 +25,7 @@ import ( blog "github.com/letsencrypt/boulder/log" "github.com/letsencrypt/boulder/revocation" sapb "github.com/letsencrypt/boulder/sa/proto" + "github.com/letsencrypt/boulder/unpause" ) var ( @@ -1411,10 +1412,9 @@ func (ssa *SQLStorageAuthority) UnpauseAccount(ctx context.Context, req *sapb.Re return nil, errIncompleteRequest } - const batchSize = 10000 total := &sapb.Count{} - for i := 0; i < 5; i++ { + for i := 0; i < unpause.MaxBatches; i++ { result, err := ssa.dbMap.ExecContext(ctx, ` UPDATE paused SET unpausedAt = ? @@ -1424,7 +1424,7 @@ func (ssa *SQLStorageAuthority) UnpauseAccount(ctx context.Context, req *sapb.Re LIMIT ?`, ssa.clk.Now(), req.Id, - batchSize, + unpause.BatchSize, ) if err != nil { return nil, err @@ -1436,7 +1436,7 @@ func (ssa *SQLStorageAuthority) UnpauseAccount(ctx context.Context, req *sapb.Re } total.Count += rowsAffected - if rowsAffected < batchSize { + if rowsAffected < unpause.BatchSize { // Fewer than batchSize rows were updated, so we're done. break } diff --git a/sfe/pages/unpause-form.html b/sfe/pages/unpause-form.html index 3d396c0eff2..4870abc8da7 100644 --- a/sfe/pages/unpause-form.html +++ b/sfe/pages/unpause-form.html @@ -56,7 +56,7 @@

Ready to unpause?

resume requesting certificates for all affected identifiers associated with your account, not just those listed above.

-
+
diff --git a/sfe/pages/unpause-status.html b/sfe/pages/unpause-status.html index 59f0df5f93f..3f1c7b5b6ad 100644 --- a/sfe/pages/unpause-status.html +++ b/sfe/pages/unpause-status.html @@ -2,26 +2,46 @@
- {{ if eq .Successful true }} - -

Account successfully unpaused

+ {{ if and .Successful (gt .Count 0) (lt .Count .Limit) }} +

Successfully unpaused all {{ .Count }} identifier(s)

To obtain a new certificate, re-attempt issuance with your ACME client. Future repeated validation failures with no successes will result in identifiers being paused again.

- {{ else }} + {{ else if and .Successful (eq .Count .Limit)}} +

Some identifiers were unpaused

+

+ We can only unpause a limited number of identifiers for each request ({{ + .Limit }}). There are potentially more identifiers paused for your + account. +

+

+ To attempt to unpause more identifiers, visit the unpause URL from + your logs again and click the "Please Unpause My Account" button. +

-

An error occurred while unpausing your account

+ {{ else if and .Successful (eq .Count 0) }} +

Account already unpaused

- Please try again later. If you face continued difficulties, please visit our community support forum for troubleshooting and advice.

+ {{ else }} +

An error occurred while unpausing your account

+

+ Please try again later. If you face continued difficulties, please visit + our community support + forum + for troubleshooting and advice. +

+ {{ end }}
-{{template "footer"}} +{{ template "footer" }} diff --git a/sfe/sfe.go b/sfe/sfe.go index 19a6e6797c5..01385c2eae8 100644 --- a/sfe/sfe.go +++ b/sfe/sfe.go @@ -6,6 +6,7 @@ import ( "fmt" "io/fs" "net/http" + "net/url" "strconv" "strings" "text/template" @@ -84,23 +85,35 @@ func NewSelfServiceFrontEndImpl( return sfe, nil } +// handleWithTimeout registers a handler with a timeout using an +// http.TimeoutHandler. +func (sfe *SelfServiceFrontEndImpl) handleWithTimeout(mux *http.ServeMux, path string, handler http.HandlerFunc) { + timeout := sfe.requestTimeout + if timeout <= 0 { + // Default to 5 minutes if no timeout is set. + timeout = 5 * time.Minute + } + timeoutHandler := http.TimeoutHandler(handler, timeout, "Request timed out") + mux.Handle(path, timeoutHandler) +} + // Handler returns an http.Handler that uses various functions for various // non-ACME-specified paths. Each endpoint should have a corresponding HTML // page that shares the same name as the endpoint. func (sfe *SelfServiceFrontEndImpl) Handler(stats prometheus.Registerer, oTelHTTPOptions ...otelhttp.Option) http.Handler { - m := http.NewServeMux() + mux := http.NewServeMux() sfs, _ := fs.Sub(staticFS, "static") staticAssetsHandler := http.StripPrefix("/static/", http.FileServerFS(sfs)) + mux.Handle("GET /static/", staticAssetsHandler) - m.Handle("GET /static/", staticAssetsHandler) - m.HandleFunc("/", sfe.Index) - m.HandleFunc("GET /build", sfe.BuildID) - m.HandleFunc("GET "+unpause.GetForm, sfe.UnpauseForm) - m.HandleFunc("POST "+unpausePostForm, sfe.UnpauseSubmit) - m.HandleFunc("GET "+unpauseStatus, sfe.UnpauseStatus) + sfe.handleWithTimeout(mux, "/", sfe.Index) + sfe.handleWithTimeout(mux, "GET /build", sfe.BuildID) + sfe.handleWithTimeout(mux, "GET "+unpause.GetForm, sfe.UnpauseForm) + sfe.handleWithTimeout(mux, "POST "+unpausePostForm, sfe.UnpauseSubmit) + sfe.handleWithTimeout(mux, "GET "+unpauseStatus, sfe.UnpauseStatus) - return measured_http.New(m, sfe.clk, stats, oTelHTTPOptions...) + return measured_http.New(mux, sfe.clk, stats, oTelHTTPOptions...) } // renderTemplate takes the name of an HTML template and optional dynamicData @@ -139,7 +152,7 @@ func (sfe *SelfServiceFrontEndImpl) BuildID(response http.ResponseWriter, reques func (sfe *SelfServiceFrontEndImpl) UnpauseForm(response http.ResponseWriter, request *http.Request) { incomingJWT := request.URL.Query().Get("jwt") - regID, identifiers, err := sfe.parseUnpauseJWT(incomingJWT) + accountID, identifiers, err := sfe.parseUnpauseJWT(incomingJWT) if err != nil { if errors.Is(err, jwt.ErrExpired) { // JWT expired before the Subscriber visited the unpause page. @@ -157,15 +170,14 @@ func (sfe *SelfServiceFrontEndImpl) UnpauseForm(response http.ResponseWriter, re } type tmplData struct { - UnpauseFormRedirectionPath string - JWT string - AccountID int64 - Identifiers []string + PostPath string + JWT string + AccountID int64 + Identifiers []string } - // Serve the actual unpause page given to a Subscriber. Populates the - // unpause form with the JWT from the URL. - sfe.renderTemplate(response, "unpause-form.html", tmplData{unpausePostForm, incomingJWT, regID, identifiers}) + // Present the unpause form to the Subscriber. + sfe.renderTemplate(response, "unpause-form.html", tmplData{unpausePostForm, incomingJWT, accountID, identifiers}) } // UnpauseSubmit serves a page showing the result of the unpause form submission. @@ -174,7 +186,7 @@ func (sfe *SelfServiceFrontEndImpl) UnpauseForm(response http.ResponseWriter, re func (sfe *SelfServiceFrontEndImpl) UnpauseSubmit(response http.ResponseWriter, request *http.Request) { incomingJWT := request.URL.Query().Get("jwt") - _, _, err := sfe.parseUnpauseJWT(incomingJWT) + accountID, _, err := sfe.parseUnpauseJWT(incomingJWT) if err != nil { if errors.Is(err, jwt.ErrExpired) { // JWT expired before the Subscriber could click the unpause button. @@ -191,13 +203,19 @@ func (sfe *SelfServiceFrontEndImpl) UnpauseSubmit(response http.ResponseWriter, return } - // TODO(#7536) Send gRPC request to the RA informing it to unpause - // the account specified in the claim. At this point we should wait - // for the RA to process the request before returning to the client, - // just in case the request fails. + unpaused, err := sfe.ra.UnpauseAccount(request.Context(), &rapb.UnpauseAccountRequest{ + RegistrationID: accountID, + }) + if err != nil { + sfe.unpauseFailed(response) + return + } - // Success, the account has been unpaused. - http.Redirect(response, request, unpauseStatus, http.StatusFound) + // Redirect to the unpause status page with the count of unpaused + // identifiers. + params := url.Values{} + params.Add("count", fmt.Sprintf("%d", unpaused.Count)) + http.Redirect(response, request, unpauseStatus+"?"+params.Encode(), http.StatusFound) } func (sfe *SelfServiceFrontEndImpl) unpauseRequestMalformed(response http.ResponseWriter) { @@ -210,14 +228,20 @@ func (sfe *SelfServiceFrontEndImpl) unpauseTokenExpired(response http.ResponseWr type unpauseStatusTemplate struct { Successful bool + Limit int64 + Count int64 } func (sfe *SelfServiceFrontEndImpl) unpauseFailed(response http.ResponseWriter) { sfe.renderTemplate(response, "unpause-status.html", unpauseStatusTemplate{Successful: false}) } -func (sfe *SelfServiceFrontEndImpl) unpauseSuccessful(response http.ResponseWriter) { - sfe.renderTemplate(response, "unpause-status.html", unpauseStatusTemplate{Successful: true}) +func (sfe *SelfServiceFrontEndImpl) unpauseSuccessful(response http.ResponseWriter, count int64) { + sfe.renderTemplate(response, "unpause-status.html", unpauseStatusTemplate{ + Successful: true, + Limit: unpause.RequestLimit, + Count: count}, + ) } // UnpauseStatus displays a success message to the Subscriber indicating that @@ -229,10 +253,13 @@ func (sfe *SelfServiceFrontEndImpl) UnpauseStatus(response http.ResponseWriter, return } - // TODO(#7580) This should only be reachable after a client has clicked the - // "Please unblock my account" button and that request succeeding. No one - // should be able to access this page otherwise. - sfe.unpauseSuccessful(response) + count, err := strconv.ParseInt(request.URL.Query().Get("count"), 10, 64) + if err != nil || count < 0 { + sfe.unpauseFailed(response) + return + } + + sfe.unpauseSuccessful(response, count) } // parseUnpauseJWT extracts and returns the subscriber's registration ID and a diff --git a/sfe/sfe_test.go b/sfe/sfe_test.go index 9ee887b8506..f6b851f5821 100644 --- a/sfe/sfe_test.go +++ b/sfe/sfe_test.go @@ -182,14 +182,34 @@ func TestUnpausePaths(t *testing.T) { URL: mustParseURL(fmt.Sprintf(unpausePostForm + "?jwt=" + validJWT)), }) test.AssertEquals(t, responseWriter.Code, http.StatusFound) - test.AssertEquals(t, unpauseStatus, responseWriter.Result().Header.Get("Location")) + test.AssertEquals(t, unpauseStatus+"?count=0", responseWriter.Result().Header.Get("Location")) // Redirecting after a successful unpause POST displays the success page. responseWriter = httptest.NewRecorder() sfe.UnpauseStatus(responseWriter, &http.Request{ Method: "GET", - URL: mustParseURL(unpauseStatus), + URL: mustParseURL(unpauseStatus + "?count=1"), }) test.AssertEquals(t, responseWriter.Code, http.StatusOK) - test.AssertContains(t, responseWriter.Body.String(), "Account successfully unpaused") + test.AssertContains(t, responseWriter.Body.String(), "Successfully unpaused all 1 identifier(s)") + + // Redirecting after a successful unpause POST with a count of 0 displays + // the already unpaused page. + responseWriter = httptest.NewRecorder() + sfe.UnpauseStatus(responseWriter, &http.Request{ + Method: "GET", + URL: mustParseURL(unpauseStatus + "?count=0"), + }) + test.AssertEquals(t, responseWriter.Code, http.StatusOK) + test.AssertContains(t, responseWriter.Body.String(), "Account already unpaused") + + // Redirecting after a successful unpause POST with a count equal to the + // maximum number of identifiers displays the success with caveat page. + responseWriter = httptest.NewRecorder() + sfe.UnpauseStatus(responseWriter, &http.Request{ + Method: "GET", + URL: mustParseURL(unpauseStatus + "?count=" + fmt.Sprintf("%d", unpause.RequestLimit)), + }) + test.AssertEquals(t, responseWriter.Code, http.StatusOK) + test.AssertContains(t, responseWriter.Body.String(), "Some identifiers were unpaused") } diff --git a/test/integration/pausing_test.go b/test/integration/pausing_test.go index 8eb194ae09f..c4e7c461a24 100644 --- a/test/integration/pausing_test.go +++ b/test/integration/pausing_test.go @@ -20,7 +20,7 @@ import ( "github.com/letsencrypt/boulder/test" ) -func TestPausedOrderFails(t *testing.T) { +func TestIdentifiersPausedForAccount(t *testing.T) { t.Parallel() if !strings.Contains(os.Getenv("BOULDER_CONFIG_DIR"), "test/config-next") { @@ -72,4 +72,12 @@ func TestPausedOrderFails(t *testing.T) { test.AssertError(t, err, "Should not be able to issue a certificate for a paused domain") test.AssertContains(t, err.Error(), "Your account is temporarily prevented from requesting certificates for") test.AssertContains(t, err.Error(), "https://boulder.service.consul:4003/sfe/v1/unpause?jwt=") + + _, err = saClient.UnpauseAccount(context.Background(), &sapb.RegistrationID{ + Id: regID, + }) + test.AssertNotError(t, err, "Failed to unpause domain") + + _, err = authAndIssue(c, nil, []string{domain}, true) + test.AssertNotError(t, err, "Should be able to issue a certificate for an unpaused domain") } diff --git a/unpause/unpause.go b/unpause/unpause.go index eeb097a3f6f..f88b589f158 100644 --- a/unpause/unpause.go +++ b/unpause/unpause.go @@ -21,6 +21,19 @@ const ( APIPrefix = "/sfe/" + APIVersion GetForm = APIPrefix + "/unpause" + // BatchSize is the maximum number of identifiers that the SA will unpause + // in a single batch. + BatchSize = 10000 + + // MaxBatches is the maximum number of batches that the SA will unpause in a + // single request. + MaxBatches = 5 + + // RequestLimit is the maximum number of identifiers that the SA will + // unpause in a single request. This is used by the SFE to infer whether + // there are more identifiers to unpause. + RequestLimit = BatchSize * MaxBatches + // JWT defaultIssuer = "WFE" defaultAudience = "SFE Unpause"