SecretsManager: Update decrypt authorization with service identity (#105668)

(cherry picked from commit 9aea342be1764c33033aa1717242829970d5f5be)

pkg/registry/apis/secret/contracts/decrypt.go

@@ -21,7 +21,7 @@ type DecryptStorage interface {
 
 // DecryptAuthorizer is the interface for authorizing decryption requests.
 type DecryptAuthorizer interface {
-	Authorize(ctx context.Context, secureValueDecrypters []string) (identity string, allowed bool)
+	Authorize(ctx context.Context, secureValueName string, secureValueDecrypters []string) (identity string, allowed bool)
 }
 
 // TEMPORARY: Needed to pass it with wire.

pkg/registry/apis/secret/decrypt/authorizer.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"strings"
 
+	"github.com/grafana/authlib/authn"
 	claims "github.com/grafana/authlib/types"
 
 	secretv0alpha1 "github.com/grafana/grafana/pkg/apis/secret/v0alpha1"
@@ -22,70 +23,79 @@ func ProvideDecryptAuthorizer(allowList contracts.DecryptAllowList) contracts.De
 }
 
 // authorize checks whether the auth info token has the right permissions to decrypt the secure value.
-func (a *decryptAuthorizer) Authorize(ctx context.Context, secureValueDecrypters []string) (string, bool) {
+func (a *decryptAuthorizer) Authorize(ctx context.Context, secureValueName string, secureValueDecrypters []string) (string, bool) {
 	authInfo, ok := claims.AuthInfoFrom(ctx)
 	if !ok {
 		return "", false
 	}
 
-	tokenPermissions := authInfo.GetTokenPermissions()
+	serviceIdentityList, ok := authInfo.GetExtra()[authn.ServiceIdentityKey]
-
+	if !ok {
-	tokenActors := make(map[string]struct{}, 0)
-	for _, permission := range tokenPermissions {
-		// Will look like `secret.grafana.app/securevalues/<actor>:decrypt` for now.
-		gr, verb, found := strings.Cut(permission, ":")
-		if !found {
-			continue
-		}
-
-		// If it isn't decrypt, then we don't care to check.
-		if verb != "decrypt" {
-			continue
-		}
-
-		parts := strings.Split(gr, "/")
-		if len(parts) != 3 {
-			continue
-		}
-
-		group, resource, actor := parts[0], parts[1], parts[2]
-		if group != secretv0alpha1.GROUP || resource != secretv0alpha1.SecureValuesResourceInfo.GetName() || actor == "" {
-			continue
-		}
-
-		// TEMPORARY: while we can't onboard every app into secrets, we can block them from decrypting
-		// securevalues preemptively here before even reaching out to the database.
-		// This check can be removed once we open the gates for any service to use secrets.
-		if _, exists := a.allowList[actor]; !exists {
-			continue
-		}
-
-		tokenActors[actor] = struct{}{}
-	}
-
-	// If we arrived here and the token actors is empty, it means the permissions either have an invalid format,
-	// or it didn't pass the allow list, meaning no allowed decryptor.
-	if len(tokenActors) == 0 {
 		return "", false
 	}
 
-	// TEMPORARY: while we still need to mix permission and identity, we can use this
+	// If there's more than one service identity, something is suspicious and we reject it.
-	// to decide whether the SecureValue can be decrypted or not.
+	if len(serviceIdentityList) != 1 {
-	// Once we have an `actor` field in the JWT claims, we can have a properly formatted permission,
+		return "", false
-	// like `secret.grafana.app/securevalues{/<name>}:decrypt` and do regular access control eval,
+	}
-	// and for the `decrypters` part here, we can just check it against the `actor` field, which at
-	// that point will have a different format, depending on how the `actor` will be formatted.
-	// Check whether at least one of declared token actors matches the allowed decrypters from the SecureValue.
-	allowed := false
 
-	var identity string
+	serviceIdentity := serviceIdentityList[0]
+
+	// TEMPORARY: while we can't onboard every app into secrets, we can block them from decrypting
+	// securevalues preemptively here before even reaching out to the database.
+	// This check can be removed once we open the gates for any service to use secrets.
+	if _, exists := a.allowList[serviceIdentity]; !exists || serviceIdentity == "" {
+		return serviceIdentity, false
+	}
+
+	// Checks whether the token has the permission to decrypt secure values.
+	if !hasPermissionInToken(authInfo.GetTokenPermissions(), secureValueName) {
+		return serviceIdentity, false
+	}
+
+	// Finally check whether the service identity is allowed to decrypt this secure value.
+	allowed := false
 	for _, decrypter := range secureValueDecrypters {
-		if _, exists := tokenActors[decrypter]; exists {
+		if decrypter == serviceIdentity {
 			allowed = true
-			identity = decrypter
 			break
 		}
 	}
 
-	return identity, allowed
+	return serviceIdentity, allowed
+}
+
+// Adapted from https://github.com/grafana/authlib/blob/1492b99410603ca15730a1805a9220ce48232bc3/authz/client.go#L138
+// Changes: 1) we don't support `*` for verbs; 2) we support specific names in the permission.
+func hasPermissionInToken(tokenPermissions []string, name string) bool {
+	var (
+		group    = secretv0alpha1.GROUP
+		resource = secretv0alpha1.SecureValuesResourceInfo.GetName()
+		verb     = "decrypt"
+	)
+
+	for _, p := range tokenPermissions {
+		tokenGR, tokenVerb, found := strings.Cut(p, ":")
+		if !found || tokenVerb != verb {
+			continue
+		}
+
+		parts := strings.SplitN(tokenGR, "/", 3)
+
+		switch len(parts) {
+		// secret.grafana.app/securevalues:decrypt
+		case 2:
+			if parts[0] == group && parts[1] == resource {
+				return true
+			}
+
+		// secret.grafana.app/securevalues/<name>:decrypt
+		case 3:
+			if parts[0] == group && parts[1] == resource && parts[2] == name && name != "" {
+				return true
+			}
+		}
+	}
+
+	return false
 }

pkg/registry/apis/secret/decrypt/authorizer_test.go

@@ -16,127 +16,323 @@ func TestDecryptAuthorizer(t *testing.T) {
 		ctx := context.Background()
 		authorizer := ProvideDecryptAuthorizer(nil)
 
-		identity, allowed := authorizer.Authorize(ctx, nil)
+		identity, allowed := authorizer.Authorize(ctx, "", nil)
 		require.Empty(t, identity)
 		require.False(t, allowed)
 	})
 
 	t.Run("when token permissions are empty, it returns false", func(t *testing.T) {
-		ctx := createAuthContext(context.Background(), []string{})
+		ctx := createAuthContext(context.Background(), "identity", []string{})
 		authorizer := ProvideDecryptAuthorizer(nil)
 
-		identity, allowed := authorizer.Authorize(ctx, nil)
+		identity, allowed := authorizer.Authorize(ctx, "", nil)
+		require.NotEmpty(t, identity)
+		require.False(t, allowed)
+	})
+
+	t.Run("when service identity is empty, it returns false", func(t *testing.T) {
+		ctx := createAuthContext(context.Background(), "", []string{})
+		authorizer := ProvideDecryptAuthorizer(nil)
+
+		identity, allowed := authorizer.Authorize(ctx, "", nil)
 		require.Empty(t, identity)
 		require.False(t, allowed)
 	})
 
 	t.Run("when permission format is malformed (missing verb), it returns false", func(t *testing.T) {
-		ctx := createAuthContext(context.Background(), []string{"secret.grafana.app/securevalues/group1"})
 		authorizer := ProvideDecryptAuthorizer(nil)
 
-		identity, allowed := authorizer.Authorize(ctx, nil)
+		// nameless
-		require.Empty(t, identity)
+		ctx := createAuthContext(context.Background(), "identity", []string{"secret.grafana.app/securevalues"})
+		identity, allowed := authorizer.Authorize(ctx, "", nil)
+		require.NotEmpty(t, identity)
+		require.False(t, allowed)
+
+		// named
+		ctx = createAuthContext(context.Background(), "identity", []string{"secret.grafana.app/securevalues/name"})
+		identity, allowed = authorizer.Authorize(ctx, "", nil)
+		require.NotEmpty(t, identity)
 		require.False(t, allowed)
 	})
 
 	t.Run("when permission verb is not exactly `decrypt`, it returns false", func(t *testing.T) {
-		ctx := createAuthContext(context.Background(), []string{"secret.grafana.app/securevalues/group1:something"})
 		authorizer := ProvideDecryptAuthorizer(nil)
 
-		identity, allowed := authorizer.Authorize(ctx, nil)
+		// nameless
-		require.Empty(t, identity)
+		ctx := createAuthContext(context.Background(), "identity", []string{"secret.grafana.app/securevalues:*"})
+		identity, allowed := authorizer.Authorize(ctx, "", nil)
+		require.NotEmpty(t, identity)
+		require.False(t, allowed)
+
+		// named
+		ctx = createAuthContext(context.Background(), "identity", []string{"secret.grafana.app/securevalues/name:something"})
+		identity, allowed = authorizer.Authorize(ctx, "", nil)
+		require.NotEmpty(t, identity)
 		require.False(t, allowed)
 	})
 
-	t.Run("when permission does not have 3 parts, it returns false", func(t *testing.T) {
+	t.Run("when permission does not have 2 or 3 parts, it returns false", func(t *testing.T) {
-		ctx := createAuthContext(context.Background(), []string{"secret.grafana.app/securevalues:decrypt"})
+		ctx := createAuthContext(context.Background(), "identity", []string{"secret.grafana.app:decrypt"})
 		authorizer := ProvideDecryptAuthorizer(nil)
 
-		identity, allowed := authorizer.Authorize(ctx, nil)
+		identity, allowed := authorizer.Authorize(ctx, "", nil)
-		require.Empty(t, identity)
+		require.NotEmpty(t, identity)
 		require.False(t, allowed)
 	})
 
 	t.Run("when permission has group that is not `secret.grafana.app`, it returns false", func(t *testing.T) {
-		ctx := createAuthContext(context.Background(), []string{"wrong.group/securevalues/invalid:decrypt"})
+		ctx := createAuthContext(context.Background(), "identity", []string{"wrong.group/securevalues/invalid:decrypt"})
 		authorizer := ProvideDecryptAuthorizer(nil)
 
-		identity, allowed := authorizer.Authorize(ctx, nil)
+		identity, allowed := authorizer.Authorize(ctx, "", nil)
-		require.Empty(t, identity)
+		require.NotEmpty(t, identity)
 		require.False(t, allowed)
 	})
 
 	t.Run("when permission has resource that is not `securevalues`, it returns false", func(t *testing.T) {
-		ctx := createAuthContext(context.Background(), []string{"secret.grafana.app/invalid-resource/invalid:decrypt"})
 		authorizer := ProvideDecryptAuthorizer(nil)
 
-		identity, allowed := authorizer.Authorize(ctx, nil)
+		// nameless
-		require.Empty(t, identity)
+		ctx := createAuthContext(context.Background(), "identity", []string{"secret.grafana.app/invalid-resource:decrypt"})
+		identity, allowed := authorizer.Authorize(ctx, "", nil)
+		require.NotEmpty(t, identity)
+		require.False(t, allowed)
+
+		// named
+		ctx = createAuthContext(context.Background(), "identity", []string{"secret.grafana.app/invalid-resource/name:decrypt"})
+		identity, allowed = authorizer.Authorize(ctx, "", nil)
+		require.NotEmpty(t, identity)
 		require.False(t, allowed)
 	})
 
-	t.Run("when the actor is not in the allow list, it returns false", func(t *testing.T) {
+	t.Run("when the identity is not in the allow list, it returns false", func(t *testing.T) {
-		ctx := createAuthContext(context.Background(), []string{"secret.grafana.app/securevalues/allowed2:decrypt"})
+		ctx := createAuthContext(context.Background(), "identity", []string{"secret.grafana.app/securevalues:decrypt"})
 		authorizer := ProvideDecryptAuthorizer(map[string]struct{}{"allowed1": {}})
 
-		identity, allowed := authorizer.Authorize(ctx, nil)
+		identity, allowed := authorizer.Authorize(ctx, "", nil)
-		require.Empty(t, identity)
+		require.NotEmpty(t, identity)
 		require.False(t, allowed)
 	})
 
-	t.Run("when the actor doesn't match any allowed decrypters, it returns false", func(t *testing.T) {
+	t.Run("when the identity doesn't match any allowed decrypters, it returns false", func(t *testing.T) {
-		ctx := createAuthContext(context.Background(), []string{"secret.grafana.app/securevalues/group1:decrypt"})
+		authorizer := ProvideDecryptAuthorizer(map[string]struct{}{"identity": {}})
-		authorizer := ProvideDecryptAuthorizer(map[string]struct{}{"group1": {}})
 
-		identity, allowed := authorizer.Authorize(ctx, []string{"group2"})
+		// nameless
-		require.Empty(t, identity)
+		ctx := createAuthContext(context.Background(), "identity", []string{"secret.grafana.app/securevalues:decrypt"})
+		identity, allowed := authorizer.Authorize(ctx, "", []string{"group2"})
+		require.NotEmpty(t, identity)
+		require.False(t, allowed)
+
+		// named
+		ctx = createAuthContext(context.Background(), "identity", []string{"secret.grafana.app/securevalues/name:decrypt"})
+		identity, allowed = authorizer.Authorize(ctx, "", []string{"group2"})
+		require.NotEmpty(t, identity)
 		require.False(t, allowed)
 	})
 
-	t.Run("when the actor matches an allowed decrypter, it returns true", func(t *testing.T) {
+	t.Run("when the identity matches an allowed decrypter, it returns true", func(t *testing.T) {
-		ctx := createAuthContext(context.Background(), []string{"secret.grafana.app/securevalues/group1:decrypt"})
+		authorizer := ProvideDecryptAuthorizer(map[string]struct{}{"identity": {}})
-		authorizer := ProvideDecryptAuthorizer(map[string]struct{}{"group1": {}})
 
-		identity, allowed := authorizer.Authorize(ctx, []string{"group1"})
+		// nameless
+		ctx := createAuthContext(context.Background(), "identity", []string{"secret.grafana.app/securevalues:decrypt"})
+		identity, allowed := authorizer.Authorize(ctx, "", []string{"identity"})
 		require.True(t, allowed)
-		require.Equal(t, "group1", identity)
+		require.Equal(t, "identity", identity)
+
+		// named
+		ctx = createAuthContext(context.Background(), "identity", []string{"secret.grafana.app/securevalues/name:decrypt"})
+		identity, allowed = authorizer.Authorize(ctx, "name", []string{"identity"})
+		require.True(t, allowed)
+		require.Equal(t, "identity", identity)
 	})
 
 	t.Run("when there are multiple permissions, some invalid, only valid ones are considered", func(t *testing.T) {
-		ctx := createAuthContext(context.Background(), []string{
+		ctx := createAuthContext(context.Background(), "identity", []string{
-			"secret.grafana.app/securevalues/group1:decrypt",
+			"secret.grafana.app/securevalues/name1:decrypt",
+			"secret.grafana.app/securevalues/name2:decrypt",
 			"secret.grafana.app/securevalues/invalid:read",
 			"wrong.group/securevalues/group2:decrypt",
-			"secret.grafana.app/securevalues/group2:decrypt",
+			"secret.grafana.app/securevalues/identity:decrypt", // old style of identity+permission
 		})
-		authorizer := ProvideDecryptAuthorizer(map[string]struct{}{"group1": {}, "group2": {}})
+		authorizer := ProvideDecryptAuthorizer(map[string]struct{}{"identity": {}})
 
-		identity, allowed := authorizer.Authorize(ctx, []string{"group2", "group3"})
+		identity, allowed := authorizer.Authorize(ctx, "name1", []string{"identity"})
 		require.True(t, allowed)
-		require.Equal(t, "group2", identity)
+		require.Equal(t, "identity", identity)
+
+		identity, allowed = authorizer.Authorize(ctx, "name2", []string{"identity"})
+		require.True(t, allowed)
+		require.Equal(t, "identity", identity)
 	})
 
-	t.Run("when multiple valid actors match decrypters, the first match already returns true", func(t *testing.T) {
+	t.Run("when empty secure value name with specific permission, it returns false", func(t *testing.T) {
-		ctx := createAuthContext(context.Background(), []string{
+		ctx := createAuthContext(context.Background(), "identity", []string{"secret.grafana.app/securevalues/name:decrypt"})
-			"secret.grafana.app/securevalues/group1:decrypt",
+		authorizer := ProvideDecryptAuthorizer(map[string]struct{}{"identity": {}})
-			"secret.grafana.app/securevalues/group2:decrypt",
-		})
-		authorizer := ProvideDecryptAuthorizer(map[string]struct{}{"group1": {}, "group2": {}})
 
-		identity, allowed := authorizer.Authorize(ctx, []string{"group2", "group1"})
+		identity, allowed := authorizer.Authorize(ctx, "", []string{"identity"})
+		require.Equal(t, "identity", identity)
+		require.False(t, allowed)
+	})
+
+	t.Run("when permission has an extra / but no name, it returns false", func(t *testing.T) {
+		ctx := createAuthContext(context.Background(), "identity", []string{"secret.grafana.app/securevalues/:decrypt"})
+		authorizer := ProvideDecryptAuthorizer(map[string]struct{}{"identity": {}})
+
+		identity, allowed := authorizer.Authorize(ctx, "", []string{"identity"})
+		require.Equal(t, "identity", identity)
+		require.False(t, allowed)
+	})
+
+	t.Run("when the decrypters list is empty, meaning nothing can decrypt the secure value, it returns false", func(t *testing.T) {
+		ctx := createAuthContext(context.Background(), "identity", []string{"secret.grafana.app/securevalues:decrypt"})
+		authorizer := ProvideDecryptAuthorizer(map[string]struct{}{"identity": {}})
+
+		identity, allowed := authorizer.Authorize(ctx, "name", []string{})
+		require.Equal(t, "identity", identity)
+		require.False(t, allowed)
+	})
+
+	t.Run("when one of decrypters matches the identity, it returns true", func(t *testing.T) {
+		ctx := createAuthContext(context.Background(), "identity1", []string{"secret.grafana.app/securevalues:decrypt"})
+		authorizer := ProvideDecryptAuthorizer(map[string]struct{}{"identity1": {}, "identity2": {}})
+
+		identity, allowed := authorizer.Authorize(ctx, "", []string{"identity1", "identity2", "identity3"})
+		require.Equal(t, "identity1", identity)
 		require.True(t, allowed)
-		require.Equal(t, "group2", identity)
+	})
+
+	t.Run("permissions must be case-sensitive and return false", func(t *testing.T) {
+		authorizer := ProvideDecryptAuthorizer(map[string]struct{}{"identity": {}})
+
+		ctx := createAuthContext(context.Background(), "identity", []string{"SECRET.grafana.app/securevalues:decrypt"})
+		identity, allowed := authorizer.Authorize(ctx, "", []string{"identity"})
+		require.Equal(t, "identity", identity)
+		require.False(t, allowed)
+
+		ctx = createAuthContext(context.Background(), "identity", []string{"secret.grafana.app/SECUREVALUES:decrypt"})
+		identity, allowed = authorizer.Authorize(ctx, "", []string{"identity"})
+		require.Equal(t, "identity", identity)
+		require.False(t, allowed)
+
+		ctx = createAuthContext(context.Background(), "identity", []string{"secret.grafana.app/securevalues:DECRYPT"})
+		identity, allowed = authorizer.Authorize(ctx, "", []string{"identity"})
+		require.Equal(t, "identity", identity)
+		require.False(t, allowed)
 	})
 }
 
-func createAuthContext(ctx context.Context, permissions []string) context.Context {
+func createAuthContext(ctx context.Context, serviceIdentity string, permissions []string) context.Context {
 	requester := &identity.StaticRequester{
 		AccessTokenClaims: &authn.Claims[authn.AccessTokenClaims]{
 			Rest: authn.AccessTokenClaims{
-				Permissions: permissions,
+				Permissions:     permissions,
+				ServiceIdentity: serviceIdentity,
 			},
 		},
 	}
 
 	return types.WithAuthInfo(ctx, requester)
 }
+
+// Adapted from https://github.com/grafana/authlib/blob/1492b99410603ca15730a1805a9220ce48232bc3/authz/client_test.go#L18
+func TestHasPermissionInToken(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		test             string
+		tokenPermissions []string
+		name             string
+		want             bool
+	}{
+		{
+			test:             "Permission matches group/resource",
+			tokenPermissions: []string{"secret.grafana.app/securevalues:decrypt"},
+			want:             true,
+		},
+		{
+			test:             "Permission does not match verb",
+			tokenPermissions: []string{"secret.grafana.app/securevalues:create"},
+			want:             false,
+		},
+		{
+			test:             "Permission does not have support for wildcard verb",
+			tokenPermissions: []string{"secret.grafana.app/securevalues:*"},
+			want:             false,
+		},
+		{
+			test:             "Invalid permission missing verb",
+			tokenPermissions: []string{"secret.grafana.app/securevalues"},
+			want:             false,
+		},
+		{
+			test:             "Permission on the wrong group",
+			tokenPermissions: []string{"other-group.grafana.app/securevalues:decrypt"},
+			want:             false,
+		},
+		{
+			test:             "Permission on the wrong resource",
+			tokenPermissions: []string{"secret.grafana.app/other-resource:decrypt"},
+			want:             false,
+		},
+		{
+			test:             "Permission without group are skipped",
+			tokenPermissions: []string{":decrypt"},
+			want:             false,
+		},
+		{
+			test:             "Group level permission is not supported",
+			tokenPermissions: []string{"secret.grafana.app:decrypt"},
+			want:             false,
+		},
+		{
+			test:             "Permission with name matches group/resource/name",
+			tokenPermissions: []string{"secret.grafana.app/securevalues/name:decrypt"},
+			name:             "name",
+			want:             true,
+		},
+		{
+			test:             "Permission with name2 does not matche group/resource/name1",
+			tokenPermissions: []string{"secret.grafana.app/securevalues/name1:decrypt"},
+			name:             "name2",
+			want:             false,
+		},
+		{
+			test:             "Parts need an exact match",
+			tokenPermissions: []string{"secret.grafana.app/secure:*"},
+			want:             false,
+		},
+		{
+			test:             "Resource specific permission should not allow access to all resources",
+			tokenPermissions: []string{"secret.grafana.app/securevalues/name:decrypt"},
+			name:             "",
+			want:             false,
+		},
+		{
+			test:             "Permission at group/resource should allow access to all resources",
+			tokenPermissions: []string{"secret.grafana.app/securevalues:decrypt"},
+			name:             "name",
+			want:             true,
+		},
+		{
+			test:             "Empty name trying to match everything is not allowed",
+			tokenPermissions: []string{"secret.grafana.app/securevalues/:decrypt"},
+			name:             "",
+			want:             false,
+		},
+		{
+			test:             "Empty name trying to match a specific name is not allowed",
+			tokenPermissions: []string{"secret.grafana.app/securevalues/:decrypt"},
+			name:             "name",
+			want:             false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.test, func(t *testing.T) {
+			t.Parallel()
+
+			got := hasPermissionInToken(tt.tokenPermissions, tt.name)
+			require.Equal(t, tt.want, got)
+		})
+	}
+}

pkg/registry/apis/secret/decrypt/noop_authorizer.go

@@ -11,6 +11,6 @@ type NoopAlwaysAllowedAuthorizer struct{}
 
 var _ contracts.DecryptAuthorizer = &NoopAlwaysAllowedAuthorizer{}
 
-func (a *NoopAlwaysAllowedAuthorizer) Authorize(ctx context.Context, secureValueDecrypters []string) (identity string, allowed bool) {
+func (a *NoopAlwaysAllowedAuthorizer) Authorize(ctx context.Context, secureValueName string, secureValueDecrypters []string) (identity string, allowed bool) {
 	return "", true
 }