Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ADR 220 - Quests RPC Service login RFC #229

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

ADR 220 - Quests RPC Service login RFC #229

wants to merge 4 commits into from

Conversation

lauti7
Copy link

@lauti7 lauti7 commented May 4, 2023

This PR adds an ADR about the authentication mechanism for Quest RPC Service

@lauti7 lauti7 requested a review from a team as a code owner May 4, 2023 20:10
@lauti7 lauti7 requested a review from menduz May 4, 2023 20:10
@lauti7 lauti7 changed the title feat: add ADR 220 - Quests RPC Service login feat: add ADR 220 - Quests RPC Service login RFC May 4, 2023
@lauti7 lauti7 changed the title feat: add ADR 220 - Quests RPC Service login RFC ADR 220 - Quests RPC Service login RFC May 4, 2023

## Context, Reach & Prioritization

The Quest RPC Service needs a way to validate who is the user that is requesting to connect to the service in order to know who is the user which is sending the events about a progress on a quest or trying to subscribe to quests' updates. The transport protocol for the service is WebSockets.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The Quest RPC Service needs a way to validate who is the user that is requesting to connect to the service in order to know who is the user which is sending the events about a progress on a quest or trying to subscribe to quests' updates. The transport protocol for the service is WebSockets.
The Quest RPC Service needs a way to validate who is the user that is requesting to connect to the service in order to identify who is sending the events about progress on a quest or trying to subscribe to quests' updates. The transport protocol for the service is WebSockets.


## Specification

The solution we proposed is to use the [Decentraland's AuthChain concept](https://docs.decentraland.org/contributor/auth/authchain/), [dcl-crypto](https://github.com/decentraland/decentraland-crypto-rust) crate, and a signature challenge after the connection upgrading.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The solution we proposed is to use the [Decentraland's AuthChain concept](https://docs.decentraland.org/contributor/auth/authchain/), [dcl-crypto](https://github.com/decentraland/decentraland-crypto-rust) crate, and a signature challenge after the connection upgrading.
The solution proposed in this document is to use the [Decentraland's AuthChain concept](https://docs.decentraland.org/contributor/auth/authchain/), [dcl-crypto](https://github.com/decentraland/decentraland-crypto-rust) crate, and a signature challenge after the connection upgrading.


The solution we proposed is to use the [Decentraland's AuthChain concept](https://docs.decentraland.org/contributor/auth/authchain/), [dcl-crypto](https://github.com/decentraland/decentraland-crypto-rust) crate, and a signature challenge after the connection upgrading.

Once the client opens the connection to the server, the server will sends the signature challenge which consists of a text message with a random unsigned 32-bit number ("signature_challenge_{random_u32}").Then, it will wait 30 seconds for the client to send a response back to the server. The response of the client must be the [AuthChain](https://docs.decentraland.org/contributor/auth/authchain/) containing the sent signature challenge signed. If 30 seconds elapse, the connection will be closed by the server or if the [AuthChain](https://docs.decentraland.org/contributor/auth/authchain/) signature or the message sent by the client is not a valid one, the connection will be also closed by the server.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Once the client opens the connection to the server, the server will sends the signature challenge which consists of a text message with a random unsigned 32-bit number ("signature_challenge_{random_u32}").Then, it will wait 30 seconds for the client to send a response back to the server. The response of the client must be the [AuthChain](https://docs.decentraland.org/contributor/auth/authchain/) containing the sent signature challenge signed. If 30 seconds elapse, the connection will be closed by the server or if the [AuthChain](https://docs.decentraland.org/contributor/auth/authchain/) signature or the message sent by the client is not a valid one, the connection will be also closed by the server.
Once the client opens the connection to the server, the server will send the signature challenge which consists of a text message with a random unsigned 32-bit number (`"signature_challenge_{random_u32}"`). Then, it will wait 30 seconds for the client to send a response back to the server. The client's response must be the [AuthChain](https://docs.decentraland.org/contributor/auth/authchain/) containing the signed sent signature challenge. If 30 seconds elapse without receiving the signature challenge or if the [AuthChain](https://docs.decentraland.org/contributor/auth/authchain/) signature of the message sent by the client is not a valid one, then the connection will be closed by the server.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented May 5, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 4626b6b
Status: ✅  Deploy successful!
Preview URL: https://e23d27f3.adr-cvq.pages.dev
Branch Preview URL: https://quests-ws-auth.adr-cvq.pages.dev

View logs

lauti7 added a commit to decentraland/quests that referenced this pull request May 9, 2023
This PR: 
- implements decentraland/adr#229 even though it
could change
Copy link
Member

@menduz menduz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, maybe mixing two protocols over the same wire is dissonant with other implementations and maybe, may raise some problems in both ends (client&server) to stop handing the "auth protocol" to start using the "rpc" protocol.

Probably using an RpcAuthService.sendChallenge that either enables the rest of the services OR closes the connection may simplify things a lot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants