Skip to content

fix(server): avoid unnecessary claims restrictions (#22973) #23202

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
crenshaw-dev merged 3 commits into from
May 29, 2025
Merged

fix(server): avoid unnecessary claims restrictions (#22973) #23202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fc661ae
fix(server): avoid unecessary claims restrictions (#22973)
crenshaw-dev May 29, 2025
8265f3e
simplify
crenshaw-dev May 29, 2025
ecd7f6f
consolidate on one util module
crenshaw-dev May 29, 2025
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
22 changes: 9 additions & 13 deletions cmd/argocd/commands/login.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
"strings"
"time"

jwtutil "github.com/argoproj/argo-cd/v3/util/jwt"

"github.com/coreos/go-oidc/v3/oidc"
"github.com/golang-jwt/jwt/v5"
log "github.com/sirupsen/logrus"
Expand All @@ -23,7 +25,6 @@ import (
argocdclient "github.com/argoproj/argo-cd/v3/pkg/apiclient"
sessionpkg "github.com/argoproj/argo-cd/v3/pkg/apiclient/session"
settingspkg "github.com/argoproj/argo-cd/v3/pkg/apiclient/settings"
claimsutil "github.com/argoproj/argo-cd/v3/util/claims"
"github.com/argoproj/argo-cd/v3/util/cli"
"github.com/argoproj/argo-cd/v3/util/errors"
grpc_util "github.com/argoproj/argo-cd/v3/util/grpc"
Expand Down Expand Up @@ -143,9 +144,7 @@ argocd login cd.argoproj.io --core`,
claims := jwt.MapClaims{}
_, _, err := parser.ParseUnverified(tokenString, &claims)
errors.CheckError(err)
argoClaims, err := claimsutil.MapClaimsToArgoClaims(claims)
errors.CheckError(err)
fmt.Printf("'%s' logged in successfully\n", userDisplayName(argoClaims))
fmt.Printf("'%s' logged in successfully\n", userDisplayName(claims))
}

// login successful. Persist the config
Expand Down Expand Up @@ -192,17 +191,14 @@ argocd login cd.argoproj.io --core`,
return command
}

func userDisplayName(claims *claimsutil.ArgoClaims) string {
if claims == nil {
return ""
}
if claims.Email != "" {
return claims.Email
func userDisplayName(claims jwt.MapClaims) string {
if email := jwtutil.StringField(claims, "email"); email != "" {
return email
}
if claims.Name != "" {
return claims.Name
if name := jwtutil.StringField(claims, "name"); name != "" {
return name
}
return claims.GetUserIdentifier()
return jwtutil.GetUserIdentifier(claims)
}

// oauth2Login opens a browser, runs a temporary HTTP server to delegate OAuth2 login flow and
Expand Down
16 changes: 5 additions & 11 deletions cmd/argocd/commands/login_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,10 @@ import (
"os"
"testing"

claimsutil "github.com/argoproj/argo-cd/v3/util/claims"
utilio "github.com/argoproj/argo-cd/v3/util/io"

"github.com/golang-jwt/jwt/v5"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func captureStdout(callback func()) (string, error) {
Expand All @@ -37,40 +35,36 @@ func captureStdout(callback func()) (string, error) {
}

func Test_userDisplayName_email(t *testing.T) {
claims, err := claimsutil.MapClaimsToArgoClaims(jwt.MapClaims{"iss": "qux", "sub": "foo", "email": "firstname.lastname@example.com", "groups": []string{"baz"}})
require.NoError(t, err)
claims := jwt.MapClaims{"iss": "qux", "sub": "foo", "email": "firstname.lastname@example.com", "groups": []string{"baz"}}
actualName := userDisplayName(claims)
expectedName := "firstname.lastname@example.com"
assert.Equal(t, expectedName, actualName)
}

func Test_userDisplayName_name(t *testing.T) {
claims, err := claimsutil.MapClaimsToArgoClaims(jwt.MapClaims{"iss": "qux", "sub": "foo", "name": "Firstname Lastname", "groups": []string{"baz"}})
require.NoError(t, err)
claims := jwt.MapClaims{"iss": "qux", "sub": "foo", "name": "Firstname Lastname", "groups": []string{"baz"}}
actualName := userDisplayName(claims)
expectedName := "Firstname Lastname"
assert.Equal(t, expectedName, actualName)
}

func Test_userDisplayName_sub(t *testing.T) {
claims, err := claimsutil.MapClaimsToArgoClaims(jwt.MapClaims{"iss": "qux", "sub": "foo", "groups": []string{"baz"}})
require.NoError(t, err)
claims := jwt.MapClaims{"iss": "qux", "sub": "foo", "groups": []string{"baz"}}
actualName := userDisplayName(claims)
expectedName := "foo"
assert.Equal(t, expectedName, actualName)
}

func Test_userDisplayName_federatedClaims(t *testing.T) {
claims, err := claimsutil.MapClaimsToArgoClaims(jwt.MapClaims{
claims := jwt.MapClaims{
"iss": "qux",
"sub": "foo",
"groups": []string{"baz"},
"federated_claims": map[string]any{
"connector_id": "dex",
"user_id": "ldap-123",
},
})
require.NoError(t, err)
}
actualName := userDisplayName(claims)
expectedName := "ldap-123"
assert.Equal(t, expectedName, actualName)
Expand Down
17 changes: 5 additions & 12 deletions cmd/argocd/commands/project_role.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
argocdclient "github.com/argoproj/argo-cd/v3/pkg/apiclient"
projectpkg "github.com/argoproj/argo-cd/v3/pkg/apiclient/project"
"github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1"
claimsutil "github.com/argoproj/argo-cd/v3/util/claims"
"github.com/argoproj/argo-cd/v3/util/errors"
utilio "github.com/argoproj/argo-cd/v3/util/io"
"github.com/argoproj/argo-cd/v3/util/jwt"
Expand Down Expand Up @@ -322,25 +321,19 @@ Create token succeeded for proj:test-project:test-role.
})
errors.CheckError(err)

var claims jwtgo.MapClaims
_, _, err = jwtgo.NewParser().ParseUnverified(tokenResponse.Token, &claims)
if err != nil {
token, err := jwtgo.Parse(tokenResponse.Token, nil)
if token == nil {
err = fmt.Errorf("received malformed token %w", err)
errors.CheckError(err)
return
}

argoClaims, err := claimsutil.MapClaimsToArgoClaims(claims)
if err != nil {
errors.CheckError(fmt.Errorf("invalid argo claims: %w", err))
return
}
claims := token.Claims.(jwtgo.MapClaims)

issuedAt, _ := jwt.IssuedAt(claims)
expiresAt := int64(jwt.Float64Field(claims, "exp"))
id := argoClaims.ID
subject := argoClaims.GetUserIdentifier()

id := jwt.StringField(claims, "jti")
subject := jwt.GetUserIdentifier(claims)
if !outputTokenOnly {
fmt.Printf("Create token succeeded for %s.\n", subject)
fmt.Printf(" ID: %s\n Issued At: %s\n Expires At: %s\n",
Expand Down
7 changes: 1 addition & 6 deletions server/rbacpolicy/rbacpolicy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (

"github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1"
applister "github.com/argoproj/argo-cd/v3/pkg/client/listers/application/v1alpha1"
claimsutil "github.com/argoproj/argo-cd/v3/util/claims"
jwtutil "github.com/argoproj/argo-cd/v3/util/jwt"
"github.com/argoproj/argo-cd/v3/util/rbac"
)
Expand Down Expand Up @@ -62,12 +61,8 @@ func (p *RBACPolicyEnforcer) EnforceClaims(claims jwt.Claims, rvals ...any) bool
if err != nil {
return false
}
argoClaims, err := claimsutil.MapClaimsToArgoClaims(mapClaims)
if err != nil {
return false
}

subject := argoClaims.GetUserIdentifier()
subject := jwtutil.GetUserIdentifier(mapClaims)
// 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
33 changes: 14 additions & 19 deletions server/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ import (
"github.com/argoproj/argo-cd/v3/ui"
"github.com/argoproj/argo-cd/v3/util/assets"
cacheutil "github.com/argoproj/argo-cd/v3/util/cache"
claimsutil "github.com/argoproj/argo-cd/v3/util/claims"
"github.com/argoproj/argo-cd/v3/util/db"
dexutil "github.com/argoproj/argo-cd/v3/util/dex"
"github.com/argoproj/argo-cd/v3/util/env"
Expand Down Expand Up @@ -1560,19 +1559,19 @@ func (server *ArgoCDServer) getClaims(ctx context.Context) (jwt.Claims, string,
return claims, "", status.Errorf(codes.Unauthenticated, "invalid session: %v", err)
}

mapClaims, err := jwtutil.MapClaims(claims)
if err != nil {
return claims, "", status.Errorf(codes.Internal, "invalid claims")
}
argoClaims, err := claimsutil.MapClaimsToArgoClaims(mapClaims)
if err != nil {
return claims, "", status.Errorf(codes.Internal, "invalid argo claims")
// Some SSO implementations (Okta) require a call to
// the OIDC user info path to get attributes like groups
// we assume that everywhere in argocd jwt.MapClaims is used as type for interface jwt.Claims
// otherwise this would cause a panic
var groupClaims jwt.MapClaims
if groupClaims, ok = claims.(jwt.MapClaims); !ok {
if tmpClaims, ok := claims.(*jwt.MapClaims); ok {
groupClaims = *tmpClaims
}
}

// Some SSO implementations (Okta) require a call to the OIDC user info path to get attributes like groups
iss := jwtutil.StringField(mapClaims, "iss")
iss := jwtutil.StringField(groupClaims, "iss")
if iss != util_session.SessionManagerClaimsIssuer && server.settings.UserInfoGroupsEnabled() && server.settings.UserInfoPath() != "" {
userInfo, unauthorized, err := server.ssoClientApp.GetUserInfo(mapClaims, server.settings.IssuerURL(), server.settings.UserInfoPath())
userInfo, unauthorized, err := server.ssoClientApp.GetUserInfo(groupClaims, server.settings.IssuerURL(), server.settings.UserInfoPath())
if unauthorized {
log.Errorf("error while quering userinfo endpoint: %v", err)
return claims, "", status.Errorf(codes.Unauthenticated, "invalid session")
Expand All @@ -1581,17 +1580,13 @@ func (server *ArgoCDServer) getClaims(ctx context.Context) (jwt.Claims, string,
log.Errorf("error fetching user info endpoint: %v", err)
return claims, "", status.Errorf(codes.Internal, "invalid userinfo response")
}
userInfoClaims, err := claimsutil.MapClaimsToArgoClaims(userInfo)
if err != nil {
return claims, "", status.Errorf(codes.Internal, "invalid userinfo claims")
}
if argoClaims.Subject != userInfoClaims.Subject {
if groupClaims["sub"] != userInfo["sub"] {
return claims, "", status.Error(codes.Unknown, "subject of claims from user info endpoint didn't match subject of idToken, see https://openid.net/specs/openid-connect-core-1_0.html#UserInfo")
}
mapClaims["groups"] = userInfo["groups"]
groupClaims["groups"] = userInfo["groups"]
}

return mapClaims, newToken, nil
return groupClaims, newToken, nil
}

// getToken extracts the token from gRPC metadata or cookie headers
Expand Down
56 changes: 0 additions & 56 deletions util/claims/claims.go
View file
Open in desktop

This file was deleted.

Loading
Loading