Redis samples should use the TTL feature

[OrangeDog]

Expected Behavior
The main point of using Redis as a backing store (as detailed in e.g. #1642) is to make use of Redis' TTL feature to automatically expire all the limited-lifetime objects.

Current Behavior
No TTL is set anywhere I can see in the Redis sample code. Performance will degrade (much faster than the JPA implementation) as it is used and the store grows with useless expired authorizations.

Users would have to add a maintenance step, e.g. using slow SCAN commands to manually check everything and delete expired things.

Context
Given the design of the model (a single auth can have multiple different token types, all with their own expirations) I don't think it can be easily solved by just putting @TimeToLive on all the relevant fields, as a single entity is only allowed one. There is also the problem that the @Indexed expiration is tied to the parent entity, and not to the token it's indexing.

The whole design of the sample may need significant changes in order to actually be useable. If it's easy to get TTL working properly with a few changes then it should include that. If it is not then the sample is of no use, as any production implementation needs to have TTL working.

At the moment, I'm looking at building a similar implementation to the old Spring OAuth2 RedisTokenStore, but there is still the fundamental problem in the new design that the OAuth2Authorization contains all the tokens, instead of them being separate objects saved independently.

[jgrandja]

@OrangeDog

The main point of using Redis as a backing store (as detailed in e.g. #1642) is to make use of Redis' TTL feature to automatically expire all the limited-lifetime objects.

The main purpose of the How-to guide/sample provided in gh-1019 was to demonstrate how to implement RegisteredClientRepository, OAuth2AuthorizationService and OAuth2AuthorizationConsentService using Redis. This is meant to be a simplified sample and to be used as a starting point for users to take and enhance however they see fit. In no way whatsoever is it meant to be a production ready sample that makes use of Redis specific features and was never advertised as such.

The whole design of the sample may need significant changes in order to actually be useable. If it's easy to get TTL working properly with a few changes then it should include that. If it is not then the sample is of no use, as any production implementation needs to have TTL working.

Feel free to build a sample yourself and please share it with the community here or in gh-1019.

There are no further plans to update the sample as it's only intended to be a simple starting point for users to take and enhance however they see fit.

[OrangeDog]

intended to be a simple starting point

As I tried to explain, it completely fails at that. You cannot start from that point. The design is fatally flawed because TTL cannot simply be added to it. A completely different design is needed.

Feel free to build a sample yourself and please share it with the community here.

Again, as I explained, I am trying to do that. But it requires a lot of work because the provided samples are unusable and the core design doesn't let you e.g. simply list all the tokens and their expiration.

[antoinelauzon-bell]

Hi @jgrandja, if I may use this issue to ask a related question:

I was looking at the provided JdbcOAuth2AuthorizationService implementation and it seems that the invalided tokens (whatever their type) stay in persistence forever. Is there something that I am missing? For example, when the device code flow is completed through OAuth2DeviceCodeAuthenticationProvider, I would expect the used user code and device code to be cleared from persistence. Could you provide some insights on this?

Thank you

[jgrandja]

@antoinelauzon-bell

Could you provide some insights on this?

Please see the following comments for further context and why the framework is not responsible for application data cleanup.

• gh-1266
• gh-1434
• gh-873
• gh-556

[OrangeDog]

OK, that attempt didn't work. When a refresh token is used, and is not being reused, the old one is not invalidated (#1128).
Thus there's no way to delete/update it, and all tokens must instead indirectly refer to the auth id.

I'll delete the above and come back when it's sorted.

Edit: not even that is sufficient. The provider does not check that the presented refresh token is in the returned authorization, and there's no easy way to add that (#1941). It can be done in the service, however.

[OrangeDog]

Here is a working example.

It will be transactional if the provided RedisTemplate is transactional.
Otherwise it probably needs manual multi()/exec()/discard() with RedisSession.

None of the token values should ever collide, so it does not bother to separate them.
Not indexing inactive tokens is optional, but may help if you're using e.g. short device codes.
If you need more token types, then add their extraction to processTokens.

You also need a working RedisTemplate, which can be challenging.
The basic JdkSerializationRedisSerializer will work, but is not safe across upgrades (#1203).
I've gone for a Jackson2JsonRedisSerializer and self-implemented the necessary mixins for what I'm using (#1970).

@Component
@RequiredArgsConstructor
public class RedisAuthorizationService implements OAuth2AuthorizationService {

    private static final Duration AUDIT_PERIOD = Duration.ofDays(7);
    private static final Duration STATE_TOKEN_EXPIRY = Duration.ofMinutes(5);
    private static final String KEY_PREFIX = "spring:security:oauth2:";
    private static final String AUTH_KEY = KEY_PREFIX + "auth:";
    private static final String TOKEN_KEY = KEY_PREFIX + "token:";

    private final RedisTemplate<String, OAuth2Authorization> redisTemplate;

    @Override
    @Transactional(propagation = Propagation.REQUIRES_NEW)
    public void save(OAuth2Authorization authorization) {
        Collection<AbstractOAuth2Token> tokens = processTokens(authorization);
        for (AbstractOAuth2Token token : tokens) {
            addLookup(TOKEN_KEY + token.getTokenValue(), authorization.getId(), token.getExpiresAt());
        }

        Instant latestExpiry = Stream.concat(Stream.of(Instant.now().plus(AUDIT_PERIOD)), tokens.stream()
                .flatMap(token -> Optional.ofNullable(token.getExpiresAt()).stream()))
                .max(Instant::compareTo)
                .orElseThrow();

        String authKey = AUTH_KEY + authorization.getId();
        redisTemplate.opsForValue().set(authKey, authorization);
        redisTemplate.expireAt(authKey, latestExpiry);
    }

    @Override
    @Transactional(propagation = Propagation.REQUIRES_NEW)
    public void remove(OAuth2Authorization authorization) {
        redisTemplate.delete(AUTH_KEY + authorization.getId());
        for (AbstractOAuth2Token token : processTokens(authorization)) {
            redisTemplate.delete(TOKEN_KEY + token.getTokenValue());
        }
    }

    @Override
    public @Nullable OAuth2Authorization findById(String id) {
        return redisTemplate.opsForValue().get(AUTH_KEY + id);
    }

    @Override
    public @Nullable OAuth2Authorization findByToken(String token, @Nullable OAuth2TokenType tokenType) {
        String tokenKey = TOKEN_KEY + token;
        byte[] authId = redisTemplate.execute((RedisCallback<byte[]>) connection ->
                connection.stringCommands().get(tokenKey.getBytes(StandardCharsets.UTF_8))
        );
        if (authId == null) {
            return null;
        }
        OAuth2Authorization auth = findById(new String(authId, StandardCharsets.UTF_8));
        if (auth == null) {
            redisTemplate.delete(tokenKey);
            return null;
        }
        if (tokenType != null && tokenType.getValue().equals(OAuth2ParameterNames.STATE)) {
            if (!token.equals(auth.getAttribute(OAuth2ParameterNames.STATE))) {
                return null;
            }
        } else {
            Token<?> tokenObj = auth.getToken(token);
            if (tokenObj == null || !tokenObj.isActive()) {
                redisTemplate.delete(tokenKey);
                return null;
            }
        }
        return auth;
    }

    private void addLookup(String key, String authId, @Nullable Instant expiresAt) {
        Expiration expiration = expiresAt == null ?
                Expiration.persistent() :  // or AUDIT_PERIOD
                Expiration.unixTimestamp(expiresAt.toEpochMilli(), TimeUnit.MILLISECONDS);
        redisTemplate.execute((RedisCallback<?>) connection ->
            connection.stringCommands().set(
                    key.getBytes(StandardCharsets.UTF_8),
                    authId.getBytes(StandardCharsets.UTF_8),
                    expiration,
                    RedisStringCommands.SetOption.UPSERT
            )
        );
    }

    private Collection<AbstractOAuth2Token> processTokens(OAuth2Authorization authorization) {
        Collection<AbstractOAuth2Token> tokens = new ArrayList<>();
        String stateToken = authorization.getAttribute(OAuth2ParameterNames.STATE);
        if (stateToken != null) {
            tokens.add(new StateToken(stateToken));
        }
        processToken(authorization.getToken(OAuth2AuthorizationCode.class), tokens::add);
        processToken(authorization.getToken(OAuth2AccessToken.class), tokens::add);
        processToken(authorization.getToken(OAuth2RefreshToken.class), tokens::add);
        return tokens;
    }

    private void processToken(Token<? extends AbstractOAuth2Token> token, Consumer<AbstractOAuth2Token> consumer) {
        if (token != null) {
            if (token.isActive()) {
                consumer.accept(token.getToken());
            } else {
                redisTemplate.delete(TOKEN_KEY + token.getToken().getTokenValue());
            }
        }
    }

    private static class StateToken extends AbstractOAuth2Token {
        private static final @Serial long serialVersionUID = 1L;
        protected StateToken(String tokenValue) {
            super(tokenValue, Instant.now(), Instant.now().plus(STATE_TOKEN_EXPIRY));
        }
    }

}

This highlights some design issues such as the state token not actually being a token (and the inconsistent usage of OAuth2TokenType vs Class<? extends AbstractOAuth2Token>), there being no way to list (or remove, if invalidated) tokens.