diff --git a/server/internal/auth/local.go b/server/internal/auth/local.go index 9d6b513..e0784a9 100644 --- a/server/internal/auth/local.go +++ b/server/internal/auth/local.go @@ -22,6 +22,35 @@ func SetSessionCookie(c *gin.Context, sessionID string) { }) } +// resolveLoginInstance decides which instance a sign-in attempt belongs to. +// +// Cloud always answers from the host: every instance has its own subdomain, and +// APP_LOGIN_URL fills the slug in, so every sign-in link already points at one. +// Self-hosted has no subdomain and exactly one instance, because a licence +// binds one instance UUID. +// +// Anything else is refused rather than guessed. Picking an instance on someone's +// behalf is how you sign them into the wrong tenant. +func resolveLoginInstance(c *gin.Context) (string, error) { + if inst, ok := InstanceFromHost(c); ok { + return inst.InstanceID, nil + } + n, err := services.CountInstances() + if err != nil { + return "", err + } + if n != 1 { + return "", fmt.Errorf( + "cannot tell which instance this sign-in is for: %d instances exist and the host %q names none of them; sign in at your instance's own address", + n, c.Request.Host) + } + inst, err := services.FirstInstance() + if err != nil { + return "", err + } + return inst.InstanceID, nil +} + func HandleLocalLogin(c *gin.Context) { var body struct { Email string `json:"email"` @@ -31,7 +60,12 @@ func HandleLocalLogin(c *gin.Context) { c.JSON(http.StatusBadRequest, gin.H{"error": "email and password required"}) return } - u, err := services.GetUserByEmail(body.Email) + instanceID, err := resolveLoginInstance(c) + if err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + return + } + u, err := services.GetUserInInstanceByEmail(instanceID, body.Email) if err != nil || !services.VerifyPassword(u, body.Password) { c.JSON(http.StatusUnauthorized, gin.H{"error": "invalid credentials"}) return diff --git a/server/internal/auth/oidc.go b/server/internal/auth/oidc.go index c1c470c..f252957 100644 --- a/server/internal/auth/oidc.go +++ b/server/internal/auth/oidc.go @@ -9,6 +9,7 @@ import ( "github.com/coreos/go-oidc/v3/oidc" "github.com/gin-gonic/gin" + "github.com/mrhid6/vantage/server/internal/models" "github.com/mrhid6/vantage/server/internal/services" "golang.org/x/oauth2" ) @@ -127,17 +128,18 @@ func HandleOIDCCallback(c *gin.Context) { } email := strings.ToLower(claims.Email) - u, err := services.GetUserByEmail(email) - if err != nil { - u, err = services.CreateUser(instanceID, email, "", "member", "oidc") + // Scoped to the instance the callback state names, so an address that also + // exists in another instance is invisible here. That scoping replaces the + // cross-instance guard this code used to need: there is no longer a way for + // the lookup to return a user belonging to somebody else. + u, err := services.GetUserInInstanceByEmail(instanceID, email) + if err != nil { + u, err = services.CreateUser(instanceID, email, "", models.RoleMember, models.AuthOIDC) if err != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": "provisioning failed"}) return } - } else if u.InstanceID != instanceID { - c.JSON(http.StatusForbidden, gin.H{"error": "email belongs to a different organization"}) - return } sessionID, err := SaveSession(ctx, &Session{ diff --git a/server/internal/services/users.go b/server/internal/services/users.go index 99b7c67..c040aa2 100644 --- a/server/internal/services/users.go +++ b/server/internal/services/users.go @@ -62,12 +62,21 @@ func CreateUser(instanceID, email, password, role, authSource string) (*models.U return u, err } -func GetUserByEmail(email string) (*models.User, error) { +// GetUserInInstanceByEmail finds a user by address WITHIN one instance. +// +// There is deliberately no unscoped lookup by email. users is unique on +// (instance_id, email), not on email alone, so an unscoped FindOne would return +// an arbitrary one of several matching users — which on the login path means +// signing someone into a tenant that is not theirs. +func GetUserInInstanceByEmail(instanceID, email string) (*models.User, error) { email = strings.ToLower(strings.TrimSpace(email)) ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() var u models.User - err := db.Col("users").FindOne(ctx, bson.M{"email": email}).Decode(&u) + err := db.Col("users").FindOne(ctx, bson.M{ + "instance_id": instanceID, + "email": email, + }).Decode(&u) if err != nil { return nil, err }