Add 307 redirect to AuthorizationListener#620
Add 307 redirect to AuthorizationListener#620oliverhausler wants to merge 1 commit intomrniko:masterfrom oliverhausler:1.7.13
Conversation
|
I have also written an improved AuthorizationListener at https://github.com/oliverhausler/netty-socketio/tree/1.7.13-improved-authorization Data can then be retrieved as `client.get("key"); untested, undocumented |
| result = configuration.getAuthorizationListener().isAuthorized(data); | ||
| redirectUrl = configuration.getAuthorizationListener().isRedirected(data); | ||
| } catch (Exception ignore) { | ||
| } |
There was a problem hiding this comment.
It's a bad practice to swallow Exceptions. Please fix
There was a problem hiding this comment.
I'm not working on this PR branch anymore. I decided to carefully break compatibility in some places to implement more features, see #631 instead, where this is fixed.
Instead returning isRedirected in a separate @OverRide, I decided to move it into the same authorize function and return a response object instead a boolean. This breaks compatibility, but enables us to do a lot more.
I'd suggest you reject this PR and switch to the newer one.
Use case:
For building a server cluster, we wanted to 307 redirect to another machine before authentication occurs. I added an
@Override isRedirected()to AuthorizationListener for this. I tried to make the change without breaking compatibility.If it interests you, feel free to merge. If there's a better way to achieve this, please let me know.
I based the change on 1.7.13 because it's the latest stable version.