Skip to content

Commit

Permalink
[Wallet] Enable 8 digit code verification and ignore attestation serv…
Browse files Browse the repository at this point in the history
…ices below 1.1.0 (#6437)

### Description

Enable 8 digit code verification and ignore attestation services below 1.1.0 as they do not support 8 digit codes.

### Other changes

Force EOA address as a signer to provide signature for attestation services.

### Tested

Android manual input and auto-import using attestation service 1.2.0 on alfajores.

### Related issues

- Fixes #6097

### Backwards compatibility

Yes
  • Loading branch information
i1skn authored Jan 12, 2021
1 parent 1d724a1 commit 0a91ca1
Show file tree
Hide file tree
Showing 12 changed files with 88 additions and 58 deletions.
22 changes: 13 additions & 9 deletions packages/env-tests/src/shared/attestation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,15 +106,19 @@ async function findValidCode(
await Promise.all(
possibleIssuers.map((a) =>
attestations
.getAttestationForSecurityCode(a.attestationServiceURL, {
account,
issuer: a.issuer,
phoneNumber,
salt: pepper,
securityCode: securityCode.slice(1),
})
.getAttestationForSecurityCode(
a.attestationServiceURL,
{
account,
issuer: a.issuer,
phoneNumber,
salt: pepper,
securityCode: securityCode.slice(1),
},
account
)
// hit the wrong service
.catch((_) => null)
.catch(() => null)
)
)
).find(Boolean)
Expand Down Expand Up @@ -285,7 +289,7 @@ async function chooseFromAvailablePhoneNumbers(twilioClient: Twilio) {
async function createPhoneNumber(twilioClient: Twilio, addressSid: string) {
const countryCodes = ['GB', 'US']
const countryCode = sample(countryCodes)
const context = await twilioClient.availablePhoneNumbers.get(countryCode!)
const context = twilioClient.availablePhoneNumbers.get(countryCode!)
const numbers = await context.mobile.list({ limit: 10 })
const usableNumber = numbers[0]
await twilioClient.incomingPhoneNumbers.create({
Expand Down
17 changes: 2 additions & 15 deletions packages/mobile/src/firebase/firebase.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,8 @@ import { expectSaga } from 'redux-saga-test-plan'
import { throwError } from 'redux-saga-test-plan/providers'
import { call, select } from 'redux-saga/effects'
import { currentLanguageSelector } from 'src/app/reducers'
import {
initializeCloudMessaging,
isVersionBelowMinimum,
registerTokenToDb,
setUserLanguage,
} from 'src/firebase/firebase'
import { initializeCloudMessaging, registerTokenToDb, setUserLanguage } from 'src/firebase/firebase'

import { mockAccount2 } from 'test/values'

const hasPermissionMock = jest.fn(() => null)
Expand Down Expand Up @@ -90,12 +86,3 @@ describe(initializeCloudMessaging, () => {
.run()
})
})

describe('Firebase version check', () => {
it('Correctly check if version is deprecated', () => {
expect(isVersionBelowMinimum('1.5.0', '1.4.0')).toBe(false)
expect(isVersionBelowMinimum('1.4.0', '1.5.0')).toBe(true)
expect(isVersionBelowMinimum('1.4.0', '1.4.0')).toBe(false)
expect(isVersionBelowMinimum('1.4.0', '1.4.0.1')).toBe(true)
})
})
17 changes: 0 additions & 17 deletions packages/mobile/src/firebase/firebase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,23 +170,6 @@ export const registerTokenToDb = async (
}
}

export function isVersionBelowMinimum(version: string, minVersion: string): boolean {
const minVersionArray = minVersion.split('.')
const versionArray = version.split('.')
const minVersionLength = Math.min(minVersionArray.length, version.length)
for (let i = 0; i < minVersionLength; i++) {
if (minVersionArray[i] > versionArray[i]) {
return true
} else if (minVersionArray[i] < versionArray[i]) {
return false
}
}
if (minVersionArray.length > versionArray.length) {
return true
}
return false
}

const VALUE_CHANGE_HOOK = 'value'

/*
Expand Down
2 changes: 1 addition & 1 deletion packages/mobile/src/flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export const features = {
PNP_USE_DEK_FOR_AUTH: true,
KOMENCI: true,
ESCROW_WITHOUT_CODE: true,
SHORT_VERIFICATION_CODES: false,
SHORT_VERIFICATION_CODES: true,
}

export const pausedFeatures = {
Expand Down
13 changes: 9 additions & 4 deletions packages/mobile/src/identity/securityCode.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Address } from '@celo/connect'
import {
ActionableAttestation,
AttestationsWrapper,
Expand Down Expand Up @@ -32,7 +33,8 @@ export async function getAttestationCodeForSecurityCode(
phoneHashDetails: PhoneNumberHashDetails,
account: string,
attestations: ActionableAttestation[],
securityCodeWithPrefix: string
securityCodeWithPrefix: string,
signer: Address
) {
const securityCodePrefix = securityCodeWithPrefix[0]
const lookupAttestations = attestations.filter(
Expand All @@ -47,7 +49,8 @@ export async function getAttestationCodeForSecurityCode(
account,
phoneHashDetails,
attestation,
securityCodeWithPrefix.substr(1) // remove prefix
securityCodeWithPrefix.substr(1), // remove prefix
signer
)
)
)
Expand All @@ -58,7 +61,8 @@ async function requestValidator(
account: string,
phoneHashDetails: PhoneNumberHashDetails,
attestation: ActionableAttestation,
securityCode: string
securityCode: string,
signer: Address
): Promise<string> {
const issuer = attestation.issuer
Logger.debug(
Expand All @@ -76,7 +80,8 @@ async function requestValidator(

return attestationsWrapper.getAttestationForSecurityCode(
attestation.attestationServiceURL,
requestBody
requestBody,
signer
)
} catch (error) {
Logger.error(
Expand Down
3 changes: 3 additions & 0 deletions packages/mobile/src/identity/verification.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,18 +125,21 @@ const mockActionableAttestations: ActionableAttestation[] = [
blockNumber: 100,
attestationServiceURL: 'https://fake.celo.org/0',
name: '',
version: '1.1.0',
},
{
issuer: attestationCode1.issuer,
blockNumber: 110,
attestationServiceURL: 'https://fake.celo.org/1',
name: '',
version: '1.1.0',
},
{
issuer: attestationCode2.issuer,
blockNumber: 120,
attestationServiceURL: 'https://fake.celo.org/2',
name: '',
version: '1.1.0',
},
]

Expand Down
14 changes: 13 additions & 1 deletion packages/mobile/src/identity/verification.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ import { stableTokenBalanceSelector } from 'src/stableToken/reducer'
import { sendTransaction } from 'src/transactions/send'
import { newTransactionContext } from 'src/transactions/types'
import Logger from 'src/utils/Logger'
import { isVersionBelowMinimum } from 'src/utils/versionCheck'
import { getContractKit } from 'src/web3/contracts'
import { registerAccountDek } from 'src/web3/dataEncryptionKey'
import {
Expand All @@ -95,6 +96,7 @@ import {
} from 'src/web3/saga'

const TAG = 'identity/verification'
const MINIMUM_VERSION_FOR_SHORT_CODES = '1.1.0'

export const NUM_ATTESTATIONS_REQUIRED = 3
export const ESTIMATED_COST_PER_ATTESTATION = 0.051
Expand Down Expand Up @@ -465,6 +467,7 @@ export function* requestAndRetrieveAttestations(
if (!isFeelessVerification) {
yield put(setRetryVerificationWithForno(false))
}

while (attestations.length < attestationsNeeded) {
ValoraAnalytics.track(VerificationEvents.verification_request_attestation_start, {
currentAttestation: attestations.length,
Expand Down Expand Up @@ -495,6 +498,13 @@ export function* requestAndRetrieveAttestations(

// Check if we have a sufficient set now by fetching the new total set
attestations = yield call(getActionableAttestations, attestationsWrapper, phoneHash, account)
if (features.SHORT_VERIFICATION_CODES) {
// we only support attestation service 1.1.0 and above for short codes
attestations = attestations.filter(
(att) => !isVersionBelowMinimum(att.version, MINIMUM_VERSION_FOR_SHORT_CODES)
)
}

ValoraAnalytics.track(
VerificationEvents.verification_request_all_attestations_refresh_progress,
{
Expand Down Expand Up @@ -678,14 +688,16 @@ export function attestationCodeReceiver(
try {
if (features.SHORT_VERIFICATION_CODES) {
securityCodeWithPrefix = extractSecurityCodeWithPrefix(message)
const signer = yield call(getConnectedUnlockedAccount)
if (securityCodeWithPrefix) {
message = yield call(
getAttestationCodeForSecurityCode,
attestationsWrapper,
phoneHashDetails,
account,
attestations,
securityCodeWithPrefix
securityCodeWithPrefix,
signer
)
} else {
Logger.error(TAG + '@attestationCodeReceiver', 'No security code in received message')
Expand Down
2 changes: 1 addition & 1 deletion packages/mobile/src/navigator/NavigatorWrapper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import { getAppLocked, getAppState } from 'src/app/selectors'
import UpgradeScreen from 'src/app/UpgradeScreen'
import { doingBackupFlowSelector, shouldForceBackupSelector } from 'src/backup/selectors'
import { DEV_RESTORE_NAV_STATE_ON_RELOAD } from 'src/config'
import { isVersionBelowMinimum } from 'src/firebase/firebase'
import i18n from 'src/i18n'
import InviteFriendModal from 'src/invite/InviteFriendModal'
import { generateInviteLink } from 'src/invite/saga'
Expand All @@ -24,6 +23,7 @@ import { Screens } from 'src/navigator/Screens'
import PincodeLock from 'src/pincode/PincodeLock'
import useTypedSelector from 'src/redux/useSelector'
import Logger from 'src/utils/Logger'
import { isVersionBelowMinimum } from 'src/utils/versionCheck'

// This uses RN Navigation's experimental nav state persistence
// to improve the hot reloading experience when in DEV mode
Expand Down
10 changes: 10 additions & 0 deletions packages/mobile/src/utils/versionCheck.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { isVersionBelowMinimum } from 'src/utils/versionCheck'
describe('Version check', () => {
it('Correctly check if version is deprecated', () => {
expect(isVersionBelowMinimum('1.5.0', '1.4.0')).toBe(false)
expect(isVersionBelowMinimum('1.4.0', '1.5.0')).toBe(true)
expect(isVersionBelowMinimum('1.4.0', '1.4.0')).toBe(false)
expect(isVersionBelowMinimum('1.4.0', '1.4.0.1')).toBe(true)
expect(isVersionBelowMinimum('1.4.0-unstable', '1.4.0')).toBe(false)
})
})
16 changes: 16 additions & 0 deletions packages/mobile/src/utils/versionCheck.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
export function isVersionBelowMinimum(version: string, minVersion: string): boolean {
const minVersionArray = minVersion.split('.')
const versionArray = version.split('.')
const minVersionLength = Math.min(minVersionArray.length, version.length)
for (let i = 0; i < minVersionLength; i++) {
if (minVersionArray[i] > versionArray[i]) {
return true
} else if (minVersionArray[i] < versionArray[i]) {
return false
}
}
if (minVersionArray.length > versionArray.length) {
return true
}
return false
}
28 changes: 19 additions & 9 deletions packages/sdk/contractkit/src/wrappers/Attestations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ export interface ActionableAttestation {
blockNumber: number
attestationServiceURL: string
name: string | undefined
version: string
}

type AttestationServiceRunningCheckResult =
Expand Down Expand Up @@ -330,15 +331,24 @@ export class AttestationsWrapper extends BaseWrapper<Attestations> {

const nameClaim = metadata.findClaim(ClaimTypes.NAME)

// TODO: Once we have status indicators, we should check if service is up
// https://github.com/celo-org/celo-monorepo/issues/1586
const resp = await fetch(`${attestationServiceURLClaim.url}status`)
if (!resp.ok) {
throw new Error(`Request failed with status ${resp.status}`)
}
const { status, version } = await resp.json()

if (status !== 'ok') {
return { isValid: false, issuer: arg.issuer }
}

return {
isValid: true,
result: {
blockNumber: arg.blockNumber,
issuer: arg.issuer,
attestationServiceURL: attestationServiceURLClaim.url,
name: nameClaim ? nameClaim.name : undefined,
version,
},
}
} catch (error) {
Expand Down Expand Up @@ -593,7 +603,8 @@ export class AttestationsWrapper extends BaseWrapper<Attestations> {
*/
async getAttestationForSecurityCode(
serviceURL: string,
requestBody: GetAttestationRequest
requestBody: GetAttestationRequest,
signer: Address
): Promise<string> {
const urlParams = new URLSearchParams({
phoneNumber: requestBody.phoneNumber,
Expand All @@ -607,13 +618,12 @@ export class AttestationsWrapper extends BaseWrapper<Attestations> {
}
if (requestBody.securityCode) {
urlParams.set('securityCode', requestBody.securityCode)
const signature = await this.kit.signTypedData(
signer,
buildSecurityCodeTypedData(requestBody.securityCode)
)
additionalHeaders = {
Authentication: SignatureUtils.serializeSignature(
await this.kit.signTypedData(
requestBody.account,
buildSecurityCodeTypedData(requestBody.securityCode)
)
),
Authentication: SignatureUtils.serializeSignature(signature),
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/sdk/utils/src/attestations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export function attestToIdentifier(
}

export function extractSecurityCodeWithPrefix(message: string) {
const matches = message.match('\\s(\\d{8})')
const matches = message.match('(\\d{8})')
if (matches && matches.length === 2) {
return matches[1]
}
Expand Down

0 comments on commit 0a91ca1

Please sign in to comment.