fix: validate roles, guard last owner, scope bootstrap status
Security review of e70b2f0. The UI gating was correctly backed by
RequireRole everywhere; these are the missing validation gaps behind it.
- UpdateUserRole and createOrgUser accepted any role string verbatim, so
an admin could self-promote to owner, create an owner outright, or set
a junk role that silently stripped a user's access. Roles are now
whitelisted, only an owner may grant or remove the owner role, and an
actor cannot change their own.
- Neither demote nor delete guarded the last owner, so an org could reach
zero owners. Both now refuse when no owner would remain, returning 409.
Self-delete rejected.
- CountUsers counted across all orgs, so a locked-out org could never
re-bootstrap once another tenant existed, and the unauthenticated
bootstrap-status endpoint reported instance-wide state. It now answers
per-org on an org host, falling back to global only on the apex.
- HandleMe repeats the middleware's host/org check; it sits outside the
middleware so it can still return its own 401.
- Post-bootstrap now sends the new owner to their org host's login page.
The session cookie is deliberately scoped to the exact host, so the old
redirect landed them unauthenticated.
- AuthProvider renders an error state instead of mounting the shell with
a null user when /auth/me fails for a reason other than 401.
- api.ts unwraps {"error": ...} so these messages render as text.
This commit is contained in:
@@ -1,10 +1,12 @@
|
||||
package api
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"net/http"
|
||||
|
||||
"github.com/gin-gonic/gin"
|
||||
"github.com/mrhid6/vantage/server/internal/auth"
|
||||
"github.com/mrhid6/vantage/server/internal/models"
|
||||
"github.com/mrhid6/vantage/server/internal/services"
|
||||
)
|
||||
|
||||
@@ -17,6 +19,13 @@ func listOrgUsers(c *gin.Context) {
|
||||
c.JSON(http.StatusOK, users)
|
||||
}
|
||||
|
||||
// Granting or removing the owner role is reserved to owners: an admin must
|
||||
// never be able to mint an owner (and log in as it) or strip the owners above
|
||||
// them. Everything below derives the actor from the session, never the body.
|
||||
func actorMayGrantOwner(c *gin.Context) bool {
|
||||
return auth.Role(c) == models.RoleOwner
|
||||
}
|
||||
|
||||
func createOrgUser(c *gin.Context) {
|
||||
var body struct {
|
||||
Email string `json:"email"`
|
||||
@@ -28,7 +37,15 @@ func createOrgUser(c *gin.Context) {
|
||||
return
|
||||
}
|
||||
if body.Role == "" {
|
||||
body.Role = "member"
|
||||
body.Role = models.RoleMember
|
||||
}
|
||||
if !models.ValidRole(body.Role) {
|
||||
c.JSON(http.StatusBadRequest, gin.H{"error": "role must be one of owner, admin, or member"})
|
||||
return
|
||||
}
|
||||
if body.Role == models.RoleOwner && !actorMayGrantOwner(c) {
|
||||
c.JSON(http.StatusForbidden, gin.H{"error": "only an owner can create another owner"})
|
||||
return
|
||||
}
|
||||
u, err := services.CreateUser(auth.OrgID(c), body.Email, body.Password, body.Role, "local")
|
||||
if err != nil {
|
||||
@@ -46,21 +63,65 @@ func updateOrgUserRole(c *gin.Context) {
|
||||
c.JSON(http.StatusBadRequest, gin.H{"error": "role required"})
|
||||
return
|
||||
}
|
||||
if err := services.UpdateUserRole(auth.OrgID(c), c.Param("id"), body.Role); err != nil {
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
|
||||
if !models.ValidRole(body.Role) {
|
||||
c.JSON(http.StatusBadRequest, gin.H{"error": "role must be one of owner, admin, or member"})
|
||||
return
|
||||
}
|
||||
|
||||
orgID, targetID := auth.OrgID(c), c.Param("id")
|
||||
if targetID == auth.UserID(c) {
|
||||
c.JSON(http.StatusForbidden, gin.H{"error": "you cannot change your own role"})
|
||||
return
|
||||
}
|
||||
target, err := services.GetUserInOrg(orgID, targetID)
|
||||
if err != nil {
|
||||
c.JSON(http.StatusNotFound, gin.H{"error": "user not found"})
|
||||
return
|
||||
}
|
||||
if (body.Role == models.RoleOwner || target.Role == models.RoleOwner) && !actorMayGrantOwner(c) {
|
||||
c.JSON(http.StatusForbidden, gin.H{"error": "only an owner can change owner roles"})
|
||||
return
|
||||
}
|
||||
|
||||
if err := services.UpdateUserRole(orgID, targetID, body.Role); err != nil {
|
||||
c.JSON(orgUserErrStatus(err), gin.H{"error": err.Error()})
|
||||
return
|
||||
}
|
||||
c.JSON(http.StatusOK, gin.H{"ok": true})
|
||||
}
|
||||
|
||||
func deleteOrgUser(c *gin.Context) {
|
||||
if err := services.DeleteUser(auth.OrgID(c), c.Param("id")); err != nil {
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
|
||||
orgID, targetID := auth.OrgID(c), c.Param("id")
|
||||
if targetID == auth.UserID(c) {
|
||||
c.JSON(http.StatusForbidden, gin.H{"error": "you cannot remove your own account"})
|
||||
return
|
||||
}
|
||||
target, err := services.GetUserInOrg(orgID, targetID)
|
||||
if err != nil {
|
||||
c.JSON(http.StatusNotFound, gin.H{"error": "user not found"})
|
||||
return
|
||||
}
|
||||
if target.Role == models.RoleOwner && !actorMayGrantOwner(c) {
|
||||
c.JSON(http.StatusForbidden, gin.H{"error": "only an owner can remove another owner"})
|
||||
return
|
||||
}
|
||||
|
||||
if err := services.DeleteUser(orgID, targetID); err != nil {
|
||||
c.JSON(orgUserErrStatus(err), gin.H{"error": err.Error()})
|
||||
return
|
||||
}
|
||||
c.JSON(http.StatusOK, gin.H{"deleted": true})
|
||||
}
|
||||
|
||||
// The last-owner guard is a rejected request, not a server fault — surface it
|
||||
// as 409 so the UI shows the message rather than a generic failure.
|
||||
func orgUserErrStatus(err error) int {
|
||||
if errors.Is(err, services.ErrLastOwner) {
|
||||
return http.StatusConflict
|
||||
}
|
||||
return http.StatusInternalServerError
|
||||
}
|
||||
|
||||
func getOrgOIDC(c *gin.Context) {
|
||||
cfg, err := services.GetOrgOIDC(auth.OrgID(c))
|
||||
if err != nil {
|
||||
|
||||
@@ -46,8 +46,20 @@ func HandleLocalLogin(c *gin.Context) {
|
||||
c.JSON(http.StatusOK, gin.H{"ok": true})
|
||||
}
|
||||
|
||||
// HandleBootstrapStatus answers "does the caller need to run setup". It is
|
||||
// unauthenticated, so it must not report instance-wide state to whoever asks:
|
||||
// on an org host the answer is scoped to that org, and only the apex — the
|
||||
// genuine first-run entry point — gets the global "no users anywhere" answer.
|
||||
func HandleBootstrapStatus(c *gin.Context) {
|
||||
n, err := services.CountUsers()
|
||||
var (
|
||||
n int64
|
||||
err error
|
||||
)
|
||||
if org, ok := OrgFromHost(c); ok {
|
||||
n, err = services.CountOrgUsers(org.OrgID)
|
||||
} else {
|
||||
n, err = services.CountUsers()
|
||||
}
|
||||
if err != nil {
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
|
||||
return
|
||||
@@ -55,6 +67,9 @@ func HandleBootstrapStatus(c *gin.Context) {
|
||||
c.JSON(http.StatusOK, gin.H{"needs_setup": n == 0})
|
||||
}
|
||||
|
||||
// HandleBootstrap creates the very first org and its owner, so its guard stays
|
||||
// deliberately global: it may run once on an empty instance and never again,
|
||||
// regardless of which host it is called on.
|
||||
func HandleBootstrap(c *gin.Context) {
|
||||
n, err := services.CountUsers()
|
||||
if err != nil {
|
||||
@@ -106,6 +121,14 @@ func HandleMe(c *gin.Context) {
|
||||
c.JSON(http.StatusUnauthorized, gin.H{"error": "session expired"})
|
||||
return
|
||||
}
|
||||
// /auth/me is registered outside Middleware, so it repeats the middleware's
|
||||
// host/org match itself. Without this, a session for org A presented on org
|
||||
// B's host would render the shell while every /api call 403s.
|
||||
if hostOrg, ok := OrgFromHost(c); ok && hostOrg.OrgID != sess.OrgID {
|
||||
c.JSON(http.StatusForbidden, gin.H{"error": "org host mismatch"})
|
||||
return
|
||||
}
|
||||
|
||||
org, _ := services.GetOrg(sess.OrgID)
|
||||
c.JSON(http.StatusOK, gin.H{"user": sess, "org": org})
|
||||
}
|
||||
|
||||
@@ -6,6 +6,22 @@ import (
|
||||
"go.mongodb.org/mongo-driver/v2/bson"
|
||||
)
|
||||
|
||||
// Org membership roles. These are the only values ever written to User.Role;
|
||||
// anything arriving from a client must be checked with ValidRole first.
|
||||
const (
|
||||
RoleOwner = "owner"
|
||||
RoleAdmin = "admin"
|
||||
RoleMember = "member"
|
||||
)
|
||||
|
||||
func ValidRole(role string) bool {
|
||||
switch role {
|
||||
case RoleOwner, RoleAdmin, RoleMember:
|
||||
return true
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
type User struct {
|
||||
ID bson.ObjectID `bson:"_id,omitempty" json:"_id,omitempty"`
|
||||
UserID string `bson:"user_id" json:"user_id"`
|
||||
|
||||
@@ -2,6 +2,7 @@ package services
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"strings"
|
||||
"time"
|
||||
@@ -14,17 +15,56 @@ import (
|
||||
"golang.org/x/crypto/bcrypt"
|
||||
)
|
||||
|
||||
// ErrLastOwner is returned when an operation would leave an org with no owner,
|
||||
// which would lock every remaining member out of org administration.
|
||||
var ErrLastOwner = errors.New("this is the organization's last owner — promote another member to owner first")
|
||||
|
||||
// CountUsers counts users across the whole instance. It answers "is this a
|
||||
// brand new deployment", so it is deliberately unscoped; anything that asks
|
||||
// about a single tenant must use CountOrgUsers.
|
||||
func CountUsers() (int64, error) {
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||
defer cancel()
|
||||
return db.Col("users").CountDocuments(ctx, bson.M{})
|
||||
}
|
||||
|
||||
func CountOrgUsers(orgID string) (int64, error) {
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||
defer cancel()
|
||||
return db.Col("users").CountDocuments(ctx, bson.M{"org_id": orgID})
|
||||
}
|
||||
|
||||
// countOtherOwners counts owner-role users in the org excluding exceptUserID,
|
||||
// i.e. how many owners would remain if that user were removed or demoted.
|
||||
func countOtherOwners(orgID, exceptUserID string) (int64, error) {
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||
defer cancel()
|
||||
return db.Col("users").CountDocuments(ctx, bson.M{
|
||||
"org_id": orgID,
|
||||
"role": models.RoleOwner,
|
||||
"user_id": bson.M{"$ne": exceptUserID},
|
||||
})
|
||||
}
|
||||
|
||||
func GetUserInOrg(orgID, userID string) (*models.User, error) {
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||
defer cancel()
|
||||
var u models.User
|
||||
err := db.Col("users").FindOne(ctx, bson.M{"user_id": userID, "org_id": orgID}).Decode(&u)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return &u, nil
|
||||
}
|
||||
|
||||
func CreateUser(orgID, email, password, role, authSource string) (*models.User, error) {
|
||||
email = strings.ToLower(strings.TrimSpace(email))
|
||||
if email == "" {
|
||||
return nil, fmt.Errorf("email required")
|
||||
}
|
||||
if !models.ValidRole(role) {
|
||||
return nil, fmt.Errorf("invalid role %q", role)
|
||||
}
|
||||
u := &models.User{
|
||||
UserID: uuid.NewString(),
|
||||
OrgID: orgID,
|
||||
@@ -95,17 +135,49 @@ func ListUsers(orgID string) ([]models.User, error) {
|
||||
}
|
||||
|
||||
func UpdateUserRole(orgID, userID, role string) error {
|
||||
if !models.ValidRole(role) {
|
||||
return fmt.Errorf("invalid role %q", role)
|
||||
}
|
||||
target, err := GetUserInOrg(orgID, userID)
|
||||
if err != nil {
|
||||
return fmt.Errorf("user not found")
|
||||
}
|
||||
// Demoting the final owner would leave nobody able to administer the org.
|
||||
if target.Role == models.RoleOwner && role != models.RoleOwner {
|
||||
others, err := countOtherOwners(orgID, userID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if others == 0 {
|
||||
return ErrLastOwner
|
||||
}
|
||||
}
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||
defer cancel()
|
||||
_, err := db.Col("users").UpdateOne(ctx,
|
||||
_, err = db.Col("users").UpdateOne(ctx,
|
||||
bson.M{"user_id": userID, "org_id": orgID},
|
||||
bson.M{"$set": bson.M{"role": role}})
|
||||
return err
|
||||
}
|
||||
|
||||
func DeleteUser(orgID, userID string) error {
|
||||
target, err := GetUserInOrg(orgID, userID)
|
||||
if err != nil {
|
||||
return fmt.Errorf("user not found")
|
||||
}
|
||||
if target.Role == models.RoleOwner {
|
||||
others, err := countOtherOwners(orgID, userID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if others == 0 {
|
||||
return ErrLastOwner
|
||||
}
|
||||
}
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||
defer cancel()
|
||||
_, err := db.Col("users").DeleteOne(ctx, bson.M{"user_id": userID, "org_id": orgID})
|
||||
_, err = db.Col("users").DeleteOne(ctx, bson.M{"user_id": userID, "org_id": orgID})
|
||||
return err
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user