diff --git a/docs/operator-manual/user-management/index.md b/docs/operator-manual/user-management/index.md index 4dbef39dc594f..73999c4fd4496 100644 --- a/docs/operator-manual/user-management/index.md +++ b/docs/operator-manual/user-management/index.md @@ -302,8 +302,9 @@ data: clientID: aaaabbbbccccddddeee clientSecret: $oidc.okta.clientSecret - # Optional list of allowed aud claims. If omitted or empty, defaults to the clientID value above. If you specify a - # list and want the clientD to be allowed, you must explicitly include it in the list. + # Optional list of allowed aud claims. If omitted or empty, defaults to the clientID value above (and the + # cliCientID, if that is also specified). If you specify a list and want the clientID to be allowed, you must + # explicitly include it in the list. # Token verification will pass if any of the token's audiences matches any of the audiences in this list. allowedAudiences: - aaaabbbbccccddddeee diff --git a/util/oidc/provider.go b/util/oidc/provider.go index 6facfdb368447..fcb1a95b60f4f 100644 --- a/util/oidc/provider.go +++ b/util/oidc/provider.go @@ -107,6 +107,13 @@ func (p *providerImpl) Verify(tokenString string, argoSettings *settings.ArgoCDS // Token must be verified for at least one allowed audience for _, aud := range allowedAudiences { idToken, err = p.verify(aud, tokenString, false) + tokenExpiredError := &gooidc.TokenExpiredError{} + if errors.As(err, &tokenExpiredError) { + // If the token is expired, we won't bother checking other audiences. It's important to return a + // TokenExpiredError instead of an error related to an incorrect audience, because the caller may + // have specific behavior to handle expired tokens. + break + } if err == nil { break } diff --git a/util/settings/settings.go b/util/settings/settings.go index e7c819474a8c4..8680d02d51b63 100644 --- a/util/settings/settings.go +++ b/util/settings/settings.go @@ -1743,12 +1743,16 @@ func (a *ArgoCDSettings) OAuth2ClientID() string { func (a *ArgoCDSettings) OAuth2AllowedAudiences() []string { if config := a.oidcConfig(); config != nil { if len(config.AllowedAudiences) == 0 { - return []string{config.ClientID} + allowedAudiences := []string{config.ClientID} + if config.CLIClientID != "" { + allowedAudiences = append(allowedAudiences, config.CLIClientID) + } + return allowedAudiences } return config.AllowedAudiences } if a.DexConfig != "" { - return []string{common.ArgoCDClientAppID} + return []string{common.ArgoCDClientAppID, common.ArgoCDCLIClientAppID} } return nil } diff --git a/util/settings/settings_test.go b/util/settings/settings_test.go index 4949729d9bae0..1c3449ee155f8 100644 --- a/util/settings/settings_test.go +++ b/util/settings/settings_test.go @@ -1343,3 +1343,68 @@ rootCA: "invalid"`}, }) } } + +func Test_OAuth2AllowedAudiences(t *testing.T) { + testCases := []struct { + name string + settings *ArgoCDSettings + expected []string + }{ + { + name: "Empty", + settings: &ArgoCDSettings{}, + expected: []string{}, + }, + { + name: "OIDC configured, no audiences specified, clientID used", + settings: &ArgoCDSettings{OIDCConfigRAW: `name: Test +issuer: aaa +clientID: xxx +clientSecret: yyy +requestedScopes: ["oidc"]`}, + expected: []string{"xxx"}, + }, + { + name: "OIDC configured, no audiences specified, clientID and cliClientID used", + settings: &ArgoCDSettings{OIDCConfigRAW: `name: Test +issuer: aaa +clientID: xxx +cliClientID: cli-xxx +clientSecret: yyy +requestedScopes: ["oidc"]`}, + expected: []string{"xxx", "cli-xxx"}, + }, + { + name: "OIDC configured, audiences specified", + settings: &ArgoCDSettings{OIDCConfigRAW: `name: Test +issuer: aaa +clientID: xxx +clientSecret: yyy +requestedScopes: ["oidc"] +allowedAudiences: ["aud1", "aud2"]`}, + expected: []string{"aud1", "aud2"}, + }, + { + name: "Dex configured", + settings: &ArgoCDSettings{DexConfig: `connectors: + - type: github + id: github + name: GitHub + config: + clientID: aabbccddeeff00112233 + clientSecret: $dex.github.clientSecret + orgs: + - name: your-github-org +`}, + expected: []string{common.ArgoCDClientAppID, common.ArgoCDCLIClientAppID}, + }, + } + + for _, tc := range testCases { + tcc := tc + t.Run(tcc.name, func(t *testing.T) { + t.Parallel() + assert.ElementsMatch(t, tcc.expected, tcc.settings.OAuth2AllowedAudiences()) + }) + } +}