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>
2024-11-27 03:11:01 -06:00 · 2023-01-16 13:16:01 +01:00 · 2023-01-16 13:16:01 +01:00 · c5e74ee607
commit c5e74ee607
parent 7e505ea49c
10 changed files with 217 additions and 27 deletions
--- 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]
--- 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).
--- 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
+```
--- 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;
 }
--- 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{}{
--- 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
--- 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)
+		})
+	}
+}
--- 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,
 			}
 		}

--- 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)
--- 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<Props> {
  async componentDidMount() {
@ -113,9 +113,8 @@ export class UserAdminPage extends PureComponent<Props> {
    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<Props> {
          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 ?? '',