Redis user-keyed upstream token read does not verify row ownership

Summary

GetLatestUpstreamTokensForUser on the Redis backend can return an upstream-token
row whose UserID differs from the requested userID, because the per-member
decode helper never compares them.

This is not reachable through the public API — see "Why this isn't exploitable"
below. Filing it as hardening: the postcondition currently rests on session-id
entropy plus an index the codebase already documents as leaky, and there are three
consumers that would misbehave if it ever failed.

Detail

parseUserUpstreamEntry (pkg/authserver/storage/redis.go:1167-1196) rejects nil
values, non-strings, nullMarker tombstones, unmarshal errors, and a mismatched
stored.ProviderID. It does not compare stored.UserID to the requested userID.
The doc comment above it enumerates that rejection set and omits UserID, so this
looks deliberate rather than accidental.

Note this is not a case of "memory validates and Redis forgot". The memory
backend has no index — it scans s.upstreamTokens, so UserID != userID
(memory.go:915) is its lookup key. Redis pushed the user filter into the index key
(redisSetKey(prefix, KeyTypeUserUpstream, userID)). Neither backend has a second
check; the asymmetry is that Redis trusts the index while memory recomputes.

filterIndexMembersByPrefix does not help here — it filters on the instance key
prefix for CROSSSLOT avoidance, not on the user.

Why this isn't exploitable

Index members are prefix:upstream:{sessionID}:{providerName} — no userID in the
member. sessionID is a 26-char rand.Text() minted per authorize flow
(pkg/authserver/server/handlers/authorize.go:93) and carried unchanged through
chain legs and refresh, so two users cannot land on the same member key. The Lua
write path also SREMs the previous owner's member atomically with the write
(redis.go:799-806), which is a second barrier.

Every stale-member vector I checked — row TTL expiry, tombstones, both delete
paths, redis_migrate.go, concurrent writes — produces members that MGet to nil
and are skipped at redis.go:1168.

Why fix it anyway

• The user-upstream set has no TTL and is never garbage collected (existing TODO at
  redis.go:46-48), so it is the one index guaranteed to accumulate dead members.
• DeleteUser (redis.go:1631) unconditionally Dels whatever that set names, so
  the same unvalidated trust has a destructive consumer, not just a read one.
• The read's only current consumer, maybeCarryForwardRefreshToken
  (handlers/callback.go:223-240), copies prior.RefreshToken into the current
  user's row. Its prior.UpstreamSubject == providerSubject guard is skipped when
  the provider is synthetic, so a foreign refresh token would be adopted silently
  in that case.

A planned user-keyed credential read for non-interactive clients would add a fourth
consumer, which is what prompted looking at this.

Proposed fix

About six lines: add a userID parameter to parseUserUpstreamEntry, update the
call site at redis.go:1122, and add the comparison next to the existing
ProviderID check at redis.go:1191.

Safe to add — every writer files a row under its own UserID (the Lua path uses
tokens.UserID for both the SET and the SADD; redis_migrate.go:216 uses
stored.UserID), and rows with an empty UserID are never indexed.

Test

Goes in TestRedisStorage_GetLatestUpstreamTokensForUser
(pkg/authserver/storage/redis_test.go:2181). newTestRedisStorage returns the
*miniredis.Miniredis handle, so a test can plant a foreign member directly with
mr.SAdd on redisSetKey(keyPrefix, KeyTypeUserUpstream, "user-A"). There is
precedent for that at redis_test.go:2899-2903.

Worth noting: both backends already have a different_user_not_matched subtest
(memory_test.go:698, redis_test.go:2236), and the Redis one carries a comment
saying it passes via the empty-SMEMBERS short-circuit rather than a per-row check.
So the gap was known; no existing test asserts the postcondition at row level.

Start in pkg/authserver/storage/redis.go at parseUserUpstreamEntry and its call site, then read TestRedisStorage_GetLatestUpstreamTokensForUser in pkg/authserver/storage/redis_test.go. Use the existing miniredis setup and the foreign-member precedent to verify that a row with a different UserID is rejected. Done means the per-row ownership check is covered by a regression test while existing provider and stale-member behavior remains intact.