Secrets: Add service name as explicit parameter for Decrypt (#109054)
* Secrets: Add service name as explicit parameter for Decrypt * Apply suggestions from code review Co-authored-by: Stephanie Hingtgen <stephanie.hingtgen@grafana.com> --------- Co-authored-by: Stephanie Hingtgen <stephanie.hingtgen@grafana.com>
This commit is contained in:
co-authored by
Stephanie Hingtgen
parent
343fe4dadf
commit
81f544e186
@@ -67,16 +67,9 @@ func (_c *MockDecryptService_Close_Call) RunAndReturn(run func() error) *MockDec
|
||||
return _c
|
||||
}
|
||||
|
||||
// Decrypt provides a mock function with given fields: ctx, namespace, names
|
||||
func (_m *MockDecryptService) Decrypt(ctx context.Context, namespace string, names ...string) (map[string]contracts.DecryptResult, error) {
|
||||
_va := make([]interface{}, len(names))
|
||||
for _i := range names {
|
||||
_va[_i] = names[_i]
|
||||
}
|
||||
var _ca []interface{}
|
||||
_ca = append(_ca, ctx, namespace)
|
||||
_ca = append(_ca, _va...)
|
||||
ret := _m.Called(_ca...)
|
||||
// Decrypt provides a mock function with given fields: ctx, serviceName, namespace, names
|
||||
func (_m *MockDecryptService) Decrypt(ctx context.Context, serviceName string, namespace string, names []string) (map[string]contracts.DecryptResult, error) {
|
||||
ret := _m.Called(ctx, serviceName, namespace, names)
|
||||
|
||||
if len(ret) == 0 {
|
||||
panic("no return value specified for Decrypt")
|
||||
@@ -84,19 +77,19 @@ func (_m *MockDecryptService) Decrypt(ctx context.Context, namespace string, nam
|
||||
|
||||
var r0 map[string]contracts.DecryptResult
|
||||
var r1 error
|
||||
if rf, ok := ret.Get(0).(func(context.Context, string, ...string) (map[string]contracts.DecryptResult, error)); ok {
|
||||
return rf(ctx, namespace, names...)
|
||||
if rf, ok := ret.Get(0).(func(context.Context, string, string, []string) (map[string]contracts.DecryptResult, error)); ok {
|
||||
return rf(ctx, serviceName, namespace, names)
|
||||
}
|
||||
if rf, ok := ret.Get(0).(func(context.Context, string, ...string) map[string]contracts.DecryptResult); ok {
|
||||
r0 = rf(ctx, namespace, names...)
|
||||
if rf, ok := ret.Get(0).(func(context.Context, string, string, []string) map[string]contracts.DecryptResult); ok {
|
||||
r0 = rf(ctx, serviceName, namespace, names)
|
||||
} else {
|
||||
if ret.Get(0) != nil {
|
||||
r0 = ret.Get(0).(map[string]contracts.DecryptResult)
|
||||
}
|
||||
}
|
||||
|
||||
if rf, ok := ret.Get(1).(func(context.Context, string, ...string) error); ok {
|
||||
r1 = rf(ctx, namespace, names...)
|
||||
if rf, ok := ret.Get(1).(func(context.Context, string, string, []string) error); ok {
|
||||
r1 = rf(ctx, serviceName, namespace, names)
|
||||
} else {
|
||||
r1 = ret.Error(1)
|
||||
}
|
||||
@@ -111,22 +104,16 @@ type MockDecryptService_Decrypt_Call struct {
|
||||
|
||||
// Decrypt is a helper method to define mock.On call
|
||||
// - ctx context.Context
|
||||
// - serviceName string
|
||||
// - namespace string
|
||||
// - names ...string
|
||||
func (_e *MockDecryptService_Expecter) Decrypt(ctx interface{}, namespace interface{}, names ...interface{}) *MockDecryptService_Decrypt_Call {
|
||||
return &MockDecryptService_Decrypt_Call{Call: _e.mock.On("Decrypt",
|
||||
append([]interface{}{ctx, namespace}, names...)...)}
|
||||
// - names []string
|
||||
func (_e *MockDecryptService_Expecter) Decrypt(ctx interface{}, serviceName interface{}, namespace interface{}, names interface{}) *MockDecryptService_Decrypt_Call {
|
||||
return &MockDecryptService_Decrypt_Call{Call: _e.mock.On("Decrypt", ctx, serviceName, namespace, names)}
|
||||
}
|
||||
|
||||
func (_c *MockDecryptService_Decrypt_Call) Run(run func(ctx context.Context, namespace string, names ...string)) *MockDecryptService_Decrypt_Call {
|
||||
func (_c *MockDecryptService_Decrypt_Call) Run(run func(ctx context.Context, serviceName string, namespace string, names []string)) *MockDecryptService_Decrypt_Call {
|
||||
_c.Call.Run(func(args mock.Arguments) {
|
||||
variadicArgs := make([]string, len(args)-2)
|
||||
for i, a := range args[2:] {
|
||||
if a != nil {
|
||||
variadicArgs[i] = a.(string)
|
||||
}
|
||||
}
|
||||
run(args[0].(context.Context), args[1].(string), variadicArgs...)
|
||||
run(args[0].(context.Context), args[1].(string), args[2].(string), args[3].([]string))
|
||||
})
|
||||
return _c
|
||||
}
|
||||
@@ -136,7 +123,7 @@ func (_c *MockDecryptService_Decrypt_Call) Return(_a0 map[string]contracts.Decry
|
||||
return _c
|
||||
}
|
||||
|
||||
func (_c *MockDecryptService_Decrypt_Call) RunAndReturn(run func(context.Context, string, ...string) (map[string]contracts.DecryptResult, error)) *MockDecryptService_Decrypt_Call {
|
||||
func (_c *MockDecryptService_Decrypt_Call) RunAndReturn(run func(context.Context, string, string, []string) (map[string]contracts.DecryptResult, error)) *MockDecryptService_Decrypt_Call {
|
||||
_c.Call.Return(run)
|
||||
return _c
|
||||
}
|
||||
|
||||
@@ -4,8 +4,6 @@ import (
|
||||
"context"
|
||||
"errors"
|
||||
|
||||
"github.com/grafana/authlib/types"
|
||||
"github.com/grafana/grafana/pkg/apimachinery/identity"
|
||||
"github.com/grafana/grafana/pkg/registry/apis/secret"
|
||||
"github.com/grafana/grafana/pkg/registry/apis/secret/contracts"
|
||||
"github.com/grafana/grafana/pkg/setting"
|
||||
@@ -109,13 +107,7 @@ func (s *secretsService) Encrypt(ctx context.Context, namespace, name string, da
|
||||
}
|
||||
|
||||
func (s *secretsService) Decrypt(ctx context.Context, namespace string, name string) ([]byte, error) {
|
||||
ns, err := types.ParseNamespace(namespace)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
ctx = identity.WithServiceIdentityContext(ctx, ns.OrgID, identity.WithServiceIdentityName(svcName))
|
||||
|
||||
results, err := s.decryptSvc.Decrypt(ctx, namespace, name)
|
||||
results, err := s.decryptSvc.Decrypt(ctx, svcName, namespace, []string{name})
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
@@ -132,12 +124,6 @@ func (s *secretsService) Decrypt(ctx context.Context, namespace string, name str
|
||||
}
|
||||
|
||||
func (s *secretsService) Delete(ctx context.Context, namespace string, name string) error {
|
||||
ns, err := types.ParseNamespace(namespace)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
ctx = identity.WithServiceIdentityContext(ctx, ns.OrgID, identity.WithServiceIdentityName(svcName))
|
||||
client, err := s.secureValues.Client(ctx, namespace)
|
||||
if err != nil {
|
||||
return err
|
||||
|
||||
@@ -266,8 +266,9 @@ func TestSecretsService_Decrypt(t *testing.T) {
|
||||
// Verify that the context is not nil (the service creates a new StaticRequester)
|
||||
return ctx != nil
|
||||
}),
|
||||
svcName,
|
||||
"test-namespace",
|
||||
"test-secret",
|
||||
[]string{"test-secret"},
|
||||
).Return(map[string]secret.DecryptResult{
|
||||
"test-secret": mockResult,
|
||||
}, nil)
|
||||
@@ -283,8 +284,9 @@ func TestSecretsService_Decrypt(t *testing.T) {
|
||||
mock.MatchedBy(func(ctx context.Context) bool {
|
||||
return ctx != nil
|
||||
}),
|
||||
svcName,
|
||||
"test-namespace",
|
||||
"test-secret",
|
||||
[]string{"test-secret"},
|
||||
).Return(nil, errors.New("decrypt service error"))
|
||||
},
|
||||
expectedError: "decrypt service error",
|
||||
@@ -298,8 +300,9 @@ func TestSecretsService_Decrypt(t *testing.T) {
|
||||
mock.MatchedBy(func(ctx context.Context) bool {
|
||||
return ctx != nil
|
||||
}),
|
||||
svcName,
|
||||
"test-namespace",
|
||||
"test-secret",
|
||||
[]string{"test-secret"},
|
||||
).Return(map[string]secret.DecryptResult{}, nil)
|
||||
},
|
||||
expectedError: secret.ErrDecryptNotFound.Error(),
|
||||
@@ -315,8 +318,9 @@ func TestSecretsService_Decrypt(t *testing.T) {
|
||||
mock.MatchedBy(func(ctx context.Context) bool {
|
||||
return ctx != nil
|
||||
}),
|
||||
svcName,
|
||||
"test-namespace",
|
||||
"test-secret",
|
||||
[]string{"test-secret"},
|
||||
).Return(map[string]secret.DecryptResult{
|
||||
"test-secret": mockResult,
|
||||
}, nil)
|
||||
@@ -363,8 +367,9 @@ func TestSecretsService_Decrypt_ServiceIdentityContext(t *testing.T) {
|
||||
// At minimum, verify the context is not nil and is different from the original
|
||||
return ctx != nil
|
||||
}),
|
||||
svcName,
|
||||
"test-namespace",
|
||||
"test-secret",
|
||||
[]string{"test-secret"},
|
||||
).Return(map[string]secret.DecryptResult{
|
||||
"test-secret": mockResult,
|
||||
}, nil)
|
||||
|
||||
@@ -29,7 +29,7 @@ type DecryptAuthorizer interface {
|
||||
|
||||
// DecryptService is the interface for the decrypt service.
|
||||
type DecryptService interface {
|
||||
Decrypt(ctx context.Context, namespace string, names ...string) (map[string]DecryptResult, error)
|
||||
Decrypt(ctx context.Context, serviceName string, namespace string, names []string) (map[string]DecryptResult, error)
|
||||
Close() error
|
||||
}
|
||||
|
||||
|
||||
@@ -10,7 +10,7 @@ import (
|
||||
|
||||
"github.com/fullstorydev/grpchan"
|
||||
authnlib "github.com/grafana/authlib/authn"
|
||||
claims "github.com/grafana/authlib/types"
|
||||
"github.com/grafana/authlib/types"
|
||||
"go.opentelemetry.io/otel/trace"
|
||||
"google.golang.org/grpc"
|
||||
"google.golang.org/grpc/credentials"
|
||||
@@ -127,25 +127,11 @@ func (g *GRPCDecryptClient) Close() error {
|
||||
return nil
|
||||
}
|
||||
|
||||
func (g *GRPCDecryptClient) Decrypt(ctx context.Context, namespace string, names ...string) (map[string]contracts.DecryptResult, error) {
|
||||
authInfo, ok := claims.AuthInfoFrom(ctx)
|
||||
if !ok {
|
||||
return nil, errors.New("missing auth info in context")
|
||||
func (g *GRPCDecryptClient) Decrypt(ctx context.Context, serviceName string, namespace string, names []string) (map[string]contracts.DecryptResult, error) {
|
||||
_, err := types.ParseNamespace(namespace)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// Up until here the identity is the one set by the internal service, but when the request goes out to the gRPC server,
|
||||
// the aggregator will use the access token which contains a different service identity for grafana as a whole.
|
||||
// This is used for logging purposes only.
|
||||
serviceIdentityList, ok := authInfo.GetExtra()[authnlib.ServiceIdentityKey]
|
||||
if !ok || len(serviceIdentityList) != 1 {
|
||||
return nil, errors.New("invalid service identity in auth info")
|
||||
}
|
||||
|
||||
serviceIdentity := serviceIdentityList[0]
|
||||
if len(serviceIdentity) == 0 {
|
||||
return nil, errors.New("empty service identity in auth info")
|
||||
}
|
||||
|
||||
req := &decryptv1beta1.SecureValueDecryptRequest{
|
||||
Namespace: namespace,
|
||||
Names: names,
|
||||
@@ -154,7 +140,7 @@ func (g *GRPCDecryptClient) Decrypt(ctx context.Context, namespace string, names
|
||||
// Decryption will still use the service identity from the auth token,
|
||||
// but we also pass the service identity from the request metadata for auditing purposes.
|
||||
md := metadata.New(map[string]string{
|
||||
contracts.HeaderGrafanaServiceIdentityName: serviceIdentity,
|
||||
contracts.HeaderGrafanaServiceIdentityName: serviceName,
|
||||
})
|
||||
ctx = metadata.NewOutgoingContext(ctx, md)
|
||||
|
||||
|
||||
@@ -3,6 +3,8 @@ package decrypt
|
||||
import (
|
||||
"context"
|
||||
|
||||
"github.com/grafana/authlib/types"
|
||||
"github.com/grafana/grafana/pkg/apimachinery/identity"
|
||||
"github.com/grafana/grafana/pkg/registry/apis/secret/contracts"
|
||||
"github.com/grafana/grafana/pkg/registry/apis/secret/xkube"
|
||||
)
|
||||
@@ -19,7 +21,14 @@ func NewLocalDecryptClient(decryptStorage contracts.DecryptStorage) (*LocalDecry
|
||||
}, nil
|
||||
}
|
||||
|
||||
func (c *LocalDecryptClient) Decrypt(ctx context.Context, namespace string, names ...string) (map[string]contracts.DecryptResult, error) {
|
||||
func (c *LocalDecryptClient) Decrypt(ctx context.Context, serviceName, namespace string, names []string) (map[string]contracts.DecryptResult, error) {
|
||||
ns, err := types.ParseNamespace(namespace)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
ctx = identity.WithServiceIdentityContext(ctx, ns.OrgID, identity.WithServiceIdentityName(serviceName))
|
||||
|
||||
results := make(map[string]contracts.DecryptResult, len(names))
|
||||
|
||||
for _, name := range names {
|
||||
|
||||
@@ -20,7 +20,6 @@ import (
|
||||
"google.golang.org/grpc/credentials"
|
||||
"google.golang.org/grpc/metadata"
|
||||
|
||||
"github.com/grafana/grafana/pkg/apimachinery/identity"
|
||||
"github.com/grafana/grafana/pkg/registry/apis/secret/contracts"
|
||||
"github.com/grafana/grafana/pkg/registry/apis/secret/xkube"
|
||||
"github.com/grafana/grafana/pkg/services/authn/clients"
|
||||
@@ -49,7 +48,7 @@ func TestDecryptService(t *testing.T) {
|
||||
decryptService, err := ProvideDecryptService(cfg, tracer, mockStorage)
|
||||
require.NoError(t, err)
|
||||
|
||||
resp, err := decryptService.Decrypt(ctx, "default", "secure-value-1")
|
||||
resp, err := decryptService.Decrypt(ctx, "svc-name", "default", []string{"secure-value-1"})
|
||||
require.NotNil(t, resp)
|
||||
require.NoError(t, err)
|
||||
require.EqualValues(t, decryptedValuesResp, resp)
|
||||
@@ -78,7 +77,7 @@ func TestDecryptService(t *testing.T) {
|
||||
decryptService, err := ProvideDecryptService(cfg, tracer, mockStorage)
|
||||
require.NoError(t, err)
|
||||
|
||||
resp, err := decryptService.Decrypt(ctx, "default", "secure-value-1", "secure-value-2")
|
||||
resp, err := decryptService.Decrypt(ctx, "svc-name", "default", []string{"secure-value-1", "secure-value-2"})
|
||||
require.NotNil(t, resp)
|
||||
require.NoError(t, err)
|
||||
require.EqualValues(t, decryptedValuesResp, resp)
|
||||
@@ -106,7 +105,7 @@ func TestDecryptService(t *testing.T) {
|
||||
decryptService, err := ProvideDecryptService(cfg, tracer, mockStorage)
|
||||
require.NoError(t, err)
|
||||
|
||||
resp, err := decryptService.Decrypt(ctx, "default", "secure-value-1", "secure-value-2")
|
||||
resp, err := decryptService.Decrypt(ctx, "svc-name", "default", []string{"secure-value-1", "secure-value-2"})
|
||||
require.NotNil(t, resp)
|
||||
require.NoError(t, err)
|
||||
require.EqualValues(t, decryptedValuesResp, resp)
|
||||
@@ -236,9 +235,8 @@ func TestDecryptService(t *testing.T) {
|
||||
t.Cleanup(func() { require.NoError(t, decryptService.Close()) })
|
||||
|
||||
svcIdentity := "provsysoning-test"
|
||||
authCtx := identity.WithServiceIdentityContext(ctx, 1, identity.WithServiceIdentityName(svcIdentity))
|
||||
|
||||
result, err := decryptService.Decrypt(authCtx, namespace, "secure-value-1")
|
||||
result, err := decryptService.Decrypt(t.Context(), svcIdentity, namespace, []string{"secure-value-1"})
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, result)
|
||||
require.Len(t, result, 1)
|
||||
|
||||
@@ -254,9 +254,7 @@ func TestIntegration_InlineSecureValue_CreateInline(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
require.NotEmpty(t, createdName)
|
||||
|
||||
decryptAuthCtx := testutils.CreateServiceAuthContext(t.Context(), serviceIdentity, owner.Namespace, []string{"secret.grafana.app/securevalues:decrypt"})
|
||||
|
||||
decryptedValues, err := tu.DecryptService.Decrypt(decryptAuthCtx, owner.Namespace, createdName)
|
||||
decryptedValues, err := tu.DecryptService.Decrypt(t.Context(), serviceIdentity, owner.Namespace, []string{createdName})
|
||||
require.NoError(t, err)
|
||||
|
||||
decryptedResult, ok := decryptedValues[createdName]
|
||||
|
||||
@@ -403,9 +403,8 @@ func TestStateMachine(t *testing.T) {
|
||||
},
|
||||
"decrypt": func(t *rapid.T) {
|
||||
input := decryptGen.Draw(t, "decryptInput")
|
||||
authCtx := testutils.CreateServiceAuthContext(t.Context(), input.decrypter, input.namespace, []string{fmt.Sprintf("secret.grafana.app/securevalues/%+v:decrypt", input.name)})
|
||||
modelResult, modelErr := model.decrypt(input.decrypter, input.namespace, input.name)
|
||||
result, err := sut.DecryptService.Decrypt(authCtx, input.namespace, input.name)
|
||||
result, err := sut.DecryptService.Decrypt(t.Context(), input.decrypter, input.namespace, []string{input.name})
|
||||
if err != nil || modelErr != nil {
|
||||
require.ErrorIs(t, err, modelErr)
|
||||
return
|
||||
@@ -440,8 +439,7 @@ func TestSecureValueServiceExampleBased(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, sv.Status.Version, deletedSv.Status.Version)
|
||||
|
||||
authCtx := testutils.CreateServiceAuthContext(t.Context(), sv.Spec.Decrypters[0], sv.Namespace, []string{fmt.Sprintf("secret.grafana.app/securevalues/%+v:decrypt", sv.Name)})
|
||||
result, err := sut.DecryptService.Decrypt(authCtx, sv.Namespace, sv.Name)
|
||||
result, err := sut.DecryptService.Decrypt(t.Context(), sv.Spec.Decrypters[0], sv.Namespace, []string{sv.Name})
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, 1, len(result))
|
||||
require.ErrorIs(t, result[sv.Name].Error(), contracts.ErrDecryptNotFound)
|
||||
|
||||
Reference in New Issue
Block a user