hapijs/cookie

Improve redirectTo behavior

sholladay opened this issue ยท 4 comments

The redirectTo feature is great, but it is way too eager to activate and does not respect the auth.mode of routes.

From the docs:

Note that using redirectTo with authentication mode 'try' will cause the protected endpoint to always redirect, voiding 'try' mode.

And also:

redirectOnTry - if false and route authentication mode is 'try', authentication errors will not trigger a redirection. Requires hapi version 6.2.0 or newer. Defaults to true

This is all needlessly silly and complicated. I bet at least 95% of the time what people want is for redirectTo to only affect requests whose auth mode is required.

Currently I have to add this to every single route where auth is optional.

plugins : {
    'hapi-auth-cookie' : {
        redirectTo : false
    }
}

I'm failing to think of a scenario where I would want redirectTo and optional / try together. But if there is one, it is definitely not the common case.

I propose that redirectTo only triggers for required auth. We could have a redirectOnTry: true to re-enable the old behavior, but it's still a breaking change. And with the new behavior, I'm not sure anyone actually even needs the redirectOnTry option at all.

@sholladay As noted in the PR, I worry about users that expect this behavior. Since there is a flag to override the behavior, why should we eliminate it?

there is a flag to override the behavior

There is no flag that fixes the redirectTo shenanigans for optional mode across routes. You could introduce a redirectOnOptional hack or just fix redirectTo. The latter is what PR #155 provides.

why should we eliminate it?

  • We should strive to eliminate any behavior that is confusing.
  • More importantly, we should respect auth mode and not duplicate its functionality.

This was fixed in #186. ๐Ÿ˜ƒ

lock commented

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.