From 895bb0868c7e671ae13998a09fb0a518c06fbc3a Mon Sep 17 00:00:00 2001 From: Andreas Christou Date: Wed, 22 Nov 2023 23:53:17 +0000 Subject: [PATCH 01/16] Build out service principal creds for user-auth --- azcredentials/credentials.go | 1 + aztokenprovider/token_provider.go | 17 +++++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/azcredentials/credentials.go b/azcredentials/credentials.go index 4d3756a..89094ca 100644 --- a/azcredentials/credentials.go +++ b/azcredentials/credentials.go @@ -14,6 +14,7 @@ type AzureCredentials interface { // AadCurrentUserCredentials "Current User" user identity credentials of the current Grafana user. type AadCurrentUserCredentials struct { + ServicePrincipal AzureClientSecretCredentials } // AzureManagedIdentityCredentials "Managed Identity" service managed identity credentials configured diff --git a/aztokenprovider/token_provider.go b/aztokenprovider/token_provider.go index a661ec9..5aa8c1d 100644 --- a/aztokenprovider/token_provider.go +++ b/aztokenprovider/token_provider.go @@ -71,6 +71,14 @@ func NewAzureAccessTokenProvider(settings *azsettings.AzureSettings, credentials err = fmt.Errorf("user identity authentication is not enabled in Grafana config") return nil, err } + + var tokenRetriever TokenRetriever + if c.ServicePrincipal.ClientId != "" && c.ServicePrincipal.ClientSecret != "" && c.ServicePrincipal.TenantId != "" { + tokenRetriever, err = getClientSecretTokenRetriever(&c.ServicePrincipal) + if err != nil { + return nil, err + } + } tokenEndpoint := settings.UserIdentityTokenEndpoint client, err := NewTokenClient(tokenEndpoint.TokenUrl, tokenEndpoint.ClientId, tokenEndpoint.ClientSecret, http.DefaultClient) if err != nil { @@ -81,6 +89,7 @@ func NewAzureAccessTokenProvider(settings *azsettings.AzureSettings, credentials tokenCache: azureTokenCache, client: client, usernameAssertion: tokenEndpoint.UsernameAssertion, + tokenRetriever: tokenRetriever, }, nil default: err = fmt.Errorf("credentials of type '%s' not supported by Azure authentication provider", c.AzureAuthType()) @@ -114,6 +123,7 @@ type userTokenProvider struct { tokenCache ConcurrentTokenCache client TokenClient usernameAssertion bool + tokenRetriever TokenRetriever } func (provider *userTokenProvider) GetAccessToken(ctx context.Context, scopes []string) (string, error) { @@ -128,6 +138,13 @@ func (provider *userTokenProvider) GetAccessToken(ctx context.Context, scopes [] currentUser, ok := azusercontext.GetCurrentUser(ctx) if !ok { + if provider.tokenRetriever != nil { + accessToken, err := provider.tokenCache.GetAccessToken(ctx, provider.tokenRetriever, scopes) + if err != nil { + return "", err + } + return accessToken, nil + } err := fmt.Errorf("user context not configured") return "", err } From 96109cd15f99dbca8c540c95b7cfa54db3eb9359 Mon Sep 17 00:00:00 2001 From: Andreas Christou Date: Tue, 28 Nov 2023 17:54:22 +0000 Subject: [PATCH 02/16] Fix condition for service principal logic - If no user present OR no ID token AND usernameAssertion is disallowed - Update tests --- aztokenprovider/token_provider.go | 2 +- aztokenprovider/token_provider_test.go | 52 ++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/aztokenprovider/token_provider.go b/aztokenprovider/token_provider.go index 5aa8c1d..ddcad74 100644 --- a/aztokenprovider/token_provider.go +++ b/aztokenprovider/token_provider.go @@ -137,7 +137,7 @@ func (provider *userTokenProvider) GetAccessToken(ctx context.Context, scopes [] } currentUser, ok := azusercontext.GetCurrentUser(ctx) - if !ok { + if !ok || currentUser.IdToken == "" && !provider.usernameAssertion { if provider.tokenRetriever != nil { accessToken, err := provider.tokenCache.GetAccessToken(ctx, provider.tokenRetriever, scopes) if err != nil { diff --git a/aztokenprovider/token_provider_test.go b/aztokenprovider/token_provider_test.go index a1fd1e6..3f72312 100644 --- a/aztokenprovider/token_provider_test.go +++ b/aztokenprovider/token_provider_test.go @@ -110,6 +110,15 @@ func TestNewAzureAccessTokenProvider_ServiceIdentity(t *testing.T) { }) } +var mockUserCredentials = &azcredentials.AadCurrentUserCredentials{ + ServicePrincipal: azcredentials.AzureClientSecretCredentials{ + AzureCloud: azsettings.AzurePublic, + TenantId: "TEST-TENANT", + ClientId: "TEST-CLIENT-ID", + ClientSecret: "TEST-CLIENT-SECRET", + }, +} + func TestNewAzureAccessTokenProvider_UserIdentity(t *testing.T) { settingsNotConfigured := &azsettings.AzureSettings{} @@ -143,6 +152,13 @@ func TestNewAzureAccessTokenProvider_UserIdentity(t *testing.T) { require.NoError(t, err) require.IsType(t, &userTokenProvider{}, provider) }) + + t.Run("should return user provider with service principal credentials when user identity configured", func(t *testing.T) { + + provider, err := NewAzureAccessTokenProvider(settings, mockUserCredentials, true) + require.NoError(t, err) + require.IsType(t, &userTokenProvider{}, provider) + }) } func TestGetAccessToken_UserIdentity(t *testing.T) { @@ -240,4 +256,40 @@ func TestGetAccessToken_UserIdentity(t *testing.T) { _, err = provider.GetAccessToken(usrctx, scopes) require.NoError(t, err) }) + + t.Run("should use clientSecretTokenRetriever when service principal credentials are available without an access token or id token", func(t *testing.T) { + + tokenRetriever, _ := getClientSecretTokenRetriever(&mockUserCredentials.ServicePrincipal) + var provider AzureTokenProvider = &userTokenProvider{ + tokenCache: &tokenCacheFake{}, + tokenRetriever: tokenRetriever, + } + + getAccessTokenFunc = func(retriever TokenRetriever, scopes []string) { + assert.IsType(t, &clientSecretTokenRetriever{}, retriever) + } + + usrctx := azusercontext.WithCurrentUser(ctx, azusercontext.CurrentUserContext{ + User: &backend.User{}, + }) + + _, err = provider.GetAccessToken(usrctx, scopes) + require.NoError(t, err) + }) + + t.Run("should use clientSecretTokenRetriever when service principal credentials are available without a user in context", func(t *testing.T) { + + tokenRetriever, _ := getClientSecretTokenRetriever(&mockUserCredentials.ServicePrincipal) + var provider AzureTokenProvider = &userTokenProvider{ + tokenCache: &tokenCacheFake{}, + tokenRetriever: tokenRetriever, + } + + getAccessTokenFunc = func(retriever TokenRetriever, scopes []string) { + assert.IsType(t, &clientSecretTokenRetriever{}, retriever) + } + + _, err = provider.GetAccessToken(ctx, scopes) + require.NoError(t, err) + }) } From 5f666e1f8ace2eac0bd49a12ead0f24a76f7f759 Mon Sep 17 00:00:00 2001 From: Andreas Christou Date: Mon, 5 Feb 2024 21:02:23 +0000 Subject: [PATCH 03/16] Add service credentials fallback flag and default to true --- azsettings/env.go | 18 ++++++--- azsettings/env_test.go | 64 +++++++++++++++++++++++++++--- azsettings/settings.go | 9 ++++- azsettings/settings_test.go | 79 ++++++++++++++++++++----------------- 4 files changed, 122 insertions(+), 48 deletions(-) diff --git a/azsettings/env.go b/azsettings/env.go index 33e25a6..b2531ea 100644 --- a/azsettings/env.go +++ b/azsettings/env.go @@ -17,11 +17,12 @@ const ( WorkloadIdentityClientID = "GFAZPL_WORKLOAD_IDENTITY_CLIENT_ID" WorkloadIdentityTokenFile = "GFAZPL_WORKLOAD_IDENTITY_TOKEN_FILE" - UserIdentityEnabled = "GFAZPL_USER_IDENTITY_ENABLED" - UserIdentityTokenURL = "GFAZPL_USER_IDENTITY_TOKEN_URL" - UserIdentityClientID = "GFAZPL_USER_IDENTITY_CLIENT_ID" - UserIdentityClientSecret = "GFAZPL_USER_IDENTITY_CLIENT_SECRET" - UserIdentityAssertion = "GFAZPL_USER_IDENTITY_ASSERTION" + UserIdentityEnabled = "GFAZPL_USER_IDENTITY_ENABLED" + UserIdentityTokenURL = "GFAZPL_USER_IDENTITY_TOKEN_URL" + UserIdentityClientID = "GFAZPL_USER_IDENTITY_CLIENT_ID" + UserIdentityClientSecret = "GFAZPL_USER_IDENTITY_CLIENT_SECRET" + UserIdentityAssertion = "GFAZPL_USER_IDENTITY_ASSERTION" + UserIdentityServiceCredentials = "GFAZPL_USER_IDENTITY_SERVICE_CREDENTIALS" // Pre Grafana 9.x variables fallbackAzureCloud = "AZURE_CLOUD" @@ -79,6 +80,11 @@ func ReadFromEnv() (*AzureSettings, error) { assertion := envutil.GetOrDefault(UserIdentityAssertion, "") usernameAssertion := assertion == "username" + serviceCredentialsFallback, err := envutil.GetBoolOrDefault(UserIdentityServiceCredentials, true) + if err != nil { + return nil, err + } + azureSettings.UserIdentityEnabled = true azureSettings.UserIdentityTokenEndpoint = &TokenEndpointSettings{ TokenUrl: tokenUrl, @@ -86,6 +92,7 @@ func ReadFromEnv() (*AzureSettings, error) { ClientSecret: clientSecret, UsernameAssertion: usernameAssertion, } + azureSettings.UserIdentityServiceCredentials = serviceCredentialsFallback } return azureSettings, nil @@ -125,6 +132,7 @@ func WriteToEnvStr(azureSettings *AzureSettings) []string { if azureSettings.UserIdentityEnabled { envs = append(envs, fmt.Sprintf("%s=true", UserIdentityEnabled)) + envs = append(envs, fmt.Sprintf("%s=%t", UserIdentityServiceCredentials, azureSettings.UserIdentityServiceCredentials)) if tokenEndpoint := azureSettings.UserIdentityTokenEndpoint; tokenEndpoint != nil { if tokenEndpoint.TokenUrl != "" { diff --git a/azsettings/env_test.go b/azsettings/env_test.go index 72986a4..a45b880 100644 --- a/azsettings/env_test.go +++ b/azsettings/env_test.go @@ -315,6 +315,45 @@ func TestReadFromEnv(t *testing.T) { require.NotNil(t, azureSettings.UserIdentityTokenEndpoint) assert.Equal(t, "87808761-ff7b-492e-bb0d-5de2437ffa55", azureSettings.UserIdentityTokenEndpoint.ClientSecret) }) + + t.Run("should be enabled and default to enabling service credentials", func(t *testing.T) { + unset1, err := setEnvVar("GFAZPL_USER_IDENTITY_TOKEN_URL", "https://login.microsoftonline.com/fd719c11-a91c-40fd-8379-1e6cd3c59568/oauth2/v2.0/token") + require.NoError(t, err) + defer unset1() + unset2, err := setEnvVar("GFAZPL_USER_IDENTITY_CLIENT_ID", "f85aa887-490d-4fac-9306-9b99ad0aa31d") + require.NoError(t, err) + defer unset2() + unset3, err := setEnvVar("GFAZPL_USER_IDENTITY_CLIENT_SECRET", "87808761-ff7b-492e-bb0d-5de2437ffa55") + require.NoError(t, err) + defer unset3() + azureSettings, err := ReadFromEnv() + require.NoError(t, err) + + assert.True(t, azureSettings.UserIdentityEnabled) + assert.True(t, azureSettings.UserIdentityServiceCredentials) + + }) + + t.Run("should be enabled and disable service credentials", func(t *testing.T) { + unset1, err := setEnvVar("GFAZPL_USER_IDENTITY_TOKEN_URL", "https://login.microsoftonline.com/fd719c11-a91c-40fd-8379-1e6cd3c59568/oauth2/v2.0/token") + require.NoError(t, err) + defer unset1() + unset2, err := setEnvVar("GFAZPL_USER_IDENTITY_CLIENT_ID", "f85aa887-490d-4fac-9306-9b99ad0aa31d") + require.NoError(t, err) + defer unset2() + unset3, err := setEnvVar("GFAZPL_USER_IDENTITY_CLIENT_SECRET", "87808761-ff7b-492e-bb0d-5de2437ffa55") + require.NoError(t, err) + defer unset3() + unset4, err := setEnvVar("GFAZPL_USER_IDENTITY_SERVICE_CREDENTIALS", "false") + require.NoError(t, err) + defer unset4() + azureSettings, err := ReadFromEnv() + require.NoError(t, err) + + assert.True(t, azureSettings.UserIdentityEnabled) + assert.False(t, azureSettings.UserIdentityServiceCredentials) + + }) }) t.Run("when user identity disabled", func(t *testing.T) { @@ -461,8 +500,22 @@ func TestWriteToEnvStr(t *testing.T) { envs := WriteToEnvStr(azureSettings) - require.Len(t, envs, 1) + require.Len(t, envs, 2) + assert.Equal(t, "GFAZPL_USER_IDENTITY_ENABLED=true", envs[0]) + assert.Equal(t, "GFAZPL_USER_IDENTITY_SERVICE_CREDENTIALS=false", envs[1]) + }) + + t.Run("should return user identity set if enabled with service credentials enabled", func(t *testing.T) { + azureSettings := &AzureSettings{ + UserIdentityEnabled: true, + UserIdentityServiceCredentials: true, + } + + envs := WriteToEnvStr(azureSettings) + + require.Len(t, envs, 2) assert.Equal(t, "GFAZPL_USER_IDENTITY_ENABLED=true", envs[0]) + assert.Equal(t, "GFAZPL_USER_IDENTITY_SERVICE_CREDENTIALS=true", envs[1]) }) t.Run("should return user identity endpoint settings if user identity enabled", func(t *testing.T) { @@ -477,11 +530,12 @@ func TestWriteToEnvStr(t *testing.T) { envs := WriteToEnvStr(azureSettings) - assert.Len(t, envs, 4) + assert.Len(t, envs, 5) assert.Equal(t, "GFAZPL_USER_IDENTITY_ENABLED=true", envs[0]) - assert.Equal(t, "GFAZPL_USER_IDENTITY_TOKEN_URL=https://login.microsoftonline.com/fd719c11-a91c-40fd-8379-1e6cd3c59568/oauth2/v2.0/token", envs[1]) - assert.Equal(t, "GFAZPL_USER_IDENTITY_CLIENT_ID=f85aa887-490d-4fac-9306-9b99ad0aa31d", envs[2]) - assert.Equal(t, "GFAZPL_USER_IDENTITY_CLIENT_SECRET=87808761-ff7b-492e-bb0d-5de2437ffa55", envs[3]) + assert.Equal(t, "GFAZPL_USER_IDENTITY_SERVICE_CREDENTIALS=false", envs[1]) + assert.Equal(t, "GFAZPL_USER_IDENTITY_TOKEN_URL=https://login.microsoftonline.com/fd719c11-a91c-40fd-8379-1e6cd3c59568/oauth2/v2.0/token", envs[2]) + assert.Equal(t, "GFAZPL_USER_IDENTITY_CLIENT_ID=f85aa887-490d-4fac-9306-9b99ad0aa31d", envs[3]) + assert.Equal(t, "GFAZPL_USER_IDENTITY_CLIENT_SECRET=87808761-ff7b-492e-bb0d-5de2437ffa55", envs[4]) }) t.Run("should not return user identity endpoint settings if user identity not enabled", func(t *testing.T) { diff --git a/azsettings/settings.go b/azsettings/settings.go index d41dbbd..424f20f 100644 --- a/azsettings/settings.go +++ b/azsettings/settings.go @@ -15,8 +15,9 @@ type AzureSettings struct { WorkloadIdentityEnabled bool WorkloadIdentitySettings *WorkloadIdentitySettings - UserIdentityEnabled bool - UserIdentityTokenEndpoint *TokenEndpointSettings + UserIdentityEnabled bool + UserIdentityTokenEndpoint *TokenEndpointSettings + UserIdentityServiceCredentials bool // This field determines which plugins will receive the settings via plugin context ForwardSettingsPlugins []string @@ -74,6 +75,7 @@ func ReadFromContext(ctx context.Context) (*AzureSettings, bool) { hasSettings = true settings.UserIdentityTokenEndpoint = &TokenEndpointSettings{} + settings.UserIdentityServiceCredentials = true if v := cfg.Get(UserIdentityClientID); v != "" { settings.UserIdentityTokenEndpoint.ClientId = v @@ -87,6 +89,9 @@ func ReadFromContext(ctx context.Context) (*AzureSettings, bool) { if v := cfg.Get(UserIdentityAssertion); v == "username" { settings.UserIdentityTokenEndpoint.UsernameAssertion = true } + if v := cfg.Get(UserIdentityServiceCredentials); v == strconv.FormatBool(false) { + settings.UserIdentityServiceCredentials = false + } } if v := cfg.Get(WorkloadIdentityEnabled); v == strconv.FormatBool(true) { diff --git a/azsettings/settings_test.go b/azsettings/settings_test.go index 56e6c1f..177a5a5 100644 --- a/azsettings/settings_test.go +++ b/azsettings/settings_test.go @@ -43,24 +43,26 @@ func TestSettingsFromContext(t *testing.T) { { name: "azure settings in config", cfg: backend.NewGrafanaCfg(map[string]string{ - AzureCloud: AzurePublic, - ManagedIdentityEnabled: "true", - ManagedIdentityClientID: "mock_managed_identity_client_id", - UserIdentityEnabled: "true", - UserIdentityClientID: "mock_user_identity_client_id", - UserIdentityClientSecret: "mock_managed_identity_client_secret", - UserIdentityTokenURL: "mock_managed_identity_token_url", - UserIdentityAssertion: "username", - WorkloadIdentityEnabled: "true", - WorkloadIdentityClientID: "mock_workload_identity_client_id", - WorkloadIdentityTenantID: "mock_workload_identity_tenant_id", - WorkloadIdentityTokenFile: "mock_workload_identity_token_file", + AzureCloud: AzurePublic, + ManagedIdentityEnabled: "true", + ManagedIdentityClientID: "mock_managed_identity_client_id", + UserIdentityEnabled: "true", + UserIdentityClientID: "mock_user_identity_client_id", + UserIdentityClientSecret: "mock_managed_identity_client_secret", + UserIdentityTokenURL: "mock_managed_identity_token_url", + UserIdentityAssertion: "username", + UserIdentityServiceCredentials: "true", + WorkloadIdentityEnabled: "true", + WorkloadIdentityClientID: "mock_workload_identity_client_id", + WorkloadIdentityTenantID: "mock_workload_identity_tenant_id", + WorkloadIdentityTokenFile: "mock_workload_identity_token_file", }), expectedAzure: &AzureSettings{ - Cloud: AzurePublic, - ManagedIdentityEnabled: true, - ManagedIdentityClientId: "mock_managed_identity_client_id", - UserIdentityEnabled: true, + Cloud: AzurePublic, + ManagedIdentityEnabled: true, + ManagedIdentityClientId: "mock_managed_identity_client_id", + UserIdentityEnabled: true, + UserIdentityServiceCredentials: true, UserIdentityTokenEndpoint: &TokenEndpointSettings{ ClientId: "mock_user_identity_client_id", ClientSecret: "mock_managed_identity_client_secret", @@ -90,10 +92,11 @@ func TestSettingsFromContext(t *testing.T) { func TestReadSettings(t *testing.T) { expectedAzureContextSettings := &AzureSettings{ - Cloud: AzurePublic, - ManagedIdentityEnabled: true, - ManagedIdentityClientId: "mock_managed_identity_client_id", - UserIdentityEnabled: true, + Cloud: AzurePublic, + ManagedIdentityEnabled: true, + ManagedIdentityClientId: "mock_managed_identity_client_id", + UserIdentityEnabled: true, + UserIdentityServiceCredentials: false, UserIdentityTokenEndpoint: &TokenEndpointSettings{ ClientId: "mock_user_identity_client_id", ClientSecret: "mock_managed_identity_client_secret", @@ -109,10 +112,11 @@ func TestReadSettings(t *testing.T) { } expectedAzureEnvSettings := &AzureSettings{ - Cloud: "ENV_CLOUD", - ManagedIdentityEnabled: true, - ManagedIdentityClientId: "ENV_MI_CLIENT_ID", - UserIdentityEnabled: true, + Cloud: "ENV_CLOUD", + ManagedIdentityEnabled: true, + ManagedIdentityClientId: "ENV_MI_CLIENT_ID", + UserIdentityEnabled: true, + UserIdentityServiceCredentials: false, UserIdentityTokenEndpoint: &TokenEndpointSettings{ ClientId: "ENV_UI_CLIENT_ID", ClientSecret: "ENV_UI_CLIENT_SECRET", @@ -143,6 +147,8 @@ func TestReadSettings(t *testing.T) { defer unsetUITokenURL() unsetUIAssertion, _ := setEnvVar(UserIdentityAssertion, "username") defer unsetUIAssertion() + unsetUIServiceCredentials, _ := setEnvVar(UserIdentityServiceCredentials, "false") + defer unsetUIServiceCredentials() unsetWIEnabled, _ := setEnvVar(WorkloadIdentityEnabled, "true") defer unsetWIEnabled() unsetWIClientID, _ := setEnvVar(WorkloadIdentityClientID, "ENV_WI_CLIENT_ID") @@ -163,18 +169,19 @@ func TestReadSettings(t *testing.T) { { name: "read from context", cfg: backend.NewGrafanaCfg(map[string]string{ - AzureCloud: AzurePublic, - ManagedIdentityEnabled: "true", - ManagedIdentityClientID: "mock_managed_identity_client_id", - UserIdentityEnabled: "true", - UserIdentityClientID: "mock_user_identity_client_id", - UserIdentityClientSecret: "mock_managed_identity_client_secret", - UserIdentityTokenURL: "mock_managed_identity_token_url", - UserIdentityAssertion: "username", - WorkloadIdentityEnabled: "true", - WorkloadIdentityClientID: "mock_workload_identity_client_id", - WorkloadIdentityTenantID: "mock_workload_identity_tenant_id", - WorkloadIdentityTokenFile: "mock_workload_identity_token_file", + AzureCloud: AzurePublic, + ManagedIdentityEnabled: "true", + ManagedIdentityClientID: "mock_managed_identity_client_id", + UserIdentityEnabled: "true", + UserIdentityClientID: "mock_user_identity_client_id", + UserIdentityClientSecret: "mock_managed_identity_client_secret", + UserIdentityTokenURL: "mock_managed_identity_token_url", + UserIdentityAssertion: "username", + UserIdentityServiceCredentials: "false", + WorkloadIdentityEnabled: "true", + WorkloadIdentityClientID: "mock_workload_identity_client_id", + WorkloadIdentityTenantID: "mock_workload_identity_tenant_id", + WorkloadIdentityTokenFile: "mock_workload_identity_token_file", }), expectedAzure: expectedAzureContextSettings, expectedError: nil, From 1342bc39e786bf6f0f4ce7ca5206a8a22dc74a74 Mon Sep 17 00:00:00 2001 From: Andreas Christou Date: Mon, 5 Feb 2024 21:44:42 +0000 Subject: [PATCH 04/16] Rename --- azcredentials/credentials.go | 2 +- aztokenprovider/token_provider.go | 4 ++-- aztokenprovider/token_provider_test.go | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/azcredentials/credentials.go b/azcredentials/credentials.go index 89094ca..6fc999b 100644 --- a/azcredentials/credentials.go +++ b/azcredentials/credentials.go @@ -14,7 +14,7 @@ type AzureCredentials interface { // AadCurrentUserCredentials "Current User" user identity credentials of the current Grafana user. type AadCurrentUserCredentials struct { - ServicePrincipal AzureClientSecretCredentials + ServiceCredentials AzureClientSecretCredentials } // AzureManagedIdentityCredentials "Managed Identity" service managed identity credentials configured diff --git a/aztokenprovider/token_provider.go b/aztokenprovider/token_provider.go index ddcad74..96833ab 100644 --- a/aztokenprovider/token_provider.go +++ b/aztokenprovider/token_provider.go @@ -73,8 +73,8 @@ func NewAzureAccessTokenProvider(settings *azsettings.AzureSettings, credentials } var tokenRetriever TokenRetriever - if c.ServicePrincipal.ClientId != "" && c.ServicePrincipal.ClientSecret != "" && c.ServicePrincipal.TenantId != "" { - tokenRetriever, err = getClientSecretTokenRetriever(&c.ServicePrincipal) + if c.ServiceCredentials.ClientId != "" && c.ServiceCredentials.ClientSecret != "" && c.ServiceCredentials.TenantId != "" { + tokenRetriever, err = getClientSecretTokenRetriever(&c.ServiceCredentials) if err != nil { return nil, err } diff --git a/aztokenprovider/token_provider_test.go b/aztokenprovider/token_provider_test.go index 3f72312..4dd7d48 100644 --- a/aztokenprovider/token_provider_test.go +++ b/aztokenprovider/token_provider_test.go @@ -111,7 +111,7 @@ func TestNewAzureAccessTokenProvider_ServiceIdentity(t *testing.T) { } var mockUserCredentials = &azcredentials.AadCurrentUserCredentials{ - ServicePrincipal: azcredentials.AzureClientSecretCredentials{ + ServiceCredentials: azcredentials.AzureClientSecretCredentials{ AzureCloud: azsettings.AzurePublic, TenantId: "TEST-TENANT", ClientId: "TEST-CLIENT-ID", @@ -259,7 +259,7 @@ func TestGetAccessToken_UserIdentity(t *testing.T) { t.Run("should use clientSecretTokenRetriever when service principal credentials are available without an access token or id token", func(t *testing.T) { - tokenRetriever, _ := getClientSecretTokenRetriever(&mockUserCredentials.ServicePrincipal) + tokenRetriever, _ := getClientSecretTokenRetriever(&mockUserCredentials.ServiceCredentials) var provider AzureTokenProvider = &userTokenProvider{ tokenCache: &tokenCacheFake{}, tokenRetriever: tokenRetriever, @@ -279,7 +279,7 @@ func TestGetAccessToken_UserIdentity(t *testing.T) { t.Run("should use clientSecretTokenRetriever when service principal credentials are available without a user in context", func(t *testing.T) { - tokenRetriever, _ := getClientSecretTokenRetriever(&mockUserCredentials.ServicePrincipal) + tokenRetriever, _ := getClientSecretTokenRetriever(&mockUserCredentials.ServiceCredentials) var provider AzureTokenProvider = &userTokenProvider{ tokenCache: &tokenCacheFake{}, tokenRetriever: tokenRetriever, From 46705817f3b7007b10e57a7255c05b18672dfb2b Mon Sep 17 00:00:00 2001 From: Andreas Christou Date: Mon, 5 Feb 2024 21:55:09 +0000 Subject: [PATCH 05/16] Add missing arg --- aztokenprovider/token_provider.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aztokenprovider/token_provider.go b/aztokenprovider/token_provider.go index 4914c44..a9fe528 100644 --- a/aztokenprovider/token_provider.go +++ b/aztokenprovider/token_provider.go @@ -74,7 +74,7 @@ func NewAzureAccessTokenProvider(settings *azsettings.AzureSettings, credentials var tokenRetriever TokenRetriever if c.ServiceCredentials.ClientId != "" && c.ServiceCredentials.ClientSecret != "" && c.ServiceCredentials.TenantId != "" { - tokenRetriever, err = getClientSecretTokenRetriever(&c.ServiceCredentials) + tokenRetriever, err = getClientSecretTokenRetriever(settings, &c.ServiceCredentials) if err != nil { return nil, err } From ae0074aaa023cf249c036cfa6517c018665fff15 Mon Sep 17 00:00:00 2001 From: Andreas Christou Date: Mon, 5 Feb 2024 21:55:20 +0000 Subject: [PATCH 06/16] Only fallback if enabled --- aztokenprovider/token_provider.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/aztokenprovider/token_provider.go b/aztokenprovider/token_provider.go index a9fe528..ba54301 100644 --- a/aztokenprovider/token_provider.go +++ b/aztokenprovider/token_provider.go @@ -135,9 +135,13 @@ func (provider *userTokenProvider) GetAccessToken(ctx context.Context, scopes [] err := fmt.Errorf("parameter 'scopes' cannot be nil") return "", err } - + settings, err := azsettings.ReadSettings(ctx) + if err != nil { + err := fmt.Errorf("error reading azure settings: %s", err) + return "", err + } currentUser, ok := azusercontext.GetCurrentUser(ctx) - if !ok || currentUser.IdToken == "" && !provider.usernameAssertion { + if (!ok || (currentUser.IdToken == "" && !provider.usernameAssertion)) && settings.UserIdentityServiceCredentials { if provider.tokenRetriever != nil { accessToken, err := provider.tokenCache.GetAccessToken(ctx, provider.tokenRetriever, scopes) if err != nil { From 28083c1983644c86a35d4850d056c274b6176513 Mon Sep 17 00:00:00 2001 From: Andreas Christou Date: Mon, 5 Feb 2024 22:12:56 +0000 Subject: [PATCH 07/16] Fix tests --- aztokenprovider/token_provider_test.go | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/aztokenprovider/token_provider_test.go b/aztokenprovider/token_provider_test.go index 4dd7d48..fa5a3c2 100644 --- a/aztokenprovider/token_provider_test.go +++ b/aztokenprovider/token_provider_test.go @@ -257,9 +257,9 @@ func TestGetAccessToken_UserIdentity(t *testing.T) { require.NoError(t, err) }) - t.Run("should use clientSecretTokenRetriever when service principal credentials are available without an access token or id token", func(t *testing.T) { + t.Run("should use clientSecretTokenRetriever when service principal credentials are available without an access token or id token if enabled", func(t *testing.T) { - tokenRetriever, _ := getClientSecretTokenRetriever(&mockUserCredentials.ServiceCredentials) + tokenRetriever, _ := getClientSecretTokenRetriever(&azsettings.AzureSettings{UserIdentityServiceCredentials: true}, &mockUserCredentials.ServiceCredentials) var provider AzureTokenProvider = &userTokenProvider{ tokenCache: &tokenCacheFake{}, tokenRetriever: tokenRetriever, @@ -272,14 +272,18 @@ func TestGetAccessToken_UserIdentity(t *testing.T) { usrctx := azusercontext.WithCurrentUser(ctx, azusercontext.CurrentUserContext{ User: &backend.User{}, }) + settingsctx := backend.WithGrafanaConfig(usrctx, backend.NewGrafanaCfg(map[string]string{ + "GFAZPL_USER_IDENTITY_ENABLED": "true", + "GFAZPL_USER_IDENTITY_SERVICE_CREDENTIALS": "true", + })) - _, err = provider.GetAccessToken(usrctx, scopes) + _, err = provider.GetAccessToken(settingsctx, scopes) require.NoError(t, err) }) - t.Run("should use clientSecretTokenRetriever when service principal credentials are available without a user in context", func(t *testing.T) { + t.Run("should use clientSecretTokenRetriever when service principal credentials are available without a user in context if enabled", func(t *testing.T) { - tokenRetriever, _ := getClientSecretTokenRetriever(&mockUserCredentials.ServiceCredentials) + tokenRetriever, _ := getClientSecretTokenRetriever(&azsettings.AzureSettings{UserIdentityServiceCredentials: true}, &mockUserCredentials.ServiceCredentials) var provider AzureTokenProvider = &userTokenProvider{ tokenCache: &tokenCacheFake{}, tokenRetriever: tokenRetriever, @@ -288,8 +292,12 @@ func TestGetAccessToken_UserIdentity(t *testing.T) { getAccessTokenFunc = func(retriever TokenRetriever, scopes []string) { assert.IsType(t, &clientSecretTokenRetriever{}, retriever) } + settingsctx := backend.WithGrafanaConfig(ctx, backend.NewGrafanaCfg(map[string]string{ + "GFAZPL_USER_IDENTITY_ENABLED": "true", + "GFAZPL_USER_IDENTITY_SERVICE_CREDENTIALS": "true", + })) - _, err = provider.GetAccessToken(ctx, scopes) + _, err = provider.GetAccessToken(settingsctx, scopes) require.NoError(t, err) }) } From 6c2a45ac6294e5ebdf64ecae97d6aa013c281b26 Mon Sep 17 00:00:00 2001 From: Andreas Christou Date: Mon, 12 Feb 2024 16:42:26 +0000 Subject: [PATCH 08/16] Allow service credentials to be any reasonable type --- azcredentials/credentials.go | 2 +- aztokenprovider/token_provider.go | 21 +++++++++++++++++---- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/azcredentials/credentials.go b/azcredentials/credentials.go index 6fc999b..10be1e6 100644 --- a/azcredentials/credentials.go +++ b/azcredentials/credentials.go @@ -14,7 +14,7 @@ type AzureCredentials interface { // AadCurrentUserCredentials "Current User" user identity credentials of the current Grafana user. type AadCurrentUserCredentials struct { - ServiceCredentials AzureClientSecretCredentials + ServiceCredentials AzureCredentials } // AzureManagedIdentityCredentials "Managed Identity" service managed identity credentials configured diff --git a/aztokenprovider/token_provider.go b/aztokenprovider/token_provider.go index ba54301..3b18cbf 100644 --- a/aztokenprovider/token_provider.go +++ b/aztokenprovider/token_provider.go @@ -73,10 +73,23 @@ func NewAzureAccessTokenProvider(settings *azsettings.AzureSettings, credentials } var tokenRetriever TokenRetriever - if c.ServiceCredentials.ClientId != "" && c.ServiceCredentials.ClientSecret != "" && c.ServiceCredentials.TenantId != "" { - tokenRetriever, err = getClientSecretTokenRetriever(settings, &c.ServiceCredentials) - if err != nil { - return nil, err + + if c.ServiceCredentials != nil { + fallbackType := c.ServiceCredentials.AzureAuthType() + if fallbackType == azcredentials.AzureAuthCurrentUserIdentity || fallbackType == azcredentials.AzureAuthClientSecretObo { + err = fmt.Errorf("user identity authentication not valid for fallback credentials") + } + switch c.ServiceCredentials.(type) { + case *azcredentials.AzureClientSecretCredentials: + tokenRetriever, err = getClientSecretTokenRetriever(settings, c.ServiceCredentials.(*azcredentials.AzureClientSecretCredentials)) + if err != nil { + return nil, err + } + case *azcredentials.AzureManagedIdentityCredentials: + tokenRetriever = getManagedIdentityTokenRetriever(settings, c.ServiceCredentials.(*azcredentials.AzureManagedIdentityCredentials)) + case *azcredentials.AzureWorkloadIdentityCredentials: + tokenRetriever = getWorkloadIdentityTokenRetriever(settings, c.ServiceCredentials.(*azcredentials.AzureWorkloadIdentityCredentials)) + } } tokenEndpoint := settings.UserIdentityTokenEndpoint From 0acc5b139e1e79884031b30e31e6141ab02eefe2 Mon Sep 17 00:00:00 2001 From: Andreas Christou Date: Mon, 12 Feb 2024 16:42:46 +0000 Subject: [PATCH 09/16] Pass Grafana ID token to plugin --- azusercontext/context.go | 7 ++++--- azusercontext/from_req.go | 24 +++++++++++++++--------- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/azusercontext/context.go b/azusercontext/context.go index 7e036b4..00b61aa 100644 --- a/azusercontext/context.go +++ b/azusercontext/context.go @@ -10,9 +10,10 @@ type userCtxKey struct { } type CurrentUserContext struct { - User *backend.User - IdToken string - AccessToken string + User *backend.User + IdToken string + AccessToken string + GrafanaIdToken string } func WithCurrentUser(ctx context.Context, currentUser CurrentUserContext) context.Context { diff --git a/azusercontext/from_req.go b/azusercontext/from_req.go index e03dfaa..56836a4 100644 --- a/azusercontext/from_req.go +++ b/azusercontext/from_req.go @@ -14,11 +14,13 @@ func WithUserFromQueryReq(ctx context.Context, req *backend.QueryDataRequest) co idToken := getQueryReqHeader(req, "X-ID-Token") accessToken := extractBearerToken(getQueryReqHeader(req, "Authorization")) + grafanaIdToken := getQueryReqHeader(req, "http_X-Grafana-Id") currentUser := CurrentUserContext{ - User: req.PluginContext.User, - IdToken: idToken, - AccessToken: accessToken, + User: req.PluginContext.User, + IdToken: idToken, + AccessToken: accessToken, + GrafanaIdToken: grafanaIdToken, } return WithCurrentUser(ctx, currentUser) @@ -31,11 +33,13 @@ func WithUserFromResourceReq(ctx context.Context, req *backend.CallResourceReque idToken := getResourceReqHeader(req, "X-ID-Token") accessToken := extractBearerToken(getResourceReqHeader(req, "Authorization")) + grafanaIdToken := getResourceReqHeader(req, "X-Grafana-Id") currentUser := CurrentUserContext{ - User: req.PluginContext.User, - IdToken: idToken, - AccessToken: accessToken, + User: req.PluginContext.User, + IdToken: idToken, + AccessToken: accessToken, + GrafanaIdToken: grafanaIdToken, } return WithCurrentUser(ctx, currentUser) @@ -48,11 +52,13 @@ func WithUserFromHealthCheckReq(ctx context.Context, req *backend.CheckHealthReq idToken := getCheckHealthReqHeader(req, "X-ID-Token") accessToken := extractBearerToken(getCheckHealthReqHeader(req, "Authorization")) + grafanaIdToken := getCheckHealthReqHeader(req, "X-Grafana-Id") currentUser := CurrentUserContext{ - User: req.PluginContext.User, - IdToken: idToken, - AccessToken: accessToken, + User: req.PluginContext.User, + IdToken: idToken, + AccessToken: accessToken, + GrafanaIdToken: grafanaIdToken, } return WithCurrentUser(ctx, currentUser) From 16e3e1566adbdb179e73651cb50a65575b4a1461 Mon Sep 17 00:00:00 2001 From: Andreas Christou Date: Mon, 12 Feb 2024 18:23:21 +0000 Subject: [PATCH 10/16] Additional fallback validation logic --- aztokenprovider/token_provider.go | 68 ++++++++++++++++++++++---- aztokenprovider/token_provider_test.go | 2 +- 2 files changed, 60 insertions(+), 10 deletions(-) diff --git a/aztokenprovider/token_provider.go b/aztokenprovider/token_provider.go index 3b18cbf..9d709f2 100644 --- a/aztokenprovider/token_provider.go +++ b/aztokenprovider/token_provider.go @@ -6,9 +6,11 @@ import ( "fmt" "net/http" + "github.com/golang-jwt/jwt/v5" "github.com/grafana/grafana-azure-sdk-go/azcredentials" "github.com/grafana/grafana-azure-sdk-go/azsettings" "github.com/grafana/grafana-azure-sdk-go/azusercontext" + "github.com/grafana/grafana-plugin-sdk-go/backend" ) var ( @@ -139,6 +141,38 @@ type userTokenProvider struct { tokenRetriever TokenRetriever } +func isAzureUser(currentUser azusercontext.CurrentUserContext) (azusercontext.CurrentUserContext, error) { + // This should always be present for logged in users. idForwarding feature toggle must be enabled. By default we return true if there's no ID token available + if currentUser.GrafanaIdToken != "" { + claims := jwt.MapClaims{} + _, _, err := jwt.NewParser(jwt.WithValidMethods([]string{"ES256"})).ParseUnverified(currentUser.GrafanaIdToken, claims) + if err != nil { + return currentUser, fmt.Errorf("failed to decode user jwt: %s", err) + } + if claims["authenticatedBy"] != "oauth_azuread" { + return currentUser, fmt.Errorf("user is not authenticated with Azure AD") + } + + return currentUser, nil + } + + return currentUser, nil +} + +func isBackendRequest(currentUser azusercontext.CurrentUserContext, idForwardingEnabled bool) bool { + // If the User struct is nil or there is no ID token then it's a backend initiated request + if currentUser.User == nil { + return true + } + + // If ID forwarding is enabled we can assume this is backend initiated when empty + if currentUser.GrafanaIdToken == "" && idForwardingEnabled { + return true + } + + return false +} + func (provider *userTokenProvider) GetAccessToken(ctx context.Context, scopes []string) (string, error) { if ctx == nil { err := fmt.Errorf("parameter 'ctx' cannot be nil") @@ -153,20 +187,36 @@ func (provider *userTokenProvider) GetAccessToken(ctx context.Context, scopes [] err := fmt.Errorf("error reading azure settings: %s", err) return "", err } + currentUser, ok := azusercontext.GetCurrentUser(ctx) - if (!ok || (currentUser.IdToken == "" && !provider.usernameAssertion)) && settings.UserIdentityServiceCredentials { - if provider.tokenRetriever != nil { - accessToken, err := provider.tokenCache.GetAccessToken(ctx, provider.tokenRetriever, scopes) - if err != nil { - return "", err + if !ok { + return "", fmt.Errorf("user context not configured") + } + + idForwardingSupported := backend.GrafanaConfigFromContext(ctx).FeatureToggles().IsEnabled("IDForwarding") + backendRequest := isBackendRequest(currentUser, idForwardingSupported) + + if backendRequest { + // Use fallback credentials if this is a backend request and fallback credentials are enabled + if settings.UserIdentityEnabled && !provider.usernameAssertion { + if provider.tokenRetriever != nil { + accessToken, err := provider.tokenCache.GetAccessToken(ctx, provider.tokenRetriever, scopes) + if err != nil { + return "", err + } + return accessToken, nil } - return accessToken, nil } - err := fmt.Errorf("user context not configured") + + return "", fmt.Errorf("user context not configured") + } + + azureUser, err := isAzureUser(currentUser) + if err != nil { return "", err } - username, err := extractUsername(currentUser) + username, err := extractUsername(azureUser) if err != nil { err := fmt.Errorf("user identity authentication only possible in context of a Grafana user: %w", err) return "", err @@ -179,7 +229,7 @@ func (provider *userTokenProvider) GetAccessToken(ctx context.Context, scopes [] username: username, } } else { - idToken := currentUser.IdToken + idToken := azureUser.IdToken if idToken == "" { err := fmt.Errorf("user identity authentication not possible because there's no ID token associated with the Grafana user") return "", err diff --git a/aztokenprovider/token_provider_test.go b/aztokenprovider/token_provider_test.go index fa5a3c2..d372150 100644 --- a/aztokenprovider/token_provider_test.go +++ b/aztokenprovider/token_provider_test.go @@ -111,7 +111,7 @@ func TestNewAzureAccessTokenProvider_ServiceIdentity(t *testing.T) { } var mockUserCredentials = &azcredentials.AadCurrentUserCredentials{ - ServiceCredentials: azcredentials.AzureClientSecretCredentials{ + ServiceCredentials: &azcredentials.AzureClientSecretCredentials{ AzureCloud: azsettings.AzurePublic, TenantId: "TEST-TENANT", ClientId: "TEST-CLIENT-ID", From 9520b087dba28c40915b06b517ed3069fcbf823e Mon Sep 17 00:00:00 2001 From: Andreas Christou Date: Mon, 12 Feb 2024 18:23:28 +0000 Subject: [PATCH 11/16] Fix header name --- azusercontext/from_req.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azusercontext/from_req.go b/azusercontext/from_req.go index 56836a4..6a20a91 100644 --- a/azusercontext/from_req.go +++ b/azusercontext/from_req.go @@ -52,7 +52,7 @@ func WithUserFromHealthCheckReq(ctx context.Context, req *backend.CheckHealthReq idToken := getCheckHealthReqHeader(req, "X-ID-Token") accessToken := extractBearerToken(getCheckHealthReqHeader(req, "Authorization")) - grafanaIdToken := getCheckHealthReqHeader(req, "X-Grafana-Id") + grafanaIdToken := getCheckHealthReqHeader(req, "http_X-Grafana-Id") currentUser := CurrentUserContext{ User: req.PluginContext.User, From 8f1130bebecb57237d3d19f3e04ad333aabf771e Mon Sep 17 00:00:00 2001 From: Andreas Christou Date: Mon, 12 Feb 2024 18:38:21 +0000 Subject: [PATCH 12/16] Fix feature toggle name --- aztokenprovider/token_provider.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aztokenprovider/token_provider.go b/aztokenprovider/token_provider.go index 9d709f2..3edc73f 100644 --- a/aztokenprovider/token_provider.go +++ b/aztokenprovider/token_provider.go @@ -193,7 +193,7 @@ func (provider *userTokenProvider) GetAccessToken(ctx context.Context, scopes [] return "", fmt.Errorf("user context not configured") } - idForwardingSupported := backend.GrafanaConfigFromContext(ctx).FeatureToggles().IsEnabled("IDForwarding") + idForwardingSupported := backend.GrafanaConfigFromContext(ctx).FeatureToggles().IsEnabled("idForwarding") backendRequest := isBackendRequest(currentUser, idForwardingSupported) if backendRequest { From 6374b3752ad01199bd70da12f196fb4c47538216 Mon Sep 17 00:00:00 2001 From: Andreas Christou Date: Wed, 14 Feb 2024 14:54:47 +0000 Subject: [PATCH 13/16] Rename --- azsettings/env.go | 18 +++--- azsettings/env_test.go | 16 ++--- azsettings/settings.go | 12 ++-- azsettings/settings_test.go | 88 +++++++++++++------------- aztokenprovider/token_provider_test.go | 12 ++-- 5 files changed, 73 insertions(+), 73 deletions(-) diff --git a/azsettings/env.go b/azsettings/env.go index 2719e40..2f2dcd5 100644 --- a/azsettings/env.go +++ b/azsettings/env.go @@ -19,12 +19,12 @@ const ( WorkloadIdentityClientID = "GFAZPL_WORKLOAD_IDENTITY_CLIENT_ID" WorkloadIdentityTokenFile = "GFAZPL_WORKLOAD_IDENTITY_TOKEN_FILE" - UserIdentityEnabled = "GFAZPL_USER_IDENTITY_ENABLED" - UserIdentityTokenURL = "GFAZPL_USER_IDENTITY_TOKEN_URL" - UserIdentityClientID = "GFAZPL_USER_IDENTITY_CLIENT_ID" - UserIdentityClientSecret = "GFAZPL_USER_IDENTITY_CLIENT_SECRET" - UserIdentityAssertion = "GFAZPL_USER_IDENTITY_ASSERTION" - UserIdentityServiceCredentials = "GFAZPL_USER_IDENTITY_SERVICE_CREDENTIALS" + UserIdentityEnabled = "GFAZPL_USER_IDENTITY_ENABLED" + UserIdentityTokenURL = "GFAZPL_USER_IDENTITY_TOKEN_URL" + UserIdentityClientID = "GFAZPL_USER_IDENTITY_CLIENT_ID" + UserIdentityClientSecret = "GFAZPL_USER_IDENTITY_CLIENT_SECRET" + UserIdentityAssertion = "GFAZPL_USER_IDENTITY_ASSERTION" + UserIdentityFallbackCredentialsEnabled = "GFAZPL_USER_IDENTITY_FALLBACK_SERVICE_CREDENTIALS_ENABLED" // Pre Grafana 9.x variables fallbackAzureCloud = "AZURE_CLOUD" @@ -90,7 +90,7 @@ func ReadFromEnv() (*AzureSettings, error) { assertion := envutil.GetOrDefault(UserIdentityAssertion, "") usernameAssertion := assertion == "username" - serviceCredentialsFallback, err := envutil.GetBoolOrDefault(UserIdentityServiceCredentials, true) + serviceCredentialsFallback, err := envutil.GetBoolOrDefault(UserIdentityFallbackCredentialsEnabled, true) if err != nil { return nil, err } @@ -102,7 +102,7 @@ func ReadFromEnv() (*AzureSettings, error) { ClientSecret: clientSecret, UsernameAssertion: usernameAssertion, } - azureSettings.UserIdentityServiceCredentials = serviceCredentialsFallback + azureSettings.UserIdentityFallbackCredentialsEnabled = serviceCredentialsFallback } return azureSettings, nil @@ -146,7 +146,7 @@ func WriteToEnvStr(azureSettings *AzureSettings) []string { if azureSettings.UserIdentityEnabled { envs = append(envs, fmt.Sprintf("%s=true", UserIdentityEnabled)) - envs = append(envs, fmt.Sprintf("%s=%t", UserIdentityServiceCredentials, azureSettings.UserIdentityServiceCredentials)) + envs = append(envs, fmt.Sprintf("%s=%t", UserIdentityFallbackCredentialsEnabled, azureSettings.UserIdentityFallbackCredentialsEnabled)) if tokenEndpoint := azureSettings.UserIdentityTokenEndpoint; tokenEndpoint != nil { if tokenEndpoint.TokenUrl != "" { diff --git a/azsettings/env_test.go b/azsettings/env_test.go index ead920b..3ba723f 100644 --- a/azsettings/env_test.go +++ b/azsettings/env_test.go @@ -341,7 +341,7 @@ func TestReadFromEnv(t *testing.T) { require.NoError(t, err) assert.True(t, azureSettings.UserIdentityEnabled) - assert.True(t, azureSettings.UserIdentityServiceCredentials) + assert.True(t, azureSettings.UserIdentityFallbackCredentialsEnabled) }) @@ -355,14 +355,14 @@ func TestReadFromEnv(t *testing.T) { unset3, err := setEnvVar("GFAZPL_USER_IDENTITY_CLIENT_SECRET", "87808761-ff7b-492e-bb0d-5de2437ffa55") require.NoError(t, err) defer unset3() - unset4, err := setEnvVar("GFAZPL_USER_IDENTITY_SERVICE_CREDENTIALS", "false") + unset4, err := setEnvVar("GFAZPL_USER_IDENTITY_FALLBACK_SERVICE_CREDENTIALS_ENABLED", "false") require.NoError(t, err) defer unset4() azureSettings, err := ReadFromEnv() require.NoError(t, err) assert.True(t, azureSettings.UserIdentityEnabled) - assert.False(t, azureSettings.UserIdentityServiceCredentials) + assert.False(t, azureSettings.UserIdentityFallbackCredentialsEnabled) }) }) @@ -524,20 +524,20 @@ func TestWriteToEnvStr(t *testing.T) { require.Len(t, envs, 2) assert.Equal(t, "GFAZPL_USER_IDENTITY_ENABLED=true", envs[0]) - assert.Equal(t, "GFAZPL_USER_IDENTITY_SERVICE_CREDENTIALS=false", envs[1]) + assert.Equal(t, "GFAZPL_USER_IDENTITY_FALLBACK_SERVICE_CREDENTIALS_ENABLED=false", envs[1]) }) t.Run("should return user identity set if enabled with service credentials enabled", func(t *testing.T) { azureSettings := &AzureSettings{ - UserIdentityEnabled: true, - UserIdentityServiceCredentials: true, + UserIdentityEnabled: true, + UserIdentityFallbackCredentialsEnabled: true, } envs := WriteToEnvStr(azureSettings) require.Len(t, envs, 2) assert.Equal(t, "GFAZPL_USER_IDENTITY_ENABLED=true", envs[0]) - assert.Equal(t, "GFAZPL_USER_IDENTITY_SERVICE_CREDENTIALS=true", envs[1]) + assert.Equal(t, "GFAZPL_USER_IDENTITY_FALLBACK_SERVICE_CREDENTIALS_ENABLED=true", envs[1]) }) t.Run("should return user identity endpoint settings if user identity enabled", func(t *testing.T) { @@ -554,7 +554,7 @@ func TestWriteToEnvStr(t *testing.T) { assert.Len(t, envs, 5) assert.Equal(t, "GFAZPL_USER_IDENTITY_ENABLED=true", envs[0]) - assert.Equal(t, "GFAZPL_USER_IDENTITY_SERVICE_CREDENTIALS=false", envs[1]) + assert.Equal(t, "GFAZPL_USER_IDENTITY_FALLBACK_SERVICE_CREDENTIALS_ENABLED=false", envs[1]) assert.Equal(t, "GFAZPL_USER_IDENTITY_TOKEN_URL=https://login.microsoftonline.com/fd719c11-a91c-40fd-8379-1e6cd3c59568/oauth2/v2.0/token", envs[2]) assert.Equal(t, "GFAZPL_USER_IDENTITY_CLIENT_ID=f85aa887-490d-4fac-9306-9b99ad0aa31d", envs[3]) assert.Equal(t, "GFAZPL_USER_IDENTITY_CLIENT_SECRET=87808761-ff7b-492e-bb0d-5de2437ffa55", envs[4]) diff --git a/azsettings/settings.go b/azsettings/settings.go index 43c9fb1..39a6baf 100644 --- a/azsettings/settings.go +++ b/azsettings/settings.go @@ -16,9 +16,9 @@ type AzureSettings struct { WorkloadIdentityEnabled bool WorkloadIdentitySettings *WorkloadIdentitySettings - UserIdentityEnabled bool - UserIdentityTokenEndpoint *TokenEndpointSettings - UserIdentityServiceCredentials bool + UserIdentityEnabled bool + UserIdentityTokenEndpoint *TokenEndpointSettings + UserIdentityFallbackCredentialsEnabled bool // This field determines which plugins will receive the settings via plugin context ForwardSettingsPlugins []string @@ -80,7 +80,7 @@ func ReadFromContext(ctx context.Context) (*AzureSettings, bool) { hasSettings = true settings.UserIdentityTokenEndpoint = &TokenEndpointSettings{} - settings.UserIdentityServiceCredentials = true + settings.UserIdentityFallbackCredentialsEnabled = true if v := cfg.Get(UserIdentityClientID); v != "" { settings.UserIdentityTokenEndpoint.ClientId = v @@ -94,8 +94,8 @@ func ReadFromContext(ctx context.Context) (*AzureSettings, bool) { if v := cfg.Get(UserIdentityAssertion); v == "username" { settings.UserIdentityTokenEndpoint.UsernameAssertion = true } - if v := cfg.Get(UserIdentityServiceCredentials); v == strconv.FormatBool(false) { - settings.UserIdentityServiceCredentials = false + if v := cfg.Get(UserIdentityFallbackCredentialsEnabled); v == strconv.FormatBool(false) { + settings.UserIdentityFallbackCredentialsEnabled = false } } diff --git a/azsettings/settings_test.go b/azsettings/settings_test.go index 3a24ecb..b5f1286 100644 --- a/azsettings/settings_test.go +++ b/azsettings/settings_test.go @@ -43,28 +43,28 @@ func TestSettingsFromContext(t *testing.T) { { name: "azure settings in config", cfg: backend.NewGrafanaCfg(map[string]string{ - AzureCloud: AzurePublic, - AzureAuthEnabled: "true", - ManagedIdentityEnabled: "true", - ManagedIdentityClientID: "mock_managed_identity_client_id", - UserIdentityEnabled: "true", - UserIdentityClientID: "mock_user_identity_client_id", - UserIdentityClientSecret: "mock_managed_identity_client_secret", - UserIdentityTokenURL: "mock_managed_identity_token_url", - UserIdentityAssertion: "username", - UserIdentityServiceCredentials: "true", - WorkloadIdentityEnabled: "true", - WorkloadIdentityClientID: "mock_workload_identity_client_id", - WorkloadIdentityTenantID: "mock_workload_identity_tenant_id", - WorkloadIdentityTokenFile: "mock_workload_identity_token_file", + AzureCloud: AzurePublic, + AzureAuthEnabled: "true", + ManagedIdentityEnabled: "true", + ManagedIdentityClientID: "mock_managed_identity_client_id", + UserIdentityEnabled: "true", + UserIdentityClientID: "mock_user_identity_client_id", + UserIdentityClientSecret: "mock_managed_identity_client_secret", + UserIdentityTokenURL: "mock_managed_identity_token_url", + UserIdentityAssertion: "username", + UserIdentityFallbackCredentialsEnabled: "true", + WorkloadIdentityEnabled: "true", + WorkloadIdentityClientID: "mock_workload_identity_client_id", + WorkloadIdentityTenantID: "mock_workload_identity_tenant_id", + WorkloadIdentityTokenFile: "mock_workload_identity_token_file", }), expectedAzure: &AzureSettings{ - Cloud: AzurePublic, - AzureAuthEnabled: true, - ManagedIdentityEnabled: true, - ManagedIdentityClientId: "mock_managed_identity_client_id", - UserIdentityEnabled: true, - UserIdentityServiceCredentials: true, + Cloud: AzurePublic, + AzureAuthEnabled: true, + ManagedIdentityEnabled: true, + ManagedIdentityClientId: "mock_managed_identity_client_id", + UserIdentityEnabled: true, + UserIdentityFallbackCredentialsEnabled: true, UserIdentityTokenEndpoint: &TokenEndpointSettings{ ClientId: "mock_user_identity_client_id", ClientSecret: "mock_managed_identity_client_secret", @@ -94,11 +94,11 @@ func TestSettingsFromContext(t *testing.T) { func TestReadSettings(t *testing.T) { expectedAzureContextSettings := &AzureSettings{ - Cloud: AzurePublic, - ManagedIdentityEnabled: true, - ManagedIdentityClientId: "mock_managed_identity_client_id", - UserIdentityEnabled: true, - UserIdentityServiceCredentials: false, + Cloud: AzurePublic, + ManagedIdentityEnabled: true, + ManagedIdentityClientId: "mock_managed_identity_client_id", + UserIdentityEnabled: true, + UserIdentityFallbackCredentialsEnabled: false, UserIdentityTokenEndpoint: &TokenEndpointSettings{ ClientId: "mock_user_identity_client_id", ClientSecret: "mock_managed_identity_client_secret", @@ -114,11 +114,11 @@ func TestReadSettings(t *testing.T) { } expectedAzureEnvSettings := &AzureSettings{ - Cloud: "ENV_CLOUD", - ManagedIdentityEnabled: true, - ManagedIdentityClientId: "ENV_MI_CLIENT_ID", - UserIdentityEnabled: true, - UserIdentityServiceCredentials: false, + Cloud: "ENV_CLOUD", + ManagedIdentityEnabled: true, + ManagedIdentityClientId: "ENV_MI_CLIENT_ID", + UserIdentityEnabled: true, + UserIdentityFallbackCredentialsEnabled: false, UserIdentityTokenEndpoint: &TokenEndpointSettings{ ClientId: "ENV_UI_CLIENT_ID", ClientSecret: "ENV_UI_CLIENT_SECRET", @@ -149,7 +149,7 @@ func TestReadSettings(t *testing.T) { defer unsetUITokenURL() unsetUIAssertion, _ := setEnvVar(UserIdentityAssertion, "username") defer unsetUIAssertion() - unsetUIServiceCredentials, _ := setEnvVar(UserIdentityServiceCredentials, "false") + unsetUIServiceCredentials, _ := setEnvVar(UserIdentityFallbackCredentialsEnabled, "false") defer unsetUIServiceCredentials() unsetWIEnabled, _ := setEnvVar(WorkloadIdentityEnabled, "true") defer unsetWIEnabled() @@ -171,19 +171,19 @@ func TestReadSettings(t *testing.T) { { name: "read from context", cfg: backend.NewGrafanaCfg(map[string]string{ - AzureCloud: AzurePublic, - ManagedIdentityEnabled: "true", - ManagedIdentityClientID: "mock_managed_identity_client_id", - UserIdentityEnabled: "true", - UserIdentityClientID: "mock_user_identity_client_id", - UserIdentityClientSecret: "mock_managed_identity_client_secret", - UserIdentityTokenURL: "mock_managed_identity_token_url", - UserIdentityAssertion: "username", - UserIdentityServiceCredentials: "false", - WorkloadIdentityEnabled: "true", - WorkloadIdentityClientID: "mock_workload_identity_client_id", - WorkloadIdentityTenantID: "mock_workload_identity_tenant_id", - WorkloadIdentityTokenFile: "mock_workload_identity_token_file", + AzureCloud: AzurePublic, + ManagedIdentityEnabled: "true", + ManagedIdentityClientID: "mock_managed_identity_client_id", + UserIdentityEnabled: "true", + UserIdentityClientID: "mock_user_identity_client_id", + UserIdentityClientSecret: "mock_managed_identity_client_secret", + UserIdentityTokenURL: "mock_managed_identity_token_url", + UserIdentityAssertion: "username", + UserIdentityFallbackCredentialsEnabled: "false", + WorkloadIdentityEnabled: "true", + WorkloadIdentityClientID: "mock_workload_identity_client_id", + WorkloadIdentityTenantID: "mock_workload_identity_tenant_id", + WorkloadIdentityTokenFile: "mock_workload_identity_token_file", }), expectedAzure: expectedAzureContextSettings, expectedError: nil, diff --git a/aztokenprovider/token_provider_test.go b/aztokenprovider/token_provider_test.go index d372150..911dca6 100644 --- a/aztokenprovider/token_provider_test.go +++ b/aztokenprovider/token_provider_test.go @@ -259,7 +259,7 @@ func TestGetAccessToken_UserIdentity(t *testing.T) { t.Run("should use clientSecretTokenRetriever when service principal credentials are available without an access token or id token if enabled", func(t *testing.T) { - tokenRetriever, _ := getClientSecretTokenRetriever(&azsettings.AzureSettings{UserIdentityServiceCredentials: true}, &mockUserCredentials.ServiceCredentials) + tokenRetriever, _ := getClientSecretTokenRetriever(&azsettings.AzureSettings{UserIdentityFallbackCredentialsEnabled: true}, &mockUserCredentials.ServiceCredentials) var provider AzureTokenProvider = &userTokenProvider{ tokenCache: &tokenCacheFake{}, tokenRetriever: tokenRetriever, @@ -273,8 +273,8 @@ func TestGetAccessToken_UserIdentity(t *testing.T) { User: &backend.User{}, }) settingsctx := backend.WithGrafanaConfig(usrctx, backend.NewGrafanaCfg(map[string]string{ - "GFAZPL_USER_IDENTITY_ENABLED": "true", - "GFAZPL_USER_IDENTITY_SERVICE_CREDENTIALS": "true", + "GFAZPL_USER_IDENTITY_ENABLED": "true", + "GFAZPL_USER_IDENTITY_FALLBACK_SERVICE_CREDENTIALS_ENABLED": "true", })) _, err = provider.GetAccessToken(settingsctx, scopes) @@ -283,7 +283,7 @@ func TestGetAccessToken_UserIdentity(t *testing.T) { t.Run("should use clientSecretTokenRetriever when service principal credentials are available without a user in context if enabled", func(t *testing.T) { - tokenRetriever, _ := getClientSecretTokenRetriever(&azsettings.AzureSettings{UserIdentityServiceCredentials: true}, &mockUserCredentials.ServiceCredentials) + tokenRetriever, _ := getClientSecretTokenRetriever(&azsettings.AzureSettings{UserIdentityFallbackCredentialsEnabled: true}, &mockUserCredentials.ServiceCredentials) var provider AzureTokenProvider = &userTokenProvider{ tokenCache: &tokenCacheFake{}, tokenRetriever: tokenRetriever, @@ -293,8 +293,8 @@ func TestGetAccessToken_UserIdentity(t *testing.T) { assert.IsType(t, &clientSecretTokenRetriever{}, retriever) } settingsctx := backend.WithGrafanaConfig(ctx, backend.NewGrafanaCfg(map[string]string{ - "GFAZPL_USER_IDENTITY_ENABLED": "true", - "GFAZPL_USER_IDENTITY_SERVICE_CREDENTIALS": "true", + "GFAZPL_USER_IDENTITY_ENABLED": "true", + "GFAZPL_USER_IDENTITY_FALLBACK_SERVICE_CREDENTIALS_ENABLED": "true", })) _, err = provider.GetAccessToken(settingsctx, scopes) From f18ffabd7c9c548b8a6a99cc170c4e186b04518c Mon Sep 17 00:00:00 2001 From: Andreas Christou Date: Thu, 15 Feb 2024 12:49:23 +0000 Subject: [PATCH 14/16] Only use credentials if enabled - Improve errors --- aztokenprovider/token_provider.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/aztokenprovider/token_provider.go b/aztokenprovider/token_provider.go index 3edc73f..2ecbb17 100644 --- a/aztokenprovider/token_provider.go +++ b/aztokenprovider/token_provider.go @@ -76,10 +76,10 @@ func NewAzureAccessTokenProvider(settings *azsettings.AzureSettings, credentials var tokenRetriever TokenRetriever - if c.ServiceCredentials != nil { + if c.ServiceCredentials != nil && settings.UserIdentityFallbackCredentialsEnabled { fallbackType := c.ServiceCredentials.AzureAuthType() if fallbackType == azcredentials.AzureAuthCurrentUserIdentity || fallbackType == azcredentials.AzureAuthClientSecretObo { - err = fmt.Errorf("user identity authentication not valid for fallback credentials") + return nil, fmt.Errorf("user identity authentication not valid for fallback credentials") } switch c.ServiceCredentials.(type) { case *azcredentials.AzureClientSecretCredentials: @@ -198,7 +198,7 @@ func (provider *userTokenProvider) GetAccessToken(ctx context.Context, scopes [] if backendRequest { // Use fallback credentials if this is a backend request and fallback credentials are enabled - if settings.UserIdentityEnabled && !provider.usernameAssertion { + if settings.UserIdentityFallbackCredentialsEnabled && !provider.usernameAssertion { if provider.tokenRetriever != nil { accessToken, err := provider.tokenCache.GetAccessToken(ctx, provider.tokenRetriever, scopes) if err != nil { @@ -208,7 +208,7 @@ func (provider *userTokenProvider) GetAccessToken(ctx context.Context, scopes [] } } - return "", fmt.Errorf("user context not configured") + return "", fmt.Errorf("fallback credentials not enabled") } azureUser, err := isAzureUser(currentUser) From e119a23c6b7895a5e5519635aa3f6137faa492b3 Mon Sep 17 00:00:00 2001 From: Andreas Christou Date: Thu, 15 Feb 2024 12:49:31 +0000 Subject: [PATCH 15/16] Update tests --- aztokenprovider/token_provider_test.go | 441 +++++++++++++++++++------ 1 file changed, 336 insertions(+), 105 deletions(-) diff --git a/aztokenprovider/token_provider_test.go b/aztokenprovider/token_provider_test.go index 911dca6..969bafd 100644 --- a/aztokenprovider/token_provider_test.go +++ b/aztokenprovider/token_provider_test.go @@ -4,6 +4,7 @@ import ( "context" "testing" + "github.com/golang-jwt/jwt/v5" "github.com/grafana/grafana-azure-sdk-go/azcredentials" "github.com/grafana/grafana-azure-sdk-go/azsettings" "github.com/grafana/grafana-azure-sdk-go/azusercontext" @@ -110,15 +111,16 @@ func TestNewAzureAccessTokenProvider_ServiceIdentity(t *testing.T) { }) } -var mockUserCredentials = &azcredentials.AadCurrentUserCredentials{ - ServiceCredentials: &azcredentials.AzureClientSecretCredentials{ - AzureCloud: azsettings.AzurePublic, - TenantId: "TEST-TENANT", - ClientId: "TEST-CLIENT-ID", - ClientSecret: "TEST-CLIENT-SECRET", - }, +var mockClientSecretCredentials = &azcredentials.AzureClientSecretCredentials{ + AzureCloud: azsettings.AzurePublic, + TenantId: "TEST-TENANT", + ClientId: "TEST-CLIENT-ID", + ClientSecret: "TEST-CLIENT-SECRET", } +var mockMsiCredentials = &azcredentials.AzureManagedIdentityCredentials{} +var mockWorkloadIdentityCredentials = &azcredentials.AzureWorkloadIdentityCredentials{} + func TestNewAzureAccessTokenProvider_UserIdentity(t *testing.T) { settingsNotConfigured := &azsettings.AzureSettings{} @@ -130,6 +132,15 @@ func TestNewAzureAccessTokenProvider_UserIdentity(t *testing.T) { ClientSecret: "FAKE_CLIENT_SECRET", }, } + settingsFallbackEnabled := &azsettings.AzureSettings{ + UserIdentityEnabled: true, + UserIdentityTokenEndpoint: &azsettings.TokenEndpointSettings{ + TokenUrl: "FAKE_TOKEN_URL", + ClientId: "FAKE_CLIENT_ID", + ClientSecret: "FAKE_CLIENT_SECRET", + }, + UserIdentityFallbackCredentialsEnabled: true, + } t.Run("should fail when user identity not supported", func(t *testing.T) { credentials := &azcredentials.AadCurrentUserCredentials{} @@ -155,10 +166,30 @@ func TestNewAzureAccessTokenProvider_UserIdentity(t *testing.T) { t.Run("should return user provider with service principal credentials when user identity configured", func(t *testing.T) { - provider, err := NewAzureAccessTokenProvider(settings, mockUserCredentials, true) + provider, err := NewAzureAccessTokenProvider(settings, &azcredentials.AadCurrentUserCredentials{ + ServiceCredentials: mockClientSecretCredentials, + }, true) require.NoError(t, err) require.IsType(t, &userTokenProvider{}, provider) }) + + t.Run("should error if fallback credentials set to user credentials", func(t *testing.T) { + + _, err := NewAzureAccessTokenProvider(settingsFallbackEnabled, &azcredentials.AadCurrentUserCredentials{ + ServiceCredentials: &azcredentials.AadCurrentUserCredentials{}, + }, true) + require.Error(t, err) + require.ErrorContains(t, err, "user identity authentication not valid for fallback credentials") + }) + + t.Run("should error if fallback credentials set to OBO credentials", func(t *testing.T) { + + _, err := NewAzureAccessTokenProvider(settingsFallbackEnabled, &azcredentials.AadCurrentUserCredentials{ + ServiceCredentials: &azcredentials.AzureClientSecretOboCredentials{}, + }, true) + require.Error(t, err) + require.ErrorContains(t, err, "user identity authentication not valid for fallback credentials") + }) } func TestGetAccessToken_UserIdentity(t *testing.T) { @@ -170,134 +201,334 @@ func TestGetAccessToken_UserIdentity(t *testing.T) { var err error - t.Run("should fail if user context not configured", func(t *testing.T) { - var provider AzureTokenProvider = &userTokenProvider{ - tokenCache: &tokenCacheFake{}, - } + t.Run("frontend requests (user in scope)", func(t *testing.T) { + t.Run("should fail if user context not configured", func(t *testing.T) { + var provider AzureTokenProvider = &userTokenProvider{ + tokenCache: &tokenCacheFake{}, + } - getAccessTokenFunc = func(retriever TokenRetriever, scopes []string) { - assert.IsType(t, &onBehalfOfTokenRetriever{}, retriever) - } + getAccessTokenFunc = func(retriever TokenRetriever, scopes []string) { + assert.IsType(t, &onBehalfOfTokenRetriever{}, retriever) + } - _, err = provider.GetAccessToken(ctx, scopes) - assert.Error(t, err) - }) + _, err = provider.GetAccessToken(ctx, scopes) + assert.Error(t, err) + assert.ErrorContains(t, err, "user context not configured") + }) - t.Run("should fail if no user in user context", func(t *testing.T) { - var provider AzureTokenProvider = &userTokenProvider{ - tokenCache: &tokenCacheFake{}, - } + t.Run("will error if user is not authenticated with Azure AD and ID forwarding enabled", func(t *testing.T) { + var provider AzureTokenProvider = &userTokenProvider{ + tokenCache: &tokenCacheFake{}, + } - getAccessTokenFunc = func(retriever TokenRetriever, scopes []string) { - assert.IsType(t, &onBehalfOfTokenRetriever{}, retriever) - } + getAccessTokenFunc = func(retriever TokenRetriever, scopes []string) { + assert.IsType(t, &onBehalfOfTokenRetriever{}, retriever) + } - usrctx := azusercontext.WithCurrentUser(ctx, azusercontext.CurrentUserContext{}) + token := jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.MapClaims{ + "authenticatedBy": "not_azuread", + }) + jwtToken, _ := token.SignedString([]byte("test-key")) // 👈 + + usrctx := azusercontext.WithCurrentUser(ctx, azusercontext.CurrentUserContext{ + User: &backend.User{ + Login: "user1@example.org", + }, + IdToken: "FAKE_ID_TOKEN", + GrafanaIdToken: jwtToken, + }) + settingsctx := backend.WithGrafanaConfig(usrctx, backend.NewGrafanaCfg(map[string]string{ + "GF_INSTANCE_FEATURE_TOGGLES_ENABLE": "idForwarding", + })) + + _, err = provider.GetAccessToken(settingsctx, scopes) + require.Error(t, err) + require.ErrorContains(t, err, "user is not authenticated with Azure AD") + }) - _, err = provider.GetAccessToken(usrctx, scopes) - assert.Error(t, err) - }) + t.Run("will assume request is frontend if user != nil and ID forwarding disabled", func(t *testing.T) { + var provider AzureTokenProvider = &userTokenProvider{ + tokenCache: &tokenCacheFake{}, + } - t.Run("should fail if no ID token in user context", func(t *testing.T) { - var provider AzureTokenProvider = &userTokenProvider{ - tokenCache: &tokenCacheFake{}, - } + getAccessTokenFunc = func(retriever TokenRetriever, scopes []string) { + assert.IsType(t, &onBehalfOfTokenRetriever{}, retriever) + } - getAccessTokenFunc = func(retriever TokenRetriever, scopes []string) { - assert.IsType(t, &onBehalfOfTokenRetriever{}, retriever) - } + usrctx := azusercontext.WithCurrentUser(ctx, azusercontext.CurrentUserContext{ + User: &backend.User{ + Login: "user1@example.org", + }, + IdToken: "FAKE_ID_TOKEN", + }) - usrctx := azusercontext.WithCurrentUser(ctx, azusercontext.CurrentUserContext{ - User: &backend.User{ - Login: "user1@example.org", - }, + _, err = provider.GetAccessToken(usrctx, scopes) + require.NoError(t, err) }) - _, err = provider.GetAccessToken(usrctx, scopes) - assert.Error(t, err) - }) + t.Run("should fail if no ID token in user context", func(t *testing.T) { + var provider AzureTokenProvider = &userTokenProvider{ + tokenCache: &tokenCacheFake{}, + } - t.Run("should use onBehalfOfTokenRetriever by default", func(t *testing.T) { - var provider AzureTokenProvider = &userTokenProvider{ - tokenCache: &tokenCacheFake{}, - } + getAccessTokenFunc = func(retriever TokenRetriever, scopes []string) { + assert.IsType(t, &onBehalfOfTokenRetriever{}, retriever) + } - getAccessTokenFunc = func(retriever TokenRetriever, scopes []string) { - assert.IsType(t, &onBehalfOfTokenRetriever{}, retriever) - } + usrctx := azusercontext.WithCurrentUser(ctx, azusercontext.CurrentUserContext{ + User: &backend.User{ + Login: "user1@example.org", + }, + }) - usrctx := azusercontext.WithCurrentUser(ctx, azusercontext.CurrentUserContext{ - User: &backend.User{ - Login: "user1@example.org", - }, - IdToken: "FAKE_ID_TOKEN", + _, err = provider.GetAccessToken(usrctx, scopes) + assert.Error(t, err) + assert.ErrorContains(t, err, "user identity authentication not possible because there's no ID token associated with the Grafana user") }) - _, err = provider.GetAccessToken(usrctx, scopes) - require.NoError(t, err) - }) + t.Run("should fail if no username in user context", func(t *testing.T) { + var provider AzureTokenProvider = &userTokenProvider{ + tokenCache: &tokenCacheFake{}, + } - t.Run("should use usernameTokenRetriever for usernameAssertion", func(t *testing.T) { - var provider AzureTokenProvider = &userTokenProvider{ - tokenCache: &tokenCacheFake{}, - usernameAssertion: true, - } + getAccessTokenFunc = func(retriever TokenRetriever, scopes []string) { + assert.IsType(t, &onBehalfOfTokenRetriever{}, retriever) + } - getAccessTokenFunc = func(retriever TokenRetriever, scopes []string) { - assert.IsType(t, &usernameTokenRetriever{}, retriever) - } + usrctx := azusercontext.WithCurrentUser(ctx, azusercontext.CurrentUserContext{ + User: &backend.User{ + Login: "", + }, + }) - usrctx := azusercontext.WithCurrentUser(ctx, azusercontext.CurrentUserContext{ - User: &backend.User{ - Login: "user1@example.org", - }, + _, err = provider.GetAccessToken(usrctx, scopes) + assert.Error(t, err) + assert.ErrorContains(t, err, "user identity authentication only possible in context of a Grafana user: request not associated with a Grafana user") }) - _, err = provider.GetAccessToken(usrctx, scopes) - require.NoError(t, err) + t.Run("should use onBehalfOfTokenRetriever by default", func(t *testing.T) { + var provider AzureTokenProvider = &userTokenProvider{ + tokenCache: &tokenCacheFake{}, + } + + getAccessTokenFunc = func(retriever TokenRetriever, scopes []string) { + assert.IsType(t, &onBehalfOfTokenRetriever{}, retriever) + } + + usrctx := azusercontext.WithCurrentUser(ctx, azusercontext.CurrentUserContext{ + User: &backend.User{ + Login: "user1@example.org", + }, + IdToken: "FAKE_ID_TOKEN", + }) + + _, err = provider.GetAccessToken(usrctx, scopes) + require.NoError(t, err) + }) + + t.Run("should use usernameTokenRetriever for usernameAssertion", func(t *testing.T) { + var provider AzureTokenProvider = &userTokenProvider{ + tokenCache: &tokenCacheFake{}, + usernameAssertion: true, + } + + getAccessTokenFunc = func(retriever TokenRetriever, scopes []string) { + assert.IsType(t, &usernameTokenRetriever{}, retriever) + } + + usrctx := azusercontext.WithCurrentUser(ctx, azusercontext.CurrentUserContext{ + User: &backend.User{ + Login: "user1@example.org", + }, + }) + + _, err = provider.GetAccessToken(usrctx, scopes) + require.NoError(t, err) + }) }) - t.Run("should use clientSecretTokenRetriever when service principal credentials are available without an access token or id token if enabled", func(t *testing.T) { + t.Run("backend requests", func(t *testing.T) { + t.Run("will be treated as a backend request if ID forwarding enabled and ID token is empty", func(t *testing.T) { + var provider AzureTokenProvider = &userTokenProvider{ + tokenCache: &tokenCacheFake{}, + tokenRetriever: &clientSecretTokenRetriever{}, + } - tokenRetriever, _ := getClientSecretTokenRetriever(&azsettings.AzureSettings{UserIdentityFallbackCredentialsEnabled: true}, &mockUserCredentials.ServiceCredentials) - var provider AzureTokenProvider = &userTokenProvider{ - tokenCache: &tokenCacheFake{}, - tokenRetriever: tokenRetriever, - } + getAccessTokenFunc = func(retriever TokenRetriever, scopes []string) { + assert.IsType(t, &clientSecretTokenRetriever{}, retriever) + } - getAccessTokenFunc = func(retriever TokenRetriever, scopes []string) { - assert.IsType(t, &clientSecretTokenRetriever{}, retriever) - } + usrctx := azusercontext.WithCurrentUser(ctx, azusercontext.CurrentUserContext{ + User: &backend.User{}, + }) + settingsctx := backend.WithGrafanaConfig(usrctx, backend.NewGrafanaCfg(map[string]string{ + "GF_INSTANCE_FEATURE_TOGGLES_ENABLE": "idForwarding", + "GFAZPL_USER_IDENTITY_ENABLED": "true", + "GFAZPL_USER_IDENTITY_FALLBACK_SERVICE_CREDENTIALS_ENABLED": "true", + })) - usrctx := azusercontext.WithCurrentUser(ctx, azusercontext.CurrentUserContext{ - User: &backend.User{}, + _, err = provider.GetAccessToken(settingsctx, scopes) + require.NoError(t, err) }) - settingsctx := backend.WithGrafanaConfig(usrctx, backend.NewGrafanaCfg(map[string]string{ - "GFAZPL_USER_IDENTITY_ENABLED": "true", - "GFAZPL_USER_IDENTITY_FALLBACK_SERVICE_CREDENTIALS_ENABLED": "true", - })) - _, err = provider.GetAccessToken(settingsctx, scopes) - require.NoError(t, err) - }) + t.Run("will be treated as a backend request if current user context is empty", func(t *testing.T) { + var provider AzureTokenProvider = &userTokenProvider{ + tokenCache: &tokenCacheFake{}, + tokenRetriever: &clientSecretTokenRetriever{}, + } - t.Run("should use clientSecretTokenRetriever when service principal credentials are available without a user in context if enabled", func(t *testing.T) { + getAccessTokenFunc = func(retriever TokenRetriever, scopes []string) { + assert.IsType(t, &clientSecretTokenRetriever{}, retriever) + } - tokenRetriever, _ := getClientSecretTokenRetriever(&azsettings.AzureSettings{UserIdentityFallbackCredentialsEnabled: true}, &mockUserCredentials.ServiceCredentials) - var provider AzureTokenProvider = &userTokenProvider{ - tokenCache: &tokenCacheFake{}, - tokenRetriever: tokenRetriever, - } + usrctx := azusercontext.WithCurrentUser(ctx, azusercontext.CurrentUserContext{}) + settingsctx := backend.WithGrafanaConfig(usrctx, backend.NewGrafanaCfg(map[string]string{ + "GF_INSTANCE_FEATURE_TOGGLES_ENABLE": "idForwarding", + "GFAZPL_USER_IDENTITY_ENABLED": "true", + "GFAZPL_USER_IDENTITY_FALLBACK_SERVICE_CREDENTIALS_ENABLED": "true", + })) - getAccessTokenFunc = func(retriever TokenRetriever, scopes []string) { - assert.IsType(t, &clientSecretTokenRetriever{}, retriever) - } - settingsctx := backend.WithGrafanaConfig(ctx, backend.NewGrafanaCfg(map[string]string{ - "GFAZPL_USER_IDENTITY_ENABLED": "true", - "GFAZPL_USER_IDENTITY_FALLBACK_SERVICE_CREDENTIALS_ENABLED": "true", - })) + _, err = provider.GetAccessToken(settingsctx, scopes) + require.NoError(t, err) + }) + + t.Run("will be treated as a backend request if current user is nil", func(t *testing.T) { + var provider AzureTokenProvider = &userTokenProvider{ + tokenCache: &tokenCacheFake{}, + tokenRetriever: &clientSecretTokenRetriever{}, + } + + getAccessTokenFunc = func(retriever TokenRetriever, scopes []string) { + assert.IsType(t, &clientSecretTokenRetriever{}, retriever) + } + + usrctx := azusercontext.WithCurrentUser(ctx, azusercontext.CurrentUserContext{ + User: nil, + }) + settingsctx := backend.WithGrafanaConfig(usrctx, backend.NewGrafanaCfg(map[string]string{ + "GF_INSTANCE_FEATURE_TOGGLES_ENABLE": "idForwarding", + "GFAZPL_USER_IDENTITY_ENABLED": "true", + "GFAZPL_USER_IDENTITY_FALLBACK_SERVICE_CREDENTIALS_ENABLED": "true", + })) + + _, err = provider.GetAccessToken(settingsctx, scopes) + require.NoError(t, err) + }) + + t.Run("will not use fallback credentials if username assertion enabled and fallback credentials enabled", func(t *testing.T) { + var provider AzureTokenProvider = &userTokenProvider{ + tokenCache: &tokenCacheFake{}, + tokenRetriever: &clientSecretTokenRetriever{}, + usernameAssertion: true, + } + + usrctx := azusercontext.WithCurrentUser(ctx, azusercontext.CurrentUserContext{ + User: nil, + }) + settingsctx := backend.WithGrafanaConfig(usrctx, backend.NewGrafanaCfg(map[string]string{ + "GF_INSTANCE_FEATURE_TOGGLES_ENABLE": "idForwarding", + "GFAZPL_USER_IDENTITY_ENABLED": "true", + "GFAZPL_USER_IDENTITY_FALLBACK_SERVICE_CREDENTIALS_ENABLED": "true", + })) + + _, err = provider.GetAccessToken(settingsctx, scopes) + require.Error(t, err) + require.ErrorContains(t, err, "fallback credentials not enabled") + }) + + t.Run("will not use fallback credentials if disabled", func(t *testing.T) { + var provider AzureTokenProvider = &userTokenProvider{ + tokenCache: &tokenCacheFake{}, + tokenRetriever: &clientSecretTokenRetriever{}, + } + + usrctx := azusercontext.WithCurrentUser(ctx, azusercontext.CurrentUserContext{ + User: nil, + }) + settingsctx := backend.WithGrafanaConfig(usrctx, backend.NewGrafanaCfg(map[string]string{ + "GF_INSTANCE_FEATURE_TOGGLES_ENABLE": "idForwarding", + "GFAZPL_USER_IDENTITY_ENABLED": "true", + "GFAZPL_USER_IDENTITY_FALLBACK_SERVICE_CREDENTIALS_ENABLED": "false", + })) + + _, err = provider.GetAccessToken(settingsctx, scopes) + require.Error(t, err) + require.ErrorContains(t, err, "fallback credentials not enabled") + }) + + t.Run("should use clientSecretTokenRetriever when service principal credentials are enabled", func(t *testing.T) { + tokenRetriever, _ := getClientSecretTokenRetriever(&azsettings.AzureSettings{UserIdentityFallbackCredentialsEnabled: true}, mockClientSecretCredentials) + var provider AzureTokenProvider = &userTokenProvider{ + tokenCache: &tokenCacheFake{}, + tokenRetriever: tokenRetriever, + } + + getAccessTokenFunc = func(retriever TokenRetriever, scopes []string) { + assert.IsType(t, &clientSecretTokenRetriever{}, retriever) + } + + usrctx := azusercontext.WithCurrentUser(ctx, azusercontext.CurrentUserContext{ + User: nil, + }) + settingsctx := backend.WithGrafanaConfig(usrctx, backend.NewGrafanaCfg(map[string]string{ + "GFAZPL_USER_IDENTITY_ENABLED": "true", + "GFAZPL_USER_IDENTITY_FALLBACK_SERVICE_CREDENTIALS_ENABLED": "true", + })) + + _, err = provider.GetAccessToken(settingsctx, scopes) + require.NoError(t, err) + }) + + t.Run("should use msiTokenRetriever when service principal credentials are enabled", func(t *testing.T) { + tokenRetriever := getManagedIdentityTokenRetriever(&azsettings.AzureSettings{UserIdentityFallbackCredentialsEnabled: true, ManagedIdentityEnabled: true, ManagedIdentityClientId: "test-msi"}, mockMsiCredentials) + var provider AzureTokenProvider = &userTokenProvider{ + tokenCache: &tokenCacheFake{}, + tokenRetriever: tokenRetriever, + } + + getAccessTokenFunc = func(retriever TokenRetriever, scopes []string) { + assert.IsType(t, &managedIdentityTokenRetriever{}, retriever) + } + + usrctx := azusercontext.WithCurrentUser(ctx, azusercontext.CurrentUserContext{ + User: nil, + }) + settingsctx := backend.WithGrafanaConfig(usrctx, backend.NewGrafanaCfg(map[string]string{ + "GFAZPL_USER_IDENTITY_ENABLED": "true", + "GFAZPL_USER_IDENTITY_FALLBACK_SERVICE_CREDENTIALS_ENABLED": "true", + })) + + _, err = provider.GetAccessToken(settingsctx, scopes) + require.NoError(t, err) + }) + + t.Run("should use workloadIdentityTokenRetriever when service principal credentials are enabled", func(t *testing.T) { + tokenRetriever := getWorkloadIdentityTokenRetriever(&azsettings.AzureSettings{UserIdentityFallbackCredentialsEnabled: true, WorkloadIdentityEnabled: true, WorkloadIdentitySettings: &azsettings.WorkloadIdentitySettings{ + TenantId: "test-tenant-id", + ClientId: "test-client-id", + TokenFile: "test-token-file", + }}, mockWorkloadIdentityCredentials) + var provider AzureTokenProvider = &userTokenProvider{ + tokenCache: &tokenCacheFake{}, + tokenRetriever: tokenRetriever, + } + + getAccessTokenFunc = func(retriever TokenRetriever, scopes []string) { + assert.IsType(t, &workloadIdentityTokenRetriever{}, retriever) + } + + usrctx := azusercontext.WithCurrentUser(ctx, azusercontext.CurrentUserContext{ + User: nil, + }) + settingsctx := backend.WithGrafanaConfig(usrctx, backend.NewGrafanaCfg(map[string]string{ + "GFAZPL_USER_IDENTITY_ENABLED": "true", + "GFAZPL_USER_IDENTITY_FALLBACK_SERVICE_CREDENTIALS_ENABLED": "true", + })) + + _, err = provider.GetAccessToken(settingsctx, scopes) + require.NoError(t, err) + }) - _, err = provider.GetAccessToken(settingsctx, scopes) - require.NoError(t, err) }) + } From 8d08bb1d03dd7b15f8e5a6d1d5d344af13faef58 Mon Sep 17 00:00:00 2001 From: Andreas Christou Date: Tue, 20 Feb 2024 18:25:45 +0000 Subject: [PATCH 16/16] Add data source level service credentials enabled option --- azcredentials/credentials.go | 3 ++- aztokenprovider/token_provider.go | 2 +- aztokenprovider/token_provider_test.go | 9 ++++++--- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/azcredentials/credentials.go b/azcredentials/credentials.go index 10be1e6..02d6af1 100644 --- a/azcredentials/credentials.go +++ b/azcredentials/credentials.go @@ -14,7 +14,8 @@ type AzureCredentials interface { // AadCurrentUserCredentials "Current User" user identity credentials of the current Grafana user. type AadCurrentUserCredentials struct { - ServiceCredentials AzureCredentials + ServiceCredentialsEnabled bool + ServiceCredentials AzureCredentials } // AzureManagedIdentityCredentials "Managed Identity" service managed identity credentials configured diff --git a/aztokenprovider/token_provider.go b/aztokenprovider/token_provider.go index 2ecbb17..921ad5d 100644 --- a/aztokenprovider/token_provider.go +++ b/aztokenprovider/token_provider.go @@ -76,7 +76,7 @@ func NewAzureAccessTokenProvider(settings *azsettings.AzureSettings, credentials var tokenRetriever TokenRetriever - if c.ServiceCredentials != nil && settings.UserIdentityFallbackCredentialsEnabled { + if c.ServiceCredentialsEnabled && c.ServiceCredentials != nil && settings.UserIdentityFallbackCredentialsEnabled { fallbackType := c.ServiceCredentials.AzureAuthType() if fallbackType == azcredentials.AzureAuthCurrentUserIdentity || fallbackType == azcredentials.AzureAuthClientSecretObo { return nil, fmt.Errorf("user identity authentication not valid for fallback credentials") diff --git a/aztokenprovider/token_provider_test.go b/aztokenprovider/token_provider_test.go index 969bafd..e36a71c 100644 --- a/aztokenprovider/token_provider_test.go +++ b/aztokenprovider/token_provider_test.go @@ -167,7 +167,8 @@ func TestNewAzureAccessTokenProvider_UserIdentity(t *testing.T) { t.Run("should return user provider with service principal credentials when user identity configured", func(t *testing.T) { provider, err := NewAzureAccessTokenProvider(settings, &azcredentials.AadCurrentUserCredentials{ - ServiceCredentials: mockClientSecretCredentials, + ServiceCredentialsEnabled: true, + ServiceCredentials: mockClientSecretCredentials, }, true) require.NoError(t, err) require.IsType(t, &userTokenProvider{}, provider) @@ -176,7 +177,8 @@ func TestNewAzureAccessTokenProvider_UserIdentity(t *testing.T) { t.Run("should error if fallback credentials set to user credentials", func(t *testing.T) { _, err := NewAzureAccessTokenProvider(settingsFallbackEnabled, &azcredentials.AadCurrentUserCredentials{ - ServiceCredentials: &azcredentials.AadCurrentUserCredentials{}, + ServiceCredentialsEnabled: true, + ServiceCredentials: &azcredentials.AadCurrentUserCredentials{}, }, true) require.Error(t, err) require.ErrorContains(t, err, "user identity authentication not valid for fallback credentials") @@ -185,7 +187,8 @@ func TestNewAzureAccessTokenProvider_UserIdentity(t *testing.T) { t.Run("should error if fallback credentials set to OBO credentials", func(t *testing.T) { _, err := NewAzureAccessTokenProvider(settingsFallbackEnabled, &azcredentials.AadCurrentUserCredentials{ - ServiceCredentials: &azcredentials.AzureClientSecretOboCredentials{}, + ServiceCredentialsEnabled: true, + ServiceCredentials: &azcredentials.AzureClientSecretOboCredentials{}, }, true) require.Error(t, err) require.ErrorContains(t, err, "user identity authentication not valid for fallback credentials")