Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: correctly calculate expired_at timestamp for FlowExpired errors #2836

Merged
merged 1 commit into from
Oct 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions cmd/clidoc/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func init() {
"NewInfoNodeLabelSave": text.NewInfoNodeLabelSave(),
"NewInfoNodeLabelSubmit": text.NewInfoNodeLabelSubmit(),
"NewInfoNodeLabelID": text.NewInfoNodeLabelID(),
"NewErrorValidationSettingsFlowExpired": text.NewErrorValidationSettingsFlowExpired(time.Second),
"NewErrorValidationSettingsFlowExpired": text.NewErrorValidationSettingsFlowExpired(aSecondAgo),
"NewInfoSelfServiceSettingsTOTPQRCode": text.NewInfoSelfServiceSettingsTOTPQRCode(),
"NewInfoSelfServiceSettingsTOTPSecret": text.NewInfoSelfServiceSettingsTOTPSecret("{secret}"),
"NewInfoSelfServiceSettingsTOTPSecretLabel": text.NewInfoSelfServiceSettingsTOTPSecretLabel(),
Expand All @@ -66,7 +66,7 @@ func init() {
"NewInfoSelfServiceRegisterWebAuthn": text.NewInfoSelfServiceSettingsRegisterWebAuthn(),
"NewInfoSelfServiceRegisterWebAuthnDisplayName": text.NewInfoSelfServiceRegisterWebAuthnDisplayName(),
"NewInfoSelfServiceRemoveWebAuthn": text.NewInfoSelfServiceRemoveWebAuthn("{name}", aSecondAgo),
"NewErrorValidationVerificationFlowExpired": text.NewErrorValidationVerificationFlowExpired(-time.Second),
"NewErrorValidationVerificationFlowExpired": text.NewErrorValidationVerificationFlowExpired(aSecondAgo),
"NewInfoSelfServiceVerificationSuccessful": text.NewInfoSelfServiceVerificationSuccessful(),
"NewVerificationEmailSent": text.NewVerificationEmailSent(),
"NewErrorValidationVerificationTokenInvalidOrAlreadyUsed": text.NewErrorValidationVerificationTokenInvalidOrAlreadyUsed(),
Expand Down Expand Up @@ -97,7 +97,7 @@ func init() {
"NewInfoLoginLookup": text.NewInfoLoginLookup(),
"NewInfoLoginVerify": text.NewInfoLoginVerify(),
"NewInfoLoginWith": text.NewInfoLoginWith("{provider}"),
"NewErrorValidationLoginFlowExpired": text.NewErrorValidationLoginFlowExpired(time.Second),
"NewErrorValidationLoginFlowExpired": text.NewErrorValidationLoginFlowExpired(aSecondAgo),
"NewErrorValidationLoginNoStrategyFound": text.NewErrorValidationLoginNoStrategyFound(),
"NewErrorValidationRegistrationNoStrategyFound": text.NewErrorValidationRegistrationNoStrategyFound(),
"NewErrorValidationSettingsNoStrategyFound": text.NewErrorValidationSettingsNoStrategyFound(),
Expand All @@ -107,8 +107,8 @@ func init() {
"NewInfoRegistration": text.NewInfoRegistration(),
"NewInfoRegistrationWith": text.NewInfoRegistrationWith("{provider}"),
"NewInfoRegistrationContinue": text.NewInfoRegistrationContinue(),
"NewErrorValidationRegistrationFlowExpired": text.NewErrorValidationRegistrationFlowExpired(time.Second),
"NewErrorValidationRecoveryFlowExpired": text.NewErrorValidationRecoveryFlowExpired(time.Second),
"NewErrorValidationRegistrationFlowExpired": text.NewErrorValidationRegistrationFlowExpired(aSecondAgo),
"NewErrorValidationRecoveryFlowExpired": text.NewErrorValidationRecoveryFlowExpired(aSecondAgo),
"NewRecoverySuccessful": text.NewRecoverySuccessful(inAMinute),
"NewRecoveryEmailSent": text.NewRecoveryEmailSent(),
"NewRecoveryEmailWithCodeSent": text.NewRecoveryEmailWithCodeSent(),
Expand Down
4 changes: 2 additions & 2 deletions internal/registrationhelpers/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ func AssertCommonErrorCases(t *testing.T, reg *driver.RegistryDefault, flows []s
actual, res := testhelpers.RegistrationMakeRequest(t, true, false, f, apiClient, "{}")
assert.Contains(t, res.Request.URL.String(), publicTS.URL+registration.RouteSubmitFlow)
assert.NotEqual(t, "00000000-0000-0000-0000-000000000000", gjson.Get(actual, "use_flow_id").String())
assertx.EqualAsJSONExcept(t, flow.NewFlowExpiredError(time.Now()), json.RawMessage(actual), []string{"use_flow_id", "since"}, "expired", "%s", actual)
assertx.EqualAsJSONExcept(t, flow.NewFlowExpiredError(time.Now()), json.RawMessage(actual), []string{"use_flow_id", "expired_at", "since"}, "expired", "%s", actual)
})

t.Run("type=spa", func(t *testing.T) {
Expand All @@ -463,7 +463,7 @@ func AssertCommonErrorCases(t *testing.T, reg *driver.RegistryDefault, flows []s
actual, res := testhelpers.RegistrationMakeRequest(t, false, true, f, browserClient, "{}")
assert.Contains(t, res.Request.URL.String(), publicTS.URL+registration.RouteSubmitFlow)
assert.NotEqual(t, "00000000-0000-0000-0000-000000000000", gjson.Get(actual, "use_flow_id").String())
assertx.EqualAsJSONExcept(t, flow.NewFlowExpiredError(time.Now()), json.RawMessage(actual), []string{"use_flow_id", "since"}, "expired", "%s", actual)
assertx.EqualAsJSONExcept(t, flow.NewFlowExpiredError(time.Now()), json.RawMessage(actual), []string{"use_flow_id", "expired_at", "since"}, "expired", "%s", actual)
})

t.Run("type=browser", func(t *testing.T) {
Expand Down
10 changes: 7 additions & 3 deletions selfservice/flow/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,11 @@ var (
type ExpiredError struct {
*herodot.DefaultError `json:"error"`

// Since when the flow has expired
Ago time.Duration `json:"since"`
// When the flow has expired
ExpiredAt time.Time `json:"expired_at"`

// DEPRECATED: Please use the "expired_at" field instead to have a more accurate result.
Since time.Duration `json:"since"`

// The flow ID that should be used for the new flow as it contains the correct messages.
FlowID uuid.UUID `json:"use_flow_id"`
Expand All @@ -59,7 +62,8 @@ func (e *ExpiredError) EnhanceJSONError() interface{} {
func NewFlowExpiredError(at time.Time) *ExpiredError {
ago := time.Since(at)
return &ExpiredError{
Ago: ago,
ExpiredAt: at.UTC(),
Since: ago,
DefaultError: x.ErrGone.WithID(text.ErrIDSelfServiceFlowExpired).
WithError("self-service flow expired").
WithReasonf("The self-service flow expired %.2f minutes ago, initialize a new one.", ago.Minutes()),
Expand Down
2 changes: 1 addition & 1 deletion selfservice/flow/login/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func (s *ErrorHandler) PrepareReplacementForExpiredFlow(w http.ResponseWriter, r
return nil, err
}

a.UI.Messages.Add(text.NewErrorValidationLoginFlowExpired(e.Ago))
a.UI.Messages.Add(text.NewErrorValidationLoginFlowExpired(e.ExpiredAt))
if err := s.d.LoginFlowPersister().UpdateLoginFlow(r.Context(), a); err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion selfservice/flow/recovery/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func (s *ErrorHandler) WriteFlowError(
return
}

a.UI.Messages.Add(text.NewErrorValidationRecoveryFlowExpired(e.Ago))
a.UI.Messages.Add(text.NewErrorValidationRecoveryFlowExpired(e.ExpiredAt))
if err := s.d.RecoveryFlowPersister().CreateRecoveryFlow(r.Context(), a); err != nil {
s.forward(w, r, a, err)
return
Expand Down
2 changes: 1 addition & 1 deletion selfservice/flow/registration/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func (s *ErrorHandler) PrepareReplacementForExpiredFlow(w http.ResponseWriter, r
return nil, err
}

a.UI.Messages.Add(text.NewErrorValidationRegistrationFlowExpired(e.Ago))
a.UI.Messages.Add(text.NewErrorValidationRegistrationFlowExpired(e.ExpiredAt))
if err := s.d.RegistrationFlowPersister().UpdateRegistrationFlow(r.Context(), a); err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion selfservice/flow/settings/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func (s *ErrorHandler) PrepareReplacementForExpiredFlow(w http.ResponseWriter, r
return nil, err
}

a.UI.Messages.Add(text.NewErrorValidationSettingsFlowExpired(e.Ago))
a.UI.Messages.Add(text.NewErrorValidationSettingsFlowExpired(e.ExpiredAt))
if err := s.d.SettingsFlowPersister().UpdateSettingsFlow(r.Context(), a); err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion selfservice/flow/verification/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func (s *ErrorHandler) WriteFlowError(
return
}

a.UI.Messages.Add(text.NewErrorValidationVerificationFlowExpired(e.Ago))
a.UI.Messages.Add(text.NewErrorValidationVerificationFlowExpired(e.ExpiredAt))
if err := s.d.VerificationFlowPersister().CreateVerificationFlow(r.Context(), a); err != nil {
s.forward(w, r, a, err)
return
Expand Down
2 changes: 1 addition & 1 deletion selfservice/strategy/code/strategy_recovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ func (s *Strategy) retryRecoveryFlowWithError(w http.ResponseWriter, r *http.Req
config := s.deps.Config()

if expired := new(flow.ExpiredError); errors.As(recErr, &expired) {
return s.retryRecoveryFlowWithMessage(w, r, ft, text.NewErrorValidationRecoveryFlowExpired(expired.Ago))
return s.retryRecoveryFlowWithMessage(w, r, ft, text.NewErrorValidationRecoveryFlowExpired(expired.ExpiredAt))
}

f, err := recovery.NewFlow(config, config.SelfServiceFlowRecoveryRequestLifespan(ctx), s.deps.CSRFHandler().RegenerateToken(w, r), r, s, ft)
Expand Down
2 changes: 1 addition & 1 deletion selfservice/strategy/link/strategy_recovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ func (s *Strategy) retryRecoveryFlowWithError(w http.ResponseWriter, r *http.Req
}

if expired := new(flow.ExpiredError); errors.As(recErr, &expired) {
return s.retryRecoveryFlowWithMessage(w, r, ft, text.NewErrorValidationRecoveryFlowExpired(expired.Ago))
return s.retryRecoveryFlowWithMessage(w, r, ft, text.NewErrorValidationRecoveryFlowExpired(expired.ExpiredAt))
} else {
if err := req.UI.ParseError(node.LinkGroup, recErr); err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion selfservice/strategy/link/strategy_verification.go
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ func (s *Strategy) retryVerificationFlowWithError(w http.ResponseWriter, r *http
}

if expired := new(flow.ExpiredError); errors.As(verErr, &expired) {
return s.retryVerificationFlowWithMessage(w, r, ft, text.NewErrorValidationVerificationFlowExpired(expired.Ago))
return s.retryVerificationFlowWithMessage(w, r, ft, text.NewErrorValidationVerificationFlowExpired(expired.ExpiredAt))
} else {
if err := f.UI.ParseError(node.LinkGroup, verErr); err != nil {
return err
Expand Down
4 changes: 2 additions & 2 deletions selfservice/strategy/password/login_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ func TestCompleteLogin(t *testing.T) {
actual, res := testhelpers.LoginMakeRequest(t, true, false, f, apiClient, testhelpers.EncodeFormAsJSON(t, true, values))
assert.Contains(t, res.Request.URL.String(), publicTS.URL+login.RouteSubmitFlow)
assert.NotEqual(t, "00000000-0000-0000-0000-000000000000", gjson.Get(actual, "use_flow_id").String())
assertx.EqualAsJSONExcept(t, flow.NewFlowExpiredError(time.Now()), json.RawMessage(actual), []string{"use_flow_id", "since"}, "expired", "%s", actual)
assertx.EqualAsJSONExcept(t, flow.NewFlowExpiredError(time.Now()), json.RawMessage(actual), []string{"use_flow_id", "since", "expired_at"}, "expired", "%s", actual)
})

t.Run("type=browser", func(t *testing.T) {
Expand All @@ -220,7 +220,7 @@ func TestCompleteLogin(t *testing.T) {
actual, res := testhelpers.LoginMakeRequest(t, false, true, f, apiClient, testhelpers.EncodeFormAsJSON(t, true, values))
assert.Contains(t, res.Request.URL.String(), publicTS.URL+login.RouteSubmitFlow)
assert.NotEqual(t, "00000000-0000-0000-0000-000000000000", gjson.Get(actual, "use_flow_id").String())
assertx.EqualAsJSONExcept(t, flow.NewFlowExpiredError(time.Now()), json.RawMessage(actual), []string{"use_flow_id", "since"}, "expired", "%s", actual)
assertx.EqualAsJSONExcept(t, flow.NewFlowExpiredError(time.Now()), json.RawMessage(actual), []string{"use_flow_id", "since", "expired_at"}, "expired", "%s", actual)
})
})

Expand Down
6 changes: 3 additions & 3 deletions text/message_login.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,13 +106,13 @@ func NewInfoLoginWith(provider string) *Message {
}
}

func NewErrorValidationLoginFlowExpired(ago time.Duration) *Message {
func NewErrorValidationLoginFlowExpired(expiredAt time.Time) *Message {
return &Message{
ID: ErrorValidationLoginFlowExpired,
Text: fmt.Sprintf("The login flow expired %.2f minutes ago, please try again.", ago.Minutes()),
Text: fmt.Sprintf("The login flow expired %.2f minutes ago, please try again.", Now().Sub(expiredAt).Minutes()),
Type: Error,
Context: context(map[string]interface{}{
"expired_at": Now().UTC().Add(ago),
"expired_at": expiredAt,
}),
}
}
Expand Down
6 changes: 3 additions & 3 deletions text/message_recovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ import (
"time"
)

func NewErrorValidationRecoveryFlowExpired(ago time.Duration) *Message {
func NewErrorValidationRecoveryFlowExpired(expiredAt time.Time) *Message {
return &Message{
ID: ErrorValidationRecoveryFlowExpired,
Text: fmt.Sprintf("The recovery flow expired %.2f minutes ago, please try again.", ago.Minutes()),
Text: fmt.Sprintf("The recovery flow expired %.2f minutes ago, please try again.", (-Until(expiredAt)).Minutes()),
Type: Error,
Context: context(map[string]interface{}{
"expired_at": Now().UTC().Add(ago),
"expired_at": expiredAt,
}),
}
}
Expand Down
6 changes: 3 additions & 3 deletions text/message_registration.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,13 @@ func NewInfoRegistrationContinue() *Message {
}
}

func NewErrorValidationRegistrationFlowExpired(ago time.Duration) *Message {
func NewErrorValidationRegistrationFlowExpired(expiredAt time.Time) *Message {
return &Message{
ID: ErrorValidationRegistrationFlowExpired,
Text: fmt.Sprintf("The registration flow expired %.2f minutes ago, please try again.", ago.Minutes()),
Text: fmt.Sprintf("The registration flow expired %.2f minutes ago, please try again.", (-Until(expiredAt)).Minutes()),
Type: Error,
Context: context(map[string]interface{}{
"expired_at": Now().UTC().Add(ago),
"expired_at": expiredAt,
}),
}
}
Expand Down
6 changes: 3 additions & 3 deletions text/message_settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ import (
"time"
)

func NewErrorValidationSettingsFlowExpired(ago time.Duration) *Message {
func NewErrorValidationSettingsFlowExpired(expiredAt time.Time) *Message {
return &Message{
ID: ErrorValidationSettingsFlowExpired,
Text: fmt.Sprintf("The settings flow expired %.2f minutes ago, please try again.", ago.Minutes()),
Text: fmt.Sprintf("The settings flow expired %.2f minutes ago, please try again.", (-Until(expiredAt)).Minutes()),
Type: Error,
Context: context(map[string]interface{}{
"expired_at": Now().UTC().Add(ago),
"expired_at": expiredAt,
}),
}
}
Expand Down
6 changes: 3 additions & 3 deletions text/message_verification.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ import (
"time"
)

func NewErrorValidationVerificationFlowExpired(ago time.Duration) *Message {
func NewErrorValidationVerificationFlowExpired(expiredAt time.Time) *Message {
return &Message{
ID: ErrorValidationVerificationFlowExpired,
Text: fmt.Sprintf("The verification flow expired %.2f minutes ago, please try again.", ago.Minutes()),
Text: fmt.Sprintf("The verification flow expired %.2f minutes ago, please try again.", (-Until(expiredAt)).Minutes()),
Type: Error,
Context: context(map[string]interface{}{
"expired_at": Now().UTC().Add(ago),
"expired_at": expiredAt,
}),
}
}
Expand Down