fix(proxy): revoke all cached tokens during backchannel logout

changelog/unreleased/fix-backchannel-logout-all-tokens.md

@@ -0,0 +1,14 @@
+Bugfix: Revoke every cached token during backchannel logout
+
+Backchannel logout now invalidates all accepted access tokens associated with
+the requested session or subject, including tokens issued before a refresh.
+Revoked tokens cannot authenticate again through local JWT verification when
+userinfo lookup is disabled, and concurrent claims writes cannot overwrite a
+revocation. Notification failures no longer prevent token invalidation.
+
+OIDC logout state uses a dedicated cache namespace with per-record expiry and
+migrates existing persistent claims on startup, accepting empty legacy caches.
+NATS delete markers are physically removed without deleting concurrent writes.
+Tokens without a verified expiry retain their logout state indefinitely.
+See the proxy caching
+documentation for persistence and upgrade details.

services/proxy/README.md

@@ -247,7 +247,7 @@ The `proxy` service can use a configured store via `PROXY_OIDC_USERINFO_CACHE_ST
   -   `memory`: Basic in-memory store and the default.
   -   `redis-sentinel`: Stores data in a configured Redis Sentinel cluster.
   -   `nats-js-kv`: Stores data using key-value-store feature of [nats jetstream](https://docs.nats.io/nats-concepts/jetstream/key-value-store)
-  -   `noop`: Stores nothing. Useful for testing. Not recommended in production environments.
+  -   `noop`: Disables claims caching. Logout state is kept in memory. Useful for testing. Not recommended in production environments.
 
 Other store types may work but are not supported currently.
 
@@ -260,6 +260,12 @@ Store specific notes:
   -   When using `nats-js-kv` it is recommended to set `OC_CACHE_STORE_NODES` to the same value as `OC_EVENTS_ENDPOINT`. That way the cache uses the same nats instance as the event bus.
   -   When using the `nats-js-kv` store, it is possible to set `OC_CACHE_DISABLE_PERSISTENCE` to instruct nats to not persist cache data on disc.
 
+Backchannel logout tracks each accepted access token separately and rejects revoked tokens even when `PROXY_OIDC_SKIP_USER_INFO` is enabled. Revocations are kept until the verified token expiry. Tokens without a verified expiry, including tokens imported from the legacy claims cache, require logout state with no automatic expiry. The claims cache continues to use the configured TTL when the token has no expiry.
+
+OIDC records use the configured database with the suffix `-oidc-v2` and the configured table with the suffix `/oidc-v2/`. This separates logout state from legacy caches and their bucket-wide TTL. On startup, persistent stores import unexpired legacy claims into the new session index. All proxy instances must use the updated code and the same persistent store to share logout decisions; memory-backed logout state is lost when the process stops. Keep the persistent OIDC namespace when clearing ordinary caches.
+
+For NATS, the new bucket has no bucket-wide TTL. The proxy enforces each record's expiry and removes expired records and their delete markers every minute. Marker cleanup is limited to the proxy's table and the observed revisions, preserving concurrent writes. An empty legacy bucket requires no migration. The deprecated `ocmem` option uses a dedicated memory store for OIDC state to prevent capacity eviction of revocations.
+
 
 ## Presigned Urls
 

services/proxy/pkg/command/oidc_cache.go

@@ -0,0 +1,63 @@
+package command
+
+import (
+	"context"
+	"time"
+
+	"github.com/nats-io/nats.go"
+	"github.com/opencloud-eu/opencloud/services/proxy/pkg/config"
+	bcl "github.com/opencloud-eu/opencloud/services/proxy/pkg/staticroutes/backchannellogout"
+	"github.com/opencloud-eu/reva/v2/pkg/store"
+	microstore "go-micro.dev/v4/store"
+)
+
+func newUserInfoCache(cfg *config.Cache) *bcl.Cache {
+	storeType := cfg.Store
+	cacheClaims := storeType != store.TypeNoop
+	if !cacheClaims || storeType == store.TypeOCMem {
+		// Security records must not use the shared, capacity-evicted ocmem
+		// cache, which also ignores the configured database namespace.
+		storeType = store.TypeMemory
+	}
+	database := cfg.Database
+	if database == "" {
+		database = "cache-userinfo"
+	}
+	// Keep the no-TTL bucket separate from other services and older proxies
+	// which may use the configured database with a bucket-wide TTL.
+	// Redis ignores Database, so give its table a dedicated prefix as well.
+	backend := newUserInfoStore(cfg, storeType, database+"-oidc-v2", cfg.Table+"/oidc-v2/", 0)
+	if storeType == store.TypeNatsJSKV {
+		cleanupConfig := *cfg
+		cleanupConfig.Nodes = append([]string(nil), cfg.Nodes...)
+		backend = bcl.WithNATSCleanup(backend, func() (*nats.Conn, error) {
+			return connectOIDCNATSCache(&cleanupConfig)
+		})
+	}
+	return bcl.NewCache(backend, cacheClaims)
+}
+
+func migrateUserInfoCache(ctx context.Context, cache *bcl.Cache, cfg *config.Cache) error {
+	// Memory and noop stores cannot contain entries from a previous process.
+	if cfg.Store == "" || cfg.Store == "mem" || cfg.Store == store.TypeMemory || cfg.Store == store.TypeNoop || cfg.Store == store.TypeOCMem {
+		return nil
+	}
+	legacy := newUserInfoStore(cfg, cfg.Store, cfg.Database, cfg.Table, cfg.TTL)
+	defer legacy.Close()
+	return cache.IndexLegacyTokens(ctx, legacy)
+}
+
+func newUserInfoStore(cfg *config.Cache, storeType, database, table string, ttl time.Duration) microstore.Store {
+	return store.Create(
+		store.Store(storeType),
+		store.TTL(ttl),
+		microstore.Nodes(cfg.Nodes...),
+		microstore.Database(database),
+		microstore.Table(table),
+		store.DisablePersistence(cfg.DisablePersistence),
+		store.Authentication(cfg.AuthUsername, cfg.AuthPassword),
+		store.TLSEnabled(cfg.EnableTLS),
+		store.TLSInsecure(cfg.TLSInsecure),
+		store.TLSRootCA(cfg.TLSRootCACertificate),
+	)
+}

services/proxy/pkg/command/oidc_cache_nats.go

@@ -0,0 +1,31 @@
+package command
+
+import (
+	"crypto/tls"
+
+	"github.com/nats-io/nats.go"
+	"github.com/opencloud-eu/opencloud/services/proxy/pkg/config"
+)
+
+// connectOIDCNATSCache mirrors the NATS authentication and TLS options used by
+// reva's store factory. Cleanup needs the same access as ordinary cache writes.
+func connectOIDCNATSCache(cfg *config.Cache) (*nats.Conn, error) {
+	opts := nats.GetDefaultOptions()
+	opts.Name = "opencloud-proxy-oidc-cleanup"
+	opts.Servers = cfg.Nodes
+	opts.User, opts.Password = cfg.AuthUsername, cfg.AuthPassword
+	if cfg.EnableTLS {
+		if cfg.TLSRootCACertificate != "" {
+			if err := nats.RootCAs(cfg.TLSRootCACertificate)(&opts); err != nil {
+				return nil, err
+			}
+		} else {
+			if err := nats.Secure(&tls.Config{
+				MinVersion: tls.VersionTLS12, InsecureSkipVerify: cfg.TLSInsecure, //nolint:gosec
+			})(&opts); err != nil {
+				return nil, err
+			}
+		}
+	}
+	return opts.Connect()
+}

services/proxy/pkg/command/oidc_cache_nats_test.go

@@ -0,0 +1,91 @@
+package command
+
+import (
+	"context"
+	"errors"
+	"testing"
+	"time"
+
+	nserver "github.com/nats-io/nats-server/v2/server"
+	"github.com/nats-io/nats.go"
+	"github.com/opencloud-eu/opencloud/services/proxy/pkg/config"
+	bcl "github.com/opencloud-eu/opencloud/services/proxy/pkg/staticroutes/backchannellogout"
+	"github.com/stretchr/testify/require"
+	"go-micro.dev/v4/store"
+)
+
+func newOIDCNATSTestConfig(t *testing.T) (*config.Cache, nats.JetStreamContext) {
+	t.Helper()
+	username, password := "oidc-test", t.Name()
+	server, err := nserver.NewServer(&nserver.Options{
+		Host: "127.0.0.1", Port: -1, JetStream: true, StoreDir: t.TempDir(), Username: username, Password: password,
+	})
+	require.NoError(t, err)
+	go server.Start()
+	t.Cleanup(func() { server.Shutdown(); server.WaitForShutdown() })
+	require.True(t, server.ReadyForConnections(5*time.Second))
+	conn, err := nats.Connect(server.ClientURL(), nats.UserInfo(username, password))
+	require.NoError(t, err)
+	t.Cleanup(conn.Close)
+	js, err := conn.JetStream()
+	require.NoError(t, err)
+	return &config.Cache{
+		Store: "nats-js-kv", Database: "cache-userinfo", Nodes: []string{server.ClientURL()}, TTL: time.Minute,
+		AuthUsername: username, AuthPassword: password,
+	}, js
+}
+
+func TestOIDCCacheMigratesEmptyNATS(t *testing.T) {
+	for _, state := range []string{"new bucket", "expired entries", "delete markers only"} {
+		t.Run(state, func(t *testing.T) {
+			cfg, js := newOIDCNATSTestConfig(t)
+			if state == "expired entries" {
+				cfg.TTL = 100 * time.Millisecond
+			}
+			legacy := newUserInfoStore(cfg, cfg.Store, cfg.Database, cfg.Table, cfg.TTL)
+			t.Cleanup(func() { _ = legacy.Close() })
+			if state != "new bucket" {
+				require.NoError(t, legacy.Write(&store.Record{Key: "old", Value: []byte("old claims")}))
+				if state == "delete markers only" {
+					require.NoError(t, legacy.Delete("old"))
+				}
+				bucket, err := js.KeyValue(cfg.Database)
+				require.NoError(t, err)
+				require.Eventually(t, func() bool {
+					_, err := bucket.Keys()
+					return errors.Is(err, nats.ErrNoKeysFound)
+				}, 3*time.Second, 10*time.Millisecond)
+			}
+			cache := newUserInfoCache(cfg)
+			t.Cleanup(func() { _ = cache.Close() })
+			require.NoError(t, migrateUserInfoCache(context.Background(), cache, cfg), "empty legacy caches must not prevent proxy startup")
+			key, err := bcl.NewKey("alice", "session")
+			require.NoError(t, err)
+			session, err := bcl.NewSuSe(key)
+			require.NoError(t, err)
+			_, err = bcl.GetLogoutRecords(session, cache)
+			require.ErrorIs(t, err, store.ErrNotFound, "an empty current cache must behave as an already logged-out session")
+		})
+	}
+}
+
+func TestOIDCCacheWiresNATSMarkerCleanup(t *testing.T) {
+	cfg, js := newOIDCNATSTestConfig(t)
+	cache := newUserInfoCache(cfg)
+	t.Cleanup(func() { _ = cache.Close() })
+	require.NoError(t, cache.Write(&store.Record{Key: "deleted"}))
+	require.NoError(t, cache.Delete("deleted"))
+	legacy := newUserInfoStore(cfg, cfg.Store, cfg.Database, cfg.Table, cfg.TTL)
+	t.Cleanup(func() { _ = legacy.Close() })
+	require.NoError(t, legacy.Write(&store.Record{Key: "legacy"}))
+	require.NoError(t, legacy.Delete("legacy"))
+	cleaner, ok := cache.Store.(interface{ PurgeDeleted(context.Context) error })
+	require.True(t, ok, "the configured NATS store must support marker maintenance")
+	require.NoError(t, cleaner.PurgeDeleted(context.Background()), "cleanup must use the configured authentication")
+	info, err := js.StreamInfo("KV_cache-userinfo-oidc-v2")
+	require.NoError(t, err)
+	require.Zero(t, info.State.Msgs)
+	info, err = js.StreamInfo("KV_cache-userinfo")
+	require.NoError(t, err)
+	require.EqualValues(t, 1, info.State.Msgs, "cleanup must leave the legacy bucket untouched")
+}

services/proxy/pkg/command/oidc_cache_test.go

@@ -0,0 +1,111 @@
+package command
+
+import (
+	"context"
+	"encoding/base64"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"testing"
+	"time"
+
+	nserver "github.com/nats-io/nats-server/v2/server"
+	"github.com/nats-io/nats.go"
+	"github.com/opencloud-eu/opencloud/services/proxy/pkg/config"
+	bcl "github.com/opencloud-eu/opencloud/services/proxy/pkg/staticroutes/backchannellogout"
+	"github.com/stretchr/testify/require"
+	"github.com/vmihailenco/msgpack/v5"
+	"go-micro.dev/v4/store"
+)
+
+func TestOIDCCacheBackends(t *testing.T) {
+	for _, backend := range []string{"memory", "nats-js-kv", "redis"} {
+		t.Run(backend, func(t *testing.T) {
+			cfg := &config.Cache{Store: backend, Database: "cache-userinfo", TTL: time.Second}
+			var natsURL string
+			switch backend {
+			case "nats-js-kv":
+				server, err := nserver.NewServer(&nserver.Options{Host: "127.0.0.1", Port: -1, JetStream: true, StoreDir: t.TempDir()})
+				require.NoError(t, err)
+				go server.Start()
+				t.Cleanup(func() { server.Shutdown(); server.WaitForShutdown() })
+				require.True(t, server.ReadyForConnections(5*time.Second))
+				natsURL = server.ClientURL()
+				cfg.Nodes = []string{natsURL}
+			case "redis":
+				binary, err := exec.LookPath("redis-server")
+				if err != nil {
+					t.Skip("redis-server is required for the Redis backend integration test")
+				}
+				socket := filepath.Join(t.TempDir(), "redis.sock")
+				cmd := exec.Command(binary, "--port", "0", "--unixsocket", socket, "--save", "", "--appendonly", "no")
+				require.NoError(t, cmd.Start())
+				t.Cleanup(func() { _ = cmd.Process.Kill(); _ = cmd.Wait() })
+				require.Eventually(t, func() bool { _, err := os.Stat(socket); return err == nil }, 5*time.Second, 10*time.Millisecond)
+				cfg.Nodes = []string{"unix://" + socket}
+			}
+
+			legacy := newUserInfoStore(cfg, cfg.Store, cfg.Database, cfg.Table, cfg.TTL)
+			t.Cleanup(func() { _ = legacy.Close() })
+			cache := newUserInfoCache(cfg)
+			defer cache.Close()
+			var tokenKeys []string
+			data, err := msgpack.Marshal(map[string]any{"sub": "alice", "sid": "session", "exp": time.Now().Add(time.Hour).Unix()})
+			require.NoError(t, err)
+			for i := range 2 {
+				bytes := make([]byte, 64)
+				bytes[0] = byte(i)
+				key := base64.URLEncoding.EncodeToString(bytes)
+				tokenKeys = append(tokenKeys, key)
+				require.NoError(t, legacy.Write(&store.Record{Key: key, Value: data, Expiry: time.Hour}))
+				lookup, err := bcl.NewKey("alice", "session")
+				require.NoError(t, err)
+				require.NoError(t, legacy.Write(&store.Record{Key: lookup, Value: []byte(key), Expiry: time.Hour}))
+			}
+			// Persistent backends import all old claims, not just the last lookup.
+			require.NoError(t, cache.IndexLegacyTokens(context.Background(), legacy))
+			lookup, err := bcl.NewKey("alice", "session")
+			require.NoError(t, err)
+			suse, err := bcl.NewSuSe(lookup)
+			require.NoError(t, err)
+			records, err := bcl.GetLogoutRecords(suse, cache)
+			require.NoError(t, err)
+			require.Len(t, records, 2)
+			for _, record := range records {
+				require.NoError(t, bcl.RevokeToken(record, cache))
+				require.NoError(t, cache.Delete(record.Key))
+				require.NoError(t, cache.Delete(string(record.Value)))
+			}
+			// Exercise known token lifetimes independently of migration's unknown TTL.
+			require.NoError(t, bcl.RevokeToken(&store.Record{Value: []byte("known-expiry"), Expiry: time.Hour}, cache))
+			if backend != "memory" {
+				peer := newUserInfoCache(cfg)
+				t.Cleanup(func() { _ = peer.Close() })
+				require.NoError(t, migrateUserInfoCache(context.Background(), peer, cfg))
+				cache = peer
+			}
+			if natsURL != "" {
+				conn, err := nats.Connect(natsURL)
+				require.NoError(t, err)
+				defer conn.Close()
+				js, err := conn.JetStream()
+				require.NoError(t, err)
+				old, err := js.StreamInfo("KV_cache-userinfo")
+				require.NoError(t, err)
+				require.Equal(t, cfg.TTL, old.Config.MaxAge)
+				current, err := js.StreamInfo("KV_cache-userinfo-oidc-v2")
+				require.NoError(t, err)
+				require.Zero(t, current.Config.MaxAge, "bucket TTL must not discard revocations")
+				require.Eventually(t, func() bool {
+					records, err := legacy.Read(tokenKeys[0])
+					return (err == nil || err == store.ErrNotFound) && len(records) == 0
+				}, 5*time.Second, 20*time.Millisecond)
+			}
+			for _, key := range append(tokenKeys, "known-expiry") {
+				revoked, err := bcl.IsTokenRevoked(key, cache)
+				require.NoError(t, err)
+				require.True(t, revoked, "revocations must survive migration, another proxy, and the legacy bucket TTL")
+			}
+		})
+	}
+}

services/proxy/pkg/command/server.go

@@ -58,18 +58,7 @@ func Server(cfg *config.Config) *cobra.Command {
 			return configlog.ReturnFatal(parser.ParseConfig(cfg))
 		},
 		RunE: func(cmd *cobra.Command, args []string) error {
-			userInfoCache := store.Create(
-				store.Store(cfg.OIDC.UserinfoCache.Store),
-				store.TTL(cfg.OIDC.UserinfoCache.TTL),
-				microstore.Nodes(cfg.OIDC.UserinfoCache.Nodes...),
-				microstore.Database(cfg.OIDC.UserinfoCache.Database),
-				microstore.Table(cfg.OIDC.UserinfoCache.Table),
-				store.DisablePersistence(cfg.OIDC.UserinfoCache.DisablePersistence),
-				store.Authentication(cfg.OIDC.UserinfoCache.AuthUsername, cfg.OIDC.UserinfoCache.AuthPassword),
-				store.TLSEnabled(cfg.OIDC.UserinfoCache.EnableTLS),
-				store.TLSInsecure(cfg.OIDC.UserinfoCache.TLSInsecure),
-				store.TLSRootCA(cfg.OIDC.UserinfoCache.TLSRootCACertificate),
-			)
+			userInfoCache := newUserInfoCache(cfg.OIDC.UserinfoCache)
 
 			signingKeyStore := store.Create(
 				store.Store(cfg.PreSignedURL.SigningKeys.Store),
@@ -121,6 +110,12 @@ func Server(cfg *config.Config) *cobra.Command {
 				cfg.Context, cancel = signal.NotifyContext(context.Background(), runner.StopSignals...)
 				defer cancel()
 			}
+			if err := migrateUserInfoCache(cfg.Context, userInfoCache, cfg.OIDC.UserinfoCache); err != nil {
+				return fmt.Errorf("failed to migrate OIDC cache: %w", err)
+			}
+			cacheContext, stopCacheCleanup := context.WithCancel(cfg.Context)
+			defer stopCacheCleanup()
+			go userInfoCache.CollectExpired(cacheContext, logger)
 
 			m := metrics.New()
 			m.BuildInfo.WithLabelValues(version.GetString()).Set(1)