Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
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
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)
}
33 changes: 33 additions & 0 deletions apps/api/internal/middleware/ratelimit.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package middleware

import (
"net/http"
"time"

"github.com/Devlaner/devlane/api/internal/redis"
"github.com/gin-gonic/gin"
)

// RateLimit caps requests per client IP to limit uses per window, backed by
// Redis. Like other optional-infra integrations in this codebase, it fails
// open (allows the request) when rdb is nil or a Redis error occurs, rather
// than turning a Redis outage into an outage of the whole API.
func RateLimit(rdb *redis.Client, prefix string, limit int, window time.Duration) gin.HandlerFunc {
return func(c *gin.Context) {
if rdb == nil {
c.Next()
return
}
key := redis.PrefixRateLimit + prefix + ":" + c.ClientIP()
ok, err := rdb.Allow(c.Request.Context(), key, limit, window)
Comment thread
nazarli-shabnam marked this conversation as resolved.
Comment thread
nazarli-shabnam marked this conversation as resolved.
if err != nil {
c.Next()
return
}
if !ok {
c.AbortWithStatusJSON(http.StatusTooManyRequests, gin.H{"error": "Too many requests, please try again later"})
return
}
c.Next()
}
}
62 changes: 62 additions & 0 deletions apps/api/internal/redis/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ const (
PrefixMagicCodeLogin = "logincode_"
PrefixLock = "lock_"
PrefixCache = "cache_"
PrefixRateLimit = "ratelimit_"
// PrefixMagicCodeVerifyFail tracks failed magic-code verification attempts
// independently of the code's own TTL/Attempts field, so requesting a new
// code does not reset the lockout counter.
PrefixMagicCodeVerifyFail = "logincode_fail_"
)

// Default TTLs.
Expand All @@ -25,6 +30,10 @@ const (
// MagicCodeMaxAttempts before the stored code is invalidated.
MagicCodeMaxAttempts = 10
LockTTL = 300 * time.Second // 5 min
// MagicCodeVerifyFailWindow/Max bound repeated verify attempts against a
// single email regardless of how many new codes are requested meanwhile.
MagicCodeVerifyFailWindow = 15 * time.Minute
MagicCodeVerifyFailMax = 10
)

// Get gets a string value. Returns redis.Nil when key does not exist.
Expand Down Expand Up @@ -229,3 +238,56 @@ func (c *Client) GetRequestOrigin(ctx context.Context, entityID string) (string,
}
return s, err
}

// --- Rate limiting (fixed window) ---

// Allow increments the counter for key and reports whether it's still within
// limit for the current fixed window; the window starts (TTL is set) on the
// first increment. Used for both per-IP and per-account throttling.
func (c *Client) Allow(ctx context.Context, key string, limit int, window time.Duration) (bool, error) {
n, err := c.Client.Incr(ctx, key).Result()
if err != nil {
return false, err
}
if n == 1 {
if err := c.Client.Expire(ctx, key, window).Err(); err != nil {
return false, err
}
}
return n <= int64(limit), nil
}

// Count returns the current counter value for key without incrementing it
// (0 if unset). Used to peek a counter before deciding whether to act.
func (c *Client) Count(ctx context.Context, key string) (int64, error) {
n, err := c.Client.Get(ctx, key).Int64()
if err == redis.Nil {
return 0, nil
}
return n, err
}

// --- Magic-code verify-failure lockout (independent of the code's own TTL) ---

func magicCodeVerifyFailKey(email string) string {
return PrefixMagicCodeVerifyFail + strings.ToLower(strings.TrimSpace(email))
}

// MagicCodeVerifyFailCount peeks the current failed-verify count for email.
func (c *Client) MagicCodeVerifyFailCount(ctx context.Context, email string) (int64, error) {
return c.Count(ctx, magicCodeVerifyFailKey(email))
}

// BumpMagicCodeVerifyFail increments the failed-verify counter for email,
// independent of the per-code Attempts field, so requesting a new code does
// not reset how many times this email has failed verification recently.
func (c *Client) BumpMagicCodeVerifyFail(ctx context.Context, email string) error {
_, err := c.Allow(ctx, magicCodeVerifyFailKey(email), MagicCodeVerifyFailMax, MagicCodeVerifyFailWindow)
return err
}

// ResetMagicCodeVerifyFail clears the failed-verify counter after a
// successful verification.
func (c *Client) ResetMagicCodeVerifyFail(ctx context.Context, email string) error {
return c.Delete(ctx, magicCodeVerifyFailKey(email))
}
9 changes: 5 additions & 4 deletions apps/api/internal/router/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package router
import (
"context"
"log/slog"
"time"

"github.com/Devlaner/devlane/api/internal/auth"
gh "github.com/Devlaner/devlane/api/internal/github"
Expand Down Expand Up @@ -494,13 +495,13 @@ func New(cfg Config) *gin.Engine {
authGroup := r.Group("/auth")
{
authGroup.GET("/config/", authHandler.InstanceAuthConfig)
authGroup.POST("/email-check/", authHandler.EmailCheck)
authGroup.POST("/sign-in/", authHandler.SignIn)
authGroup.POST("/email-check/", middleware.RateLimit(cfg.Redis, "emailcheck", 30, 15*time.Minute), authHandler.EmailCheck)
authGroup.POST("/sign-in/", middleware.RateLimit(cfg.Redis, "signin", 20, 15*time.Minute), authHandler.SignIn)
authGroup.POST("/sign-up/", authHandler.SignUp)
authGroup.POST("/sign-out/", authHandler.SignOut)
authGroup.POST("/forgot-password/", authHandler.ForgotPassword)
authGroup.POST("/forgot-password/", middleware.RateLimit(cfg.Redis, "forgotpw", 10, 15*time.Minute), authHandler.ForgotPassword)
authGroup.POST("/reset-password/", authHandler.ResetPassword)
authGroup.POST("/magic-code/request/", authHandler.MagicCodeRequest)
authGroup.POST("/magic-code/request/", middleware.RateLimit(cfg.Redis, "magiccode", 10, 15*time.Minute), authHandler.MagicCodeRequest)
authGroup.POST("/magic-code/verify/", authHandler.MagicCodeVerify)
authGroup.POST("/set-password/", middleware.RequireAuth(authSvc, cfg.Log), authHandler.SetPassword)
}
Expand Down
Loading
Loading