Skip to content

Commit

Permalink
fix: don't return on logout, make it idempotent
Browse files Browse the repository at this point in the history
  • Loading branch information
hf committed Nov 1, 2024
1 parent 6ac5624 commit e39473f
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 5 deletions.
10 changes: 7 additions & 3 deletions internal/api/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"net/http"
"strings"

"github.com/go-chi/chi/v5"
"github.com/gofrs/uuid"
jwt "github.com/golang-jwt/jwt/v5"
"github.com/supabase/auth/internal/conf"
Expand All @@ -25,7 +26,10 @@ func (a *API) requireAuthentication(w http.ResponseWriter, r *http.Request) (con
return ctx, err
}

ctx, err = a.maybeLoadUserOrSession(ctx)
routeContext := chi.RouteContext(ctx)
skipSessionMissingError := routeContext != nil && routeContext.RouteMethod == http.MethodPost && routeContext.RoutePath == "/logout"

ctx, err = a.maybeLoadUserOrSession(ctx, skipSessionMissingError)
if err != nil {
return ctx, err
}
Expand Down Expand Up @@ -94,7 +98,7 @@ func (a *API) parseJWTClaims(bearer string, r *http.Request) (context.Context, e
return withToken(ctx, token), nil
}

func (a *API) maybeLoadUserOrSession(ctx context.Context) (context.Context, error) {
func (a *API) maybeLoadUserOrSession(ctx context.Context, skipSessionMissingError bool) (context.Context, error) {
db := a.db.WithContext(ctx)
claims := getClaims(ctx)

Expand Down Expand Up @@ -130,7 +134,7 @@ func (a *API) maybeLoadUserOrSession(ctx context.Context) (context.Context, erro
}
session, err = models.FindSessionByID(db, sessionId, false)
if err != nil {
if models.IsNotFoundError(err) {
if models.IsNotFoundError(err) && !skipSessionMissingError {
return ctx, forbiddenError(ErrorCodeSessionNotFound, "Session from session_id claim in JWT does not exist").WithInternalError(err).WithInternalMessage(fmt.Sprintf("session id (%s) doesn't exist", sessionId))
}
return ctx, err
Expand Down
2 changes: 1 addition & 1 deletion internal/api/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ func (ts *AuthTestSuite) TestMaybeLoadUserOrSession() {

ctx, err := ts.API.parseJWTClaims(userJwt, req)
require.NoError(ts.T(), err)
ctx, err = ts.API.maybeLoadUserOrSession(ctx)
ctx, err = ts.API.maybeLoadUserOrSession(ctx, false)
if c.ExpectedError != nil {
require.Equal(ts.T(), c.ExpectedError.Error(), err.Error())
} else {
Expand Down
2 changes: 1 addition & 1 deletion internal/api/mfa_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,7 @@ func (ts *MFATestSuite) TestSessionsMaintainAALOnRefresh() {

ctx, err := ts.API.parseJWTClaims(data.Token, req)
require.NoError(ts.T(), err)
ctx, err = ts.API.maybeLoadUserOrSession(ctx)
ctx, err = ts.API.maybeLoadUserOrSession(ctx, false)
require.NoError(ts.T(), err)
require.True(ts.T(), getSession(ctx).IsAAL2())
}
Expand Down

0 comments on commit e39473f

Please sign in to comment.