SFE: Call RA.UnpauseAccount and handle result (letsencrypt#7638)

Call `RA.UnpauseAccount` for valid unpause form submissions.

Determine and display the appropriate outcome to the Subscriber based on
the count returned by `RA.UnpauseAccount`:
- If the count is zero, display the "Account already unpaused" message.
- If the count equals the max number of identifiers allowed in a single
request, display a page explaining the need to visit the unpause URL
again.
- Otherwise, display the "Successfully unpaused all N identifiers"
message.

Apply per-request timeout from the SFE configuration.

Part of letsencrypt#7406

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
 		}

sfe/pages/unpause-form.html

@@ -56,7 +56,7 @@ <h1>Ready to unpause?</h1>
         resume requesting certificates for all affected identifiers associated
         with your account, not just those listed above.
     </p>
-    <form action="{{ .UnpauseFormRedirectionPath }}?jwt={{ .JWT }}" method="POST">
+    <form action="{{ .PostPath }}?jwt={{ .JWT }}" method="POST">
         <button class="primary" id="submit">Please Unpause My Account</button>
     </form>
 </div>

sfe/pages/unpause-status.html

@@ -2,26 +2,46 @@
 
 <div class="section">
 
-    {{ if eq .Successful true }}
-
-    <h1>Account successfully unpaused</h1>
+    {{ if and .Successful (gt .Count 0) (lt .Count .Limit) }}
+    <h1>Successfully unpaused all {{ .Count }} identifier(s)</h1>
     <p>
         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.
     </p>
 
-    {{ else }}
+    {{ else if and .Successful (eq .Count .Limit)}}
+    <h1>Some identifiers were unpaused</h1>
+    <p>
+        We can only unpause a limited number of identifiers for each request ({{
+        .Limit }}). There are potentially more identifiers paused for your
+        account.
+    </p>
+    <p>
+        To attempt to unpause more identifiers, visit the unpause URL from
+        your logs again and click the "Please Unpause My Account" button.
+    </p>
 
-    <h1>An error occurred while unpausing your account</h1>
+    {{ else if and .Successful (eq .Count 0) }}
+    <h1>Account already unpaused</h1>
     <p>
-        Please try again later. If you face continued difficulties, please visit our <a
+        There were no identifiers to unpause for your account. If you face
+        continued difficulties, please visit our <a
         href="https://community.letsencrypt.org">community support forum</a>
         for troubleshooting and advice.
     </p>
 
+    {{ else }}
+    <h1>An error occurred while unpausing your account</h1>
+    <p>
+        Please try again later. If you face continued difficulties, please visit
+        our <a href="https://community.letsencrypt.org">community support
+        forum</a>
+        for troubleshooting and advice.
+    </p>
+
     {{ end }}
 
 </div>
 
-{{template "footer"}}
+{{ template "footer" }}

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

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")
 }

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")
 }

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"