Skip to content

Basic auth is not allowed on endpoints that aren't the token endpoint #6

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

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

Conversation

mcicoria
Copy link

@mcicoria mcicoria commented Jun 4, 2013

Right now, if I'm using restify-oauth2 then I can't have basic auth on any of my endpoints because it will respond with a 401 before it gets to my route. In some cases, I would like to loosely secure an endpoint with just basic auth and nothing else or allow basic auth and oauth authorization.

This change allows for users to implement their own basic auth endpoints if they see fit without compromising the req.username or req.clientId checks.

@domenic
Copy link
Owner

domenic commented Jun 7, 2013

So I definitely will to accept this. It makes sense; Restify–OAuth2 was being presumptuous to think all requests that have an authorization header are going to be signed with an OAuth2 token. Better to assume all requests with authorization type Bearer are. And indeed, as long as it only adds username or clientId properties when a valid token is passed, this seems fine.

But yeah, I want to fix up the tests, add some unit tests too, maybe make that example a bit clearer, and so on. So I'll merge this over the weekend when I have time to do all those things :).

Sorry for the delay in replying. I kept thinking I'd have time to just merge it and do all those things, but it's been a few days now, so I thought you deserved to know what's up.

@gmaniac
Copy link
Collaborator

gmaniac commented Jan 30, 2015

+1

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.

3 participants