feat: Authenticated Fetch

[MarcoPolo]

Title

Description

Adds the ability to send application request earlier in the handshake, after authenticating the server (but before the server has authenticated us).

Notes & open questions

• How do we feel about this API? What about the naming of checkID? I iterated a bit here, and found this API would avoid some duplication and traps in other approaches. But I'm open to a different api.

Change checklist

• I have performed a self-review of my own code
• I have made corresponding changes to the documentation if necessary (this includes comments as well)
• I have added tests that prove my fix is effective or that my feature works

[achingbrain]

How do we feel about this API?

• I think switching the request arg to be a Request object makes the users's job a bit more difficult since they now can't use the fetch(resource: string | URL, options?: RequestIinit) signature, it's probably better to try to replicate the fetch API for familiarity.
• Making the checkId function mandatory increases friction in the case where the caller doesn't know the remote's PeerID ahead of time or doesn't care (such as when the remote needs to authenticate the caller, not the other way round).

I think there's a lot of value in presenting developers with a familiar API so a nice one would make this all transparent.

Either something like configuring the (optional) remote peer verification ahead of time:

import { authenticatedFetch } from '@libp2p/authenticated-fetch'

const fetch = authenticatedFetch(privateKey, {
  verifyPeer: async (peerId: PeerId, request: Request, options: AbortOptions) => {
    return true
  }
})

// just like regular fetch
const result = await fetch('http://example.com', {
  method: 'POST',
  body: 'hello',
  signal: AbortSignal.timeout(5000)
})

or possibly more flexibly by extending the RequestInit arg (again, optional):

import { authenticatedFetch } from '@libp2p/authenticated-fetch'

const fetch = authenticatedFetch(privateKey)

// almost just like regular fetch
const result = await fetch('http://example.com', {
  method: 'POST',
  body: 'hello',
  signal: AbortSignal.timeout(5000),
  verifyPeer: async (peerId: PeerId, options: AbortOptions) => {
    return true
  }
})

We could also do both - accept an option to the fetch function with fall back to any passed to the factory.

I've put the above as @libp2p/authenticated-fetch because maybe now this is something separate from this module, that this module could reuse?

---

One other thing is that if the Request object is consumed, and the body is a ReadableStream, we won't be able to re-use it for a subsequent request because we can't start reading from the stream again.

If the body is a ReadbleStream I guess we need to either tee the stream to make it reusable (this will negatively impact memory usage when the stream is very large), or we should do something like an OPTIONS or HEAD request first to do the authentication.

OPTIONS may be slightly better than HEAD as it's not cachable and HEAD implies GET which might be disallowed by the resource. The server might need to be set up to handle these verbs.

[MarcoPolo]

I think switching the request arg to be a Request object makes the users's job a bit more difficult since they now can't use the fetch(resource: string | URL, options?: RequestIinit) signature, it's probably better to try to replicate the fetch API for familiarity.

Agreed

Making the checkId function mandatory increases friction in the case where the caller doesn't know the remote's PeerID ahead of time or doesn't care (such as when the remote needs to authenticate the caller, not the other way round).

I'm a bit torn here. It does increase friction, but isn't this something that we want the user to think about? Even if they think "I don't care", that's probably better than "wasn't I suppose to check the id somewhere?"

---

Of the two APIs, I prefer the second as it doesn't involve creating a new fetch for every peer. I've made these changes.

---

One other thing is that if the Request object is consumed, and the body is a ReadableStream, we won't be able to re-use it for a subsequent request because we can't start reading from the stream again.

I think this would only happen once: in the case where our bearer token is expired or rejected by the server. (well twice in the case of redirects, but that should hopefully be caught in the first handshake message). Remember that we don't send the request body on the first (unauthenticated) handshake message.

We don't have a standard way to ask the user to give us a body factory for us to retry the request (Go's http package has a GetBody field for this. Which makes me wonder how redirects work in fetch?[1]). So our options as I understand them are:

1. Duplicate the body ourselves. As you say, there's a lot of perf downsides here and complexity.
2. Add an extra round trip with "OPTIONS", but this might not even solve the problem since the token might become expired between this request and the next.
3. Bubble up an error to the user requesting a retry.

Of the options 3 feels annoying, but might be the simplest option for now.

---

[1]: Went for a little detour as I wrote this:

1. The fetch standard says that you should return a network error:

If internalResponse’s status is not 303, request’s body is non-null, and request’s body’s source is null, then return a network error.
https://fetch.spec.whatwg.org/#http-redirect-fetch

2. Safari doesn't even support posting with a readable stream. Returns the error "Error: ReadableStream uploading is not supported"
3. Firefox stringifies the ReadableStream (??) before sending it? so both the original endpoint and redirected endpoint see "[object ReadableStream]"
4. Chrome does what the standard says and returns a network error (net::ERR_INVALID_ARGUMENT).

[achingbrain]

LGTM, just a few last bits to tidy up.

[achingbrain]

What's the thinking behind making the VerifyPeerOptions a single-member interface?

Certainly the generated docs are easier to read if the options types accepted by methods are not unions, which is the approach taken elsewhere in the JS stack.

This also avoids a breaking change here due to the removal of extends AbortOptions which means any signal prop passed as a member of an object of this type would fail to compile.

Suggested change

	export interface AuthenticateServerOptions {
	export interface AuthenticateServerOptions extends RequestInit {
	/**
	* Accepts the PeerId of the server, return true if it was the expected
	* peer or false if not.
	*/
	verifyPeer?(peerId: PeerId, options: AbortOptions): boolean | Promise<boolean>

This suggestion implies also removing the VerifyPeerOptions interface and removing the AbortOptions union from the authenticateServer method definition.

[MarcoPolo]

No Opinion here. I'll defer to your opinion on what is more ergonomic to use.

[github-actions]

🎉 This PR is included in version 2.1.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