From d9e099d48049672354dcaffdeb3abce9052ff14d Mon Sep 17 00:00:00 2001 From: Charandas <542168+charandas@users.noreply.github.com> Date: Thu, 17 Jul 2025 09:50:25 -0700 Subject: [PATCH] Feature-Flag service: signing middleware for cloud usecase (#107745) --- .github/CODEOWNERS | 1 + pkg/clientauth/middleware/token-exchange.go | 110 ++++++++++++++++++ .../featuremgmt/fake-token-exchange.go | 22 ++++ pkg/services/featuremgmt/goff_provider.go | 7 +- pkg/services/featuremgmt/openfeature.go | 61 +++++++++- pkg/services/featuremgmt/openfeature_test.go | 87 +++++++++++++- .../featuremgmt/static_provider_test.go | 1 + 7 files changed, 273 insertions(+), 16 deletions(-) create mode 100644 pkg/clientauth/middleware/token-exchange.go create mode 100644 pkg/services/featuremgmt/fake-token-exchange.go diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 2c4dd8322f9..ecd61d20be0 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -89,6 +89,7 @@ /pkg/apis/query @grafana/grafana-datasources-core-services /pkg/apis/userstorage @grafana/grafana-app-platform-squad @grafana/plugins-platform-backend /pkg/bus/ @grafana/grafana-search-and-storage +/pkg/clientauth/ @grafana/grafana-app-platform-squad /pkg/cmd/ @grafana/grafana-backend-group /pkg/cmd/grafana-cli/commands/install_command.go @grafana/plugins-platform-backend /pkg/cmd/grafana-cli/commands/install_command_test.go @grafana/plugins-platform-backend diff --git a/pkg/clientauth/middleware/token-exchange.go b/pkg/clientauth/middleware/token-exchange.go new file mode 100644 index 00000000000..1202aa30821 --- /dev/null +++ b/pkg/clientauth/middleware/token-exchange.go @@ -0,0 +1,110 @@ +package middleware + +import ( + "fmt" + "net/http" + + authlib "github.com/grafana/authlib/authn" + sdkhttpclient "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient" + + "github.com/grafana/grafana/pkg/apimachinery/identity" + infralog "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/setting" +) + +type TokenExchangeMiddleware struct { + tokenExchangeClient authlib.TokenExchanger +} + +type tokenExchangeMiddlewareImpl struct { + tokenExchangeClient authlib.TokenExchanger + audiences []string + next http.RoundTripper +} + +type signerSettings struct { + token string + tokenExchangeURL string +} + +var _ http.RoundTripper = &tokenExchangeMiddlewareImpl{} + +func TestingTokenExchangeMiddleware(tokenExchangeClient authlib.TokenExchanger) *TokenExchangeMiddleware { + return &TokenExchangeMiddleware{ + tokenExchangeClient: tokenExchangeClient, + } +} + +func NewTokenExchangeMiddleware(cfg *setting.Cfg) (*TokenExchangeMiddleware, error) { + clientCfg, err := readSignerSettings(cfg) + if err != nil { + return nil, err + } + + tokenExchangeClient, err := authlib.NewTokenExchangeClient(authlib.TokenExchangeConfig{ + Token: clientCfg.token, + TokenExchangeURL: clientCfg.tokenExchangeURL, + }) + if err != nil { + return nil, err + } + return &TokenExchangeMiddleware{ + tokenExchangeClient: tokenExchangeClient, + }, nil +} + +func (p *TokenExchangeMiddleware) New(audiences []string) sdkhttpclient.MiddlewareFunc { + return func(opts sdkhttpclient.Options, next http.RoundTripper) http.RoundTripper { + return &tokenExchangeMiddlewareImpl{ + tokenExchangeClient: p.tokenExchangeClient, + audiences: audiences, + next: next, + } + } +} + +func (m tokenExchangeMiddlewareImpl) RoundTrip(req *http.Request) (res *http.Response, e error) { + log := infralog.New("token-exchange-middleware") + + user, err := identity.GetRequester(req.Context()) + if err != nil { + return nil, err + } + + namespace := user.GetNamespace() + + if namespace == "" { + return nil, fmt.Errorf("cluster scoped resources are currently not supported") + } + + log.Debug("signing request", "url", req.URL.Path, "audience", m.audiences, "namespace", namespace) + token, err := m.tokenExchangeClient.Exchange(req.Context(), authlib.TokenExchangeRequest{ + Namespace: namespace, + Audiences: m.audiences, + }) + + if err != nil { + return nil, fmt.Errorf("failed to exchange token: %w", err) + } + req.Header.Set("X-Access-Token", "Bearer "+token.Token) + return m.next.RoundTrip(req) +} + +// we exercise the below code path in OSS but would rather have it fail +// instead of documenting these non-pertinent settings and requiring mock values for them. +// hence, the error return is handled above as non-critical and a mock +// exchange client is returned. +func readSignerSettings(cfg *setting.Cfg) (*signerSettings, error) { + grpcClientAuthSection := cfg.SectionWithEnvOverrides("grpc_client_authentication") + + s := &signerSettings{} + + s.token = grpcClientAuthSection.Key("token").MustString("") + s.tokenExchangeURL = grpcClientAuthSection.Key("token_exchange_url").MustString("") + + if s.token == "" || s.tokenExchangeURL == "" { + return nil, fmt.Errorf("authorization: missing token or tokenExchangeUrl") + } + + return s, nil +} diff --git a/pkg/services/featuremgmt/fake-token-exchange.go b/pkg/services/featuremgmt/fake-token-exchange.go new file mode 100644 index 00000000000..db5495a1cd9 --- /dev/null +++ b/pkg/services/featuremgmt/fake-token-exchange.go @@ -0,0 +1,22 @@ +package featuremgmt + +import ( + "context" + "errors" + + authlib "github.com/grafana/authlib/authn" + "github.com/stretchr/testify/mock" +) + +type fakeTokenExchangeClient struct { + expectedErr error + *mock.Mock +} + +func (c *fakeTokenExchangeClient) Exchange(ctx context.Context, r authlib.TokenExchangeRequest) (*authlib.TokenExchangeResponse, error) { + c.Called(ctx, r) + if c.expectedErr != nil { + return nil, errors.New("error signing token") + } + return &authlib.TokenExchangeResponse{Token: "signed-token"}, c.expectedErr +} diff --git a/pkg/services/featuremgmt/goff_provider.go b/pkg/services/featuremgmt/goff_provider.go index c9ec290d870..9aafe320145 100644 --- a/pkg/services/featuremgmt/goff_provider.go +++ b/pkg/services/featuremgmt/goff_provider.go @@ -2,19 +2,16 @@ package featuremgmt import ( "net/http" - "time" gofeatureflag "github.com/open-feature/go-sdk-contrib/providers/go-feature-flag/pkg" "github.com/open-feature/go-sdk/openfeature" ) -func newGOFFProvider(url string) (openfeature.FeatureProvider, error) { +func newGOFFProvider(url string, client *http.Client) (openfeature.FeatureProvider, error) { options := gofeatureflag.ProviderOptions{ Endpoint: url, // consider using github.com/grafana/grafana/pkg/infra/httpclient/provider.go - HTTPClient: &http.Client{ - Timeout: 10 * time.Second, - }, + HTTPClient: client, } provider, err := gofeatureflag.NewProvider(options) return provider, err diff --git a/pkg/services/featuremgmt/openfeature.go b/pkg/services/featuremgmt/openfeature.go index aef992e1ca5..9a47e76a6ba 100644 --- a/pkg/services/featuremgmt/openfeature.go +++ b/pkg/services/featuremgmt/openfeature.go @@ -2,20 +2,41 @@ package featuremgmt import ( "fmt" + "net/http" "net/url" + "time" + clientauthmiddleware "github.com/grafana/grafana/pkg/clientauth/middleware" "github.com/grafana/grafana/pkg/setting" + sdkhttpclient "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient" "github.com/open-feature/go-sdk/openfeature" ) +const ( + featuresProviderAudience = "features.grafana.app" +) + func InitOpenFeatureWithCfg(cfg *setting.Cfg) error { confFlags, err := setting.ReadFeatureTogglesFromInitFile(cfg.Raw.Section("feature_toggles")) if err != nil { return fmt.Errorf("failed to read feature flags from config: %w", err) } - err = initOpenFeature(cfg.OpenFeature.ProviderType, cfg.OpenFeature.URL, confFlags) + var httpcli *http.Client + if cfg.OpenFeature.ProviderType == setting.GOFFProviderType { + m, err := clientauthmiddleware.NewTokenExchangeMiddleware(cfg) + if err != nil { + return fmt.Errorf("failed to create token exchange middleware: %w", err) + } + + httpcli, err = goffHTTPClient(m) + if err != nil { + return err + } + } + + err = initOpenFeature(cfg.OpenFeature.ProviderType, cfg.OpenFeature.URL, confFlags, httpcli) if err != nil { return fmt.Errorf("failed to initialize OpenFeature: %w", err) } @@ -24,10 +45,15 @@ func InitOpenFeatureWithCfg(cfg *setting.Cfg) error { return nil } -func initOpenFeature(providerType string, u *url.URL, staticFlags map[string]bool) error { - p, err := createProvider(providerType, u, staticFlags) +func initOpenFeature( + providerType string, + u *url.URL, + staticFlags map[string]bool, + httpClient *http.Client, +) error { + p, err := createProvider(providerType, u, staticFlags, httpClient) if err != nil { - return fmt.Errorf("failed to create feature provider: type %s, %w", providerType, err) + return err } if err := openfeature.SetProviderAndWait(p); err != nil { @@ -37,7 +63,12 @@ func initOpenFeature(providerType string, u *url.URL, staticFlags map[string]boo return nil } -func createProvider(providerType string, u *url.URL, staticFlags map[string]bool) (openfeature.FeatureProvider, error) { +func createProvider( + providerType string, + u *url.URL, + staticFlags map[string]bool, + httpClient *http.Client, +) (openfeature.FeatureProvider, error) { if providerType != setting.GOFFProviderType { return newStaticProvider(staticFlags) } @@ -46,5 +77,23 @@ func createProvider(providerType string, u *url.URL, staticFlags map[string]bool return nil, fmt.Errorf("feature provider url is required for GOFFProviderType") } - return newGOFFProvider(u.String()) + return newGOFFProvider(u.String(), httpClient) +} + +func goffHTTPClient(m *clientauthmiddleware.TokenExchangeMiddleware) (*http.Client, error) { + httpcli, err := sdkhttpclient.NewProvider().New(sdkhttpclient.Options{ + TLS: &sdkhttpclient.TLSOptions{InsecureSkipVerify: true}, + Timeouts: &sdkhttpclient.TimeoutOptions{ + Timeout: 10 * time.Second, + }, + Middlewares: []sdkhttpclient.Middleware{ + m.New([]string{featuresProviderAudience}), + }, + }) + + if err != nil { + return nil, fmt.Errorf("failed to create http client for openfeature: %w", err) + } + + return httpcli, nil } diff --git a/pkg/services/featuremgmt/openfeature_test.go b/pkg/services/featuremgmt/openfeature_test.go index a58c8ade9e0..7a028ec0659 100644 --- a/pkg/services/featuremgmt/openfeature_test.go +++ b/pkg/services/featuremgmt/openfeature_test.go @@ -1,24 +1,33 @@ package featuremgmt import ( + "context" + "errors" "net/url" "testing" + "github.com/grafana/grafana/pkg/apimachinery/identity" + "github.com/grafana/grafana/pkg/clientauth/middleware" "github.com/grafana/grafana/pkg/setting" + authlib "github.com/grafana/authlib/authn" gofeatureflag "github.com/open-feature/go-sdk-contrib/providers/go-feature-flag/pkg" + "github.com/open-feature/go-sdk/openfeature" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) func TestCreateProvider(t *testing.T) { - u, err := url.Parse("http://localhost:1031") + u, err := url.Parse("http://localhost:10333") require.NoError(t, err) testCases := []struct { - name string - cfg setting.OpenFeatureSettings - expectedProvider string + name string + cfg setting.OpenFeatureSettings + expectedProvider string + expectExchangeRequest *authlib.TokenExchangeRequest + failSigning bool }{ { name: "static provider", @@ -31,7 +40,25 @@ func TestCreateProvider(t *testing.T) { URL: u, TargetingKey: "grafana", }, + expectExchangeRequest: &authlib.TokenExchangeRequest{ + Namespace: "*", + Audiences: []string{"features.grafana.app"}, + }, + expectedProvider: setting.GOFFProviderType, + }, + { + name: "goff provider with failing token exchange", + cfg: setting.OpenFeatureSettings{ + ProviderType: setting.GOFFProviderType, + URL: u, + TargetingKey: "grafana", + }, + expectExchangeRequest: &authlib.TokenExchangeRequest{ + Namespace: "*", + Audiences: []string{"features.grafana.app"}, + }, expectedProvider: setting.GOFFProviderType, + failSigning: true, }, { name: "invalid provider", @@ -42,14 +69,46 @@ func TestCreateProvider(t *testing.T) { }, } + require.NoError(t, err) + for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - provider, err := createProvider(tc.cfg.ProviderType, tc.cfg.URL, nil) + cfg := setting.NewCfg() + cfg.OpenFeature = tc.cfg + + var tokenExchangeClient *fakeTokenExchangeClient + + if tc.expectExchangeRequest != nil { + tokenExchangeClient = &fakeTokenExchangeClient{ + Mock: &mock.Mock{ + ExpectedCalls: []*mock.Call{ + { + Method: "Exchange", + Arguments: mock.Arguments{mock.Anything, *tc.expectExchangeRequest}, + }, + }, + }, + } + + if tc.failSigning { + tokenExchangeClient.expectedErr = errors.New("failed signing access token") + } + } + + tokenExchangeMiddleware := middleware.TestingTokenExchangeMiddleware(tokenExchangeClient) + goffClient, err := goffHTTPClient(tokenExchangeMiddleware) + require.NoError(t, err, "failed to create goff http client") + provider, err := createProvider(tc.cfg.ProviderType, tc.cfg.URL, nil, goffClient) require.NoError(t, err) + err = openfeature.SetProviderAndWait(provider) + require.NoError(t, err, "failed to set provider") + if tc.expectedProvider == setting.GOFFProviderType { _, ok := provider.(*gofeatureflag.Provider) assert.True(t, ok, "expected provider to be of type goff.Provider") + + testGoFFProvider(t, tc.failSigning) } else { _, ok := provider.(*inMemoryBulkProvider) assert.True(t, ok, "expected provider to be of type memprovider.InMemoryProvider") @@ -57,3 +116,21 @@ func TestCreateProvider(t *testing.T) { }) } } + +func testGoFFProvider(t *testing.T, failSigning bool) { + // this tests with a fake identity with * namespace access, but in any case, it proves what the requester + // is scoped to is what is used to sign the token with + ctx, _ := identity.WithServiceIdentity(context.Background(), 1) + + // Test that the flag evaluation can be attempted (though it will fail due to non-existent service) + // The important thing is that the authentication middleware is properly integrated + _, err := openfeature.GetApiInstance().GetClient().BooleanValueDetails(ctx, "test", false, openfeature.NewEvaluationContext("test", map[string]interface{}{"test": "test"})) + + // Error related to the token exchange should be returned if signing fails + // otherwise, it should return a connection refused error since the goff URL is not set + if failSigning { + assert.ErrorContains(t, err, "failed to exchange token: error signing token", "should return an error when signing fails") + } else { + assert.ErrorContains(t, err, "connect: connection refused", "should return an error when goff url is not set") + } +} diff --git a/pkg/services/featuremgmt/static_provider_test.go b/pkg/services/featuremgmt/static_provider_test.go index d47711f6269..93a45d899ba 100644 --- a/pkg/services/featuremgmt/static_provider_test.go +++ b/pkg/services/featuremgmt/static_provider_test.go @@ -51,6 +51,7 @@ func setup(t *testing.T, conf []byte) { t.Helper() cfg, err := setting.NewCfgFromBytes(conf) require.NoError(t, err) + err = InitOpenFeatureWithCfg(cfg) require.NoError(t, err) }