Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Now it works with Express 4+ relative urls #49

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

delian
Copy link

@delian delian commented Oct 1, 2015

Hello,
I am proposing a little fix that adds req.baseUrl to the req.url to creds.uri checkup and this way allows the passport-http digest authentication to work fine with Express4 and 5 and especially with the relative urls it introduces.

If you build default project with Express4, you cannot simply add passport-http digest authentication to a sand-boxed route file, because digest.js verify req.uri to creds.uri. But req.uri is always relative while creds.uri is always full. Therefore the correct comparison is req.baseUrl+req.url == creds.uri.

This fix is quite simple and works very well to me, while it shall preserve compatibility with the old express versions.

Signed-off-by: Delian Delchev delian.delchev@gmail.com

Signed-off-by: Delian Delchev <delian.delchev@gmail.com>
@cirinoalejandro
Copy link

I would like this to be merged, just found the same issue, and the fix works.

@sashkamal
Copy link

Can we get this merged. Thanks.

@matthewjustice
Copy link

I ran into the same issue and the fix looks good. Can this be merged? Thank you!

@beaulac
Copy link

beaulac commented Jan 30, 2020

This is still an issue 4 years later :(

Is there a reason this change has not been merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants