feat(server): scope every user lookup by instance
users is unique on (instance_id, email) now, so an unscoped FindOne could return an arbitrary one of several matching users. On the login path that means signing someone into a tenant that is not theirs. GetUserByEmail is deleted rather than left unused. Local sign-in resolves its instance from the host, falling back to the single instance a self-hosted deployment has, and refuses to guess otherwise. The OIDC cross-instance guard goes: a scoped lookup cannot return another instance's user, which is a stronger guarantee than the check it replaces. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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{
|
||||
|
||||
Reference in New Issue
Block a user