Suggestions for API changes and the addition of an Authorize combinator

[m-renaud]

Hey, moving the discussion from haskell-servant/servant#799 here. I had a few ideas for changes to the servant-auth API with the goal of separating the concerns of authentication, authorization, and the handler. The notable changes being:

• New Authorize combinator which ensures an authenticated user is authorized to access a route
  • The handler for an Authorized route does not deal with the authentication and/or authorization failures that can happen
• Existing Auth combinator becomes Authenticate with some small changes
  • Handler no longer takes AuthResult user, instead the Handler is enhanced with maybeAuthUser :: Handler (Maybe User) which can be used if desired
• The AuthorizeHandler (name tbd) typeclass which centralizes the logic for the authorization check (:: user -> AuthorizationState user), what to do for an authorized route when either: a) authentication has not been performed, b) the user is unauthorized.

A short example of a route/handler before and after:

Current API:

type API auths
  = (Auth auths User :> Protected)
  :<|> ...

type Protected
   = "name" :> Get '[JSON] String
   :<|> "email" :> Get '[JSON] String

protected :: AuthResult User -> Server Protected
protected (Authenticated user) =
  if username user == "admin"
  then return (name user) :<|> return (email user)
  else throwAll err401
protected _ = throwAll err401

Proposed API:

data IsAdmin

type API auths
    = (Authenticate auths User :> Authorize IsAdmin User :> Protected)
    :<|> ...

type Protected
   = "name" :> Get '[JSON] String
   :<|> "email" :> Get '[JSON] String

instance AuthorizeHandler (IsAdmin User) where
  whenAuthenticationRequired _ = redirect "login"
  whenUnauthorized _ = throwAll err401
  authorize _ user =
    if username user == "admin"
    then return (Authorized user)
    else return (Unauthorized "Not admin")

protected :: User -> Server Protected
protected user = return (name user) :<|> return (email user)

I have a doc where I have most of the details (I thought it would be easier to collect thoughts there than in an issue). Feel free to leave comments there or here, it's still and early draft and would appreciate any feedback :)

[jkarni]

I like the proposal quite a bit! Broadly, it seems like the right way to incorporate authorization.

I'm still a little unclear on how you propose to implement it. In particular, I'm not sure I see how the Authorize combinator would in fact have access to the AuthenticationState (notice that implementations of route only have access to Server api, so I don't think adding ReaderT does what you hope). And I anyhow think it would be a bad design, since it would mean that handlers change their monads in ways that would be somewhat annoying to predict.

The good news is that I think it ought to be possible to use the context here. Roughly:

instance
  (... , HasServer (AddSetCookiesApi n api) (Authenticated usr ': ctxs)) =>
  HasServer (Authenticate auth usr :> api) ctxs where
  ...
  route = _ -- as before, but add the user to the context

instance
  (... , HasConfigEntry usr ctxts) =>
  HasServer (Authorize auth usr :> api) ctxs where
  ...
  route = _ -- get the user from the config entry, check that it is authorized

Though there will likely be subtleties since the Auth/Authenticate combinator isn't really performing authentication in route - it's just scheduling it. So perhaps the correct way to implement the Authorize instance is to change Delayed in servant-server. Something like:

data Delayed env c where
  Delayed :: { capturesD :: env -> DelayedIO captures
             , methodD   :: DelayedIO ()
             , authD     :: DelayedIO auth
             -- this is new
             , authorizeD     :: auth -> DelayedIO auth'
             , acceptD   :: DelayedIO ()
             , contentD  :: DelayedIO contentType
             , paramsD   :: DelayedIO params
             , headersD  :: DelayedIO headers
             , bodyD     :: contentType -> DelayedIO body
             , serverD   :: captures
                         -> params
                         -> headers
                         -> auth'
                         -> body
                         -> Request
                         -> RouteResult c
             } -> Delayed env c

And then the implementation of Authorize would hopefully just be "addAuthorizationCheck _".

I don't at the moment need this, and so won't have time to work on it, but if you're willing to investigate, I'm happy to help!

[m-renaud]

Thanks for the feedback!

I'm still a little unclear on how you propose to implement it.

As am I :) I haven't dug very deeply into the internals of Servant yet, I was hoping to do that over the coming weeks when I have some spare time. ReaderT was the first thing that came to mind for a way to express adding access to the authentication state to the handler function, but as you mention changing the monad in unpredictable ways doesn't sound very nice.

Though there will likely be subtleties since the Auth/Authenticate combinator isn't really performing authentication in route - it's just scheduling it.

Ah yes, I noticed the Delayed type when reading through the code and wasn't sure exactly what the semantics of it were with relation to when actions are performed. From my little knowledge of how Servant works internally your idea to use context and modify Delayed seems like it could work.

I don't at the moment need this, and so won't have time to work on it, but if you're willing to investigate, I'm happy to help!

I would love to! Do you know of any docs on how Servant and/or servant-auth work internally? I've dug around in the code a little bit but a doc that says "Here are all the pieces and how they fit together" would be super helpful. Thanks!

[jkarni]

@defanor pointed out in the servant freenode channel something that in retrospect seems obvious - namely, that authorization can depend on other components of the API. For instance GET /account/<userid> might only be authorized if the userid captured matches that of the person logged in. It isn't clear how to incorporate that here.

[jkarni]

I would love to! Do you know of any docs on how Servant and/or servant-auth work internally? I've dug around in the code a little bit but a doc that says "Here are all the pieces and how they fit together" would be super helpful. Thanks!

The internals of servant are described in the paper and more succinctly and accessibly in implementing a minimal version of haskell-servant. That said, neither of them describe the Delayed mechanism (which was added later).

servant-auth doesn't really have any documentation about internals, but hopefully it is relatively comprehensible after understanding servant's internals.

If you have questions, I'm happy to answer them here, or you can drop by the #servant freenode IRC channel.

[m-renaud]

authorization can depend on other components of the API....

That's a good point, since query params become arguments to the handler function this seems to imply that this type of authorization can only be performed in the handler. I'll have to dig into the internal details first though.

The internals of servant are described in...

Thanks, I'll check these out and pop into the IRC channel.

[m-renaud]

Okay, some initial thoughts (read: non-solutions) to @defanor's comment about other parts of the API taking part in authorization.

Non-solution 1: If only the logged in user can ever access their account page, then when designing the API just drop the <userid> part of the path so you just have GET /account. Then the authorization becomes "is the user authenticated, if so then they are authorized" and the handler will have type User -> Handler a.

Non-solution 2: Use the current available mechanism, in the handler you can still bail out if the authorized user does not match the capture param.

Both of these are dodging the question of how we incorporate this case (and ones like) it into the authorization combinator. One thought I had was if there was some way for the authorize function's type to change based on the :> rest of the path component through some use of the ServerT associated type in the HasServer instance. I need to think about it some more though.

[domenkozar]

Merging into #73 as it has slightly better title, but same contents :)