AIP-X Sign in with Aptos #556
Conversation
|
|
||
| Addressing these challenges is key to unlocking the full potential of decentralized authentication and delivering a seamless, secure, and user-friendly experience. | ||
|
|
||
|  |
There was a problem hiding this comment.
A phishing.com domain should NOT even be allowed to ask the user for a SIWA signature for dapp.xyz.
This picture suggests that it can.
I see no benefit to doing this; only problems. Can we change this please?
| For example, a user may choose to use one of the wallet experiences to authenticate into an application: | ||
|
|
||
| - **Petra** – _Passwords/Biometric_: - Fully non-custodial. Requires no personal information, ensuring complete privacy and resistance to censorship. | ||
| - **Aptos Connect** – _Social Providers_: Fully non-custodial. Shares limited personal information like name and email but may be subject to censorship by social providers. |
There was a problem hiding this comment.
Please clarify who is sharing information with whom! I think you mean that the user is sharing its info (i.e., JWT) with AC.
Like we discussed before, I find this confusing: the chain / dapp does not learn anything, actually.
Of course, the wallet (i.e., AC) sees the JWT and learns your OIDC identity. I wouldn't necesarily call that "personal information" though; it's too broad. It's more helpful to be specific and say what it is: "name, profile picture, email, OIDC sub"
|
|
||
| However, while this approach offers significant benefits, it also presents several challenges: | ||
|
|
||
| 1. **Lack of standardization:** Applications on Aptos lack a unified authentication message format, leading to inconsistencies that confuse users. |
There was a problem hiding this comment.
Missed opportunity: Refer the reader to your screenshot that beautifully showcases this. Showing is better than telling.
| 1. **Lack of standardization:** Applications on Aptos lack a unified authentication message format, leading to inconsistencies that confuse users. | ||
| 2. **Unreadable messages:** Authentication messages are displayed in plaintext, making them difficult for users to comprehend. | ||
| 3. **Limited wallet support:** Without standardization, wallets cannot effectively support authentication UIs. | ||
| 4. **Security risks:** Malicious websites can deceive users into signing messages intended for legitimate dApps. |
There was a problem hiding this comment.
Isn't the signMessage flow domain-separated? Doesn't it include the requesting dapp's URL in the signed message? I guess it is optional?
There was a problem hiding this comment.
Can you point via URL to AIP-62's *MessageInput structs with the optional application field, for extra clarity.
| 2. **Unreadable messages:** Authentication messages are displayed in plaintext, making them difficult for users to comprehend. | ||
| 3. **Limited wallet support:** Without standardization, wallets cannot effectively support authentication UIs. | ||
| 4. **Security risks:** Malicious websites can deceive users into signing messages intended for legitimate dApps. | ||
| 5. **Unintuitive processes:** The traditional `connect` + `signMessage` workflow involves two separate steps, creating a clunky and unintuitive user experience. |
There was a problem hiding this comment.
Explain what are these two steps in parentheses.
There was a problem hiding this comment.
Is it connect + signMessage?
I didn't initially understand how the connect step disappears. I suppose with SIWA, when you click "Connect wallet" you'll just get the signIn request before the wallet is actually connected, thereby merging everything into one step?
If so, this may be worth depicting via pictures & explaining.
| 5. **Unintuitive processes:** The traditional `connect` + `signMessage` workflow involves two separate steps, creating a clunky and unintuitive user experience. | ||
|
|
||
| Addressing these challenges is key to unlocking the full potential of decentralized authentication and delivering a seamless, secure, and user-friendly experience. | ||
|
|
There was a problem hiding this comment.
The image appears suddenly, without any context or explanation.
Maybe move it in its own [sub]section and explain what the image showcases.
| | version | string | | Current version of the message. | | ||
| | statement | string | | Human-readable ASCII assertion that the user will sign. It MUST NOT contain `\n`. | | ||
| | nonce | string | | Randomized token to prevent signature replay attacks. | | ||
| | chainId | string | | Identifier for the network locating the address listed separately above. | |
There was a problem hiding this comment.
"Identifier for the network locating the address listed separately above"
-->
"Identifier for the network where the address above lives (e.g., for Aptos mainnet, the chain ID would be 1)"
|
|
||
| | Name | Type | Mandatory | Description | | ||
| | --------------------- | --------------- | --------- | ------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | fields.domain | string | ✓ | `dnsauthority` that is requesting the signing | |
There was a problem hiding this comment.
From the text, it is not clear: is AptosSignInMessage::fields.domain set to AptosSignInInput::domain?
I am a bit confused as to why you are you re-stating the same fields here.
Is there a less redundant way to describe this? May need to meet to clarify.
| | ----------------- | ------------- | --------- | ---------------------------------------------------------------------------------------------------------------------------------- | | ||
| | account.address | string | ✓ | The address of the user that is signing in. | | ||
| | account.publicKey | PublicKey | ✓ | The public key of the user that is signing in. | | ||
| | message | SignInMessage | ✓ | A SignInMessage object that contains information about the message that was signed, it's signature, and plain text representation. | |
There was a problem hiding this comment.
Is this SignInMessage or AptosSignInMessage?
| | fields.requestId | string | | System-specific identifier used to uniquely refer to the authentication request. | | ||
| | fields.resources | string[] | | List of information or references to information the user wishes to have resolved as part of the authentication by the relying party. | | ||
| | plainText | string | ✓ | The constructed message using the standardized message format. | | ||
| | signature | bytes | ✓ | The signature of the message. This must conform to the domain separated signing algorithm. | |
There was a problem hiding this comment.
So there's a signature here over (some serialization of) AptosSignInMessage?
| | account.address | string | ✓ | The address of the user that is signing in. | | ||
| | account.publicKey | PublicKey | ✓ | The public key of the user that is signing in. | | ||
| | message | SignInMessage | ✓ | A SignInMessage object that contains information about the message that was signed, it's signature, and plain text representation. | | ||
| | signature | Signature | ✓ | The signature of the message. | |
There was a problem hiding this comment.
If we already had a signature above over (some serialization of) AptosSignInMessage, then what is this signature?
|
|
||
| ``` | ||
| example.com wants you to sign in with your Aptos account: | ||
| 0x0000000000000000000000000000000000000000000000000000000000000001 |
There was a problem hiding this comment.
I don't think 0x000...0001 is a good example here: it will never arise in practice.
| - resource2 | ||
| ``` | ||
|
|
||
| ### Augment Backus-Naur Form (ABNF) Expression |
| Chain ID: mainnet | ||
| ``` | ||
|
|
||
| _Detailed Example_ |
There was a problem hiding this comment.
It would be very helpful if this AIP described when field like nonce should be set in the message.
This section on examples could be a good place to do that: go over a few sample messages and detail an application where that message suffices. I am actually wondering where the "minimal example" above would actually arise.
| message-resources = \\*( LF "- " URI ) | ||
| ``` | ||
|
|
||
| ### Domain Separating Signing Algorithm |
There was a problem hiding this comment.
Domain-separated signing algorithm
| ``` | ||
| sha3_256(b"SIGN_IN_WITH_APTOS::") || <siwa_message> | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Maybe add the following clarification:
where
<siwa_message>is a sign-in with Aptos message formatted as explained in "Message Format" above.
|
|
||
| ### Wallet Standard | ||
|
|
||
| The [Wallet Standard](https://github.com/aptos-labs/wallet-standard) is a chain-agnostic set of interfaces and conventions that aim to improve how applications interact with injected wallets. |
There was a problem hiding this comment.
Are you explaining what the wallet standard is because you are about to propose a modification/extension of it?
If so, maybe clarify from the beginning:
"This AIP also modifies / extends the wallet standard, which is a chain-agnostic set of [...]"
|
|
||
| #### Suggested Features | ||
|
|
||
| The features below will be added to the wallet as suggested features. Features will be optional to maintain compatibility with existing wallets. |
There was a problem hiding this comment.
*to the wallet standard
no?
|
|
||
| ### Wallet Implementer | ||
|
|
||
| To ensure the safety of our users, we want to take a conservative approach when allowing users to sign potentially harmful messages. |
There was a problem hiding this comment.
We can't be both "allowing users to sign potentially-harmful messages" and "take a conservative approach."
They are kind of in contradiction 😛
I propose we nix the 1st and only do the 2nd :)
|
|
||
| #### Bound Fields Verification | ||
|
|
||
| The following fields MUST be verified to prevent phishing attacks: `domain` , `address` , `uri` , `version` , and `chainId` . If the fields do not match the signing request, errors must be displayed to the users detailing them out. It is suggested to add additional steps to the approval process, such as an opt-in checkmark field, that confirms the user’s intent if errors are present. |
There was a problem hiding this comment.
Saying that "fields MUST be verified" is not precise:
- Who is doing the verification?
- What exactly is this verification for each field?
|
|
||
| The following fields MUST be verified to prevent phishing attacks: `domain` , `address` , `uri` , `version` , and `chainId` . If the fields do not match the signing request, errors must be displayed to the users detailing them out. It is suggested to add additional steps to the approval process, such as an opt-in checkmark field, that confirms the user’s intent if errors are present. | ||
|
|
||
| A wallet may also decide to prevent the approval of invalid signing requests. |
There was a problem hiding this comment.
This should be a MUST, not a may.
|
|
||
| #### @aptos-labs/siwa | ||
|
|
||
| A new package named `@aptos-labs/siwa` will be created to house the utilities for the _Sign in with Aptos_ standard. These utilities will be responsible for creating messages, verifying messages, and deserializing public key and/or signatures according. |
There was a problem hiding this comment.
NPM package? What kind of package?
|
|
||
| ```ts | ||
| /** | ||
| * Create a SignIn message text from the input following the ABNF format defined in the Sign in with Aptos |
There was a problem hiding this comment.
Pro-tip: In technical writing, you always want to refer to the same thing by one name. It can't be either of SignMessage, AptosSignInMessage or SignIn message. You have to pick one and use it consistently. This way, the term is drilled down into your reader's head and they will know what you mean every time.
| function createSignInMessageText(input: AptosSignInMessage["fields"]): string; | ||
|
|
||
| /** | ||
| * Verifies an input SignIn message against expected fields. |
|
|
||
| ### AptosSignInOutput | ||
|
|
||
| When the user has approved and signed the message, the wallet will respond with an `AptosSignInOutput` to the application. The application should then use this response to verify if the outputs are valid with the inputs. |
There was a problem hiding this comment.
Unclear what this means: "verify if the outputs are valid with the inputs."
| ): VerificationResult<AptosSignInMessage["fields"]>; | ||
|
|
||
| /** | ||
| * Generate a signing message using the Sign in with Aptos signing algorithm. |
There was a problem hiding this comment.
Is a "signing message" different than a "[Aptos]SignInMessage"?
If so, did the AIPs text properly introduce both & differentiate between them? I don't think "signing message" has been introduced.
| * Generate a signing message using the Sign in with Aptos signing algorithm. | ||
| * sha3_256( sha3_256(b"SIGN_IN_WITH_APTOS::" ) || <message> ) | ||
| * | ||
| * @param message The SIWA message to sign. |
There was a problem hiding this comment.
Again, what is "SIWA message"? It looks like another unnecessary synonym.
|
|
||
| #### @aptos-labs/wallet-standard | ||
|
|
||
| The following interfaces would be introduced into the `@aptos-labs/wallet-standard` package. |
There was a problem hiding this comment.
Include a link to it here, please.
| resources?: string[]; | ||
| }; | ||
|
|
||
| export type AptosSignInMessage = { |
There was a problem hiding this comment.
I think you should solely use this AptosSignInMessage term throughout the text, instead of synonyms for it.
|
|
||
| ### Wallet Implementation | ||
|
|
||
| The wallet is responsible for taking `AptosSignInInput` fields defined by an an application and returning a response object with a signature, plain text representation of the message, and account information. |
There was a problem hiding this comment.
What is a "response object"? Again, no synonyms.
|
|
||
| The wallet is responsible for taking `AptosSignInInput` fields defined by an an application and returning a response object with a signature, plain text representation of the message, and account information. | ||
|
|
||
| The wallet must bind the `address`, `chainId` , `domain` , `uri` , and `version` fields if not present. If there is a mismatch between these fields and the ones defined by the application, it is important that it is flagged and shown to the user. |
There was a problem hiding this comment.
Not sure I understand what "bind" means. You mean "set to the correct value"?
There was a problem hiding this comment.
Again, "flagged" is not good enough. It must be forbidden.
| } | ||
| ``` | ||
|
|
||
| 2. The Wallet will construct a `AptosSignInMessage` with the input. The `createSignInMessageText` utility from `@aptos-labs/siwa` can be used. Once the plaintext representation of the message has been constructed, we want to generate a signing message using `generateSignInSigningMessage` that conforms to _Sign in with Aptos_’s domain separated signing algorithm. **It is important to bind the following fields: `address`, `chainId` , `domain`, `uri` , and `version`.** |
There was a problem hiding this comment.
Typo: will construct an AptosSignInMessage
There was a problem hiding this comment.
As mentioned before, not sure if bind is the right word here. If it is, it may be worth clarifying this earlier in the AIP.
|
|
||
| ### Application Implementation | ||
|
|
||
| The application will be responsible for sending the `signIn` request with `AptosSignInput` fields to the wallet. The application should typically create the fields in the backend and store them to verified at the end of the flow. |
There was a problem hiding this comment.
Typo: AptosSignInput --> AptosSignInRequest
There was a problem hiding this comment.
A couple of high level thoughts:
AptosSignInInputis not as descriptive of a name asAptosSignInRequest, no? It is a request, no?- I propose you replace
AptosSignInwithSiWAorSIWAeverywhere. Not only less verbose, but also more to the point: this AIP is about SIWA. - There is quite a bit of terminology here and the AIP currently uses too many names for the same term. This has to be fixed: one term, one name.
- You may even want to have a section where you overview the terminology. Not sure.
- I think the "High-level overview" should actually overview what is happening in SIWA: dapp makes a request to a wallet (in a certain format), wallet displays a prompt to the user, user clicks approve, wallet signs (in a certain format), dapp gets signature.
There was a problem hiding this comment.
I should've also highlighted the things you did very well when writing this AIP:
- Well-structured
- Writing is generally good
- Figures / screenshots / illustrations 👌👌👌
- Nice tables describing the formats
- You included a formal description of the grammar
- Code examples
- Lots of examples in general
|
|
||
| The wallet must bind the `address`, `chainId` , `domain` , `uri` , and `version` fields if not present. If there is a mismatch between these fields and the ones defined by the application, it is important that it is flagged and shown to the user. | ||
|
|
||
| #### Overview |
There was a problem hiding this comment.
I found this hard to read / understand. Not a TS guy so that may be why. But there may also be too many things here. Do you need all of this? Can anything be left out / simplified?
There was a problem hiding this comment.
This is probably otherwise a very good section for TS devs / wallet devs / dapps devs.
|
|
||
| ## Testing | ||
|
|
||
| The new `@aptos-labs/siwa` package will have its utilities tested to ensure that validation is done correctly. Coverage is most important here. In addition, signature verification methods will be used from the `@aptos-labs/ts-sdk`, it's important that public key verification methods are supported and tested. |
There was a problem hiding this comment.
it's important that public key verification methods are supported and tested
What are you referring to here?
There was a problem hiding this comment.
After convo: You mean "signature verification" (which takes a message m, a public key pk and a signature \sigma as input and outputs either 0 or 1)
|
|
||
| ## Risks and Drawbacks | ||
|
|
||
| The ecosystem is responsible for adopting the new standard as an alternative to the `connect` + `signMessage` flow. If applications/wallets do not properly implement the `signIn` flow, they may suffer vulnerabilities that can lead to potential phishing, replay, or forgery attacks. If vulnerabilities are found within _Aptos Labs_ or ecosystem utilities, it’s important to patch and deprecate them as soon as possible. Applications that accept the `connect` + `signMessage` flow may still be vulnerable to security attacks. |
There was a problem hiding this comment.
If applications/wallets do not properly implement the
signInflow, they may suffer vulnerabilities that can lead to potential phishing, replay, or forgery attacks.
I think the AIP actually needs to discuss ways in which applications can mess this up. Ideally, there should not be (m?)any.
There was a problem hiding this comment.
If vulnerabilities are found within Aptos Labs or ecosystem utilities, it’s important to patch and deprecate them as soon as possible.
This is self-evident. Not sure why you are stating it here.
There was a problem hiding this comment.
Regarding #1:
I think the only opportunities to mess this up are:
- the application does not pick the
noncerandomly (but even this the TS SDK could fix, by not even letting the application pick the nonce, no?) - the session management via cookies
|
|
||
| The attack vectors that must be accounted for are the following: | ||
|
|
||
| - Phishing attacks — A user must have visibility on signing a message that is for a different application. |
There was a problem hiding this comment.
Such a phishing prompt should never be shown to the user. (So, really, the user should not have any visibility into this.)
|
|
||
| ## Future Potential | ||
|
|
||
| This AIP formally introduces a standard for authentication via Aptos decentralized identities. In the future, the standard may be extended to provide additional information from the wallet. |
There was a problem hiding this comment.
"via Aptos decentralized identities."
This is not precise. Unclear what "decentralized identity" means. It's a fluff umbrella term, AFAICT.
"via Aptos accounts" would be precise.
|
|
||
| This AIP formally introduces a standard for authentication via Aptos decentralized identities. In the future, the standard may be extended to provide additional information from the wallet. | ||
|
|
||
| With the rise of Keyless on the Aptos Blockchain, wallet connections may want to be extended to provide additional personal information about the user, this would include full names, phone numbers, or emails. |
There was a problem hiding this comment.
This could actually be made possible using the extra_field feature of the keyless ZKP (see AIP-61).
There was a problem hiding this comment.
(no need to riff on this in this AIP)
|
|
||
| ## High-level Overview | ||
|
|
||
| **Decentralized authentication** is a privacy-first solution that empowers individuals to safeguard their information, personalize their experiences, and maintain freedom from censorship. |
There was a problem hiding this comment.
I don't think this AIP is about decentralized authentication, nor privacy-first anything...
- there are no privacy benefits here over the
connect+signInbaseline, right? - "Decentralized authentication" sounds like a vague umbrella term to me
This AIP is about users signing into a dapp using their Aptos account and, as a result, getting access to protected resources.
| | signature | bytes | ✓ | The signature of the message. This must conform to the domain separated signing algorithm. | | ||
| | type | string | ✓ | The public key scheme used to sign the message (e.g. 'ed25519', 'single_key', 'multi_key', 'multi_ed25519', etc.) | | ||
|
|
||
| ### AptosSignInOutput |
There was a problem hiding this comment.
I don't like this generic "output" name. Shouldn't this be something more like a AptosSignInSignature or SiwaSignature?
| sha3_256(b"SIGN_IN_WITH_APTOS::") || <siwa_message> | ||
| ``` | ||
|
|
||
| ### Wallet Standard |
There was a problem hiding this comment.
Retitle: Modifications to the wallet standard
| signIn(input: AptosSignInInput): Promise<UserResponse<AptosSignInOutput>> | ||
| ``` | ||
|
|
||
| ### Wallet Implementer |
There was a problem hiding this comment.
Retitle: How to implement this AIP safely as a wallet developer
| | uri | string | | URI referring to the resource that is the subject of the signing i.e. the subject of the claim. | | ||
| | version | string | | Current version of the message. | | ||
| | statement | string | | Human-readable ASCII assertion that the user will sign. It MUST NOT contain `\n`. | | ||
| | nonce | string | | Randomized token to prevent signature replay attacks. | |
There was a problem hiding this comment.
This feels like it should be mandatory, AFAIU.
Description
Initial draft for Sign in with Aptos, a standard for secure and user-friendly way to authenticate to off-chain resources using Aptos blockchain addresses.