Skip to content
4 changes: 4 additions & 0 deletions docs/operator-manual/user-management/keycloak.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ data:
issuer: https://keycloak.example.com/realms/master
clientID: argocd
clientSecret: $oidc.keycloak.clientSecret
refreshTokenThreshold: 2m
requestedScopes: ["openid", "profile", "email", "groups"]
```

Expand All @@ -77,6 +78,7 @@ Make sure that:
- __clientID__ is set to the Client ID you configured in Keycloak
- __clientSecret__ points to the right key you created in the _argocd-secret_ Secret
- __requestedScopes__ contains the _groups_ claim if you didn't add it to the Default scopes
- __refreshTokenThreshold__ is less than the client token lifetime. If this setting is not less than the token lifetime, a new token will be obtained for every request. Keycloak sets the client token lifetime to 5 minutes by default.

## Keycloak and ArgoCD with PKCE

Expand Down Expand Up @@ -135,6 +137,7 @@ data:
issuer: https://keycloak.example.com/realms/master
clientID: argocd
enablePKCEAuthentication: true
refreshTokenThreshold: 2m
requestedScopes: ["openid", "profile", "email", "groups"]
```

Expand All @@ -145,6 +148,7 @@ Make sure that:
- __clientID__ is set to the Client ID you configured in Keycloak
- __enablePKCEAuthentication__ must be set to true to enable correct ArgoCD behaviour with PKCE
- __requestedScopes__ contains the _groups_ claim if you didn't add it to the Default scopes
- __refreshTokenThreshold__ is less than the client token lifetime. If this setting is not less than the token lifetime, a new token will be obtained for every request. Keycloak sets the client token lifetime to 5 minutes by default.

## Configuring the groups claim

Expand Down
2 changes: 1 addition & 1 deletion server/application/websocket.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ func (t *terminalSession) performValidationsAndReconnect(p []byte) (int, error)
}

