Support One-Time Tokens in a Clustered Environment

[franticticktick]

It would be nice to implement session based OneTimeTokenService. Now this is difficult to do, because OneTimeTokenService accepts GenerateOneTimeTokenRequest. It is better to change the design of the generate method - replace GenerateOneTimeTokenRequest with HttpServletRequest.
It is not yet clear how you can call consume for an http session.

[franticticktick]

Maybe the best solution would be to split this component into two? For example OneTimeTokenGenerator and OneTimeTokenRepository.
Then

OneTimeToken ott = this.oneTimeTokenGenerator.generate(generateRequest);
this.oneTimeTokenGenerator.save(ott);

With this implementation it is much easier to ensure OneTimeTokenRepository works in a cluster environment.

[marcusdacoregio]

Hi @CrazyParanoid, thanks for the report.

Can you elaborate more on your use case? Would you like to authenticate the session that started the request instead of the session that submits the token?

An alternative that I had in mind is to provide a GenerateOneTimeTokenRequestResolver that can be used in the GenerateOneTimeTokenFilter, something like:

public interface GenerateOneTimeTokenRequestResolver {

    GenerateOneTimeTokenRequest resolve(HttpServletRequest request);

}

This way you can return a subclass of GenerateOneTimeTokenRequest with the appropriate information.

[franticticktick]

Hi @marcusdacoregio

An alternative that I had in mind is to provide a GenerateOneTimeTokenRequestResolver that can be used in the GenerateOneTimeTokenFilter, something like:

This may be a good solution, but I would first look at the design of the OneTimeTokenService. It seems to me that two components for generation a token and storing it would give much more flexibility in the API, as well as the single responsibility principle.
Overall, separating the token generation logic from its storage will make the API simpler, cleaner, and easier to use.

[franticticktick]

Can you elaborate more on your use case? Would you like to authenticate the session that started the request instead of the session that submits the token?

I'm more interested in the ability of the OneTimeTokenService to work in a distributed environment. Maybe JbcOneTimeTokenService or CachedOneTimeTokenService? Something like this.

[marcusdacoregio]

I'm unsure because OneTimeTokenService#consume is like a delete operation. I don't think I'd like to see:

this.oneTimeTokenService.consume(ott);
this.oneTimeTokenRepository.delete(ott);

What if the token value is a JWT, for example? It wouldn't have to be saved anywhere and the repository would be a no-op.

[franticticktick]

@marcusdacoregio
This may make sense, I would like to discuss the idea of ​​​​implementing OneTimeTokenService to be able to work with multiple instances of the same application. What do you think about it?

[rwinch]

@CrazyParanoid I think that it makes sense for us to provide a OneTimeTokenService that works in multiple instances. I'm not keen on the idea of using a JWT because the lack of state means that it can be reused and isn't really a one time token. I think creating a JdbcOneTimeTokenService makes the most sense. If we need to, some of the shared logic can be extracted from InMemoryOneTimeTokenService and shared with JdbcOneTimeTokenService. Is this something you would be interested in submitting a Pull Request for?

[franticticktick]

Is this something you would be interested in submitting a Pull Request for?

Yes, I'll take it to work. I'll think about how best to extract the shared code, maybe something like:

public final class OneTimeTokenUtils {
	public static long DEFAULT_ONE_TIME_TOKEN_TIME_TO_LIVE = 300;

	private static Clock clock = Clock.systemUTC();

	private OneTimeTokenUtils() {
	}

	public static OneTimeToken generateOneTimeToken(GenerateOneTimeTokenRequest request, long timeToLive) {
		String token = UUID.randomUUID().toString();
		Instant fiveMinutesFromNow = clock.instant().plusSeconds(timeToLive);
		return new DefaultOneTimeToken(token, request.getUsername(), fiveMinutesFromNow);
	}

	public static boolean isExpired(OneTimeToken token) {
		return clock.instant().isAfter(token.getExpiresAt());
	}
}

Or something like this.

[franticticktick]

@marcusdacoregio @rwinch could you please review PR?

[rwinch]

@CrazyParanoid Sorry about the delay. I've submitted a review now.