From bc1d458fe6e2713a5cf2893a09fe568893b1f05f Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Tue, 5 May 2026 13:01:33 -0700 Subject: [PATCH] fix(iam): reject empty issuer in ComputeParentUser (#9326) Without iss, the same `sub` from two different IDPs would collapse to the same parent_user hash. Short-circuit to empty when either input is missing so callers see "no identity" instead of a colliding hash. Addresses coderabbit review on PR #9318. --- weed/iam/sts/parent_user_test.go | 8 ++++++++ weed/iam/sts/session_claims.go | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/weed/iam/sts/parent_user_test.go b/weed/iam/sts/parent_user_test.go index 6b17b4ce4..0059dda9c 100644 --- a/weed/iam/sts/parent_user_test.go +++ b/weed/iam/sts/parent_user_test.go @@ -43,6 +43,14 @@ func TestComputeParentUserEmptySub(t *testing.T) { } } +func TestComputeParentUserEmptyIss(t *testing.T) { + // Without an issuer, two different IDPs that both name a user "alice" + // would collide on the same parent_user. Refuse rather than hash. + if got := ComputeParentUser("alice", ""); got != "" { + t.Fatalf("empty iss should produce empty parent user, got %q", got) + } +} + func TestComputeParentUserEncoding(t *testing.T) { got := ComputeParentUser("alice", "https://idp.example/") // Base64 RawURL has no padding and uses URL-safe alphabet — important diff --git a/weed/iam/sts/session_claims.go b/weed/iam/sts/session_claims.go index 975a2f9f2..4b00b4eb9 100644 --- a/weed/iam/sts/session_claims.go +++ b/weed/iam/sts/session_claims.go @@ -17,7 +17,7 @@ import ( // id. The hash is base64-rawurl-encoded SHA-256 over "openid::" so // it stays filesystem-safe and bounded in length for storage in audit paths. func ComputeParentUser(sub, iss string) string { - if sub == "" { + if sub == "" || iss == "" { return "" } h := sha256.Sum256([]byte("openid:" + sub + ":" + iss))