From 9a44296bc2a2caceadd57f6cd9a78957cdd53b06 Mon Sep 17 00:00:00 2001 From: Misi Date: Mon, 10 Jun 2024 12:08:30 +0200 Subject: [PATCH] Auth: Add org to role mappings support to AzureAD/Entra integration (#88861) * Added implementation and tests * Add docs, simplify implementation * Remove unused func * Update docs --- conf/defaults.ini | 1 + conf/sample.ini | 1 + .../configure-authentication/azuread/index.md | 19 +- .../configure-authentication/gitlab/index.md | 2 + pkg/login/social/connectors/azuread_oauth.go | 83 ++--- .../social/connectors/azuread_oauth_test.go | 331 +++++++++++++----- pkg/login/social/connectors/social_base.go | 12 - pkg/services/authn/clients/oauth.go | 4 +- 8 files changed, 294 insertions(+), 159 deletions(-) diff --git a/conf/defaults.ini b/conf/defaults.ini index 5da264003de..20449cc273a 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -756,6 +756,7 @@ allowed_domains = allowed_groups = allowed_organizations = role_attribute_strict = false +org_mapping = allow_assign_grafana_admin = false force_use_graph_api = false tls_skip_verify_insecure = false diff --git a/conf/sample.ini b/conf/sample.ini index f9c7554ca78..62280de14eb 100644 --- a/conf/sample.ini +++ b/conf/sample.ini @@ -694,6 +694,7 @@ ;allowed_groups = ;allowed_organizations = ;role_attribute_strict = false +;org_mapping = ;allow_assign_grafana_admin = false ;use_pkce = true # prevent synchronizing users organization roles 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 fba188be8fa..4c43f2e180b 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 @@ -411,12 +411,27 @@ By default, Azure AD authentication will map users to organization roles based o 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. +You can disable this default role assignment by setting `role_attribute_strict = true`. This setting denies user access if no role or an invalid role is returned and the `org_mapping` expression evaluates to an empty mapping. + +You can use the `org_mapping` configuration option to assign the user to multiple organizations and specify their role based on their Entra ID group membership. For more information, refer to [Org roles mapping example](#org-roles-mapping-example). If the org role mapping (`org_mapping`) is specified and Entra ID returns a valid role, then the user will get the highest of the two roles. **On every login** the user organization role will be reset to match Entra ID's application role and their organization membership will be reset to the default organization. +#### Org roles mapping example + +The Entra ID integration uses the external users' groups in the `org_mapping` configuration to map organizations and roles based on their Entra ID group membership. + +In this example, the user has been granted the role of a `Viewer` in the `org_foo` organization, and the role of an `Editor` in the `org_bar` and `org_baz` orgs. + +The external user is part of the following Entra ID groups: `032cb8e0-240f-4347-9120-6f33013e817a` and `bce1c492-0679-4989-941b-8de5e6789cb9`. + +Config: + +```ini +org_mapping = ["032cb8e0-240f-4347-9120-6f33013e817a:org_foo:Viewer", "bce1c492-0679-4989-941b-8de5e6789cb9:org_bar:Editor", "*:org_baz:Editor"] +``` + ## Skip organization role sync If Azure AD authentication is not intended to sync user roles and organization membership and prevent the sync of org roles from Entra ID, set `skip_org_role_sync` to `true`. This is useful if you want to manage the organization roles for your users from within Grafana or that your organization roles are synced from another provider. diff --git a/docs/sources/setup-grafana/configure-security/configure-authentication/gitlab/index.md b/docs/sources/setup-grafana/configure-security/configure-authentication/gitlab/index.md index 3084f5b24c2..11e1a1db46a 100644 --- a/docs/sources/setup-grafana/configure-security/configure-authentication/gitlab/index.md +++ b/docs/sources/setup-grafana/configure-security/configure-authentication/gitlab/index.md @@ -148,6 +148,8 @@ The user's role is retrieved using a [JMESPath](http://jmespath.org/examples.htm To map the server administrator role, use the `allow_assign_grafana_admin` configuration option. Refer to [configuration options]({{< relref "#configuration-options" >}}) for more information. +You can use the `org_mapping` configuration option to assign the user to multiple organizations and specify their role based on their GitLab group membership. For more information, refer to [Org roles mapping example](#org-roles-mapping-example). If the org role mapping (`org_mapping`) is specified and Entra ID returns a valid role, then the user will get the highest of the two roles. + If no valid 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`. This setting denies user access if no role or an invalid role is returned after evaluating the `role_attribute_path` and the `org_mapping` expressions. diff --git a/pkg/login/social/connectors/azuread_oauth.go b/pkg/login/social/connectors/azuread_oauth.go index 32876aa7a5c..e6f0997357f 100644 --- a/pkg/login/social/connectors/azuread_oauth.go +++ b/pkg/login/social/connectors/azuread_oauth.go @@ -17,7 +17,6 @@ import ( "github.com/grafana/grafana/pkg/infra/remotecache" "github.com/grafana/grafana/pkg/login/social" - "github.com/grafana/grafana/pkg/models/roletype" "github.com/grafana/grafana/pkg/services/auth/identity" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/org" @@ -121,26 +120,42 @@ func (s *SocialAzureAD) UserInfo(ctx context.Context, client *http.Client, token return nil, ErrEmailNotFound } - // 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.info.SkipOrgRoleSync { - role, grafanaAdmin, err = s.extractRoleAndAdmin(claims) - if err != nil { - return nil, err - } - - if !role.IsValid() { - return nil, errInvalidRole.Errorf("AzureAD OAuth: invalid role %q", role) - } - } - s.log.Debug("AzureAD OAuth: extracted role", "email", email, "role", role) - groups, err := s.extractGroups(ctx, client, claims, token) if err != nil { return nil, fmt.Errorf("failed to extract groups: %w", err) } + s.log.Debug("AzureAD OAuth: extracted groups", "email", email, "groups", fmt.Sprintf("%v", groups)) + + userInfo := &social.BasicUserInfo{ + Id: claims.ID, + Name: claims.Name, + Email: email, + Login: email, + Groups: groups, + } + + if !s.info.SkipOrgRoleSync { + directlyMappedRole, grafanaAdmin := s.extractRoleAndAdminOptional(claims) + + s.log.Debug("AzureAD OAuth: extracted role", "email", email, "role", directlyMappedRole) + + if s.info.AllowAssignGrafanaAdmin { + userInfo.IsGrafanaAdmin = &grafanaAdmin + } + + userInfo.OrgRoles = s.orgRoleMapper.MapOrgRoles(s.orgMappingCfg, userInfo.Groups, directlyMappedRole) + if s.info.RoleAttributeStrict && len(userInfo.OrgRoles) == 0 { + return nil, errRoleAttributeStrictViolation.Errorf("could not evaluate any valid roles using IdP provided data") + } + + s.log.Debug("AzureAD OAuth: mapped org roles", "email", email, "roles", fmt.Sprintf("%v", userInfo.OrgRoles)) + } + + if s.info.AllowAssignGrafanaAdmin && s.info.SkipOrgRoleSync { + s.log.Debug("AllowAssignGrafanaAdmin and skipOrgRoleSync are both set, Grafana Admin role will not be synced, consider setting one or the other") + } + if !s.isGroupMember(groups) { if len(groups) == 0 { // either they do not have a group or misconfiguration @@ -150,24 +165,7 @@ func (s *SocialAzureAD) UserInfo(ctx context.Context, client *http.Client, token return nil, errMissingGroupMembership } - var isGrafanaAdmin *bool = nil - if s.info.AllowAssignGrafanaAdmin { - isGrafanaAdmin = &grafanaAdmin - } - - if s.info.AllowAssignGrafanaAdmin && s.info.SkipOrgRoleSync { - s.log.Debug("AllowAssignGrafanaAdmin and skipOrgRoleSync are both set, Grafana Admin role will not be synced, consider setting one or the other") - } - - return &social.BasicUserInfo{ - Id: claims.ID, - Name: claims.Name, - Email: email, - Login: email, - Role: role, - IsGrafanaAdmin: isGrafanaAdmin, - Groups: groups, - }, nil + return userInfo, nil } func (s *SocialAzureAD) Reload(ctx context.Context, settings ssoModels.SSOSettings) error { @@ -289,12 +287,9 @@ func (claims *azureClaims) extractEmail() string { } // extractRoleAndAdmin extracts the role from the claims and returns the role and whether the user is a Grafana admin. -func (s *SocialAzureAD) extractRoleAndAdmin(claims *azureClaims) (org.RoleType, bool, error) { +func (s *SocialAzureAD) extractRoleAndAdminOptional(claims *azureClaims) (org.RoleType, bool) { if len(claims.Roles) == 0 { - if s.info.RoleAttributeStrict { - return "", false, errRoleAttributeStrictViolation.Errorf("AzureAD OAuth: unset role") - } - return s.defaultRole(), false, nil + return "", false } roleOrder := []org.RoleType{social.RoleGrafanaAdmin, org.RoleAdmin, org.RoleEditor, @@ -302,18 +297,14 @@ func (s *SocialAzureAD) extractRoleAndAdmin(claims *azureClaims) (org.RoleType, for _, role := range roleOrder { if found := hasRole(claims.Roles, role); found { if role == social.RoleGrafanaAdmin { - return org.RoleAdmin, true, nil + return org.RoleAdmin, true } - return role, false, nil + return role, false } } - if s.info.RoleAttributeStrict { - return "", false, errRoleAttributeStrictViolation.Errorf("AzureAD OAuth: idP did not return a valid role %q", claims.Roles) - } - - return s.defaultRole(), false, nil + return "", false } func hasRole(roles []string, role org.RoleType) bool { diff --git a/pkg/login/social/connectors/azuread_oauth_test.go b/pkg/login/social/connectors/azuread_oauth_test.go index 92aa7f67c10..d78bc202ea2 100644 --- a/pkg/login/social/connectors/azuread_oauth_test.go +++ b/pkg/login/social/connectors/azuread_oauth_test.go @@ -20,6 +20,8 @@ import ( "github.com/grafana/grafana/pkg/login/social" "github.com/grafana/grafana/pkg/services/auth/identity" "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/org" + "github.com/grafana/grafana/pkg/services/org/orgtest" "github.com/grafana/grafana/pkg/services/ssosettings" ssoModels "github.com/grafana/grafana/pkg/services/ssosettings/models" "github.com/grafana/grafana/pkg/services/ssosettings/ssosettingstests" @@ -75,12 +77,12 @@ func TestSocialAzureAD_UserInfo(t *testing.T) { }, }, want: &social.BasicUserInfo{ - Id: "1234", - Name: "My Name", - Email: "me@example.com", - Login: "me@example.com", - Role: "Viewer", - Groups: []string{}, + Id: "1234", + Name: "My Name", + Email: "me@example.com", + Login: "me@example.com", + OrgRoles: map[int64]org.RoleType{1: org.RoleViewer}, + Groups: []string{}, }, }, { @@ -139,12 +141,12 @@ func TestSocialAzureAD_UserInfo(t *testing.T) { usGovURL: true, }, want: &social.BasicUserInfo{ - Id: "1234", - Name: "My Name", - Email: "me@example.com", - Login: "me@example.com", - Role: "Viewer", - Groups: []string{}, + Id: "1234", + Name: "My Name", + Email: "me@example.com", + Login: "me@example.com", + OrgRoles: map[int64]org.RoleType{1: org.RoleViewer}, + Groups: []string{}, }, }, { @@ -166,12 +168,12 @@ func TestSocialAzureAD_UserInfo(t *testing.T) { }, }, want: &social.BasicUserInfo{ - Id: "1234", - Name: "My Name", - Email: "me@example.com", - Login: "me@example.com", - Role: "Viewer", - Groups: []string{}, + Id: "1234", + Name: "My Name", + Email: "me@example.com", + Login: "me@example.com", + OrgRoles: map[int64]org.RoleType{1: org.RoleViewer}, + Groups: []string{}, }, }, { @@ -193,12 +195,12 @@ func TestSocialAzureAD_UserInfo(t *testing.T) { ID: "1234", }, want: &social.BasicUserInfo{ - Id: "1234", - Name: "My Name", - Email: "me@example.com", - Login: "me@example.com", - Role: "Admin", - Groups: []string{}, + Id: "1234", + Name: "My Name", + Email: "me@example.com", + Login: "me@example.com", + OrgRoles: map[int64]org.RoleType{1: org.RoleAdmin}, + Groups: []string{}, }, }, { @@ -220,12 +222,12 @@ func TestSocialAzureAD_UserInfo(t *testing.T) { ID: "1234", }, want: &social.BasicUserInfo{ - Id: "1234", - Name: "My Name", - Email: "me@example.com", - Login: "me@example.com", - Role: "Admin", - Groups: []string{}, + Id: "1234", + Name: "My Name", + Email: "me@example.com", + Login: "me@example.com", + OrgRoles: map[int64]org.RoleType{1: org.RoleAdmin}, + Groups: []string{}, }, }, { @@ -247,15 +249,14 @@ func TestSocialAzureAD_UserInfo(t *testing.T) { ID: "1234", }, want: &social.BasicUserInfo{ - Id: "1234", - Name: "My Name", - Email: "me@example.com", - Login: "me@example.com", - Role: "Viewer", - Groups: []string{}, + Id: "1234", + Name: "My Name", + Email: "me@example.com", + Login: "me@example.com", + OrgRoles: map[int64]org.RoleType{1: org.RoleViewer}, + Groups: []string{}, }, }, - // TODO: @mgyongyosi check this test { name: "role from env variable", claims: &azureClaims{ @@ -275,12 +276,12 @@ func TestSocialAzureAD_UserInfo(t *testing.T) { }, }, want: &social.BasicUserInfo{ - Id: "1234", - Name: "My Name", - Email: "me@example.com", - Login: "me@example.com", - Role: "Editor", - Groups: []string{}, + Id: "1234", + Name: "My Name", + Email: "me@example.com", + Login: "me@example.com", + OrgRoles: map[int64]org.RoleType{1: org.RoleEditor}, + Groups: []string{}, }, }, { @@ -302,12 +303,12 @@ func TestSocialAzureAD_UserInfo(t *testing.T) { }, }, want: &social.BasicUserInfo{ - Id: "1234", - Name: "My Name", - Email: "me@example.com", - Login: "me@example.com", - Role: "Editor", - Groups: []string{}, + Id: "1234", + Name: "My Name", + Email: "me@example.com", + Login: "me@example.com", + OrgRoles: map[int64]org.RoleType{1: org.RoleEditor}, + Groups: []string{}, }, }, { @@ -329,12 +330,12 @@ func TestSocialAzureAD_UserInfo(t *testing.T) { ID: "1234", }, want: &social.BasicUserInfo{ - Id: "1234", - Name: "My Name", - Email: "me@example.com", - Login: "me@example.com", - Role: "Admin", - Groups: []string{}, + Id: "1234", + Name: "My Name", + Email: "me@example.com", + Login: "me@example.com", + OrgRoles: map[int64]org.RoleType{1: org.RoleAdmin}, + Groups: []string{}, }, }, { @@ -362,7 +363,7 @@ func TestSocialAzureAD_UserInfo(t *testing.T) { Name: "My Name", Email: "me@example.com", Login: "me@example.com", - Role: "Admin", + OrgRoles: map[int64]org.RoleType{1: org.RoleAdmin}, Groups: []string{}, IsGrafanaAdmin: nil, }, @@ -391,7 +392,7 @@ func TestSocialAzureAD_UserInfo(t *testing.T) { Name: "My Name", Email: "me@example.com", Login: "me@example.com", - Role: "Editor", + OrgRoles: map[int64]org.RoleType{1: org.RoleEditor}, Groups: []string{}, IsGrafanaAdmin: falseBoolPtr(), }, @@ -420,7 +421,7 @@ func TestSocialAzureAD_UserInfo(t *testing.T) { Name: "My Name", Email: "me@example.com", Login: "me@example.com", - Role: "Admin", + OrgRoles: map[int64]org.RoleType{1: org.RoleAdmin}, Groups: []string{}, IsGrafanaAdmin: trueBoolPtr(), }, @@ -500,12 +501,12 @@ func TestSocialAzureAD_UserInfo(t *testing.T) { ID: "1234", }, want: &social.BasicUserInfo{ - Id: "1234", - Name: "My Name", - Email: "me@example.com", - Login: "me@example.com", - Role: "Viewer", - Groups: []string{"foo", "bar"}, + Id: "1234", + Name: "My Name", + Email: "me@example.com", + Login: "me@example.com", + OrgRoles: map[int64]org.RoleType{1: org.RoleViewer}, + Groups: []string{"foo", "bar"}, }, wantErr: false, }, @@ -531,12 +532,12 @@ func TestSocialAzureAD_UserInfo(t *testing.T) { ID: "1234", }, want: &social.BasicUserInfo{ - Id: "1234", - Name: "My Name", - Email: "me@example.com", - Login: "me@example.com", - Role: "Viewer", - Groups: []string{"foo"}, + Id: "1234", + Name: "My Name", + Email: "me@example.com", + Login: "me@example.com", + OrgRoles: map[int64]org.RoleType{1: org.RoleViewer}, + Groups: []string{"foo"}, }, }, { @@ -585,12 +586,12 @@ func TestSocialAzureAD_UserInfo(t *testing.T) { }, settingAutoAssignOrgRole: "", want: &social.BasicUserInfo{ - Id: "1", - Name: "test", - Email: "test@test.com", - Login: "test@test.com", - Role: "Viewer", - Groups: []string{"from_server"}, + Id: "1", + Name: "test", + Email: "test@test.com", + Login: "test@test.com", + OrgRoles: map[int64]org.RoleType{1: org.RoleViewer}, + Groups: []string{"from_server"}, }, wantErr: false, }, @@ -620,12 +621,12 @@ func TestSocialAzureAD_UserInfo(t *testing.T) { }, settingAutoAssignOrgRole: "", want: &social.BasicUserInfo{ - Id: "1", - Name: "test", - Email: "test@test.com", - Login: "test@test.com", - Role: "Viewer", - Groups: []string{"from_server"}, + Id: "1", + Name: "test", + Email: "test@test.com", + Login: "test@test.com", + OrgRoles: map[int64]org.RoleType{1: org.RoleViewer}, + Groups: []string{"from_server"}, }, wantErr: false, }, @@ -675,6 +676,131 @@ func TestSocialAzureAD_UserInfo(t *testing.T) { want: nil, wantErr: true, }, + { + name: "should map role when org mapping is set, IdP returns with invalid role and role attribute strict is enabled", + fields: fields{ + providerCfg: &social.OAuthInfo{ + Name: "azuread", + ClientId: "client-id-example", + RoleAttributeStrict: true, + OrgMapping: []string{"group1:Org4:Editor", "*:5:Viewer"}, + }, + cfg: &setting.Cfg{}, + }, + claims: &azureClaims{ + PreferredUsername: "", + Roles: []string{"Invalid"}, + Groups: []string{"group1", "group3"}, + Name: "My Name", + ID: "1234", + Email: "me@example.com", + }, + want: &social.BasicUserInfo{ + Id: "1234", + Name: "My Name", + Email: "me@example.com", + Login: "me@example.com", + OrgRoles: map[int64]org.RoleType{4: org.RoleEditor, 5: org.RoleViewer}, + Groups: []string{"group1", "group3"}, + }, + }, + { + name: "should map role when org mapping is set and IdP returns with empty role list", + fields: fields{ + providerCfg: &social.OAuthInfo{ + Name: "azuread", + ClientId: "client-id-example", + OrgMapping: []string{"group1:Org4:Editor", "group2:5:Viewer"}, + }, + cfg: &setting.Cfg{ + AutoAssignOrgRole: "Viewer", + }, + }, + claims: &azureClaims{ + Email: "me@example.com", + PreferredUsername: "", + Roles: []string{}, + Groups: []string{"group1"}, + Name: "My Name", + ID: "1234", + }, + want: &social.BasicUserInfo{ + Id: "1234", + Name: "My Name", + Email: "me@example.com", + Login: "me@example.com", + OrgRoles: map[int64]org.RoleType{4: org.RoleEditor}, + Groups: []string{"group1"}, + }, + }, + { + name: "should map role when only org mapping is set and role attribute strict is enabled", + fields: fields{ + providerCfg: &social.OAuthInfo{ + Name: "azuread", + ClientId: "client-id-example", + RoleAttributeStrict: true, + OrgMapping: []string{"group1:Org4:Editor", "*:5:Viewer"}, + }, + cfg: &setting.Cfg{}, + }, + claims: &azureClaims{ + PreferredUsername: "", + Roles: []string{}, + Groups: []string{"group1", "group3"}, + Name: "My Name", + ID: "1234", + Email: "me@example.com", + }, + want: &social.BasicUserInfo{ + Id: "1234", + Name: "My Name", + Email: "me@example.com", + Login: "me@example.com", + OrgRoles: map[int64]org.RoleType{4: org.RoleEditor, 5: org.RoleViewer}, + Groups: []string{"group1", "group3"}, + }, + }, + { + name: "should return error when roles claim is empty and org mapping doesn't evaluate to any role and role attribute strict is enabled", + fields: fields{ + providerCfg: &social.OAuthInfo{ + Name: "azuread", + ClientId: "client-id-example", + RoleAttributeStrict: true, + OrgMapping: []string{"group1:Org4:Editor"}, + }, + cfg: &setting.Cfg{}, + }, + claims: &azureClaims{ + PreferredUsername: "", + Roles: []string{}, + Groups: []string{"group2"}, + Name: "My Name", + ID: "1234", + }, + wantErr: true, + }, + { + name: "should return error when roles claim is empty and org mapping is empty and role attribute strict is enabled", + fields: fields{ + providerCfg: &social.OAuthInfo{ + Name: "azuread", + ClientId: "client-id-example", + RoleAttributeStrict: true, + OrgMapping: []string{}, + }, + cfg: &setting.Cfg{}, + }, + claims: &azureClaims{ + PreferredUsername: "", + Roles: []string{}, + Groups: []string{"group2"}, + Name: "My Name", + ID: "1234", + }, + wantErr: true, + }, } privateKey, err := rsa.GenerateKey(rand.Reader, 2048) @@ -711,7 +837,13 @@ func TestSocialAzureAD_UserInfo(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s := NewAzureADProvider(tt.fields.providerCfg, tt.fields.cfg, nil, &ssosettingstests.MockService{}, featuremgmt.WithFeatures(), cache) + s := NewAzureADProvider(tt.fields.providerCfg, + tt.fields.cfg, + ProvideOrgRoleMapper(tt.fields.cfg, + &orgtest.FakeOrgService{ExpectedOrgs: []*org.OrgDTO{{ID: 4, Name: "Org4"}, {ID: 5, Name: "Org5"}}}), + &ssosettingstests.MockService{}, + featuremgmt.WithFeatures(), + cache) if tt.fields.usGovURL { s.SocialBase.Endpoint.AuthURL = usGovAuthURL @@ -765,11 +897,12 @@ func TestSocialAzureAD_UserInfo(t *testing.T) { tt.args.client = s.Client(context.Background(), token) got, err := s.UserInfo(context.Background(), tt.args.client, token) - if (err != nil) != tt.wantErr { - t.Errorf("UserInfo() error = %v, wantErr %v", err, tt.wantErr) + if tt.wantErr { + require.Error(t, err) return } + require.NoError(t, err) require.EqualValues(t, tt.want, got) }) } @@ -791,7 +924,7 @@ func TestSocialAzureAD_SkipOrgRole(t *testing.T) { wantErr bool }{ { - name: "Grafana Admin and Editor roles in claim, skipOrgRoleSync disabled should get roles, skipOrgRoleSyncBase disabled", + name: "Grafana Admin and Editor roles in claim, skipOrgRoleSync disabled should get roles", fields: fields{ providerCfg: &social.OAuthInfo{ Name: "azuread", @@ -815,19 +948,19 @@ func TestSocialAzureAD_SkipOrgRole(t *testing.T) { Name: "My Name", Email: "me@example.com", Login: "me@example.com", - Role: "Admin", + OrgRoles: map[int64]org.RoleType{1: org.RoleAdmin}, IsGrafanaAdmin: trueBoolPtr(), Groups: []string{}, }, }, { - name: "Grafana Admin and Editor roles in claim, skipOrgRoleSync disabled should not get roles", + name: "Grafana Admin and Editor roles in claim, skipOrgRoleSync enabled should not get roles", fields: fields{ providerCfg: &social.OAuthInfo{ Name: "azuread", ClientId: "client-id-example", AllowAssignGrafanaAdmin: true, - SkipOrgRoleSync: false, + SkipOrgRoleSync: true, }, cfg: &setting.Cfg{ AutoAssignOrgRole: "", @@ -841,13 +974,11 @@ func TestSocialAzureAD_SkipOrgRole(t *testing.T) { ID: "1234", }, want: &social.BasicUserInfo{ - Id: "1234", - Name: "My Name", - Email: "me@example.com", - Login: "me@example.com", - Role: "Admin", - IsGrafanaAdmin: trueBoolPtr(), - Groups: []string{}, + Id: "1234", + Name: "My Name", + Email: "me@example.com", + Login: "me@example.com", + Groups: []string{}, }, }, } @@ -884,7 +1015,13 @@ func TestSocialAzureAD_SkipOrgRole(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s := NewAzureADProvider(tt.fields.providerCfg, tt.fields.cfg, nil, &ssosettingstests.MockService{}, featuremgmt.WithFeatures(), cache) + s := NewAzureADProvider(tt.fields.providerCfg, + tt.fields.cfg, + ProvideOrgRoleMapper(tt.fields.cfg, + &orgtest.FakeOrgService{ExpectedOrgs: []*org.OrgDTO{{ID: 4, Name: "Org4"}, {ID: 5, Name: "Org5"}}}), + &ssosettingstests.MockService{}, + featuremgmt.WithFeatures(), + cache) s.SocialBase.Endpoint.AuthURL = authURL diff --git a/pkg/login/social/connectors/social_base.go b/pkg/login/social/connectors/social_base.go index ffacee4bf68..6f801a02ad4 100644 --- a/pkg/login/social/connectors/social_base.go +++ b/pkg/login/social/connectors/social_base.go @@ -177,18 +177,6 @@ func (s *SocialBase) extractOrgs(rawJSON []byte) ([]string, error) { return util.SearchJSONForStringSliceAttr(s.info.OrgAttributePath, rawJSON) } -// defaultRole returns the default role for the user based on the autoAssignOrgRole setting -// if legacy is enabled "" is returned indicating the previous role assignment is used. -func (s *SocialBase) defaultRole() org.RoleType { - if s.cfg.AutoAssignOrgRole != "" { - s.log.Debug("No role found, returning default.") - return org.RoleType(s.cfg.AutoAssignOrgRole) - } - - // should never happen - return org.RoleViewer -} - func (s *SocialBase) isGroupMember(groups []string) bool { if len(s.info.AllowedGroups) == 0 { return true diff --git a/pkg/services/authn/clients/oauth.go b/pkg/services/authn/clients/oauth.go index 7db433bcbcf..bb18f19972d 100644 --- a/pkg/services/authn/clients/oauth.go +++ b/pkg/services/authn/clients/oauth.go @@ -168,8 +168,8 @@ func (c *OAuth) Authenticate(ctx context.Context, r *authn.Request) (*authn.Iden // This is required to implement OrgRole mapping for OAuth providers step by step switch c.providerName { - case social.GenericOAuthProviderName, social.GitHubProviderName, - social.GitlabProviderName, social.OktaProviderName, social.GoogleProviderName: + case social.GenericOAuthProviderName, social.GitHubProviderName, social.GitlabProviderName, + social.OktaProviderName, social.GoogleProviderName, social.AzureADProviderName: // Do nothing, these providers already supports OrgRole mapping default: userInfo.OrgRoles, userInfo.IsGrafanaAdmin, _ = getRoles(c.cfg, func() (org.RoleType, *bool, error) {