From fd675245876e065b4501767eb8a2d0025d1d9abd Mon Sep 17 00:00:00 2001 From: Patrik Date: Mon, 24 Oct 2022 17:46:58 +0200 Subject: [PATCH] fix: correctly calculate `expired_at` timestamp for FlowExpired errors --- cmd/clidoc/main.go | 10 +++++----- internal/registrationhelpers/helpers.go | 4 ++-- selfservice/flow/error.go | 10 +++++++--- selfservice/flow/login/error.go | 2 +- selfservice/flow/recovery/error.go | 2 +- selfservice/flow/registration/error.go | 2 +- selfservice/flow/settings/error.go | 2 +- selfservice/flow/verification/error.go | 2 +- selfservice/strategy/code/strategy_recovery.go | 2 +- selfservice/strategy/link/strategy_recovery.go | 2 +- selfservice/strategy/link/strategy_verification.go | 2 +- selfservice/strategy/password/login_test.go | 4 ++-- text/message_login.go | 6 +++--- text/message_recovery.go | 6 +++--- text/message_registration.go | 6 +++--- text/message_settings.go | 6 +++--- text/message_verification.go | 6 +++--- 17 files changed, 39 insertions(+), 35 deletions(-) diff --git a/cmd/clidoc/main.go b/cmd/clidoc/main.go index a8689c85975..4318216df14 100644 --- a/cmd/clidoc/main.go +++ b/cmd/clidoc/main.go @@ -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(), @@ -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(), @@ -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(), @@ -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(), diff --git a/internal/registrationhelpers/helpers.go b/internal/registrationhelpers/helpers.go index 5c14807505a..c6e16d249f2 100644 --- a/internal/registrationhelpers/helpers.go +++ b/internal/registrationhelpers/helpers.go @@ -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) { @@ -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) { diff --git a/selfservice/flow/error.go b/selfservice/flow/error.go index 650a8bdc7b7..98747388ac1 100644 --- a/selfservice/flow/error.go +++ b/selfservice/flow/error.go @@ -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"` @@ -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()), diff --git a/selfservice/flow/login/error.go b/selfservice/flow/login/error.go index e14848eab56..6a2447fc065 100644 --- a/selfservice/flow/login/error.go +++ b/selfservice/flow/login/error.go @@ -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 } diff --git a/selfservice/flow/recovery/error.go b/selfservice/flow/recovery/error.go index 925b098c1d0..4f57384238c 100644 --- a/selfservice/flow/recovery/error.go +++ b/selfservice/flow/recovery/error.go @@ -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 diff --git a/selfservice/flow/registration/error.go b/selfservice/flow/registration/error.go index 41cfadd8adf..43d1f45bf66 100644 --- a/selfservice/flow/registration/error.go +++ b/selfservice/flow/registration/error.go @@ -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 } diff --git a/selfservice/flow/settings/error.go b/selfservice/flow/settings/error.go index 86b20be7fa7..168b0ee857a 100644 --- a/selfservice/flow/settings/error.go +++ b/selfservice/flow/settings/error.go @@ -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 } diff --git a/selfservice/flow/verification/error.go b/selfservice/flow/verification/error.go index 9d50f9623f1..85470b19b23 100644 --- a/selfservice/flow/verification/error.go +++ b/selfservice/flow/verification/error.go @@ -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 diff --git a/selfservice/strategy/code/strategy_recovery.go b/selfservice/strategy/code/strategy_recovery.go index 3e2f26bd731..4e522374eda 100644 --- a/selfservice/strategy/code/strategy_recovery.go +++ b/selfservice/strategy/code/strategy_recovery.go @@ -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) diff --git a/selfservice/strategy/link/strategy_recovery.go b/selfservice/strategy/link/strategy_recovery.go index 007b2194c25..5fd720dbff5 100644 --- a/selfservice/strategy/link/strategy_recovery.go +++ b/selfservice/strategy/link/strategy_recovery.go @@ -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 diff --git a/selfservice/strategy/link/strategy_verification.go b/selfservice/strategy/link/strategy_verification.go index 9815abcaf3f..3dab75f9261 100644 --- a/selfservice/strategy/link/strategy_verification.go +++ b/selfservice/strategy/link/strategy_verification.go @@ -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 diff --git a/selfservice/strategy/password/login_test.go b/selfservice/strategy/password/login_test.go index b1ea33df3c7..746df44733f 100644 --- a/selfservice/strategy/password/login_test.go +++ b/selfservice/strategy/password/login_test.go @@ -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) { @@ -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) }) }) diff --git a/text/message_login.go b/text/message_login.go index 2543ab78edf..3f268afd95e 100644 --- a/text/message_login.go +++ b/text/message_login.go @@ -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, }), } } diff --git a/text/message_recovery.go b/text/message_recovery.go index 0ac63807e82..fb565a4c5ce 100644 --- a/text/message_recovery.go +++ b/text/message_recovery.go @@ -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, }), } } diff --git a/text/message_registration.go b/text/message_registration.go index 56c06ebcaea..53820640d91 100644 --- a/text/message_registration.go +++ b/text/message_registration.go @@ -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, }), } } diff --git a/text/message_settings.go b/text/message_settings.go index 9b2a95a1236..08611c95fe7 100644 --- a/text/message_settings.go +++ b/text/message_settings.go @@ -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, }), } } diff --git a/text/message_verification.go b/text/message_verification.go index 2e7a1b54f49..4c7885392ab 100644 --- a/text/message_verification.go +++ b/text/message_verification.go @@ -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, }), } }