fix: impose expiry on auth code instead of magic link (#1440)

## What kind of change does this PR introduce?

Currently, we check for flow state expiry rather than auth code expiry.
The auth code is created at the point when `/magiclink` is called and
expiry starts from then. However, the auth code should probably start
expiring when the link is verified and the auth code is issued.

We can eventually extend this to other magic link like flows if need. 

Note that the Flow State expiry is capped at 24 hours, as that is when
the regular cleanup takes place. Considered adding a hard restriction on
the maximum validity of `GOTRUE_MAILER_OTP_EXP` but there are a handful
of projects which have it >86400.

The handful of existing projects (number on internal channel) with a OTP
expiry of longer than 24 hours will continue to have the expiry capped
at 24 hours when using PKCE. This should be the same as the current
behaviour since we aren't changing the cleanup duration.

---------

Co-authored-by: joel <[email protected]>

Author: J0

internal/api/external.go

@@ -223,6 +223,9 @@ func (a *API) internalExternalProviderCallback(w http.ResponseWriter, r *http.Re
 			flowState.ProviderAccessToken = providerAccessToken
 			flowState.ProviderRefreshToken = providerRefreshToken
 			flowState.UserID = &(user.ID)
+			issueTime := time.Now()
+			flowState.AuthCodeIssuedAt = &issueTime
+
 			terr = tx.Update(flowState)
 		} else {
 			token, terr = a.issueRefreshToken(ctx, tx, user, models.OAuth, grantParams)

internal/api/pkce.go

@@ -45,6 +45,10 @@ func issueAuthCode(tx *storage.Connection, user *models.User, authenticationMeth
 	} else if err != nil {
 		return "", err
 	}
+	if err := flowState.RecordAuthCodeIssuedAtTime(tx); err != nil {
+		return "", err
+	}
+
 	return flowState.AuthCode, nil
 }
 

internal/api/verify.go

@@ -181,9 +181,9 @@ func (a *API) verifyGet(w http.ResponseWriter, r *http.Request, params *VerifyPa
 		if terr := tx.Reload(user); err != nil {
 			return terr
 		}
+
 		if isImplicitFlow(flowType) {
 			token, terr = a.issueRefreshToken(ctx, tx, user, models.OTP, grantParams)
-
 			if terr != nil {
 				return terr
 			}

internal/models/flow_state.go

@@ -28,6 +28,7 @@ type FlowState struct {
 	ProviderType         string     `json:"provider_type" db:"provider_type"`
 	ProviderAccessToken  string     `json:"provider_access_token" db:"provider_access_token"`
 	ProviderRefreshToken string     `json:"provider_refresh_token" db:"provider_refresh_token"`
+	AuthCodeIssuedAt     *time.Time `json:"auth_code_issued_at" db:"auth_code_issued_at"`
 	CreatedAt            time.Time  `json:"created_at" db:"created_at"`
 	UpdatedAt            time.Time  `json:"updated_at" db:"updated_at"`
 }
@@ -152,5 +153,17 @@ func (f *FlowState) VerifyPKCE(codeVerifier string) error {
 }
 
 func (f *FlowState) IsExpired(expiryDuration time.Duration) bool {
+	if f.AuthCodeIssuedAt != nil && f.AuthenticationMethod == MagicLink.String() {
+		return time.Now().After(f.AuthCodeIssuedAt.Add(expiryDuration))
+	}
 	return time.Now().After(f.CreatedAt.Add(expiryDuration))
 }
+
+func (f *FlowState) RecordAuthCodeIssuedAtTime(tx *storage.Connection) error {
+	issueTime := time.Now()
+	f.AuthCodeIssuedAt = &issueTime
+	if err := tx.Update(f); err != nil {
+		return err
+	}
+	return nil
+}

migrations/20240306115329_add_issued_at_to_flow_state.up.sql

@@ -0,0 +1,3 @@
+do $$ begin
+alter table {{ index .Options "Namespace" }}.flow_state add column if not exists auth_code_issued_at timestamptz null;
+end $$