From 1985b544e7d7eac10c53970693030dd7be4bdfce Mon Sep 17 00:00:00 2001 From: Martian <150589141+martian56@users.noreply.github.com> Date: Tue, 23 Jun 2026 09:21:02 +0400 Subject: [PATCH] Revert "Chore/security OSV triage safe upgrades" --- api/Dockerfile | 2 +- api/go.mod | 19 +- api/go.sum | 40 ++-- api/internal/auth/service.go | 29 +-- api/internal/config/config.go | 17 -- api/internal/handler/auth.go | 4 +- api/internal/handler/instance.go | 200 ++++------------- api/internal/handler/instance_test.go | 131 ----------- api/internal/handler/oauth_config.go | 2 +- api/internal/model/instance_admin.go | 35 --- api/internal/model/workspace.go | 9 - api/internal/oauth/github.go | 14 +- api/internal/oauth/google.go | 7 - api/internal/oauth/oauth.go | 1 - api/internal/router/router.go | 11 +- api/internal/service/attachment.go | 23 +- api/internal/service/cycle.go | 13 -- api/internal/service/github_sync.go | 9 +- api/internal/service/integration.go | 20 +- api/internal/service/issue_view.go | 7 +- api/internal/service/module.go | 12 - api/internal/service/project.go | 44 +--- api/internal/service/workspace.go | 63 +----- api/internal/store/instance_admin.go | 76 ------- api/internal/store/session.go | 17 -- api/internal/testutil/factory.go | 17 -- .../000006_instance_admins.down.sql | 3 - api/migrations/000006_instance_admins.up.sql | 13 -- ui/package-lock.json | 25 +-- ui/package.json | 3 +- ui/src/api/types.ts | 12 - .../components/layout/InstanceAdminLayout.tsx | 26 --- ui/src/lib/sanitize.ts | 36 --- ui/src/pages/EpicDetailPage.tsx | 3 +- ui/src/pages/IssueDetailPage.tsx | 5 +- ui/src/pages/PageDetailPage.tsx | 3 +- .../InstanceAdminAdminsPage.tsx | 211 ------------------ ui/src/pages/instance-admin/index.ts | 1 - ui/src/routes/index.tsx | 13 -- ui/src/services/index.ts | 2 +- ui/src/services/instanceService.ts | 23 -- 41 files changed, 126 insertions(+), 1075 deletions(-) delete mode 100644 api/internal/model/instance_admin.go delete mode 100644 api/internal/store/instance_admin.go delete mode 100644 api/migrations/000006_instance_admins.down.sql delete mode 100644 api/migrations/000006_instance_admins.up.sql delete mode 100644 ui/src/lib/sanitize.ts delete mode 100644 ui/src/pages/instance-admin/InstanceAdminAdminsPage.tsx diff --git a/api/Dockerfile b/api/Dockerfile index e70bdb5..58e9b19 100644 --- a/api/Dockerfile +++ b/api/Dockerfile @@ -1,4 +1,4 @@ -FROM golang:1.26.4-alpine AS builder +FROM golang:1.25-alpine AS builder WORKDIR /src diff --git a/api/go.mod b/api/go.mod index 47d7bc4..e9ce3a4 100644 --- a/api/go.mod +++ b/api/go.mod @@ -1,6 +1,6 @@ module github.com/Devlaner/devlane/api -go 1.26.4 +go 1.25.5 require ( github.com/gin-gonic/gin v1.11.0 @@ -14,8 +14,8 @@ require ( github.com/stretchr/testify v1.11.1 github.com/testcontainers/testcontainers-go v0.42.0 github.com/testcontainers/testcontainers-go/modules/postgres v0.42.0 - golang.org/x/crypto v0.52.0 - golang.org/x/net v0.54.0 + golang.org/x/crypto v0.50.0 + golang.org/x/net v0.53.0 gorm.io/driver/postgres v1.6.0 gorm.io/gorm v1.31.1 ) @@ -56,7 +56,7 @@ require ( github.com/goccy/go-yaml v1.18.0 // indirect github.com/jackc/pgpassfile v1.0.0 // indirect github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 // indirect - github.com/jackc/pgx/v5 v5.9.2 // indirect + github.com/jackc/pgx/v5 v5.6.0 // indirect github.com/jackc/puddle/v2 v2.2.2 // indirect github.com/jinzhu/inflection v1.0.0 // indirect github.com/jinzhu/now v1.1.5 // indirect @@ -88,8 +88,8 @@ require ( github.com/philhofer/fwd v1.2.0 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/power-devops/perfstat v0.0.0-20240221224432-82ca36839d55 // indirect - github.com/quic-go/qpack v0.6.0 // indirect - github.com/quic-go/quic-go v0.59.1 // indirect + github.com/quic-go/qpack v0.5.1 // indirect + github.com/quic-go/quic-go v0.54.0 // indirect github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect github.com/rs/xid v1.6.0 // indirect github.com/shirou/gopsutil/v4 v4.26.3 // indirect @@ -105,11 +105,14 @@ require ( go.opentelemetry.io/otel v1.41.0 // indirect go.opentelemetry.io/otel/metric v1.41.0 // indirect go.opentelemetry.io/otel/trace v1.41.0 // indirect + go.uber.org/mock v0.5.0 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect golang.org/x/arch v0.20.0 // indirect + golang.org/x/mod v0.34.0 // indirect golang.org/x/sync v0.20.0 // indirect - golang.org/x/sys v0.45.0 // indirect - golang.org/x/text v0.37.0 // indirect + golang.org/x/sys v0.43.0 // indirect + golang.org/x/text v0.36.0 // indirect + golang.org/x/tools v0.43.0 // indirect google.golang.org/protobuf v1.36.9 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect modernc.org/libc v1.22.5 // indirect diff --git a/api/go.sum b/api/go.sum index eb5aad6..6169729 100644 --- a/api/go.sum +++ b/api/go.sum @@ -101,8 +101,8 @@ github.com/jackc/pgpassfile v1.0.0 h1:/6Hmqy13Ss2zCq62VdNG8tM1wchn8zjSGOBJ6icpsI github.com/jackc/pgpassfile v1.0.0/go.mod h1:CEx0iS5ambNFdcRtxPj5JhEz+xB6uRky5eyVu/W2HEg= github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 h1:iCEnooe7UlwOQYpKFhBabPMi4aNAfoODPEFNiAnClxo= github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761/go.mod h1:5TJZWKEWniPve33vlWYSoGYefn3gLQRzjfDlhSJ9ZKM= -github.com/jackc/pgx/v5 v5.9.2 h1:3ZhOzMWnR4yJ+RW1XImIPsD1aNSz4T4fyP7zlQb56hw= -github.com/jackc/pgx/v5 v5.9.2/go.mod h1:mal1tBGAFfLHvZzaYh77YS/eC6IX9OWbRV1QIIM0Jn4= +github.com/jackc/pgx/v5 v5.6.0 h1:SWJzexBzPL5jb0GEsrPMLIsi/3jOo7RHlzTjcAeDrPY= +github.com/jackc/pgx/v5 v5.6.0/go.mod h1:DNZ/vlrUnhWCoFGxHAG8U2ljioxukquj7utPDgtQdTw= github.com/jackc/puddle/v2 v2.2.2 h1:PR8nw+E/1w0GLuRFSmiioY6UooMp6KJv0/61nB7icHo= github.com/jackc/puddle/v2 v2.2.2/go.mod h1:vriiEXHvEE654aYKXXjOvZM39qJ0q+azkZFrfEOc3H4= github.com/jinzhu/inflection v1.0.0 h1:K317FqzuhWc8YvSVlFMCCUb36O/S9MCKRDI7QkRKD/E= @@ -181,10 +181,10 @@ github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRI github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/power-devops/perfstat v0.0.0-20240221224432-82ca36839d55 h1:o4JXh1EVt9k/+g42oCprj/FisM4qX9L3sZB3upGN2ZU= github.com/power-devops/perfstat v0.0.0-20240221224432-82ca36839d55/go.mod h1:OmDBASR4679mdNQnz2pUhc2G8CO2JrUAVFDRBDP/hJE= -github.com/quic-go/qpack v0.6.0 h1:g7W+BMYynC1LbYLSqRt8PBg5Tgwxn214ZZR34VIOjz8= -github.com/quic-go/qpack v0.6.0/go.mod h1:lUpLKChi8njB4ty2bFLX2x4gzDqXwUpaO1DP9qMDZII= -github.com/quic-go/quic-go v0.59.1 h1:0Gmua0HW1Tv7ANR7hUYwRyD0MG5OJfgvYSZasGZzBic= -github.com/quic-go/quic-go v0.59.1/go.mod h1:upnsH4Ju1YkqpLXC305eW3yDZ4NfnNbmQRCMWS58IKU= +github.com/quic-go/qpack v0.5.1 h1:giqksBPnT/HDtZ6VhtFKgoLOWmlyo9Ei6u9PqzIMbhI= +github.com/quic-go/qpack v0.5.1/go.mod h1:+PC4XFrEskIVkcLzpEkbLqq1uCoxPhQuvK5rH1ZgaEg= +github.com/quic-go/quic-go v0.54.0 h1:6s1YB9QotYI6Ospeiguknbp2Znb/jZYjZLRXn9kMQBg= +github.com/quic-go/quic-go v0.54.0/go.mod h1:e68ZEaCdyviluZmy44P6Iey98v/Wfz6HCjQEm+l8zTY= github.com/rabbitmq/amqp091-go v1.10.0 h1:STpn5XsHlHGcecLmMFCtg7mqq0RnD+zFr4uzukfVhBw= github.com/rabbitmq/amqp091-go v1.10.0/go.mod h1:Hy4jKW5kQART1u+JkDTF9YYOQUHXqMuhrgxOEeS7G4o= github.com/redis/go-redis/v9 v9.17.3 h1:fN29NdNrE17KttK5Ndf20buqfDZwGNgoUr9qjl1DQx4= @@ -244,28 +244,32 @@ go.opentelemetry.io/otel/trace v1.41.0 h1:Vbk2co6bhj8L59ZJ6/xFTskY+tGAbOnCtQGVVa go.opentelemetry.io/otel/trace v1.41.0/go.mod h1:U1NU4ULCoxeDKc09yCWdWe+3QoyweJcISEVa1RBzOis= go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= -go.uber.org/mock v0.5.2 h1:LbtPTcP8A5k9WPXj54PPPbjcI4Y6lhyOZXn+VS7wNko= -go.uber.org/mock v0.5.2/go.mod h1:wLlUxC2vVTPTaE3UD51E0BGOAElKrILxhVSDYQLld5o= +go.uber.org/mock v0.5.0 h1:KAMbZvZPyBPWgD14IrIQ38QCyjwpvVVV6K/bHl1IwQU= +go.uber.org/mock v0.5.0/go.mod h1:ge71pBPLYDk7QIi1LupWxdAykm7KIEFchiOqd6z7qMM= go.yaml.in/yaml/v3 v3.0.4 h1:tfq32ie2Jv2UxXFdLJdh3jXuOzWiL1fo0bu/FbuKpbc= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= golang.org/x/arch v0.20.0 h1:dx1zTU0MAE98U+TQ8BLl7XsJbgze2WnNKF/8tGp/Q6c= golang.org/x/arch v0.20.0/go.mod h1:bdwinDaKcfZUGpH09BB7ZmOfhalA8lQdzl62l8gGWsk= -golang.org/x/crypto v0.52.0 h1:RMs7fP2rXdep0CftQlK8Uf+kibLm7qkCcradZWYz988= -golang.org/x/crypto v0.52.0/go.mod h1:1QgfPxDqh0T2M/elOJtp9RvuR95kVjir0e6/BvEmGbc= -golang.org/x/net v0.54.0 h1:2zJIZAxAHV/OHCDTCOHAYehQzLfSXuf/5SoL/Dv6w/w= -golang.org/x/net v0.54.0/go.mod h1:Sj4oj8jK6XmHpBZU/zWHw3BV3abl4Kvi+Ut7cQcY+cQ= +golang.org/x/crypto v0.50.0 h1:zO47/JPrL6vsNkINmLoo/PH1gcxpls50DNogFvB5ZGI= +golang.org/x/crypto v0.50.0/go.mod h1:3muZ7vA7PBCE6xgPX7nkzzjiUq87kRItoJQM1Yo8S+Q= +golang.org/x/mod v0.34.0 h1:xIHgNUUnW6sYkcM5Jleh05DvLOtwc6RitGHbDk4akRI= +golang.org/x/mod v0.34.0/go.mod h1:ykgH52iCZe79kzLLMhyCUzhMci+nQj+0XkbXpNYtVjY= +golang.org/x/net v0.53.0 h1:d+qAbo5L0orcWAr0a9JweQpjXF19LMXJE8Ey7hwOdUA= +golang.org/x/net v0.53.0/go.mod h1:JvMuJH7rrdiCfbeHoo3fCQU24Lf5JJwT9W3sJFulfgs= golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= golang.org/x/sys v0.0.0-20190916202348-b4ddaad3f8a3/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20201204225414-ed752295db88/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210616094352-59db8d763f22/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.45.0 h1:dO4czNzziLiiXplLQgBCEpCvXQ3dnkn0SdaZSYdQ+FY= -golang.org/x/sys v0.45.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= -golang.org/x/term v0.43.0 h1:S4RLU2sB31O/NCl+zFN9Aru9A/Cq2aqKpTZJ6B+DwT4= -golang.org/x/term v0.43.0/go.mod h1:lrhlHNdQJHO+1qVYiHfFKVuVioJIheAc3fBSMFYEIsk= -golang.org/x/text v0.37.0 h1:Cqjiwd9eSg8e0QAkyCaQTNHFIIzWtidPahFWR83rTrc= -golang.org/x/text v0.37.0/go.mod h1:a5sjxXGs9hsn/AJVwuElvCAo9v8QYLzvavO5z2PiM38= +golang.org/x/sys v0.43.0 h1:Rlag2XtaFTxp19wS8MXlJwTvoh8ArU6ezoyFsMyCTNI= +golang.org/x/sys v0.43.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/term v0.42.0 h1:UiKe+zDFmJobeJ5ggPwOshJIVt6/Ft0rcfrXZDLWAWY= +golang.org/x/term v0.42.0/go.mod h1:Dq/D+snpsbazcBG5+F9Q1n2rXV8Ma+71xEjTRufARgY= +golang.org/x/text v0.36.0 h1:JfKh3XmcRPqZPKevfXVpI1wXPTqbkE5f7JA92a55Yxg= +golang.org/x/text v0.36.0/go.mod h1:NIdBknypM8iqVmPiuco0Dh6P5Jcdk8lJL0CUebqK164= +golang.org/x/tools v0.43.0 h1:12BdW9CeB3Z+J/I/wj34VMl8X+fEXBxVR90JeMX5E7s= +golang.org/x/tools v0.43.0/go.mod h1:uHkMso649BX2cZK6+RpuIPXS3ho2hZo4FVwfoy1vIk0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/protobuf v1.36.9 h1:w2gp2mA27hUeUzj9Ex9FBjsBm40zfaDtEWow293U7Iw= google.golang.org/protobuf v1.36.9/go.mod h1:fuxRtAxBytpl4zzqUh6/eyUujkJdNiuEkXntxiD/uRU= diff --git a/api/internal/auth/service.go b/api/internal/auth/service.go index 830feff..b0eb37e 100644 --- a/api/internal/auth/service.go +++ b/api/internal/auth/service.go @@ -247,10 +247,7 @@ func (s *Service) UpdateProfile(ctx context.Context, u *model.User) error { return s.userStore.Update(ctx, u) } -// ChangePassword updates the user's password after verifying the current one. -// All of the user's other sessions are evicted (keeping keepSessionKey, the -// caller's current session) so a stolen session cannot survive the change. -func (s *Service) ChangePassword(ctx context.Context, userID uuid.UUID, currentPassword, newPassword, keepSessionKey string) error { +func (s *Service) ChangePassword(ctx context.Context, userID uuid.UUID, currentPassword, newPassword string) error { if err := ValidatePasswordStrength(newPassword); err != nil { return err } @@ -272,13 +269,7 @@ func (s *Service) ChangePassword(ctx context.Context, userID uuid.UUID, currentP return err } u.Password = string(hash) - if err := s.userStore.Update(ctx, u); err != nil { - return err - } - if s.sessionStore != nil { - _ = s.sessionStore.DeleteByUserIDExcept(ctx, userID, keepSessionKey) - } - return nil + return s.userStore.Update(ctx, u) } // EmailCheck determines whether an email is already registered. @@ -351,19 +342,13 @@ func (s *Service) ResetPassword(ctx context.Context, token, newPassword string) return err } _ = s.resetTokenStore.InvalidateForUser(ctx, rt.UserID) - // Evict every existing session for the user: a password reset is the canonical - // post-compromise recovery step and must not leave an attacker's session alive. - if s.sessionStore != nil { - _ = s.sessionStore.DeleteByUserID(ctx, rt.UserID) - } return nil } var ErrPasswordAlreadySet = errors.New("password is already set") // SetPassword lets a user who signed up via OAuth/magic set their first password. -// Other sessions are evicted (keeping keepSessionKey, the caller's session). -func (s *Service) SetPassword(ctx context.Context, userID uuid.UUID, newPassword, keepSessionKey string) error { +func (s *Service) SetPassword(ctx context.Context, userID uuid.UUID, newPassword string) error { if err := ValidatePasswordStrength(newPassword); err != nil { return err } @@ -383,13 +368,7 @@ func (s *Service) SetPassword(ctx context.Context, userID uuid.UUID, newPassword } u.Password = string(hash) u.IsPasswordAutoset = false - if err := s.userStore.Update(ctx, u); err != nil { - return err - } - if s.sessionStore != nil { - _ = s.sessionStore.DeleteByUserIDExcept(ctx, userID, keepSessionKey) - } - return nil + return s.userStore.Update(ctx, u) } // OAuthLogin finds or creates a user from OAuth provider data and creates a session. diff --git a/api/internal/config/config.go b/api/internal/config/config.go index a0adb8e..2fa2808 100644 --- a/api/internal/config/config.go +++ b/api/internal/config/config.go @@ -100,23 +100,6 @@ func Load() (*Config, error) { MagicCodeSecret: getEnv("MAGIC_CODE_SECRET", ""), } - // Fail closed in non-development environments: a missing MAGIC_CODE_SECRET - // falls back to a public compiled-in HMAC key, and a missing - // INSTANCE_ENCRYPTION_KEY causes instance secrets to be stored in plaintext. - // Both are unacceptable outside local dev. - if cfg.Env != "development" { - var missing []string - if cfg.MagicCodeSecret == "" { - missing = append(missing, "MAGIC_CODE_SECRET") - } - if os.Getenv("INSTANCE_ENCRYPTION_KEY") == "" { - missing = append(missing, "INSTANCE_ENCRYPTION_KEY") - } - if len(missing) > 0 { - return nil, fmt.Errorf("missing required production secrets %v (set ENV=development only for local dev)", missing) - } - } - return cfg, nil } diff --git a/api/internal/handler/auth.go b/api/internal/handler/auth.go index cbc294f..d372985 100644 --- a/api/internal/handler/auth.go +++ b/api/internal/handler/auth.go @@ -293,7 +293,7 @@ func (h *AuthHandler) ChangePassword(c *gin.Context) { c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid request", "detail": err.Error()}) return } - if err := h.Auth.ChangePassword(c.Request.Context(), user.ID, req.CurrentPassword, req.NewPassword, middleware.SessionKeyFromCookieOrBearer(c)); err != nil { + if err := h.Auth.ChangePassword(c.Request.Context(), user.ID, req.CurrentPassword, req.NewPassword); err != nil { if errors.Is(err, auth.ErrPasswordTooWeak) { c.JSON(http.StatusBadRequest, gin.H{"error": "Password must contain at least 8 characters, one uppercase, one lowercase, one digit, and one special character."}) return @@ -1127,7 +1127,7 @@ func (h *AuthHandler) SetPassword(c *gin.Context) { c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid request", "detail": err.Error()}) return } - if err := h.Auth.SetPassword(c.Request.Context(), user.ID, body.Password, middleware.SessionKeyFromCookieOrBearer(c)); err != nil { + if err := h.Auth.SetPassword(c.Request.Context(), user.ID, body.Password); err != nil { if errors.Is(err, auth.ErrPasswordTooWeak) { c.JSON(http.StatusBadRequest, gin.H{"error": "Password must contain at least 8 characters, one uppercase, one lowercase, one digit, and one special character."}) return diff --git a/api/internal/handler/instance.go b/api/internal/handler/instance.go index acc5aed..c3906a4 100644 --- a/api/internal/handler/instance.go +++ b/api/internal/handler/instance.go @@ -5,7 +5,6 @@ import ( "crypto/rand" "encoding/hex" "encoding/json" - "errors" "io" "net/http" "net/url" @@ -13,11 +12,9 @@ import ( "github.com/Devlaner/devlane/api/internal/auth" "github.com/Devlaner/devlane/api/internal/crypto" - "github.com/Devlaner/devlane/api/internal/middleware" "github.com/Devlaner/devlane/api/internal/model" "github.com/Devlaner/devlane/api/internal/store" "github.com/gin-gonic/gin" - "github.com/google/uuid" ) // Allowed instance setting section keys (must match migration seed). @@ -31,15 +28,11 @@ type InstanceHandler struct { Auth *auth.Service Users *store.UserStore Settings *store.InstanceSettingStore - Admins *store.InstanceAdminStore } -// InstanceSettingsHandler serves instance settings (GET/PATCH) and instance-admin -// management; all endpoints require the caller to be an instance admin. +// InstanceSettingsHandler serves instance settings (GET/PATCH); requires auth. type InstanceSettingsHandler struct { Settings *store.InstanceSettingStore - Admins *store.InstanceAdminStore - Users *store.UserStore // OnSectionUpdated, if set, is invoked after a successful update with the // section key. Used for hot-reload of integration clients (e.g. github_app) // so the new credentials take effect without an API restart. @@ -107,15 +100,6 @@ func (h *InstanceHandler) InstanceSetup(c *gin.Context) { return } - // Register the first user as an instance admin (mirrors Plane's setup flow). - if h.Admins != nil { - _ = h.Admins.Create(c.Request.Context(), &model.InstanceAdmin{ - UserID: user.ID, - Role: model.RoleOwner, - IsVerified: true, - }) - } - // Seed general instance settings: generated instance_id, admin email from setup, instance name from company name if h.Settings != nil { instanceID := generateInstanceID() @@ -144,121 +128,9 @@ func generateInstanceID() string { return hex.EncodeToString(b) } -// isInstanceAdmin reports whether the authenticated user is an instance admin, -// looked up in the instance_admins table by user id + role (mirrors Plane's -// InstanceAdminPermission). Fails closed on any error. -func (h *InstanceSettingsHandler) isInstanceAdmin(c *gin.Context) bool { - user := middleware.GetUser(c) - if user == nil || h.Admins == nil { - return false - } - ok, err := h.Admins.IsAdmin(c.Request.Context(), user.ID) - return err == nil && ok -} - -// requireInstanceAdmin writes a 403 and returns false if the caller is not an -// instance administrator. -func (h *InstanceSettingsHandler) requireInstanceAdmin(c *gin.Context) bool { - if !h.isInstanceAdmin(c) { - c.JSON(http.StatusForbidden, gin.H{"error": "Instance admin access required"}) - return false - } - return true -} - -// ListAdmins returns all instance admins. Admin-gated. -// GET /api/instance/admins/ -func (h *InstanceSettingsHandler) ListAdmins(c *gin.Context) { - if !h.requireInstanceAdmin(c) { - return - } - admins, err := h.Admins.List(c.Request.Context()) - if err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to load instance admins"}) - return - } - c.JSON(http.StatusOK, admins) -} - -type addAdminRequest struct { - Email string `json:"email" binding:"required,email"` - Role *int16 `json:"role"` -} - -// AddAdmin grants instance-admin to an existing user by email. Admin-gated. -// POST /api/instance/admins/ -func (h *InstanceSettingsHandler) AddAdmin(c *gin.Context) { - if !h.requireInstanceAdmin(c) { - return - } - if h.Users == nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": "Not configured"}) - return - } - var req addAdminRequest - if err := c.ShouldBindJSON(&req); err != nil { - c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid request", "detail": err.Error()}) - return - } - u, err := h.Users.GetByEmail(c.Request.Context(), strings.TrimSpace(strings.ToLower(req.Email))) - if err != nil || u == nil { - c.JSON(http.StatusNotFound, gin.H{"error": "No user with that email"}) - return - } - // Idempotent: if already an admin, return the existing row. - if existing, err := h.Admins.GetByUserID(c.Request.Context(), u.ID); err == nil && existing != nil { - c.JSON(http.StatusOK, existing) - return - } - // Default to Owner when omitted; otherwise the role must be an explicit, - // valid admin level — reject anything else rather than silently coercing it. - role := model.RoleOwner - if req.Role != nil { - if *req.Role != model.RoleAdmin && *req.Role != model.RoleOwner { - c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid role"}) - return - } - role = *req.Role - } - admin := &model.InstanceAdmin{UserID: u.ID, Role: role, IsVerified: true} - if err := h.Admins.Create(c.Request.Context(), admin); err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to add instance admin"}) - return - } - c.JSON(http.StatusCreated, admin) -} - -// RemoveAdmin revokes instance-admin by row id. Admin-gated; refuses to remove -// the last remaining admin so the instance can't be locked out. -// DELETE /api/instance/admins/:id/ -func (h *InstanceSettingsHandler) RemoveAdmin(c *gin.Context) { - if !h.requireInstanceAdmin(c) { - return - } - id, err := uuid.Parse(strings.TrimSpace(c.Param("id"))) - if err != nil { - c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid admin id"}) - return - } - if err := h.Admins.DeleteByPKIfNotLast(c.Request.Context(), id); err != nil { - if errors.Is(err, store.ErrLastInstanceAdmin) { - c.JSON(http.StatusBadRequest, gin.H{"error": "Cannot remove the last instance admin"}) - return - } - c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to remove instance admin"}) - return - } - c.Status(http.StatusNoContent) -} - -// GetSettings returns all instance settings sections with secrets decrypted for -// the admin UI (mirrors Plane). Admin access is required — the gate is the -// protection, not masking. +// GetSettings returns all instance settings sections; secrets are decrypted for admin UI. // GET /api/instance/settings/ func (h *InstanceSettingsHandler) GetSettings(c *gin.Context) { - if !h.requireInstanceAdmin(c) { - return - } all, err := h.Settings.GetAll(c.Request.Context()) if err != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to load settings"}) @@ -266,7 +138,7 @@ func (h *InstanceSettingsHandler) GetSettings(c *gin.Context) { } out := make(map[string]model.JSONMap) for k, row := range all { - out[k] = decryptSectionSecretsInternal(k, row.Value) + out[k] = decryptSectionSecrets(k, row.Value) } // Ensure all sections exist with defaults (migration seed may not have run if DB was created before seed) for _, key := range []string{"general", "email", "auth", "oauth", "ai", "image", "github_app"} { @@ -277,27 +149,35 @@ func (h *InstanceSettingsHandler) GetSettings(c *gin.Context) { c.JSON(http.StatusOK, out) } -// secretKeysBySection lists the encrypted secret fields stored in each settings section. -var secretKeysBySection = map[string][]string{ - "email": {"password"}, - "oauth": {"google_client_secret", "github_client_secret", "gitlab_client_secret"}, - "ai": {"api_key"}, - "image": {"unsplash_access_key"}, - "github_app": {"private_key", "client_secret", "webhook_secret"}, -} - -// decryptSectionSecretsInternal returns a copy of m with secret fields decrypted. -// Used both for the admin settings responses (admin-gated) and for server-side -// use (building OAuth providers, proxying Unsplash). -func decryptSectionSecretsInternal(sectionKey string, m model.JSONMap) model.JSONMap { +// decryptSectionSecrets returns a copy of m with secret keys decrypted for response. +func decryptSectionSecrets(sectionKey string, m model.JSONMap) model.JSONMap { if m == nil { return nil } - out := make(model.JSONMap, len(m)) + var secretKeys []string + switch sectionKey { + case "email": + secretKeys = []string{"password"} + case "oauth": + secretKeys = []string{"google_client_secret", "github_client_secret", "gitlab_client_secret"} + case "ai": + secretKeys = []string{"api_key"} + case "image": + secretKeys = []string{"unsplash_access_key"} + case "github_app": + // We never echo private_key / client_secret / webhook_secret back to the + // admin UI in plain text; only the *_set boolean flags are exposed. + // Returning the section unchanged is fine because the response builder + // strips these via stripSecretValues below. + return stripSecretValues(m, "private_key", "client_secret", "webhook_secret") + default: + return m + } + out := make(model.JSONMap) for k, v := range m { out[k] = v } - for _, sk := range secretKeysBySection[sectionKey] { + for _, sk := range secretKeys { if v, ok := out[sk].(string); ok { out[sk] = crypto.DecryptOrPlain(v) } @@ -305,6 +185,25 @@ func decryptSectionSecretsInternal(sectionKey string, m model.JSONMap) model.JSO return out } +// stripSecretValues returns a copy of m with the named keys replaced by an +// empty string. Used for sections (like github_app) where a secret is stored +// encrypted and exposed to the admin UI only through a *_set boolean. +func stripSecretValues(m model.JSONMap, keys ...string) model.JSONMap { + out := make(model.JSONMap, len(m)) + stripped := make(map[string]bool, len(keys)) + for _, k := range keys { + stripped[k] = true + } + for k, v := range m { + if stripped[k] { + out[k] = "" + continue + } + out[k] = v + } + return out +} + func defaultSettingValue(key string) model.JSONMap { switch key { case "general": @@ -341,9 +240,6 @@ type UpdateSettingRequest struct { // UpdateSetting updates one instance settings section by key. // PATCH /api/instance/settings/:key func (h *InstanceSettingsHandler) UpdateSetting(c *gin.Context) { - if !h.requireInstanceAdmin(c) { - return - } key := strings.TrimSpace(strings.ToLower(c.Param("key"))) if key == "" || !allowedSettingKeys[key] { c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid settings key"}) @@ -527,8 +423,8 @@ func (h *InstanceSettingsHandler) UpdateSetting(c *gin.Context) { if h.OnSectionUpdated != nil { h.OnSectionUpdated(c.Request.Context(), key) } - // Return decrypted secrets so the admin UI reflects the saved value (Plane parity). - responseValue := decryptSectionSecretsInternal(key, value) + // Return decrypted secrets so client sees the value they just set + responseValue := decryptSectionSecrets(key, value) c.JSON(http.StatusOK, gin.H{"key": key, "value": responseValue}) } @@ -566,7 +462,7 @@ func (h *InstanceSettingsHandler) UnsplashSearch(c *gin.Context) { c.JSON(http.StatusBadRequest, gin.H{"error": "Unsplash is not configured"}) return } - decrypted := decryptSectionSecretsInternal("image", row.Value) + decrypted := decryptSectionSecrets("image", row.Value) keyVal, _ := decrypted["unsplash_access_key"].(string) keyVal = strings.TrimSpace(keyVal) if keyVal == "" { diff --git a/api/internal/handler/instance_test.go b/api/internal/handler/instance_test.go index e1bec2e..9c80f61 100644 --- a/api/internal/handler/instance_test.go +++ b/api/internal/handler/instance_test.go @@ -4,7 +4,6 @@ import ( "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" @@ -87,7 +86,6 @@ func TestInstance_Settings_RequiresAuth(t *testing.T) { func TestInstance_Settings_GetWithAuth(t *testing.T) { ts := testutil.NewTestServer(t) user := testutil.CreateUser(t, ts.DB) - testutil.SeedInstanceAdmin(t, ts.DB, user) session := testutil.LoginAs(t, ts.DB, user) rr := ts.GET("/api/instance/settings/", session) @@ -100,137 +98,9 @@ func TestInstance_Settings_GetWithAuth(t *testing.T) { } } -func TestInstance_Settings_NonAdminForbidden(t *testing.T) { - ts := testutil.NewTestServer(t) - admin := testutil.CreateUser(t, ts.DB) - testutil.SeedInstanceAdmin(t, ts.DB, admin) - // A different, non-admin authenticated user must not read instance settings. - other := testutil.CreateUser(t, ts.DB) - session := testutil.LoginAs(t, ts.DB, other) - - rr := ts.GET("/api/instance/settings/", session) - require.Equal(t, http.StatusForbidden, rr.Code, "body=%s", rr.Body.String()) - - rr2 := ts.PATCH("/api/instance/settings/email", map[string]any{ - "value": map[string]any{"host": "attacker.example.com"}, - }, session) - require.Equal(t, http.StatusForbidden, rr2.Code, "body=%s", rr2.Body.String()) -} - -func TestInstance_Settings_SecretsReturnedToAdmin(t *testing.T) { - ts := testutil.NewTestServer(t) - user := testutil.CreateUser(t, ts.DB) - testutil.SeedInstanceAdmin(t, ts.DB, user) - session := testutil.LoginAs(t, ts.DB, user) - - // Store an SMTP password. - rr := ts.PATCH("/api/instance/settings/email", map[string]any{ - "value": map[string]any{"host": "smtp.example.com", "password": "super-secret"}, - }, session) - require.Equal(t, http.StatusOK, rr.Code, "body=%s", rr.Body.String()) - - // Matching Plane: the admin GET returns the decrypted secret value. - rr2 := ts.GET("/api/instance/settings/", session) - require.Equal(t, http.StatusOK, rr2.Code) - body := testutil.MustJSONMap(t, rr2) - email, _ := body["email"].(map[string]any) - require.NotNil(t, email) - assert.Equal(t, "super-secret", email["password"]) -} - -func TestInstance_Admins_ListAddRemove(t *testing.T) { - ts := testutil.NewTestServer(t) - admin := testutil.CreateUser(t, ts.DB) - testutil.SeedInstanceAdmin(t, ts.DB, admin) - session := testutil.LoginAs(t, ts.DB, admin) - - // Initially exactly one admin. - rr := ts.GET("/api/instance/admins/", session) - require.Equal(t, http.StatusOK, rr.Code, "body=%s", rr.Body.String()) - list := testutil.DecodeJSON[[]map[string]any](t, rr) - require.Len(t, list, 1) - - // Add a second user as admin by email. - other := testutil.CreateUser(t, ts.DB) - rrAdd := ts.POST("/api/instance/admins/", map[string]any{"email": *other.Email}, session) - require.Equal(t, http.StatusCreated, rrAdd.Code, "body=%s", rrAdd.Body.String()) - - rr2 := ts.GET("/api/instance/admins/", session) - require.Len(t, testutil.DecodeJSON[[]map[string]any](t, rr2), 2) - - // The newly added admin can now reach the gated settings. - otherSession := testutil.LoginAs(t, ts.DB, other) - require.Equal(t, http.StatusOK, ts.GET("/api/instance/settings/", otherSession).Code) - - // Remove the second admin. - added := testutil.DecodeJSON[map[string]any](t, rrAdd) - id, _ := added["id"].(string) - require.NotEmpty(t, id) - rrDel := ts.DELETE("/api/instance/admins/"+id+"/", session) - require.Equal(t, http.StatusNoContent, rrDel.Code, "body=%s", rrDel.Body.String()) - - // They lose access afterwards. - require.Equal(t, http.StatusForbidden, ts.GET("/api/instance/settings/", otherSession).Code) -} - -func TestInstance_Admins_AddRejectsInvalidRole(t *testing.T) { - ts := testutil.NewTestServer(t) - admin := testutil.CreateUser(t, ts.DB) - testutil.SeedInstanceAdmin(t, ts.DB, admin) - session := testutil.LoginAs(t, ts.DB, admin) - other := testutil.CreateUser(t, ts.DB) - - // A non-admin role value (5 = guest) must be rejected, not coerced to Owner. - rr := ts.POST("/api/instance/admins/", map[string]any{"email": *other.Email, "role": 5}, session) - require.Equal(t, http.StatusBadRequest, rr.Code, "body=%s", rr.Body.String()) -} - -func TestInstance_Admins_NonAdminForbidden(t *testing.T) { - ts := testutil.NewTestServer(t) - admin := testutil.CreateUser(t, ts.DB) - testutil.SeedInstanceAdmin(t, ts.DB, admin) - other := testutil.CreateUser(t, ts.DB) - session := testutil.LoginAs(t, ts.DB, other) - - require.Equal(t, http.StatusForbidden, ts.GET("/api/instance/admins/", session).Code) - require.Equal(t, http.StatusForbidden, ts.POST("/api/instance/admins/", map[string]any{"email": *admin.Email}, session).Code) -} - -func TestInstance_Admins_CannotRemoveLast(t *testing.T) { - ts := testutil.NewTestServer(t) - admin := testutil.CreateUser(t, ts.DB) - testutil.SeedInstanceAdmin(t, ts.DB, admin) - session := testutil.LoginAs(t, ts.DB, admin) - - row := testutil.DecodeJSON[[]map[string]any](t, ts.GET("/api/instance/admins/", session)) - require.Len(t, row, 1) - id, _ := row[0]["id"].(string) - require.NotEmpty(t, id) - - rr := ts.DELETE("/api/instance/admins/"+id+"/", session) - require.Equal(t, http.StatusBadRequest, rr.Code, "body=%s", rr.Body.String()) -} - -func TestInstance_Setup_SeedsInstanceAdmin(t *testing.T) { - ts := testutil.NewTestServer(t) - - rr := ts.POST("/api/instance/setup/", map[string]any{ - "first_name": "Ada", "last_name": "Lovelace", - "email": "ada@test.local", "password": "S3cur3!Pass", - }, "") - require.Equal(t, http.StatusCreated, rr.Code, "body=%s", rr.Body.String()) - - // The first user is now an instance admin and can reach the gated settings. - var u model.User - require.NoError(t, ts.DB.Where("email = ?", "ada@test.local").First(&u).Error) - session := testutil.LoginAs(t, ts.DB, &u) - require.Equal(t, http.StatusOK, ts.GET("/api/instance/settings/", session).Code) -} - func TestInstance_Settings_UpdateGeneral(t *testing.T) { ts := testutil.NewTestServer(t) user := testutil.CreateUser(t, ts.DB) - testutil.SeedInstanceAdmin(t, ts.DB, user) session := testutil.LoginAs(t, ts.DB, user) rr := ts.PATCH("/api/instance/settings/general", map[string]any{ @@ -252,7 +122,6 @@ func TestInstance_Settings_UpdateGeneral(t *testing.T) { func TestInstance_Settings_UpdateInvalidKey(t *testing.T) { ts := testutil.NewTestServer(t) user := testutil.CreateUser(t, ts.DB) - testutil.SeedInstanceAdmin(t, ts.DB, user) session := testutil.LoginAs(t, ts.DB, user) rr := ts.PATCH("/api/instance/settings/not-a-real-section", map[string]any{ diff --git a/api/internal/handler/oauth_config.go b/api/internal/handler/oauth_config.go index b4a0db6..4500c6e 100644 --- a/api/internal/handler/oauth_config.go +++ b/api/internal/handler/oauth_config.go @@ -17,7 +17,7 @@ func loadOAuthSettingsMap(ctx context.Context, st *store.InstanceSettingStore) m if err != nil || row == nil || row.Value == nil { return nil } - return decryptSectionSecretsInternal("oauth", row.Value) + return decryptSectionSecrets("oauth", row.Value) } func jsonString(m model.JSONMap, key string) string { diff --git a/api/internal/model/instance_admin.go b/api/internal/model/instance_admin.go deleted file mode 100644 index e878ed8..0000000 --- a/api/internal/model/instance_admin.go +++ /dev/null @@ -1,35 +0,0 @@ -package model - -import ( - "time" - - "github.com/google/uuid" - "gorm.io/gorm" -) - -// InstanceAdmin matches the instance_admins table — the set of users authorized -// to manage instance-level settings (mirrors Plane's InstanceAdmin). Devlane is -// single-instance, so the row keys on user_id only (no instance FK). Role uses -// the shared Role* levels; the gate allows role >= RoleAdmin. -type InstanceAdmin struct { - ID uuid.UUID `gorm:"type:uuid;primaryKey;default:gen_random_uuid()" json:"id"` - UserID uuid.UUID `gorm:"type:uuid;not null" json:"user_id"` - Role int16 `gorm:"not null;default:20" json:"role"` - IsVerified bool `gorm:"column:is_verified;default:false" json:"is_verified"` - CreatedAt time.Time `json:"created_at"` - UpdatedAt time.Time `json:"updated_at"` - DeletedAt gorm.DeletedAt `gorm:"index" json:"-"` - - // Read-only join fields for list responses (populated via SELECT, not stored). - UserEmail *string `gorm:"->" json:"user_email,omitempty"` - UserDisplayName string `gorm:"->" json:"user_display_name,omitempty"` -} - -func (InstanceAdmin) TableName() string { return "instance_admins" } - -func (a *InstanceAdmin) BeforeCreate(tx *gorm.DB) error { - if a.ID == uuid.Nil { - a.ID = uuid.New() - } - return nil -} diff --git a/api/internal/model/workspace.go b/api/internal/model/workspace.go index c8c07d0..3654afd 100644 --- a/api/internal/model/workspace.go +++ b/api/internal/model/workspace.go @@ -26,15 +26,6 @@ type Workspace struct { func (Workspace) TableName() string { return "workspaces" } -// Workspace/project member role levels (int16, ascending privilege). -// These mirror the values used throughout the API and tests. -const ( - RoleGuest int16 = 5 - RoleMember int16 = 10 - RoleAdmin int16 = 15 - RoleOwner int16 = 20 -) - func (w *Workspace) BeforeCreate(tx *gorm.DB) error { if w.ID == uuid.Nil { w.ID = uuid.New() diff --git a/api/internal/oauth/github.go b/api/internal/oauth/github.go index 1fb0978..45c4f55 100644 --- a/api/internal/oauth/github.go +++ b/api/internal/oauth/github.go @@ -60,12 +60,9 @@ func (g *GitHubProvider) GetUserInfo(ctx context.Context, token *TokenData) (*Us if err != nil { return nil, err } - // Resolve a VERIFIED email from /user/emails. The top-level profile email is - // not necessarily verified, so we never trust it directly — otherwise an - // account with an unverified address matching a victim could be linked. - email, err := g.fetchPrimaryEmail(ctx, token.AccessToken) - if err != nil || email == "" { - return nil, ErrEmailNotVerified + email := strVal(resp, "email") + if email == "" { + email, _ = g.fetchPrimaryEmail(ctx, token.AccessToken) } return &UserInfo{ Email: email, @@ -101,11 +98,10 @@ func (g *GitHubProvider) fetchPrimaryEmail(ctx context.Context, accessToken stri return e.Email, nil } } - // Fall back to any verified email, but never to an unverified one. for _, e := range emails { - if e.Verified { + if e.Primary { return e.Email, nil } } - return "", fmt.Errorf("no verified email found") + return "", fmt.Errorf("no primary email found") } diff --git a/api/internal/oauth/google.go b/api/internal/oauth/google.go index 355c3d7..0f74f47 100644 --- a/api/internal/oauth/google.go +++ b/api/internal/oauth/google.go @@ -61,13 +61,6 @@ func (g *GoogleProvider) GetUserInfo(ctx context.Context, token *TokenData) (*Us if err != nil { return nil, err } - // Refuse an unverified email: a provider account asserting (but not having - // verified) the victim's address must not be linked to their account. - if v, ok := resp["verified_email"]; ok { - if b, ok := v.(bool); ok && !b { - return nil, ErrEmailNotVerified - } - } return &UserInfo{ Email: strVal(resp, "email"), FirstName: strVal(resp, "given_name"), diff --git a/api/internal/oauth/oauth.go b/api/internal/oauth/oauth.go index 3a75295..73529a0 100644 --- a/api/internal/oauth/oauth.go +++ b/api/internal/oauth/oauth.go @@ -18,7 +18,6 @@ var ( ErrCodeMissing = errors.New("oauth code missing") ErrTokenExchange = errors.New("oauth token exchange failed") ErrUserInfo = errors.New("oauth user info fetch failed") - ErrEmailNotVerified = errors.New("oauth provider email is not verified") ) // oauthHTTPClient bounds OAuth HTTP latency; requests also respect ctx cancellation. diff --git a/api/internal/router/router.go b/api/internal/router/router.go index 8578635..5c54cc7 100644 --- a/api/internal/router/router.go +++ b/api/internal/router/router.go @@ -88,7 +88,6 @@ func New(cfg Config) *gin.Engine { // Password reset tokens passwordResetTokenStore := store.NewPasswordResetTokenStore(cfg.DB) accountStore := store.NewAccountStore(cfg.DB) - instanceAdminStore := store.NewInstanceAdminStore(cfg.DB) // Auth authSvc := auth.NewService(userStore, sessionStore, passwordResetTokenStore) @@ -114,7 +113,7 @@ func New(cfg Config) *gin.Engine { Log: cfg.Log, } // Instance setup (no auth) — first-run flow; seeds general settings (instance_id, admin_email, instance_name) - instanceHandler := &handler.InstanceHandler{Auth: authSvc, Users: userStore, Settings: instanceSettingStore, Admins: instanceAdminStore} + instanceHandler := &handler.InstanceHandler{Auth: authSvc, Users: userStore, Settings: instanceSettingStore} r.GET("/api/instance/setup-status/", instanceHandler.SetupStatus) r.POST("/api/instance/setup/", instanceHandler.InstanceSetup) @@ -122,7 +121,7 @@ func New(cfg Config) *gin.Engine { r.GET("/api/invitations/by-token/", invitationHandler.GetInviteByToken) r.POST("/api/invitations/decline/", invitationHandler.DeclineInviteByToken) - instanceSettingsHandler := &handler.InstanceSettingsHandler{Settings: instanceSettingStore, Admins: instanceAdminStore, Users: userStore} + instanceSettingsHandler := &handler.InstanceSettingsHandler{Settings: instanceSettingStore} // Services workspaceSvc := service.NewWorkspaceService(workspaceStore, workspaceInviteStore, userStore) @@ -134,9 +133,7 @@ func New(cfg Config) *gin.Engine { issueSvc.SetActivityStore(issueActivityStore) attachmentSvc := service.NewAttachmentService(issueStore, projectStore, workspaceStore, cfg.Minio) cycleSvc := service.NewCycleService(cycleStore, projectStore, workspaceStore) - cycleSvc.SetIssueStore(issueStore) moduleSvc := service.NewModuleService(moduleStore, projectStore, workspaceStore) - moduleSvc.SetIssueStore(issueStore) issueViewSvc := service.NewIssueViewService(issueViewStore, projectStore, workspaceStore, userFavoriteStore) pageSvc := service.NewPageService(pageStore, projectStore, workspaceStore) pageSvc.SetFavoriteStore(userFavoriteStore) @@ -242,10 +239,6 @@ func New(cfg Config) *gin.Engine { api.GET("/instance/settings/", instanceSettingsHandler.GetSettings) api.PATCH("/instance/settings/:key", instanceSettingsHandler.UpdateSetting) api.GET("/instance/unsplash/search", instanceSettingsHandler.UnsplashSearch) - // Instance-admin management (admin-gated inside the handler). - api.GET("/instance/admins/", instanceSettingsHandler.ListAdmins) - api.POST("/instance/admins/", instanceSettingsHandler.AddAdmin) - api.DELETE("/instance/admins/:id/", instanceSettingsHandler.RemoveAdmin) uploadHandler := &handler.UploadHandler{Minio: cfg.Minio} api.POST("/upload", uploadHandler.Upload) diff --git a/api/internal/service/attachment.go b/api/internal/service/attachment.go index 91dbb93..e4d5115 100644 --- a/api/internal/service/attachment.go +++ b/api/internal/service/attachment.go @@ -69,27 +69,12 @@ func (s *AttachmentService) ensureProjectAccess(ctx context.Context, workspaceSl return nil } -// ensureIssueAccess validates workspace membership + project-in-workspace AND -// that the target issue actually belongs to the URL project. Without the issue -// check, any workspace member could read/modify attachments on an issue from a -// different project/workspace by supplying that issue's id. -func (s *AttachmentService) ensureIssueAccess(ctx context.Context, workspaceSlug string, projectID, issueID, userID uuid.UUID) error { - if err := s.ensureProjectAccess(ctx, workspaceSlug, projectID, userID); err != nil { - return err - } - issue, err := s.is.GetByID(ctx, issueID) - if err != nil || issue == nil || issue.ProjectID != projectID { - return ErrAttachmentNotFound - } - return nil -} - // InitiateUpload creates the DB records and returns the presigned upload URL + attachment shape. func (s *AttachmentService) InitiateUpload(ctx context.Context, workspaceSlug string, projectID, issueID uuid.UUID, userID uuid.UUID, name string, size float64, contentType string) (*PresignedUploadResponse, error) { if s.minio == nil { return nil, errors.New("file storage is not configured") } - if err := s.ensureIssueAccess(ctx, workspaceSlug, projectID, issueID, userID); err != nil { + if err := s.ensureProjectAccess(ctx, workspaceSlug, projectID, userID); err != nil { return nil, err } wrk, err := s.ws.GetBySlug(ctx, workspaceSlug) @@ -154,7 +139,7 @@ func (s *AttachmentService) InitiateUpload(ctx context.Context, workspaceSlug st // ConfirmUpload marks the file asset as uploaded (PATCH step). func (s *AttachmentService) ConfirmUpload(ctx context.Context, workspaceSlug string, projectID, issueID, assetID uuid.UUID, userID uuid.UUID) error { - if err := s.ensureIssueAccess(ctx, workspaceSlug, projectID, issueID, userID); err != nil { + if err := s.ensureProjectAccess(ctx, workspaceSlug, projectID, userID); err != nil { return err } asset, err := s.is.GetFileAssetByID(ctx, assetID) @@ -172,7 +157,7 @@ func (s *AttachmentService) ConfirmUpload(ctx context.Context, workspaceSlug str // ListAttachments returns uploaded attachments for an issue. func (s *AttachmentService) ListAttachments(ctx context.Context, workspaceSlug string, projectID, issueID uuid.UUID, userID uuid.UUID) ([]AttachmentResponse, error) { - if err := s.ensureIssueAccess(ctx, workspaceSlug, projectID, issueID, userID); err != nil { + if err := s.ensureProjectAccess(ctx, workspaceSlug, projectID, userID); err != nil { return nil, err } attachments, assets, err := s.is.ListAttachmentsWithAssets(ctx, issueID) @@ -209,7 +194,7 @@ func (s *AttachmentService) ListAttachments(ctx context.Context, workspaceSlug s // DeleteAttachment removes an attachment and its file asset. func (s *AttachmentService) DeleteAttachment(ctx context.Context, workspaceSlug string, projectID, issueID, assetID uuid.UUID, userID uuid.UUID) error { - if err := s.ensureIssueAccess(ctx, workspaceSlug, projectID, issueID, userID); err != nil { + if err := s.ensureProjectAccess(ctx, workspaceSlug, projectID, userID); err != nil { return err } asset, err := s.is.GetFileAssetByID(ctx, assetID) diff --git a/api/internal/service/cycle.go b/api/internal/service/cycle.go index 6e1e916..56269b3 100644 --- a/api/internal/service/cycle.go +++ b/api/internal/service/cycle.go @@ -44,17 +44,12 @@ type CycleService struct { cs *store.CycleStore ps *store.ProjectStore ws *store.WorkspaceStore - is *store.IssueStore // optional: validates issues added to a cycle belong to the project } func NewCycleService(cs *store.CycleStore, ps *store.ProjectStore, ws *store.WorkspaceStore) *CycleService { return &CycleService{cs: cs, ps: ps, ws: ws} } -// SetIssueStore enables validation that an issue added to a cycle belongs to the -// same project. -func (s *CycleService) SetIssueStore(is *store.IssueStore) { s.is = is } - func (s *CycleService) ensureProjectAccess(ctx context.Context, workspaceSlug string, projectID uuid.UUID, userID uuid.UUID) error { wrk, err := s.ws.GetBySlug(ctx, workspaceSlug) if err != nil { @@ -183,14 +178,6 @@ func (s *CycleService) AddCycleIssue(ctx context.Context, workspaceSlug string, if err != nil { return err } - // The issue must belong to the same project — otherwise a member could attach - // an issue from another project/workspace into this cycle. - if s.is != nil { - issue, err := s.is.GetByID(ctx, issueID) - if err != nil || issue == nil || issue.ProjectID != projectID { - return ErrIssueNotFound - } - } ci := &model.CycleIssue{ CycleID: cy.ID, IssueID: issueID, diff --git a/api/internal/service/github_sync.go b/api/internal/service/github_sync.go index 3021ae1..c900b9c 100644 --- a/api/internal/service/github_sync.go +++ b/api/internal/service/github_sync.go @@ -93,7 +93,8 @@ func (s *GithubSyncService) CreateSync(ctx context.Context, workspaceSlug string if err != nil { return nil, nil, ErrWorkspaceNotFound } - if m, err := s.ws.GetMember(ctx, w.ID, userID); err != nil || m == nil || m.Role < model.RoleAdmin { + ok, _ := s.ws.IsMember(ctx, w.ID, userID) + if !ok { return nil, nil, ErrWorkspaceForbidden } inWorkspace, _ := s.ps.IsInWorkspace(ctx, projectID, w.ID) @@ -145,9 +146,6 @@ func (s *GithubSyncService) UpdateSync(ctx context.Context, workspaceSlug string if err != nil { return nil, err } - if m, err := s.ws.GetMember(ctx, sync.WorkspaceID, userID); err != nil || m == nil || m.Role < model.RoleAdmin { - return nil, ErrWorkspaceForbidden - } if autoLink != nil { sync.AutoLink = *autoLink } @@ -206,9 +204,6 @@ func (s *GithubSyncService) DeleteSync(ctx context.Context, workspaceSlug string if err != nil { return err } - if m, err := s.ws.GetMember(ctx, sync.WorkspaceID, userID); err != nil || m == nil || m.Role < model.RoleAdmin { - return ErrWorkspaceForbidden - } if err := s.rs.Delete(ctx, sync.ID); err != nil { return err } diff --git a/api/internal/service/integration.go b/api/internal/service/integration.go index 90b7a85..da78051 100644 --- a/api/internal/service/integration.go +++ b/api/internal/service/integration.go @@ -19,7 +19,6 @@ var ( ErrIntegrationAlreadyInstalled = errors.New("integration already installed in this workspace") ErrGitHubAppNotConfigured = errors.New("github app is not configured") ErrInstallationFetch = errors.New("failed to fetch github installation") - ErrInstallationAlreadyLinked = errors.New("github installation is already linked to another workspace") ) // IntegrationService coordinates the generic Integration / WorkspaceIntegration @@ -124,9 +123,8 @@ func (s *IntegrationService) InstallGitHub(ctx context.Context, workspaceSlug st if err != nil { return nil, ErrWorkspaceNotFound } - // Linking a GitHub App installation grants the workspace access to the - // installation's repos — restrict it to workspace admins. - if m, err := s.ws.GetMember(ctx, w.ID, userID); err != nil || m == nil || m.Role < model.RoleAdmin { + ok, _ := s.ws.IsMember(ctx, w.ID, userID) + if !ok { return nil, ErrWorkspaceForbidden } gh, err := s.is.GetByProvider(ctx, "github") @@ -163,11 +161,11 @@ func (s *IntegrationService) InstallGitHub(ctx context.Context, workspaceSlug st existing.Provider = "github" return existing, nil } - // Bound to a different workspace already. Refuse — otherwise any admin who - // guesses/enumerates an installation_id could steal another org's - // installation (and the private-repo access it carries) into their own - // workspace. The rightful owner must uninstall first. - return nil, ErrInstallationAlreadyLinked + // Different workspace — soft-delete the old row so the unique partial + // index frees up, then create the new row. + if err := s.wis.Delete(ctx, existing.ID); err != nil { + return nil, err + } } // New installation in this workspace — error if the workspace already has @@ -218,10 +216,6 @@ func (s *IntegrationService) Uninstall(ctx context.Context, workspaceSlug, provi if err != nil { return err } - // Disconnecting a workspace integration is admin-only. - if m, err := s.ws.GetMember(ctx, wi.WorkspaceID, userID); err != nil || m == nil || m.Role < model.RoleAdmin { - return ErrWorkspaceForbidden - } if provider == "github" && s.githubClient != nil && wi.InstallationID != nil { s.githubClient.InvalidateInstallation(*wi.InstallationID) } diff --git a/api/internal/service/issue_view.go b/api/internal/service/issue_view.go index 9f1e10c..567c356 100644 --- a/api/internal/service/issue_view.go +++ b/api/internal/service/issue_view.go @@ -141,7 +141,7 @@ func (s *IssueViewService) Create(ctx context.Context, workspaceSlug string, pro } func (s *IssueViewService) Get(ctx context.Context, workspaceSlug string, viewID uuid.UUID, userID uuid.UUID) (*model.IssueView, error) { - workspaceID, err := s.ensureWorkspaceAccess(ctx, workspaceSlug, userID) + _, err := s.ensureWorkspaceAccess(ctx, workspaceSlug, userID) if err != nil { return nil, err } @@ -149,11 +149,6 @@ func (s *IssueViewService) Get(ctx context.Context, workspaceSlug string, viewID if err != nil { return nil, ErrIssueViewNotFound } - // The view must belong to the workspace in the URL — otherwise any member of - // any workspace could read another workspace's view by id. - if iv.WorkspaceID != workspaceID { - return nil, ErrIssueViewNotFound - } if s.fav != nil { ok, ferr := s.fav.IsIssueViewFavorited(ctx, userID, viewID) if ferr == nil { diff --git a/api/internal/service/module.go b/api/internal/service/module.go index 58ade01..9b7cbb1 100644 --- a/api/internal/service/module.go +++ b/api/internal/service/module.go @@ -17,17 +17,12 @@ type ModuleService struct { ms *store.ModuleStore ps *store.ProjectStore ws *store.WorkspaceStore - is *store.IssueStore // optional: validates issues added to a module belong to the project } func NewModuleService(ms *store.ModuleStore, ps *store.ProjectStore, ws *store.WorkspaceStore) *ModuleService { return &ModuleService{ms: ms, ps: ps, ws: ws} } -// SetIssueStore enables validation that an issue added to a module belongs to the -// same project. -func (s *ModuleService) SetIssueStore(is *store.IssueStore) { s.is = is } - func (s *ModuleService) ensureProjectAccess(ctx context.Context, workspaceSlug string, projectID uuid.UUID, userID uuid.UUID) error { wrk, err := s.ws.GetBySlug(ctx, workspaceSlug) if err != nil { @@ -158,13 +153,6 @@ func (s *ModuleService) AddModuleIssue(ctx context.Context, workspaceSlug string if err != nil { return err } - // The issue must belong to the same project. - if s.is != nil { - issue, err := s.is.GetByID(ctx, issueID) - if err != nil || issue == nil || issue.ProjectID != projectID { - return ErrIssueNotFound - } - } mi := &model.ModuleIssue{ ModuleID: mod.ID, IssueID: issueID, diff --git a/api/internal/service/project.go b/api/internal/service/project.go index cc41c89..a6e0151 100644 --- a/api/internal/service/project.go +++ b/api/internal/service/project.go @@ -59,20 +59,6 @@ func (s *ProjectService) GetByID(ctx context.Context, workspaceSlug string, proj return s.ps.GetByID(ctx, projectID) } -// requireProjectAdmin allows the action when the caller is a workspace -// admin/owner, or a project member with at least the Admin role. Read access -// (workspace membership) is validated separately by GetByID. -func (s *ProjectService) requireProjectAdmin(ctx context.Context, workspaceID, projectID, userID uuid.UUID) error { - if wm, err := s.ws.GetMember(ctx, workspaceID, userID); err == nil && wm != nil && wm.Role >= model.RoleAdmin { - return nil - } - pm, err := s.ps.GetProjectMember(ctx, projectID, userID) - if err != nil || pm == nil || pm.Role < model.RoleAdmin { - return ErrProjectForbidden - } - return nil -} - func (s *ProjectService) Create(ctx context.Context, workspaceSlug, name, identifier string, userID uuid.UUID) (*model.Project, error) { wrk, err := s.ws.GetBySlug(ctx, workspaceSlug) if err != nil { @@ -102,9 +88,6 @@ func (s *ProjectService) Update(ctx context.Context, workspaceSlug string, proje if err != nil { return nil, err } - if err := s.requireProjectAdmin(ctx, p.WorkspaceID, p.ID, userID); err != nil { - return nil, err - } if name != nil { p.Name = *name } @@ -166,13 +149,10 @@ func (s *ProjectService) Update(ctx context.Context, workspaceSlug string, proje } func (s *ProjectService) Delete(ctx context.Context, workspaceSlug string, projectID uuid.UUID, userID uuid.UUID) error { - p, err := s.GetByID(ctx, workspaceSlug, projectID, userID) + _, err := s.GetByID(ctx, workspaceSlug, projectID, userID) if err != nil { return err } - if err := s.requireProjectAdmin(ctx, p.WorkspaceID, p.ID, userID); err != nil { - return err - } return s.ps.Delete(ctx, projectID) } @@ -197,17 +177,10 @@ func (s *ProjectService) GetMember(ctx context.Context, workspaceSlug string, pr } func (s *ProjectService) UpdateMemberRole(ctx context.Context, workspaceSlug string, projectID uuid.UUID, memberPK uuid.UUID, userID uuid.UUID, role int16) (*model.ProjectMember, error) { - p, err := s.GetByID(ctx, workspaceSlug, projectID, userID) + m, err := s.GetMember(ctx, workspaceSlug, projectID, memberPK, userID) if err != nil { return nil, err } - if err := s.requireProjectAdmin(ctx, p.WorkspaceID, p.ID, userID); err != nil { - return nil, err - } - m, err := s.ps.GetProjectMemberByPK(ctx, memberPK) - if err != nil || m.ProjectID != projectID { - return nil, ErrMemberNotFound - } m.Role = role if err := s.ps.UpdateProjectMember(ctx, m); err != nil { return nil, err @@ -216,13 +189,10 @@ func (s *ProjectService) UpdateMemberRole(ctx context.Context, workspaceSlug str } func (s *ProjectService) DeleteMember(ctx context.Context, workspaceSlug string, projectID uuid.UUID, memberPK uuid.UUID, userID uuid.UUID) error { - p, err := s.GetByID(ctx, workspaceSlug, projectID, userID) + _, err := s.GetByID(ctx, workspaceSlug, projectID, userID) if err != nil { return err } - if err := s.requireProjectAdmin(ctx, p.WorkspaceID, p.ID, userID); err != nil { - return err - } m, err := s.ps.GetProjectMemberByPK(ctx, memberPK) if err != nil || m.ProjectID != projectID { return ErrMemberNotFound @@ -252,9 +222,6 @@ func (s *ProjectService) CreateInvite(ctx context.Context, workspaceSlug string, if err != nil { return nil, err } - if err := s.requireProjectAdmin(ctx, p.WorkspaceID, p.ID, userID); err != nil { - return nil, err - } inv := &model.ProjectMemberInvite{ ProjectID: p.ID, WorkspaceID: p.WorkspaceID, @@ -290,13 +257,10 @@ func (s *ProjectService) GetInvite(ctx context.Context, workspaceSlug string, pr } func (s *ProjectService) DeleteInvite(ctx context.Context, workspaceSlug string, projectID uuid.UUID, inviteID uuid.UUID, userID uuid.UUID) error { - inv, err := s.GetInvite(ctx, workspaceSlug, projectID, inviteID, userID) + _, err := s.GetInvite(ctx, workspaceSlug, projectID, inviteID, userID) if err != nil { return err } - if err := s.requireProjectAdmin(ctx, inv.WorkspaceID, inv.ProjectID, userID); err != nil { - return err - } return s.pinv.Delete(ctx, inviteID) } diff --git a/api/internal/service/workspace.go b/api/internal/service/workspace.go index 9de8f03..063d93d 100644 --- a/api/internal/service/workspace.go +++ b/api/internal/service/workspace.go @@ -93,9 +93,6 @@ func (s *WorkspaceService) Update(ctx context.Context, slug string, userID uuid. if err != nil { return nil, err } - if _, err := s.requireAdmin(ctx, w.ID, userID); err != nil { - return nil, err - } if name != nil { w.Name = *name } @@ -146,28 +143,6 @@ func (s *WorkspaceService) ListMembers(ctx context.Context, slug string, userID return s.ws.ListMembers(ctx, w.ID) } -// callerRole returns the authenticated user's role in the workspace, or -// ErrWorkspaceForbidden if they are not a member. -func (s *WorkspaceService) callerRole(ctx context.Context, workspaceID, userID uuid.UUID) (int16, error) { - m, err := s.ws.GetMember(ctx, workspaceID, userID) - if err != nil || m == nil { - return 0, ErrWorkspaceForbidden - } - return m.Role, nil -} - -// requireAdmin ensures the caller is at least a workspace admin. -func (s *WorkspaceService) requireAdmin(ctx context.Context, workspaceID, userID uuid.UUID) (int16, error) { - role, err := s.callerRole(ctx, workspaceID, userID) - if err != nil { - return 0, err - } - if role < model.RoleAdmin { - return 0, ErrWorkspaceForbidden - } - return role, nil -} - func (s *WorkspaceService) GetMember(ctx context.Context, slug string, memberID uuid.UUID, userID uuid.UUID) (*model.WorkspaceMember, error) { w, err := s.GetBySlug(ctx, slug, userID) if err != nil { @@ -181,26 +156,10 @@ func (s *WorkspaceService) GetMember(ctx context.Context, slug string, memberID } func (s *WorkspaceService) UpdateMemberRole(ctx context.Context, slug string, memberID uuid.UUID, userID uuid.UUID, role int16) (*model.WorkspaceMember, error) { - w, err := s.GetBySlug(ctx, slug, userID) - if err != nil { - return nil, err - } - callerRole, err := s.requireAdmin(ctx, w.ID, userID) + m, err := s.GetMember(ctx, slug, memberID, userID) if err != nil { return nil, err } - m, err := s.ws.GetMemberByPK(ctx, memberID) - if err != nil || m.WorkspaceID != w.ID { - return nil, ErrMemberNotFound - } - // Only the owner may change the owner's role; nobody may demote the owner. - if m.MemberID == w.OwnerID && userID != w.OwnerID { - return nil, ErrWorkspaceForbidden - } - // Cannot grant a role above your own, and only the owner may grant Owner. - if role > callerRole || (role >= model.RoleOwner && userID != w.OwnerID) { - return nil, ErrWorkspaceForbidden - } m.Role = role if err := s.ws.UpdateMember(ctx, m); err != nil { return nil, err @@ -213,17 +172,10 @@ func (s *WorkspaceService) DeleteMember(ctx context.Context, slug string, member if err != nil { return err } - if _, err := s.requireAdmin(ctx, w.ID, userID); err != nil { - return err - } m, err := s.ws.GetMemberByPK(ctx, memberID) if err != nil || m.WorkspaceID != w.ID { return ErrMemberNotFound } - // The workspace owner cannot be removed. - if m.MemberID == w.OwnerID { - return ErrWorkspaceForbidden - } return s.ws.DeleteMember(ctx, w.ID, m.MemberID) } @@ -243,14 +195,6 @@ func (s *WorkspaceService) CreateInvite(ctx context.Context, slug string, userID if err != nil { return nil, err } - callerRole, err := s.requireAdmin(ctx, w.ID, userID) - if err != nil { - return nil, err - } - // Cannot invite at a role above your own. - if role > callerRole { - return nil, ErrWorkspaceForbidden - } token := genInviteToken() inv := &model.WorkspaceMemberInvite{ WorkspaceID: w.ID, @@ -286,13 +230,10 @@ func (s *WorkspaceService) GetInvite(ctx context.Context, slug string, inviteID } func (s *WorkspaceService) DeleteInvite(ctx context.Context, slug string, inviteID uuid.UUID, userID uuid.UUID) error { - inv, err := s.GetInvite(ctx, slug, inviteID, userID) + _, err := s.GetInvite(ctx, slug, inviteID, userID) if err != nil { return err } - if _, err := s.requireAdmin(ctx, inv.WorkspaceID, userID); err != nil { - return err - } return s.winv.Delete(ctx, inviteID) } diff --git a/api/internal/store/instance_admin.go b/api/internal/store/instance_admin.go deleted file mode 100644 index dbb1cde..0000000 --- a/api/internal/store/instance_admin.go +++ /dev/null @@ -1,76 +0,0 @@ -package store - -import ( - "context" - "errors" - - "github.com/Devlaner/devlane/api/internal/model" - "github.com/google/uuid" - "gorm.io/gorm" - "gorm.io/gorm/clause" -) - -// ErrLastInstanceAdmin is returned when a delete would remove the final admin. -var ErrLastInstanceAdmin = errors.New("cannot remove the last instance admin") - -// InstanceAdminStore handles instance_admins persistence — the set of users -// authorized to manage instance settings (mirrors Plane's InstanceAdmin). -type InstanceAdminStore struct{ db *gorm.DB } - -func NewInstanceAdminStore(db *gorm.DB) *InstanceAdminStore { - return &InstanceAdminStore{db: db} -} - -// IsAdmin reports whether the user is an instance admin (role >= RoleAdmin). -func (s *InstanceAdminStore) IsAdmin(ctx context.Context, userID uuid.UUID) (bool, error) { - var count int64 - err := s.db.WithContext(ctx).Model(&model.InstanceAdmin{}). - Where("user_id = ? AND role >= ?", userID, model.RoleAdmin). - Count(&count).Error - return count > 0, err -} - -// Create inserts an instance admin row. -func (s *InstanceAdminStore) Create(ctx context.Context, a *model.InstanceAdmin) error { - return s.db.WithContext(ctx).Create(a).Error -} - -// GetByUserID returns the (non-deleted) admin row for a user, if any. -func (s *InstanceAdminStore) GetByUserID(ctx context.Context, userID uuid.UUID) (*model.InstanceAdmin, error) { - var a model.InstanceAdmin - if err := s.db.WithContext(ctx).Where("user_id = ?", userID).First(&a).Error; err != nil { - return nil, err - } - return &a, nil -} - -// List returns all instance admins with the user's email/display name joined for display. -func (s *InstanceAdminStore) List(ctx context.Context) ([]model.InstanceAdmin, error) { - var admins []model.InstanceAdmin - err := s.db.WithContext(ctx). - Table("instance_admins"). - Select("instance_admins.*, users.email AS user_email, users.display_name AS user_display_name"). - Joins("LEFT JOIN users ON users.id = instance_admins.user_id"). - Where("instance_admins.deleted_at IS NULL"). - Order("instance_admins.created_at ASC"). - Scan(&admins).Error - return admins, err -} - -// DeleteByPKIfNotLast soft-deletes the admin with the given id, but only when -// more than one active admin exists. The count and delete run in a single -// transaction with the active admin rows locked FOR UPDATE, so two concurrent -// removals cannot both pass the guard and leave the instance with zero admins. -// Returns ErrLastInstanceAdmin if the delete would remove the final admin. -func (s *InstanceAdminStore) DeleteByPKIfNotLast(ctx context.Context, id uuid.UUID) error { - return s.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error { - var admins []model.InstanceAdmin - if err := tx.Clauses(clause.Locking{Strength: "UPDATE"}).Find(&admins).Error; err != nil { - return err - } - if len(admins) <= 1 { - return ErrLastInstanceAdmin - } - return tx.Delete(&model.InstanceAdmin{}, "id = ?", id).Error - }) -} diff --git a/api/internal/store/session.go b/api/internal/store/session.go index 8e7b6d8..33b0673 100644 --- a/api/internal/store/session.go +++ b/api/internal/store/session.go @@ -50,23 +50,6 @@ func (s *SessionStore) Delete(ctx context.Context, sessionKey string) error { return s.db.WithContext(ctx).Where("session_key = ?", sessionKey).Delete(&model.Session{}).Error } -// DeleteByUserID removes every session belonging to the user. Used after a -// password reset so a stolen session cannot outlive the credential change. -func (s *SessionStore) DeleteByUserID(ctx context.Context, userID uuid.UUID) error { - return s.db.WithContext(ctx). - Where("session_data::jsonb->>'user_id' = ?", userID.String()). - Delete(&model.Session{}).Error -} - -// DeleteByUserIDExcept removes all of the user's sessions except keepKey. Used -// on self-service password change so the acting session survives while any other -// (possibly attacker-held) sessions are evicted. -func (s *SessionStore) DeleteByUserIDExcept(ctx context.Context, userID uuid.UUID, keepKey string) error { - return s.db.WithContext(ctx). - Where("session_data::jsonb->>'user_id' = ? AND session_key <> ?", userID.String(), keepKey). - Delete(&model.Session{}).Error -} - func (s *SessionStore) RefreshExpire(ctx context.Context, sessionKey string) error { expire := time.Now().UTC().AddDate(0, 0, sessionExpireDays) return s.db.WithContext(ctx).Model(&model.Session{}).Where("session_key = ?", sessionKey).Update("expire_date", expire).Error diff --git a/api/internal/testutil/factory.go b/api/internal/testutil/factory.go index f2d7153..ca96d41 100644 --- a/api/internal/testutil/factory.go +++ b/api/internal/testutil/factory.go @@ -120,23 +120,6 @@ func CreateUser(t testing.TB, db *gorm.DB, opts ...UserOpt) *model.User { return u } -// SeedInstanceAdmin registers user as an instance admin by inserting an -// instance_admins row (matches what InstanceSetup does on first run). Required -// for tests that hit the admin-gated /instance/ endpoints. -func SeedInstanceAdmin(t testing.TB, db *gorm.DB, user *model.User) { - t.Helper() - if user == nil { - t.Fatal("SeedInstanceAdmin: user is nil") - } - if err := store.NewInstanceAdminStore(db).Create(context.Background(), &model.InstanceAdmin{ - UserID: user.ID, - Role: RoleOwner, - IsVerified: true, - }); err != nil { - t.Fatalf("SeedInstanceAdmin: %v", err) - } -} - // Workspace member roles (matches int16 values used by the API). const ( RoleGuest int16 = 5 diff --git a/api/migrations/000006_instance_admins.down.sql b/api/migrations/000006_instance_admins.down.sql deleted file mode 100644 index 3bf3cca..0000000 --- a/api/migrations/000006_instance_admins.down.sql +++ /dev/null @@ -1,3 +0,0 @@ -DROP INDEX IF EXISTS idx_instance_admins_user_active; -ALTER TABLE instance_admins DROP COLUMN IF EXISTS deleted_at; --- instance_id is left nullable; re-adding NOT NULL could fail if NULLs exist. diff --git a/api/migrations/000006_instance_admins.up.sql b/api/migrations/000006_instance_admins.up.sql deleted file mode 100644 index 22717f2..0000000 --- a/api/migrations/000006_instance_admins.up.sql +++ /dev/null @@ -1,13 +0,0 @@ --- The instance_admins table already exists from 000001 (copied from Plane's --- schema) with a NOT NULL instance_id FK to `instances`. Devlane is --- single-instance and tracks instance info in instance_settings, so it never --- populates `instances`. Adapt the existing table to key admins on user_id: --- 1. allow a NULL instance_id (no instances row required), --- 2. add a soft-delete column, --- 3. add a partial unique index so one user maps to one active admin row. -ALTER TABLE instance_admins ALTER COLUMN instance_id DROP NOT NULL; -ALTER TABLE instance_admins ADD COLUMN IF NOT EXISTS deleted_at TIMESTAMPTZ; - -CREATE UNIQUE INDEX IF NOT EXISTS idx_instance_admins_user_active - ON instance_admins (user_id) - WHERE deleted_at IS NULL; diff --git a/ui/package-lock.json b/ui/package-lock.json index 9b11135..721a187 100644 --- a/ui/package-lock.json +++ b/ui/package-lock.json @@ -33,7 +33,6 @@ "@tiptap/suggestion": "^3.22.3", "axios": "^1.16.0", "clsx": "^2.1.1", - "dompurify": "^3.2.7", "lucide-react": "^0.563.0", "react": "^19.2.0", "react-dom": "^19.2.0", @@ -55,7 +54,7 @@ "prettier": "^3.8.1", "typescript": "~5.9.3", "typescript-eslint": "^8.48.0", - "vite": "^7.3.2" + "vite": "^7.3.1" } }, "node_modules/@babel/code-frame": { @@ -2843,13 +2842,6 @@ "@types/react": "^19.2.0" } }, - "node_modules/@types/trusted-types": { - "version": "2.0.7", - "resolved": "https://registry.npmjs.org/@types/trusted-types/-/trusted-types-2.0.7.tgz", - "integrity": "sha512-ScaPdn1dQczgbl0QFTeTOmVHFULt394XJgOQNoyVhZ6r2vLnMLJfBPd53SB52T/3G36VI1/g2MZaX0cwDuXsfw==", - "license": "MIT", - "optional": true - }, "node_modules/@types/use-sync-external-store": { "version": "0.0.6", "resolved": "https://registry.npmjs.org/@types/use-sync-external-store/-/use-sync-external-store-0.0.6.tgz", @@ -3609,15 +3601,6 @@ "node": ">=8" } }, - "node_modules/dompurify": { - "version": "3.4.11", - "resolved": "https://registry.npmjs.org/dompurify/-/dompurify-3.4.11.tgz", - "integrity": "sha512-zhlUV12GsaRzMsf9q5M254YhA4+VuF0fG+QFqu6aYpoGlKtz+w8//jBcGVYBgQkR5GHjUomejY84AV+/uPbWdw==", - "license": "(MPL-2.0 OR Apache-2.0)", - "optionalDependencies": { - "@types/trusted-types": "^2.0.7" - } - }, "node_modules/dunder-proto": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/dunder-proto/-/dunder-proto-1.0.1.tgz", @@ -5769,9 +5752,9 @@ } }, "node_modules/vite": { - "version": "7.3.5", - "resolved": "https://registry.npmjs.org/vite/-/vite-7.3.5.tgz", - "integrity": "sha512-KuOaNhcnGFN2zIPGA7wRmzF+lJA1sea7rHq17aiJ++9lzY1WWG6Jpwqwe1KNbRVPIqHmr8GLYx7jbrQcN/7/ww==", + "version": "7.3.1", + "resolved": "https://registry.npmjs.org/vite/-/vite-7.3.1.tgz", + "integrity": "sha512-w+N7Hifpc3gRjZ63vYBXA56dvvRlNWRczTdmCBBa+CotUzAPf5b7YMdMR/8CQoeYE5LX3W4wj6RYTgonm1b9DA==", "license": "MIT", "dependencies": { "esbuild": "^0.27.0", diff --git a/ui/package.json b/ui/package.json index dc8acd7..08cc4c3 100644 --- a/ui/package.json +++ b/ui/package.json @@ -39,7 +39,6 @@ "@tiptap/suggestion": "^3.22.3", "axios": "^1.16.0", "clsx": "^2.1.1", - "dompurify": "^3.2.7", "lucide-react": "^0.563.0", "react": "^19.2.0", "react-dom": "^19.2.0", @@ -61,6 +60,6 @@ "prettier": "^3.8.1", "typescript": "~5.9.3", "typescript-eslint": "^8.48.0", - "vite": "^7.3.2" + "vite": "^7.3.1" } } diff --git a/ui/src/api/types.ts b/ui/src/api/types.ts index f7d9bef..41679a0 100644 --- a/ui/src/api/types.ts +++ b/ui/src/api/types.ts @@ -394,18 +394,6 @@ export type InstanceSettingsResponse = Record>; /** Section value for PATCH /api/instance/settings/:key */ export type InstanceSettingSectionValue = Record; -/** Instance admin row (from GET /api/instance/admins/) */ -export interface InstanceAdminApiResponse { - id: string; - user_id: string; - role: number; - is_verified: boolean; - created_at: string; - updated_at: string; - user_email?: string | null; - user_display_name?: string; -} - /** General section shape */ export interface InstanceGeneralSection { instance_name?: string; diff --git a/ui/src/components/layout/InstanceAdminLayout.tsx b/ui/src/components/layout/InstanceAdminLayout.tsx index 386b569..b9887d8 100644 --- a/ui/src/components/layout/InstanceAdminLayout.tsx +++ b/ui/src/components/layout/InstanceAdminLayout.tsx @@ -191,25 +191,6 @@ const IconArrowLeft = () => ( ); -const IconUsers = () => ( - - - - - - -); - const SECTIONS = [ { path: 'general', @@ -217,12 +198,6 @@ const SECTIONS = [ desc: 'Identify your instances and get key details.', Icon: IconSettings, }, - { - path: 'admins', - label: 'Admins', - desc: 'Manage instance administrators.', - Icon: IconUsers, - }, { path: 'workspace', label: 'Workspaces', @@ -263,7 +238,6 @@ const SECTIONS = [ const BREADCRUMB_LABEL: Record = { general: 'General', - admins: 'Admins', workspace: 'Workspace', email: 'Email', authentication: 'Authentication', diff --git a/ui/src/lib/sanitize.ts b/ui/src/lib/sanitize.ts deleted file mode 100644 index 3f4998d..0000000 --- a/ui/src/lib/sanitize.ts +++ /dev/null @@ -1,36 +0,0 @@ -import DOMPurify from 'dompurify'; - -/** - * Sanitize untrusted HTML (comment bodies, page/version content, etc.) before - * injecting it via dangerouslySetInnerHTML. Strips