diff --git a/server/auth/serviceaccount/claims.go b/server/auth/serviceaccount/claims.go index 58d89e6cc3f1..f116b06eb157 100644 --- a/server/auth/serviceaccount/claims.go +++ b/server/auth/serviceaccount/claims.go @@ -17,32 +17,46 @@ func ClaimSetFor(restConfig *rest.Config) (*types.Claims, error) { username := restConfig.Username if username != "" { return &types.Claims{Claims: jwt.Claims{Subject: username}}, nil - } else if restConfig.BearerToken != "" || restConfig.BearerTokenFile != "" { - bearerToken := restConfig.BearerToken - if bearerToken == "" { - // should only ever be used for service accounts - data, err := os.ReadFile(restConfig.BearerTokenFile) - if err != nil { - return nil, fmt.Errorf("failed to read bearer token file: %w", err) - } - bearerToken = string(data) - } - parts := strings.SplitN(bearerToken, ".", 3) - if len(parts) != 3 { - return nil, fmt.Errorf("expected bearer token to be a JWT and therefore have 3 dot-delimited parts") - } - payload := parts[1] - data, err := base64.RawStdEncoding.DecodeString(payload) - if err != nil { - return nil, fmt.Errorf("failed to decode bearer token's JWT payload: %w", err) - } - claims := &types.Claims{} - err = json.Unmarshal(data, &claims) + } + if restConfig.BearerToken == "" && restConfig.BearerTokenFile == "" { + return nil, nil + } + + bearerToken := restConfig.BearerToken + if bearerToken == "" { + // should only ever be used for service accounts + data, err := os.ReadFile(restConfig.BearerTokenFile) if err != nil { - return nil, fmt.Errorf("failed to unmarshal bearer token's JWT payload: %w", err) + return nil, fmt.Errorf("failed to read bearer token file: %w", err) } + bearerToken = string(data) + } + + parts := strings.SplitN(bearerToken, ".", 3) + if len(parts) != 3 { + return nil, fmt.Errorf("expected bearer token to be a JWT and therefore have 3 dot-delimited parts") + } + payload := parts[1] + data, err := base64.RawStdEncoding.DecodeString(payload) + if err != nil { + return nil, fmt.Errorf("failed to decode bearer token's JWT payload: %w", err) + } + + claims := &types.Claims{} + err = json.Unmarshal(data, &claims) + if err != nil { + return nil, fmt.Errorf("failed to unmarshal bearer token's JWT payload: %w", err) + } + + // attempt to derive SA name and namespace from Subject + // "system:serviceaccount:argo:jenkins" -> "argo", "jenkins" + // note that the SA name can have a colon in it, although the rest cannot + parts = strings.SplitN(claims.Subject, ":", 4) + if len(parts) < 4 { return claims, nil - } else { - return nil, nil } + claims.ServiceAccountNamespace = parts[2] + claims.ServiceAccountName = parts[3] + + return claims, nil } diff --git a/server/auth/serviceaccount/claims_test.go b/server/auth/serviceaccount/claims_test.go index 8caa3c5ef254..e53e2dd097b0 100644 --- a/server/auth/serviceaccount/claims_test.go +++ b/server/auth/serviceaccount/claims_test.go @@ -5,12 +5,19 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "k8s.io/client-go/rest" ) -// sub = 1234567890 +const sub = "1234567890" const token = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c" //nolint:gosec +const sub2 = "system:serviceaccount:argo:jenkins" +const saName2 = "jenkins" +const saNs2 = "argo" +const iss2 = "https://kubernetes.default.svc.cluster.local" +const token2 = "eyJhbGciOiJSUzI1NiIsImtpZCI6Ijc5dVprMUl0VHZkTXFpLWc4dVQwVkV1Y05UZ21XXzJvZjNuZi1iZkpfVW8ifQ.eyJhdWQiOlsiaHR0cHM6Ly9rdWJlcm5ldGVzLmRlZmF1bHQuc3ZjLmNsdXN0ZXIubG9jYWwiLCJrM3MiXSwiZXhwIjoxNzIwNzYzNjgyLCJpYXQiOjE3MjA3NjAwODIsImlzcyI6Imh0dHBzOi8va3ViZXJuZXRlcy5kZWZhdWx0LnN2Yy5jbHVzdGVyLmxvY2FsIiwia3ViZXJuZXRlcy5pbyI6eyJuYW1lc3BhY2UiOiJhcmdvIiwic2VydmljZWFjY291bnQiOnsibmFtZSI6ImplbmtpbnMiLCJ1aWQiOiIyMGNjYzBjNS00NmNjLTQ3MjctYmUxMi1iZWY0ZTQ0ZTkxMjYifX0sIm5iZiI6MTcyMDc2MDA4Miwic3ViIjoic3lzdGVtOnNlcnZpY2VhY2NvdW50OmFyZ286amVua2lucyJ9.k0SKX4yKga70R12ScI-mSpEWbwdRtlphxXB-drHNyxPyzz2fpur29ZzdcjE8-TlZE2fQrrV-MaqMBWqaD0TWsld0ILBYRYFOcIwuUOX8s611vqmpDejTT6oAroBEd4WSduP9WoxMsiN82EdDvDJeMpNpq8i-Nz8nYgfWe2VkqV_oYCenWKe9JC3QL1TOxdkqer2qgflGnIpTzSVf7y47vmdlqsS9GPbdXyCg4MdcyDnLgY2VoVQnymD22uTG3Ugp3dO3zhaS-puHMPDOa-_EbAn2MsqETrA8H8iKal-z4sx-R6MxN8fdBrrIkWZLZZD4i_EhBS5paFTNcXXq0-T90w" //nolint:gosec + func TestClaimSetFor(t *testing.T) { t.Run("Empty", func(t *testing.T) { claims, err := ClaimSetFor(&rest.Config{}) @@ -19,10 +26,11 @@ func TestClaimSetFor(t *testing.T) { } }) t.Run("Basic", func(t *testing.T) { - claims, err := ClaimSetFor(&rest.Config{Username: "my-username"}) + const username = "my-username" + claims, err := ClaimSetFor(&rest.Config{Username: username}) if assert.NoError(t, err) { assert.Empty(t, claims.Issuer) - assert.Equal(t, "my-username", claims.Subject) + assert.Equal(t, username, claims.Subject) } }) t.Run("BadBearerToken", func(t *testing.T) { @@ -33,7 +41,7 @@ func TestClaimSetFor(t *testing.T) { claims, err := ClaimSetFor(&rest.Config{BearerToken: token}) if assert.NoError(t, err) { assert.Empty(t, claims.Issuer) - assert.Equal(t, "1234567890", claims.Subject) + assert.Equal(t, sub, claims.Subject) } }) @@ -48,7 +56,16 @@ func TestClaimSetFor(t *testing.T) { claims, err := ClaimSetFor(&rest.Config{BearerTokenFile: tmp.Name()}) if assert.NoError(t, err) { assert.Empty(t, claims.Issuer) - assert.Equal(t, "1234567890", claims.Subject) + assert.Equal(t, sub, claims.Subject) } }) + + t.Run("BearerToken with SA details", func(t *testing.T) { + claims, err := ClaimSetFor(&rest.Config{BearerToken: token2}) + require.NoError(t, err) + assert.Equal(t, iss2, claims.Issuer) + assert.Equal(t, sub2, claims.Subject) + assert.Equal(t, saName2, claims.ServiceAccountName) + assert.Equal(t, saNs2, claims.ServiceAccountNamespace) + }) } diff --git a/ui/src/app/app-router.tsx b/ui/src/app/app-router.tsx index c572297150ca..95dcb5273598 100644 --- a/ui/src/app/app-router.tsx +++ b/ui/src/app/app-router.tsx @@ -82,7 +82,7 @@ export function AppRouter({popupManager, history, notificationsManager}: {popupM .catch(setError); }, []); - const namespaceSuffix = Utils.managedNamespace ? '' : '/' + namespace; + const namespaceSuffix = Utils.managedNamespace ? '' : '/' + (namespace || ''); return ( <> {popupProps && } diff --git a/ui/src/app/shared/utils.ts b/ui/src/app/shared/utils.ts index 3f7accc7c3cc..ebc29b0b227a 100644 --- a/ui/src/app/shared/utils.ts +++ b/ui/src/app/shared/utils.ts @@ -84,9 +84,10 @@ export const Utils = { fixLocalStorageString(x: string): string { // empty string is valid, so we cannot use `truthy` - if (x !== null && x !== 'null' && x !== 'undefined') { - return x; + if (x == null || x == 'null' || x == 'undefined') { + return undefined; // explicitly return undefined } + return x; }, // TODO: some of these utils should probably be moved to context @@ -105,12 +106,7 @@ export const Utils = { }, get currentNamespace() { - // we always prefer the managed namespace - if (localStorage.getItem(currentNamespaceKey) === null) { - return this.userNamespace || this.managedNamespace; - } else { - return this.fixLocalStorageString(localStorage.getItem(currentNamespaceKey)); - } + return this.fixLocalStorageString(localStorage.getItem(currentNamespaceKey)) ?? (this.userNamespace || this.managedNamespace); }, // return a namespace, favoring managed namespace when set