feat(security): require the aud claim from OIDC providers by default (

#12187)

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

docs/operator-manual/upgrading/2.5-2.6.md

@@ -10,4 +10,14 @@ Masterminds/semver v3 changed the behavior of the `^` prefix in semantic version
 ## Applications with suspended jobs now marked "Suspended" instead of "Progressing"
 Prior to Argo CD v2.6, an Application managing a suspended Job would be marked as "Progressing". This was confusing/unexpected behavior for many. Starting with v2.6, Argo CD will mark such Applications as "Suspended".
 
-If you have processes which rely on the previous behavior (for example, a CI job with an argocd app wait call), update those before upgrading to v2.6.
+If you have processes which rely on the previous behavior (for example, a CI job with an argocd app wait call), update those before upgrading to v2.6.
+
+## The API Server now requires tokens to include the `aud` claim by default
+
+Argo CD v2.6 now requires that the `aud` claim be present in the token used to authenticate to the API Server. This is a 
+security improvement, as it prevents tokens from being used against the API Server which were not intended for it.
+
+If you rely on an OIDC provider which does not provide a `aud` claim, you can disable this requirement by setting the 
+`skipAudienceCheckWhenTokenHasNoAudience` flag to `true` in your Argo CD OIDC configuration. (See the 
+[OIDC configuration documentation](https://argo-cd.readthedocs.io/en/stable/operator-manual/user-management/#existing-oidc-provider)
+for an example.)

util/session/sessionmanager_test.go

@@ -893,7 +893,43 @@ requestedScopes: ["oidc"]`, oidcTestServer.URL),
 		require.NoError(t, err)
 
 		_, _, err = mgr.VerifyToken(tokenString)
+		assert.Error(t, err)
+	})
+
+	t.Run("OIDC provider is external, audience is not specified, absent audience is allowed", func(t *testing.T) {
+		config := map[string]string{
+			"url": "",
+			"oidc.config": fmt.Sprintf(`
+name: Test
+issuer: %s
+clientID: xxx
+clientSecret: yyy
+requestedScopes: ["oidc"]
+skipAudienceCheckWhenTokenHasNoAudience: true`, oidcTestServer.URL),
+			"oidc.tls.insecure.skip.verify": "true", // This isn't what we're testing.
+		}
+
+		// This is not actually used in the test. The test only calls the OIDC test server. But a valid cert/key pair
+		// must be set to test VerifyToken's behavior when Argo CD is configured with TLS enabled.
+		secretConfig := map[string][]byte{
+			"tls.crt": utiltest.Cert,
+			"tls.key": utiltest.PrivateKey,
+		}
+
+		settingsMgr := settings.NewSettingsManager(context.Background(), getKubeClientWithConfig(config, secretConfig), "argocd")
+		mgr := NewSessionManager(settingsMgr, getProjLister(), "", nil, NewUserStateStorage(nil))
+		mgr.verificationDelayNoiseEnabled = false
+
+		claims := jwt.RegisteredClaims{Subject: "admin", ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Hour * 24))}
+		claims.Issuer = oidcTestServer.URL
+		token := jwt.NewWithClaims(jwt.SigningMethodRS512, claims)
+		key, err := jwt.ParseRSAPrivateKeyFromPEM(utiltest.PrivateKey)
+		require.NoError(t, err)
+		tokenString, err := token.SignedString(key)
 		require.NoError(t, err)
+
+		_, _, err = mgr.VerifyToken(tokenString)
+		assert.NoError(t, err)
 	})
 
 	t.Run("OIDC provider is external, audience is not specified but is required", func(t *testing.T) {

util/settings/settings.go

@@ -1762,7 +1762,7 @@ func (a *ArgoCDSettings) SkipAudienceCheckWhenTokenHasNoAudience() bool {
 		if config.SkipAudienceCheckWhenTokenHasNoAudience != nil {
 			return *config.SkipAudienceCheckWhenTokenHasNoAudience
 		}
-		return true
+		return false
 	}
 	// When using the bundled Dex, the audience check is required. Dex will always send JWTs with an audience.
 	return false