feat: add accept and reject functions to upgrade hook interface for more explicit auth flow

[LukeHagar]

Related #88

This PR is intended to discuss the interface for the upgrade hook, as it stands right now the usage can be a bit ambiguous, this PR shows what I think is a simpler flow for most people to understand, while also allowing for easier integrations in different environments.

upgrade(req, socket) {
      if (!authorizedCheck(req)) {
        socket.reject("Unauthorized")
      } else {
        socket.accept()
      }
    },

in this example socket is just an object containing two functions, intended to execute callbacks specific to the adapter they are supporting

IE Bun:

async handleUpgrade(request, server) {
        let response: Response | undefined;

        /** Accept the Websocket upgrade request. */
        function accept(params?: { headers?: HeadersInit }): void {
          if (!server.upgrade(request, {
            headers: params?.headers,
            data: {
              server,
              request,
            } satisfies ContextData,
          })) {
            response = new Response("Upgrade failed", { status: 500 });
          };
        }

        /** Reject the Websocket upgrade request */
        function reject(reason: Reasons): void {
          response = formatRejection({ reason, type: "Response" })
        }

        await hooks.callHook("upgrade", request, { accept, reject });
        return response ?? new Response("Upgrade failed", { status: 500 });
      }

This is just an example of what I think would provide a better user experience/easier integration into frameworks.

[pi0]

I appreciate the proposal and PR. Going with a more explicit flow is a good idea.

However I think in order to be more explicit, we should avoid callbacks with side-effects (socket.{accept,reject} can be called in any nested method) and also errors without status code are probably not best idea.

I suggest instead of reject, support throw new Error() / throw new Response() which is explicit and natively propagates from nested functions (like an auth handler).

Current support of return Response is not only for fail-handling but also for fallback (from Upgrade) handling. But we can support 101 status as accept signal if users prefer that (note that web-spec for Response is not even designed for WS upgrade handling nor runtimes allow it today they each have their explicit API).

[LukeHagar]

What about the responses that need to be sent through the socket?

Youll notice for the node adapter for example the response is not being sent back as a close after the upgrade, this was done so the Client gets clear context and data as the why the upgrade was rejected, but I am generally of the opinion that for unauthorized requests the server should do as little work to reject as possible.

I personally love the idea of keeping it response native, I think that would be best, I just want to make sure its a good experience.

[pi0]

I quite like the ReasonMap idea for mapping status codes.

Perhaps we can split it into smaller PRs to progress? Starting with throw support and then another for Response status map.

[LukeHagar]

So throw responses out from the handler, and then return the responses for all platforms but node, and for node map the response to an equivalent socket response?

Here is the thing I'm wondering if we should transition the other adapters to ALSO accept the socket before closing with the clear code, do you think that is worth it for getting clear close frame information on the client?

[pi0]

ideally there should no be difference in API and behqvior between runtimes that’s what mainly matters to me.

Let’s in step one, support throwables for abortion.

In next steps we can support createWebSocketError() that can be exceptionally handled in Node (and potentially others that support) to send clear abort frame.

[LukeHagar]

Sounds good to me, I'll create a set of smaller PRs starting with that first one.

[LukeHagar]

@pi0 The first PR can be found here
#91

This should start with just throwing, If we like this as a start, I can examine another PR that will support the close frames for each adapter.