Add hostname and extensionId to site metadata #7218
Conversation
|
@coinsquareMike is this a suitable alternative to #7169 ? Let me know if this meets your needs, or if anything is still missing. The title is pretty ugly right now for an extension (it has the full origin, including protocol). I might switch things around to use the hostname as the title instead, and use the full origin for the light-grey text below the title. |
Builds ready [e63cb33]
|
|
@Gudahtt This does work, however, the resulting UI for the approval is
However, it is a good starting point. Additional work can be done on the UI side to show "An extension" when that For now though, this absolutely resolves the issue. |
|
@coinsquareMike great, thanks for the feedback! I'll see if i can improve the UI before this is merged. |
Gudahtt
force-pushed
the
add-hostname-and-extension-id-to-site-metadata
branch
2 times, most recently
from
1dea2a9 to
2e2c620
Compare
Builds ready [2e2c620]
|
Gudahtt
force-pushed
the
add-hostname-and-extension-id-to-site-metadata
branch
from
2e2c620 to
69a8aec
Compare
|
The UI has been updated to use the title
I think this is good enough for the initial implementation. It definitely could use more emphasis on the extension ID so that the user knows that's the only trustworthy piece of information, and maybe a link to a help article explaining how to verify it. |
Builds ready [69a8aec]
|
If the extension ID is set, an alternate title and subtitle are used for the Connect Request screen. The title is always `External Extension`, and the subtitle is `Extension ID: [id]` instead of the origin (which would just be `[extension-scheme]://[id]` anyway). The hostname for the site is used as a fallback in case it has no title. The artificial hostname set for internal connections has been renamed from 'MetaMask' to 'metamask' because URL objects automatically normalize hostnames to be all lower-case, and it was more convenient to use a URL object so that the parameter would be the same type as used for an untrusted connection.
Gudahtt
force-pushed
the
add-hostname-and-extension-id-to-site-metadata
branch
from
69a8aec to
4aa297d
Compare
Builds ready [4aa297d]
|
|
These changes are mostly incompatible with the site metadata handling in #7004. There, site metadata retrieval has been moved completely to What we do about that in relation to this PR depends on when we intend to merge #7004. I think almost all of what's done here will need to be moved/re-implemented when we do. |
|
Generally I don't think moving the metadata retrieval to the The changes to the |
The site metadata was updated in #7218 to include the extension id of the extension connecting to MetaMask. This was done to allow external extensions to connect with MetaMask, so that we could show the id on the provider approval screen. Unbeknownst to me at the time, the extension id was being set for all connections to MetaMask from dapps. The id was set to MetaMask's id, because the connections are made through MetaMask's contentscript. This has been updated to only set the id when accepting a connection from a different extension.
In #7218 a few things were added to the site metadata, so the provider approval controller was middleware was updated to accept the site metadata as an object rather than accepting each property as a separate parameter. Unfortunately we failed to notice that the site name and icon were named differently in the site metadata than they were in the provider approval controller, so the names of those properties were unintentionally changed in the controller state. The provider approval controller has been updated to restore the original property names of `siteTitle` and `siteIcon`. An unused prop that was added to the provider approval page in #7218 has also been removed.
* Omit MetaMask `extensionId` from site metadata The site metadata was updated in #7218 to include the extension id of the extension connecting to MetaMask. This was done to allow external extensions to connect with MetaMask, so that we could show the id on the provider approval screen. Unbeknownst to me at the time, the extension id was being set for all connections to MetaMask from dapps. The id was set to MetaMask's id, because the connections are made through MetaMask's contentscript. This has been updated to only set the id when accepting a connection from a different extension. * Fix `siteMetadata` property names In #7218 a few things were added to the site metadata, so the provider approval controller was middleware was updated to accept the site metadata as an object rather than accepting each property as a separate parameter. Unfortunately we failed to notice that the site name and icon were named differently in the site metadata than they were in the provider approval controller, so the names of those properties were unintentionally changed in the controller state. The provider approval controller has been updated to restore the original property names of `siteTitle` and `siteIcon`. An unused prop that was added to the provider approval page in #7218 has also been removed.
* Omit MetaMask `extensionId` from site metadata The site metadata was updated in #7218 to include the extension id of the extension connecting to MetaMask. This was done to allow external extensions to connect with MetaMask, so that we could show the id on the provider approval screen. Unbeknownst to me at the time, the extension id was being set for all connections to MetaMask from dapps. The id was set to MetaMask's id, because the connections are made through MetaMask's contentscript. This has been updated to only set the id when accepting a connection from a different extension. * Fix `siteMetadata` property names In #7218 a few things were added to the site metadata, so the provider approval controller was middleware was updated to accept the site metadata as an object rather than accepting each property as a separate parameter. Unfortunately we failed to notice that the site name and icon were named differently in the site metadata than they were in the provider approval controller, so the names of those properties were unintentionally changed in the controller state. The provider approval controller has been updated to restore the original property names of `siteTitle` and `siteIcon`. An unused prop that was added to the provider approval page in #7218 has also been removed.
External extensions can connect to MetaMask by using `postMessage` directly. When this is done, the extension id of the sender is accessible. This extension id has been added to the metadata for the extension, so that we're able to recognize it as an extension rather than a dapp. This was originally added in #7218. The controller with this logic was replaced in LoginPerSite, so it needed to be re-added.
The hostname is now used as a fallback in case the siteTitle is missing from the metadata (as it would be for connections with extensions).
The artificial hostname set for internal connections has been renamed from 'MetaMask' to 'metamask' because URL objects automatically normalize hostnames to be all lower-case, and it was more convenient to use a URL object so that the parameter would be the same type as used for an untrusted connection.