Setting `failWithError` should not set status code and headers

[blakeembrey]

I'm writing custom middleware to handle authentication and implementing it with Passport, but I just run into the headers being set even though I asked for the error: https://github.com/jaredhanson/passport/blob/0b3931330e245d8e8851328a7dc436433d6411c9/lib/middleware/authenticate.js#L147-L150. Could this instead give me the challenges along with the error and I can decide whether it's still an issue?

[jaredhanson]

What's the issue? They are set with the intention that the error middleware just needs to format the response body

If you don't like the defaults, your error middleware can unset them or change their values

Sent from my iPhone

On Jul 28, 2015, at 12:13 AM, Blake Embrey notifications@github.com wrote:

I'm writing custom middleware to handle authentication and implementing it with Passport, but I just run into the headers being set even though I asked for the error: https://github.com/jaredhanson/passport/blob/0b3931330e245d8e8851328a7dc436433d6411c9/lib/middleware/authenticate.js#L147-L150. Could this instead give me the challenges with the errors and I can decide whether it's still an error?

— Reply to this email directly or view it on GitHub.

[blakeembrey]

@jaredhanson I have ended up doing that, but I'd expect that you wouldn't mess with the response if we're handing the error. Also, it should be possible to access the challenges on the error, for our own middleware to use. I guess I could always intercept and parse it from the set header and then remove the header myself, but it's not a great solution.

[jaredhanson]

There's lots of middleware that automatically set headers for you, independent of error handling.

This is the API that Passport exposes. It's proven useful for the vast majority of use cases. For use cases outside of that, it seems like you've identified a good workaround.

[jaredhanson]

Could this instead give me the challenges along with the error and I can decide whether it's still an issue?

In what cases would an authentication failure not be an issue, and something you choose to ignore? I'm open to suggestions, but this seems dangerous. If you could explain your use case further, I'm sure there are more idiomatic approaches.

I've closed the issue, because this is something that unlikely requires a code change to passport. But, feel free to continue the discussion.

[blakeembrey]

@jaredhanson I'm not sure how much middleware out there does both though - an error and setting response information. That's the awkward part.

Aside from that, yes, I am working around it. My use case is related to bundling multiple authentication middleware (like you already support with arrays) except that it may not be coming from Passport. I would attempt to convert the middleware responses into handlers for Passport myself, but the models aren't exactly compatible. I'll look into it more for the future, but for reference I was working on: https://github.com/mulesoft/osprey/pull/71. The handlers are passed around as middleware for each route currently, instead of registering strategies and implicitly using the names. This is mostly important since we could be using wildly different schemes (OAuth 2.0 vs basic auth) and the custom options could enforce certain scopes to be used with only OAuth 2.0.

TL;DR: I'm working on using RAML to generate middleware with Passport.

[jaredhanson]

Interesting. I'm interested in RAML, and other declarative approaches to wiring APIs. I'm about out of time for the night, but I'll take a look at this. Keep me in the loop as to any specifics, especially where auth and passport are concerned.

[blakeembrey]

Will do, everything so far has integrated mostly flawlessly with the plain HTTP router (just needed to parse query and attach a redirect method for OAuth 2.0 - plus other middleware for sessions, etc). Do you foresee a way to either subclass a strategy or run "forks" with Passport itself? That's the only way I can think to keep this logic inside Passport because the flow can be something like (oauth 2 && scope) || basic auth. However, it's not a huge concern because it all does work without Passport explicit anyway. Thanks for the great work!