riposte icon indicating copy to clipboard operation
riposte copied to clipboard

RequestSecurityValidator should have the ability to return a full response

Open nicmunroe opened this issue 8 years ago • 1 comments

RequestSecurityValidator.validateSecureRequestForEndpoint(...) currently has a void return type, with the idea that if the request passes auth you do nothing, and if it fails auth you throw an appropriate exception.

We should change the return type to Optional<ResponseInfo<?>> to allow you to short circuit with an explicit full-flexibility response if you wanted to. For example, you might want to send a 302/307 redirect.

So there would be three options for handling a request in RequestSecurityValidator.validateSecureRequestForEndpoint(...):

  • Return Optional.empty() (or null) to indicate the request passed auth and request processing should proceed.
  • Return a non-empty Optional<ResponseInfo<?>> to short circuit with the provided response immediately, bypassing any endpoint and any after-security-validator-RequestAndResponseFilters. (The response side of RequestAndResponseFilter should still run).
  • Throw an exception with the same support we have now.

This would be an API breaking change.

nicmunroe avatar Nov 17 '17 19:11 nicmunroe

To toss out another API breaking change, maybe considering changing the API to return a CompletableFuture instead of assuming a quick, fast response on the main thread when you make this change.

CompletableFuture<Optional<ResponseInfo<?>>> validateSecureRequestForEndpoint

This would remove the need for isFastEnoughToRunOnNettyWorkerThread flag

rabeyta avatar Nov 21 '17 17:11 rabeyta