fix: remove server side cookie token methods (supabase#1742)

## What kind of change does this PR introduce? Remove set cookie tokens and clear cookie token methods as it looks like they aren't done server side. Task: https://www.notion.so/supabase/team-auth-113cb19144c1419ea5fb1d600281d959?p=ff083dad758e44ef9b4e230804e0fee7&pm=s
uxodb · Aug 28, 2024 · 646523f · 646523f
1 parent 370881c
commit 646523f
Show file tree

Hide file tree

Showing 14 changed files with 2 additions and 131 deletions.
diff --git a/example.env b/example.env
@@ -216,9 +216,6 @@ GOTRUE_OPERATOR_TOKEN="unused-operator-token"
 GOTRUE_RATE_LIMIT_HEADER="X-Forwarded-For"
 GOTRUE_RATE_LIMIT_EMAIL_SENT="100"
 
-# Cookie config 
-GOTRUE_COOKIE_KEY="sb"
-GOTRUE_COOKIE_DOMAIN="localhost"
 GOTRUE_MAX_VERIFIED_FACTORS=10
 
 # Auth Hook Configuration

diff --git a/internal/api/anonymous.go b/internal/api/anonymous.go
@@ -44,9 +44,6 @@ func (a *API) SignupAnonymously(w http.ResponseWriter, r *http.Request) error {
 		if terr != nil {
 			return terr
 		}
-		if terr := a.setCookieTokens(config, token, false, w); terr != nil {
-			return terr
-		}
 		return nil
 	})
 	if err != nil {

diff --git a/internal/api/auth.go b/internal/api/auth.go
@@ -16,21 +16,17 @@ import (
 // requireAuthentication checks incoming requests for tokens presented using the Authorization header
 func (a *API) requireAuthentication(w http.ResponseWriter, r *http.Request) (context.Context, error) {
 	token, err := a.extractBearerToken(r)
-	config := a.config
 	if err != nil {
-		a.clearCookieTokens(config, w)
 		return nil, err
 	}
 
 	ctx, err := a.parseJWTClaims(token, r)
 	if err != nil {
-		a.clearCookieTokens(config, w)
 		return ctx, err
 	}
 
 	ctx, err = a.maybeLoadUserOrSession(ctx)
 	if err != nil {
-		a.clearCookieTokens(config, w)
 		return ctx, err
 	}
 	return ctx, err

diff --git a/internal/api/external.go b/internal/api/external.go
@@ -164,7 +164,6 @@ func (a *API) handleOAuthCallback(r *http.Request) (*OAuthProviderData, error) {
 func (a *API) internalExternalProviderCallback(w http.ResponseWriter, r *http.Request) error {
 	ctx := r.Context()
 	db := a.db.WithContext(ctx)
-	config := a.config
 
 	var grantParams models.GrantParams
 	grantParams.FillGrantParams(r)
@@ -264,9 +263,6 @@ func (a *API) internalExternalProviderCallback(w http.ResponseWriter, r *http.Re
 
 		rurl = token.AsRedirectURL(rurl, q)
 
-		if err := a.setCookieTokens(config, token, false, w); err != nil {
-			return internalServerError("Failed to set JWT cookie. %s", err)
-		}
 	}
 
 	http.Redirect(w, r, rurl, http.StatusFound)

diff --git a/internal/api/logout.go b/internal/api/logout.go
@@ -20,8 +20,6 @@ const (
 func (a *API) Logout(w http.ResponseWriter, r *http.Request) error {
 	ctx := r.Context()
 	db := a.db.WithContext(ctx)
-	config := a.config
-
 	scope := LogoutGlobal
 
 	if r.URL.Query() != nil {
@@ -64,7 +62,6 @@ func (a *API) Logout(w http.ResponseWriter, r *http.Request) error {
 		return internalServerError("Error logging out user").WithInternalError(err)
 	}
 
-	a.clearCookieTokens(config, w)
 	w.WriteHeader(http.StatusNoContent)
 
 	return nil

diff --git a/internal/api/logout_test.go b/internal/api/logout_test.go
@@ -71,13 +71,5 @@ func (ts *LogoutTestSuite) TestLogoutSuccess() {
 
 		ts.API.handler.ServeHTTP(w, req)
 		require.Equal(ts.T(), http.StatusNoContent, w.Code)
-
-		accessTokenKey := fmt.Sprintf("%v-access-token", ts.Config.Cookie.Key)
-		refreshTokenKey := fmt.Sprintf("%v-refresh-token", ts.Config.Cookie.Key)
-		for _, c := range w.Result().Cookies() {
-			if c.Name == accessTokenKey || c.Name == refreshTokenKey {
-				require.Equal(ts.T(), "", c.Value)
-			}
-		}
 	}
 }
diff --git a/internal/api/mfa.go b/internal/api/mfa.go
@@ -544,9 +544,6 @@ func (a *API) verifyTOTPFactor(w http.ResponseWriter, r *http.Request, params *V
 		if terr != nil {
 			return terr
 		}
-		if terr = a.setCookieTokens(config, token, false, w); terr != nil {
-			return internalServerError("Failed to set JWT cookie. %s", terr)
-		}
 		if terr = models.InvalidateSessionsWithAALLessThan(tx, user.ID, models.AAL2.String()); terr != nil {
 			return internalServerError("Failed to update sessions. %s", terr)
 		}
@@ -663,9 +660,6 @@ func (a *API) verifyPhoneFactor(w http.ResponseWriter, r *http.Request, params *
 		if terr != nil {
 			return terr
 		}
-		if terr = a.setCookieTokens(config, token, false, w); terr != nil {
-			return internalServerError("Failed to set JWT cookie. %s", terr)
-		}
 		if terr = models.InvalidateSessionsWithAALLessThan(tx, user.ID, models.AAL2.String()); terr != nil {
 			return internalServerError("Failed to update sessions. %s", terr)
 		}

diff --git a/internal/api/middleware.go b/internal/api/middleware.go
@@ -143,7 +143,6 @@ func (a *API) requireAdminCredentials(w http.ResponseWriter, req *http.Request)
 
 	ctx, err := a.parseJWTClaims(t, req)
 	if err != nil {
-		a.clearCookieTokens(a.config, w)
 		return nil, err
 	}
 

diff --git a/internal/api/samlacs.go b/internal/api/samlacs.go
@@ -307,10 +307,6 @@ func (a *API) handleSamlAcs(w http.ResponseWriter, r *http.Request) error {
 		return err
 	}
 
-	if err := a.setCookieTokens(config, token, false, w); err != nil {
-		return internalServerError("Failed to set JWT cookie").WithInternalError(err)
-	}
-
 	if !utilities.IsRedirectURLValid(config, redirectTo) {
 		redirectTo = config.SiteURL
 	}

diff --git a/internal/api/signup.go b/internal/api/signup.go
@@ -323,10 +323,6 @@ func (a *API) Signup(w http.ResponseWriter, r *http.Request) error {
 			if terr != nil {
 				return terr
 			}
-
-			if terr = a.setCookieTokens(config, token, false, w); terr != nil {
-				return internalServerError("Failed to set JWT cookie. %s", terr)
-			}
 			return nil
 		})
 		if err != nil {

diff --git a/internal/api/token.go b/internal/api/token.go
@@ -2,7 +2,6 @@ package api
 
 import (
 	"context"
-	"errors"
 	"net/http"
 	"net/url"
 	"strconv"
@@ -14,7 +13,6 @@ import (
 	"github.com/golang-jwt/jwt/v5"
 	"github.com/xeipuuv/gojsonschema"
 
-	"github.com/supabase/auth/internal/conf"
 	"github.com/supabase/auth/internal/hooks"
 	"github.com/supabase/auth/internal/metering"
 	"github.com/supabase/auth/internal/models"
@@ -224,9 +222,6 @@ func (a *API) ResourceOwnerPasswordGrant(ctx context.Context, w http.ResponseWri
 			return terr
 		}
 
-		if terr = a.setCookieTokens(config, token, false, w); terr != nil {
-			return internalServerError("Failed to set JWT cookie. %s", terr)
-		}
 		return nil
 	})
 	if err != nil {
@@ -486,59 +481,6 @@ func (a *API) updateMFASessionAndClaims(r *http.Request, tx *storage.Connection,
 	}, nil
 }
 
-// setCookieTokens sets the access_token & refresh_token in the cookies
-func (a *API) setCookieTokens(config *conf.GlobalConfiguration, token *AccessTokenResponse, session bool, w http.ResponseWriter) error {
-	// don't need to catch error here since we always set the cookie name
-	_ = a.setCookieToken(config, "access-token", token.Token, session, w)
-	_ = a.setCookieToken(config, "refresh-token", token.RefreshToken, session, w)
-	return nil
-}
-
-func (a *API) setCookieToken(config *conf.GlobalConfiguration, name string, tokenString string, session bool, w http.ResponseWriter) error {
-	if name == "" {
-		return errors.New("failed to set cookie, invalid name")
-	}
-	cookieName := config.Cookie.Key + "-" + name
-	exp := time.Second * time.Duration(config.Cookie.Duration)
-	cookie := &http.Cookie{
-		Name:     cookieName,
-		Value:    tokenString,
-		Secure:   true,
-		HttpOnly: true,
-		Path:     "/",
-		Domain:   config.Cookie.Domain,
-	}
-	if !session {
-		cookie.Expires = time.Now().Add(exp)
-		cookie.MaxAge = config.Cookie.Duration
-	}
-
-	http.SetCookie(w, cookie)
-	return nil
-}
-
-func (a *API) clearCookieTokens(config *conf.GlobalConfiguration, w http.ResponseWriter) {
-	a.clearCookieToken(config, "access-token", w)
-	a.clearCookieToken(config, "refresh-token", w)
-}
-
-func (a *API) clearCookieToken(config *conf.GlobalConfiguration, name string, w http.ResponseWriter) {
-	cookieName := config.Cookie.Key
-	if name != "" {
-		cookieName += "-" + name
-	}
-	http.SetCookie(w, &http.Cookie{
-		Name:     cookieName,
-		Value:    "",
-		Expires:  time.Now().Add(-1 * time.Hour * 10),
-		MaxAge:   -1,
-		Secure:   true,
-		HttpOnly: true,
-		Path:     "/",
-		Domain:   config.Cookie.Domain,
-	})
-}
-
 func validateTokenClaims(outputClaims map[string]interface{}) error {
 	schemaLoader := gojsonschema.NewStringLoader(hooks.MinimumViableTokenSchema)
 

diff --git a/internal/api/token_refresh.go b/internal/api/token_refresh.go
@@ -182,9 +182,7 @@ func (a *API) RefreshTokenGrant(ctx context.Context, w http.ResponseWriter, r *h
 						time.Second * time.Duration(config.Security.RefreshTokenReuseInterval))
 
 					if a.Now().After(reuseUntil) {
-						a.clearCookieTokens(config, w)
 						// not OK to reuse this token
-
 						if config.Security.RefreshTokenRotationEnabled {
 							// Revoke all tokens in token family
 							if err := models.RevokeTokenFamily(tx, token); err != nil {
@@ -248,9 +246,6 @@ func (a *API) RefreshTokenGrant(ctx context.Context, w http.ResponseWriter, r *h
 				RefreshToken: issuedToken.Token,
 				User:         user,
 			}
-			if terr = a.setCookieTokens(config, newTokenResponse, false, w); terr != nil {
-				return internalServerError("Failed to set JWT cookie. %s", terr)
-			}
 
 			return nil
 		})

diff --git a/internal/api/verify.go b/internal/api/verify.go
@@ -117,7 +117,6 @@ func (a *API) Verify(w http.ResponseWriter, r *http.Request) error {
 func (a *API) verifyGet(w http.ResponseWriter, r *http.Request, params *VerifyParams) error {
 	ctx := r.Context()
 	db := a.db.WithContext(ctx)
-	config := a.config
 
 	var (
 		user        *models.User
@@ -185,9 +184,6 @@ func (a *API) verifyGet(w http.ResponseWriter, r *http.Request, params *VerifyPa
 				return terr
 			}
 
-			if terr = a.setCookieTokens(config, token, false, w); terr != nil {
-				return internalServerError("Failed to set JWT cookie. %s", terr)
-			}
 		} else if isPKCEFlow(flowType) {
 			if authCode, terr = issueAuthCode(tx, user, authenticationMethod); terr != nil {
 				return badRequestError(ErrorCodeFlowStateNotFound, "No associated flow state found. %s", terr)
@@ -227,7 +223,6 @@ func (a *API) verifyGet(w http.ResponseWriter, r *http.Request, params *VerifyPa
 func (a *API) verifyPost(w http.ResponseWriter, r *http.Request, params *VerifyParams) error {
 	ctx := r.Context()
 	db := a.db.WithContext(ctx)
-	config := a.config
 
 	var (
 		user        *models.User
@@ -285,10 +280,6 @@ func (a *API) verifyPost(w http.ResponseWriter, r *http.Request, params *VerifyP
 		if terr != nil {
 			return terr
 		}
-
-		if terr = a.setCookieTokens(config, token, false, w); terr != nil {
-			return internalServerError("Failed to set JWT cookie. %s", terr)
-		}
 		return nil
 	})
 	if err != nil {

diff --git a/internal/conf/configuration.go b/internal/conf/configuration.go
@@ -257,13 +257,8 @@ type GlobalConfiguration struct {
 	Security        SecurityConfiguration    `json:"security"`
 	Sessions        SessionsConfiguration    `json:"sessions"`
 	MFA             MFAConfiguration         `json:"MFA"`
-	Cookie          struct {
-		Key      string `json:"key"`
-		Domain   string `json:"domain"`
-		Duration int    `json:"duration"`
-	} `json:"cookies"`
-	SAML SAMLConfiguration `json:"saml"`
-	CORS CORSConfiguration `json:"cors"`
+	SAML            SAMLConfiguration        `json:"saml"`
+	CORS            CORSConfiguration        `json:"cors"`
 }
 
 type CORSConfiguration struct {
@@ -844,18 +839,6 @@ func (config *GlobalConfiguration) ApplyDefaults() error {
 		config.Sms.Template = ""
 	}
 
-	if config.Cookie.Key == "" {
-		config.Cookie.Key = "sb"
-	}
-
-	if config.Cookie.Domain == "" {
-		config.Cookie.Domain = ""
-	}
-
-	if config.Cookie.Duration == 0 {
-		config.Cookie.Duration = 86400
-	}
-
 	if config.URIAllowList == nil {
 		config.URIAllowList = []string{}
 	}