Add new azure flag to obtain groups from the oidc ticket

Oleg Skoromnik · Oleg Skoromnik · commit c4c13fff651d · 2024-02-28T22:59:10.000+01:00
It is possible in azure to configure an optional claim such as groups.
In this case there is no need to make a request to Microsoft Graph api
to obtain groups. Instead azure will send the configured groups as a
part of oidc ticket directly. This is usefull when it is not possible to
obtain GroupMember.Read.All permissions for the applictaion.

Here we provide a new flag `azure-groups-in-ticket`, which does exactly
this. When the flag is set the request to the Graph api is not done and
instead the groups are taken from the session directly.
diff --git a/contrib/oauth2-proxy_autocomplete.sh b/contrib/oauth2-proxy_autocomplete.sh
@@ -24,7 +24,7 @@ _oauth2_proxy() {
 			COMPREPLY=( $(compgen -W 'X-Real-IP X-Forwarded-For X-ProxyUser-IP' -- ${cur}) )
 			return 0
 			;;
-		--@(http-address|https-address|redirect-url|upstream|basic-auth-password|skip-auth-regex|flush-interval|extra-jwt-issuers|email-domain|whitelist-domain|trusted-ip|keycloak-group|azure-tenant|bitbucket-team|bitbucket-repository|github-org|github-team|github-repo|github-token|gitlab-group|github-user|google-group|google-admin-email|google-service-account-json|client-id|client_secret|banner|footer|proxy-prefix|ping-path|ready-path|cookie-name|cookie-secret|cookie-domain|cookie-path|cookie-expire|cookie-refresh|cookie-samesite|redist-sentinel-master-name|redist-sentinel-connection-urls|redist-cluster-connection-urls|logging-max-size|logging-max-age|logging-max-backups|standard-logging-format|request-logging-format|exclude-logging-paths|auth-logging-format|oidc-issuer-url|oidc-jwks-url|login-url|redeem-url|profile-url|resource|validate-url|scope|approval-prompt|signature-key|acr-values|jwt-key|pubjwk-url|force-json-errors))
+		--@(http-address|https-address|redirect-url|upstream|basic-auth-password|skip-auth-regex|flush-interval|extra-jwt-issuers|email-domain|whitelist-domain|trusted-ip|keycloak-group|azure-tenant|azure-groups-in-ticket|bitbucket-team|bitbucket-repository|github-org|github-team|github-repo|github-token|gitlab-group|github-user|google-group|google-admin-email|google-service-account-json|client-id|client_secret|banner|footer|proxy-prefix|ping-path|ready-path|cookie-name|cookie-secret|cookie-domain|cookie-path|cookie-expire|cookie-refresh|cookie-samesite|redist-sentinel-master-name|redist-sentinel-connection-urls|redist-cluster-connection-urls|logging-max-size|logging-max-age|logging-max-backups|standard-logging-format|request-logging-format|exclude-logging-paths|auth-logging-format|oidc-issuer-url|oidc-jwks-url|login-url|redeem-url|profile-url|resource|validate-url|scope|approval-prompt|signature-key|acr-values|jwt-key|pubjwk-url|force-json-errors))
 			return 0
 			;;
 	esac
