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.

Sign up for GitHub

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

Jump to bottom

feat: Verify Server API spec NG #173

Open
wants to merge 1 commit into
base: main-lfs-DO-NOT-USE
Choose a base branch
from
Open

feat: Verify Server API spec NG #173

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
155 changes: 147 additions & 8 deletions docs/specs/servers/verify/verify-server-api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,30 +1,169 @@
# Verify Server API

## Generate Challenge (iOS only)

Generates a unique one-time challenge to be later used with other requests as a mean to prevent replay attacks.

### Request

`POST /challenge`
llbartekll marked this conversation as resolved.
Show resolved Hide resolved

### Responses

- `201` - Challenge generated

```jsonc
{
"id": string
}
```

## Register Attestor (iOS only)

Registers an instance of iOS dApp.

["Verify the Attestation"](https://developer.apple.com/documentation/devicecheck/validating_apps_that_connect_to_your_server#3576643) step from Apple docs.

### Request

`POST /attestor/:id`
```jsonc
{
"attestationObject": [number]
}
```
- `attestationObject` - byte array representing CBOR encoded ["Attestation Object"](https://developer.apple.com/documentation/devicecheck/validating_apps_that_connect_to_your_server#3576643)

### Responses

- `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)
Copy link
Contributor

@jakubuid jakubuid Nov 3, 2023 •
edited
Loading

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

ofc not


## Register Attestation

Used to register a new attestation
Registers dApp attestation.

`POST /attestation`
### Web

Body:
Uses our custom iframe Enclave solution.

#### Request

`POST /attestation/:id`
```jsonc
{
"attestationId": string,
"origin": string
}
```
- `origin` - Web dApp origin

#### Responses

- `201` - Attestation registered
- `400` - Invalid request body

### Android

Uses [Google Play Integrity API](https://developer.android.com/google/play/integrity/overview).

["Decrypt and verify the integrity verdict"](https://developer.android.com/google/play/integrity/standard#decrypt-and) step from Android docs.

#### Request

`POST /attestation/:id`
```jsonc
{
"integrityToken": string
}
```
- `integrityToken` - token provided to the Android dApp by [Integrity API](https://developer.android.com/google/play/integrity/standard#request-integrity)

#### Responses

- `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)
Copy link
Contributor

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?


### iOS

Uses [Apple DeviceCheck](https://developer.apple.com/documentation/devicecheck/).

["Verify the Assertion"](https://developer.apple.com/documentation/devicecheck/validating_apps_that_connect_to_your_server#3576644) step from Apple docs.

#### Request

`POST /attestation/:id`
```jsonc
{
"keyId": string,
"assertion": [number]
}
```
- `keyId` - key identifier used when registering the Attestor
- `assertion` - byte array representing CBOR encoded ["Assertion Object"](https://developer.apple.com/documentation/devicecheck/validating_apps_that_connect_to_your_server#3576644)

#### Responses

- `201` - Attestation registered
- `400` - Invalid request body or `assertion` format
- `401` - Failed to authenticate the provided `keyId`

## Resolve Attestation

Used to resolve an attestation
Resolves previously registered dApp attestation for a wallet.

### Web

`GET /attestation/:attestationId`
#### Request

Response:
`GET /attestation/:id`

#### Responses

- `200` - Attestation exists
- `origin` - origin of the Web dApp that registered this attestation
- `isScam` - indicator of whether this dApp is marked as a scam in our databases
- nullable, `null` meaning we don't know
```jsonc
{
"origin": string
"origin": string,
"isScam": boolean
}
```
- `404` - Attestation doesn't exist

### 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
}
Comment on lines +138 to 168
Copy link
Contributor

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
}

Copy link
Member Author

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?

Copy link
Member Author

@xDarksome xDarksome Nov 3, 2023 •
edited
Loading

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

Copy link
Contributor

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

Copy link
Member Author

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.

Copy link
Contributor

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

Copy link
Member Author

@xDarksome xDarksome Nov 3, 2023 •
edited
Loading

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.

Copy link
Member Author

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?

Copy link
Contributor

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

Copy link
Contributor

@jakubuid jakubuid Nov 3, 2023 •
edited
Loading

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

```