diff --git a/cmd/argocd/commands/logout.go b/cmd/argocd/commands/logout.go index aad7ef2bff..d79192a521 100644 --- a/cmd/argocd/commands/logout.go +++ b/cmd/argocd/commands/logout.go @@ -1,15 +1,23 @@ package commands import ( + "context" + "crypto/tls" + "errors" "fmt" + "net/http" "os" + "time" log "github.com/sirupsen/logrus" "github.com/spf13/cobra" "github.com/argoproj/argo-cd/v3/cmd/argocd/commands/utils" + "github.com/argoproj/argo-cd/v3/common" argocdclient "github.com/argoproj/argo-cd/v3/pkg/apiclient" - "github.com/argoproj/argo-cd/v3/util/errors" + "github.com/argoproj/argo-cd/v3/util/cli" + errutil "github.com/argoproj/argo-cd/v3/util/errors" + grpc_util "github.com/argoproj/argo-cd/v3/util/grpc" "github.com/argoproj/argo-cd/v3/util/localconfig" ) @@ -34,7 +42,7 @@ argocd logout cd.argoproj.io context := args[0] localCfg, err := localconfig.ReadLocalConfig(globalClientOpts.ConfigPath) - errors.CheckError(err) + errutil.CheckError(err) if localCfg == nil { log.Fatalf("Nothing to logout from") } @@ -43,6 +51,43 @@ argocd logout cd.argoproj.io canLogout := promptUtil.Confirm(fmt.Sprintf("Are you sure you want to log out from '%s'?", context)) if canLogout { + if tlsTestResult, err := grpc_util.TestTLS(context, common.BearerTokenTimeout); err != nil { + log.Warnf("failed to check the TLS config settings for the server : %v.", err) + globalClientOpts.PlainText = true + } else { + if !tlsTestResult.TLS { + if !globalClientOpts.PlainText { + if !cli.AskToProceed("WARNING: server is not configured with TLS. Proceed (y/n)? ") { + os.Exit(1) + } + globalClientOpts.PlainText = true + } + } else if tlsTestResult.InsecureErr != nil { + if !globalClientOpts.Insecure { + if !cli.AskToProceed(fmt.Sprintf("WARNING: server certificate had error: %s. Proceed insecurely (y/n)? ", tlsTestResult.InsecureErr)) { + os.Exit(1) + } + globalClientOpts.Insecure = true + } + } + } + + scheme := "https" + if globalClientOpts.PlainText { + scheme = "http" + } + if res, err := revokeServerToken(scheme, context, localCfg.GetToken(context), globalClientOpts.Insecure); err != nil { + log.Warnf("failed to invalidate token on server: %v.", err) + } else { + _ = res.Body.Close() + if res.StatusCode != http.StatusOK && res.StatusCode != http.StatusSeeOther { + log.Warnf("server returned unexpected status code %d during logout", res.StatusCode) + } else { + log.Infof("token successfully invalidated on server") + } + } + + // Remove token from local config ok := localCfg.RemoveToken(context) if !ok { log.Fatalf("Context %s does not exist", context) @@ -53,7 +98,7 @@ argocd logout cd.argoproj.io log.Fatalf("Error in logging out: %s", err) } err = localconfig.WriteLocalConfig(*localCfg, globalClientOpts.ConfigPath) - errors.CheckError(err) + errutil.CheckError(err) fmt.Printf("Logged out from '%s'\n", context) } else { @@ -63,3 +108,31 @@ argocd logout cd.argoproj.io } return command } + +// revokeServerToken makes a call to the server logout endpoint to revoke the token server side +func revokeServerToken(scheme, hostName, token string, insecure bool) (res *http.Response, err error) { + if token == "" { + return nil, errors.New("error getting token from local context file") + } + logoutURL := fmt.Sprintf("%s://%s%s", scheme, hostName, common.LogoutEndpoint) + ctx, cancel := context.WithTimeout(context.Background(), time.Second*30) + defer cancel() + req, err := http.NewRequestWithContext(ctx, http.MethodPost, logoutURL, http.NoBody) + if err != nil { + return nil, err + } + cookie := &http.Cookie{ + Name: common.AuthCookieName, + Value: token, + } + req.AddCookie(cookie) + + client := &http.Client{Timeout: common.TokenRevocationClientTimeout} + + if insecure { + tr := http.DefaultTransport.(*http.Transport).Clone() + tr.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} + client.Transport = tr + } + return client.Do(req) +} diff --git a/cmd/argocd/commands/logout_test.go b/cmd/argocd/commands/logout_test.go index 3ff28b2455..0f717c3924 100644 --- a/cmd/argocd/commands/logout_test.go +++ b/cmd/argocd/commands/logout_test.go @@ -1,12 +1,29 @@ package commands import ( + "bytes" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "fmt" + "math/big" + "net" + "net/http" + "net/http/httptest" "os" + "path/filepath" "testing" + "time" + "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "google.golang.org/grpc" + grpccreds "google.golang.org/grpc/credentials" + "github.com/argoproj/argo-cd/v3/common" argocdclient "github.com/argoproj/argo-cd/v3/pkg/apiclient" "github.com/argoproj/argo-cd/v3/util/localconfig" ) @@ -36,3 +53,218 @@ func TestLogout(t *testing.T) { assert.Contains(t, localConfig.Contexts, localconfig.ContextRef{Name: "argocd2.example.com:443", Server: "argocd2.example.com:443", User: "argocd2.example.com:443"}) assert.Contains(t, localConfig.Contexts, localconfig.ContextRef{Name: "localhost:8080", Server: "localhost:8080", User: "localhost:8080"}) } + +func TestRevokeServerToken_EmptyToken(t *testing.T) { + res, err := revokeServerToken("http", "localhost:8080", "", false) + require.EqualError(t, err, "error getting token from local context file") + assert.Nil(t, res) +} + +func TestRevokeServerToken_SuccessfulRequest(t *testing.T) { + var receivedCookie string + var receivedMethod string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + receivedMethod = r.Method + cookie, err := r.Cookie(common.AuthCookieName) + if err == nil { + receivedCookie = cookie.Value + } + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + // Strip the "http://" prefix to get the hostName + hostName := server.Listener.Addr().String() + + res, err := revokeServerToken("http", hostName, "test-token", false) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, res.StatusCode) + assert.Equal(t, http.MethodPost, receivedMethod) + assert.Equal(t, "test-token", receivedCookie) +} + +func TestRevokeServerToken_ServerReturnsBadRequest(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusBadRequest) + })) + defer server.Close() + + hostName := server.Listener.Addr().String() + + res, err := revokeServerToken("http", hostName, "test-token", false) + require.NoError(t, err) + assert.Equal(t, http.StatusBadRequest, res.StatusCode) +} + +func TestRevokeServerToken_InvalidURL(t *testing.T) { + // A hostname containing a control character produces an invalid URL, + // causing http.NewRequestWithContext to fail. + res, err := revokeServerToken("http", "invalid\x00host", "test-token", false) + require.Error(t, err) + assert.Nil(t, res) + assert.Contains(t, err.Error(), "invalid control character in URL") +} + +func TestRevokeServerToken_InsecureTLS(t *testing.T) { + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + hostName := server.Listener.Addr().String() + + // Without insecure, the self-signed cert should cause a failure + res, err := revokeServerToken("https", hostName, "test-token", false) + require.Error(t, err) + assert.Nil(t, res) + + // With insecure=true, should succeed despite self-signed cert + res, err = revokeServerToken("https", hostName, "test-token", true) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, res.StatusCode) +} + +// createTestLocalConfig creates a temporary local config file with a single context +// pointing to the given address and returns the config file path. +func createTestLocalConfig(t *testing.T, addr string) string { + t.Helper() + configFile := filepath.Join(t.TempDir(), "config") + cfg := localconfig.LocalConfig{ + CurrentContext: addr, + Contexts: []localconfig.ContextRef{{Name: addr, Server: addr, User: addr}}, + Servers: []localconfig.Server{{Server: addr}}, + Users: []localconfig.User{{Name: addr, AuthToken: "test-token"}}, + } + err := localconfig.WriteLocalConfig(cfg, configFile) + require.NoError(t, err) + return configFile +} + +// generateSelfSignedCert creates a self-signed TLS certificate for 127.0.0.1. +func generateSelfSignedCert(t *testing.T) tls.Certificate { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + + template := &x509.Certificate{ + SerialNumber: big.NewInt(1), + NotBefore: time.Now(), + NotAfter: time.Now().Add(time.Hour), + IPAddresses: []net.IP{net.ParseIP("127.0.0.1")}, + } + + certDER, err := x509.CreateCertificate(rand.Reader, template, template, &key.PublicKey, key) + require.NoError(t, err) + + return tls.Certificate{ + Certificate: [][]byte{certDER}, + PrivateKey: key, + } +} + +// TestLogout_TLSCheckFails_AutoSetsPlainText verifies that when grpc_util.TestTLS +// returns an error (e.g., nothing listening), the logout command automatically +// switches to plain text and still completes the logout successfully. +func TestLogout_TLSCheckFails_AutoSetsPlainText(t *testing.T) { + // Listen on a random port, then close it immediately so nothing is listening. + // This causes grpc_util.TestTLS to fail, triggering the PlainText auto-set path. + lc := net.ListenConfig{} + lis, err := lc.Listen(t.Context(), "tcp", "127.0.0.1:0") + require.NoError(t, err, "Unable to start server") + addr := lis.Addr().String() + lis.Close() + + configFile := createTestLocalConfig(t, addr) + + // Verify token exists before logout + localCfg, err := localconfig.ReadLocalConfig(configFile) + require.NoError(t, err) + assert.Equal(t, "test-token", localCfg.GetToken(addr)) + + command := NewLogoutCommand(&argocdclient.ClientOptions{ConfigPath: configFile}) + command.Run(nil, []string{addr}) + + // Verify token was removed despite TLS check failure + localCfg, err = localconfig.ReadLocalConfig(configFile) + require.NoError(t, err) + assert.Empty(t, localCfg.GetToken(addr)) +} + +// TestLogout_InsecureTLS_SetsInsecureFlag verifies that when the server has an +// insecure (self-signed) TLS certificate, the logout command detects it via +// grpc_util.TestTLS (which sets InsecureErr), prompts the user, and sets the +// Insecure flag to true before proceeding with logout. +func TestLogout_InsecureTLS_SetsInsecureFlag(t *testing.T) { + // Start a gRPC server with TLS credentials using a self-signed certificate. + // grpc_util.TestTLS will: + // 1. Connect with InsecureSkipVerify=true → succeeds (TLS=true) + // 2. Connect with InsecureSkipVerify=false → fails (InsecureErr set) + cert := generateSelfSignedCert(t) + + lc := net.ListenConfig{} + lis, err := lc.Listen(t.Context(), "tcp", "127.0.0.1:0") + require.NoError(t, err, "Unable to start server") + + serverCreds := grpccreds.NewServerTLSFromCert(&cert) + grpcServer := grpc.NewServer(grpc.Creds(serverCreds)) + go func() { + err := grpcServer.Serve(lis) + require.NoError(t, err, "Unable to start the grpc server") + }() + defer grpcServer.Stop() + + addr := lis.Addr().String() + + // Mock os.Stdin to provide "y\n" for cli.AskToProceed (insecure cert warning). + oldStdin := os.Stdin + r, w, err := os.Pipe() + require.NoError(t, err) + _, err = w.WriteString("y\n") + require.NoError(t, err) + w.Close() + os.Stdin = r + defer func() { os.Stdin = oldStdin }() + + configFile := createTestLocalConfig(t, addr) + + // Verify token exists before logout + localCfg, err := localconfig.ReadLocalConfig(configFile) + require.NoError(t, err) + assert.Equal(t, "test-token", localCfg.GetToken(addr)) + + command := NewLogoutCommand(&argocdclient.ClientOptions{ConfigPath: configFile}) + command.Run(nil, []string{addr}) + + // Verify token was removed after accepting the insecure cert warning + localCfg, err = localconfig.ReadLocalConfig(configFile) + require.NoError(t, err) + assert.Empty(t, localCfg.GetToken(addr)) +} + +// TestLogout_ConfigFileNotFound verifies that when the local config file does not +// exist, the logout command reports "Nothing to logout from" via log.Fatalf. +func TestLogout_ConfigFileNotFound(t *testing.T) { + // Point to a config path that does not exist. + configFile := filepath.Join(t.TempDir(), "nonexistent", "config") + + // Override logrus ExitFunc so log.Fatalf panics instead of calling os.Exit, + // allowing us to verify the fatal message in the test. + origExitFunc := logrus.StandardLogger().ExitFunc + logrus.StandardLogger().ExitFunc = func(code int) { + panic(fmt.Sprintf("os.Exit(%d)", code)) + } + defer func() { logrus.StandardLogger().ExitFunc = origExitFunc }() + + // Capture log output to verify the error message. + var buf bytes.Buffer + origOutput := logrus.StandardLogger().Out + logrus.SetOutput(&buf) + defer logrus.SetOutput(origOutput) + + assert.Panics(t, func() { + command := NewLogoutCommand(&argocdclient.ClientOptions{ConfigPath: configFile}) + command.Run(nil, []string{"some-context"}) + }) + + assert.Contains(t, buf.String(), "Nothing to logout from") +} diff --git a/common/common.go b/common/common.go index 6b07cc8a30..350b97c302 100644 --- a/common/common.go +++ b/common/common.go @@ -372,6 +372,12 @@ const ( BearerTokenTimeout = 30 * time.Second ) +// TokenRevocationTimeout is the maximum time allowed for a server-side token revocation call during logout. +const TokenRevocationTimeout = 10 * time.Second + +// TokenRevocationClientTimeout is the maximum time the CLI waits for the server to complete token revocation. +const TokenRevocationClientTimeout = 15 * time.Second + const ( DefaultGitRetryMaxDuration time.Duration = time.Second * 5 // 5s DefaultGitRetryDuration time.Duration = time.Millisecond * 250 // 0.25s diff --git a/docs/operator-manual/user-management/index.md b/docs/operator-manual/user-management/index.md index 3cd50c15a0..27426eb5cd 100644 --- a/docs/operator-manual/user-management/index.md +++ b/docs/operator-manual/user-management/index.md @@ -429,6 +429,41 @@ You are not required to specify a logoutRedirectURL as this is automatically gen > [!NOTE] > The post logout redirect URI may need to be whitelisted against your OIDC provider's client settings for ArgoCD. +### Token Revocation and Session Management + +Argo CD implements server-side token revocation to enhance security when users log out. This is particularly important for SSO configurations using Dex or other OIDC providers. + +#### How Token Revocation Works + +When a user logs out (either via the UI or CLI using `argocd logout`), Argo CD: + +1. **Invalidates the token on the server**: The token is added to a revocation list stored in Redis +2. **Removes the token locally**: The token is removed from the local configuration +3. **Redirects to OIDC provider** (if configured): The user is redirected to the OIDC provider's logout URL to terminate the SSO session + +Revoked tokens cannot be used for API calls, even if they haven't expired yet. This prevents: +- Unauthorized access after logout +- Token reuse if a token is compromised +- Security gaps with Dex SSO where tokens are not automatically invalidated + +#### Graceful Degradation + +The `argocd logout` command will gracefully handle scenarios where the server is unreachable: + +```bash +$ argocd logout my-argocd-server +WARN[0000] Failed to invalidate token on server: connection refused. Proceeding with local logout. +Logged out from 'my-argocd-server' +``` + +This allows users to logout locally even if the server is down, though the token will not be revoked server-side until it expires naturally. + +#### Security Best Practices + +1.**Use short-lived tokens**: Configure reasonable token expiration times in the OIDC provider to limit the window of exposure +2.**Enable logout URLs**: Configure `logoutURL` in `oidc.config` for your OIDC provider to ensure SSO sessions are also terminated +3.**Monitor token usage**: Use Argo CD's audit logging to track token creation and revocation events + ### Configuring a custom root CA certificate for communicating with the OIDC provider If your OIDC provider is setup with a certificate which is not signed by one of the well known certificate authorities diff --git a/go.mod b/go.mod index 94c993d6e7..579bbea16d 100644 --- a/go.mod +++ b/go.mod @@ -71,6 +71,7 @@ require ( github.com/mattn/go-zglob v0.0.6 github.com/microsoft/azure-devops-go-api/azuredevops/v7 v7.1.1-0.20241014080628-3045bdf43455 github.com/minio/blake2b-simd v0.0.0-20160723061019-3f5f724cb5b1 + github.com/oauth2-proxy/mockoidc v0.0.0-20240214162133-caebfff84d25 github.com/olekukonko/tablewriter v1.1.3 github.com/opencontainers/go-digest v1.0.0 github.com/opencontainers/image-spec v1.1.1 @@ -187,6 +188,7 @@ require ( github.com/go-fed/httpsig v1.1.0 // indirect github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 // indirect github.com/go-git/go-billy/v5 v5.6.2 // indirect + github.com/go-jose/go-jose/v3 v3.0.1 // indirect github.com/go-logr/stdr v1.2.2 // indirect github.com/go-openapi/analysis v0.24.3 // indirect github.com/go-openapi/errors v0.22.7 // indirect diff --git a/go.sum b/go.sum index 7855ad4f0d..2a1216bbdf 100644 --- a/go.sum +++ b/go.sum @@ -311,6 +311,8 @@ github.com/go-git/go-git/v5 v5.14.0/go.mod h1:Z5Xhoia5PcWA3NF8vRLURn9E5FRhSl7dGj github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= +github.com/go-jose/go-jose/v3 v3.0.1 h1:pWmKFVtt+Jl0vBZTIpz/eAKwsm6LkIxDVVbFHKkchhA= +github.com/go-jose/go-jose/v3 v3.0.1/go.mod h1:RNkWWRld676jZEYoV3+XK8L2ZnNSvIsxFMht0mSX+u8= github.com/go-jose/go-jose/v4 v4.1.3 h1:CVLmWDhDVRa6Mi/IgCgaopNosCaHz7zrMeF9MlZRkrs= github.com/go-jose/go-jose/v4 v4.1.3/go.mod h1:x4oUasVrzR7071A4TnHLGSPpNOm2a21K9Kf04k1rs08= github.com/go-kit/kit v0.8.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as= @@ -722,6 +724,8 @@ github.com/nlopes/slack v0.5.0/go.mod h1:jVI4BBK3lSktibKahxBF74txcK2vyvkza1z/+rR github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A= github.com/nxadm/tail v1.4.8 h1:nPr65rt6Y5JFSKQO7qToXr7pePgD6Gwiw05lkbyAQTE= github.com/nxadm/tail v1.4.8/go.mod h1:+ncqLTQzXmGhMZNUePPaPqPvBxHAIsmXswZKocGu+AU= +github.com/oauth2-proxy/mockoidc v0.0.0-20240214162133-caebfff84d25 h1:9bCMuD3TcnjeqjPT2gSlha4asp8NvgcFRYExCaikCxk= +github.com/oauth2-proxy/mockoidc v0.0.0-20240214162133-caebfff84d25/go.mod h1:eDjgYHYDJbPLBLsyZ6qRaugP0mX8vePOhZ5id1fdzJw= github.com/oklog/ulid/v2 v2.1.1 h1:suPZ4ARWLOJLegGFiZZ1dFAkqzhMjL3J1TzI+5wHz8s= github.com/oklog/ulid/v2 v2.1.1/go.mod h1:rcEKHmBBKfef9DhnvX7y1HZBYxjXb0cP5ExxNsTT1QQ= github.com/olekukonko/cat v0.0.0-20250911104152-50322a0618f6 h1:zrbMGy9YXpIeTnGj4EljqMiZsIcE09mmF8XsD5AYOJc= @@ -1002,6 +1006,7 @@ golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACk golang.org/x/crypto v0.0.0-20190422183909-d864b10871cd/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20190605123033-f99c8df09eb5/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= +golang.org/x/crypto v0.0.0-20190911031432-227b76d455e7/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20201216223049-8b5274cf687f/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I= diff --git a/server/logout/logout.go b/server/logout/logout.go index b1e7628972..33aa733c7a 100644 --- a/server/logout/logout.go +++ b/server/logout/logout.go @@ -111,9 +111,22 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { issuer := jwtutil.StringField(mapClaims, "iss") id := jwtutil.StringField(mapClaims, "jti") + // Workaround for Dex token, because does not have jti. + if id == "" { + id = jwtutil.StringField(mapClaims, "at_hash") + } + if exp, err := jwtutil.ExpirationTime(mapClaims); err == nil && id != "" { - if err := h.revokeToken(context.Background(), id, time.Until(exp)); err != nil { - log.Warnf("failed to invalidate token '%s': %v", id, err) + ttl := time.Until(exp) + if ttl <= 0 { + // Token already expired; no need to persist revocation + log.Infof("token '%s' already expired, skipping revocation", id) + } else { + revokeCtx, cancel := context.WithTimeout(r.Context(), common.TokenRevocationTimeout) + defer cancel() + if err := h.revokeToken(revokeCtx, id, ttl); err != nil { + log.Warnf("failed to invalidate token '%s': %v", id, err) + } } } diff --git a/server/logout/logout_test.go b/server/logout/logout_test.go index a500668a51..40bcd8c611 100644 --- a/server/logout/logout_test.go +++ b/server/logout/logout_test.go @@ -8,6 +8,7 @@ import ( "regexp" "strconv" "testing" + "time" "github.com/argoproj/argo-cd/v3/common" "github.com/argoproj/argo-cd/v3/test" @@ -15,7 +16,6 @@ import ( "github.com/argoproj/argo-cd/v3/util/settings" "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" @@ -32,9 +32,11 @@ var ( baseLogoutURLwithRedirectURL = "http://localhost:4000/logout?post_logout_redirect_uri={{logoutRedirectURL}}" baseLogoutURLwithTokenAndRedirectURL = "http://localhost:4000/logout?id_token_hint={{token}}&post_logout_redirect_uri={{logoutRedirectURL}}" invalidToken = "sample-token" + dexToken = "eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsImtpZCI6Ijc5YTFlNTgzOWM2ZTRjMTZlOTIwYzM1YTU0MmMwNmZhIn0.eyJzdWIiOiIwMHVqNnM1NDVyNU5peVNLcjVkNSIsIm5hbWUiOiJqZCByIiwiZW1haWwiOiJqYWlkZWVwMTdydWx6QGdtYWlsLmNvbSIsInZlciI6MSwiaXNzIjoiaHR0cHM6Ly9kZXYtNTY5NTA5OC5va3RhLmNvbSIsImF1ZCI6IjBvYWowM2FmSEtqN3laWXJwNWQ1IiwiaWF0IjoxNjA1NTcyMzU5LCJleHAiOjE2MDU1NzU5NTksImFtciI6WyJwd2QiXSwiaWRwIjoiMDBvaWdoZmZ2SlFMNjNaOGg1ZDUiLCJwcmVmZXJyZWRfdXNlcm5hbWUiOiJqYWlkZWVwMTdydWx6QGdtYWlsLmNvbSIsImF1dGhfdGltZSI6MTYwNTU3MjM1NywiYXRfaGFzaCI6ImplUTBGaXZqT2c0YjZNSldEMjE5bGcifQ.Xt_5G-4dNZef1egOYmvruszudlAvUXVQzqrI4YwkWJeZ0zZDk4lyhPUVuxVGjB3pCCUCUMloTL6xC7IVFNj53Eb7WNH_hxsFqemJ80HZYbUpo2G9fMjkPmFTaeFVMC4p3qxIaBAT9_uJbTRSyRGYLV-95KDpU-GNDFXlbFq-2bVvhppiYmKszyHbREZkB87Pi7K3Bk0NxAlDOJ7O5lhwjpwuOJ1WGCJptUetePm5MnpVT2ZCyjvntlzwHlIhMSKNlFZuFS_JMca5Ww0fQSBUlarQU9MMyZKBw-QuD5sJw3xjwQpxOG-T9mJz7F8VA5znLi_LJNutHVgcpt3T_TW_0NbgqsHe8Lw" oidcToken = "eyJraWQiOiJYQi1MM3ZFdHhYWXJLcmRSQnVEV0NwdnZsSnk3SEJVb2d5N253M1U1Z1ZZIiwiYWxnIjoiUlMyNTYifQ.eyJzdWIiOiIwMHVqNnM1NDVyNU5peVNLcjVkNSIsIm5hbWUiOiJqZCByIiwiZW1haWwiOiJqYWlkZWVwMTdydWx6QGdtYWlsLmNvbSIsInZlciI6MSwiaXNzIjoiaHR0cHM6Ly9kZXYtNTY5NTA5OC5va3RhLmNvbSIsImF1ZCI6IjBvYWowM2FmSEtqN3laWXJwNWQ1IiwiaWF0IjoxNjA1NTcyMzU5LCJleHAiOjE2MDU1NzU5NTksImp0aSI6IklELl9ORDJxVG5iREFtc3hIZUt2U2ZHeVBqTXRicXFEQXdkdlRQTDZCTnpfR3ciLCJhbXIiOlsicHdkIl0sImlkcCI6IjAwb2lnaGZmdkpRTDYzWjhoNWQ1IiwicHJlZmVycmVkX3VzZXJuYW1lIjoiamFpZGVlcDE3cnVsekBnbWFpbC5jb20iLCJhdXRoX3RpbWUiOjE2MDU1NzIzNTcsImF0X2hhc2giOiJqZVEwRml2ak9nNGI2TUpXRDIxOWxnIn0.GHkqwXgW-lrAhJdypW7SVjW0YdNLFQiRL8iwgT6DHJxP9Nb0OtkH2NKcBYAA5N6bTPLRQUHgYwWcgm5zSXmvqa7ciIgPF3tiQI8UmJA9VFRRDR-x9ExX15nskCbXfiQ67MriLslUrQUyzSCfUrSjXKwnDxbKGQncrtmRsh5asfCzJFb9excn311W9HKbT3KA0Ot7eOMnVS6V7SGfXxnKs6szcXIEMa_FhB4zDAVLr-dnxvSG_uuWcHrAkLTUVhHbdQQXF7hXIEfyr5lkMJN-drjdz-bn40GaYulEmUvO1bjcL9toCVQ3Ismypyr0b8phj4w3uRsLDZQxTxK7jAXlyQ" nonOidcToken = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpYXQiOjE2MDU1NzQyMTIsImlzcyI6ImFyZ29jZCIsIm5iZiI6MTYwNTU3NDIxMiwic3ViIjoiYWRtaW4ifQ.zDJ4piwWnwsHON-oPusHMXWINlnrRDTQykYogT7afeE" expectedNonOIDCLogoutURL = "http://localhost:4000" + expectedDexLogoutURL = "http://localhost:4000" expectedNonOIDCLogoutURLOnSecondHost = "http://argocd.my-corp.tld" expectedOIDCLogoutURL = "https://dev-5695098.okta.com/oauth2/v1/logout?id_token_hint=" + oidcToken + "&post_logout_redirect_uri=" + baseURL expectedOIDCLogoutURLWithRootPath = "https://dev-5695098.okta.com/oauth2/v1/logout?id_token_hint=" + oidcToken + "&post_logout_redirect_uri=" + baseURL + "/" + rootPath @@ -80,12 +82,46 @@ func TestConstructLogoutURL(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { constructedLogoutURL := constructLogoutURL(tt.logoutURL, tt.token, tt.logoutRedirectURL) - assert.Equal(t, tt.expectedLogoutURL, constructedLogoutURL) + require.Equal(t, tt.expectedLogoutURL, constructedLogoutURL) }) } } func TestHandlerConstructLogoutURL(t *testing.T) { + kubeClientWithDexConfig := fake.NewClientset( + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: common.ArgoCDConfigMapName, + Namespace: "default", + Labels: map[string]string{ + "app.kubernetes.io/part-of": "argocd", + }, + }, + Data: map[string]string{ + "dex.config": "connectors: \n" + + "- type: dev \n" + + "name: Dev \n" + + "config: \n" + + "issuer: https://dev-5695098.okta.com \n" + + "clientID: aabbccddeeff00112233 \n" + + "clientSecret: aabbccddeeff00112233", + "url": "http://localhost:4000", + }, + }, + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: common.ArgoCDSecretName, + Namespace: "default", + Labels: map[string]string{ + "app.kubernetes.io/part-of": "argocd", + }, + }, + Data: map[string][]byte{ + "admin.password": nil, + "server.secretkey": nil, + }, + }, + ) kubeClientWithOIDCConfig := fake.NewClientset( &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ @@ -237,14 +273,24 @@ func TestHandlerConstructLogoutURL(t *testing.T) { }, ) + settingsManagerWithDexConfig := settings.NewSettingsManager(t.Context(), kubeClientWithDexConfig, "default") settingsManagerWithOIDCConfig := settings.NewSettingsManager(t.Context(), kubeClientWithOIDCConfig, "default") settingsManagerWithoutOIDCConfig := settings.NewSettingsManager(t.Context(), kubeClientWithoutOIDCConfig, "default") settingsManagerWithOIDCConfigButNoLogoutURL := settings.NewSettingsManager(t.Context(), kubeClientWithOIDCConfigButNoLogoutURL, "default") settingsManagerWithoutOIDCAndMultipleURLs := settings.NewSettingsManager(t.Context(), kubeClientWithoutOIDCAndMultipleURLs, "default") settingsManagerWithOIDCConfigButNoURL := settings.NewSettingsManager(t.Context(), kubeClientWithOIDCConfigButNoURL, "default") - sessionManager := session.NewSessionManager(settingsManagerWithOIDCConfig, test.NewFakeProjLister(), "", nil, session.NewUserStateStorage(nil)) + redisClient, closer := test.NewInMemoryRedis() + defer closer() + sessionManager := session.NewSessionManager(settingsManagerWithOIDCConfig, test.NewFakeProjLister(), "", nil, session.NewUserStateStorage(redisClient)) + dexHandler := NewHandler(settingsManagerWithDexConfig, sessionManager, rootPath, baseHRef) + dexHandler.verifyToken = func(_ context.Context, tokenString string) (jwt.Claims, string, error) { + if !validJWTPattern.MatchString(tokenString) { + return nil, "", errors.New("invalid jwt") + } + return &jwt.RegisteredClaims{Issuer: "dev"}, "", nil + } oidcHandler := NewHandler(settingsManagerWithOIDCConfig, sessionManager, rootPath, baseHRef) oidcHandler.verifyToken = func(_ context.Context, tokenString string) (jwt.Claims, string, error) { if !validJWTPattern.MatchString(tokenString) { @@ -281,6 +327,9 @@ func TestHandlerConstructLogoutURL(t *testing.T) { } return &jwt.RegisteredClaims{Issuer: "okta"}, "", nil } + + dexTokenHeader := make(map[string][]string) + dexTokenHeader["Cookie"] = []string{"argocd.token=" + dexToken} oidcTokenHeader := make(map[string][]string) oidcTokenHeader["Cookie"] = []string{"argocd.token=" + oidcToken} nonOidcTokenHeader := make(map[string][]string) @@ -291,6 +340,9 @@ func TestHandlerConstructLogoutURL(t *testing.T) { emptyHeader["Cookie"] = []string{"argocd.token="} ctx := t.Context() + dexRequest, err := http.NewRequestWithContext(ctx, http.MethodGet, "http://localhost:4000/api/logout", http.NoBody) + require.NoError(t, err) + dexRequest.Header = dexTokenHeader oidcRequest, err := http.NewRequestWithContext(ctx, http.MethodGet, "http://localhost:4000/api/logout", http.NoBody) require.NoError(t, err) oidcRequest.Header = oidcTokenHeader @@ -298,9 +350,9 @@ func TestHandlerConstructLogoutURL(t *testing.T) { require.NoError(t, err) nonoidcRequest.Header = nonOidcTokenHeader nonoidcRequestOnSecondHost, err := http.NewRequestWithContext(ctx, http.MethodGet, "http://argocd.my-corp.tld/api/logout", http.NoBody) - assert.NoError(t, err) + require.NoError(t, err) nonoidcRequestOnSecondHost.Header = nonOidcTokenHeader - assert.NoError(t, err) + require.NoError(t, err) requestWithInvalidToken, err := http.NewRequestWithContext(ctx, http.MethodGet, "http://localhost:4000/api/logout", http.NoBody) require.NoError(t, err) requestWithInvalidToken.Header = invalidHeader @@ -320,6 +372,14 @@ func TestHandlerConstructLogoutURL(t *testing.T) { expectedLogoutURL string wantErr bool }{ + { + name: "Case: Dex logout request with valid token", + handler: dexHandler, + request: dexRequest, + responseRecorder: httptest.NewRecorder(), + expectedLogoutURL: expectedDexLogoutURL, + wantErr: false, + }, { name: "Case: OIDC logout request with valid token", handler: oidcHandler, @@ -405,9 +465,235 @@ func TestHandlerConstructLogoutURL(t *testing.T) { if tt.wantErr { t.Errorf("expected error but did not get one") } else { - assert.Equal(t, tt.expectedLogoutURL, tt.responseRecorder.Result().Header["Location"][0]) + require.Equal(t, tt.expectedLogoutURL, tt.responseRecorder.Result().Header["Location"][0]) } } }) } } + +func TestHandlerRevokeToken(t *testing.T) { + kubeClient := fake.NewClientset( + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: common.ArgoCDConfigMapName, + Namespace: "default", + Labels: map[string]string{ + "app.kubernetes.io/part-of": "argocd", + }, + }, + Data: map[string]string{ + "url": "http://localhost:4000", + }, + }, + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: common.ArgoCDSecretName, + Namespace: "default", + Labels: map[string]string{ + "app.kubernetes.io/part-of": "argocd", + }, + }, + Data: map[string][]byte{ + "admin.password": nil, + "server.secretkey": nil, + }, + }, + ) + + settingsMgr := settings.NewSettingsManager(t.Context(), kubeClient, "default") + redisClient, closer := test.NewInMemoryRedis() + defer closer() + sessionMgr := session.NewSessionManager(settingsMgr, test.NewFakeProjLister(), "", nil, session.NewUserStateStorage(redisClient)) + + t.Run("Token with jti calls revokeToken with jti value", func(t *testing.T) { + var revokedID string + var revokeCalled bool + + handler := NewHandler(settingsMgr, sessionMgr, "", baseHRef) + handler.verifyToken = func(_ context.Context, _ string) (jwt.Claims, string, error) { + return jwt.MapClaims{ + "iss": session.SessionManagerClaimsIssuer, + "jti": "token-id-123", + "exp": float64(time.Now().Add(time.Hour).Unix()), + }, "", nil + } + handler.revokeToken = func(_ context.Context, id string, _ time.Duration) error { + revokeCalled = true + revokedID = id + return nil + } + + req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, "http://localhost:4000/api/logout", http.NoBody) + require.NoError(t, err) + req.Header.Set("Cookie", "argocd.token="+nonOidcToken) + + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + require.True(t, revokeCalled, "revokeToken should have been called") + require.Equal(t, "token-id-123", revokedID) + require.Equal(t, http.StatusSeeOther, rec.Code) + }) + + t.Run("Dex token without jti uses at_hash as fallback", func(t *testing.T) { + var revokedID string + var revokeCalled bool + + handler := NewHandler(settingsMgr, sessionMgr, "", baseHRef) + handler.verifyToken = func(_ context.Context, _ string) (jwt.Claims, string, error) { + return jwt.MapClaims{ + "iss": "dex", + "at_hash": "dex-at-hash-456", + "exp": float64(time.Now().Add(time.Hour).Unix()), + }, "", nil + } + handler.revokeToken = func(_ context.Context, id string, _ time.Duration) error { + revokeCalled = true + revokedID = id + return nil + } + + req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, "http://localhost:4000/api/logout", http.NoBody) + require.NoError(t, err) + req.Header.Set("Cookie", "argocd.token="+nonOidcToken) + + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + require.True(t, revokeCalled, "revokeToken should have been called with at_hash fallback") + require.Equal(t, "dex-at-hash-456", revokedID) + }) + + t.Run("Token with both jti and at_hash uses jti", func(t *testing.T) { + var revokedID string + + handler := NewHandler(settingsMgr, sessionMgr, "", baseHRef) + handler.verifyToken = func(_ context.Context, _ string) (jwt.Claims, string, error) { + return jwt.MapClaims{ + "iss": session.SessionManagerClaimsIssuer, + "jti": "primary-jti", + "at_hash": "secondary-at-hash", + "exp": float64(time.Now().Add(time.Hour).Unix()), + }, "", nil + } + handler.revokeToken = func(_ context.Context, id string, _ time.Duration) error { + revokedID = id + return nil + } + + req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, "http://localhost:4000/api/logout", http.NoBody) + require.NoError(t, err) + req.Header.Set("Cookie", "argocd.token="+nonOidcToken) + + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + require.Equal(t, "primary-jti", revokedID, "should use jti when both jti and at_hash are present") + }) + + t.Run("Token with neither jti nor at_hash does not call revokeToken", func(t *testing.T) { + revokeCalled := false + + handler := NewHandler(settingsMgr, sessionMgr, "", baseHRef) + handler.verifyToken = func(_ context.Context, _ string) (jwt.Claims, string, error) { + return jwt.MapClaims{ + "iss": session.SessionManagerClaimsIssuer, + "exp": float64(time.Now().Add(time.Hour).Unix()), + }, "", nil + } + handler.revokeToken = func(_ context.Context, _ string, _ time.Duration) error { + revokeCalled = true + return nil + } + + req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, "http://localhost:4000/api/logout", http.NoBody) + require.NoError(t, err) + req.Header.Set("Cookie", "argocd.token="+nonOidcToken) + + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + require.False(t, revokeCalled, "revokeToken should not be called when id is empty") + require.Equal(t, http.StatusSeeOther, rec.Code) + }) + + t.Run("Expired token skips revocation", func(t *testing.T) { + revokeCalled := false + + handler := NewHandler(settingsMgr, sessionMgr, "", baseHRef) + handler.verifyToken = func(_ context.Context, _ string) (jwt.Claims, string, error) { + return jwt.MapClaims{ + "iss": session.SessionManagerClaimsIssuer, + "jti": "expired-token-id", + "exp": float64(time.Now().Add(-time.Hour).Unix()), // already expired + }, "", nil + } + handler.revokeToken = func(_ context.Context, _ string, _ time.Duration) error { + revokeCalled = true + return nil + } + + req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, "http://localhost:4000/api/logout", http.NoBody) + require.NoError(t, err) + req.Header.Set("Cookie", "argocd.token="+nonOidcToken) + + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + require.False(t, revokeCalled, "revokeToken should not be called for expired tokens") + require.Equal(t, http.StatusSeeOther, rec.Code) + }) + + t.Run("Revocation timeout does not block logout", func(t *testing.T) { + handler := NewHandler(settingsMgr, sessionMgr, "", baseHRef) + handler.verifyToken = func(_ context.Context, _ string) (jwt.Claims, string, error) { + return jwt.MapClaims{ + "iss": session.SessionManagerClaimsIssuer, + "jti": "timeout-token-id", + "exp": float64(time.Now().Add(time.Hour).Unix()), + }, "", nil + } + handler.revokeToken = func(ctx context.Context, _ string, _ time.Duration) error { + // Simulate a slow backend that exceeds the context timeout + select { + case <-ctx.Done(): + return ctx.Err() + case <-time.After(10 * time.Second): + return nil + } + } + + req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, "http://localhost:4000/api/logout", http.NoBody) + require.NoError(t, err) + req.Header.Set("Cookie", "argocd.token="+nonOidcToken) + + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + require.Equal(t, http.StatusSeeOther, rec.Code, "should redirect even when revocation times out") + }) + + t.Run("revokeToken error does not prevent redirect", func(t *testing.T) { + handler := NewHandler(settingsMgr, sessionMgr, "", baseHRef) + handler.verifyToken = func(_ context.Context, _ string) (jwt.Claims, string, error) { + return jwt.MapClaims{ + "iss": session.SessionManagerClaimsIssuer, + "jti": "some-token-id", + "exp": float64(time.Now().Add(time.Hour).Unix()), + }, "", nil + } + handler.revokeToken = func(_ context.Context, _ string, _ time.Duration) error { + return errors.New("redis connection refused") + } + + req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, "http://localhost:4000/api/logout", http.NoBody) + require.NoError(t, err) + req.Header.Set("Cookie", "argocd.token="+nonOidcToken) + + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + require.Equal(t, http.StatusSeeOther, rec.Code, "should still redirect even when revokeToken fails") + }) +} diff --git a/test/e2e/fixture/fixture.go b/test/e2e/fixture/fixture.go index 500f3d81fc..ddb3e08aa3 100644 --- a/test/e2e/fixture/fixture.go +++ b/test/e2e/fixture/fixture.go @@ -1029,6 +1029,44 @@ func RunCliWithStdin(stdin string, isKubeConextOnlyCli bool, args ...string) (st return RunWithStdinWithRedactor(stdin, "", "../../dist/argocd", redactor, args...) } +// RunCliWithToken executes an Argo CD CLI command using a specific auth token +// instead of the global test token. This is useful for session/logout tests +// that need to verify behavior with multiple or revoked tokens. +func RunCliWithToken(authToken string, args ...string) (string, error) { + if plainText { + args = append(args, "--plaintext") + } + + args = append(args, "--server", apiServerAddress, "--auth-token", authToken, "--insecure") + + redactor := func(text string) string { + if authToken == "" { + return text + } + authTokenPattern := "--auth-token " + authToken + return strings.ReplaceAll(text, authTokenPattern, "--auth-token ******") + } + + return RunWithStdinWithRedactor("", "", "../../dist/argocd", redactor, args...) +} + +// RunCliWithConfigFile executes an Argo CD CLI command using a custom config file +// instead of the global test config. This is used by session/logout tests to +// isolate per-token state across login/logout cycles. +func RunCliWithConfigFile(configPath string, args ...string) (string, error) { + if plainText { + args = append(args, "--plaintext") + } + + args = append(args, "--server", apiServerAddress, "--config", configPath, "--insecure") + + redactor := func(text string) string { + return text + } + + return RunWithStdinWithRedactor("", "", "../../dist/argocd", redactor, args...) +} + // RunPluginCli executes an Argo CD CLI plugin with optional stdin input. func RunPluginCli(stdin string, args ...string) (string, error) { return RunWithStdin(stdin, "", "../../dist/argocd", args...) @@ -1334,3 +1372,21 @@ func GetToken() string { func IsPlainText() bool { return plainText } + +// SetParamInRBACConfigMap sets the parameter in argocd-rbac-cm config map +func SetParamInRBACConfigMap(key, value string) error { + return updateRBACConfigMap(func(cm *corev1.ConfigMap) error { + cm.Data[key] = value + return nil + }) +} + +// SetOIDCConfig sets the oidc.config in argocd-cm config map +func SetOIDCConfig(value string) error { + return updateSettingConfigMap(func(cm *corev1.ConfigMap) error { + cm.Data["oidc.config"] = value + cm.Data["url"] = "http://" + GetApiServerAddress() + delete(cm.Data, "dex.config") + return nil + }) +} diff --git a/test/e2e/fixture/session/actions.go b/test/e2e/fixture/session/actions.go new file mode 100644 index 0000000000..60ed82e101 --- /dev/null +++ b/test/e2e/fixture/session/actions.go @@ -0,0 +1,297 @@ +package session + +import ( + "context" + "crypto/rand" + "crypto/rsa" + "fmt" + "net" + "os" + "path/filepath" + "sync" + "time" + + "github.com/oauth2-proxy/mockoidc" + log "github.com/sirupsen/logrus" + "github.com/stretchr/testify/require" + + "github.com/argoproj/argo-cd/v3/test/e2e/fixture" + "github.com/argoproj/argo-cd/v3/util/localconfig" +) + +var ( + // mockServer is a package-level singleton mock OIDC server shared across all tests. + mockServer *mockoidc.MockOIDC + mockServerOnce sync.Once +) + +// Actions implements the "when" part of given/when/then for session/logout tests. +type Actions struct { + context *Context + + // configPaths maps token names to per-token CLI config file paths + configPaths map[string]string + + // tokenStore tracks named tokens read from CLI config files + tokenStore map[string]string + + // lastOutput holds the stdout from the most recent CLI action + lastOutput string + + // lastError holds the error from the most recent action + lastError error +} + +// getTokenStore initializes the token store +func (a *Actions) getTokenStore() map[string]string { + if a.tokenStore == nil { + a.tokenStore = make(map[string]string) + } + return a.tokenStore +} + +// getConfigPaths initializes the config paths map +func (a *Actions) getConfigPaths() map[string]string { + if a.configPaths == nil { + a.configPaths = make(map[string]string) + } + return a.configPaths +} + +// configPathFor returns (or creates) a per-token temp config file path. +func (a *Actions) configPathFor(tokenName string) string { + a.context.T().Helper() + paths := a.getConfigPaths() + if p, ok := paths[tokenName]; ok { + return p + } + p := filepath.Join(a.context.T().TempDir(), tokenName+"-config") + paths[tokenName] = p + return p +} + +func (a *Actions) getSharedMockOIDCServer() *mockoidc.MockOIDC { + mockServerOnce.Do(func() { + t := a.context.T() + // Create a fresh RSA Private Key for token signing + rsaKey, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + oidcPort := os.Getenv("ARGOCD_E2E_OIDC_PORT") + if oidcPort == "" { + oidcPort = "5556" + } + lc := net.ListenConfig{} + ctx, cancelFunc := context.WithTimeout(context.Background(), 60*time.Second) + defer cancelFunc() + ln, err := lc.Listen(ctx, "tcp", "localhost:"+oidcPort) + require.NoError(t, err) + mockServer, err = mockoidc.NewServer(rsaKey) + require.NoError(t, err) + err = mockServer.Start(ln, nil) + require.NoError(t, err) + t.Logf("OIDC server listening on %s", ln.Addr()) + t.Cleanup(func() { + err := mockServer.Shutdown() + require.NoError(t, err, "error shutting down mock oidc server") + }) + }) + return mockServer +} + +// WithDirectOIDC configures ArgoCD with oidc.config pointing directly to +// a shared mock OIDC server (bypassing Dex). Tests using this setup should +// use LoginWithSSO to mint OIDC tokens programmatically. This exercises the IDP +// token verification and revocation code path in SessionManager.VerifyToken. +func (a *Actions) WithDirectOIDC() *Actions { + a.context.T().Helper() + + m := a.getSharedMockOIDCServer() + + // Do NOT shut down the mock server in t.Cleanup. The API server's + // SessionManager caches the OIDC provider (mgr.prov) with this server's + // issuer URL. If we shut it down, subsequent tests would start a new + // server on a different port, causing issuer mismatch errors. + + // Configure oidc.config in argocd-cm pointing to the mock OIDC server + oidcConfig := fmt.Sprintf("name: Mock OIDC\nissuer: %s\nclientID: %s\nclientSecret: %s\n", + m.Issuer(), m.Config().ClientID, m.Config().ClientSecret) + + err := fixture.SetOIDCConfig(oidcConfig) + require.NoError(a.context.T(), err) + + // Grant all users admin access so OIDC-authenticated users can call APIs + err = fixture.SetParamInRBACConfigMap("policy.default", "role:admin") + require.NoError(a.context.T(), err) + + a.context.T().Logf("Direct OIDC Issuer: %s, ClientID: %s", m.Issuer(), m.Config().ClientID) + + fixture.RestartAPIServer(a.context.T()) + return a +} + +// Login creates a new session as admin using the argocd CLI login command. +// Each token name gets its own isolated config file so multiple sessions +// can coexist without interfering with each other. +func (a *Actions) Login(tokenName string) *Actions { + a.context.T().Helper() + + cfgPath := a.configPathFor(tokenName) + + output, err := fixture.RunCliWithConfigFile(cfgPath, + "login", + fixture.GetApiServerAddress(), + "--username", "admin", + "--password", fixture.AdminPassword, + "--skip-test-tls", + ) + require.NoError(a.context.T(), err, "CLI login failed: %s", output) + + // Read the token back from the config file + localCfg, err := localconfig.ReadLocalConfig(cfgPath) + require.NoError(a.context.T(), err) + require.NotNil(a.context.T(), localCfg) + + token := localCfg.GetToken(localCfg.CurrentContext) + require.NotEmpty(a.context.T(), token, "no token found in config file after login") + + a.getTokenStore()[tokenName] = token + return a +} + +// Logout revokes the named token using the argocd CLI logout command. +// This triggers the full CLI logout flow including server-side token revocation. +func (a *Actions) Logout(tokenName string) *Actions { + a.context.T().Helper() + + cfgPath := a.getConfigPaths()[tokenName] + require.NotEmpty(a.context.T(), cfgPath, "config path for %q not found; call Login first", tokenName) + + output, err := fixture.RunCliWithConfigFile(cfgPath, + "logout", + fixture.GetApiServerAddress(), + ) + require.NoError(a.context.T(), err, "CLI logout failed: %s", output) + + return a +} + +// LoginWithSSO mints an OIDC token directly from the shared mock OIDC server, +// bypassing the normal browser-based SSO flow. Each call creates a unique user +// identity based on tokenName so multiple SSO sessions produce different tokens. +// This exercises the IDP token verification and revocation code path in +// SessionManager.VerifyToken. +func (a *Actions) LoginWithSSO(tokenName string) *Actions { + a.context.T().Helper() + + require.NotNil(a.context.T(), mockServer, "LoginWithSSO requires WithDirectOIDC (mockServer is nil)") + + m := mockServer + + // Create a unique user for each token name + user := &mockoidc.MockUser{ + Subject: "sso-" + tokenName, + Email: tokenName + "@example.com", + EmailVerified: true, + PreferredUsername: tokenName, + Groups: []string{"admins"}, + } + + // Create a session on the mock OIDC server + session, err := m.SessionStore.NewSession( + "openid email profile groups", + "", // nonce + user, + "", // codeChallenge + "", // codeChallengeMethod + ) + require.NoError(a.context.T(), err) + + // Mint a signed ID token using the mock server's keypair + idToken, err := session.IDToken( + m.Config(), + m.Keypair, + m.Now(), + ) + require.NoError(a.context.T(), err) + + a.getTokenStore()[tokenName] = idToken + + // Write a local config file so CLI logout can find and revoke this token + serverAddr := fixture.GetApiServerAddress() + cfgPath := a.configPathFor(tokenName) + localCfg := localconfig.LocalConfig{ + CurrentContext: serverAddr, + Contexts: []localconfig.ContextRef{ + {Name: serverAddr, Server: serverAddr, User: serverAddr}, + }, + Servers: []localconfig.Server{ + {Server: serverAddr, PlainText: true, Insecure: true}, + }, + Users: []localconfig.User{ + {Name: serverAddr, AuthToken: idToken}, + }, + } + err = localconfig.WriteLocalConfig(localCfg, cfgPath) + require.NoError(a.context.T(), err) + + return a +} + +// runCli executes an argocd CLI command using the named token and records +// the output and error for assertion in Then(). +func (a *Actions) runCli(tokenName string, args ...string) { + a.context.T().Helper() + + token := a.getTokenStore()[tokenName] + require.NotEmpty(a.context.T(), token, "token %q not found; call Login first", tokenName) + + a.lastOutput, a.lastError = fixture.RunCliWithToken(token, args...) +} + +// GetUserInfo calls "argocd account get-user-info" using the named token. +func (a *Actions) GetUserInfo(tokenName string) *Actions { + a.context.T().Helper() + a.runCli(tokenName, "account", "get-user-info", "--grpc-web") + return a +} + +// ListProjects calls "argocd proj list" using the named token. +func (a *Actions) ListProjects(tokenName string) *Actions { + a.context.T().Helper() + a.runCli(tokenName, "proj", "list", "--grpc-web") + return a +} + +// ListApplications calls "argocd app list" using the named token. +func (a *Actions) ListApplications(tokenName string) *Actions { + a.context.T().Helper() + a.runCli(tokenName, "app", "list", "--grpc-web") + log.Debugf("[token: %s, output: %s, err: %s]", tokenName, a.lastOutput, a.lastError) + return a +} + +// ListRepositories calls "argocd repo list" using the named token. +func (a *Actions) ListRepositories(tokenName string) *Actions { + a.context.T().Helper() + a.runCli(tokenName, "repo", "list", "--grpc-web") + return a +} + +// ListAccounts calls "argocd account list" using the named token. +func (a *Actions) ListAccounts(tokenName string) *Actions { + a.context.T().Helper() + a.runCli(tokenName, "account", "list") + return a +} + +// Sleep pauses execution for the given duration. +func (a *Actions) Sleep(d time.Duration) *Actions { + time.Sleep(d) + return a +} + +func (a *Actions) Then() *Consequences { + a.context.T().Helper() + time.Sleep(fixture.WhenThenSleepInterval) + return &Consequences{context: a.context, actions: a} +} diff --git a/test/e2e/fixture/session/consequences.go b/test/e2e/fixture/session/consequences.go new file mode 100644 index 0000000000..581499d2f9 --- /dev/null +++ b/test/e2e/fixture/session/consequences.go @@ -0,0 +1,53 @@ +package session + +import ( + "time" + + "github.com/argoproj/argo-cd/v3/test/e2e/fixture" +) + +// Consequences implements the "then" part of given/when/then for session/logout tests. +type Consequences struct { + context *Context + actions *Actions +} + +// ActionShouldSucceed asserts that the last action completed without error. +func (c *Consequences) ActionShouldSucceed() *Consequences { + c.context.T().Helper() + if c.actions.lastError != nil { + c.context.T().Errorf("expected action to succeed, got error: %v", c.actions.lastError) + } + return c +} + +// ActionShouldFail asserts that the last action returned an error and passes +// it to the callback for further inspection. +func (c *Consequences) ActionShouldFail(block func(err error)) *Consequences { + c.context.T().Helper() + if c.actions.lastError == nil { + c.context.T().Error("expected action to fail, but it succeeded") + return c + } + block(c.actions.lastError) + return c +} + +// AndCLIOutput passes the CLI output and error from the last action to the +// callback for custom assertions. +func (c *Consequences) AndCLIOutput(block func(output string, err error)) *Consequences { + c.context.T().Helper() + block(c.actions.lastOutput, c.actions.lastError) + return c +} + +// Given returns the Context to allow chaining back to setup. +func (c *Consequences) Given() *Context { + return c.context +} + +// When returns the Actions to allow chaining back to actions. +func (c *Consequences) When() *Actions { + time.Sleep(fixture.WhenThenSleepInterval) + return c.actions +} diff --git a/test/e2e/fixture/session/context.go b/test/e2e/fixture/session/context.go new file mode 100644 index 0000000000..e136a3053f --- /dev/null +++ b/test/e2e/fixture/session/context.go @@ -0,0 +1,30 @@ +package session + +import ( + "testing" + "time" + + "github.com/argoproj/argo-cd/v3/test/e2e/fixture" +) + +// Context implements the "given" part of given/when/then for session/logout tests. +type Context struct { + *fixture.TestState +} + +func Given(t *testing.T) *Context { + t.Helper() + state := fixture.EnsureCleanState(t) + return &Context{TestState: state} +} + +// GivenWithSameState creates a new Context that shares the same TestState as an existing context. +func GivenWithSameState(ctx fixture.TestContext) *Context { + ctx.T().Helper() + return &Context{TestState: fixture.NewTestStateFromContext(ctx)} +} + +func (c *Context) When() *Actions { + time.Sleep(fixture.WhenThenSleepInterval) + return &Actions{context: c} +} diff --git a/test/e2e/logout_test.go b/test/e2e/logout_test.go new file mode 100644 index 0000000000..744f12c4e8 --- /dev/null +++ b/test/e2e/logout_test.go @@ -0,0 +1,172 @@ +package e2e + +import ( + "testing" + + "github.com/stretchr/testify/require" + + sessionFixture "github.com/argoproj/argo-cd/v3/test/e2e/fixture/session" +) + +// TestLogoutRevokesToken verifies that after logging out via the /auth/logout endpoint, +// the JWT token is revoked and can no longer be used for API calls. +func TestLogoutRevokesToken(t *testing.T) { + sessionFixture.Given(t). + When(). + Login("token1"). + GetUserInfo("token1"). + ListAccounts("token1"). + Then(). + ActionShouldSucceed(). + When(). + Logout("token1"). + GetUserInfo("token1"). + ListAccounts("token1"). + Then(). + ActionShouldFail(func(err error) { + require.Contains(t, err.Error(), "token is revoked") + }) +} + +// TestLogoutDoesNotAffectOtherSessions verifies that revoking one session token +// does not invalidate a different session token for the same user. +func TestLogoutDoesNotAffectOtherSessions(t *testing.T) { + sessionFixture.Given(t). + When(). + Login("token1"). + Login("token2"). + GetUserInfo("token1"). + GetUserInfo("token2"). + ListAccounts("token1"). + Then(). + ActionShouldSucceed(). + When(). + GetUserInfo("token2"). + ListAccounts("token2"). + Then(). + ActionShouldSucceed(). + When(). + Logout("token1"). + GetUserInfo("token1"). + ListApplications("token1"). + Then(). + ActionShouldFail(func(err error) { + require.Contains(t, err.Error(), "token is revoked") + }). + When(). + ListApplications("token2"). + Then(). + ActionShouldSucceed() +} + +// TestLogoutRevokedTokenCannotAccessAPIs verifies that a revoked token is rejected +// across different API endpoints, not just GetUserInfo. +func TestLogoutRevokedTokenCannotAccessAPIs(t *testing.T) { + sessionFixture.Given(t). + When(). + Login("token1"). + GetUserInfo("token1"). + Logout("token1"). + GetUserInfo("token1"). + ListAccounts("token1"). + Then(). + ActionShouldFail(func(err error) { + require.Contains(t, err.Error(), "token is revoked") + }). + // Project API + When(). + ListProjects("token1"). + Then(). + ActionShouldFail(func(err error) { + require.Contains(t, err.Error(), "token is revoked") + }). + // Repository API + When(). + ListRepositories("token1"). + Then(). + ActionShouldFail(func(err error) { + require.Contains(t, err.Error(), "token is revoked") + }). + // Application API + When(). + ListApplications("token1"). + Then(). + ActionShouldFail(func(err error) { + require.Contains(t, err.Error(), "token is revoked") + }) +} + +// TestLogoutRevokesOIDCToken verifies that token revocation works for tokens +// issued directly by an external OIDC provider (bypassing Dex). This exercises +// the IDP token verification and revocation code path in SessionManager.VerifyToken. +func TestLogoutRevokesOIDCToken(t *testing.T) { + sessionFixture.Given(t). + When(). + WithDirectOIDC(). + LoginWithSSO("token1"). + ListApplications("token1"). + Then(). + ActionShouldSucceed(). + When(). + Logout("token1"). + ListApplications("token1"). + Then(). + ActionShouldFail(func(err error) { + require.Contains(t, err.Error(), "token is revoked") + }) +} + +// TestLogoutDoesNotAffectOtherOIDCSessions verifies that revoking one OIDC session +// token does not invalidate a different OIDC session token. +func TestLogoutDoesNotAffectOtherOIDCSessions(t *testing.T) { + sessionFixture.Given(t). + When(). + WithDirectOIDC(). + LoginWithSSO("token1"). + LoginWithSSO("token2"). + ListApplications("token1"). + Then(). + ActionShouldSucceed(). + When(). + Logout("token1"). + ListApplications("token1"). + Then(). + ActionShouldFail(func(err error) { + require.Contains(t, err.Error(), "token is revoked") + }). + When(). + ListApplications("token2"). + Then(). + ActionShouldSucceed() +} + +// TestLogoutRevokedOIDCTokenCannotAccessAPIs verifies that a revoked OIDC token +// is rejected across different API endpoints. +func TestLogoutRevokedOIDCTokenCannotAccessAPIs(t *testing.T) { + sessionFixture.Given(t). + When(). + WithDirectOIDC(). + LoginWithSSO("token1"). + GetUserInfo("token1"). + Logout("token1"). + GetUserInfo("token1"). + ListApplications("token1"). + Then(). + ActionShouldFail(func(err error) { + require.Contains(t, err.Error(), "token is revoked") + }). + // Project API + When(). + ListProjects("token1"). + Then(). + ActionShouldFail(func(err error) { + require.Contains(t, err.Error(), "token is revoked") + }). + // Repository API + When(). + ListRepositories("token1"). + Then(). + ActionShouldFail(func(err error) { + require.Contains(t, err.Error(), "token is revoked") + }) +} diff --git a/util/localconfig/localconfig.go b/util/localconfig/localconfig.go index e4f8d9879b..e3b061b19f 100644 --- a/util/localconfig/localconfig.go +++ b/util/localconfig/localconfig.go @@ -219,6 +219,16 @@ func (l *LocalConfig) RemoveUser(serverName string) bool { return false } +// GetToken returns the token stored in the local file for the given server name +func (l *LocalConfig) GetToken(serverName string) string { + for _, u := range l.Users { + if u.Name == serverName { + return u.AuthToken + } + } + return "" +} + // Returns true if user was removed successfully func (l *LocalConfig) RemoveToken(serverName string) bool { for i, u := range l.Users { diff --git a/util/localconfig/localconfig_test.go b/util/localconfig/localconfig_test.go index 0cee991996..2548bcfa4d 100644 --- a/util/localconfig/localconfig_test.go +++ b/util/localconfig/localconfig_test.go @@ -9,17 +9,15 @@ import ( "path/filepath" "testing" - "github.com/argoproj/argo-cd/v3/util/config" - "github.com/stretchr/testify/require" - "github.com/stretchr/testify/assert" + "github.com/argoproj/argo-cd/v3/util/config" ) func TestGetUsername(t *testing.T) { - assert.Equal(t, "admin", GetUsername("admin:login")) - assert.Equal(t, "admin", GetUsername("admin")) - assert.Empty(t, GetUsername("")) + require.Equal(t, "admin", GetUsername("admin:login")) + require.Equal(t, "admin", GetUsername("admin")) + require.Empty(t, GetUsername("")) } func TestFilePermission(t *testing.T) { @@ -76,7 +74,7 @@ func TestFilePermission(t *testing.T) { f, err := os.Create(filePath) require.NoError(t, err, "Could not write create config file: %v", err) defer func() { - assert.NoError(t, f.Close()) + require.NoError(t, f.Close()) }() err = f.Chmod(c.perm) @@ -86,7 +84,7 @@ func TestFilePermission(t *testing.T) { require.NoError(t, err, "Could not access the fileinfo: %v", err) if err := getFilePermission(fi); err != nil { - assert.EqualError(t, err, c.expectedError.Error()) + require.EqualError(t, err, c.expectedError.Error()) } else { require.NoError(t, c.expectedError) } @@ -120,12 +118,16 @@ users: - auth-token: vErrYS3c3tReFRe$hToken name: localhost:8080` -const testConfigFilePath = "./testdata/local.config" +const ( + testConfigFilePath = "./testdata/local.config" + testConfigFileName = "local.config" + testWriteFileName = "write-local.config" +) func loadOpts(t *testing.T, opts string) { t.Helper() t.Setenv("ARGOCD_OPTS", opts) - assert.NoError(t, config.LoadFlags()) + require.NoError(t, config.LoadFlags()) } func TestGetPromptsEnabled_useCLIOpts_false_localConfigPromptsEnabled_true(t *testing.T) { @@ -140,7 +142,7 @@ func TestGetPromptsEnabled_useCLIOpts_false_localConfigPromptsEnabled_true(t *te loadOpts(t, "--config "+testConfigFilePath) - assert.True(t, GetPromptsEnabled(false)) + require.True(t, GetPromptsEnabled(false)) } func TestGetPromptsEnabled_useCLIOpts_false_localConfigPromptsEnabled_false(t *testing.T) { @@ -155,7 +157,7 @@ func TestGetPromptsEnabled_useCLIOpts_false_localConfigPromptsEnabled_false(t *t loadOpts(t, "--config "+testConfigFilePath) - assert.False(t, GetPromptsEnabled(false)) + require.False(t, GetPromptsEnabled(false)) } func TestGetPromptsEnabled_useCLIOpts_true_forcePromptsEnabled_default(t *testing.T) { @@ -170,7 +172,7 @@ func TestGetPromptsEnabled_useCLIOpts_true_forcePromptsEnabled_default(t *testin loadOpts(t, "--config "+testConfigFilePath+" --prompts-enabled") - assert.True(t, GetPromptsEnabled(true)) + require.True(t, GetPromptsEnabled(true)) } func TestGetPromptsEnabled_useCLIOpts_true_forcePromptsEnabled_true(t *testing.T) { @@ -185,7 +187,7 @@ func TestGetPromptsEnabled_useCLIOpts_true_forcePromptsEnabled_true(t *testing.T loadOpts(t, "--config "+testConfigFilePath+" --prompts-enabled=true") - assert.True(t, GetPromptsEnabled(true)) + require.True(t, GetPromptsEnabled(true)) } func TestGetPromptsEnabled_useCLIOpts_true_forcePromptsEnabled_false(t *testing.T) { @@ -200,13 +202,135 @@ func TestGetPromptsEnabled_useCLIOpts_true_forcePromptsEnabled_false(t *testing. loadOpts(t, "--config "+testConfigFilePath+" --prompts-enabled=false") - assert.False(t, GetPromptsEnabled(true)) + require.False(t, GetPromptsEnabled(true)) +} + +func TestGetToken_Exist(t *testing.T) { + testFilePath := filepath.Join(t.TempDir(), testConfigFileName) + err := os.WriteFile(testFilePath, []byte(testConfig), os.ModePerm) + require.NoError(t, err) + + err = os.Chmod(testFilePath, 0o600) + require.NoError(t, err, "Could not change the file permission to 0600 %v", err) + localConfig, err := ReadLocalConfig(testFilePath) + require.NoError(t, err) + + token := localConfig.GetToken(localConfig.CurrentContext) + + require.Equal(t, "vErrYS3c3tReFRe$hToken", token) +} + +func TestGetToken_Not_Exist(t *testing.T) { + testFilePath := filepath.Join(t.TempDir(), testConfigFileName) + err := os.WriteFile(testFilePath, []byte(testConfig), os.ModePerm) + require.NoError(t, err) + + err = os.Chmod(testFilePath, 0o600) + require.NoError(t, err, "Could not change the file permission to 0600 %v", err) + localConfig, err := ReadLocalConfig(testFilePath) + require.NoError(t, err) + + // serverName does exist in TestConfig + token := localConfig.GetToken("localhost") + + require.Empty(t, token) +} + +func TestRemoveToken_Exist(t *testing.T) { + testFilePath := filepath.Join(t.TempDir(), testConfigFileName) + err := os.WriteFile(testFilePath, []byte(testConfig), os.ModePerm) + require.NoError(t, err) + + err = os.Chmod(testFilePath, 0o600) + require.NoError(t, err, "Could not change the file permission to 0600 %v", err) + localConfig, err := ReadLocalConfig(testFilePath) + require.NoError(t, err) + + removed := localConfig.RemoveToken(localConfig.CurrentContext) + + require.True(t, removed) +} + +func TestRemoveToken_ClearsBothTokens(t *testing.T) { + testFilePath := filepath.Join(t.TempDir(), testConfigFileName) + err := os.WriteFile(testFilePath, []byte(testConfig), os.ModePerm) + require.NoError(t, err) + + err = os.Chmod(testFilePath, 0o600) + require.NoError(t, err, "Could not change the file permission to 0600 %v", err) + localConfig, err := ReadLocalConfig(testFilePath) + require.NoError(t, err) + + // Verify tokens exist before removal (use argocd1 which has both auth-token and refresh-token) + serverName := "argocd1.example.com:443" + require.Equal(t, "vErrYS3c3tReFRe$hToken", localConfig.GetToken(serverName)) + + removed := localConfig.RemoveToken(serverName) + require.True(t, removed) + + // Verify both AuthToken and RefreshToken are cleared + require.Empty(t, localConfig.GetToken(serverName)) + for _, u := range localConfig.Users { + if u.Name == serverName { + require.Empty(t, u.AuthToken, "AuthToken should be cleared after RemoveToken") + require.Empty(t, u.RefreshToken, "RefreshToken should be cleared after RemoveToken") + return + } + } + t.Fatal("user entry should still exist after RemoveToken") +} + +func TestRemoveToken_Not_Exist(t *testing.T) { + testFilePath := filepath.Join(t.TempDir(), testConfigFileName) + err := os.WriteFile(testFilePath, []byte(testConfig), os.ModePerm) + require.NoError(t, err) + + err = os.Chmod(testFilePath, 0o600) + require.NoError(t, err, "Could not change the file permission to 0600 %v", err) + localConfig, err := ReadLocalConfig(testFilePath) + require.NoError(t, err) + + // serverName does exist in TestConfig + removed := localConfig.RemoveToken("localhost") + + require.False(t, removed) +} + +func TestValidateLocalConfig(t *testing.T) { + testFilePath := filepath.Join(t.TempDir(), testConfigFileName) + err := os.WriteFile(testFilePath, []byte(testConfig), os.ModePerm) + require.NoError(t, err) + + err = os.Chmod(testFilePath, 0o600) + require.NoError(t, err, "Could not change the file permission to 0600 %v", err) + localConfig, err := ReadLocalConfig(testFilePath) + require.NoError(t, err) + + err = ValidateLocalConfig(*localConfig) + + require.NoError(t, err) +} + +func TestWriteLocalConfig(t *testing.T) { + testFilePath := filepath.Join(t.TempDir(), testConfigFileName) + err := os.WriteFile(testFilePath, []byte(testConfig), os.ModePerm) + require.NoError(t, err) + + err = os.Chmod(testFilePath, 0o600) + require.NoError(t, err, "Could not change the file permission to 0600 %v", err) + localConfig, err := ReadLocalConfig(testFilePath) + require.NoError(t, err) + + testWriteFilePath := filepath.Join(t.TempDir(), testWriteFileName) + err = WriteLocalConfig(*localConfig, testWriteFilePath) + + require.NoError(t, err) } func TestReadLocalConfig_FileNotExist(t *testing.T) { config, err := ReadLocalConfig("/nonexistent/path/config") require.NoError(t, err) - assert.Nil(t, config) + require.Nil(t, config) } func TestReadLocalConfig_InvalidYAML(t *testing.T) { @@ -216,7 +340,7 @@ func TestReadLocalConfig_InvalidYAML(t *testing.T) { config, err := ReadLocalConfig(tmpFile) require.Error(t, err) - assert.Nil(t, config) + require.Nil(t, config) } func TestReadLocalConfig_EmptyFile(t *testing.T) { @@ -227,7 +351,7 @@ func TestReadLocalConfig_EmptyFile(t *testing.T) { config, err := ReadLocalConfig(tmpFile) require.NoError(t, err) // Empty file results in empty config, not nil - assert.NotNil(t, config) + require.NotNil(t, config) } func TestReadLocalConfig_ValidConfig(t *testing.T) { @@ -237,7 +361,7 @@ func TestReadLocalConfig_ValidConfig(t *testing.T) { config, err := ReadLocalConfig(tmpFile) require.NoError(t, err) - assert.NotNil(t, config) - assert.Equal(t, "localhost:8080", config.CurrentContext) - assert.Len(t, config.Contexts, 3) + require.NotNil(t, config) + require.Equal(t, "localhost:8080", config.CurrentContext) + require.Len(t, config.Contexts, 3) } diff --git a/util/session/sessionmanager.go b/util/session/sessionmanager.go index 1c98319129..44b9ee0846 100644 --- a/util/session/sessionmanager.go +++ b/util/session/sessionmanager.go @@ -287,7 +287,10 @@ func (mgr *SessionManager) Parse(tokenString string) (jwt.Claims, string, error) return nil, "", fmt.Errorf("account %s does not have '%s' capability", subject, capability) } - if id == "" || mgr.storage.IsTokenRevoked(id) { + if id == "" { + return nil, "", errors.New("token does not have a unique identifier (jti claim) and cannot be validated") + } + if mgr.storage.IsTokenRevoked(id) { return nil, "", errors.New("token is revoked, please re-login") } else if capability == settings.AccountCapabilityApiKey && account.TokenIndex(id) == -1 { return nil, "", fmt.Errorf("account %s does not have token with id %s", subject, id) @@ -303,10 +306,12 @@ func (mgr *SessionManager) Parse(tokenString string) (jwt.Claims, string, error) remainingDuration := time.Until(exp) if remainingDuration < autoRegenerateTokenDuration && capability == settings.AccountCapabilityLogin { - if uniqueId, err := uuid.NewRandom(); err == nil { - if val, err := mgr.Create(fmt.Sprintf("%s:%s", subject, settings.AccountCapabilityLogin), int64(tokenExpDuration.Seconds()), uniqueId.String()); err == nil { - newToken = val - } + var uniqueId uuid.UUID + if uniqueId, err = uuid.NewRandom(); err != nil { + return nil, "", fmt.Errorf("could not create UUID for new JWT token: %w", err) + } + if newToken, err = mgr.Create(fmt.Sprintf("%s:%s", subject, settings.AccountCapabilityLogin), int64(tokenExpDuration.Seconds()), uniqueId.String()); err != nil { + return nil, "", fmt.Errorf("could not create new JWT token: %w", err) } } } @@ -596,6 +601,20 @@ func (mgr *SessionManager) VerifyToken(ctx context.Context, tokenString string) return nil, "", common.ErrTokenVerification } + id, ok := claims["jti"].(string) + if !ok { + log.Warnf("token does not have jti claim") + id = "" + } + // Workaround for Dex token, because does not have jti. + if id == "" { + id = idToken.AccessTokenHash + } + + if mgr.storage.IsTokenRevoked(id) { + return nil, "", errors.New("token is revoked, please re-login") + } + var claims jwt.MapClaims err = idToken.Claims(&claims) if err != nil { diff --git a/util/session/sessionmanager_test.go b/util/session/sessionmanager_test.go index 304fbf21d8..12c97918d2 100644 --- a/util/session/sessionmanager_test.go +++ b/util/session/sessionmanager_test.go @@ -34,6 +34,7 @@ import ( "github.com/argoproj/argo-cd/v3/util" "github.com/argoproj/argo-cd/v3/util/cache" "github.com/argoproj/argo-cd/v3/util/crypto" + "github.com/argoproj/argo-cd/v3/util/dex" jwtutil "github.com/argoproj/argo-cd/v3/util/jwt" "github.com/argoproj/argo-cd/v3/util/oidc" "github.com/argoproj/argo-cd/v3/util/password" @@ -143,21 +144,80 @@ func TestSessionManager_AdminToken_Revoked(t *testing.T) { mgr := newSessionManager(settingsMgr, getProjLister(), storage) - token, err := mgr.Create("admin:login", 0, "123") + token, err := mgr.Create("admin:login", int64(autoRegenerateTokenDuration.Seconds()*2), "123") require.NoError(t, err) - err = storage.RevokeToken(t.Context(), "123", time.Hour) + err = storage.RevokeToken(t.Context(), "123", autoRegenerateTokenDuration*2) require.NoError(t, err) _, _, err = mgr.Parse(token) - assert.EqualError(t, err, "token is revoked, please re-login") + assert.Equal(t, "token is revoked, please re-login", err.Error()) +} + +func TestSessionManager_AdminToken_EmptyJti(t *testing.T) { + redisClient, closer := test.NewInMemoryRedis() + defer closer() + + settingsMgr := settings.NewSettingsManager(t.Context(), getKubeClient(t, "pass", true), "argocd") + mgr := newSessionManager(settingsMgr, getProjLister(), NewUserStateStorage(redisClient)) + + // Create a token with an empty jti + token, err := mgr.Create("admin:login", int64(autoRegenerateTokenDuration.Seconds()*2), "") + require.NoError(t, err) + + _, _, err = mgr.Parse(token) + require.Error(t, err) + assert.Equal(t, "token does not have a unique identifier (jti claim) and cannot be validated", err.Error()) +} + +func TestSessionManager_AdminToken_NoExpirationTime(t *testing.T) { + redisClient, closer := test.NewInMemoryRedis() + defer closer() + + settingsMgr := settings.NewSettingsManager(t.Context(), getKubeClient(t, "pass", true), "argocd") + mgr := newSessionManager(settingsMgr, getProjLister(), NewUserStateStorage(redisClient)) + + token, err := mgr.Create("admin:login", 0, "123") + if err != nil { + t.Errorf("Could not create token: %v", err) + } + + claims, newToken, err := mgr.Parse(token) + require.NoError(t, err) + assert.Empty(t, newToken) + + mapClaims, err := jwtutil.MapClaims(claims) + require.NoError(t, err) + + sub, err := mapClaims.GetSubject() + require.NoError(t, err) + require.Equal(t, "admin", sub) +} + +func TestSessionManager_AdminToken_Expired(t *testing.T) { + redisClient, closer := test.NewInMemoryRedis() + defer closer() + + settingsMgr := settings.NewSettingsManager(t.Context(), getKubeClient(t, "pass", true), "argocd") + mgr := newSessionManager(settingsMgr, getProjLister(), NewUserStateStorage(redisClient)) + + token, err := mgr.Create("admin:login", 2, "123") + if err != nil { + t.Errorf("Could not create token: %v", err) + } + + time.Sleep(3 * time.Second) + + _, _, err = mgr.Parse(token) + require.Error(t, err) + assert.ErrorContains(t, err, "token is expired") } func TestSessionManager_AdminToken_Deactivated(t *testing.T) { settingsMgr := settings.NewSettingsManager(t.Context(), getKubeClient(t, "pass", false), "argocd") mgr := newSessionManager(settingsMgr, getProjLister(), NewUserStateStorage(nil)) - token, err := mgr.Create("admin:login", 0, "abc") + token, err := mgr.Create("admin:login", int64(autoRegenerateTokenDuration.Seconds()*2), "abc") require.NoError(t, err, "Could not create token") _, _, err = mgr.Parse(token) @@ -168,7 +228,7 @@ func TestSessionManager_AdminToken_LoginCapabilityDisabled(t *testing.T) { settingsMgr := settings.NewSettingsManager(t.Context(), getKubeClient(t, "pass", true, settings.AccountCapabilityLogin), "argocd") mgr := newSessionManager(settingsMgr, getProjLister(), NewUserStateStorage(nil)) - token, err := mgr.Create("admin", 0, "abc") + token, err := mgr.Create("admin", int64(autoRegenerateTokenDuration.Seconds()*2), "abc") require.NoError(t, err, "Could not create token") _, _, err = mgr.Parse(token) @@ -202,7 +262,7 @@ func TestSessionManager_ProjectToken(t *testing.T) { mapClaims, err := jwtutil.MapClaims(claims) require.NoError(t, err) - assert.Equal(t, "proj:default:test", mapClaims["sub"]) + require.Equal(t, "proj:default:test", mapClaims["sub"]) }) t.Run("Token Revoked", func(t *testing.T) { @@ -1287,6 +1347,184 @@ requestedScopes: ["oidc"]`, oidcTestServer.URL), require.Error(t, err) assert.ErrorIs(t, err, common.ErrTokenVerification) }) + + t.Run("OIDC provider is external, token is revoked", func(t *testing.T) { + tokenID := "123" + redisClient, closer := test.NewInMemoryRedis() + defer closer() + + storage := NewUserStateStorage(redisClient) + err := storage.RevokeToken(t.Context(), tokenID, autoRegenerateTokenDuration*2) + require.NoError(t, err) + + config := map[string]string{ + "url": "", + "oidc.config": fmt.Sprintf(` + name: Test + issuer: %s + clientID: xxx + clientSecret: yyy + requestedScopes: ["oidc"]`, 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(t.Context(), getKubeClientWithConfig(config, secretConfig), "argocd") + mgr := NewSessionManager(settingsMgr, getProjLister(), "", nil, storage) + mgr.verificationDelayNoiseEnabled = false + + claims := jwt.RegisteredClaims{Audience: jwt.ClaimStrings{"xxx"}, Subject: "admin", ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Hour * 24))} + claims.Issuer = oidcTestServer.URL + claims.ID = tokenID + 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(t.Context(), tokenString) + require.Error(t, err) + assert.Equal(t, "token is revoked, please re-login", err.Error()) + }) + + t.Run("OIDC provider is Dex, token is revoked", func(t *testing.T) { + tokenID := "123" + redisClient, closer := test.NewInMemoryRedis() + defer closer() + + storage := NewUserStateStorage(redisClient) + err := storage.RevokeToken(t.Context(), tokenID, autoRegenerateTokenDuration*2) + require.NoError(t, err) + + config := map[string]string{ + "url": dexTestServer.URL, + "dex.config": `connectors: + - type: github + name: GitHub + config: + clientID: aabbccddeeff00112233 + clientSecret: aabbccddeeff00112233`, + } + + // 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(t.Context(), getKubeClientWithConfig(config, secretConfig), "argocd") + mgr := NewSessionManager(settingsMgr, getProjLister(), dexTestServer.URL, &dex.DexTLSConfig{StrictValidation: false}, storage) + mgr.verificationDelayNoiseEnabled = false + + claims := jwt.RegisteredClaims{Audience: jwt.ClaimStrings{"argo-cd-cli"}, Subject: "admin", ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Hour * 24))} + claims.Issuer = dexTestServer.URL + "/api/dex" + claims.ID = tokenID + 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(t.Context(), tokenString) + require.Error(t, err) + assert.Equal(t, "token is revoked, please re-login", err.Error()) + }) +} + +func TestSessionManager_RevokeToken(t *testing.T) { + redisClient, closer := test.NewInMemoryRedis() + defer closer() + + settingsMgr := settings.NewSettingsManager(t.Context(), getKubeClient(t, "pass", true), "argocd") + storage := NewUserStateStorage(redisClient) + mgr := newSessionManager(settingsMgr, getProjLister(), storage) + + t.Run("Revoke and verify", func(t *testing.T) { + tokenID := "revoke-test-id" + assert.False(t, storage.IsTokenRevoked(tokenID), "token should not be revoked initially") + + err := mgr.RevokeToken(t.Context(), tokenID, time.Hour) + require.NoError(t, err) + + assert.True(t, storage.IsTokenRevoked(tokenID), "token should be revoked after RevokeToken") + }) + + t.Run("Revoked token rejected by Parse", func(t *testing.T) { + tokenID := "parse-revoke-id" + token, err := mgr.Create("admin:login", int64(autoRegenerateTokenDuration.Seconds()*2), tokenID) + require.NoError(t, err) + + // Token should parse fine before revocation + _, _, err = mgr.Parse(token) + require.NoError(t, err) + + // Revoke and verify Parse rejects it + err = mgr.RevokeToken(t.Context(), tokenID, autoRegenerateTokenDuration*2) + require.NoError(t, err) + + _, _, err = mgr.Parse(token) + require.Error(t, err) + assert.Equal(t, "token is revoked, please re-login", err.Error()) + }) +} + +func TestSessionManager_VerifyToken_DexTokenRevokedByAccessTokenHash(t *testing.T) { + dexTestServer := utiltest.GetDexTestServer(t) + t.Cleanup(dexTestServer.Close) + + accessTokenHash := "dex-access-token-hash-xyz" + redisClient, closer := test.NewInMemoryRedis() + defer closer() + + storage := NewUserStateStorage(redisClient) + // Revoke using the access token hash (the fallback ID for Dex tokens without jti) + err := storage.RevokeToken(t.Context(), accessTokenHash, autoRegenerateTokenDuration*2) + require.NoError(t, err) + + config := map[string]string{ + "url": dexTestServer.URL, + "dex.config": `connectors: + - type: github + name: GitHub + config: + clientID: aabbccddeeff00112233 + clientSecret: aabbccddeeff00112233`, + } + + secretConfig := map[string][]byte{ + "tls.crt": utiltest.Cert, + "tls.key": utiltest.PrivateKey, + } + + settingsMgr := settings.NewSettingsManager(t.Context(), getKubeClientWithConfig(config, secretConfig), "argocd") + mgr := NewSessionManager(settingsMgr, getProjLister(), dexTestServer.URL, &dex.DexTLSConfig{StrictValidation: false}, storage) + mgr.verificationDelayNoiseEnabled = false + + // Create a Dex token WITHOUT jti — the at_hash field in the OIDC ID token should be used as fallback + claims := jwt.MapClaims{ + "aud": []string{"argo-cd-cli"}, + "sub": "admin", + "exp": jwt.NewNumericDate(time.Now().Add(time.Hour * 24)), + "iss": dexTestServer.URL + "/api/dex", + "at_hash": accessTokenHash, + // Deliberately no "jti" claim + } + 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(t.Context(), tokenString) + require.Error(t, err) + assert.Equal(t, "token is revoked, please re-login", err.Error()) } func Test_PickFailureAttemptWhenOverflowed(t *testing.T) { diff --git a/util/test/testutil.go b/util/test/testutil.go index 0271a6241c..862c927ba2 100644 --- a/util/test/testutil.go +++ b/util/test/testutil.go @@ -134,6 +134,20 @@ func dexMockHandler(t *testing.T, url string) func(http.ResponseWriter, *http.Re "claims_supported": ["sub", "aud", "exp"] }`, url) require.NoError(t, err) + case "/api/dex/keys": + pubKey, err := jwt.ParseRSAPublicKeyFromPEM(Cert) + require.NoError(t, err) + jwks := jose.JSONWebKeySet{ + Keys: []jose.JSONWebKey{ + { + Key: pubKey, + }, + }, + } + out, err := json.Marshal(jwks) + require.NoError(t, err) + _, err = w.Write(out) + require.NoError(t, err) default: w.WriteHeader(http.StatusNotFound) }