Repository: dexidp/dex
Pull request #3475 (2024-04-13 11:04:15)

feat: Add support for configurable prompt type for Google connector
<!--
Thank you for sending a pull request! Here are some tips for contributors:

1. Fill the description template below.
2. Sign a DCO (if you haven't already signed it).
3. Include appropriate tests (if necessary). Make sure that all CI checks passed.
4. If the Pull Request is a work in progress, make use of GitHub's "Draft PR" feature and mark it as such.
-->

#### Overview

Add support for configurable prompt type for Google Connector. Reference https://github.com/dexidp/dex/issues/3450

#### What this PR does / why we need it

<!--
- Please state in detail why we need this PR and what it solves.
- If your PR closes some of the existing issues, please add links to them here.
  Mentioned issues will be automatically closed.
  Usage: "Closes #<issue number>", or "Closes (paste link of issue)"
-->

- Allows the user to explicitly configure the value of `prompt` parameter in login URL generated by Google connector

#### Special notes for your reviewer

Ref https://github.com/dexidp/dex/issues/3450

The original ask was to avoid the user experience of consent screen every time authentication flow goes through `dex` into Google by skipping `prompt=consent` flag. As per OIDC specification, there is no valid value for `prompt` param to achieve this. So this PR just allows the ability to set the value of `prompt` parameter through configuration. Setting an explicit `""` (empty) value for prompt should be functionally equivalent for skipping it for Google connector



Patch:
diff --git a/connector/google/google.go b/connector/google/google.go
index d5908672..c2df1100 100644
--- a/connector/google/google.go
+++ b/connector/google/google.go
@@ -58,6 +58,10 @@ type Config struct {
 
 	// If this field is true, fetch direct group membership and transitive group membership
 	FetchTransitiveGroupMembership bool `json:"fetchTransitiveGroupMembership"`
+
+	// Optional value for the prompt parameter, defaults to consent when offline_access
+	// scope is requested
+	PromptType *string `json:"promptType"`
 }
 
 // Open returns a connector which can be used to login users through Google.
@@ -107,6 +111,11 @@ func (c *Config) Open(id string, logger log.Logger) (conn connector.Connector, e
 		}
 	}
 
+	promptType := "consent"
+	if c.PromptType != nil {
+		promptType = *c.PromptType
+	}
+
 	clientID := c.ClientID
 	return &googleConnector{
 		redirectURI: c.RedirectURI,
@@ -128,6 +137,7 @@ func (c *Config) Open(id string, logger log.Logger) (conn connector.Connector, e
 		domainToAdminEmail:             c.DomainToAdminEmail,
 		fetchTransitiveGroupMembership: c.FetchTransitiveGroupMembership,
 		adminSrv:                       adminSrv,
+		promptType:                     promptType,
 	}, nil
 }
 
@@ -148,6 +158,7 @@ type googleConnector struct {
 	domainToAdminEmail             map[string]string
 	fetchTransitiveGroupMembership bool
 	adminSrv                       map[string]*admin.Service
+	promptType                     string
 }
 
 func (c *googleConnector) Close() error {
@@ -170,8 +181,9 @@ func (c *googleConnector) LoginURL(s connector.Scopes, callbackURL, state string
 	}
 
 	if s.OfflineAccess {
-		opts = append(opts, oauth2.AccessTypeOffline, oauth2.SetAuthURLParam("prompt", "consent"))
+		opts = append(opts, oauth2.AccessTypeOffline, oauth2.SetAuthURLParam("prompt", c.promptType))
 	}
+
 	return c.oauth2Config.AuthCodeURL(state, opts...), nil
 }
 
diff --git a/connector/oidc/oidc.go b/connector/oidc/oidc.go
index c2918006..fd715970 100644
--- a/connector/oidc/oidc.go
+++ b/connector/oidc/oidc.go
@@ -75,7 +75,7 @@ type Config struct {
 
 	UserNameKey string `json:"userNameKey"`
 
-	// PromptType will be used fot the prompt parameter (when offline_access, by default prompt=consent)
+	// PromptType will be used for the prompt parameter (when offline_access, by default prompt=consent)
 	PromptType *string `json:"promptType"`
 
 	// OverrideClaimMapping will be used to override the options defined in claimMappings.


Test patch:
diff --git a/connector/google/google_test.go b/connector/google/google_test.go
index aaefb879..c56a5e78 100644
--- a/connector/google/google_test.go
+++ b/connector/google/google_test.go
@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"net/http"
 	"net/http/httptest"
+	"net/url"
 	"os"
 	"testing"
 
@@ -13,6 +14,8 @@ import (
 	"github.com/stretchr/testify/assert"
 	admin "google.golang.org/api/admin/directory/v1"
 	"google.golang.org/api/option"
+
+	"github.com/dexidp/dex/connector"
 )
 
 var (
@@ -291,3 +294,60 @@ func TestDomainToAdminEmailConfig(t *testing.T) {
 		})
 	}
 }
+
+func TestPromptTypeConfig(t *testing.T) {
+	promptTypeLogin := "login"
+	cases := []struct {
+		name                    string
+		promptType              *string
+		expectedPromptTypeValue string
+	}{
+		{
+			name:                    "prompt type is nil",
+			promptType:              nil,
+			expectedPromptTypeValue: "consent",
+		},
+		{
+			name:                    "prompt type is empty",
+			promptType:              new(string),
+			expectedPromptTypeValue: "",
+		},
+		{
+			name:                    "prompt type is set",
+			promptType:              &promptTypeLogin,
+			expectedPromptTypeValue: "login",
+		},
+	}
+
+	ts := testSetup()
+	defer ts.Close()
+
+	serviceAccountFilePath, err := tempServiceAccountKey()
+	assert.Nil(t, err)
+
+	os.Setenv("GOOGLE_APPLICATION_CREDENTIALS", serviceAccountFilePath)
+
+	for _, test := range cases {
+		t.Run(test.name, func(t *testing.T) {
+			conn, err := newConnector(&Config{
+				ClientID:           "testClient",
+				ClientSecret:       "testSecret",
+				RedirectURI:        ts.URL + "/callback",
+				Scopes:             []string{"openid", "groups", "offline_access"},
+				DomainToAdminEmail: map[string]string{"dexidp.com": "admin@dexidp.com"},
+				PromptType:         test.promptType,
+			})
+
+			assert.Nil(t, err)
+			assert.Equal(t, test.expectedPromptTypeValue, conn.promptType)
+
+			loginURL, err := conn.LoginURL(connector.Scopes{OfflineAccess: true}, ts.URL+"/callback", "state")
+			assert.Nil(t, err)
+
+			urlp, err := url.Parse(loginURL)
+			assert.Nil(t, err)
+
+			assert.Equal(t, test.expectedPromptTypeValue, urlp.Query().Get("prompt"))
+		})
+	}
+}