fix: repair migration collection names and cross-cutting scoping gaps
Findings from the final whole-branch review. - scopedCollections named "audit" and "channels", but the code writes to audit_logs and notification_channels. On upgrade from single-tenant, legacy audit events and channels would never get org_id, becoming invisible to org-filtered reads while channels silently stopped firing — and the detection loop counted the wrong names, so the 0001 marker could be written having migrated nothing. Names fixed, plus migration 0003 so an incorrectly-migrated instance converges with a fresh one. - EnsureAuthIndexes failure is now fatal. GetUserByEmail is unscoped and the OIDC cross-org guard compares against whichever duplicate Mongo returns first, so users.email uniqueness is a security invariant, and a legacy collection with duplicate emails is the realistic upgrade case. - Evict the per-org OIDC provider cache on save; rotating away from a compromised IdP previously had no effect until restart. - Build the oauth2 config per request instead of mutating a shared cached pointer outside the mutex, which raced on RedirectURL between concurrent logins for the same org. - Stamp org_id on console_sessions, incidents and monitor_rollups, the last collections with no tenant column. 0003 derives their org from the owning server/monitor rather than defaulting, so one org's console history and incident timeline cannot merge into another's. - Seed default steps when an org is created, not only at boot. - Reject an empty session OrgID at the middleware. - Derive the app root label from APP_ROOT_LABEL instead of hardcoding "vantage", which silently disabled the host guard off that domain. - Stop caching negative slug lookups, so a new org's subdomain resolves immediately.
This commit is contained in:
@@ -62,6 +62,13 @@ func Middleware() gin.HandlerFunc {
|
||||
return
|
||||
}
|
||||
|
||||
// An org-less session would turn every downstream scope into
|
||||
// {"org_id": ""} — fail closed rather than query across tenants.
|
||||
if sess.OrgID == "" {
|
||||
c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": "session has no organization"})
|
||||
return
|
||||
}
|
||||
|
||||
c.Set(ctxSessionKey, sess)
|
||||
|
||||
if hostOrg, ok := OrgFromHost(c); ok && hostOrg.OrgID != sess.OrgID {
|
||||
|
||||
@@ -13,16 +13,20 @@ import (
|
||||
"golang.org/x/oauth2"
|
||||
)
|
||||
|
||||
type orgProvider struct {
|
||||
provider *oidc.Provider
|
||||
oauth *oauth2.Config
|
||||
}
|
||||
|
||||
var (
|
||||
provMu sync.Mutex
|
||||
provCache = map[string]*orgProvider{}
|
||||
provCache = map[string]*oidc.Provider{}
|
||||
)
|
||||
|
||||
// EvictOIDCProvider drops an org's cached provider so the next login rediscovers
|
||||
// it from the (possibly changed) issuer. Called by the API layer after the org's
|
||||
// OIDC config is saved — services cannot import auth, so the handler wires it.
|
||||
func EvictOIDCProvider(orgID string) {
|
||||
provMu.Lock()
|
||||
delete(provCache, orgID)
|
||||
provMu.Unlock()
|
||||
}
|
||||
|
||||
func redirectURL(c *gin.Context) string {
|
||||
scheme := "https"
|
||||
if c.Request.TLS == nil && c.GetHeader("X-Forwarded-Proto") != "https" {
|
||||
@@ -31,34 +35,36 @@ func redirectURL(c *gin.Context) string {
|
||||
return fmt.Sprintf("%s://%s/auth/oidc/callback", scheme, c.Request.Host)
|
||||
}
|
||||
|
||||
func providerForOrg(ctx context.Context, c *gin.Context, orgID string) (*orgProvider, error) {
|
||||
// providerForOrg returns the org's (cached) OIDC provider plus a request-local
|
||||
// oauth2 config. The config is never stored on the cached entry: RedirectURL is
|
||||
// derived from this request's Host, so sharing it would let one in-flight login
|
||||
// overwrite another's redirect URI.
|
||||
func providerForOrg(ctx context.Context, c *gin.Context, orgID string) (*oidc.Provider, *oauth2.Config, error) {
|
||||
cfg, err := services.GetOrgOIDC(orgID)
|
||||
if err != nil || !cfg.Enabled {
|
||||
return nil, fmt.Errorf("org SSO not configured")
|
||||
return nil, nil, fmt.Errorf("org SSO not configured")
|
||||
}
|
||||
secret, err := services.GetOrgOIDCSecret(orgID)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
return nil, nil, err
|
||||
}
|
||||
provMu.Lock()
|
||||
op := provCache[orgID]
|
||||
p := provCache[orgID]
|
||||
provMu.Unlock()
|
||||
if op == nil || op.provider == nil {
|
||||
p, err := oidc.NewProvider(ctx, cfg.Issuer)
|
||||
if p == nil {
|
||||
p, err = oidc.NewProvider(ctx, cfg.Issuer)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
return nil, nil, err
|
||||
}
|
||||
op = &orgProvider{provider: p}
|
||||
provMu.Lock()
|
||||
provCache[orgID] = op
|
||||
provCache[orgID] = p
|
||||
provMu.Unlock()
|
||||
}
|
||||
op.oauth = &oauth2.Config{
|
||||
return p, &oauth2.Config{
|
||||
ClientID: cfg.ClientID, ClientSecret: secret,
|
||||
RedirectURL: redirectURL(c), Endpoint: op.provider.Endpoint(),
|
||||
RedirectURL: redirectURL(c), Endpoint: p.Endpoint(),
|
||||
Scopes: []string{oidc.ScopeOpenID, "profile", "email"},
|
||||
}
|
||||
return op, nil
|
||||
}, nil
|
||||
}
|
||||
|
||||
func HandleOIDCStart(c *gin.Context) {
|
||||
@@ -68,7 +74,7 @@ func HandleOIDCStart(c *gin.Context) {
|
||||
return
|
||||
}
|
||||
ctx := c.Request.Context()
|
||||
op, err := providerForOrg(ctx, c, org.OrgID)
|
||||
_, oauthCfg, err := providerForOrg(ctx, c, org.OrgID)
|
||||
if err != nil {
|
||||
c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
|
||||
return
|
||||
@@ -82,7 +88,7 @@ func HandleOIDCStart(c *gin.Context) {
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": "state save failed"})
|
||||
return
|
||||
}
|
||||
c.Redirect(http.StatusFound, op.oauth.AuthCodeURL(state))
|
||||
c.Redirect(http.StatusFound, oauthCfg.AuthCodeURL(state))
|
||||
}
|
||||
|
||||
func HandleOIDCCallback(c *gin.Context) {
|
||||
@@ -92,12 +98,12 @@ func HandleOIDCCallback(c *gin.Context) {
|
||||
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid state"})
|
||||
return
|
||||
}
|
||||
op, err := providerForOrg(ctx, c, orgID)
|
||||
provider, oauthCfg, err := providerForOrg(ctx, c, orgID)
|
||||
if err != nil {
|
||||
c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
|
||||
return
|
||||
}
|
||||
token, err := op.oauth.Exchange(ctx, c.Query("code"))
|
||||
token, err := oauthCfg.Exchange(ctx, c.Query("code"))
|
||||
if err != nil {
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": "token exchange failed"})
|
||||
return
|
||||
@@ -107,7 +113,7 @@ func HandleOIDCCallback(c *gin.Context) {
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": "missing id_token"})
|
||||
return
|
||||
}
|
||||
idToken, err := op.provider.Verifier(&oidc.Config{ClientID: op.oauth.ClientID}).Verify(ctx, rawIDToken)
|
||||
idToken, err := provider.Verifier(&oidc.Config{ClientID: oauthCfg.ClientID}).Verify(ctx, rawIDToken)
|
||||
if err != nil {
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": "token verification failed"})
|
||||
return
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package auth
|
||||
|
||||
import (
|
||||
"os"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
@@ -22,6 +23,16 @@ var (
|
||||
|
||||
const orgCacheTTL = 60 * time.Second
|
||||
|
||||
// appRootLabel is the DNS label the app is deployed under, i.e. the "vantage"
|
||||
// in <slug>.vantage.<tld>. Deployments on another root must set APP_ROOT_LABEL
|
||||
// or every host resolves to no org, disabling the host/session mismatch guard.
|
||||
func appRootLabel() string {
|
||||
if v := os.Getenv("APP_ROOT_LABEL"); v != "" {
|
||||
return strings.ToLower(v)
|
||||
}
|
||||
return "vantage"
|
||||
}
|
||||
|
||||
// hostSlug extracts the leftmost DNS label if the host is a subdomain of the
|
||||
// app root. Returns "" for the apex or an unknown host shape.
|
||||
func hostSlug(host string) string {
|
||||
@@ -29,15 +40,16 @@ func hostSlug(host string) string {
|
||||
if i := strings.IndexByte(host, ':'); i >= 0 {
|
||||
host = host[:i]
|
||||
}
|
||||
// Expect <slug>.vantage.<...>; apex is vantage.<...>
|
||||
root := appRootLabel()
|
||||
// Expect <slug>.<root>.<...>; apex is <root>.<...>
|
||||
parts := strings.Split(host, ".")
|
||||
if len(parts) < 3 {
|
||||
return ""
|
||||
}
|
||||
if parts[1] != "vantage" {
|
||||
if parts[1] != root {
|
||||
return ""
|
||||
}
|
||||
if parts[0] == "vantage" || parts[0] == "www" {
|
||||
if parts[0] == root || parts[0] == "www" {
|
||||
return ""
|
||||
}
|
||||
return parts[0]
|
||||
@@ -56,11 +68,13 @@ func OrgFromHost(c *gin.Context) (*models.Org, bool) {
|
||||
orgCacheMu.Unlock()
|
||||
|
||||
org, err := services.GetOrgBySlug(slug)
|
||||
if err != nil {
|
||||
org = nil
|
||||
if err != nil || org == nil {
|
||||
// Never cache a miss: a just-bootstrapped org would otherwise 404 on its
|
||||
// own subdomain for the rest of the TTL. Misses are cheap and rare.
|
||||
return nil, false
|
||||
}
|
||||
orgCacheMu.Lock()
|
||||
orgCache[slug] = cachedOrg{org: org, at: time.Now()}
|
||||
orgCacheMu.Unlock()
|
||||
return org, org != nil
|
||||
return org, true
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user