Giraffe icon indicating copy to clipboard operation
Giraffe copied to clipboard

Feature request: authentication handler modifications

Open baronfel opened this issue 5 years ago • 2 comments

The current authentication auth handlers don't add the authenticated identity (if any) to the context User's identities, so they can't really be used in conjunction with the current requiresAuthentication handler. In addition, the requiresAuthentication handler only checks the first Identity on the User, so if other authentication mechanisms add on additional identities (as is the expected practice) this handler will erroneously detect those requests as unauthorized.

I propose the following:

  • add an authenticate: (scheme: string) -> HttpHandler that will attempt to authenticate the given scheme and add the authenticated identity to the current User
    • this handler is best-effort and will always chain forward. this allows for use with other handlers like requiresAuthentication
  • update requiresAuthentication to check against all user identities
    • this makes it more correct while still keeping the expected semantics

Sample implementations of these can be found at https://gist.github.com/baronfel/b66f33b98febe2592a305aa2e782183f, which is an extraction of I've used in a production setting to get around the problems I've listed.

The other handlers (authenticateMany, tryAuthenticate, etc) could all be convenience ones built on those two primitives.

baronfel avatar Dec 23 '19 15:12 baronfel

@dustinmoris we're gonna start working on this if this is approved

TheAngryByrd avatar Feb 26 '20 21:02 TheAngryByrd

@TheAngryByrd Sorry for the late reply but that sounds good to me!

dustinmoris avatar Apr 12 '20 19:04 dustinmoris