diff --git a/common/common.go b/common/common.go index dc5ad38bdf6c7..5467d6d8c98e9 100644 --- a/common/common.go +++ b/common/common.go @@ -158,6 +158,8 @@ const ( ArgoCDCLIClientAppName = "Argo CD CLI" // ArgoCDCLIClientAppID is the Oauth client ID we will use when registering our CLI to dex ArgoCDCLIClientAppID = "argo-cd-cli" + // DexFederatedScope allows to receive the federated_claims from Dex. https://dexidp.io/docs/configuration/custom-scopes-claims-clients/ + DexFederatedScope = "federated:id" ) // Resource metadata labels and annotations (keys and values) used by Argo CD components diff --git a/docs/operator-manual/upgrading/2.14-3.0.md b/docs/operator-manual/upgrading/2.14-3.0.md index 34a7b7ae032b1..9d362ad9939db 100644 --- a/docs/operator-manual/upgrading/2.14-3.0.md +++ b/docs/operator-manual/upgrading/2.14-3.0.md @@ -25,9 +25,9 @@ Read the [RBAC documentation](../rbac.md#fine-grained-permissions-for-updatedele ### Removal of `argocd_app_sync_status`, `argocd_app_health_status` and `argocd_app_created_time` Metrics -The `argocd_app_sync_status`, `argocd_app_health_status` and `argocd_app_created_time`, deprecated and disabled by +The `argocd_app_sync_status`, `argocd_app_health_status` and `argocd_app_created_time`, deprecated and disabled by default since 1.5.0, have been removed. The information previously provided by these metrics is now available as labels -on the `argocd_app_info` metric. +on the `argocd_app_info` metric. #### Detection @@ -39,6 +39,38 @@ If it is not set to true, you can safely upgrade with no changes. If you are using these metrics, you will need to update your monitoring dashboards and alerts to use the new metric and labels before upgrading. +### Changes to RBAC with Dex SSO Authentication + +When using Dex, the `sub` claim returned in the authentication was used as the subject for RBAC. That value depends on +the Dex internal implementation and should not be considered an immutable value that represent the subject. + +The new behavior will request the `federated:id` [scope](https://dexidp.io/docs/configuration/custom-scopes-claims-clients/) from Dex, and the new value used as the RBAC subject will be based +on the `federated_claims.user_id` claim instead of the `sub` claim. + +If you were using the Dex sub claim in RBAC policies, you will need to update them to maintain the same access. + +You can know the correct `user_id` to use by decoding the current `sub` claims defined in your policies. You can also configure which +value is used as `user_id` for some [connectors](https://dexidp.io/docs/connectors/). + +```sh +$> echo "ChdleGFtcGxlQGFyZ29wcm9qLmlvEgJkZXhfY29ubl9pZA" | base64 -d + +example@argoproj.iodex_conn_i% +``` + +```yaml +# Policies based on the Dex sub claim (wrong) +- g, ChdleGFtcGxlQGFyZ29wcm9qLmlvEgJkZXhfY29ubl9pZA, role:example +- p, ChdleGFtcGxlQGFyZ29wcm9qLmlvEgJkZXhfY29ubl9pZA, applications, *, *, allow + +# Policies now based on federated_claims.user_id claim (correct) +- g, example@argoproj.io, role:example +- p, example@argoproj.io, applications, *, *, allow +``` + +If authenticating with the CLI, make sure to use the new version as well to obtain an authentication token with the +appropriate claims. + ## Other changes ### Using `cluster.inClusterEnabled: "false"` diff --git a/pkg/apiclient/apiclient.go b/pkg/apiclient/apiclient.go index ea56a3b939444..0890360c25cf6 100644 --- a/pkg/apiclient/apiclient.go +++ b/pkg/apiclient/apiclient.go @@ -327,9 +327,10 @@ func (c *client) OIDCConfig(ctx context.Context, set *settingspkg.Settings) (*oa clientID = set.OIDCConfig.ClientID } issuerURL = set.OIDCConfig.Issuer - scopes = set.OIDCConfig.Scopes + scopes = oidcutil.GetScopesOrDefault(set.OIDCConfig.Scopes) case set.DexConfig != nil && len(set.DexConfig.Connectors) > 0: clientID = common.ArgoCDCLIClientAppID + scopes = append(oidcutil.GetScopesOrDefault(nil), common.DexFederatedScope) issuerURL = fmt.Sprintf("%s%s", set.URL, common.DexAPIEndpoint) default: return nil, nil, fmt.Errorf("%s is not configured with SSO", c.ServerAddr) @@ -342,7 +343,6 @@ func (c *client) OIDCConfig(ctx context.Context, set *settingspkg.Settings) (*oa if err != nil { return nil, nil, fmt.Errorf("Failed to parse provider config: %w", err) } - scopes = oidcutil.GetScopesOrDefault(scopes) if oidcutil.OfflineAccess(oidcConf.ScopesSupported) { scopes = append(scopes, oidc.ScopeOfflineAccess) } diff --git a/util/oidc/oidc.go b/util/oidc/oidc.go index 2633cd8f308c3..7ef37bd0a0d09 100644 --- a/util/oidc/oidc.go +++ b/util/oidc/oidc.go @@ -314,10 +314,13 @@ func (a *ClientApp) HandleLogin(w http.ResponseWriter, r *http.Request) { scopes := make([]string, 0) var opts []oauth2.AuthCodeOption if config := a.settings.OIDCConfig(); config != nil { - scopes = config.RequestedScopes + scopes = GetScopesOrDefault(config.RequestedScopes) opts = AppendClaimsAuthenticationRequestParameter(opts, config.RequestedIDTokenClaims) + } else if a.settings.IsDexConfigured() { + scopes = append(GetScopesOrDefault(nil), common.DexFederatedScope) } - oauth2Config, err := a.oauth2Config(r, GetScopesOrDefault(scopes)) + + oauth2Config, err := a.oauth2Config(r, scopes) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return diff --git a/util/oidc/oidc_test.go b/util/oidc/oidc_test.go index 46d1e1f7904f6..a98d4ed761c6b 100644 --- a/util/oidc/oidc_test.go +++ b/util/oidc/oidc_test.go @@ -21,6 +21,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/oauth2" + "sigs.k8s.io/yaml" "github.com/argoproj/argo-cd/v3/common" "github.com/argoproj/argo-cd/v3/server/settings/oidc" @@ -204,6 +205,108 @@ requestedScopes: ["oidc"]`, oidcTestServer.URL), assert.NotContains(t, w.Body.String(), "certificate signed by unknown authority") }) + t.Run("OIDC auth", func(t *testing.T) { + cdSettings := &settings.ArgoCDSettings{ + URL: "https://argocd.example.com", + OIDCTLSInsecureSkipVerify: true, + } + oidcConfig := settings.OIDCConfig{ + Name: "Test", + Issuer: oidcTestServer.URL, + ClientID: "xxx", + ClientSecret: "yyy", + } + oidcConfigRaw, err := yaml.Marshal(oidcConfig) + require.NoError(t, err) + cdSettings.OIDCConfigRAW = string(oidcConfigRaw) + + app, err := NewClientApp(cdSettings, dexTestServer.URL, &dex.DexTLSConfig{StrictValidation: false}, "https://argocd.example.com", cache.NewInMemoryCache(24*time.Hour)) + require.NoError(t, err) + + req := httptest.NewRequest(http.MethodGet, "https://argocd.example.com/auth/login", nil) + w := httptest.NewRecorder() + app.HandleLogin(w, req) + + assert.Equal(t, http.StatusSeeOther, w.Code) + location, err := url.Parse(w.Header().Get("Location")) + require.NoError(t, err) + values, err := url.ParseQuery(location.RawQuery) + require.NoError(t, err) + assert.Equal(t, []string{"openid", "profile", "email", "groups"}, strings.Split(values.Get("scope"), " ")) + assert.Equal(t, "xxx", values.Get("client_id")) + assert.Equal(t, "code", values.Get("response_type")) + }) + + t.Run("OIDC auth with custom scopes", func(t *testing.T) { + cdSettings := &settings.ArgoCDSettings{ + URL: "https://argocd.example.com", + OIDCTLSInsecureSkipVerify: true, + } + oidcConfig := settings.OIDCConfig{ + Name: "Test", + Issuer: oidcTestServer.URL, + ClientID: "xxx", + ClientSecret: "yyy", + RequestedScopes: []string{"oidc"}, + } + oidcConfigRaw, err := yaml.Marshal(oidcConfig) + require.NoError(t, err) + cdSettings.OIDCConfigRAW = string(oidcConfigRaw) + + app, err := NewClientApp(cdSettings, dexTestServer.URL, &dex.DexTLSConfig{StrictValidation: false}, "https://argocd.example.com", cache.NewInMemoryCache(24*time.Hour)) + require.NoError(t, err) + + req := httptest.NewRequest(http.MethodGet, "https://argocd.example.com/auth/login", nil) + w := httptest.NewRecorder() + app.HandleLogin(w, req) + + assert.Equal(t, http.StatusSeeOther, w.Code) + location, err := url.Parse(w.Header().Get("Location")) + require.NoError(t, err) + values, err := url.ParseQuery(location.RawQuery) + require.NoError(t, err) + assert.Equal(t, []string{"oidc"}, strings.Split(values.Get("scope"), " ")) + assert.Equal(t, "xxx", values.Get("client_id")) + assert.Equal(t, "code", values.Get("response_type")) + }) + + t.Run("Dex auth", func(t *testing.T) { + cdSettings := &settings.ArgoCDSettings{ + URL: dexTestServer.URL, + } + dexConfig := map[string]any{ + "connectors": []map[string]any{ + { + "type": "github", + "name": "GitHub", + "config": map[string]any{ + "clientId": "aabbccddeeff00112233", + "clientSecret": "aabbccddeeff00112233", + }, + }, + }, + } + dexConfigRaw, err := yaml.Marshal(dexConfig) + require.NoError(t, err) + cdSettings.DexConfig = string(dexConfigRaw) + + app, err := NewClientApp(cdSettings, dexTestServer.URL, &dex.DexTLSConfig{StrictValidation: false}, "https://argocd.example.com", cache.NewInMemoryCache(24*time.Hour)) + require.NoError(t, err) + + req := httptest.NewRequest(http.MethodGet, "https://argocd.example.com/auth/login", nil) + w := httptest.NewRecorder() + app.HandleLogin(w, req) + + assert.Equal(t, http.StatusSeeOther, w.Code) + location, err := url.Parse(w.Header().Get("Location")) + require.NoError(t, err) + values, err := url.ParseQuery(location.RawQuery) + require.NoError(t, err) + assert.Equal(t, []string{"openid", "profile", "email", "groups", common.DexFederatedScope}, strings.Split(values.Get("scope"), " ")) + assert.Equal(t, common.ArgoCDClientAppID, values.Get("client_id")) + assert.Equal(t, "code", values.Get("response_type")) + }) + t.Run("with additional base URL", func(t *testing.T) { cdSettings := &settings.ArgoCDSettings{ URL: "https://argocd.example.com",