// check if token still valid
_, newToken, err := t.sessionManager.VerifyToken(*t.token)
_, newToken, err := t.sessionManager.VerifyToken(t.ctx, *t.token)
// err in case if token is revoked, newToken in case if refresh happened
if err != nil || newToken != "" {
// need to send reconnect code in case if token was refreshed
Expand Down
4 changes: 2 additions & 2 deletions server/logout/logout.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func NewHandler(settingsMrg *settings.SettingsManager, sessionMgr *session.Sessi
type Handler struct {
settingsMgr *settings.SettingsManager
rootPath string
verifyToken func(tokenString string) (jwt.Claims, string, error)
verifyToken func(ctx context.Context, tokenString string) (jwt.Claims, string, error)
revokeToken func(ctx context.Context, id string, expiringAt time.Duration) error
baseHRef string
}
Expand Down Expand Up @@ -94,7 +94,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
w.Header().Add("Set-Cookie", argocdCookie.String())
}

claims, _, err := h.verifyToken(tokenString)
claims, _, err := h.verifyToken(r.Context(), tokenString)
if err != nil {
http.Redirect(w, r, logoutRedirectURL, http.StatusSeeOther)
return
Expand Down
11 changes: 6 additions & 5 deletions server/logout/logout_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package logout

import (
"context"
"errors"
"net/http"
"net/http/httptest"
Expand Down Expand Up @@ -245,36 +246,36 @@ func TestHandlerConstructLogoutURL(t *testing.T) {
sessionManager := session.NewSessionManager(settingsManagerWithOIDCConfig, test.NewFakeProjLister(), "", nil, session.NewUserStateStorage(nil))

oidcHandler := NewHandler(settingsManagerWithOIDCConfig, sessionManager, rootPath, baseHRef)
oidcHandler.verifyToken = func(tokenString string) (jwt.Claims, string, error) {
oidcHandler.verifyToken = func(_ context.Context, tokenString string) (jwt.Claims, string, error) {
if !validJWTPattern.MatchString(tokenString) {
return nil, "", errors.New("invalid jwt")
}
return &jwt.RegisteredClaims{Issuer: "okta"}, "", nil
}
nonoidcHandler := NewHandler(settingsManagerWithoutOIDCConfig, sessionManager, "", baseHRef)
nonoidcHandler.verifyToken = func(tokenString string) (jwt.Claims, string, error) {
nonoidcHandler.verifyToken = func(_ context.Context, tokenString string) (jwt.Claims, string, error) {
if !validJWTPattern.MatchString(tokenString) {
return nil, "", errors.New("invalid jwt")
}
return &jwt.RegisteredClaims{Issuer: session.SessionManagerClaimsIssuer}, "", nil
}
oidcHandlerWithoutLogoutURL := NewHandler(settingsManagerWithOIDCConfigButNoLogoutURL, sessionManager, "", baseHRef)
oidcHandlerWithoutLogoutURL.verifyToken = func(tokenString string) (jwt.Claims, string, error) {
oidcHandlerWithoutLogoutURL.verifyToken = func(_ context.Context, tokenString string) (jwt.Claims, string, error) {
if !validJWTPattern.MatchString(tokenString) {
return nil, "", errors.New("invalid jwt")
}
return &jwt.RegisteredClaims{Issuer: "okta"}, "", nil
}
nonoidcHandlerWithMultipleURLs := NewHandler(settingsManagerWithoutOIDCAndMultipleURLs, sessionManager, "", baseHRef)
nonoidcHandlerWithMultipleURLs.verifyToken = func(tokenString string) (jwt.Claims, string, error) {
nonoidcHandlerWithMultipleURLs.verifyToken = func(_ context.Context, tokenString string) (jwt.Claims, string, error) {
if !validJWTPattern.MatchString(tokenString) {
return nil, "", errors.New("invalid jwt")
}
return &jwt.RegisteredClaims{Issuer: "okta"}, "", nil
}

oidcHandlerWithoutBaseURL := NewHandler(settingsManagerWithOIDCConfigButNoURL, sessionManager, "argocd", baseHRef)
oidcHandlerWithoutBaseURL.verifyToken = func(tokenString string) (jwt.Claims, string, error) {
oidcHandlerWithoutBaseURL.verifyToken = func(_ context.Context, tokenString string) (jwt.Claims, string, error) {
if !validJWTPattern.MatchString(tokenString) {
return nil, "", errors.New("invalid jwt")
}
Expand Down
39 changes: 19 additions & 20 deletions server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,8 @@ func NewServer(ctx context.Context, opts ArgoCDServerOpts, appsetOpts Applicatio
appsetLister := appFactory.Argoproj().V1alpha1().ApplicationSets().Lister()

userStateStorage := util_session.NewUserStateStorage(opts.RedisClient)
ssoClientApp, err := oidc.NewClientApp(settings, opts.DexServerAddr, opts.DexTLSConfig, opts.BaseHRef, cacheutil.NewRedisCache(opts.RedisClient, settings.UserInfoCacheExpiration(), cacheutil.RedisCompressionNone))
errorsutil.CheckError(err)
sessionMgr := util_session.NewSessionManager(settingsMgr, projLister, opts.DexServerAddr, opts.DexTLSConfig, userStateStorage)
enf := rbac.NewEnforcer(opts.KubeClientset, opts.Namespace, common.ArgoCDRBACConfigMapName, nil)
enf.EnableEnforce(!opts.DisableAuth)
Expand Down Expand Up @@ -370,6 +372,7 @@ func NewServer(ctx context.Context, opts ArgoCDServerOpts, appsetOpts Applicatio
a := &ArgoCDServer{
ArgoCDServerOpts: opts,
ApplicationSetOpts: appsetOpts,
ssoClientApp: ssoClientApp,
log: logger,
settings: settings,
sessionMgr: sessionMgr,
Expand Down Expand Up @@ -1125,19 +1128,7 @@ func (server *ArgoCDServer) translateGrpcCookieHeader(ctx context.Context, w htt
}

func (server *ArgoCDServer) setTokenCookie(token string, w http.ResponseWriter) error {
cookiePath := "path=/" + strings.TrimRight(strings.TrimLeft(server.BaseHRef, "/"), "/")
flags := []string{cookiePath, "SameSite=lax", "httpOnly"}
if !server.Insecure {
flags = append(flags, "Secure")
}
cookies, err := httputil.MakeCookieMetadata(common.AuthCookieName, token, flags...)
if err != nil {
return fmt.Errorf("error creating cookie metadata: %w", err)
}
for _, cookie := range cookies {
w.Header().Add("Set-Cookie", cookie)
}
return nil
return httputil.SetTokenCookie(token, server.BaseHRef, !server.Insecure, w)
}

func withRootPath(handler http.Handler, a *ArgoCDServer) http.Handler {
Expand Down Expand Up @@ -1221,9 +1212,6 @@ func (server *ArgoCDServer) newHTTPServer(ctx context.Context, port int, grpcWeb

terminalOpts := application.TerminalOptions{DisableAuth: server.DisableAuth, Enf: server.enf}

// SSO ClientApp
server.ssoClientApp, _ = oidc.NewClientApp(server.settings, server.DexServerAddr, server.DexTLSConfig, server.BaseHRef, cacheutil.NewRedisCache(server.RedisClient, server.settings.UserInfoCacheExpiration(), cacheutil.RedisCompressionNone))

terminal := application.NewHandler(server.appLister, server.Namespace, server.ApplicationNamespaces, server.db, appResourceTreeFn, server.settings.ExecShells, server.sessionMgr, &terminalOpts).
WithFeatureFlagMiddleware(server.settingsMgr.GetSettings)
th := util_session.WithAuthMiddleware(server.DisableAuth, server.settings.IsSSOConfigured(), server.ssoClientApp, server.sessionMgr, terminal)
Expand Down Expand Up @@ -1368,9 +1356,7 @@ func (server *ArgoCDServer) registerDexHandlers(mux *http.ServeMux) {
return
}
// Run dex OpenID Connect Identity Provider behind a reverse proxy (served at /api/dex)
var err error
mux.HandleFunc(common.DexAPIEndpoint+"/", dexutil.NewDexHTTPReverseProxy(server.DexServerAddr, server.BaseHRef, server.DexTLSConfig))
errorsutil.CheckError(err)
mux.HandleFunc(common.LoginEndpoint, server.ssoClientApp.HandleLogin)
mux.HandleFunc(common.CallbackEndpoint, server.ssoClientApp.HandleCallback)
}
Expand Down Expand Up @@ -1566,6 +1552,7 @@ func (server *ArgoCDServer) Authenticate(ctx context.Context) (context.Context,
return ctx, nil
}

// getClaims extracts, validates and refreshes a JWT token from an incoming request context.
func (server *ArgoCDServer) getClaims(ctx context.Context) (jwt.Claims, string, error) {
md, ok := metadata.FromIncomingContext(ctx)
if !ok {
Expand All @@ -1575,17 +1562,29 @@ func (server *ArgoCDServer) getClaims(ctx context.Context) (jwt.Claims, string,
if tokenString == "" {
return nil, "", ErrNoSession
}
claims, newToken, err := server.sessionMgr.VerifyToken(tokenString)
// A valid argocd-issued token is automatically refreshed here prior to expiration.
// OIDC tokens will be verified but will not be refreshed here.
claims, newToken, err := server.sessionMgr.VerifyToken(ctx, tokenString)
if err != nil {
return claims, "", status.Errorf(codes.Unauthenticated, "invalid session: %v", err)
}

finalClaims := claims
if server.settings.IsSSOConfigured() {
finalClaims, err = server.ssoClientApp.SetGroupsFromUserInfo(claims, util_session.SessionManagerClaimsIssuer)
updatedClaims, err := server.ssoClientApp.SetGroupsFromUserInfo(ctx, claims, util_session.SessionManagerClaimsIssuer)
if err != nil {
return claims, "", status.Errorf(codes.Unauthenticated, "invalid session: %v", err)
}
finalClaims = updatedClaims
// OIDC tokens are automatically refreshed here prior to expiration
refreshedToken, err := server.ssoClientApp.CheckAndRefreshToken(ctx, updatedClaims, server.settings.OIDCRefreshTokenThreshold)
if err != nil {
log.Errorf("error checking and refreshing token: %v", err)
}
if refreshedToken != "" && refreshedToken != tokenString {
newToken = refreshedToken
log.Infof("refreshed token for subject: %v", jwtutil.StringField(updatedClaims, "sub"))
}
}

return finalClaims, newToken, nil
Expand Down
20 changes: 20 additions & 0 deletions util/http/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,3 +241,23 @@ func drainBody(body io.ReadCloser) {
log.Warnf("error reading response body: %s", err.Error())
}
}

func SetTokenCookie(token string, baseHRef string, isSecure bool, w http.ResponseWriter) error {
var path string
if baseHRef != "" {
path = strings.TrimRight(strings.TrimLeft(baseHRef, "/"), "/")
}
cookiePath := "path=/" + path
flags := []string{cookiePath, "SameSite=lax", "httpOnly"}
if isSecure {
flags = append(flags, "Secure")
}
cookies, err := MakeCookieMetadata(common.AuthCookieName, token, flags...)
if err != nil {
return fmt.Errorf("error creating cookie metadata: %w", err)
}
for _, cookie := range cookies {
w.Header().Add("Set-Cookie", cookie)
}
return nil
}
98 changes: 98 additions & 0 deletions util/http/http_test.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
package http

import (
"fmt"
"net/http"
"strings"
"testing"

"github.com/argoproj/argo-cd/v3/common"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -49,6 +52,101 @@ func TestSplitCookie(t *testing.T) {
assert.Equal(t, cookieValue, token)
}

// mockResponseWriter is a mock implementation of http.ResponseWriter.
// It captures added headers for verification in tests.
type mockResponseWriter struct {
header http.Header
}

func (m *mockResponseWriter) Header() http.Header {
if m.header == nil {
m.header = make(http.Header)
}
return m.header
}
func (m *mockResponseWriter) Write([]byte) (int, error) { return 0, nil }
func (m *mockResponseWriter) WriteHeader(_ int) {}

func TestSetTokenCookie(t *testing.T) {
tests := []struct {
name string
token string
baseHRef string
isSecure bool
expectedCookies []string // Expected Set-Cookie header values
}{
{
name: "Insecure cookie",
token: "insecure-token",
baseHRef: "",
isSecure: false,
expectedCookies: []string{
fmt.Sprintf("%s=%s; path=/; SameSite=lax; httpOnly", common.AuthCookieName, "insecure-token"),
},
},
{
name: "Secure cookie",
token: "secure-token",
baseHRef: "",
isSecure: true,
expectedCookies: []string{
fmt.Sprintf("%s=%s; path=/; SameSite=lax; httpOnly; Secure", common.AuthCookieName, "secure-token"),
},
},
{
name: "Insecure cookie with baseHRef",
token: "token-with-path",
baseHRef: "/app",
isSecure: false,
expectedCookies: []string{
fmt.Sprintf("%s=%s; path=/app; SameSite=lax; httpOnly", common.AuthCookieName, "token-with-path"),
},
},
{
name: "Secure cookie with baseHRef",
token: "secure-token-with-path",
baseHRef: "app/",
isSecure: true,
expectedCookies: []string{
fmt.Sprintf("%s=%s; path=/app; SameSite=lax; httpOnly; Secure", common.AuthCookieName, "secure-token-with-path"),
},
},
{
name: "Unsecured cookie, baseHRef with multiple segments and mixed slashes",
token: "complex-path-token",
baseHRef: "///api/v1/auth///",
isSecure: false,
expectedCookies: []string{
fmt.Sprintf("%s=%s; path=/api/v1/auth; SameSite=lax; httpOnly", common.AuthCookieName, "complex-path-token"),
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
w := &mockResponseWriter{}

err := SetTokenCookie(tt.token, tt.baseHRef, tt.isSecure, w)
if err != nil {
t.Fatalf("%s: Unexpected error: %v", tt.name, err)
}

setCookieHeaders := w.Header()["Set-Cookie"]

if len(setCookieHeaders) != len(tt.expectedCookies) {
t.Errorf("Mistmatch in Set-Cookie header length: %s\nExpected: %d\nGot: %d",
tt.name, len(tt.expectedCookies), len(setCookieHeaders))
return
}

if len(tt.expectedCookies) > 0 && setCookieHeaders[0] != tt.expectedCookies[0] {
t.Errorf("Mismatch in Set-Cookie header: %s\nExpected: %s\nGot: %s",
tt.name, tt.expectedCookies[0], setCookieHeaders[0])
}
})
}
}

// TestRoundTripper just copy request headers to the resposne.
type TestRoundTripper struct{}

Expand Down
Loading
Loading