Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions assets/builtin-policy.csv
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,4 @@ p, role:admin, exec, create, */*, allow

g, role:admin, role:readonly
g, admin, role:admin
g, admin@local, role:admin
5 changes: 5 additions & 0 deletions docs/operator-manual/argocd-cm.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,11 @@ data:
# We highly recommend that this be set to `true`. The next major release will set the default to be `true`.
application.sync.requireOverridePrivilegeForRevisionSync: "true"

# If true, local account names are disambiguated from SSO users during RBAC enforcement by appending an "@local"
# suffix to the local account name (for example, reference local user `sally` as `sally@local` in RBAC policies).
# Users still log in with their normal account name; the suffix only affects RBAC matching. Disabled by default.
rbac.local.user.strictmode: "true"

### SourceHydrator commit author name (optional).
# Configures the author name for commits created by the Source Hydrator.
# If not specified, defaults to "Argo CD".
Expand Down
43 changes: 43 additions & 0 deletions docs/operator-manual/rbac.md
Original file line number Diff line number Diff line change
Expand Up @@ -413,8 +413,51 @@ g, my-local-user, role:admin
> p, my-local-user, *, *, *, allow
> ```

### Disambiguating Local Users with Strict Mode

To remove the ambiguity described above without changing how you assign roles, you can enable strict mode for local
users. When enabled, Argo CD appends an `@local` suffix to local account names *during RBAC enforcement only*. This
ensures that a policy bound to a local user does not accidentally apply to an SSO user with a matching scope.

Enable strict mode by setting the following key in the `argocd-cm` ConfigMap:

```yaml
apiVersion: v1
kind: ConfigMap
metadata:
name: argocd-cm
namespace: argocd
labels:
app.kubernetes.io/name: argocd-cm
app.kubernetes.io/part-of: argocd
data:
rbac.local.user.strictmode: "true"
```

With strict mode enabled, reference local users in your RBAC policies using the `@local` suffix. For example, to grant
the local user `sally` the `role:developer` role:

```yaml
g, sally@local, role:developer
```

> [!NOTE]
> Users continue to log in with their normal account name (for example `sally`) via the CLI and UI. The `@local`
> suffix is used exclusively for RBAC matching. When strict mode is enabled, the user info section of the UI displays
> the `@local` identity (for example `sally@local`) so you know which subject to reference in your policies.

> [!WARNING]
> When you enable strict mode, any existing RBAC policies that reference local users by their plain name (for example
> `g, sally, role:developer`) will stop matching those local users. You must update such bindings to use the `@local`
> suffix. The built-in `admin` account is already granted `role:admin` for both `admin` and `admin@local`, so the
> default admin access is preserved.
>
> Strict mode is disabled by default; when it is unset, local account names are matched verbatim (the pre-existing
> behavior).

## Policy CSV Composition


It is possible to provide additional entries in the `argocd-rbac-cm` configmap to compose the final policy csv.
In this case, the key must follow the pattern `policy.<any string>.csv`.
Argo CD will concatenate all additional policies it finds with this pattern below the main one ('policy.csv').
Expand Down
36 changes: 36 additions & 0 deletions docs/operator-manual/user-management/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,44 @@ argocd account update-password \
argocd account generate-token --account <username>
```

### Disambiguating local users from SSO users (strict mode)

When both local users and [SSO](#sso) are configured, an SSO user whose scope (for example a group or org name)
matches a local user's name inherits the same RBAC roles as that local user. See
[Ambiguous Group Assignments](../rbac.md#local-usersaccounts) for details.

To avoid this, enable strict mode for local users in the `argocd-cm` ConfigMap:

```yaml
apiVersion: v1
kind: ConfigMap
metadata:
name: argocd-cm
namespace: argocd
labels:
app.kubernetes.io/name: argocd-cm
app.kubernetes.io/part-of: argocd
data:
rbac.local.user.strictmode: "true"
```

When enabled, Argo CD appends an `@local` suffix to local account names during RBAC enforcement only. Reference local
users in your RBAC policies using that suffix, for example `g, sally@local, role:developer`.

> [!NOTE]
> Users still log in with their normal account name (for example `sally`) in both the CLI and UI. The `@local` suffix
> only affects RBAC matching. While strict mode is enabled, the UI user info section displays the `@local` identity so
> you know which subject to reference in your policies.

> [!WARNING]
> Enabling strict mode causes existing RBAC bindings that reference local users by their plain name to stop matching.
> Update those bindings to use the `@local` suffix. Strict mode is disabled by default, preserving the existing
> verbatim-matching behavior. See [Local Users/Accounts](../rbac.md#local-usersaccounts) in the RBAC documentation for
> more details.

### Failed logins rate limiting


Argo CD rejects login attempts after too many failed in order to prevent password brute-forcing.
The following environments variables are available to control throttling settings:

Expand Down
35 changes: 32 additions & 3 deletions server/rbacpolicy/rbacpolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,25 @@ import (
"github.com/argoproj/argo-cd/v3/util/rbac"
)

// LocalUserRBACSuffix is appended to Argo CD local account names during RBAC enforcement when
// strict mode (rbac.local.user.strictmode) is enabled. This disambiguates local accounts from SSO
// users that happen to share the same name/scope, so that e.g. a policy bound to `sally@local`
// only applies to the local `sally` account and not to an SSO user named `sally`.
const LocalUserRBACSuffix = "@local"

// localAccountIssuer is the value of the "iss" claim for Argo CD local account tokens. It mirrors
// session.SessionManagerClaimsIssuer, which cannot be imported here because the session package
// already imports this package (which would create an import cycle).
const localAccountIssuer = "argocd"

// RBACPolicyEnforcer provides an RBAC Claims Enforcer which additionally consults AppProject
// roles, jwt tokens, and groups. It is backed by a AppProject informer/lister cache and does not
// make any API calls during enforcement.
type RBACPolicyEnforcer struct {
enf *rbac.Enforcer
projLister applister.AppProjectNamespaceLister
scopes []string
enf *rbac.Enforcer
projLister applister.AppProjectNamespaceLister
scopes []string
enableLocalUserStrictMode bool
}

// NewRBACPolicyEnforcer returns a new RBAC Enforcer for the Argo CD API Server
Expand All @@ -42,6 +54,12 @@ func (p *RBACPolicyEnforcer) GetScopes() []string {
return scopes
}

// SetEnableLocalUserStrictMode toggles whether local account names are suffixed with
// LocalUserRBACSuffix ("@local") during RBAC enforcement to disambiguate them from SSO users.
func (p *RBACPolicyEnforcer) SetEnableLocalUserStrictMode(enabled bool) {
p.enableLocalUserStrictMode = enabled
}

func IsProjectSubject(subject string) bool {
_, _, ok := GetProjectRoleFromSubject(subject)
return ok
Expand All @@ -55,6 +73,12 @@ func GetProjectRoleFromSubject(subject string) (string, string, bool) {
return "", "", false
}

// isLocalAccount returns true if the claims belong to an Argo CD local account token (as opposed
// to an SSO/IDP token), determined by the token issuer.
func isLocalAccount(mapClaims jwt.MapClaims) bool {
return jwtutil.StringField(mapClaims, "iss") == localAccountIssuer
}

// EnforceClaims is an RBAC claims enforcer specific to the Argo CD API server
func (p *RBACPolicyEnforcer) EnforceClaims(claims jwt.Claims, rvals ...any) bool {
mapClaims, err := jwtutil.MapClaims(claims)
Expand All @@ -63,6 +87,11 @@ func (p *RBACPolicyEnforcer) EnforceClaims(claims jwt.Claims, rvals ...any) bool
}

subject := jwtutil.GetUserIdentifier(mapClaims)
// When strict mode is enabled, disambiguate local accounts from SSO users by appending the
// "@local" suffix to the subject. Project tokens (proj:...) are left untouched.
if p.enableLocalUserStrictMode && isLocalAccount(mapClaims) && !IsProjectSubject(subject) {
subject += LocalUserRBACSuffix
}
// Check if the request is for an application resource. We have special enforcement which takes
// into consideration the project's token and group bindings
var runtimePolicy string
Expand Down
74 changes: 74 additions & 0 deletions server/rbacpolicy/rbacpolicy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,3 +241,77 @@ func Test_getProjectFromRequest(t *testing.T) {
})
}
}

func TestIsLocalAccount(t *testing.T) {
assert.True(t, isLocalAccount(jwt.MapClaims{"iss": localAccountIssuer}))
assert.False(t, isLocalAccount(jwt.MapClaims{"iss": "https://accounts.google.com"}))
assert.False(t, isLocalAccount(jwt.MapClaims{"iss": "sso"}))
assert.False(t, isLocalAccount(jwt.MapClaims{}))
}

func TestSetEnableLocalUserStrictMode(t *testing.T) {
rbacEnf := NewRBACPolicyEnforcer(nil, nil)
assert.False(t, rbacEnf.enableLocalUserStrictMode)
rbacEnf.SetEnableLocalUserStrictMode(true)
assert.True(t, rbacEnf.enableLocalUserStrictMode)
rbacEnf.SetEnableLocalUserStrictMode(false)
assert.False(t, rbacEnf.enableLocalUserStrictMode)
}

// TestEnforceLocalUserStrictMode verifies that, when strict mode is enabled, an RBAC policy bound
// to `sally@local` only grants permissions to the local `sally` account (iss=argocd) and not to an
// SSO user who happens to also be named `sally`. With strict mode disabled, both match the plain
// `sally` binding (the pre-existing, ambiguous behavior).
func TestEnforceLocalUserStrictMode(t *testing.T) {
newEnforcer := func(t *testing.T) (*rbac.Enforcer, *RBACPolicyEnforcer) {
t.Helper()
kubeclientset := fake.NewClientset(test.NewFakeConfigMap())
projLister := test.NewFakeProjLister(newFakeProj())
enf := rbac.NewEnforcer(kubeclientset, test.FakeArgoCDNamespace, common.ArgoCDConfigMapName, nil)
enf.EnableLog(true)
rbacEnf := NewRBACPolicyEnforcer(enf, projLister)
enf.SetClaimsEnforcerFunc(rbacEnf.EnforceClaims)
return enf, rbacEnf
}

localClaims := jwt.MapClaims{"sub": "sally", "iss": localAccountIssuer}
ssoClaims := jwt.MapClaims{"sub": "sally", "iss": "https://accounts.google.com"}

t.Run("strict mode: only the local account matches an @local binding", func(t *testing.T) {
enf, rbacEnf := newEnforcer(t)
rbacEnf.SetEnableLocalUserStrictMode(true)
_ = enf.SetBuiltinPolicy(`p, sally@local, applications, create, my-proj/*, allow`)

assert.True(t, enf.Enforce(localClaims, "applications", "create", "my-proj/my-app"),
"local sally should match the @local binding")
assert.False(t, enf.Enforce(ssoClaims, "applications", "create", "my-proj/my-app"),
"SSO sally must not inherit the local user's role")
})

t.Run("strict mode: a plain binding no longer matches the local account", func(t *testing.T) {
enf, rbacEnf := newEnforcer(t)
rbacEnf.SetEnableLocalUserStrictMode(true)
_ = enf.SetBuiltinPolicy(`p, sally, applications, create, my-proj/*, allow`)

assert.False(t, enf.Enforce(localClaims, "applications", "create", "my-proj/my-app"),
"local sally is enforced as sally@local and should not match the plain binding")
assert.True(t, enf.Enforce(ssoClaims, "applications", "create", "my-proj/my-app"),
"SSO sally is not suffixed and still matches the plain binding")
})

t.Run("strict mode disabled: both local and SSO match a plain binding", func(t *testing.T) {
enf, _ := newEnforcer(t)
_ = enf.SetBuiltinPolicy(`p, sally, applications, create, my-proj/*, allow`)

assert.True(t, enf.Enforce(localClaims, "applications", "create", "my-proj/my-app"))
assert.True(t, enf.Enforce(ssoClaims, "applications", "create", "my-proj/my-app"))
})

t.Run("strict mode: project tokens are left untouched", func(t *testing.T) {
enf, rbacEnf := newEnforcer(t)
rbacEnf.SetEnableLocalUserStrictMode(true)
claims := jwt.MapClaims{"sub": "proj:my-proj:my-role", "iat": 1234}
assert.True(t, enf.Enforce(claims, "applications", "create", "my-proj/my-app"),
"project role tokens must not be suffixed with @local")
})
}
2 changes: 2 additions & 0 deletions server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,7 @@ func NewServer(ctx context.Context, opts ArgoCDServerOpts, appsetOpts Applicatio
enf.EnableLog(os.Getenv(common.EnvVarRBACDebug) == "1")

policyEnf := rbacpolicy.NewRBACPolicyEnforcer(enf, projLister)
policyEnf.SetEnableLocalUserStrictMode(settings.RBACLocalUserStrictMode)
enf.SetClaimsEnforcerFunc(policyEnf.EnforceClaims)

staticFS, err := fs.Sub(ui.Embedded, "dist/app")
Expand Down Expand Up @@ -821,6 +822,7 @@ func (server *ArgoCDServer) watchSettings() {
for {
newSettings := <-updateCh
server.settings = newSettings
server.policyEnforcer.SetEnableLocalUserStrictMode(newSettings.RBACLocalUserStrictMode)
newDexCfgBytes, err := dexutil.GenerateDexConfigYAML(server.settings, server.DexTLSConfig == nil || server.DexTLSConfig.DisableTLS)
errorsutil.CheckError(err)
if !bytes.Equal(newDexCfgBytes, prevDexCfgBytes) {
Expand Down
14 changes: 12 additions & 2 deletions server/session/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,20 @@ func (s *Server) AuthFuncOverride(ctx context.Context, _ string) (context.Contex
}

func (s *Server) GetUserInfo(ctx context.Context, _ *session.GetUserInfoRequest) (*session.GetUserInfoResponse, error) {
username := sessionmgr.Username(ctx)
iss := sessionmgr.Iss(ctx)
// When strict mode is enabled, surface the "@local" RBAC identity for local accounts so users
// know which subject to reference in their RBAC policies. The underlying account name (used for
// actions such as changing the password) is unchanged.
if iss == sessionmgr.SessionManagerClaimsIssuer && username != "" && !rbacpolicy.IsProjectSubject(username) {
if argoCDSettings, err := s.settingsMgr.GetSettings(); err == nil && argoCDSettings.RBACLocalUserStrictMode {
username += rbacpolicy.LocalUserRBACSuffix
}
}
return &session.GetUserInfoResponse{
LoggedIn: sessionmgr.LoggedIn(ctx),
Username: sessionmgr.Username(ctx),
Iss: sessionmgr.Iss(ctx),
Username: username,
Iss: iss,
Groups: sessionmgr.Groups(ctx, s.policyEnf.GetScopes()),
}, nil
}
77 changes: 77 additions & 0 deletions server/session/session_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
package session

import (
"context"
"testing"

"github.com/golang-jwt/jwt/v5"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes/fake"

"github.com/argoproj/argo-cd/v3/common"
"github.com/argoproj/argo-cd/v3/server/rbacpolicy"
"github.com/argoproj/argo-cd/v3/util/rbac"
sessionmgr "github.com/argoproj/argo-cd/v3/util/session"
"github.com/argoproj/argo-cd/v3/util/settings"
)

func newTestSessionServer(t *testing.T, cmData map[string]string) *Server {
t.Helper()
const ns = "default"
kubeClient := fake.NewClientset(
&corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: common.ArgoCDConfigMapName,
Namespace: ns,
Labels: map[string]string{"app.kubernetes.io/part-of": "argocd"},
},
Data: cmData,
},
&corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: common.ArgoCDSecretName,
Namespace: ns,
},
Data: map[string][]byte{"server.secretkey": []byte("test")},
},
)
settingsMgr := settings.NewSettingsManager(t.Context(), kubeClient, ns)
enf := rbac.NewEnforcer(kubeClient, ns, common.ArgoCDConfigMapName, nil)
policyEnf := rbacpolicy.NewRBACPolicyEnforcer(enf, nil)
return NewServer(nil, settingsMgr, nil, policyEnf, nil)
}

func ctxWithClaims(claims jwt.MapClaims) context.Context {
//nolint:staticcheck // the production code reads the "claims" string key from context
return context.WithValue(context.Background(), "claims", claims)
}

func TestGetUserInfo_LocalUserStrictMode(t *testing.T) {
localClaims := jwt.MapClaims{"sub": "sally", "iss": sessionmgr.SessionManagerClaimsIssuer}
ssoClaims := jwt.MapClaims{"sub": "sally", "iss": "https://accounts.google.com", "email": "sally@example.com"}

t.Run("strict mode appends @local for local accounts", func(t *testing.T) {
s := newTestSessionServer(t, map[string]string{"rbac.local.user.strictmode": "true"})
resp, err := s.GetUserInfo(ctxWithClaims(localClaims), nil)
require.NoError(t, err)
assert.Equal(t, "sally@local", resp.Username)
assert.Equal(t, sessionmgr.SessionManagerClaimsIssuer, resp.Iss)
})

t.Run("strict mode does not affect SSO users", func(t *testing.T) {
s := newTestSessionServer(t, map[string]string{"rbac.local.user.strictmode": "true"})
resp, err := s.GetUserInfo(ctxWithClaims(ssoClaims), nil)
require.NoError(t, err)
assert.Equal(t, "sally@example.com", resp.Username)
})

t.Run("strict mode disabled leaves local username unchanged", func(t *testing.T) {
s := newTestSessionServer(t, map[string]string{})
resp, err := s.GetUserInfo(ctxWithClaims(localClaims), nil)
require.NoError(t, err)
assert.Equal(t, "sally", resp.Username)
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ export const UserInfoComponent = ({userInfo}: {userInfo: UserInfo}) => {
}>
<h4>Update account password</h4>
<Form
onSubmit={(params: PasswordFormData) => changePassword(userInfo.username, params.currentPassword, params.newPassword)}
onSubmit={(params: PasswordFormData) => changePassword('', params.currentPassword, params.newPassword)}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a UI expert but is this change correct?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The panel only renders for the logged-in local user (iss === 'argocd'), so this is always a self-service password change.

Passing an empty name tells the backend's UpdatePassword to resolve the account from the authenticated token instead of a client-supplied name. This is needed because the new strict mode now displays userInfo.username as sally@local (for example), which would no longer match the real account name (e.g. sally) and would break the update.

Empty name is correct in both strict and non-strict modes.

getApi={api => (formApiPassword.current = api)}
validateError={validatePasswordForm}>
{formApi => (
Expand Down
Loading
Loading