-
Notifications
You must be signed in to change notification settings - Fork 22
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
feat: Verify Server API spec NG #173
base: main-lfs-DO-NOT-USE
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
- `201` - Attestation registered | ||
- `400` - Invalid request body or `integrityToken` format | ||
- `401` - Failed to verify the `integrityToken` according to ["Decrypt and verify the integrity verdict"](https://developer.android.com/google/play/integrity/standard#decrypt-and) | ||
- `403` - Bundle ID is not registered on [WalletConnect Cloud](https://cloud.walletconnect.com) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Cali93 can you add specs for the Explorer endpoint, where dapp's metadata URL is returned based on the bundle_id?
- `201` - Attestor registered | ||
- `400` - Invalid request body or `attestationObject` format | ||
- `401` - Failed to verify the `attestationObject` according to ["Verify the Attestation"](https://developer.apple.com/documentation/devicecheck/validating_apps_that_connect_to_your_server#3576643) | ||
- `403` - App ID is not registered on [WalletConnect Cloud](https://cloud.walletconnect.com) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Cali93 can you add specs for the Explorer endpoint, where dapp's metadata URL is returned based on the app_id?
I think when making app_id unique in Explorer, we don't have to send the project_id
@llbartekll can two apps on AppleStore have the same apple_id?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ofc not
### Android | ||
|
||
#### Request | ||
|
||
`GET /attestation/:id` | ||
|
||
#### Responses | ||
|
||
- `200` - Attestation exists | ||
- `bundleId` - Bundle ID of the Android dApp that registered this attestation | ||
```jsonc | ||
{ | ||
"bundleId": string | ||
} | ||
``` | ||
- `404` - Attestation doesn't exist | ||
|
||
### iOS | ||
|
||
#### Request | ||
|
||
`GET /attestation/:id` | ||
|
||
#### Responses | ||
|
||
- `200` - Attestation exists | ||
- `appId` - App ID of the iOS dApp that registered this attestation | ||
```jsonc | ||
{ | ||
"appId": string | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GET /attestation/:id
should be one endpoint across all platforms (web, iOS, Android) because a wallet doesn't know if a given request comes from web, iOS or Android Dapp.
This assumption forces having one response:
{
"origin": string // for native it'll be metadata URL returned from the Explorer
"bindleId": string?, //optional - it's android bundle_id or iOS app_id
"isScam": boolean? //optional - for native it can be true when attestation exists but it indicates that dapp is not legit
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, so Android
<-> iOS
doesn't make sense, obviously. Valid combinations of dApp -> Wallet are:
- Web -> Anything? (is Web wallet a thing?)
- Android -> Android
- iOS -> iOS
So technically the responses should be:
- iOS -
(origin, isScam) | appId
- Android -
(origin, isScam) | bundleId
We don't have scam checking for non-Web dApps.
Makes sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, as we want to return a sum type maybe we should have those fields inside an object that explicitly defines the attestation source.
{
"web": { "origin": "..", "isScam": false }
"android": { "bundleId": ".." }
"ios": { "appId": ".." }
}
The response would contain only 1.
Android expects web
and android
, iOS expects web
and ios
Adding more platforms doesn't seem to break this approach. Because generally a platform cares only about a dApps from it's own platform + web
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for iOS we can use bundleId too but I am still not sure this approach makes sense
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I expect the field set to evolve independently, so it should be more easy to comprehend the differences between platforms. But you are the one who's going to consume it, so it's your call.
Technically, from the backend perspective I don't care whether it's a tagged or untagged union.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I also don't see reasons for it now, we can skip it and add later
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How Android
<-> iOS
is possible?
A wallet shouldn't be aware of dapp's platform, it should only know if a given request is verified, unknown, scam, mismatched
This abstraction already leaks anyway, because native wallet needs to know about both origin
and bundleId
and bundleId
may be null.
If you really want to abstract this away, then I should probably do it server side. And just return you a boolean or something.
Also, we can't change the existing GET /attestation:id response, because it'll break existing users
Good point
Okay, if you guys want an object with optional fields, so be it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, so you don't actually need the bundleId
but do need origin
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's give it a few more days 😅 I still feel like we haven't broken it yet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, so you don't actually need the bundleId but do need origin?
It seems so but we definitely need origin from Explorer
- `201` - Attestor registered | ||
- `400` - Invalid request body or `attestationObject` format | ||
- `401` - Failed to verify the `attestationObject` according to ["Verify the Attestation"](https://developer.apple.com/documentation/devicecheck/validating_apps_that_connect_to_your_server#3576643) | ||
- `403` - App ID is not registered on [WalletConnect Cloud](https://cloud.walletconnect.com) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ofc not
Couple of steps that have to happen on VerifyServer for Android attestation:
cc @xDarksome Reference: https://developer.android.com/google/play/integrity/verdicts |
@jakubuid sounds good
I would rather do this check on |
@xDarksome |
Updates Verify Server API spec to support Native dApp attestation