From f1c3f678649cdc37cc9ac852e18a2502da52f6cc Mon Sep 17 00:00:00 2001 From: mrhid6 Date: Mon, 3 Aug 2026 10:45:23 +0100 Subject: [PATCH] feat: per-provider SSO start and callback routes --- server/internal/api/handlers.go | 4 +- server/internal/api/instance.go | 2 +- server/internal/auth/github.go | 20 ++++ server/internal/auth/oidc.go | 186 +++++++++++++++++++++----------- 4 files changed, 148 insertions(+), 64 deletions(-) create mode 100644 server/internal/auth/github.go diff --git a/server/internal/api/handlers.go b/server/internal/api/handlers.go index 3cbcad1..ff4972f 100644 --- a/server/internal/api/handlers.go +++ b/server/internal/api/handlers.go @@ -36,8 +36,8 @@ func RegisterRoutes(r *gin.Engine) { r.POST("/auth/login", auth.HandleLocalLogin) r.POST("/auth/logout", auth.HandleLogout) r.GET("/auth/me", auth.HandleMe) - r.GET("/auth/oidc/start", auth.HandleOIDCStart) - r.GET("/auth/oidc/callback", auth.HandleOIDCCallback) + r.GET("/auth/oidc/:providerId/start", auth.HandleSSOStart) + r.GET("/auth/oidc/:providerId/callback", auth.HandleSSOCallback) apiGroup := r.Group("/api") apiGroup.Use(auth.Middleware()) diff --git a/server/internal/api/instance.go b/server/internal/api/instance.go index 6e953ba..92a1236 100644 --- a/server/internal/api/instance.go +++ b/server/internal/api/instance.go @@ -152,6 +152,6 @@ func putInstanceOIDC(c *gin.Context) { return } - auth.EvictOIDCProvider(auth.InstanceID(c)) + auth.EvictProvider(auth.InstanceID(c)) c.JSON(http.StatusOK, gin.H{"saved": true}) } diff --git a/server/internal/auth/github.go b/server/internal/auth/github.go new file mode 100644 index 0000000..d8284f2 --- /dev/null +++ b/server/internal/auth/github.go @@ -0,0 +1,20 @@ +package auth + +import ( + "context" + "errors" + + "gitea.hostxtra.co.uk/mrhid6/vantage/server/internal/models" + "golang.org/x/oauth2" +) + +// githubOAuthConfig and githubIdentity are temporary stubs. Task 7 replaces +// this file with the real GitHub OAuth2 provider implementation. + +func githubOAuthConfig(p *models.AuthProvider, secret, redirectURL string) *oauth2.Config { + return nil +} + +func githubIdentity(ctx context.Context, cfg *oauth2.Config, token *oauth2.Token) (string, string, error) { + return "", "", errors.New("not implemented") +} diff --git a/server/internal/auth/oidc.go b/server/internal/auth/oidc.go index fd64f8c..088d6b0 100644 --- a/server/internal/auth/oidc.go +++ b/server/internal/auth/oidc.go @@ -19,52 +19,76 @@ var ( provCache = map[string]*oidc.Provider{} ) -func EvictOIDCProvider(instanceID string) { +// EvictProvider drops a cached discovery document. Keyed on provider, not +// instance: an instance now has several, and evicting all of them because one +// changed would re-fetch discovery for providers nobody touched. +func EvictProvider(providerID string) { provMu.Lock() - delete(provCache, instanceID) + delete(provCache, providerID) provMu.Unlock() } -func redirectURL(c *gin.Context) string { - scheme := "https" +func requestScheme(c *gin.Context) string { if c.Request.TLS == nil && c.GetHeader("X-Forwarded-Proto") != "https" { - scheme = "http" + return "http" } - return fmt.Sprintf("%s://%s/auth/oidc/callback", scheme, c.Request.Host) + return "https" } -func providerForInstance(ctx context.Context, c *gin.Context, instanceID string) (*oidc.Provider, *oauth2.Config, error) { - cfg, err := services.GetInstanceOIDC(instanceID) - if err != nil || !cfg.Enabled { - return nil, nil, fmt.Errorf("inst SSO not configured") - } - secret, err := services.GetInstanceOIDCSecret(instanceID) - if err != nil { - return nil, nil, err +// CallbackURL must return the same string in the start and callback halves of +// one flow, or the identity provider rejects the token exchange. +func CallbackURL(c *gin.Context, providerID string) string { + return fmt.Sprintf("%s://%s/auth/oidc/%s/callback", requestScheme(c), c.Request.Host, providerID) +} + +func oauthConfigFor(ctx context.Context, c *gin.Context, p *models.AuthProvider, secret string) (*oidc.Provider, *oauth2.Config, error) { + if p.Kind == models.KindOAuth2 { + return nil, githubOAuthConfig(p, secret, CallbackURL(c, p.ProviderID)), nil } provMu.Lock() - p := provCache[instanceID] + prov := provCache[p.ProviderID] provMu.Unlock() - if p == nil { - p, err = oidc.NewProvider(ctx, cfg.Issuer) + if prov == nil { + var err error + prov, err = oidc.NewProvider(ctx, p.Issuer) if err != nil { - return nil, nil, err + return nil, nil, fmt.Errorf("provider discovery failed: %w", err) } provMu.Lock() - provCache[instanceID] = p + provCache[p.ProviderID] = prov provMu.Unlock() } - return p, &oauth2.Config{ - ClientID: cfg.ClientID, ClientSecret: secret, - RedirectURL: redirectURL(c), Endpoint: p.Endpoint(), - Scopes: []string{oidc.ScopeOpenID, "profile", "email"}, + return prov, &oauth2.Config{ + ClientID: p.ClientID, + ClientSecret: secret, + RedirectURL: CallbackURL(c, p.ProviderID), + Endpoint: prov.Endpoint(), + Scopes: p.Scopes, }, nil } -func HandleOIDCStart(c *gin.Context) { +// loadProvider resolves a provider strictly within one instance. There is no +// unscoped lookup: a provider ID travels in a URL, and an unscoped one would +// let a request against one instance's host drive another instance's provider. +func loadProvider(instanceID, providerID string) (*models.AuthProvider, string, error) { + p, err := services.GetAuthProvider(instanceID, providerID) + if err != nil { + return nil, "", fmt.Errorf("unknown provider") + } + if !p.Enabled { + return nil, "", fmt.Errorf("provider is disabled") + } + secret, err := services.GetAuthProviderSecret(instanceID, providerID) + if err != nil { + return nil, "", err + } + return p, secret, nil +} + +func HandleSSOStart(c *gin.Context) { inst, ok := InstanceFromHost(c) if !ok { - c.JSON(http.StatusBadRequest, gin.H{"error": "unknown instance host"}) + c.Redirect(http.StatusFound, "/login?error=unknown_host") return } // Losing the feature stops new SSO logins. It deliberately does not touch @@ -73,32 +97,46 @@ func HandleOIDCStart(c *gin.Context) { c.Redirect(http.StatusFound, "/login?error=oidc_unavailable") return } - ctx := c.Request.Context() - _, oauthCfg, err := providerForInstance(ctx, c, inst.InstanceID) + providerID := c.Param("providerId") + p, secret, err := loadProvider(inst.InstanceID, providerID) if err != nil { - c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + c.Redirect(http.StatusFound, "/login?error=provider_unavailable") + return + } + ctx := c.Request.Context() + _, oauthCfg, err := oauthConfigFor(ctx, c, p, secret) + if err != nil { + c.Redirect(http.StatusFound, "/login?error=provider_unreachable") return } state, err := randomHex(16) if err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": "state gen failed"}) + c.Redirect(http.StatusFound, "/login?error=state_failed") return } - if err := saveState(ctx, state, oidcState{InstanceID: inst.InstanceID, ProviderID: ""}); err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": "state save failed"}) + if err := saveState(ctx, state, oidcState{InstanceID: inst.InstanceID, ProviderID: p.ProviderID}); err != nil { + c.Redirect(http.StatusFound, "/login?error=state_failed") return } c.Redirect(http.StatusFound, oauthCfg.AuthCodeURL(state)) } -func HandleOIDCCallback(c *gin.Context) { +func HandleSSOCallback(c *gin.Context) { ctx := c.Request.Context() st, ok := consumeState(ctx, c.Query("state")) if !ok { - c.JSON(http.StatusBadRequest, gin.H{"error": "invalid state"}) + c.Redirect(http.StatusFound, "/login?error=invalid_state") + return + } + + // The path segment is attacker-controlled; the state was issued by the start + // handler. A mismatch means the two halves of this flow disagree about which + // provider is signing somebody in, and that is not a thing to resolve by + // picking one. The state has already been consumed, so this is not replayable. + if c.Param("providerId") != st.ProviderID { + c.Redirect(http.StatusFound, "/login?error=invalid_state") return } - instanceID := st.InstanceID // The start handler checks this too, but an ungated callback is the half // that matters: a start that refuses is a dead end, while a callback that @@ -108,41 +146,67 @@ func HandleOIDCCallback(c *gin.Context) { // Resolved from the consumed state rather than from the host, because on // this route the instance is whatever the state said and nobody is signed // in yet. - if !services.GetLicenseState(instanceID).Feature("oidc") { + if !services.GetLicenseState(st.InstanceID).Feature("oidc") { c.Redirect(http.StatusFound, "/login?error=oidc_unavailable") return } - provider, oauthCfg, err := providerForInstance(ctx, c, instanceID) + p, secret, err := loadProvider(st.InstanceID, st.ProviderID) if err != nil { - c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + c.Redirect(http.StatusFound, "/login?error=provider_unavailable") + return + } + provider, oauthCfg, err := oauthConfigFor(ctx, c, p, secret) + if err != nil { + c.Redirect(http.StatusFound, "/login?error=provider_unreachable") return } token, err := oauthCfg.Exchange(ctx, c.Query("code")) if err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": "token exchange failed"}) - return - } - rawIDToken, ok := token.Extra("id_token").(string) - if !ok { - c.JSON(http.StatusInternalServerError, gin.H{"error": "missing id_token"}) - return - } - 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 - } - var claims struct { - Email string `json:"email"` - Name string `json:"name"` - } - if err := idToken.Claims(&claims); err != nil || claims.Email == "" { - c.JSON(http.StatusInternalServerError, gin.H{"error": "claims extraction failed"}) + c.Redirect(http.StatusFound, "/login?error=exchange_failed") return } - email := strings.ToLower(claims.Email) + var email, name string + if p.Kind == models.KindOAuth2 { + email, name, err = githubIdentity(ctx, oauthCfg, token) + if err != nil { + c.Redirect(http.StatusFound, "/login?error=identity_failed") + return + } + } else { + rawIDToken, ok := token.Extra("id_token").(string) + if !ok { + c.Redirect(http.StatusFound, "/login?error=missing_id_token") + return + } + idToken, err := provider.Verifier(&oidc.Config{ClientID: oauthCfg.ClientID}).Verify(ctx, rawIDToken) + if err != nil { + c.Redirect(http.StatusFound, "/login?error=verification_failed") + return + } + var claims struct { + Email string `json:"email"` + Name string `json:"name"` + } + if err := idToken.Claims(&claims); err != nil || claims.Email == "" { + c.Redirect(http.StatusFound, "/login?error=missing_email") + return + } + email, name = claims.Email, claims.Name + } + + completeSSOLogin(c, st.InstanceID, email, name) +} + +// completeSSOLogin is the tail both provider kinds share: resolve the user +// within the instance, provision on first sign-in, mint the session. +func completeSSOLogin(c *gin.Context, instanceID, email, name string) { + email = strings.ToLower(strings.TrimSpace(email)) + if email == "" { + c.Redirect(http.StatusFound, "/login?error=missing_email") + return + } // Scoped to the instance the callback state names, so an address that also // exists in another instance is invisible here. That scoping replaces the @@ -152,16 +216,16 @@ func HandleOIDCCallback(c *gin.Context) { 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"}) + c.Redirect(http.StatusFound, "/login?error=provisioning_failed") return } } - sessionID, err := SaveSession(ctx, &Session{ - UserID: u.UserID, InstanceID: u.InstanceID, Role: u.Role, Email: u.Email, Name: claims.Name, + sessionID, err := SaveSession(c.Request.Context(), &Session{ + UserID: u.UserID, InstanceID: u.InstanceID, Role: u.Role, Email: u.Email, Name: name, }) if err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": "session save failed"}) + c.Redirect(http.StatusFound, "/login?error=session_failed") return } _ = services.TouchLastLogin(u.UserID)