diff --git a/cmd/argocd/commands/login.go b/cmd/argocd/commands/login.go index 94449f1eae..7aecaca335 100644 --- a/cmd/argocd/commands/login.go +++ b/cmd/argocd/commands/login.go @@ -243,7 +243,7 @@ func oauth2Login(ctx context.Context, port int, oauth2conf *oauth2.Config, provi fmt.Printf("Opening browser for authentication\n") var url string - grantType := oidcutil.InferGrantType(oauth2conf, oidcConf) + grantType := oidcutil.InferGrantType(oidcConf) switch grantType { case oidcutil.GrantTypeAuthorizationCode: url = oauth2conf.AuthCodeURL(stateNonce, oauth2.AccessTypeOffline) diff --git a/util/oidc/oidc.go b/util/oidc/oidc.go index 6c4657e53a..a8a5e9fcc3 100644 --- a/util/oidc/oidc.go +++ b/util/oidc/oidc.go @@ -175,7 +175,7 @@ func (a *ClientApp) HandleLogin(w http.ResponseWriter, r *http.Request) { } returnURL := r.FormValue("return_url") stateNonce := a.generateAppState(returnURL) - grantType := InferGrantType(oauth2Config, oidcConf) + grantType := InferGrantType(oidcConf) var url string switch grantType { case GrantTypeAuthorizationCode: @@ -358,17 +358,16 @@ func OfflineAccess(scopes []string) bool { // InferGrantType infers the proper grant flow depending on the OAuth2 client config and OIDC configuration. // Returns either: "authorization_code" or "implicit" -func InferGrantType(oauth2conf *oauth2.Config, oidcConf *OIDCConfiguration) string { - if oauth2conf.ClientSecret != "" { - // If we know the client secret, we are using the 'authorization_code' flow - return GrantTypeAuthorizationCode +func InferGrantType(oidcConf *OIDCConfiguration) string { + // Check the supported response types. If the list contains the response type 'code', + // then grant type is 'authorization_code'. This is preferred over the implicit + // grant type since refresh tokens cannot be issued that way. + for _, supportedType := range oidcConf.ResponseTypesSupported { + if supportedType == ResponseTypeCode { + return GrantTypeAuthorizationCode + } } - if len(oidcConf.ResponseTypesSupported) == 1 && oidcConf.ResponseTypesSupported[0] == ResponseTypeCode { - // If we don't have the secret, check the supported response types. If the list is a single - // response type of type 'code', then grant type is 'authorization_code'. This is the Dex - // case, which does not support implicit login flow (https://github.com/dexidp/dex/issues/1254) - return GrantTypeAuthorizationCode - } - // If we don't have the client secret (e.g. SPA app), we can assume to be implicit + + // Assume implicit otherwise return GrantTypeImplicit } diff --git a/util/oidc/oidc_test.go b/util/oidc/oidc_test.go index 47ed7cdedc..33ace0b85b 100644 --- a/util/oidc/oidc_test.go +++ b/util/oidc/oidc_test.go @@ -6,43 +6,29 @@ import ( "testing" "github.com/stretchr/testify/assert" - "golang.org/x/oauth2" -) - -var ( - spaOauth2Conf = &oauth2.Config{ - ClientID: "spa-id", - } - - webOauth2Conf = &oauth2.Config{ - ClientID: "spa-id", - ClientSecret: "my-super-secret", - } ) func TestInferGrantType(t *testing.T) { - var grantType string - dexRAW, err := ioutil.ReadFile("testdata/dex.json") - assert.NoError(t, err) - var dexConfig OIDCConfiguration - err = json.Unmarshal(dexRAW, &dexConfig) - assert.NoError(t, err) - grantType = InferGrantType(spaOauth2Conf, &dexConfig) - // Dex does not support implicit login flow (https://github.com/dexidp/dex/issues/1254) - assert.Equal(t, GrantTypeAuthorizationCode, grantType) - grantType = InferGrantType(webOauth2Conf, &dexConfig) - assert.Equal(t, GrantTypeAuthorizationCode, grantType) + for _, path := range []string{"dex", "okta", "auth0", "onelogin"} { + t.Run(path, func(t *testing.T) { + rawConfig, err := ioutil.ReadFile("testdata/" + path + ".json") + assert.NoError(t, err) + var config OIDCConfiguration + err = json.Unmarshal(rawConfig, &config) + assert.NoError(t, err) + grantType := InferGrantType(&config) + assert.Equal(t, GrantTypeAuthorizationCode, grantType) - testFiles := []string{"testdata/okta.json", "testdata/auth0.json", "testdata/onelogin.json"} - for _, path := range testFiles { - oktaRAW, err := ioutil.ReadFile(path) - assert.NoError(t, err) - var oktaConfig OIDCConfiguration - err = json.Unmarshal(oktaRAW, &oktaConfig) - assert.NoError(t, err) - grantType = InferGrantType(spaOauth2Conf, &oktaConfig) - assert.Equal(t, GrantTypeImplicit, grantType) - grantType = InferGrantType(webOauth2Conf, &oktaConfig) - assert.Equal(t, GrantTypeAuthorizationCode, grantType) + var noCodeResponseTypes []string + for _, supportedResponseType := range config.ResponseTypesSupported { + if supportedResponseType != ResponseTypeCode { + noCodeResponseTypes = append(noCodeResponseTypes, supportedResponseType) + } + } + + config.ResponseTypesSupported = noCodeResponseTypes + grantType = InferGrantType(&config) + assert.Equal(t, GrantTypeImplicit, grantType) + }) } }