Skip to content

Commit 706d42e

Browse files
fix(api): accept API tokens in auth middleware (#162)
1 parent 95f65c8 commit 706d42e

5 files changed

Lines changed: 143 additions & 16 deletions

File tree

apps/api/internal/auth/service.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,17 @@ func (s *Service) UserFromSession(ctx context.Context, sessionKey string) (*mode
253253
return user, nil
254254
}
255255

256+
// ActiveUserByID returns the user if they exist and are active, otherwise
257+
// nil. Used by API-token authentication so a deactivated user's still-valid
258+
// token is rejected the same way a deactivated user's session is.
259+
func (s *Service) ActiveUserByID(ctx context.Context, id uuid.UUID) (*model.User, error) {
260+
user, err := s.userStore.GetByID(ctx, id)
261+
if err != nil || user == nil || !user.IsActive {
262+
return nil, nil
263+
}
264+
return user, nil
265+
}
266+
256267
func (s *Service) UpdateProfile(ctx context.Context, u *model.User) error {
257268
return s.userStore.Update(ctx, u)
258269
}

apps/api/internal/handler/auth_test.go

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,75 @@ func TestAuth_Tokens_RequiresAuth(t *testing.T) {
303303
require.Equal(t, http.StatusUnauthorized, rr.Code)
304304
}
305305

306+
// TestAuth_ApiToken_AuthenticatesRequests proves an API token created via
307+
// the tokens endpoint actually authenticates requests (the bug in #162 was
308+
// that tokens were issued but never accepted by RequireAuth).
309+
func TestAuth_ApiToken_AuthenticatesRequests(t *testing.T) {
310+
ts := testutil.NewTestServer(t)
311+
user := testutil.CreateUser(t, ts.DB)
312+
session := testutil.LoginAs(t, ts.DB, user)
313+
314+
rr := ts.POST("/api/users/me/tokens/", map[string]any{"label": "ci-token"}, session)
315+
require.Equal(t, http.StatusCreated, rr.Code, "body=%s", rr.Body.String())
316+
plainToken, _ := testutil.MustJSONMap(t, rr)["token"].(string)
317+
require.NotEmpty(t, plainToken)
318+
319+
rr2 := ts.DoWithHeaders(http.MethodGet, "/api/users/me/", nil, http.Header{
320+
"Authorization": []string{"Bearer " + plainToken},
321+
})
322+
require.Equal(t, http.StatusOK, rr2.Code, "body=%s", rr2.Body.String())
323+
assert.Equal(t, user.ID.String(), testutil.MustJSONMap(t, rr2)["id"])
324+
}
325+
326+
// TestAuth_ApiToken_RevokedRejected proves a revoked token no longer
327+
// authenticates.
328+
func TestAuth_ApiToken_RevokedRejected(t *testing.T) {
329+
ts := testutil.NewTestServer(t)
330+
user := testutil.CreateUser(t, ts.DB)
331+
session := testutil.LoginAs(t, ts.DB, user)
332+
333+
rr := ts.POST("/api/users/me/tokens/", map[string]any{"label": "ci-token"}, session)
334+
require.Equal(t, http.StatusCreated, rr.Code, "body=%s", rr.Body.String())
335+
createdBody := testutil.MustJSONMap(t, rr)
336+
plainToken, _ := createdBody["token"].(string)
337+
require.NotEmpty(t, plainToken)
338+
339+
listRR := ts.GET("/api/users/me/tokens/", session)
340+
tokens, _ := testutil.MustJSONMap(t, listRR)["tokens"].([]any)
341+
require.Len(t, tokens, 1)
342+
tokenID, _ := tokens[0].(map[string]any)["id"].(string)
343+
require.NotEmpty(t, tokenID)
344+
345+
revokeRR := ts.DELETE("/api/users/me/tokens/"+tokenID+"/", session)
346+
require.Equal(t, http.StatusNoContent, revokeRR.Code)
347+
348+
rr2 := ts.DoWithHeaders(http.MethodGet, "/api/users/me/", nil, http.Header{
349+
"Authorization": []string{"Bearer " + plainToken},
350+
})
351+
require.Equal(t, http.StatusUnauthorized, rr2.Code, "body=%s", rr2.Body.String())
352+
}
353+
354+
// TestAuth_ApiToken_DeactivatedUserRejected proves a deactivated user's
355+
// still-valid API token is rejected, mirroring the #155 protection for
356+
// cookie sessions.
357+
func TestAuth_ApiToken_DeactivatedUserRejected(t *testing.T) {
358+
ts := testutil.NewTestServer(t)
359+
user := testutil.CreateUser(t, ts.DB)
360+
session := testutil.LoginAs(t, ts.DB, user)
361+
362+
rr := ts.POST("/api/users/me/tokens/", map[string]any{"label": "ci-token"}, session)
363+
require.Equal(t, http.StatusCreated, rr.Code, "body=%s", rr.Body.String())
364+
plainToken, _ := testutil.MustJSONMap(t, rr)["token"].(string)
365+
require.NotEmpty(t, plainToken)
366+
367+
require.NoError(t, ts.DB.Exec("UPDATE users SET is_active = false WHERE id = ?", user.ID).Error)
368+
369+
rr2 := ts.DoWithHeaders(http.MethodGet, "/api/users/me/", nil, http.Header{
370+
"Authorization": []string{"Bearer " + plainToken},
371+
})
372+
require.Equal(t, http.StatusUnauthorized, rr2.Code, "body=%s", rr2.Body.String())
373+
}
374+
306375
func TestAuth_ForgotPassword_NoSMTPReturns503(t *testing.T) {
307376
ts := testutil.NewTestServer(t)
308377
testutil.CreateUser(t, ts.DB, testutil.WithUserEmail("forgot@test.local"))

apps/api/internal/middleware/auth.go

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77

88
"github.com/Devlaner/devlane/api/internal/auth"
99
"github.com/Devlaner/devlane/api/internal/model"
10+
"github.com/Devlaner/devlane/api/internal/store"
1011
"github.com/gin-gonic/gin"
1112
)
1213

@@ -27,20 +28,44 @@ func SessionKeyFromCookieOrBearer(c *gin.Context) string {
2728
return sessionKey
2829
}
2930

30-
// RequireAuth loads the user from session and returns 401 if not authenticated.
31-
func RequireAuth(authSvc *auth.Service, log *slog.Logger) gin.HandlerFunc {
31+
// RequireAuth loads the user from a session cookie or Authorization: Bearer
32+
// header, and returns 401 if not authenticated. Bearer values are tried
33+
// first as an API token (hashed + looked up in api_tokens); if that doesn't
34+
// match, they fall back to being treated as a raw session key — kept for
35+
// the cross-origin OAuth SPA fragment flow (see SessionKeyFromCookieOrBearer).
36+
func RequireAuth(authSvc *auth.Service, apiTokens *store.ApiTokenStore, log *slog.Logger) gin.HandlerFunc {
3237
return func(c *gin.Context) {
33-
sessionKey := SessionKeyFromCookieOrBearer(c)
34-
user, err := authSvc.UserFromSession(c.Request.Context(), sessionKey)
35-
if err != nil || user == nil {
36-
if log != nil {
37-
log.Debug("auth required", "error", err, "has_session_key", sessionKey != "")
38+
ctx := c.Request.Context()
39+
40+
if cookieKey, _ := c.Cookie(SessionCookieName); cookieKey != "" {
41+
if user, err := authSvc.UserFromSession(ctx, cookieKey); err == nil && user != nil {
42+
c.Set(UserContextKey, user)
43+
c.Next()
44+
return
45+
}
46+
} else if authHeader := c.GetHeader("Authorization"); len(authHeader) > 7 && strings.EqualFold(authHeader[:7], "bearer ") {
47+
bearer := strings.TrimSpace(authHeader[7:])
48+
if apiTokens != nil {
49+
if tok, err := apiTokens.GetActiveByHash(ctx, store.HashToken(bearer)); err == nil && tok != nil {
50+
if user, err := authSvc.ActiveUserByID(ctx, tok.UserID); err == nil && user != nil {
51+
_ = apiTokens.UpdateLastUsed(ctx, tok.ID)
52+
c.Set(UserContextKey, user)
53+
c.Next()
54+
return
55+
}
56+
}
57+
}
58+
if user, err := authSvc.UserFromSession(ctx, bearer); err == nil && user != nil {
59+
c.Set(UserContextKey, user)
60+
c.Next()
61+
return
3862
}
39-
c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": "Authentication required"})
40-
return
4163
}
42-
c.Set(UserContextKey, user)
43-
c.Next()
64+
65+
if log != nil {
66+
log.Debug("auth required", "has_session_key", SessionKeyFromCookieOrBearer(c) != "")
67+
}
68+
c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": "Authentication required"})
4469
}
4570
}
4671

apps/api/internal/router/router.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ func New(cfg Config) *gin.Engine {
236236

237237
// Protected API: require auth
238238
api := r.Group("/api")
239-
api.Use(middleware.RequireAuth(authSvc, cfg.Log))
239+
api.Use(middleware.RequireAuth(authSvc, apiTokenStore, cfg.Log))
240240
{
241241
api.GET("/users/me/", authHandler.Me)
242242
api.PATCH("/users/me/", authHandler.UpdateMe)
@@ -503,7 +503,7 @@ func New(cfg Config) *gin.Engine {
503503
authGroup.POST("/reset-password/", authHandler.ResetPassword)
504504
authGroup.POST("/magic-code/request/", middleware.RateLimit(cfg.Redis, "magiccode", 10, 15*time.Minute), authHandler.MagicCodeRequest)
505505
authGroup.POST("/magic-code/verify/", authHandler.MagicCodeVerify)
506-
authGroup.POST("/set-password/", middleware.RequireAuth(authSvc, cfg.Log), authHandler.SetPassword)
506+
authGroup.POST("/set-password/", middleware.RequireAuth(authSvc, apiTokenStore, cfg.Log), authHandler.SetPassword)
507507
}
508508

509509
// OAuth routes (no auth required); provider resolved from instance settings at request time.
@@ -522,16 +522,16 @@ func New(cfg Config) *gin.Engine {
522522
// GitHub App install flow (separate from OAuth user sign-in). Both
523523
// require the user to be signed in so we can attach the installation to
524524
// their workspace.
525-
r.GET("/auth/github-app/install", middleware.RequireAuth(authSvc, cfg.Log), integrationHandler.GitHubInstallStart)
526-
r.GET("/auth/github-app/callback", middleware.RequireAuth(authSvc, cfg.Log), integrationHandler.GitHubInstallCallback)
525+
r.GET("/auth/github-app/install", middleware.RequireAuth(authSvc, apiTokenStore, cfg.Log), integrationHandler.GitHubInstallStart)
526+
r.GET("/auth/github-app/callback", middleware.RequireAuth(authSvc, apiTokenStore, cfg.Log), integrationHandler.GitHubInstallCallback)
527527

528528
// GitHub webhook receiver — public; HMAC-signature-verified.
529529
r.POST("/webhooks/github", integrationHandler.GitHubWebhook)
530530
r.POST("/webhooks/github/", integrationHandler.GitHubWebhook)
531531

532532
// Legacy /api/v1
533533
v1 := r.Group("/api/v1")
534-
v1.Use(middleware.RequireAuth(authSvc, cfg.Log))
534+
v1.Use(middleware.RequireAuth(authSvc, apiTokenStore, cfg.Log))
535535
{
536536
v1.GET("/", func(c *gin.Context) {
537537
c.JSON(200, gin.H{"message": "Devlane API v1"})

apps/api/internal/store/api_token.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@ func hashToken(plain string) string {
3030
return hex.EncodeToString(h[:])
3131
}
3232

33+
// HashToken hashes a plain-text API token for lookup (auth middleware never
34+
// stores or compares the plain value).
35+
func HashToken(plain string) string { return hashToken(plain) }
36+
3337
// Create generates a new token, stores its hash, and returns the plain token (caller must show it once).
3438
func (s *ApiTokenStore) Create(ctx context.Context, userID uuid.UUID, label, description string, expiredAt *time.Time) (plainToken string, err error) {
3539
plain, err := generateToken()
@@ -75,3 +79,21 @@ func (s *ApiTokenStore) Delete(ctx context.Context, tokenID, userID uuid.UUID) e
7579
}
7680
return nil
7781
}
82+
83+
// GetActiveByHash returns the token matching hash if it is active and not
84+
// expired, for use by the auth middleware.
85+
func (s *ApiTokenStore) GetActiveByHash(ctx context.Context, hash string) (*model.ApiToken, error) {
86+
var t model.ApiToken
87+
err := s.db.WithContext(ctx).
88+
Where("token = ? AND is_active = true AND (expired_at IS NULL OR expired_at > ?)", hash, time.Now().UTC()).
89+
First(&t).Error
90+
if err != nil {
91+
return nil, err
92+
}
93+
return &t, nil
94+
}
95+
96+
// UpdateLastUsed stamps the token's last_used time.
97+
func (s *ApiTokenStore) UpdateLastUsed(ctx context.Context, id uuid.UUID) error {
98+
return s.db.WithContext(ctx).Model(&model.ApiToken{}).Where("id = ?", id).Update("last_used", time.Now().UTC()).Error
99+
}

0 commit comments

Comments
 (0)