[Feat] Request overriding _attachHandler() for custom authentication middleware #97

Hi again,

I would like to apologize beforehand for the wall of text below, but I was wondering if you could help me with some aspects of implementing this. 😅

I'm reworking my custom authentication handler, and I need to detect whether my Authentication handler needs to be called or not. One of the conditions is that the route actually exist, as I don't want to protect non-existent routes (think of robots.txt, favicon.ico, need to get 404).

Some context

Upon closer look it seems that ESPAsyncWebServer:

• Handles the routes that are assigned using .on('/url', ...) by creating a AsyncWebHandler of type AsyncCallbackWebHandler and add it to the list of _handlers that canHandle() the request.
• There is aslo onNotFound() that modify a special handler called _catchAllHandler on how it needs to behave when no other handlers are found, e.g. no routes are matched.

This is excellent, because to quote the docs:

[...] the server goes through all attached Handlers (in the order they were added) trying to find one that canHandle the given request. If none are found, the default (catch-all) handler is attached.

Now my AuthenticationHandler needs something to detect if the Handler that comes after is the _catchAllHandler or not. This means that I need to override the _attachHandler() (the method that is running through all Handlers to find one that canHandle()) to modify its logic a bit so that if there is a handler that can handle the request, the custom AuthenticationHandler is called first, otherwise the catch all handler is called, where we can respond to OPTIONS requests and for other requests send a 404.

Proposed change

Can you please either:

• Allow _attachHandler() to be overridable
• Introduce a new pointer function that would be called in the appropriate moment so that I can attach the AuthenticationHandler (prefrered)

Or, introduce a new method (such as addMiddleware(), or addHandler()) with a custom parameter) that would change the order that the handler can be attached.

Alternative approach

To avoid asking a potential XY Problem, let's see what the ideal approach for this problem could be.

I am trying to implement this custom handler, as this project doesn't rely on a username/password combo for authentication, instead it relies on a auth token (that optionally can be acquired through a login process with a username/password, but it's irrelevant to the authentication step). Think of it as an API key.

Now, ideally, there would be one place to manage and put the actual authentication logic, but it appears that this is spread all through the code.

• Regular requests

  if ((_username != "" && _password != "") && !request->authenticate(_username.c_str(), _password.c_str()))
    return request->requestAuthentication();

• WebSockets

  if ((_username.length() && _password.length()) && !request->authenticate(_username.c_str(), _password.c_str())) {
    return request->requestAuthentication();
  }

Each handler is separately looking for the username/password, and if set, individually trying to authenticate and respond with a requestAuthentication().

WebSocket and SSE go one step even further by implementing a custom _handshakeHandler():

• Add Custom Handshake Handlers for Websocket and EventSource Plugins me-no-dev/ESPAsyncWebServer#684

This is actually useful, but besides being dandy and all, has some drawbacks:

• The authentication method is restricted only to a username/password combo, with an additional token-based auth only for WS/SSE step
• The check for authentication is made in several places, and no control is given to the user regarding how, when and in which conditions it should be called
• There is a hard coded response code of 401 always being returned in case of an un-authenticated request, disallowing the user from using other types of responses (e.g. a 302 redirect, or a JSON response)

I would be happy if the request->authenticate() and request->requestAuthentication() part was actually overridable, I bet the original onHandshake() of WS/SSE wouldn't even be needed to be implemented if this was the case.

However, as this is not the case, the next best thing would be to completely forgo the built-in authentication handler in AsyncCallbackWebHandler/AsyncWebSocket and the one for Server Events, and instead implement a custom one from scratch; however it needs to be called in the right moment, hence the original request above.

With that being said, I would also appreciate it, if there was a re-factor to move all the duplicated request->authenticate()/request->requestAuthentication() calls to a shared method that would be overridable by the user, allowing us to attach the custom authentication handler.

Once again I am sorry for this wall of text, but these were my thoughts when reviewing the code for the library, and as I have previously said, I'm invested in a good and clear solutions that can be benefited from in the long run, instead of just patching the library for my own use case.

Thanks for taking time to read this, @mathieucarbou, I deeply appreciate your thoughts and feedback. 👍