Redirect only when auth is required

[sholladay]

Update: for hapi 17 support, see PR #186

Fixes #154.

Q: What's the purpose of redirectTo? A: Redirecting preempts the original route handler to protect it from unauthenticated requests.

Q: But what about routes that explicitly want to process unauthenticated requests? Isn't this the purpose of optional and try modes? A: redirectTo screws them up. Oops.

Okay, so ... let's fix this and respect the auth mode.

---

This changes two closely related things. First and most importantly, it changes the behavior of redirectTo so that its effects only apply to auth mode required. Thus it no longer conflicts with the intent of optional and try modes. Secondly, it removes redirectOnTry.

I removed redirectOnTry because:

• It seems to only be a workaround for a problem that this PR solves.
• I don't believe anyone actually wants to redirect on try. I think everyone who is using try just set this to false, which is redundant now given the new behavior of redirectTo.
• The changing default behavior of redirectTo makes this semver major anyway. Now is a good time to remove it so that migration is easier and more intuitive.

The diff for the tests is poor. Basically just added a test for optional auth mode, fixed the one for try, and removed the obsolete redirectOnTry test. Also fixed uri -> url (see this) and the weird executables while I was here. Happy to split things up if necessary.

[jaw187]

@sholladay Thanks for the PR.

This feature was inherited with the project, so I'm not entirely sure what the motivation is for redirecting with optional auth. Since the feature exists, I'm not overly interested in making such a big change if we're overlooking some common case.

Considering there is a flag that can be set to address your changes, I think it's best to leave everything as is. But I'm willing to think about it more.

[sholladay]

there is a flag that can be set to address your changes

Nope. 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 this PR provides.

[sholladay]

Hi @mrlannigan thanks for stepping up to maintain this module. Would you mind giving your thoughts on this and letting me know if there is any chance of getting this in? If so, I will fix the merge conflicts.

I have been using a fork of this project because it's been too much of a burden to add boilerplate to every route and then explain to people on my team why the behavior is inconsistent for different auth modes.

[mrlannigan]

@sholladay Thanks for the contribution and #186. Over the next week, I will continue the conversation in the new PR.

Closing this one.

[lock[bot]]

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.