SFE: Improve UX when the JWT is malformed or expired (#7637)
Currently, the SFE displays "An error occurred while unpausing your account" in scenarios where it's not correct or helpful. - Return a helpful message when a Subscriber attempts to access the unpause form but fails to copy the entire link - Return a helpful message when a Subscriber attempts to unpause using an expired JWT - Some small cleanups that make the code a little more mistake-proof. Part of https://github.com/letsencrypt/boulder/issues/7406
This commit is contained in:
Samantha Frank
GitHub
parent
a21c417bc0
commit
36a617a55b
|
|
@ -0,0 +1,19 @@
|
|||
{{ template "header" }}
|
||||
|
||||
<div class="section">
|
||||
<h1>Expired unpause URL</h1>
|
||||
<p>
|
||||
If you got here by visiting a URL found in your ACME client logs, please
|
||||
try an unpause URL from a more recent log entry. Each unpause URL is
|
||||
only valid for a short period of time. If you cannot find a valid
|
||||
unpause URL, you may need to re-run your ACME client to generate a new
|
||||
one.
|
||||
</p>
|
||||
<p>
|
||||
If you continue to encounter difficulties, or if you need more help, our
|
||||
<a href='https://community.letsencrypt.org'>community support forum</a>
|
||||
is a great resource for troubleshooting and advice.
|
||||
</p>
|
||||
</div>
|
||||
|
||||
{{template "footer"}}
|
||||
|
|
@ -2,7 +2,7 @@
|
|||
|
||||
<div class="section">
|
||||
|
||||
{{ if eq .UnpauseSuccessful true }}
|
||||
{{ if eq .Successful true }}
|
||||
|
||||
<h1>Account successfully unpaused</h1>
|
||||
<p>
|
||||
|
|
|
|||
83
sfe/sfe.go
83
sfe/sfe.go
|
|
@ -11,6 +11,7 @@ import (
|
|||
"text/template"
|
||||
"time"
|
||||
|
||||
"github.com/go-jose/go-jose/v4/jwt"
|
||||
"github.com/jmhodges/clock"
|
||||
"github.com/prometheus/client_golang/prometheus"
|
||||
"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
|
||||
|
|
@ -137,14 +138,21 @@ func (sfe *SelfServiceFrontEndImpl) BuildID(response http.ResponseWriter, reques
|
|||
// in this form.
|
||||
func (sfe *SelfServiceFrontEndImpl) UnpauseForm(response http.ResponseWriter, request *http.Request) {
|
||||
incomingJWT := request.URL.Query().Get("jwt")
|
||||
if incomingJWT == "" {
|
||||
sfe.unpauseInvalidRequest(response)
|
||||
return
|
||||
}
|
||||
|
||||
regID, identifiers, err := sfe.parseUnpauseJWT(incomingJWT)
|
||||
if err != nil {
|
||||
sfe.unpauseStatusHelper(response, false)
|
||||
if errors.Is(err, jwt.ErrExpired) {
|
||||
// JWT expired before the Subscriber visited the unpause page.
|
||||
sfe.unpauseTokenExpired(response)
|
||||
return
|
||||
}
|
||||
if errors.Is(err, unpause.ErrMalformedJWT) {
|
||||
// JWT is malformed. This could happen if the Subscriber failed to
|
||||
// copy the entire URL from their logs.
|
||||
sfe.unpauseRequestMalformed(response)
|
||||
return
|
||||
}
|
||||
sfe.unpauseFailed(response)
|
||||
return
|
||||
}
|
||||
|
||||
|
|
@ -160,20 +168,26 @@ func (sfe *SelfServiceFrontEndImpl) UnpauseForm(response http.ResponseWriter, re
|
|||
sfe.renderTemplate(response, "unpause-form.html", tmplData{unpausePostForm, incomingJWT, regID, identifiers})
|
||||
}
|
||||
|
||||
// UnpauseSubmit serves a page indicating if the unpause form submission
|
||||
// succeeded or failed upon clicking the unpause button. We are explicitly
|
||||
// choosing to not address CSRF at this time because we control creation and
|
||||
// redemption of the JWT.
|
||||
// UnpauseSubmit serves a page showing the result of the unpause form submission.
|
||||
// CSRF is not addressed because a third party causing submission of an unpause
|
||||
// form is not harmful.
|
||||
func (sfe *SelfServiceFrontEndImpl) UnpauseSubmit(response http.ResponseWriter, request *http.Request) {
|
||||
incomingJWT := request.URL.Query().Get("jwt")
|
||||
if incomingJWT == "" {
|
||||
sfe.unpauseInvalidRequest(response)
|
||||
return
|
||||
}
|
||||
|
||||
_, _, err := sfe.parseUnpauseJWT(incomingJWT)
|
||||
if err != nil {
|
||||
sfe.unpauseStatusHelper(response, false)
|
||||
if errors.Is(err, jwt.ErrExpired) {
|
||||
// JWT expired before the Subscriber could click the unpause button.
|
||||
sfe.unpauseTokenExpired(response)
|
||||
return
|
||||
}
|
||||
if errors.Is(err, unpause.ErrMalformedJWT) {
|
||||
// JWT is malformed. This should never happen if the request came
|
||||
// from our form.
|
||||
sfe.unpauseRequestMalformed(response)
|
||||
return
|
||||
}
|
||||
sfe.unpauseFailed(response)
|
||||
return
|
||||
}
|
||||
|
||||
|
|
@ -186,24 +200,24 @@ func (sfe *SelfServiceFrontEndImpl) UnpauseSubmit(response http.ResponseWriter,
|
|||
http.Redirect(response, request, unpauseStatus, http.StatusFound)
|
||||
}
|
||||
|
||||
// unpauseInvalidRequest is a helper that displays a page indicating the
|
||||
// Subscriber perform basic troubleshooting due to lack of JWT in the data
|
||||
// object.
|
||||
func (sfe *SelfServiceFrontEndImpl) unpauseInvalidRequest(response http.ResponseWriter) {
|
||||
func (sfe *SelfServiceFrontEndImpl) unpauseRequestMalformed(response http.ResponseWriter) {
|
||||
sfe.renderTemplate(response, "unpause-invalid-request.html", nil)
|
||||
}
|
||||
|
||||
type unpauseStatusTemplateData struct {
|
||||
UnpauseSuccessful bool
|
||||
func (sfe *SelfServiceFrontEndImpl) unpauseTokenExpired(response http.ResponseWriter) {
|
||||
sfe.renderTemplate(response, "unpause-expired.html", nil)
|
||||
}
|
||||
|
||||
// unpauseStatus is a helper that, by default, displays a failure message to the
|
||||
// Subscriber indicating that their account has failed to unpause. For failure
|
||||
// scenarios, only when the JWT validation should call this. Other types of
|
||||
// failures should use unpauseInvalidRequest. For successes, call UnpauseStatus
|
||||
// instead.
|
||||
func (sfe *SelfServiceFrontEndImpl) unpauseStatusHelper(response http.ResponseWriter, status bool) {
|
||||
sfe.renderTemplate(response, "unpause-status.html", unpauseStatusTemplateData{status})
|
||||
type unpauseStatusTemplate struct {
|
||||
Successful bool
|
||||
}
|
||||
|
||||
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})
|
||||
}
|
||||
|
||||
// UnpauseStatus displays a success message to the Subscriber indicating that
|
||||
|
|
@ -218,19 +232,22 @@ func (sfe *SelfServiceFrontEndImpl) UnpauseStatus(response http.ResponseWriter,
|
|||
// 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.unpauseStatusHelper(response, true)
|
||||
sfe.unpauseSuccessful(response)
|
||||
}
|
||||
|
||||
// parseUnpauseJWT extracts and returns the subscriber's registration ID and a
|
||||
// slice of paused identifiers from the claims. If the JWT cannot be parsed or
|
||||
// is otherwise invalid, an error is returned.
|
||||
// is otherwise invalid, an error is returned. If the JWT is missing or
|
||||
// malformed, unpause.ErrMalformedJWT is returned.
|
||||
func (sfe *SelfServiceFrontEndImpl) parseUnpauseJWT(incomingJWT string) (int64, []string, error) {
|
||||
slug := strings.Split(unpause.APIPrefix, "/")
|
||||
if len(slug) != 3 {
|
||||
return 0, nil, errors.New("failed to parse API version")
|
||||
if incomingJWT == "" || len(strings.Split(incomingJWT, ".")) != 3 {
|
||||
// JWT is missing or malformed. This could happen if the Subscriber
|
||||
// failed to copy the entire URL from their logs. This should never
|
||||
// happen if the request came from our form.
|
||||
return 0, nil, unpause.ErrMalformedJWT
|
||||
}
|
||||
|
||||
claims, err := unpause.RedeemJWT(incomingJWT, sfe.unpauseHMACKey, slug[2], sfe.clk)
|
||||
claims, err := unpause.RedeemJWT(incomingJWT, sfe.unpauseHMACKey, unpause.APIVersion, sfe.clk)
|
||||
if err != nil {
|
||||
return 0, nil, err
|
||||
}
|
||||
|
|
|
|||
|
|
@ -94,6 +94,8 @@ func TestBuildIDPath(t *testing.T) {
|
|||
func TestUnpausePaths(t *testing.T) {
|
||||
t.Parallel()
|
||||
sfe, fc := setupSFE(t)
|
||||
unpauseSigner, err := unpause.NewJWTSigner(cmd.HMACKeyConfig{KeyFile: "../test/secrets/sfe_unpause_key"})
|
||||
test.AssertNotError(t, err, "Should have been able to create JWT signer, but could not")
|
||||
|
||||
// GET with no JWT
|
||||
responseWriter := httptest.NewRecorder()
|
||||
|
|
@ -111,11 +113,22 @@ func TestUnpausePaths(t *testing.T) {
|
|||
URL: mustParseURL(fmt.Sprintf(unpause.GetForm + "?jwt=x")),
|
||||
})
|
||||
test.AssertEquals(t, responseWriter.Code, http.StatusOK)
|
||||
test.AssertContains(t, responseWriter.Body.String(), "An error occurred while unpausing your account")
|
||||
test.AssertContains(t, responseWriter.Body.String(), "Invalid unpause URL")
|
||||
|
||||
// GET with an expired JWT
|
||||
expiredJWT, err := unpause.GenerateJWT(unpauseSigner, 1234567890, []string{"example.net"}, time.Hour, fc)
|
||||
test.AssertNotError(t, err, "Should have been able to create JWT, but could not")
|
||||
responseWriter = httptest.NewRecorder()
|
||||
// Advance the clock by 337 hours to make the JWT expired.
|
||||
fc.Add(time.Hour * 337)
|
||||
sfe.UnpauseForm(responseWriter, &http.Request{
|
||||
Method: "GET",
|
||||
URL: mustParseURL(fmt.Sprintf(unpause.GetForm + "?jwt=" + expiredJWT)),
|
||||
})
|
||||
test.AssertEquals(t, responseWriter.Code, http.StatusOK)
|
||||
test.AssertContains(t, responseWriter.Body.String(), "Expired unpause URL")
|
||||
|
||||
// GET with a valid JWT
|
||||
unpauseSigner, err := unpause.NewJWTSigner(cmd.HMACKeyConfig{KeyFile: "../test/secrets/sfe_unpause_key"})
|
||||
test.AssertNotError(t, err, "Should have been able to create JWT signer, but could not")
|
||||
validJWT, err := unpause.GenerateJWT(unpauseSigner, 1234567890, []string{"example.com"}, time.Hour, fc)
|
||||
test.AssertNotError(t, err, "Should have been able to create JWT, but could not")
|
||||
responseWriter = httptest.NewRecorder()
|
||||
|
|
@ -126,6 +139,15 @@ func TestUnpausePaths(t *testing.T) {
|
|||
test.AssertEquals(t, responseWriter.Code, http.StatusOK)
|
||||
test.AssertContains(t, responseWriter.Body.String(), "Action required to unpause your account")
|
||||
|
||||
// POST with an expired JWT
|
||||
responseWriter = httptest.NewRecorder()
|
||||
sfe.UnpauseSubmit(responseWriter, &http.Request{
|
||||
Method: "POST",
|
||||
URL: mustParseURL(fmt.Sprintf(unpausePostForm + "?jwt=" + expiredJWT)),
|
||||
})
|
||||
test.AssertEquals(t, responseWriter.Code, http.StatusOK)
|
||||
test.AssertContains(t, responseWriter.Body.String(), "Expired unpause URL")
|
||||
|
||||
// POST with no JWT
|
||||
responseWriter = httptest.NewRecorder()
|
||||
sfe.UnpauseSubmit(responseWriter, &http.Request{
|
||||
|
|
@ -135,14 +157,23 @@ func TestUnpausePaths(t *testing.T) {
|
|||
test.AssertEquals(t, responseWriter.Code, http.StatusOK)
|
||||
test.AssertContains(t, responseWriter.Body.String(), "Invalid unpause URL")
|
||||
|
||||
// POST with an invalid JWT
|
||||
// POST with an invalid JWT, missing one of the three parts
|
||||
responseWriter = httptest.NewRecorder()
|
||||
sfe.UnpauseSubmit(responseWriter, &http.Request{
|
||||
Method: "POST",
|
||||
URL: mustParseURL(fmt.Sprintf(unpausePostForm + "?jwt=x")),
|
||||
URL: mustParseURL(fmt.Sprintf(unpausePostForm + "?jwt=x.x")),
|
||||
})
|
||||
test.AssertEquals(t, responseWriter.Code, http.StatusOK)
|
||||
test.AssertContains(t, responseWriter.Body.String(), "An error occurred while unpausing your account")
|
||||
test.AssertContains(t, responseWriter.Body.String(), "Invalid unpause URL")
|
||||
|
||||
// POST with an invalid JWT, all parts present but missing some characters
|
||||
responseWriter = httptest.NewRecorder()
|
||||
sfe.UnpauseSubmit(responseWriter, &http.Request{
|
||||
Method: "POST",
|
||||
URL: mustParseURL(fmt.Sprintf(unpausePostForm + "?jwt=x.x.x")),
|
||||
})
|
||||
test.AssertEquals(t, responseWriter.Code, http.StatusOK)
|
||||
test.AssertContains(t, responseWriter.Body.String(), "Invalid unpause URL")
|
||||
|
||||
// POST with a valid JWT redirects to a success page
|
||||
responseWriter = httptest.NewRecorder()
|
||||
|
|
|
|||
|
|
@ -17,8 +17,8 @@ const (
|
|||
// API
|
||||
|
||||
// Changing this value will invalidate all existing JWTs.
|
||||
apiVersion = "v1"
|
||||
APIPrefix = "/sfe/" + apiVersion
|
||||
APIVersion = "v1"
|
||||
APIPrefix = "/sfe/" + APIVersion
|
||||
GetForm = APIPrefix + "/unpause"
|
||||
|
||||
// JWT
|
||||
|
|
@ -67,7 +67,7 @@ func GenerateJWT(signer JWTSigner, regID int64, identifiers []string, lifetime t
|
|||
IssuedAt: jwt.NewNumericDate(clk.Now()),
|
||||
Expiry: jwt.NewNumericDate(clk.Now().Add(lifetime)),
|
||||
},
|
||||
V: apiVersion,
|
||||
V: APIVersion,
|
||||
I: strings.Join(identifiers, ","),
|
||||
}
|
||||
|
||||
|
|
@ -79,6 +79,9 @@ func GenerateJWT(signer JWTSigner, regID int64, identifiers []string, lifetime t
|
|||
return serialized, nil
|
||||
}
|
||||
|
||||
// ErrMalformedJWT is returned when the JWT is malformed.
|
||||
var ErrMalformedJWT = errors.New("malformed JWT")
|
||||
|
||||
// RedeemJWT deserializes an unpause JWT and returns the validated claims. The
|
||||
// key is used to validate the signature of the JWT. The version is the expected
|
||||
// API version of the JWT. This function validates that the JWT is:
|
||||
|
|
@ -90,16 +93,18 @@ func GenerateJWT(signer JWTSigner, regID int64, identifiers []string, lifetime t
|
|||
// - subject can be parsed as a 64-bit integer,
|
||||
// - contains a set of paused identifiers as 'Identifiers', and
|
||||
// - contains the API the expected version as 'Version'.
|
||||
//
|
||||
// If the JWT is malformed or invalid in any way, ErrMalformedJWT is returned.
|
||||
func RedeemJWT(token string, key []byte, version string, clk clock.Clock) (JWTClaims, error) {
|
||||
parsedToken, err := jwt.ParseSigned(token, []jose.SignatureAlgorithm{jose.HS256})
|
||||
if err != nil {
|
||||
return JWTClaims{}, fmt.Errorf("parsing JWT: %s", err)
|
||||
return JWTClaims{}, errors.Join(ErrMalformedJWT, err)
|
||||
}
|
||||
|
||||
claims := JWTClaims{}
|
||||
err = parsedToken.Claims(key, &claims)
|
||||
if err != nil {
|
||||
return JWTClaims{}, fmt.Errorf("verifying JWT: %s", err)
|
||||
return JWTClaims{}, errors.Join(ErrMalformedJWT, err)
|
||||
}
|
||||
|
||||
err = claims.Validate(jwt.Expected{
|
||||
|
|
|
|||
|
|
@ -40,7 +40,7 @@ func TestUnpauseJWT(t *testing.T) {
|
|||
name: "valid one identifier",
|
||||
args: args{
|
||||
key: hmacKey,
|
||||
version: apiVersion,
|
||||
version: APIVersion,
|
||||
account: 1234567890,
|
||||
identifiers: []string{"example.com"},
|
||||
lifetime: time.Hour,
|
||||
|
|
@ -53,7 +53,7 @@ func TestUnpauseJWT(t *testing.T) {
|
|||
Audience: jwt.Audience{defaultAudience},
|
||||
Expiry: jwt.NewNumericDate(fc.Now().Add(time.Hour)),
|
||||
},
|
||||
V: apiVersion,
|
||||
V: APIVersion,
|
||||
I: "example.com",
|
||||
},
|
||||
wantGenerateJWTErr: false,
|
||||
|
|
@ -63,7 +63,7 @@ func TestUnpauseJWT(t *testing.T) {
|
|||
name: "valid multiple identifiers",
|
||||
args: args{
|
||||
key: hmacKey,
|
||||
version: apiVersion,
|
||||
version: APIVersion,
|
||||
account: 1234567890,
|
||||
identifiers: []string{"example.com", "example.org", "example.net"},
|
||||
lifetime: time.Hour,
|
||||
|
|
@ -76,7 +76,7 @@ func TestUnpauseJWT(t *testing.T) {
|
|||
Audience: jwt.Audience{defaultAudience},
|
||||
Expiry: jwt.NewNumericDate(fc.Now().Add(time.Hour)),
|
||||
},
|
||||
V: apiVersion,
|
||||
V: APIVersion,
|
||||
I: "example.com,example.org,example.net",
|
||||
},
|
||||
wantGenerateJWTErr: false,
|
||||
|
|
@ -86,7 +86,7 @@ func TestUnpauseJWT(t *testing.T) {
|
|||
name: "invalid no account",
|
||||
args: args{
|
||||
key: hmacKey,
|
||||
version: apiVersion,
|
||||
version: APIVersion,
|
||||
account: 0,
|
||||
identifiers: []string{"example.com"},
|
||||
lifetime: time.Hour,
|
||||
|
|
@ -103,7 +103,7 @@ func TestUnpauseJWT(t *testing.T) {
|
|||
name: "invalid key too small",
|
||||
args: args{
|
||||
key: []byte("key"),
|
||||
version: apiVersion,
|
||||
version: APIVersion,
|
||||
account: 1234567890,
|
||||
identifiers: []string{"example.com"},
|
||||
lifetime: time.Hour,
|
||||
|
|
@ -117,7 +117,7 @@ func TestUnpauseJWT(t *testing.T) {
|
|||
name: "invalid no identifiers",
|
||||
args: args{
|
||||
key: hmacKey,
|
||||
version: apiVersion,
|
||||
version: APIVersion,
|
||||
account: 1234567890,
|
||||
identifiers: nil,
|
||||
lifetime: time.Hour,
|
||||
|
|
|
|||
Loading…
Reference in New Issue