Unexpected Auth interaction

[Vlix]

I have noticed a peculiar interaction with using the Auth API part. Even when you use a ReqBody afterwards in that same endpoint, the Auth handling will be done AFTER the ReqBody has been parsed/transformed. i.e.:

type MyApi = Auth [JWT] MyFields :> ReqBody '[JSON] MyBody :> Post '[PlainText] NoContent

In this case, even when the JWT fails to verify or when the header isn't even present, the endpoint will FIRST reply with 400 BAD CONTENT responses until the JSON is parsed correctly, and THEN the 401 UNAUTHORIZED I throw when the result of AuthResult is not Authenticated.

Is this the expected behaviour? Because from a security perspective, I would like to not leak my data format to any person that knows (or stumbles upon) my endpoints.

I would expect that it might work that way when you define ReqBody :> Auth, but even then I'd prefer the Auth to be checked first.

[Vlix]

I think I found the problem. It's the authCheck in the HasServer instance for Auth.
The runAuth will happily return something other than Authenticated, but this doesn't actually throw a delayedFail(Fatal) in the DelayedIO section of that authCheck function, so it will happily first try and parse the request body before then passing the AuthResult to the handler and THEN let the user decide to throw a 401 UNAUTHORIZED or not.

I think the 401 should be thrown immediately if the jwtAuthCheck doesn't result in an Authenticated{}.
Possibly have the JWTSettings take a handler the user can pass in how to handle the AuthResult? If the user doesn't want the immediate 401 or maybe if they want to return something different (like a different response body?)

---

EDIT: Thinking about it a bit more, would it make more sense to change the ServerT definition to v -> ServerT api m (instead of the AuthResult v -> ServerT api m? Given there'd be a user-provided handler that would be AuthResult v -> DelayedIO v or AuthResult v -> Maybe ServerError or something else to decide how to short-circuit the handler?

[alpmestan]

This reminds me of the Required/Optional and Strict/Lenient modifiers that we have for query params, request bodies, headers, etc, so I'd probably just reuse those and make the auth combinator more flexible, allowing modifiers to be given to it to determine how things should go. I'd say AuthResult v -> ... was a good default, because it's easy to implement "reject anything unauthenticated" on top of that, whereas with v -> ... you wouldn't be able to take unauthenticated requests and e.g hand back less data, like only public stuffs. But more flexibility using a pattern we've been applying elsewhere seems reasonable.

@tchoutri Food for thought for #1484 I guess.