Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions apps/api/internal/auth/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,17 @@ func (s *Service) UserFromSession(ctx context.Context, sessionKey string) (*mode
return user, nil
}

// ActiveUserByID returns the user if they exist and are active, otherwise
// nil. Used by API-token authentication so a deactivated user's still-valid
// token is rejected the same way a deactivated user's session is.
func (s *Service) ActiveUserByID(ctx context.Context, id uuid.UUID) (*model.User, error) {
user, err := s.userStore.GetByID(ctx, id)
if err != nil || user == nil || !user.IsActive {
return nil, nil
}
return user, nil
}

func (s *Service) UpdateProfile(ctx context.Context, u *model.User) error {
return s.userStore.Update(ctx, u)
}
Expand Down
26 changes: 25 additions & 1 deletion apps/api/internal/handler/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,16 @@ func (h *AuthHandler) SignIn(c *gin.Context) {
return
}
}
sessionKey, user, err := h.Auth.SignIn(c.Request.Context(), auth.SignInRequest{Email: req.Email, Password: req.Password})
ctx := c.Request.Context()
if h.Redis != nil {
emailNorm := strings.ToLower(strings.TrimSpace(req.Email))
ok, err := h.Redis.Allow(ctx, redis.PrefixRateLimit+"signinacct:"+emailNorm, 10, 15*time.Minute)
if err == nil && !ok {
c.JSON(http.StatusTooManyRequests, gin.H{"error": "Too many sign-in attempts for this account, please try again later"})
return
}
}
sessionKey, user, err := h.Auth.SignIn(ctx, auth.SignInRequest{Email: req.Email, Password: req.Password})
if err != nil {
if errors.Is(err, auth.ErrUserDeactivated) {
c.JSON(http.StatusForbidden, gin.H{"error": "Your account has been deactivated. Please contact the administrator.", "error_code": "USER_ACCOUNT_DEACTIVATED"})
Expand Down Expand Up @@ -884,6 +893,19 @@ func (h *AuthHandler) MagicCodeVerify(c *gin.Context) {
return
}

// Failed-verify attempts are tracked on a key independent of the code's
// own TTL/Attempts, so requesting a new code does not reset this lockout.
failCount, err := h.Redis.MagicCodeVerifyFailCount(ctx, body.Email)
if err != nil {
h.log().Error("magic code verify-fail count", "error", err)
c.JSON(http.StatusInternalServerError, gin.H{"error": "Verification failed"})
return
}
if failCount >= redis.MagicCodeVerifyFailMax {
c.JSON(http.StatusTooManyRequests, gin.H{"error": "Too many incorrect attempts. Please request a new code later."})
return
}

stored, err := h.Redis.GetMagicCodeLogin(ctx, body.Email)
if err != nil {
h.log().Error("magic code redis get", "error", err)
Expand All @@ -898,6 +920,7 @@ func (h *AuthHandler) MagicCodeVerify(c *gin.Context) {
tryMAC := auth.MagicCodeHMAC(h.MagicCodeSecret, body.Email, body.Code)
if subtle.ConstantTimeCompare([]byte(stored.CodeMAC), []byte(tryMAC)) != 1 {
_ = h.Redis.BumpMagicCodeLoginFailedAttempt(ctx, body.Email)
_ = h.Redis.BumpMagicCodeVerifyFail(ctx, body.Email)
c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid or expired code"})
return
}
Expand All @@ -909,6 +932,7 @@ func (h *AuthHandler) MagicCodeVerify(c *gin.Context) {
}

_ = h.Redis.DeleteMagicCodeLogin(ctx, body.Email)
_ = h.Redis.ResetMagicCodeVerifyFail(ctx, body.Email)

if stored.IsSignup {
sessionKey, user, err := h.Auth.SignUpMagic(ctx, body.Email, body.FirstName, body.LastName)
Expand Down
69 changes: 69 additions & 0 deletions apps/api/internal/handler/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,75 @@ func TestAuth_Tokens_RequiresAuth(t *testing.T) {
require.Equal(t, http.StatusUnauthorized, rr.Code)
}

// TestAuth_ApiToken_AuthenticatesRequests proves an API token created via
// the tokens endpoint actually authenticates requests (the bug in #162 was
// that tokens were issued but never accepted by RequireAuth).
func TestAuth_ApiToken_AuthenticatesRequests(t *testing.T) {
ts := testutil.NewTestServer(t)
user := testutil.CreateUser(t, ts.DB)
session := testutil.LoginAs(t, ts.DB, user)

rr := ts.POST("/api/users/me/tokens/", map[string]any{"label": "ci-token"}, session)
require.Equal(t, http.StatusCreated, rr.Code, "body=%s", rr.Body.String())
plainToken, _ := testutil.MustJSONMap(t, rr)["token"].(string)
require.NotEmpty(t, plainToken)

rr2 := ts.DoWithHeaders(http.MethodGet, "/api/users/me/", nil, http.Header{
"Authorization": []string{"Bearer " + plainToken},
})
require.Equal(t, http.StatusOK, rr2.Code, "body=%s", rr2.Body.String())
assert.Equal(t, user.ID.String(), testutil.MustJSONMap(t, rr2)["id"])
}

// TestAuth_ApiToken_RevokedRejected proves a revoked token no longer
// authenticates.
func TestAuth_ApiToken_RevokedRejected(t *testing.T) {
ts := testutil.NewTestServer(t)
user := testutil.CreateUser(t, ts.DB)
session := testutil.LoginAs(t, ts.DB, user)

rr := ts.POST("/api/users/me/tokens/", map[string]any{"label": "ci-token"}, session)
require.Equal(t, http.StatusCreated, rr.Code, "body=%s", rr.Body.String())
createdBody := testutil.MustJSONMap(t, rr)
plainToken, _ := createdBody["token"].(string)
require.NotEmpty(t, plainToken)

listRR := ts.GET("/api/users/me/tokens/", session)
tokens, _ := testutil.MustJSONMap(t, listRR)["tokens"].([]any)
require.Len(t, tokens, 1)
tokenID, _ := tokens[0].(map[string]any)["id"].(string)
require.NotEmpty(t, tokenID)

revokeRR := ts.DELETE("/api/users/me/tokens/"+tokenID+"/", session)
require.Equal(t, http.StatusNoContent, revokeRR.Code)

rr2 := ts.DoWithHeaders(http.MethodGet, "/api/users/me/", nil, http.Header{
"Authorization": []string{"Bearer " + plainToken},
})
require.Equal(t, http.StatusUnauthorized, rr2.Code, "body=%s", rr2.Body.String())
}

// TestAuth_ApiToken_DeactivatedUserRejected proves a deactivated user's
// still-valid API token is rejected, mirroring the #155 protection for
// cookie sessions.
func TestAuth_ApiToken_DeactivatedUserRejected(t *testing.T) {
ts := testutil.NewTestServer(t)
user := testutil.CreateUser(t, ts.DB)
session := testutil.LoginAs(t, ts.DB, user)

rr := ts.POST("/api/users/me/tokens/", map[string]any{"label": "ci-token"}, session)
require.Equal(t, http.StatusCreated, rr.Code, "body=%s", rr.Body.String())
plainToken, _ := testutil.MustJSONMap(t, rr)["token"].(string)
require.NotEmpty(t, plainToken)

require.NoError(t, ts.DB.Exec("UPDATE users SET is_active = false WHERE id = ?", user.ID).Error)

rr2 := ts.DoWithHeaders(http.MethodGet, "/api/users/me/", nil, http.Header{
"Authorization": []string{"Bearer " + plainToken},
})
require.Equal(t, http.StatusUnauthorized, rr2.Code, "body=%s", rr2.Body.String())
}

func TestAuth_ForgotPassword_NoSMTPReturns503(t *testing.T) {
ts := testutil.NewTestServer(t)
testutil.CreateUser(t, ts.DB, testutil.WithUserEmail("forgot@test.local"))
Expand Down
91 changes: 91 additions & 0 deletions apps/api/internal/handler/join_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
package handler_test

import (
"context"
"net/http"
"testing"

"github.com/Devlaner/devlane/api/internal/model"
"github.com/Devlaner/devlane/api/internal/testutil"
"github.com/stretchr/testify/require"
)

// TestWorkspace_JoinByToken_RejectsEmailMismatch proves a workspace invite
// token can't be redeemed by an account whose email doesn't match the
// invited email (a leaked/forwarded token must not grant membership).
func TestWorkspace_JoinByToken_RejectsEmailMismatch(t *testing.T) {
ts := testutil.NewTestServer(t)
owner := testutil.CreateUser(t, ts.DB)
w := testutil.CreateWorkspace(t, ts.DB, owner.ID)
inv := testutil.CreateWorkspaceInvite(t, ts.DB, w.ID, "invited@test.local", "tok-mismatch-111")

stranger := testutil.CreateUser(t, ts.DB) // email is stranger-N@test.local, not invited@test.local
session := testutil.LoginAs(t, ts.DB, stranger)

rr := ts.POST("/api/workspaces/join/", map[string]any{"token": inv.Token}, session)
require.Equal(t, http.StatusNotFound, rr.Code, "body=%s", rr.Body.String())
}

// TestWorkspace_JoinByToken_AcceptsEmailMatch proves the happy path still
// works when the joining account's email matches the invite.
func TestWorkspace_JoinByToken_AcceptsEmailMatch(t *testing.T) {
ts := testutil.NewTestServer(t)
owner := testutil.CreateUser(t, ts.DB)
w := testutil.CreateWorkspace(t, ts.DB, owner.ID)

invitee := testutil.CreateUser(t, ts.DB)
email := invitee.Email
require.NotNil(t, email)
inv := testutil.CreateWorkspaceInvite(t, ts.DB, w.ID, *email, "tok-match-111")
session := testutil.LoginAs(t, ts.DB, invitee)

rr := ts.POST("/api/workspaces/join/", map[string]any{"token": inv.Token}, session)
require.Equal(t, http.StatusOK, rr.Code, "body=%s", rr.Body.String())
}

func TestProject_JoinByToken_RejectsEmailMismatch(t *testing.T) {
ts := testutil.NewTestServer(t)
owner := testutil.CreateUser(t, ts.DB)
w := testutil.CreateWorkspace(t, ts.DB, owner.ID)
p := testutil.CreateProject(t, ts.DB, w.ID, owner.ID)

inv := &model.ProjectMemberInvite{
ProjectID: p.ID,
WorkspaceID: w.ID,
Email: "invited@test.local",
Token: "proj-tok-mismatch-111",
Role: testutil.RoleMember,
}
require.NoError(t, ts.DB.WithContext(context.Background()).Create(inv).Error)

stranger := testutil.CreateUser(t, ts.DB)
session := testutil.LoginAs(t, ts.DB, stranger)

rr := ts.POST("/api/workspaces/"+w.Slug+"/projects/join/", map[string]any{"token": inv.Token}, session)
require.Equal(t, http.StatusNotFound, rr.Code, "body=%s", rr.Body.String())
}

func TestProject_JoinByToken_AcceptsEmailMatch(t *testing.T) {
ts := testutil.NewTestServer(t)
owner := testutil.CreateUser(t, ts.DB)
w := testutil.CreateWorkspace(t, ts.DB, owner.ID)
p := testutil.CreateProject(t, ts.DB, w.ID, owner.ID)

invitee := testutil.CreateUser(t, ts.DB)
testutil.AddWorkspaceMember(t, ts.DB, w.ID, invitee.ID, testutil.RoleMember)
email := invitee.Email
require.NotNil(t, email)

inv := &model.ProjectMemberInvite{
ProjectID: p.ID,
WorkspaceID: w.ID,
Email: *email,
Token: "proj-tok-match-111",
Role: testutil.RoleMember,
}
require.NoError(t, ts.DB.WithContext(context.Background()).Create(inv).Error)

session := testutil.LoginAs(t, ts.DB, invitee)
rr := ts.POST("/api/workspaces/"+w.Slug+"/projects/join/", map[string]any{"token": inv.Token}, session)
require.Equal(t, http.StatusOK, rr.Code, "body=%s", rr.Body.String())
}
28 changes: 28 additions & 0 deletions apps/api/internal/handler/label_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package handler_test

import (
"context"
"net/http"
"testing"

"github.com/Devlaner/devlane/api/internal/model"
"github.com/Devlaner/devlane/api/internal/testutil"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -40,3 +42,29 @@ func TestLabel_CRUD(t *testing.T) {
rr4 := ts.DELETE(base+id+"/", w.Session)
require.Equal(t, http.StatusNoContent, rr4.Code)
}

// TestLabel_WorkspaceLevel_RejectsForeignWorkspace proves a workspace-level
// label (ProjectID == nil) from a foreign workspace can't be read, updated,
// or deleted just by supplying its UUID alongside any project in a workspace
// the caller does belong to.
func TestLabel_WorkspaceLevel_RejectsForeignWorkspace(t *testing.T) {
ts := testutil.NewTestServer(t)
w := testutil.SeedWorld(t, ts.DB)
base := "/api/workspaces/" + w.Workspace.Slug + "/projects/" + w.Project.ID.String() + "/issue-labels/"

otherOwner := testutil.CreateUser(t, ts.DB)
otherWs := testutil.CreateWorkspace(t, ts.DB, otherOwner.ID)
foreignLabel := &model.Label{
Name: "foreign workspace label",
Color: "#00ff00",
ProjectID: nil,
WorkspaceID: otherWs.ID,
}
require.NoError(t, ts.DB.WithContext(context.Background()).Create(foreignLabel).Error)

rr := ts.PATCH(base+foreignLabel.ID.String()+"/", map[string]any{"name": "hijacked"}, w.Session)
require.Equal(t, http.StatusNotFound, rr.Code, "body=%s", rr.Body.String())

rr2 := ts.DELETE(base+foreignLabel.ID.String()+"/", w.Session)
require.Equal(t, http.StatusNotFound, rr2.Code, "body=%s", rr2.Body.String())
}
10 changes: 10 additions & 0 deletions apps/api/internal/handler/upload.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ var allowedImageTypes = map[string]bool{
"image/webp": true,
}

// maxUploadSize caps generic uploads (avatars/covers/logos) to a sane size
// for profile-type images; the larger issue-attachment flow has its own cap.
const maxUploadSize = 5 << 20 // 5 MiB

// Upload accepts a multipart file and uploads it to MinIO.
// POST /api/upload
// Form: file (required). Returns { "url": "/api/files/uploads/..." }.
Expand All @@ -46,6 +50,10 @@ func (h *UploadHandler) Upload(c *gin.Context) {
c.JSON(http.StatusBadRequest, gin.H{"error": "No file provided", "detail": err.Error()})
return
}
if file.Size > maxUploadSize {
c.JSON(http.StatusBadRequest, gin.H{"error": "File too large. Maximum size is 5MB."})
return
}

f, err := file.Open()
if err != nil {
Expand Down Expand Up @@ -117,5 +125,7 @@ func (h *UploadHandler) ServeFile(c *gin.Context) {
}

c.Header("Content-Type", info.ContentType)
c.Header("X-Content-Type-Options", "nosniff")
c.Header("Content-Disposition", "inline")
c.DataFromReader(http.StatusOK, info.Size, info.ContentType, obj, nil)
}
47 changes: 36 additions & 11 deletions apps/api/internal/middleware/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/Devlaner/devlane/api/internal/auth"
"github.com/Devlaner/devlane/api/internal/model"
"github.com/Devlaner/devlane/api/internal/store"
"github.com/gin-gonic/gin"
)

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

// RequireAuth loads the user from session and returns 401 if not authenticated.
func RequireAuth(authSvc *auth.Service, log *slog.Logger) gin.HandlerFunc {
// RequireAuth loads the user from a session cookie or Authorization: Bearer
// header, and returns 401 if not authenticated. Bearer values are tried
// first as an API token (hashed + looked up in api_tokens); if that doesn't
// match, they fall back to being treated as a raw session key — kept for
// the cross-origin OAuth SPA fragment flow (see SessionKeyFromCookieOrBearer).
func RequireAuth(authSvc *auth.Service, apiTokens *store.ApiTokenStore, log *slog.Logger) gin.HandlerFunc {
return func(c *gin.Context) {
sessionKey := SessionKeyFromCookieOrBearer(c)
user, err := authSvc.UserFromSession(c.Request.Context(), sessionKey)
if err != nil || user == nil {
if log != nil {
log.Debug("auth required", "error", err, "has_session_key", sessionKey != "")
ctx := c.Request.Context()

if cookieKey, _ := c.Cookie(SessionCookieName); cookieKey != "" {
if user, err := authSvc.UserFromSession(ctx, cookieKey); err == nil && user != nil {
c.Set(UserContextKey, user)
c.Next()
return
}
} else if authHeader := c.GetHeader("Authorization"); len(authHeader) > 7 && strings.EqualFold(authHeader[:7], "bearer ") {
bearer := strings.TrimSpace(authHeader[7:])
if apiTokens != nil {
if tok, err := apiTokens.GetActiveByHash(ctx, store.HashToken(bearer)); err == nil && tok != nil {
if user, err := authSvc.ActiveUserByID(ctx, tok.UserID); err == nil && user != nil {
_ = apiTokens.UpdateLastUsed(ctx, tok.ID)
c.Set(UserContextKey, user)
c.Next()
return
}
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
if user, err := authSvc.UserFromSession(ctx, bearer); err == nil && user != nil {
c.Set(UserContextKey, user)
c.Next()
return
}
c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": "Authentication required"})
return
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
c.Set(UserContextKey, user)
c.Next()

if log != nil {
log.Debug("auth required", "has_session_key", SessionKeyFromCookieOrBearer(c) != "")
}
c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": "Authentication required"})
}
}

Expand Down
Loading
Loading