Skip to content

fix(backend): API keys access user integrations via SA annotation #901

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Gkrumbach07 merged 3 commits into from
Mar 13, 2026
Merged

fix(backend): API keys access user integrations via SA annotation #901

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b862874
fix(backend): resolve userID from SA annotation for API key auth
Mar 12, 2026
33c6e28
refactor(backend): deduplicate ExtractServiceAccountFromAuth into ser…
Gkrumbach07 Mar 12, 2026
1237c27
fix(backend): use TokenReview for SA identity, reject empty userID on…
Gkrumbach07 Mar 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 5 additions & 54 deletions components/backend/handlers/middleware.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Comment on lines +266 to +269
Copy link

@coderabbitai coderabbitai bot Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Keep bearer-token ServiceAccount extraction in this compatibility wrapper.

server.ExtractServiceAccountFromAuth only handles X-Remote-User, but callers in this package still rely on ExtractServiceAccountFromAuth for bearer-token API-key requests (getUserSubjectFromContext), and the existing handler tests cover that path. After this delegation, those requests no longer round-trip to system:serviceaccount:<ns>:<name> and instead fall back to the creator userID, which changes behavior for any subject-based logic.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@components/backend/handlers/middleware.go` around lines 266 - 269, The
compatibility wrapper ExtractServiceAccountFromAuth should continue to handle
bearer-token API-key serviceAccount extraction instead of delegating entirely to
server.ExtractServiceAccountFromAuth; restore or reimplement the prior
bearer-token logic here by first calling server.ExtractServiceAccountFromAuth to
handle X-Remote-User, and if that returns empty/false then perform the existing
bearer-token extraction (the same path used by getUserSubjectFromContext) to
reconstruct the "system:serviceaccount:<ns>:<name>" subject; update
ExtractServiceAccountFromAuth to return the correct subject and boolean so
callers in this package continue to receive service account subjects.

}

// ValidateProjectContext is middleware for project context validation
Expand Down
16 changes: 12 additions & 4 deletions components/backend/handlers/permissions.go
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -390,10 +397,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": creatorUserID,
},
},
}
Expand Down
68 changes: 68 additions & 0 deletions components/backend/server/server.go
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ 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"
)

// RouterFunc is a function that can register routes on a Gin router
Expand Down Expand Up @@ -158,6 +160,72 @@ func forwardedIdentityMiddleware() gin.HandlerFunc {
if v := c.GetHeader("X-Forwarded-Access-Token"); v != "" {
c.Set("forwardedAccessToken", v)
}

// 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") == "" && 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 != "" {
c.Set("userID", uid)
}
}
Comment on lines +169 to +176
Copy link

@coderabbitai coderabbitai bot Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Do not treat a mutable ServiceAccount annotation as authoritative user identity.

Once the token is verified, this block copies ambient-code.io/created-by-user-id straight into userID without checking that the ServiceAccount is platform-managed. These ServiceAccounts are created in-project with the caller-scoped Kubernetes client in components/backend/handlers/permissions.go, so this annotation is not a strong trust boundary. Any annotated ServiceAccount token can become a user-integration identity; at minimum gate this on a platform-managed marker, and preferably move the mapping to server-owned or tamper-evident state.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@components/backend/server/server.go` around lines 169 - 176, The code
currently trusts the ServiceAccount annotation
"ambient-code.io/created-by-user-id" and sets c.Set("userID", ...) directly (in
the block using K8sClient and resolveServiceAccountFromToken), which allows
caller-controlled SA annotations to impersonate users; change this by requiring
a platform-managed marker on the ServiceAccount before accepting its created-by
annotation (e.g. check for a trusted annotation/label such as an
"ambient-code.io/platform-managed" marker or other server-controlled flag on
sa.Annotations/sa.Labels), and if that marker is absent do not set
c.Set("userID") from the SA annotation — instead record the mapping in
server-owned/tamper-evident state (or reject/ignore the annotation) and only
populate c.Set("userID") after verifying the SA is platform-managed or after
consulting the server-owned mapping.

}
}

c.Next()
}
}

// 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") {
return "", "", false
}
token := strings.TrimSpace(parts[1])
Comment on lines +188 to +193
Copy link

@coderabbitai coderabbitai bot Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Query-string API-key auth still misses this fallback.

This resolver only reads Authorization, but ?token= is promoted into that header later in components/backend/handlers/middleware.go inside ValidateProjectContext. Because forwardedIdentityMiddleware() runs first, websocket/agent callers using query-token auth will still reach this path with empty userID, so the new integration fallback never runs for them.

Possible fix 
 func Run(registerRoutes RouterFunc) error {
 	// Setup Gin router with custom logger that redacts tokens
 	r := gin.New()
 	r.Use(gin.Recovery())
+	r.Use(func(c *gin.Context) {
+		if c.GetHeader("Authorization") == "" && c.GetHeader("X-Forwarded-Access-Token") == "" {
+			if qp := strings.TrimSpace(c.Query("token")); qp != "" {
+				c.Request.Header.Set("Authorization", "Bearer "+qp)
+			}
+		}
+		c.Next()
+	})
 	r.Use(gin.LoggerWithFormatter(func(param gin.LogFormatterParams) string {
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@components/backend/server/server.go` around lines 188 - 193, The
Authorization header parsing in server.go only checks
c.GetHeader("Authorization") (rawAuth) and thus misses query-string ?token=
values used by websocket/agent callers; update the logic in the same function
that computes token (using rawAuth, parts, token) to fallback to the query param
(e.g., c.Query("token") or r.URL.Query().Get("token")) when rawAuth is empty or
parts length != 2, so token gets populated from the ?token= value (this keeps
behavior consistent with ValidateProjectContext and
forwardedIdentityMiddleware); ensure you still trim/validate the token string
before returning.

if token == "" {
return "", "", false
}

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
}

subj := strings.TrimSpace(rv.Status.User.Username)
const prefix = "system:serviceaccount:"
if !strings.HasPrefix(subj, prefix) {
return "", "", false
}
rest := strings.TrimPrefix(subj, prefix)
segs := strings.SplitN(rest, ":", 2)
if len(segs) != 2 {
return "", "", false
}
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
}
Loading