diff --git a/docs/docs/configuration/overview.md b/docs/docs/configuration/overview.md
@@ -74,6 +74,7 @@ An example [oauth2-proxy.cfg](https://github.com/oauth2-proxy/oauth2-proxy/blob/
 | `--auth-logging-format` | string | Template for authentication log lines | see [Logging Configuration](#logging-configuration) |
 | `--authenticated-emails-file` | string | authenticate against emails via file (one per line) | |
 | `--azure-tenant` | string | go to a tenant-specific or common (tenant-independent) endpoint. | `"common"` |
+| `--azure-groups-in-ticket` | bool | If true do not fetch groups from Microsoft graph api, but instead take them from the oidc ticket, see [docs](https://learn.microsoft.com/en-us/entra/identity-platform/optional-claims#configure-groups-optional-claims) | `false` |
 | `--backend-logout-url` | string | URL to perform backend logout, if you use `{id_token}` in the url it will be replaced by the actual `id_token` of the user session | |
 | `--basic-auth-password` | string | the password to set when passing the HTTP Basic Auth header | |
 | `--client-id` | string | the OAuth Client ID, e.g. `"123456.apps.googleusercontent.com"` | |
diff --git a/docs/docs/configuration/providers/azure.md b/docs/docs/configuration/providers/azure.md
@@ -14,6 +14,14 @@ title: Azure
    <br/>**IMPORTANT**: Even if this permission is listed with **"Admin consent required=No"** the consent might actually 
    be required, due to AAD policies you won't be able to see. If you get a **"Need admin approval"** during login, 
    most likely this is what you're missing!
+
+   Alternatively to require **Group.Read.All** we can configure that the groups come as a part of the oidc ticket itself.
+   This is enabled by providing a command line argument `--azure-groups-in-ticket`. In this case there will be no request
+   to Microsoft Graph api to obtain the groups that the user have.
+   For this go to **App registrations** and select your application. Then in **Manage** select **Token configuration** and
+   click on **+ Add groups claim**. On the right a menu will open where you can select the group claims. Typically it makes sense
+   to choose **Groups assigned to the application** in order not to exceed the number of groups that can be placed in the
+   oidc ticket itself.
 4. Next, if you are planning to use v2.0 Azure Auth endpoint, go to the **Manifest** page and set `"accessTokenAcceptedVersion": 2`
    in the App registration manifest file.
 5. On the **Certificates & secrets** page of the app, add a new client secret and note down the value after hitting **Add**.
diff --git a/pkg/apis/options/legacy_options.go b/pkg/apis/options/legacy_options.go
@@ -488,6 +488,7 @@ type LegacyProvider struct {
 	KeycloakGroups                         []string `flag:"keycloak-group" cfg:"keycloak_groups"`
 	AzureTenant                            string   `flag:"azure-tenant" cfg:"azure_tenant"`
 	AzureGraphGroupField                   string   `flag:"azure-graph-group-field" cfg:"azure_graph_group_field"`
+	AzureGroupsInTicket                    bool     `flag:"azure-groups-in-ticket" cfg:"azure_groups_in_ticket"`
 	BitbucketTeam                          string   `flag:"bitbucket-team" cfg:"bitbucket_team"`
 	BitbucketRepository                    string   `flag:"bitbucket-repository" cfg:"bitbucket_repository"`
 	GitHubOrg                              string   `flag:"github-org" cfg:"github_org"`
@@ -550,6 +551,7 @@ func legacyProviderFlagSet() *pflag.FlagSet {
 	flagSet.StringSlice("keycloak-group", []string{}, "restrict logins to members of these groups (may be given multiple times)")
 	flagSet.String("azure-tenant", "common", "go to a tenant-specific or common (tenant-independent) endpoint.")
 	flagSet.String("azure-graph-group-field", "", "configures the group field to be used when building the groups list(`id` or `displayName`. Default is `id`) from Microsoft Graph(available only for v2.0 oidc url). Based on this value, the `allowed-group` config values should be adjusted accordingly. If using `id` as group field, `allowed-group` should contains groups IDs, if using `displayName` as group field, `allowed-group` should contains groups name")
+	flagSet.Bool("azure-groups-in-ticket", false, "configures server to take groups from azure oidc ticket. It is possible in azure to configure that the groups are sent as a part of oidc ticket. When true request to graph api is not performed and the groups are taken from the ticket.")
 	flagSet.String("bitbucket-team", "", "restrict logins to members of this team")
 	flagSet.String("bitbucket-repository", "", "restrict logins to user with access to this repository")
 	flagSet.String("github-org", "", "restrict logins to members of this organisation")
@@ -703,8 +705,9 @@ func (l *LegacyProvider) convert() (Providers, error) {
 	// This part is out of the switch section because azure has a default tenant
 	// that needs to be added from legacy options
 	provider.AzureConfig = AzureOptions{
-		Tenant:          l.AzureTenant,
-		GraphGroupField: l.AzureGraphGroupField,
+		Tenant:              l.AzureTenant,
+		GraphGroupField:     l.AzureGraphGroupField,
+		AzureGroupsInTicket: l.AzureGroupsInTicket,
 	}
 
 	switch provider.Type {
diff --git a/pkg/apis/options/providers.go b/pkg/apis/options/providers.go
@@ -153,6 +153,11 @@ type AzureOptions struct {
 	// GraphGroupField configures the group field to be used when building the groups list from Microsoft Graph
 	// Default value is 'id'
 	GraphGroupField string `json:"graphGroupField,omitempty"`
+	// AzureGroupsInTicket determines whether request to Microsoft Graph api
+	// should be done to obtain user groups, or the groups should be taken
+	// from the ticket
+	// Default value is false
+	AzureGroupsInTicket bool `json:"groupsInTicket,omitempty"`
 }
 
 type ADFSOptions struct {
diff --git a/providers/azure.go b/providers/azure.go
@@ -26,6 +26,7 @@ type AzureProvider struct {
 	Tenant          string
 	GraphGroupField string
 	isV2Endpoint    bool
+	isGroupsInTicket  bool
 }
 
 var _ Provider = (*AzureProvider)(nil)
@@ -108,10 +109,11 @@ func NewAzureProvider(p *ProviderData, opts options.AzureOptions) *AzureProvider
 	}
 
 	return &AzureProvider{
-		ProviderData:    p,
-		Tenant:          tenant,
-		GraphGroupField: graphGroupField,
-		isV2Endpoint:    isV2Endpoint,
+		ProviderData:     p,
+		Tenant:           tenant,
+		GraphGroupField:  graphGroupField,
+		isV2Endpoint:     isV2Endpoint,
+		isGroupsInTicket: opts.AzureGroupsInTicket,
 	}
 }
 
@@ -133,7 +135,7 @@ func getMicrosoftGraphGroupsURL(profileURL *url.URL, graphGroupField string) *ur
 
 	// Select only security groups. Due to the filter option, count param is mandatory even if unused otherwise
 	return &url.URL{
-		Scheme:   "https",
+		Scheme:   profileURL.Scheme,
 		Host:     profileURL.Host,
 		Path:     "/v1.0/me/transitiveMemberOf",
 		RawQuery: "$count=true&$filter=securityEnabled+eq+true&" + selectStatement,
@@ -210,7 +212,7 @@ func (p *AzureProvider) EnrichSession(ctx context.Context, session *sessions.Ses
 	}
 
 	// If using the v2.0 oidc endpoint we're also querying Microsoft Graph
-	if p.isV2Endpoint {
+	if shouldCallGraphApi(p.isV2Endpoint, p.isGroupsInTicket) {
 		groups, err := p.getGroupsFromProfileAPI(ctx, session)
 		if err != nil {
 			return fmt.Errorf("unable to get groups from Microsoft Graph: %v", err)
@@ -220,6 +222,10 @@ func (p *AzureProvider) EnrichSession(ctx context.Context, session *sessions.Ses
 	return nil
 }
 
+func shouldCallGraphApi(isV2Endpoint bool, isGroupsInTicket bool) bool {
+	return isV2Endpoint && !isGroupsInTicket
+}
+
 func (p *AzureProvider) prepareRedeem(redirectURL, code, codeVerifier string) (url.Values, error) {
 	params := url.Values{}
 	if code == "" {
diff --git a/providers/azure_test.go b/providers/azure_test.go
@@ -141,17 +141,20 @@ func TestAzureSetTenant(t *testing.T) {
 	assert.Equal(t, "openid", p.Data().Scope)
 }
 
-func testAzureBackend(payload string, accessToken, refreshToken string) *httptest.Server {
-	return testAzureBackendWithError(payload, accessToken, refreshToken, false)
+func testAzureBackend(t *testing.T, payload string, accessToken, refreshToken string) *httptest.Server {
+	return testAzureBackendWithError(t, payload, accessToken, refreshToken, false, false)
 }
 
-func testAzureBackendWithError(payload string, accessToken, refreshToken string, injectError bool) *httptest.Server {
+func testAzureBackendWithError(t *testing.T, payload string, accessToken, refreshToken string, injectError bool, noCallToGraphApi bool) *httptest.Server {
 	path := "/v1.0/me"
-	pathGroups := path + "/transitiveMemberOf/microsoft.graph.group"
+	pathGroups := path + "/transitiveMemberOf"
 
 	return httptest.NewServer(http.HandlerFunc(
 		func(w http.ResponseWriter, r *http.Request) {
 			if r.URL.Path == pathGroups && r.Method == http.MethodGet {
+				if noCallToGraphApi {
+					assert.FailNow(t, "should be no call to graph api")
+				}
 				w.Write([]byte(`{
 					"@odata.context": "https://graph.microsoft.com/v1.0/$metadata#groups(displayName,id)",
 					"value": [
@@ -189,6 +192,9 @@ func TestAzureProviderEnrichSession(t *testing.T) {
 		Description             string
 		Email                   string
 		PayloadFromAzureBackend string
+		IsV2Endpoint		bool
+		IsGroupsInTicket	bool
+		ExpectedGroups		[]string
 		ExpectedEmail           string
 		ExpectedError           error
 	}{
@@ -197,6 +203,30 @@ func TestAzureProviderEnrichSession(t *testing.T) {
 			PayloadFromAzureBackend: `{ "mail": "user@windows.net", "groups": ["aa", "bb"] }`,
 			ExpectedEmail:           "user@windows.net",
 		},
+		{
+			Description:             "should return groups from graph api for v2 endpoint",
+			PayloadFromAzureBackend: `{ "mail": "user@windows.net" }`,
+			ExpectedEmail:           "user@windows.net",
+			ExpectedGroups:          []string{"11111111-2222-3333-4444-555555555555", "555555555555-4444-3333-2222-11111111"},
+			IsV2Endpoint:            true,
+			IsGroupsInTicket:        false,
+		},
+		{
+			Description:             "should be no call to graph api for v1 endpoint",
+			PayloadFromAzureBackend: `{ "mail": "user@windows.net" }`,
+			ExpectedEmail:           "user@windows.net",
+			ExpectedGroups:          nil,
+			IsV2Endpoint:            false,
+			IsGroupsInTicket:        false,
+		},
+		{
+			Description:             "should be no call to graph api for v2 endpoint and groups in ticket",
+			PayloadFromAzureBackend: `{ "mail": "user@windows.net" }`,
+			ExpectedEmail:           "user@windows.net",
+			ExpectedGroups:          nil,
+			IsV2Endpoint:            false,
+			IsGroupsInTicket:        true,
+		},
 		{
 			Description:             "should return email using otherMails property returned from Azure backend",
 			PayloadFromAzureBackend: `{ "mail": null, "otherMails": ["user@windows.net", "altuser@windows.net"] }`,
@@ -236,18 +266,20 @@ func TestAzureProviderEnrichSession(t *testing.T) {
 				host string
 			)
 
-			b = testAzureBackend(testCase.PayloadFromAzureBackend, authorizedAccessToken, "")
+			b = testAzureBackendWithError(t, testCase.PayloadFromAzureBackend, authorizedAccessToken, "", false, !shouldCallGraphApi(testCase.IsV2Endpoint, testCase.IsGroupsInTicket))
 			defer b.Close()
 
 			bURL, _ := url.Parse(b.URL)
 			host = bURL.Host
 
-			p := testAzureProvider(host, options.AzureOptions{})
+			p := testAzureProvider(host, options.AzureOptions{AzureGroupsInTicket: testCase.IsGroupsInTicket})
+			p.isV2Endpoint = testCase.IsV2Endpoint
 			session := CreateAuthorizedSession()
 			session.Email = testCase.Email
 			err := p.EnrichSession(context.Background(), session)
 			assert.Equal(t, testCase.ExpectedError, err)
 			assert.Equal(t, testCase.ExpectedEmail, session.Email)
+			assert.Equal(t, testCase.ExpectedGroups, session.Groups)
 		})
 	}
 }
@@ -341,7 +373,7 @@ func TestAzureProviderRedeem(t *testing.T) {
 			payloadBytes, err := json.Marshal(payload)
 			assert.NoError(t, err)
 
-			b := testAzureBackendWithError(string(payloadBytes), accessTokenString, testCase.RefreshToken, testCase.InjectRedeemURLError)
+			b := testAzureBackendWithError(t, string(payloadBytes), accessTokenString, testCase.RefreshToken, testCase.InjectRedeemURLError, false)
 			defer b.Close()
 
 			bURL, _ := url.Parse(b.URL)
@@ -412,7 +444,7 @@ func TestAzureProviderRefresh(t *testing.T) {
 	assert.NoError(t, err)
 
 	refreshToken := "some_refresh_token"
-	b := testAzureBackend(string(payloadBytes), newAccessToken, refreshToken)
+	b := testAzureBackend(t, string(payloadBytes), newAccessToken, refreshToken)
 	defer b.Close()
 	bURL, _ := url.Parse(b.URL)
 	p := testAzureProvider(bURL.Host, options.AzureOptions{})
