Skip to content

Fix/document redirectTo as function#169

Closed
flippidippi wants to merge 17 commits intohapijs:masterfrom
flippidippi:master
Closed

Fix/document redirectTo as function#169
flippidippi wants to merge 17 commits intohapijs:masterfrom
flippidippi:master

Conversation

@flippidippi
Copy link
Copy Markdown
Contributor

@flippidippi flippidippi commented Oct 3, 2017 •
edited
Loading

Currently when redirectTo is a function and the function returns blank, hapi-auth-cookie stills tries to do the redirect instead of behaving like redirectTo: undefined. Fixing this will allow issues like #144 where you might want different behavior on an API call vs the browser.

This change will allow something like following code to redirect or not according to a header set

redirectTo (request) {
  const requestedWith = request.headers['x-requested-with'] && request.headers['x-requested-with'].toLowerCase()
  if (requestedWith && (requestedWith === 'ajax' || requestedWith === 'xmlhttprequest')) {
    return undefined
  } else {
    return '/login'
  }
}

@flippidippi
Copy link
Copy Markdown
Contributor Author

flippidippi commented Nov 9, 2017

@nlf? This seems like a simple enough addition. Can you check it out?

@mrlannigan
Copy link
Copy Markdown
Contributor

mrlannigan commented Dec 13, 2017

Hello @flipxfx,

Thank you very much for the contribution! I think is a reasonable change. Do you mind resolving the conflicts and added at least a test to cover the new use case for this?

@mrlannigan mrlannigan added the feature New functionality or improvement label Dec 13, 2017
@flippidippi
Copy link
Copy Markdown
Contributor Author

flippidippi commented Dec 13, 2017 •
edited
Loading

Shit I may have done that wrong 😞. I resolved the conflicts and added a test, but I think I messed up how I merged or something.

@flippidippi flippidippi mentioned this pull request Dec 18, 2017
Merged
@flippidippi
Copy link
Copy Markdown
Contributor Author

flippidippi commented Dec 18, 2017

@mrlannigan I royally destroyed this pull request somehow. I created a new cleaner one #183.

@lock
Copy link
Copy Markdown

lock bot commented Jan 9, 2020

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.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

feature New functionality or improvement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@flippidippi @mrlannigan @aknuds1