exhal icon indicating copy to clipboard operation
exhal copied to clipboard

Use a pluggable authorizer to calculated the `Authorization` header field value.

Open pezra opened this issue 6 years ago • 7 comments

pezra avatar Oct 01 '18 14:10 pezra

I'm excited about the switch to mox!

adherr avatar Oct 01 '18 20:10 adherr

I'd like to cache my authorization header value in my Authorizer impl. How do you feel about adding a reset callback to the protocol that is called when a request fails due to authorization problems? I can put a case statement in my client code for this, but it would be cleaner if I didn't have to wrap my ExHal requests with this logic.

adherr avatar Oct 01 '18 20:10 adherr

I am torn about mox. It seems to work but i am pretty sure it increased the chance of bugs lurking in the code. My use of it couples the tests to HTTP library and interface of the Client module. In a very real sense those tests now verify that the functions are implemented a particular way, rather than that they achieve the desired goal.

pezra avatar Oct 01 '18 22:10 pezra

@adherr, how would you like that "reset" callback to work?

Maybe, an new function

unauthorized_response_received(url, [challenge]) :: :retry | :continue

Upon receive a 401 Unauthorized response ExHal would call unauthorized_response_received/2 for the current authorizer. If it returned :retry then the request would be retried form the top. If it returned :continue the current error flow would be executed.

Questions:

  • Could we add this as a separate PR?
  • How digested should the challenge be? Values of the WWW-Authenticate header field are a standardized format so we could parse it.

pezra avatar Oct 01 '18 22:10 pezra

@adherr, @alakra, another question: should authorization/2 return a header fields map, rather than just the credentials? This might allow it to support esoteric auth schemes that use non-standard and/or support for the Signature header, etc

pezra avatar Oct 01 '18 22:10 pezra

general curious question: Why a protocol instead of a behaviour?

I think the choice to return a header map from the Authorizer was a good one.

I like the proposed unauthorized_response_received function, especially the ability to retry. Parsing the WWW-Authenticate header is fine, but our services don't issue them, so ExHal shouldn't count on them being sent with a 401 or 403. I doubt (I may be wrong here) we would do anything useful with the challenge argument, we'd just attempt to reauthenticate and retry any time the function was called.

A separate PR for that work is 👍

adherr avatar Oct 04 '18 17:10 adherr

@adherr asked:

Why a protocol instead of a behaviour

Excellent question. I'm not entirely sure protocol is the best choice. The two seem basically isomorphic for most use cases. Three use cases i specifically considered are the built-in SimpleAuthorizer, a config based basic auth authorizer, and a password grant oauth2 authorizer.

Protocols make SimpleAuthorizer very simple to implement. No state holding processes or attending supervisors are required. We just shove the state in a struct and have the protocol implementation access that data.

For the config based basic auth authorizer use case a behavior might be marginally simpler. In this scenario the authorizer module would just Application.get_env with the appropriate application and item names. Having to define a struct is pure, albeit minimal, overhead.

A password grant oauth2 authorizer is going to need a state holding process, and attending supervisors. It would convert a username and password into an access token at runtime and then remember that token for the remainder of the session. A behavior might be slightly more natural, but it doesn't seem to be a big win either way.

As you can see, two out of those three use cases would seem to benefit from using a behavior, rather than a protocol. There is, apparently, a pretty significant performance penalty to protocols vs behaviors, so that is a potential downside. However, SimpleAuthorizer would be substantially more complex as a behavior.

What you do you think? Should i convert authorizers into a behavior?

pezra avatar Oct 07 '18 23:10 pezra