diff --git a/cmd/backend/dependencies.go b/cmd/backend/dependencies.go index f80cbd4..146f7b6 100644 --- a/cmd/backend/dependencies.go +++ b/cmd/backend/dependencies.go @@ -35,32 +35,6 @@ func AuthStorage(redisClient rueidis.Client) auth.Storage { return auth.NewRedisStorage(redisClient) } -// AuthMiddleware creates an auth.Middleware that can be injected into gin. -func AuthMiddleware(storage auth.Storage) Middleware { - return Middleware{ - Handler: auth.Middleware(storage), - } -} - -// MachineMiddleware creates a machine middleware that can be injected into gin. -func MachineMiddleware() Middleware { - return Middleware{ - Handler: httputils.MachineMiddleware(), - } -} - -// CorsMiddleware creates a cors middleware that can be injected into gin. -func CorsMiddleware(cfg config.Config) Middleware { - return Middleware{ - Handler: cors.New(cors.Config{ - AllowOrigins: cfg.AllowedOrigins, - AllowMethods: []string{"GET", "POST", "OPTIONS"}, - AllowHeaders: []string{"Content-Type", "User-Agent", "Referer"}, - AllowCredentials: true, - }), - } -} - func SqlRunner(cfg config.Config) *sqlrunner.SqlRunner { return sqlrunner.NewSqlRunner(cfg.SqlRunner) } @@ -92,24 +66,29 @@ func AuthService(entClient *ent.Client, storage auth.Storage, config config.Conf } // GinEngine creates a gin engine. -func GinEngine(services []httpapi.Service, middlewares []Middleware, gqlgenHandler *handler.Server, cfg config.Config) *gin.Engine { +func GinEngine(services []httpapi.Service, authStorage auth.Storage, gqlgenHandler *handler.Server, cfg config.Config) *gin.Engine { engine := gin.New() if err := engine.SetTrustedProxies(cfg.TrustProxies); err != nil { slog.Error("error setting trusted proxies", "error", err) } - for _, middleware := range middlewares { - engine.Use(middleware.Handler) - } - engine.Use(gin.Recovery()) - - engine.GET("/", func(ctx *gin.Context) { + engine.Use(httputils.MachineMiddleware()) + engine.Use(cors.New(cors.Config{ + AllowOrigins: cfg.AllowedOrigins, + AllowMethods: []string{"GET", "POST", "OPTIONS"}, + AllowHeaders: []string{"Content-Type", "User-Agent", "Referer"}, + AllowCredentials: true, + })) + + router := engine.Group("/") + router.Use(auth.Middleware(authStorage)) + router.GET("/", func(ctx *gin.Context) { handler := playground.Handler("GraphQL playground", "/query") handler.ServeHTTP(ctx.Writer, ctx.Request) }) - engine.POST("/query", func(ctx *gin.Context) { + router.POST("/query", func(ctx *gin.Context) { gqlgenHandler.ServeHTTP(ctx.Writer, ctx.Request) }) @@ -174,19 +153,6 @@ func GinLifecycle(lifecycle fx.Lifecycle, engine *gin.Engine, cfg config.Config) }) } -// Middleware is a middleware that can be injected into gin. -type Middleware struct { - Handler gin.HandlerFunc -} - -// AnnotateMiddleware annotates a middleware function to be injected into gin. -func AnnotateMiddleware(f any) any { - return fx.Annotate( - f, - fx.ResultTags(`group:"middlewares"`), - ) -} - // AnnotateService annotates a service function to be injected into gin. func AnnotateService(f any) any { return fx.Annotate( diff --git a/cmd/backend/server.go b/cmd/backend/server.go index 0c1ffdb..da10f44 100644 --- a/cmd/backend/server.go +++ b/cmd/backend/server.go @@ -16,14 +16,11 @@ func main() { fx.Provide( AuthStorage, SqlRunner, - AnnotateMiddleware(AuthMiddleware), - AnnotateMiddleware(MachineMiddleware), - AnnotateMiddleware(CorsMiddleware), AnnotateService(AuthService), GqlgenHandler, fx.Annotate( GinEngine, - fx.ParamTags(`group:"services"`, `group:"middlewares"`), + fx.ParamTags(`group:"services"`), ), ), fx.Invoke(GinLifecycle), diff --git a/graph/user.graphqls b/graph/user.graphqls index 309f222..5f18ed0 100644 --- a/graph/user.graphqls +++ b/graph/user.graphqls @@ -92,14 +92,14 @@ extend type Mutation { impersonateUser(userID: ID!): String! @scope(scope: "user:impersonate") """ - Logout from all the devices of the current user. + Logout a user from all his devices. """ - logoutAll: Boolean! @scope(scope: "me:write") + logoutUser(userID: ID!): Boolean! @scope(scope: "user:write") """ - Delete the current user. + Logout from all the devices of the current user. """ - deleteMe: Boolean! @scope(scope: "me:delete") + logoutAll: Boolean! @scope(scope: "me:write") """ Verify the registration of this user. diff --git a/graph/user.resolvers.go b/graph/user.resolvers.go index d4a7844..e195f17 100644 --- a/graph/user.resolvers.go +++ b/graph/user.resolvers.go @@ -189,17 +189,11 @@ func (r *mutationResolver) ImpersonateUser(ctx context.Context, userID int) (str return token, nil } -// LogoutAll is the resolver for the logoutAll field. -func (r *mutationResolver) LogoutAll(ctx context.Context) (bool, error) { - user, ok := auth.GetUser(ctx) - if !ok { - // this should never happen since we have set proper scope - return false, defs.ErrUnauthorized - } - +// LogoutUser is the resolver for the logoutUser field. +func (r *mutationResolver) LogoutUser(ctx context.Context, userID int) (bool, error) { userAccountSrv := r.UserAccount(ctx) - err := userAccountSrv.RevokeAllTokens(ctx, user.UserID) + err := userAccountSrv.RevokeAllTokens(ctx, userID) if err != nil { return false, err } @@ -207,22 +201,15 @@ func (r *mutationResolver) LogoutAll(ctx context.Context) (bool, error) { return true, nil } -// DeleteMe is the resolver for the deleteMe field. -func (r *mutationResolver) DeleteMe(ctx context.Context) (bool, error) { +// LogoutAll is the resolver for the logoutAll field. +func (r *mutationResolver) LogoutAll(ctx context.Context) (bool, error) { user, ok := auth.GetUser(ctx) if !ok { // this should never happen since we have set proper scope return false, defs.ErrUnauthorized } - userAccountSrv := r.UserAccount(ctx) - - err := userAccountSrv.DeleteUser(ctx, user.UserID) - if err != nil { - return false, err - } - - return true, nil + return r.LogoutUser(ctx, user.UserID) } // VerifyRegistration is the resolver for the verifyRegistration field. diff --git a/graph/user.resolvers_test.go b/graph/user.resolvers_test.go index 520b7c7..8ee10be 100644 --- a/graph/user.resolvers_test.go +++ b/graph/user.resolvers_test.go @@ -1251,165 +1251,6 @@ func TestMutationResolver_UpdateMe(t *testing.T) { }) } -func TestMutationResolver_DeleteMe(t *testing.T) { - t.Run("success", func(t *testing.T) { - entClient := testhelper.NewEntSqliteClient(t) - - // Setup database with proper groups and scopes - _, err := setup.Setup(context.Background(), entClient) - require.NoError(t, err) - - // Get the new-user group (which has me:delete scope) - newUserGroup, err := entClient.Group.Query().Where(group.NameEQ(useraccount.NewUserGroupSlug)).Only(context.Background()) - require.NoError(t, err) - - // Create a test user in new-user group - user, err := entClient.User.Create(). - SetName("testuser"). - SetEmail("test@example.com"). - SetGroup(newUserGroup). - Save(context.Background()) - require.NoError(t, err) - - resolver := &Resolver{ - ent: entClient, - auth: &mockAuthStorage{}, - } - - // Create test server with scope directive - cfg := Config{ - Resolvers: resolver, - Directives: DirectiveRoot{Scope: directive.ScopeDirective}, - } - srv := handler.New(NewExecutableSchema(cfg)) - srv.AddTransport(transport.POST{}) - c := client.New(srv) - - // Execute mutation - var resp struct { - DeleteMe bool - } - err = c.Post(`mutation { deleteMe }`, &resp, func(bd *client.Request) { - bd.HTTP = bd.HTTP.WithContext(auth.WithUser(bd.HTTP.Context(), auth.TokenInfo{ - UserID: user.ID, - Scopes: []string{"me:delete"}, - })) - }) - - // Verify response - require.NoError(t, err) - require.True(t, resp.DeleteMe) - - // Verify user was actually deleted - _, err = entClient.User.Get(context.Background(), user.ID) - require.Error(t, err) - require.True(t, ent.IsNotFound(err)) - }) - - t.Run("unauthenticated", func(t *testing.T) { - entClient := testhelper.NewEntSqliteClient(t) - resolver := &Resolver{ - ent: entClient, - auth: &mockAuthStorage{}, - } - - // Create test server with scope directive - cfg := Config{ - Resolvers: resolver, - Directives: DirectiveRoot{Scope: directive.ScopeDirective}, - } - srv := handler.New(NewExecutableSchema(cfg)) - srv.AddTransport(transport.POST{}) - c := client.New(srv) - - // Execute mutation with no auth context - var resp struct { - DeleteMe bool - } - err := c.Post(`mutation { deleteMe }`, &resp) - - // Verify error - require.Error(t, err) - require.Contains(t, err.Error(), defs.ErrUnauthorized.Error()) - }) - - t.Run("insufficient scope", func(t *testing.T) { - entClient := testhelper.NewEntSqliteClient(t) - - resolver := &Resolver{ - ent: entClient, - auth: &mockAuthStorage{}, - } - - // Create test server with scope directive - cfg := Config{ - Resolvers: resolver, - Directives: DirectiveRoot{Scope: directive.ScopeDirective}, - } - srv := handler.New(NewExecutableSchema(cfg)) - srv.AddTransport(transport.POST{}) - c := client.New(srv) - - // Create context with authenticated user but wrong scope - ctx := auth.WithUser(context.Background(), auth.TokenInfo{ - UserID: 1, - Scopes: []string{"user:read"}, - }) - - // Execute mutation - var resp struct { - DeleteMe bool - } - err := c.Post(`mutation { deleteMe }`, &resp, func(bd *client.Request) { - bd.HTTP = bd.HTTP.WithContext(ctx) - }) - - // Verify error - require.Error(t, err) - require.Contains(t, err.Error(), defs.NewErrNoSufficientScope("me:delete").Error()) - }) - - t.Run("user not found", func(t *testing.T) { - entClient := testhelper.NewEntSqliteClient(t) - - // Setup database with proper groups and scopes - _, err := setup.Setup(context.Background(), entClient) - require.NoError(t, err) - - resolver := &Resolver{ - ent: entClient, - auth: &mockAuthStorage{}, - } - - // Create test server with scope directive - cfg := Config{ - Resolvers: resolver, - Directives: DirectiveRoot{Scope: directive.ScopeDirective}, - } - srv := handler.New(NewExecutableSchema(cfg)) - srv.AddTransport(transport.POST{}) - c := client.New(srv) - - // Create context with authenticated user but non-existent user ID - ctx := auth.WithUser(context.Background(), auth.TokenInfo{ - UserID: 999, // Non-existent user ID - Scopes: []string{"me:delete"}, - }) - - // Execute mutation - var resp struct { - DeleteMe bool - } - err = c.Post(`mutation { deleteMe }`, &resp, func(bd *client.Request) { - bd.HTTP = bd.HTTP.WithContext(ctx) - }) - - // Verify error - require.Error(t, err) - require.Contains(t, err.Error(), useraccount.ErrUserNotFound.Error()) - }) -} - func TestMutationResolver_VerifyRegistration(t *testing.T) { t.Run("success", func(t *testing.T) { entClient := testhelper.NewEntSqliteClient(t) diff --git a/httpapi/README.md b/httpapi/README.md index ff2357c..4dbaefe 100644 --- a/httpapi/README.md +++ b/httpapi/README.md @@ -2,4 +2,7 @@ Database Playground 大部分的 API 均以 GraphQL 形式提供 (`/query`)，但部分為 BFF (Backend for Frontend) 設定的 Stateful Endpoints 則是以 HTTP API 進行設計，並以 `/api` 為開頭。 +> [!WARNING] +> 注意 HTTP API 不會帶入 AuthMiddleware。如果你的 API 需要鑒權，請手動帶入 `auth.Middleware`。 + - [認證](./auth)：相關方法均列於 `/api/auth` 路徑底下。 diff --git a/httpapi/auth/README.md b/httpapi/auth/README.md index 91157ab..02e02f1 100644 --- a/httpapi/auth/README.md +++ b/httpapi/auth/README.md @@ -6,14 +6,6 @@ Auth 端點提供適合供網頁應用程式使用的認證 API。 您可以使用 `POST /api/logout` 登出帳號。 -如果沒有 Auth Token 或者是 token 無效，會回傳如這種結構的 HTTP 401 錯誤： - -```json -{ - "error": "You should be logged in to logout.", -} -``` - 如果 Token 撤回失敗，則會回傳 HTTP 500 錯誤並帶上錯誤資訊： ```json @@ -25,6 +17,8 @@ Auth 端點提供適合供網頁應用程式使用的認證 API。 如果 Token 撤回成功，則回傳 HTTP 205 (Reset Content)，此時您可以重新整理登入狀態。 +如果沒有 Auth Token 或者是 token 無效，則依然回傳 HTTP 205。請引導使用者重新登入。 + ## Google 登入 如果您要觸發 Google 登入的流程，請前往 `GET /api/auth/google/login`。可以帶入 `redirect_uri` 參數來在登入完成後轉導到指定畫面。 diff --git a/httpapi/auth/logout.go b/httpapi/auth/logout.go index 3cfdb39..ec5a29f 100644 --- a/httpapi/auth/logout.go +++ b/httpapi/auth/logout.go @@ -1,6 +1,7 @@ package authservice import ( + "errors" "net/http" "github.com/database-playground/backend-v2/internal/auth" @@ -13,20 +14,16 @@ func (s *AuthService) Logout(c *gin.Context) { // check if there is a token in the cookie token, err := c.Cookie(auth.CookieAuthToken) if err != nil { - c.JSON(http.StatusUnauthorized, gin.H{ - "error": "You should be logged in to logout.", - }) - + c.Status(http.StatusResetContent) return } // revoke the token - if err := s.storage.Delete(c.Request.Context(), token); err != nil { + if err := s.storage.Delete(c.Request.Context(), token); err != nil && !errors.Is(err, auth.ErrNotFound) { c.JSON(http.StatusInternalServerError, gin.H{ "error": "Failed to revoke the token. Please try again later.", "detail": err.Error(), }) - return } diff --git a/httpapi/auth/logout_test.go b/httpapi/auth/logout_test.go index 13f1b1e..ea9902b 100644 --- a/httpapi/auth/logout_test.go +++ b/httpapi/auth/logout_test.go @@ -134,7 +134,7 @@ func TestAuthService_Logout(t *testing.T) { assert.True(t, authCookie.HttpOnly) }) - t.Run("logout without token returns unauthorized", func(t *testing.T) { + t.Run("logout without token should do nothing", func(t *testing.T) { authService, _ := setupTestAuthService(t) // Setup router @@ -149,16 +149,10 @@ func TestAuthService_Logout(t *testing.T) { router.ServeHTTP(rr, req) // Verify response - assert.Equal(t, http.StatusUnauthorized, rr.Code) - - // Verify error message - var response map[string]interface{} - err := json.NewDecoder(rr.Body).Decode(&response) - require.NoError(t, err) - assert.Equal(t, "You should be logged in to logout.", response["error"]) + assert.Equal(t, http.StatusResetContent, rr.Code) }) - t.Run("logout with invalid token returns internal server error", func(t *testing.T) { + t.Run("logout with invalid token should just revoke", func(t *testing.T) { authService, _ := setupTestAuthService(t) // Setup router @@ -177,14 +171,23 @@ func TestAuthService_Logout(t *testing.T) { router.ServeHTTP(rr, req) // Verify response - assert.Equal(t, http.StatusInternalServerError, rr.Code) + assert.Equal(t, http.StatusResetContent, rr.Code) - // Verify error message - var response map[string]interface{} - err := json.NewDecoder(rr.Body).Decode(&response) - require.NoError(t, err) - assert.Equal(t, "Failed to revoke the token. Please try again later.", response["error"]) - assert.Equal(t, "no such token", response["detail"]) + // Verify token was deleted from response + cookies := rr.Result().Cookies() + var authCookie *http.Cookie + for _, cookie := range cookies { + if cookie.Name == auth.CookieAuthToken { + authCookie = cookie + break + } + } + require.NotNil(t, authCookie) + assert.Equal(t, "", authCookie.Value) + assert.Equal(t, -1, authCookie.MaxAge) + assert.Equal(t, "/", authCookie.Path) + assert.True(t, authCookie.Secure) + assert.True(t, authCookie.HttpOnly) }) t.Run("logout with storage error returns internal server error", func(t *testing.T) { @@ -230,7 +233,7 @@ func TestAuthService_Logout(t *testing.T) { assert.Equal(t, storageErr.Error(), response["detail"]) }) - t.Run("logout with malformed cookie returns unauthorized", func(t *testing.T) { + t.Run("logout with malformed cookie should do nothing", func(t *testing.T) { authService, _ := setupTestAuthService(t) // Setup router @@ -246,13 +249,7 @@ func TestAuthService_Logout(t *testing.T) { router.ServeHTTP(rr, req) // Verify response - assert.Equal(t, http.StatusUnauthorized, rr.Code) - - // Verify error message - var response map[string]interface{} - err := json.NewDecoder(rr.Body).Decode(&response) - require.NoError(t, err) - assert.Equal(t, "You should be logged in to logout.", response["error"]) + assert.Equal(t, http.StatusResetContent, rr.Code) }) t.Run("logout sets SameSite cookie attribute", func(t *testing.T) { @@ -299,7 +296,7 @@ func TestAuthService_Logout(t *testing.T) { assert.Equal(t, http.SameSiteStrictMode, authCookie.SameSite) }) - t.Run("logout with empty token value returns internal server error", func(t *testing.T) { + t.Run("logout with empty token value should just revoke", func(t *testing.T) { authService, _ := setupTestAuthService(t) // Setup router @@ -318,14 +315,7 @@ func TestAuthService_Logout(t *testing.T) { router.ServeHTTP(rr, req) // Verify response - assert.Equal(t, http.StatusInternalServerError, rr.Code) - - // Verify error message - var response map[string]interface{} - err := json.NewDecoder(rr.Body).Decode(&response) - require.NoError(t, err) - assert.Equal(t, "Failed to revoke the token. Please try again later.", response["error"]) - assert.Equal(t, "no such token", response["detail"]) + assert.Equal(t, http.StatusResetContent, rr.Code) }) t.Run("logout with multiple cookies handles correctly", func(t *testing.T) { diff --git a/internal/auth/http.go b/internal/auth/http.go index 37a8b66..d533995 100644 --- a/internal/auth/http.go +++ b/internal/auth/http.go @@ -111,6 +111,10 @@ func ExtractToken(r *http.Request, storage Storage) (context.Context, error) { tokenInfo, err := storage.Get(r.Context(), token) if err != nil { + if errors.Is(err, ErrNotFound) { + return r.Context(), nil + } + return nil, err } diff --git a/internal/auth/http_test.go b/internal/auth/http_test.go index 1e21882..dc5a1f3 100644 --- a/internal/auth/http_test.go +++ b/internal/auth/http_test.go @@ -251,6 +251,64 @@ func TestExtractToken(t *testing.T) { t.Fatalf("expected user email %s, got %s", tokenInfo.UserEmail, user.UserEmail) } }) + + t.Run("revoked token should be treated like no token", func(t *testing.T) { + storage := &memoryTokenStorage{ + storage: map[string]auth.TokenInfo{}, + } + + // Create a valid token first + tokenInfo := auth.TokenInfo{ + UserID: 1, + UserEmail: "user@example.com", + Machine: "test", + Scopes: []string{"*"}, + } + + token, err := storage.Create(context.Background(), tokenInfo) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + // Verify token exists and works + r := http.Request{ + Header: http.Header{"Authorization": {"Bearer " + token}}, + } + ctx, err := auth.ExtractToken(&r, storage) + if err != nil { + t.Fatalf("expected no error for valid token, got %v", err) + } + + user, ok := auth.GetUser(ctx) + if !ok { + t.Fatalf("expected user for valid token, got none") + } + if user.UserID != tokenInfo.UserID { + t.Fatalf("expected user %d, got %d", tokenInfo.UserID, user.UserID) + } + + // Now revoke/delete the token + err = storage.Delete(context.Background(), token) + if err != nil { + t.Fatalf("expected no error deleting token, got %v", err) + } + + // Test that the revoked token is treated like no token + ctx, err = auth.ExtractToken(&r, storage) + if err != nil { + t.Fatalf("expected no error for revoked token, got %v", err) + } + + // Should not have user context (same as no token) + if ctx != r.Context() { + t.Fatalf("expected context to be the same as original, got different context") + } + + _, ok = auth.GetUser(ctx) + if ok { + t.Fatalf("expected no user for revoked token, got one") + } + }) } func TestMiddleware(t *testing.T) {