From b86287477b503c96b4b746aa7156d2965b145f42 Mon Sep 17 00:00:00 2001 From: Ambient Code Bot Date: Thu, 12 Mar 2026 19:02:07 +0000 Subject: [PATCH 1/3] fix(backend): resolve userID from SA annotation for API key auth API keys (ServiceAccount JWTs) couldn't access user integrations because userID was only read from OAuth proxy headers. Now store creating user's ID as SA annotation and read it in middleware fallback. Fixes: RHOAIENG-52967 Co-Authored-By: Claude Opus 4.6 --- components/backend/handlers/permissions.go | 9 +-- components/backend/server/server.go | 73 ++++++++++++++++++++++ 2 files changed, 78 insertions(+), 4 deletions(-) mode change 100644 => 100755 components/backend/handlers/permissions.go mode change 100644 => 100755 components/backend/server/server.go diff --git a/components/backend/handlers/permissions.go b/components/backend/handlers/permissions.go old mode 100644 new mode 100755 index 44cd404ce..9565917f9 --- a/components/backend/handlers/permissions.go +++ b/components/backend/handlers/permissions.go @@ -390,10 +390,11 @@ func CreateProjectKey(c *gin.Context) { Namespace: projectName, Labels: map[string]string{"app": "ambient-access-key"}, Annotations: map[string]string{ - "ambient-code.io/key-name": req.Name, - "ambient-code.io/description": req.Description, - "ambient-code.io/created-at": time.Now().Format(time.RFC3339), - "ambient-code.io/role": role, + "ambient-code.io/key-name": req.Name, + "ambient-code.io/description": req.Description, + "ambient-code.io/created-at": time.Now().Format(time.RFC3339), + "ambient-code.io/role": role, + "ambient-code.io/created-by-user-id": c.GetString("userID"), }, }, } diff --git a/components/backend/server/server.go b/components/backend/server/server.go old mode 100644 new mode 100755 index 7739118fc..44bc162dd --- a/components/backend/server/server.go +++ b/components/backend/server/server.go @@ -2,6 +2,8 @@ package server import ( + "encoding/base64" + "encoding/json" "fmt" "log" "os" @@ -9,6 +11,7 @@ import ( "github.com/gin-contrib/cors" "github.com/gin-gonic/gin" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // RouterFunc is a function that can register routes on a Gin router @@ -158,6 +161,76 @@ func forwardedIdentityMiddleware() gin.HandlerFunc { if v := c.GetHeader("X-Forwarded-Access-Token"); v != "" { c.Set("forwardedAccessToken", v) } + + // Fallback: if userID is still empty, try to resolve it from the + // ServiceAccount's created-by-user-id annotation. This enables API + // key-authenticated requests to inherit the creating user's identity + // so that integration credentials (GitHub, Jira, GitLab) are accessible. + if c.GetString("userID") == "" { + if ns, saName, ok := extractServiceAccountFromBearer(c); ok && K8sClient != nil { + sa, err := K8sClient.CoreV1().ServiceAccounts(ns).Get(c.Request.Context(), saName, v1.GetOptions{}) + if err == nil && sa.Annotations != nil { + if uid := sa.Annotations["ambient-code.io/created-by-user-id"]; uid != "" { + c.Set("userID", uid) + } + } + } + } + c.Next() } } + +// extractServiceAccountFromBearer extracts namespace and SA name from a Bearer +// JWT's "sub" claim (format: "system:serviceaccount::"). +// Also checks the X-Remote-User header for the same format. +func extractServiceAccountFromBearer(c *gin.Context) (string, string, bool) { + // Check X-Remote-User header (OpenShift OAuth proxy format) + if remoteUser := c.GetHeader("X-Remote-User"); remoteUser != "" { + const prefix = "system:serviceaccount:" + if strings.HasPrefix(remoteUser, prefix) { + parts := strings.SplitN(strings.TrimPrefix(remoteUser, prefix), ":", 2) + if len(parts) == 2 { + return parts[0], parts[1], true + } + } + } + + // Standard Authorization Bearer JWT parsing + rawAuth := c.GetHeader("Authorization") + parts := strings.SplitN(rawAuth, " ", 2) + if len(parts) != 2 || !strings.EqualFold(parts[0], "Bearer") { + return "", "", false + } + token := strings.TrimSpace(parts[1]) + if token == "" { + return "", "", false + } + segs := strings.Split(token, ".") + if len(segs) < 2 { + return "", "", false + } + payloadB64 := segs[1] + if m := len(payloadB64) % 4; m != 0 { + payloadB64 += strings.Repeat("=", 4-m) + } + data, err := base64.URLEncoding.DecodeString(payloadB64) + if err != nil { + return "", "", false + } + var payload map[string]interface{} + if err := json.Unmarshal(data, &payload); err != nil { + return "", "", false + } + sub, _ := payload["sub"].(string) + const prefix = "system:serviceaccount:" + if !strings.HasPrefix(sub, prefix) { + return "", "", false + } + rest := strings.TrimPrefix(sub, prefix) + parts2 := strings.SplitN(rest, ":", 2) + if len(parts2) != 2 { + return "", "", false + } + return parts2[0], parts2[1], true +} From 33c6e2892c985e7f5d07b696a5d51a8fcc23190a Mon Sep 17 00:00:00 2001 From: Gage Krumbach Date: Thu, 12 Mar 2026 15:59:29 -0500 Subject: [PATCH 2/3] refactor(backend): deduplicate ExtractServiceAccountFromAuth into server package Move the canonical implementation to server.ExtractServiceAccountFromAuth (where it is needed by forwardedIdentityMiddleware) and keep a thin forwarding wrapper in handlers for backward compatibility. This eliminates the duplicate extractServiceAccountFromBearer that was introduced in the PR. Co-Authored-By: Claude Opus 4.6 (1M context) --- components/backend/handlers/middleware.go | 59 ++--------------------- components/backend/server/server.go | 10 ++-- 2 files changed, 10 insertions(+), 59 deletions(-) diff --git a/components/backend/handlers/middleware.go b/components/backend/handlers/middleware.go index d810c175b..f4c24165a 100644 --- a/components/backend/handlers/middleware.go +++ b/components/backend/handlers/middleware.go @@ -9,6 +9,8 @@ import ( "strings" "time" + "ambient-code-backend/server" + "github.com/gin-gonic/gin" authv1 "k8s.io/api/authorization/v1" "k8s.io/apimachinery/pkg/api/errors" @@ -261,61 +263,10 @@ func updateAccessKeyLastUsedAnnotation(c *gin.Context) { } } -// ExtractServiceAccountFromAuth extracts namespace and ServiceAccount name from the Authorization Bearer JWT 'sub' claim -// Also checks X-Remote-User header for service account format (OpenShift OAuth proxy format) -// Returns (namespace, saName, true) when a SA subject is present, otherwise ("","",false) +// ExtractServiceAccountFromAuth delegates to server.ExtractServiceAccountFromAuth. +// Kept as a forwarding function for backward compatibility with callers in this package. func ExtractServiceAccountFromAuth(c *gin.Context) (string, string, bool) { - // Check X-Remote-User header (OpenShift OAuth proxy format) - // This is a production feature, not just for tests - remoteUser := c.GetHeader("X-Remote-User") - if remoteUser != "" { - const prefix = "system:serviceaccount:" - if strings.HasPrefix(remoteUser, prefix) { - rest := strings.TrimPrefix(remoteUser, prefix) - parts := strings.SplitN(rest, ":", 2) - if len(parts) == 2 { - return parts[0], parts[1], true - } - } - } - - // Standard Authorization Bearer JWT parsing - rawAuth := c.GetHeader("Authorization") - parts := strings.SplitN(rawAuth, " ", 2) - if len(parts) != 2 || !strings.EqualFold(parts[0], "Bearer") { - return "", "", false - } - token := strings.TrimSpace(parts[1]) - if token == "" { - return "", "", false - } - segs := strings.Split(token, ".") - if len(segs) < 2 { - return "", "", false - } - payloadB64 := segs[1] - if m := len(payloadB64) % 4; m != 0 { - payloadB64 += strings.Repeat("=", 4-m) - } - data, err := base64.URLEncoding.DecodeString(payloadB64) - if err != nil { - return "", "", false - } - var payload map[string]interface{} - if err := json.Unmarshal(data, &payload); err != nil { - return "", "", false - } - sub, _ := payload["sub"].(string) - const prefix = "system:serviceaccount:" - if !strings.HasPrefix(sub, prefix) { - return "", "", false - } - rest := strings.TrimPrefix(sub, prefix) - parts2 := strings.SplitN(rest, ":", 2) - if len(parts2) != 2 { - return "", "", false - } - return parts2[0], parts2[1], true + return server.ExtractServiceAccountFromAuth(c) } // ValidateProjectContext is middleware for project context validation diff --git a/components/backend/server/server.go b/components/backend/server/server.go index 44bc162dd..beee788f5 100755 --- a/components/backend/server/server.go +++ b/components/backend/server/server.go @@ -167,7 +167,7 @@ func forwardedIdentityMiddleware() gin.HandlerFunc { // key-authenticated requests to inherit the creating user's identity // so that integration credentials (GitHub, Jira, GitLab) are accessible. if c.GetString("userID") == "" { - if ns, saName, ok := extractServiceAccountFromBearer(c); ok && K8sClient != nil { + if ns, saName, ok := ExtractServiceAccountFromAuth(c); ok && K8sClient != nil { sa, err := K8sClient.CoreV1().ServiceAccounts(ns).Get(c.Request.Context(), saName, v1.GetOptions{}) if err == nil && sa.Annotations != nil { if uid := sa.Annotations["ambient-code.io/created-by-user-id"]; uid != "" { @@ -181,10 +181,10 @@ func forwardedIdentityMiddleware() gin.HandlerFunc { } } -// extractServiceAccountFromBearer extracts namespace and SA name from a Bearer -// JWT's "sub" claim (format: "system:serviceaccount::"). -// Also checks the X-Remote-User header for the same format. -func extractServiceAccountFromBearer(c *gin.Context) (string, string, bool) { +// ExtractServiceAccountFromAuth extracts namespace and ServiceAccount name from the Authorization Bearer JWT 'sub' claim. +// Also checks X-Remote-User header for service account format (OpenShift OAuth proxy format). +// Returns (namespace, saName, true) when a SA subject is present, otherwise ("","",false). +func ExtractServiceAccountFromAuth(c *gin.Context) (string, string, bool) { // Check X-Remote-User header (OpenShift OAuth proxy format) if remoteUser := c.GetHeader("X-Remote-User"); remoteUser != "" { const prefix = "system:serviceaccount:" From 1237c27de075127dc0d9e75442b8c8978ab5f5c3 Mon Sep 17 00:00:00 2001 From: Gage Krumbach Date: Thu, 12 Mar 2026 23:09:05 -0500 Subject: [PATCH 3/3] fix(backend): use TokenReview for SA identity, reject empty userID on key creation Replace insecure raw JWT parsing with K8s TokenReview API to verify the bearer token before trusting the ServiceAccount identity. This prevents forged tokens from spoofing SA annotations. Also reject key creation when userID is empty to avoid persisting keys that can never resolve integration credentials. Co-Authored-By: Claude Opus 4.6 (1M context) --- components/backend/handlers/permissions.go | 9 ++- components/backend/server/server.go | 81 ++++++++++------------ 2 files changed, 46 insertions(+), 44 deletions(-) diff --git a/components/backend/handlers/permissions.go b/components/backend/handlers/permissions.go index 9565917f9..38c63005a 100755 --- a/components/backend/handlers/permissions.go +++ b/components/backend/handlers/permissions.go @@ -381,6 +381,13 @@ func CreateProjectKey(c *gin.Context) { return } + // Require authenticated user identity so integration access works + creatorUserID := c.GetString("userID") + if creatorUserID == "" { + c.JSON(http.StatusBadRequest, gin.H{"error": "User identity required to create access keys"}) + return + } + // Create a dedicated ServiceAccount per key ts := time.Now().Unix() saName := fmt.Sprintf("ambient-key-%s-%d", sanitizeName(req.Name), ts) @@ -394,7 +401,7 @@ func CreateProjectKey(c *gin.Context) { "ambient-code.io/description": req.Description, "ambient-code.io/created-at": time.Now().Format(time.RFC3339), "ambient-code.io/role": role, - "ambient-code.io/created-by-user-id": c.GetString("userID"), + "ambient-code.io/created-by-user-id": creatorUserID, }, }, } diff --git a/components/backend/server/server.go b/components/backend/server/server.go index beee788f5..1bb440e5a 100755 --- a/components/backend/server/server.go +++ b/components/backend/server/server.go @@ -2,8 +2,6 @@ package server import ( - "encoding/base64" - "encoding/json" "fmt" "log" "os" @@ -11,6 +9,7 @@ import ( "github.com/gin-contrib/cors" "github.com/gin-gonic/gin" + authnv1 "k8s.io/api/authentication/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -162,12 +161,13 @@ func forwardedIdentityMiddleware() gin.HandlerFunc { c.Set("forwardedAccessToken", v) } - // Fallback: if userID is still empty, try to resolve it from the - // ServiceAccount's created-by-user-id annotation. This enables API - // key-authenticated requests to inherit the creating user's identity + // Fallback: if userID is still empty, verify the Bearer token via + // TokenReview to securely resolve the ServiceAccount identity, then + // read the created-by-user-id annotation. This enables API key- + // authenticated requests to inherit the creating user's identity // so that integration credentials (GitHub, Jira, GitLab) are accessible. - if c.GetString("userID") == "" { - if ns, saName, ok := ExtractServiceAccountFromAuth(c); ok && K8sClient != nil { + if c.GetString("userID") == "" && K8sClient != nil { + if ns, saName, ok := resolveServiceAccountFromToken(c); ok { sa, err := K8sClient.CoreV1().ServiceAccounts(ns).Get(c.Request.Context(), saName, v1.GetOptions{}) if err == nil && sa.Annotations != nil { if uid := sa.Annotations["ambient-code.io/created-by-user-id"]; uid != "" { @@ -181,22 +181,10 @@ func forwardedIdentityMiddleware() gin.HandlerFunc { } } -// ExtractServiceAccountFromAuth extracts namespace and ServiceAccount name from the Authorization Bearer JWT 'sub' claim. -// Also checks X-Remote-User header for service account format (OpenShift OAuth proxy format). -// Returns (namespace, saName, true) when a SA subject is present, otherwise ("","",false). -func ExtractServiceAccountFromAuth(c *gin.Context) (string, string, bool) { - // Check X-Remote-User header (OpenShift OAuth proxy format) - if remoteUser := c.GetHeader("X-Remote-User"); remoteUser != "" { - const prefix = "system:serviceaccount:" - if strings.HasPrefix(remoteUser, prefix) { - parts := strings.SplitN(strings.TrimPrefix(remoteUser, prefix), ":", 2) - if len(parts) == 2 { - return parts[0], parts[1], true - } - } - } - - // Standard Authorization Bearer JWT parsing +// resolveServiceAccountFromToken verifies the Bearer token via K8s TokenReview +// and extracts the ServiceAccount namespace and name from the authenticated identity. +// Returns (namespace, saName, true) when verified, otherwise ("","",false). +func resolveServiceAccountFromToken(c *gin.Context) (string, string, bool) { rawAuth := c.GetHeader("Authorization") parts := strings.SplitN(rawAuth, " ", 2) if len(parts) != 2 || !strings.EqualFold(parts[0], "Bearer") { @@ -206,31 +194,38 @@ func ExtractServiceAccountFromAuth(c *gin.Context) (string, string, bool) { if token == "" { return "", "", false } - segs := strings.Split(token, ".") - if len(segs) < 2 { - return "", "", false - } - payloadB64 := segs[1] - if m := len(payloadB64) % 4; m != 0 { - payloadB64 += strings.Repeat("=", 4-m) - } - data, err := base64.URLEncoding.DecodeString(payloadB64) - if err != nil { - return "", "", false - } - var payload map[string]interface{} - if err := json.Unmarshal(data, &payload); err != nil { + + tr := &authnv1.TokenReview{Spec: authnv1.TokenReviewSpec{Token: token}} + rv, err := K8sClient.AuthenticationV1().TokenReviews().Create(c.Request.Context(), tr, v1.CreateOptions{}) + if err != nil || !rv.Status.Authenticated || rv.Status.Error != "" { return "", "", false } - sub, _ := payload["sub"].(string) + + subj := strings.TrimSpace(rv.Status.User.Username) const prefix = "system:serviceaccount:" - if !strings.HasPrefix(sub, prefix) { + if !strings.HasPrefix(subj, prefix) { return "", "", false } - rest := strings.TrimPrefix(sub, prefix) - parts2 := strings.SplitN(rest, ":", 2) - if len(parts2) != 2 { + rest := strings.TrimPrefix(subj, prefix) + segs := strings.SplitN(rest, ":", 2) + if len(segs) != 2 { return "", "", false } - return parts2[0], parts2[1], true + return segs[0], segs[1], true +} + +// ExtractServiceAccountFromAuth extracts namespace and ServiceAccount name +// from the X-Remote-User header (OpenShift OAuth proxy format). +// Returns (namespace, saName, true) when a SA subject is present, otherwise ("","",false). +func ExtractServiceAccountFromAuth(c *gin.Context) (string, string, bool) { + if remoteUser := c.GetHeader("X-Remote-User"); remoteUser != "" { + const prefix = "system:serviceaccount:" + if strings.HasPrefix(remoteUser, prefix) { + parts := strings.SplitN(strings.TrimPrefix(remoteUser, prefix), ":", 2) + if len(parts) == 2 { + return parts[0], parts[1], true + } + } + } + return "", "", false }