diff --git a/api/v1/server/authn/middleware.go b/api/v1/server/authn/middleware.go index 7a2d7f6332..8c4adc9f5e 100644 --- a/api/v1/server/authn/middleware.go +++ b/api/v1/server/authn/middleware.go @@ -60,22 +60,6 @@ func (a *AuthN) authenticate(c echo.Context, r *middleware.RouteInfo) error { return a.handleNoAuth(c) } - var cookieErr error - - if r.Security.CookieAuth() { - cookieErr = a.handleCookieAuth(c) - - c.Set("auth_strategy", "cookie") - - if cookieErr == nil { - return nil - } - } - - if cookieErr != nil && !r.Security.BearerAuth() && !r.Security.CustomAuth() { - return cookieErr - } - var bearerErr error if r.Security.BearerAuth() { @@ -88,7 +72,7 @@ func (a *AuthN) authenticate(c echo.Context, r *middleware.RouteInfo) error { } } - if bearerErr != nil && !r.Security.CustomAuth() { + if bearerErr != nil && !r.Security.CustomAuth() && !r.Security.CookieAuth() { return bearerErr } @@ -104,10 +88,28 @@ func (a *AuthN) authenticate(c echo.Context, r *middleware.RouteInfo) error { } } - if customErr != nil { + if customErr != nil && !r.Security.CookieAuth() { return customErr } + var cookieErr error + + if r.Security.CookieAuth() { + // FIXME(gregfurman): handleCookieAuth (practically) always creates a new UserSession entry. + // Hence, the ordering of bearer -> custom -> cookie intentionally prevents this. + cookieErr = a.handleCookieAuth(c) + + c.Set("auth_strategy", "cookie") + + if cookieErr == nil { + return nil + } + } + + if cookieErr != nil { + return cookieErr + } + return fmt.Errorf("no auth strategy found") } diff --git a/pkg/auth/cookie/sessionstore_test.go b/pkg/auth/cookie/sessionstore_test.go index 9bd17da896..1db48e75ec 100644 --- a/pkg/auth/cookie/sessionstore_test.go +++ b/pkg/auth/cookie/sessionstore_test.go @@ -6,7 +6,6 @@ import ( "context" "net/http" "net/http/httptest" - "os" "testing" "time" @@ -20,7 +19,7 @@ import ( ) func TestSessionStoreSave(t *testing.T) { - _ = os.Setenv("SERVER_MSGQUEUE_RABBITMQ_URL", "amqp://user:password@localhost:5672/") + t.Setenv("SERVER_MSGQUEUE_RABBITMQ_URL", "amqp://user:password@localhost:5672/") time.Sleep(10 * time.Second) // TODO temp hack for tenant non-upsert issue testutils.RunTestWithDatabase(t, func(conf *database.Layer) error { @@ -41,6 +40,7 @@ func TestSessionStoreSave(t *testing.T) { } func TestSessionStoreLogoutClearsCookieWhenSessionRowMissing(t *testing.T) { + t.Setenv("SERVER_MSGQUEUE_RABBITMQ_URL", "amqp://user:password@localhost:5672/") testutils.RunTestWithDatabase(t, func(conf *database.Layer) error { const cookieName = "hatchet" @@ -98,6 +98,7 @@ func TestSessionStoreLogoutClearsCookieWhenSessionRowMissing(t *testing.T) { } func TestSessionStoreGet(t *testing.T) { + t.Setenv("SERVER_MSGQUEUE_RABBITMQ_URL", "amqp://user:password@localhost:5672/") testutils.RunTestWithDatabase(t, func(conf *database.Layer) error { const cookieName = "hatchet"