DPop key should never leave the server

[matthieusieben]

DPoP keys are meant to protect the access-token in transit (from attacks like heartbleed, etc.). This is only efficient if the DPoP key is never sent over the wire.

The current implementation stores the OAuth session data in (encrypted, but still) cookies, which goes against this:

npmx.dev/server/api/auth/atproto.get.ts

Lines 64 to 65 in b73edaf

	const session = await useServerSession(event)
	const { stateStore, sessionStore } = useOAuthStorage(session)

The same is also true for "state data". The State store is used to keep track of the state of a particular oauth flow. In particular, it should allow to prevent replays. Being stored in cookies, replays can be made possible.

Both SessionStore and StateStore should be using backend databases to store their data.

[matthieusieben]

Using cookies to store the oauth data has the advantage of ensuring that the right browser is using performing the oauth callback.

IF the storage of oauth data is moved to a backend database (as it should), the oauth callback logic should enforce that the right browser is performing the redirect.

This is what #1327 does.

[matthieusieben]

Note that using cookies to store session data means that multiple parallel incoming HTTP requests might trigger multiple refreshes of the same oauth refresh token. Normally, this is protected using the requestLock (see getOAuthLock). However, because all parallel request will have the same exact cookie value, the refresh will be performed using the same refresh token multiple times. This will cause the AS to invalidate the OAuth session. Reading oauth session data from a central place allows to ensure proper handling of parallel refresh attemts.

[matthieusieben]

Note that an alternative solution would be to use browser side oauth sessions (instead of server side), using the OAuthClientBrowser.

[fatfingers23]

Thanks for the PR earlier and this! After the discussion in #1327 do you still think it would be best to move it back to the backend again especially since we are going to add it to be a confidential client for the longer sessions?

edit: Took some more time to read over it all and look at some previous OAuth implementation's I've done. Going get it all moved back server side again. Thanks again for checking over everything! Really want to make sure I nail the atproto stuff for npmx

[matthieusieben]

Yes, the PR was complementary to fixing this issue!

[matthieusieben]

Feel free to ping me as reviewer