From c5e74ee607ebd07f27dbe10444a296be94be2485 Mon Sep 17 00:00:00 2001 From: Eric Leijonmarck Date: Mon, 16 Jan 2023 13:16:01 +0100 Subject: [PATCH] Auth: Add skip_org_role_sync for AzureAD OAuth (#60322) * [WIP] Auth: add backend skipOrgRoleSync to AzureAD OAuth - add: skipOrgRoleSync - rename: skipOrgRoleSync to skipOrgRoleSyncBase (to make it clear that it is the base version of SocialBase) - add: tests for skipOrgRoleSync in AzureAD TODO: - [ ] frontend changes * add: docs * refactor: remove role from basicinfo * add: settings for grafanacom * add: settigns for frontend * add: logic for azureAD user skip org role * add: docs for skip_org_role_sync * refactor: docs a bit * add: tests for userinfo * refactor: to only extract if skiporgrolesync false * refactor: based on review comments * Update docs/sources/setup-grafana/configure-grafana/_index.md Co-authored-by: Christopher Moyer <35463610+chri2547@users.noreply.github.com> * Update docs/sources/setup-grafana/configure-grafana/_index.md Co-authored-by: Christopher Moyer <35463610+chri2547@users.noreply.github.com> Co-authored-by: Christopher Moyer <35463610+chri2547@users.noreply.github.com> --- conf/sample.ini | 2 + .../setup-grafana/configure-grafana/_index.md | 20 ++- .../configure-authentication/azuread/index.md | 44 ++++-- packages/grafana-data/src/types/config.ts | 1 + pkg/api/frontendsettings.go | 1 + pkg/login/social/azuread_oauth.go | 11 +- pkg/login/social/azuread_oauth_test.go | 144 ++++++++++++++++++ pkg/login/social/social.go | 1 + pkg/setting/setting.go | 9 ++ public/app/features/admin/UserAdminPage.tsx | 11 +- 10 files changed, 217 insertions(+), 27 deletions(-) diff --git a/conf/sample.ini b/conf/sample.ini index 9fef1b9c5a4..284c8304f1c 100644 --- a/conf/sample.ini +++ b/conf/sample.ini @@ -582,6 +582,8 @@ ;allowed_groups = ;role_attribute_strict = false ;allow_assign_grafana_admin = false +# prevent synchronizing users organization roles +;skip_org_role_sync = false #################################### Okta OAuth ####################### [auth.okta] diff --git a/docs/sources/setup-grafana/configure-grafana/_index.md b/docs/sources/setup-grafana/configure-grafana/_index.md index 7c8883d7c9d..696c0a112f1 100644 --- a/docs/sources/setup-grafana/configure-grafana/_index.md +++ b/docs/sources/setup-grafana/configure-grafana/_index.md @@ -849,7 +849,7 @@ Administrators can increase this if they experience OAuth login state mismatch e ### oauth_skip_org_role_update_sync -> **Note**: This option will soon be a legacy option in favor of OAuth provider specific `skip_org_role_sync` settings. +> **Note**: This option will soon be a legacy option in favor of OAuth provider specific `skip_org_role_sync` settings. The following sections explain settings for each provider. Skip forced assignment of OrgID `1` or `auto_assign_org_id` for external logins. Default is `false`. Use this setting to allow users with external login to be manually assigned to multiple organizations. @@ -860,13 +860,13 @@ By default, the users' organization and role is reset on every new login. > With Grafana 10, if `oauth_skip_org_role_update_sync` option is set to `false`, users with no mapping will be > reset to the default organization role on every login. [See `auto_assign_org_role` option]({{< relref ".#auto_assign_org_role" >}}). -### [auth.grafana_com] skip_org_role_update_sync +### [auth.grafana_com] skip_org_role_sync To prevent synchronization of organization roles for a specific OAuth integration, you can set the `skip_org_role_sync` option to `true`. Please note that there is also a separate setting called `oauth_skip_org_role_update_sync` which has a different scope. While `skip_org_role_sync` only applies to the specific OAuth provider, `oauth_skip_org_role_update_sync` is a generic setting that affects all configured OAuth providers. The setting `oauth_skip_org_role_update_sync` will be deprecated in favor of provider-specific settings. -The table below shows the available OAuth providers and their setting with the default value and the skip org role sync setting. +The table below show the OAuth provider and their setting with the default value and the skip org role sync setting. | OAuth Provider | `oauth_skip_org_role_sync_update` | `skip_org_role_sync` | Behavior | | --- | --- | --- | --- | | Grafana.com | false | false | will sync with Grafana.com roles | @@ -874,6 +874,20 @@ The table below shows the available OAuth providers and their setting with the d | Grafana.com | false | true | skip org role sync for grafana.com users | | Grafana.com | true | true | skip org role sync for Grafana.com users and all other OAuth providers | +### [auth.azuread] skip_org_role_sync + +To prevent synchronization of organization roles for a specific OAuth integration, you can set the `skip_org_role_sync` option to `true`. Please note that there is also a separate setting called `oauth_skip_org_role_update_sync` which has a different scope. While `skip_org_role_sync` only applies to the specific OAuth provider, `oauth_skip_org_role_update_sync` is a generic setting that affects all configured OAuth providers. + +The setting `oauth_skip_org_role_update_sync` will be deprecated in favor of provider-specific settings. + +The following table shows the OAuth providers, the default value setting, and the skip org role sync setting. +| OAuth Provider | `oauth_skip_org_role_sync_update` | `skip_org_role_sync` | Behavior | +| --- | --- | --- | --- | +| AzureAD | false | false | will sync with AzureAD roles | +| AzureAD | true | false | skip org role sync for OAuth providers including AzureAD users | +| AzureAD | false | true | skip org role sync for AzureAD users | +| AzureAD | true | true | skip org role sync for AzureAD users and all other OAuth providers | + ### api_key_max_seconds_to_live Limit of API key seconds to live before expiration. Default is -1 (unlimited). diff --git a/docs/sources/setup-grafana/configure-security/configure-authentication/azuread/index.md b/docs/sources/setup-grafana/configure-security/configure-authentication/azuread/index.md index 321056dc7e4..c3243a11409 100644 --- a/docs/sources/setup-grafana/configure-security/configure-authentication/azuread/index.md +++ b/docs/sources/setup-grafana/configure-security/configure-authentication/azuread/index.md @@ -99,22 +99,6 @@ To enable the Azure AD OAuth2, register your application with Azure AD. 1. Click on **Users and Groups** and add Users/Groups to the Grafana roles by using **Add User**. -### Map roles - -By default, Azure AD authentication will map users to organization roles based on the most privileged application role assigned to the user in AzureAD. - -If no application role is found, the user is assigned the role specified by -[the `auto_assign_org_role` option]({{< relref "../../../configure-grafana#auto_assign_org_role" >}}). -You can disable this default role assignment by setting `role_attribute_strict = true`. -It denies user access if no role or an invalid role is returned. - -**On every login** the user organization role will be reset to match AzureAD's application role and -their organization membership will be reset to the default organization. - -If Azure AD authentication is not intended to sync user roles and organization membership, -`oauth_skip_org_role_update_sync` should be enabled. -See [configure-grafana]({{< relref "../../../configure-grafana#oauth_skip_org_role_update_sync" >}}) for more details. - ### Assign server administrator privileges > Available in Grafana v9.2 and later versions. @@ -157,6 +141,7 @@ allowed_domains = allowed_groups = role_attribute_strict = false allow_assign_grafana_admin = false +skip_org_role_sync = false ``` You can also use these environment variables to configure **client_id** and **client_secret**: @@ -244,3 +229,30 @@ To force fetching groups from Microsoft Graph API instead of the `id_token`. You ``` force_use_graph_api = true ``` + +### Map roles + +By default, Azure AD authentication will map users to organization roles based on the most privileged application role assigned to the user in AzureAD. + +If no application role is found, the user is assigned the role specified by +[the `auto_assign_org_role` option]({{< relref "../../../configure-grafana#auto_assign_org_role" >}}). +You can disable this default role assignment by setting `role_attribute_strict = true`. +It denies user access if no role or an invalid role is returned. + +**On every login** the user organization role will be reset to match AzureAD's application role and +their organization membership will be reset to the default organization. + +## Skip organization role sync + +If Azure AD authentication is not intended to sync user roles and organization membership, +`oauth_skip_org_role_update_sync` should be enabled, this is not recommended to use in favor of setting provider specific `skip_org_role_sync` option. +See [configure-grafana]({{< relref "../../../configure-grafana#oauth_skip_org_role_update_sync" >}}) for more details. + +To prevent the sync of org roles from Grafana.com, set `skip_org_role_sync` to `true`. This is useful if you want to manage the organization roles for your users from within Grafana. + +```ini +[auth.azuread] +# .. +# prevents the sync of org roles from Grafana.com +skip_org_role_sync = true +``` diff --git a/packages/grafana-data/src/types/config.ts b/packages/grafana-data/src/types/config.ts index 739bde84869..d3919b65ce2 100644 --- a/packages/grafana-data/src/types/config.ts +++ b/packages/grafana-data/src/types/config.ts @@ -224,5 +224,6 @@ export interface AuthSettings { SAMLSkipOrgRoleSync?: boolean; LDAPSkipOrgRoleSync?: boolean; GrafanaComSkipOrgRoleSync?: boolean; + AzureADSkipOrgRoleSync?: boolean; DisableSyncLock?: boolean; } diff --git a/pkg/api/frontendsettings.go b/pkg/api/frontendsettings.go index 6e49a583422..23f355753fd 100644 --- a/pkg/api/frontendsettings.go +++ b/pkg/api/frontendsettings.go @@ -149,6 +149,7 @@ func (hs *HTTPServer) getFrontendSettingsMap(c *models.ReqContext) (map[string]i "SAMLSkipOrgRoleSync": hs.Cfg.SectionWithEnvOverrides("auth.saml").Key("skip_org_role_sync").MustBool(false), "LDAPSkipOrgRoleSync": hs.Cfg.LDAPSkipOrgRoleSync, "GrafanaComSkipOrgRoleSync": hs.Cfg.GrafanaComSkipOrgRoleSync, + "AzureADSkipOrgRoleSync": hs.Cfg.AzureADSkipOrgRoleSync, "DisableSyncLock": hs.Cfg.DisableSyncLock, }, "buildInfo": map[string]interface{}{ diff --git a/pkg/login/social/azuread_oauth.go b/pkg/login/social/azuread_oauth.go index df6c94ca4eb..8f10f8e812c 100644 --- a/pkg/login/social/azuread_oauth.go +++ b/pkg/login/social/azuread_oauth.go @@ -8,6 +8,7 @@ import ( "net/http" "strings" + "github.com/grafana/grafana/pkg/models/roletype" "github.com/grafana/grafana/pkg/services/org" "golang.org/x/oauth2" @@ -18,6 +19,7 @@ type SocialAzureAD struct { *SocialBase allowedGroups []string forceUseGraphAPI bool + skipOrgRoleSync bool } type azureClaims struct { @@ -70,18 +72,21 @@ func (s *SocialAzureAD) UserInfo(client *http.Client, token *oauth2.Token) (*Bas return nil, ErrEmailNotFound } - role, grafanaAdmin := s.extractRoleAndAdmin(&claims) + // setting the role, grafanaAdmin to empty to reflect that we are not syncronizing with the external provider + var role roletype.RoleType + var grafanaAdmin bool + if !s.skipOrgRoleSync { + role, grafanaAdmin = s.extractRoleAndAdmin(&claims) + } if s.roleAttributeStrict && !role.IsValid() { return nil, &InvalidBasicRoleError{idP: "Azure", assignedRole: string(role)} } - logger.Debug("AzureAD OAuth: extracted role", "email", email, "role", role) groups, err := s.extractGroups(client, claims, token) if err != nil { return nil, fmt.Errorf("failed to extract groups: %w", err) } - logger.Debug("AzureAD OAuth: extracted groups", "email", email, "groups", fmt.Sprintf("%v", groups)) if !s.IsGroupMember(groups) { return nil, errMissingGroupMembership diff --git a/pkg/login/social/azuread_oauth_test.go b/pkg/login/social/azuread_oauth_test.go index 4a03dc12002..26a005ddb22 100644 --- a/pkg/login/social/azuread_oauth_test.go +++ b/pkg/login/social/azuread_oauth_test.go @@ -483,3 +483,147 @@ func TestSocialAzureAD_UserInfo(t *testing.T) { }) } } + +func TestSocialAzureAD_SkipOrgRole(t *testing.T) { + type fields struct { + SocialBase *SocialBase + allowedGroups []string + forceUseGraphAPI bool + skipOrgRoleSync bool + } + type args struct { + client *http.Client + } + + tests := []struct { + name string + fields fields + claims *azureClaims + args args + settingAutoAssignOrgRole string + want *BasicUserInfo + wantErr bool + }{ + { + name: "Grafana Admin and Editor roles in claim, skipOrgRoleSync disabled should get roles, skipOrgRoleSyncBase disabled", + fields: fields{ + SocialBase: newSocialBase("azuread", &oauth2.Config{}, &OAuthInfo{AllowAssignGrafanaAdmin: true}, "", false, *featuremgmt.WithFeatures()), + skipOrgRoleSync: false, + }, + claims: &azureClaims{ + Email: "me@example.com", + PreferredUsername: "", + Roles: []string{"GrafanaAdmin", "Editor"}, + Name: "My Name", + ID: "1234", + }, + want: &BasicUserInfo{ + Id: "1234", + Name: "My Name", + Email: "me@example.com", + Login: "me@example.com", + Role: "Admin", + IsGrafanaAdmin: trueBoolPtr(), + Groups: []string{}, + }, + }, + { + name: "Grafana Admin and Editor roles in claim, skipOrgRoleSync disabled should not get roles", + fields: fields{ + SocialBase: newSocialBase("azuread", &oauth2.Config{}, &OAuthInfo{AllowAssignGrafanaAdmin: true}, "", false, *featuremgmt.WithFeatures()), + skipOrgRoleSync: false, + }, + claims: &azureClaims{ + Email: "me@example.com", + PreferredUsername: "", + Roles: []string{"GrafanaAdmin", "Editor"}, + Name: "My Name", + ID: "1234", + }, + want: &BasicUserInfo{ + Id: "1234", + Name: "My Name", + Email: "me@example.com", + Login: "me@example.com", + Role: "Admin", + IsGrafanaAdmin: trueBoolPtr(), + Groups: []string{}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := &SocialAzureAD{ + SocialBase: tt.fields.SocialBase, + allowedGroups: tt.fields.allowedGroups, + forceUseGraphAPI: tt.fields.forceUseGraphAPI, + skipOrgRoleSync: tt.fields.skipOrgRoleSync, + } + + if tt.fields.SocialBase == nil { + s.SocialBase = newSocialBase("azuread", &oauth2.Config{}, &OAuthInfo{}, "", false, *featuremgmt.WithFeatures()) + } + + key := []byte("secret") + sig, err := jose.NewSigner(jose.SigningKey{Algorithm: jose.HS256, Key: key}, (&jose.SignerOptions{}).WithType("JWT")) + if err != nil { + panic(err) + } + + cl := jwt.Claims{ + Subject: "subject", + Issuer: "issuer", + NotBefore: jwt.NewNumericDate(time.Date(2016, 1, 1, 0, 0, 0, 0, time.UTC)), + Audience: jwt.Audience{"leela", "fry"}, + } + + var raw string + if tt.claims != nil { + if tt.claims.ClaimNames.Groups != "" { + server := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { + tokenParts := strings.Split(request.Header.Get("Authorization"), " ") + require.Len(t, tokenParts, 2) + require.Equal(t, "fake_token", tokenParts[1]) + + writer.WriteHeader(http.StatusOK) + + type response struct { + Value []string + } + res := response{Value: []string{"from_server"}} + require.NoError(t, json.NewEncoder(writer).Encode(&res)) + })) + // need to set the fake servers url as endpoint to capture request + tt.claims.ClaimSources = map[string]claimSource{ + tt.claims.ClaimNames.Groups: {Endpoint: server.URL}, + } + } + raw, err = jwt.Signed(sig).Claims(cl).Claims(tt.claims).CompactSerialize() + require.NoError(t, err) + } else { + raw, err = jwt.Signed(sig).Claims(cl).CompactSerialize() + require.NoError(t, err) + } + + token := &oauth2.Token{ + AccessToken: "fake_token", + } + if tt.claims != nil { + token = token.WithExtra(map[string]interface{}{"id_token": raw}) + } + + if tt.fields.SocialBase != nil { + tt.args.client = s.Client(context.Background(), token) + } + + got, err := s.UserInfo(tt.args.client, token) + if (err != nil) != tt.wantErr { + t.Errorf("UserInfo() error = %v, wantErr %v", err, tt.wantErr) + return + } + + require.EqualValues(t, tt.want, got) + }) + } +} diff --git a/pkg/login/social/social.go b/pkg/login/social/social.go index 19ac4b311de..056457a8238 100644 --- a/pkg/login/social/social.go +++ b/pkg/login/social/social.go @@ -171,6 +171,7 @@ func ProvideService(cfg *setting.Cfg, features *featuremgmt.FeatureManager) *Soc SocialBase: newSocialBase(name, &config, info, cfg.AutoAssignOrgRole, cfg.OAuthSkipOrgRoleUpdateSync, *features), allowedGroups: util.SplitString(sec.Key("allowed_groups").String()), forceUseGraphAPI: sec.Key("force_use_graph_api").MustBool(false), + skipOrgRoleSync: cfg.AzureADSkipOrgRoleSync, } } diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index 921fdadadaf..441adea7d0f 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -303,6 +303,7 @@ type Cfg struct { SigV4AuthEnabled bool SigV4VerboseLogging bool AzureAuthEnabled bool + AzureSkipOrgRoleSync bool BasicAuthEnabled bool AdminUser string AdminPassword string @@ -422,6 +423,9 @@ type Cfg struct { ApplicationInsightsEndpointUrl string FeedbackLinksEnabled bool + // AzureAD + AzureADSkipOrgRoleSync bool + // LDAP LDAPEnabled bool LDAPSkipOrgRoleSync bool @@ -1354,6 +1358,10 @@ func readSecuritySettings(iniFile *ini.File, cfg *Cfg) error { return nil } +func readAuthAzureADSettings(iniFile *ini.File, cfg *Cfg) { + sec := iniFile.Section("auth.azuread") + cfg.AzureADSkipOrgRoleSync = sec.Key("skip_org_role_sync").MustBool(false) +} func readAuthGrafanaComSettings(iniFile *ini.File, cfg *Cfg) { sec := iniFile.Section("auth.grafana_com") @@ -1406,6 +1414,7 @@ func readAuthSettings(iniFile *ini.File, cfg *Cfg) (err error) { // Azure Auth AzureAuthEnabled = auth.Key("azure_auth_enabled").MustBool(false) cfg.AzureAuthEnabled = AzureAuthEnabled + readAuthAzureADSettings(iniFile, cfg) // anonymous access AnonymousEnabled = iniFile.Section("auth.anonymous").Key("enabled").MustBool(false) diff --git a/public/app/features/admin/UserAdminPage.tsx b/public/app/features/admin/UserAdminPage.tsx index 155adb3abd5..71456012b4b 100644 --- a/public/app/features/admin/UserAdminPage.tsx +++ b/public/app/features/admin/UserAdminPage.tsx @@ -39,7 +39,7 @@ interface OwnProps extends GrafanaRouteComponentProps<{ id: string }> { error?: UserAdminError; } -const SyncedOAuthLabels: string[] = ['GitHub', 'GitLab', 'AzureAD', 'OAuth']; +const SyncedOAuthLabels: string[] = ['GitHub', 'GitLab', 'OAuth']; export class UserAdminPage extends PureComponent { async componentDidMount() { @@ -113,9 +113,8 @@ export class UserAdminPage extends PureComponent { const isSAMLUser = user?.isExternal && user?.authLabels?.includes('SAML'); const isGoogleUser = user?.isExternal && user?.authLabels?.includes('Google'); const isAuthProxyUser = user?.isExternal && user?.authLabels?.includes('Auth Proxy'); + const isAzureADUser = user?.isExternal && user?.authLabels?.includes('AzureAD'); const isGrafanaComUser = user?.isExternal && user?.authLabels?.includes('grafana.com'); - // isGrafanaComUser true - // isOAuthUserWithSkippableSync true const isUserSynced = !config.auth.DisableSyncLock && ((user?.isExternal && @@ -125,13 +124,15 @@ export class UserAdminPage extends PureComponent { isOAuthUserWithSkippableSync || isSAMLUser || isLDAPUser || + isAzureADUser || isGrafanaComUser )) || (!config.auth.OAuthSkipOrgRoleUpdateSync && isOAuthUserWithSkippableSync) || (!config.auth.SAMLSkipOrgRoleSync && isSAMLUser) || (!config.auth.LDAPSkipOrgRoleSync && isLDAPUser) || - // both OAuthSkipOrgRoleUpdateSync and GrafanaComSkipOrgRoleSync needs to be false for a GrafanaComUser to be synced - (!config.auth.OAuthSkipOrgRoleUpdateSync && !config.auth.GrafanaComSkipOrgRoleSync && isGrafanaComUser)); + // both OAuthSkipOrgRoleUpdateSync and specific provider settings needs to be false for a user to be synced + (!config.auth.OAuthSkipOrgRoleUpdateSync && !config.auth.GrafanaComSkipOrgRoleSync && isGrafanaComUser) || + (!config.auth.OAuthSkipOrgRoleUpdateSync && !config.auth.AzureADSkipOrgRoleSync && isAzureADUser)); const pageNav: NavModelItem = { text: user?.login ?? '',