From 93683a33a6bac04375603172ca3671720a4e4b86 Mon Sep 17 00:00:00 2001 From: janniks Date: Mon, 21 Mar 2022 17:11:55 +0100 Subject: [PATCH 1/5] fix: remove username checking --- packages/auth/src/userSession.ts | 24 ++---------------------- packages/auth/src/verification.ts | 16 +++------------- packages/auth/tests/auth.test.ts | 15 +++++---------- 3 files changed, 10 insertions(+), 45 deletions(-) diff --git a/packages/auth/src/userSession.ts b/packages/auth/src/userSession.ts index 65885ab0c..f558b8bdd 100644 --- a/packages/auth/src/userSession.ts +++ b/packages/auth/src/userSession.ts @@ -25,7 +25,7 @@ import { nextHour, } from '@stacks/common'; import { extractProfile } from '@stacks/profile'; -import { AuthScope, DEFAULT_PROFILE, NAME_LOOKUP_PATH } from './constants'; +import { AuthScope, DEFAULT_PROFILE } from './constants'; import * as queryString from 'query-string'; import { UserData } from './userData'; import { StacksMainnet } from '@stacks/network'; @@ -237,27 +237,7 @@ export class UserSession { throw new Error('Unexpected token payload type of string'); } - // Section below is removed since the config was never persisted and therefore useless - - // if (isLaterVersion(tokenPayload.version as string, '1.3.0') - // && tokenPayload.blockstackAPIUrl !== null && tokenPayload.blockstackAPIUrl !== undefined) { - // // override globally - // Logger.info(`Overriding ${config.network.blockstackAPIUrl} ` - // + `with ${tokenPayload.blockstackAPIUrl}`) - // // TODO: this config is never saved so the user node preference - // // is not respected in later sessions.. - // config.network.blockstackAPIUrl = tokenPayload.blockstackAPIUrl as string - // coreNode = tokenPayload.blockstackAPIUrl as string - // } - - const nameLookupURL = `${coreNode}${NAME_LOOKUP_PATH}`; - - const fallbackLookupURLs = [ - `https://stacks-node-api.stacks.co${NAME_LOOKUP_PATH}`, - `https://registrar.stacks.co${NAME_LOOKUP_PATH}`, - ].filter(url => url !== nameLookupURL); - - const isValid = await verifyAuthResponse(authResponseToken, nameLookupURL, fallbackLookupURLs); + const isValid = await verifyAuthResponse(authResponseToken); if (!isValid) { throw new LoginFailedError('Invalid authentication response.'); } diff --git a/packages/auth/src/verification.ts b/packages/auth/src/verification.ts index 082033253..b2ea64698 100644 --- a/packages/auth/src/verification.ts +++ b/packages/auth/src/verification.ts @@ -275,22 +275,12 @@ export async function verifyAuthRequestAndLoadManifest(token: string): Promise { - const values = await Promise.all([ +export async function verifyAuthResponse(token: string): Promise { + const conditions = await Promise.all([ isExpirationDateValid(token), isIssuanceDateValid(token), doSignaturesMatchPublicKeys(token), doPublicKeysMatchIssuer(token), ]); - const usernameMatchings = await Promise.all( - [nameLookupURL] - .concat(fallbackLookupURLs || []) - .map(url => doPublicKeysMatchUsername(token, url)) - ); - const someUsernameMatches = usernameMatchings.includes(true); - return !!someUsernameMatches && values.every(val => val); + return conditions.every(val => val); } diff --git a/packages/auth/tests/auth.test.ts b/packages/auth/tests/auth.test.ts index 0788231e2..57e205dbc 100644 --- a/packages/auth/tests/auth.test.ts +++ b/packages/auth/tests/auth.test.ts @@ -183,7 +183,7 @@ test('makeAuthResponse && verifyAuthResponse', async () => { ); expect((decodedToken.payload as any).username).toBe(null); - await verifyAuthResponse(authResponse, nameLookupURL).then(verifiedResult => { + await verifyAuthResponse(authResponse).then(verifiedResult => { expect(verifiedResult).toBe(true); }); @@ -257,11 +257,11 @@ test('auth response with username', async () => { expect(verified).toBe(true); }); - await verifyAuthResponse(authResponse, nameLookupURL).then(verifiedResult => { + await verifyAuthResponse(authResponse).then(verifiedResult => { expect(verifiedResult).toBe(true); }); - expect(fetchMock.mock.calls.length).toEqual(2); + expect(fetchMock.mock.calls.length).toEqual(1); }); test('auth response with invalid private key', async () => { @@ -308,8 +308,6 @@ test('auth response with invalid private key', async () => { }); test('handlePendingSignIn with authResponseToken', async () => { - const url = `${nameLookupURL}ryan.id`; - fetchMock.mockResponse(JSON.stringify(sampleNameRecords.ryan)); const appPrivateKey = makeECPrivateKey(); @@ -338,12 +336,10 @@ test('handlePendingSignIn with authResponseToken', async () => { expect(fail).toBeCalledTimes(0); expect(pass).toBeCalledTimes(1); - expect(fetchMock.mock.calls.length).toEqual(3); - expect(fetchMock.mock.calls[0][0]).toEqual(url); + expect(fetchMock.mock.calls.length).toEqual(0); }); test('handlePendingSignIn 2', async () => { - const url = `${nameLookupURL}ryan.id`; fetchMock.mockResponse(JSON.stringify(sampleNameRecords.ryan)); const appPrivateKey = makeECPrivateKey(); @@ -371,8 +367,7 @@ test('handlePendingSignIn 2', async () => { await blockstack.handlePendingSignIn(authResponse).then(pass).catch(fail); expect(fail).toBeCalledTimes(0); expect(pass).toBeCalledTimes(1); - expect(fetchMock.mock.calls.length).toEqual(3); - expect(fetchMock.mock.calls[0][0]).toEqual(url); + expect(fetchMock.mock.calls.length).toEqual(0); }); test('handlePendingSignIn with existing user session', async () => { From 2e95a3225fff5066ed9d61095545e1632e6e667b Mon Sep 17 00:00:00 2001 From: janniks Date: Thu, 14 Apr 2022 15:45:40 +0200 Subject: [PATCH 2/5] fix: remove doPublicKeysMatchUsername --- package-lock.json | 2 - packages/auth/package.json | 1 - packages/auth/src/index.ts | 1 - packages/auth/src/verification.ts | 69 +------------------------------ packages/auth/tests/auth.test.ts | 12 +----- 5 files changed, 3 insertions(+), 82 deletions(-) diff --git a/package-lock.json b/package-lock.json index 4b68572bf..a469bce46 100644 --- a/package-lock.json +++ b/package-lock.json @@ -21494,7 +21494,6 @@ "@stacks/encryption": "^3.3.0", "@stacks/network": "^3.3.0", "@stacks/profile": "^3.3.0", - "c32check": "^1.1.3", "cross-fetch": "^3.1.4", "jsontokens": "^3.0.0", "query-string": "^6.13.1" @@ -24526,7 +24525,6 @@ "@stacks/network": "^3.3.0", "@stacks/profile": "^3.3.0", "@types/jest": "^26.0.22", - "c32check": "^1.1.3", "cross-fetch": "^3.1.4", "jest": "^26.6.3", "jest-fetch-mock": "^3.0.3", diff --git a/packages/auth/package.json b/packages/auth/package.json index 4c34624f8..a9396219a 100644 --- a/packages/auth/package.json +++ b/packages/auth/package.json @@ -44,7 +44,6 @@ "@stacks/encryption": "^3.3.0", "@stacks/network": "^3.3.0", "@stacks/profile": "^3.3.0", - "c32check": "^1.1.3", "cross-fetch": "^3.1.4", "jsontokens": "^3.0.0", "query-string": "^6.13.1" diff --git a/packages/auth/src/index.ts b/packages/auth/src/index.ts index 6fd7a0b49..f0db6c5f2 100644 --- a/packages/auth/src/index.ts +++ b/packages/auth/src/index.ts @@ -6,7 +6,6 @@ export { verifyAuthResponse, isExpirationDateValid, isIssuanceDateValid, - doPublicKeysMatchUsername, doPublicKeysMatchIssuer, doSignaturesMatchPublicKeys, isManifestUriValid, diff --git a/packages/auth/src/verification.ts b/packages/auth/src/verification.ts index b2ea64698..878314772 100644 --- a/packages/auth/src/verification.ts +++ b/packages/auth/src/verification.ts @@ -1,9 +1,8 @@ +import { isSameOriginAbsoluteUrl } from '@stacks/common'; +import { publicKeyToAddress } from '@stacks/encryption'; import { decodeToken, TokenVerifier } from 'jsontokens'; import { getAddressFromDID } from './dids'; -import { publicKeyToAddress } from '@stacks/encryption'; -import { fetchPrivate, isSameOriginAbsoluteUrl } from '@stacks/common'; import { fetchAppManifest } from './provider'; -import { c32ToB58 } from 'c32check'; /** * Checks if the ES256k signature on passed `token` match the claimed public key @@ -65,70 +64,6 @@ export function doPublicKeysMatchIssuer(token: string): boolean { return false; } -/** - * Looks up the identity address that owns the claimed username - * in `token` using the lookup endpoint provided in `nameLookupURL` - * to determine if the username is owned by the identity address - * that matches the claimed public key - * - * @param {String} token encoded and signed authentication token - * @param {String} nameLookupURL a URL to the name lookup endpoint of the Blockstack Core API - * @return {Promise} returns a `Promise` that resolves to - * `true` if the username is owned by the public key, otherwise the - * `Promise` resolves to `false` - * @private - * @ignore - */ -export async function doPublicKeysMatchUsername( - token: string, - nameLookupURL: string -): Promise { - try { - const payload = decodeToken(token).payload; - if (typeof payload === 'string') { - throw new Error('Unexpected token payload type of string'); - } - if (!payload.username) { - return true; - } - - if (payload.username === null) { - return true; - } - - if (nameLookupURL === null) { - return false; - } - - const username = payload.username; - const url = `${nameLookupURL.replace(/\/$/, '')}/${username}`; - const response = await fetchPrivate(url); - const responseText = await response.text(); - const responseJSON = JSON.parse(responseText); - if (responseJSON.hasOwnProperty('address')) { - const nameOwningAddress = responseJSON.address; - let nameOwningAddressBtc = nameOwningAddress; - try { - // try converting STX to BTC - // if this throws, it's already a BTC address - nameOwningAddressBtc = c32ToB58(nameOwningAddress, 0); - } catch {} - const addressFromIssuer = getAddressFromDID(payload.iss); - if (nameOwningAddressBtc === addressFromIssuer) { - return true; - } else { - return false; - } - } else { - return false; - } - } catch (error) { - console.log(error); - console.log('Error checking `doPublicKeysMatchUsername`'); - return false; - } -} - /** * Checks if the if the token issuance time and date is after the * current time and date. diff --git a/packages/auth/tests/auth.test.ts b/packages/auth/tests/auth.test.ts index 57e205dbc..5bc169346 100644 --- a/packages/auth/tests/auth.test.ts +++ b/packages/auth/tests/auth.test.ts @@ -8,7 +8,6 @@ import { isIssuanceDateValid, doSignaturesMatchPublicKeys, doPublicKeysMatchIssuer, - doPublicKeysMatchUsername, isManifestUriValid, isRedirectUriValid, verifyAuthRequestAndLoadManifest, @@ -30,7 +29,6 @@ beforeEach(() => { const privateKey = 'a5c61c6ca7b3e7e55edee68566aeab22e4da26baa285c7bd10e8d2218aa3b229'; const publicKey = '027d28f9951ce46538951e3697c62588a87f1f1f295de4a14fdd4c780fc52cfe69'; -const nameLookupURL = 'https://stacks-node-api.mainnet.stacks.co/v1/names/'; test('makeAuthRequest && verifyAuthRequest', async () => { const appConfig = new AppConfig(['store_write'], 'http://localhost:3000'); @@ -191,10 +189,6 @@ test('makeAuthResponse && verifyAuthResponse', async () => { expect(isIssuanceDateValid(authResponse)).toBe(true); expect(doSignaturesMatchPublicKeys(authResponse)).toBe(true); expect(doPublicKeysMatchIssuer(authResponse)).toBe(true); - - await doPublicKeysMatchUsername(authResponse, nameLookupURL).then(verifiedResult => { - expect(verifiedResult).toBe(true); - }); }); test('auth response with invalid or empty appPrivateKeyFromWalletSalt', async () => { @@ -253,15 +247,11 @@ test('auth response with username', async () => { const authResponse = await makeAuthResponse(privateKey, sampleProfiles.ryan, 'ryan.id', null); - await doPublicKeysMatchUsername(authResponse, nameLookupURL).then(verified => { - expect(verified).toBe(true); - }); - await verifyAuthResponse(authResponse).then(verifiedResult => { expect(verifiedResult).toBe(true); }); - expect(fetchMock.mock.calls.length).toEqual(1); + expect(fetchMock.mock.calls.length).toEqual(0); }); test('auth response with invalid private key', async () => { From 98bff3f0e6acd9ab7a433801293b4d90370660e7 Mon Sep 17 00:00:00 2001 From: janniks Date: Mon, 18 Apr 2022 19:06:55 +0200 Subject: [PATCH 3/5] feat: remove username from payload and userdata BREAKING CHANGE: This change REMOVES the `username` attribute from auth token payloads and therefore also from userData in @stacks/connect. Hence, there is NO MORE username verification done by @stacks/connect automatically. --- packages/auth/src/messages.ts | 3 --- packages/auth/src/userData.ts | 2 -- packages/auth/src/userSession.ts | 1 - packages/auth/tests/auth.test.ts | 16 +++++----------- packages/keychain/src/identity.ts | 1 - packages/wallet-sdk/src/models/account.ts | 1 - 6 files changed, 5 insertions(+), 19 deletions(-) diff --git a/packages/auth/src/messages.ts b/packages/auth/src/messages.ts index 2e1963577..eccf8ff19 100644 --- a/packages/auth/src/messages.ts +++ b/packages/auth/src/messages.ts @@ -159,7 +159,6 @@ export async function decryptPrivateKey( * @param {String} privateKey the identity key of the Blockstack ID generating * the authentication response * @param {Object} profile the profile object for the Blockstack ID - * @param {String} username the username of the Blockstack ID if any, otherwise `null` * @param {AuthMetadata} metadata an object containing metadata sent as part of the authentication * response including `email` if requested and available and a URL to the profile * @param {String} coreToken core session token when responding to a legacy auth request @@ -181,7 +180,6 @@ export async function makeAuthResponse( privateKey: string, // eslint-disable-next-line @typescript-eslint/ban-types profile: {} = {}, - username: string | null = null, metadata: AuthMetadata | null, coreToken: string | null = null, appPrivateKey: string | null = null, @@ -232,7 +230,6 @@ export async function makeAuthResponse( public_keys: [publicKey], appPrivateKeyFromWalletSalt, profile, - username, core_token: coreTokenPayload, }, additionalProperties diff --git a/packages/auth/src/userData.ts b/packages/auth/src/userData.ts index a9e428b17..9d1707a15 100644 --- a/packages/auth/src/userData.ts +++ b/packages/auth/src/userData.ts @@ -2,8 +2,6 @@ * Returned from the [[UserSession.loadUserData]] function. */ export interface UserData { - // public: the blockstack ID (for example: stackerson.id or alice.blockstack.id) - username: string; // public: the email address for the user. only available if the `email` // scope is requested, and if the user has entered a valid email into // their profile. diff --git a/packages/auth/src/userSession.ts b/packages/auth/src/userSession.ts index f558b8bdd..eaace9092 100644 --- a/packages/auth/src/userSession.ts +++ b/packages/auth/src/userSession.ts @@ -297,7 +297,6 @@ export class UserSession { } const userData: UserData = { - username: tokenPayload.username as string, profile: tokenPayload.profile, email: tokenPayload.email as string, decentralizedID: tokenPayload.iss, diff --git a/packages/auth/tests/auth.test.ts b/packages/auth/tests/auth.test.ts index 5bc169346..ccf87f740 100644 --- a/packages/auth/tests/auth.test.ts +++ b/packages/auth/tests/auth.test.ts @@ -165,7 +165,7 @@ test('invalid auth request - invalid manifest uri', async () => { }); test('makeAuthResponse && verifyAuthResponse', async () => { - const authResponse = await makeAuthResponse(privateKey, sampleProfiles.ryan, null, null); + const authResponse = await makeAuthResponse(privateKey, sampleProfiles.ryan, null); expect(authResponse).toBeTruthy(); const decodedToken = decodeToken(authResponse); @@ -179,7 +179,9 @@ test('makeAuthResponse && verifyAuthResponse', async () => { expect(JSON.stringify((decodedToken.payload as any).profile)).toEqual( JSON.stringify(sampleProfiles.ryan) ); - expect((decodedToken.payload as any).username).toBe(null); + + // username was removed from payload + expect('username' in (decodedToken.payload as any)).toBeFalsy(); await verifyAuthResponse(authResponse).then(verifiedResult => { expect(verifiedResult).toBe(true); @@ -199,7 +201,6 @@ test('auth response with invalid or empty appPrivateKeyFromWalletSalt', async () null, null, null, - null, undefined, null, null, @@ -223,7 +224,6 @@ test('auth response with valid appPrivateKeyFromWalletSalt', async () => { null, null, null, - null, undefined, null, null, @@ -245,7 +245,7 @@ test('auth response with valid appPrivateKeyFromWalletSalt', async () => { test('auth response with username', async () => { fetchMock.mockResponse(JSON.stringify(sampleNameRecords.ryan)); - const authResponse = await makeAuthResponse(privateKey, sampleProfiles.ryan, 'ryan.id', null); + const authResponse = await makeAuthResponse(privateKey, sampleProfiles.ryan, null); await verifyAuthResponse(authResponse).then(verifiedResult => { expect(verifiedResult).toBe(true); @@ -270,7 +270,6 @@ test('auth response with invalid private key', async () => { const authResponse = await makeAuthResponse( privateKey, sampleProfiles.ryan, - 'ryan.id', metadata, undefined, appPrivateKey, @@ -312,7 +311,6 @@ test('handlePendingSignIn with authResponseToken', async () => { const authResponse = await makeAuthResponse( privateKey, sampleProfiles.ryan, - 'ryan.id', metadata, undefined, appPrivateKey, @@ -340,7 +338,6 @@ test('handlePendingSignIn 2', async () => { const authResponse = await makeAuthResponse( privateKey, sampleProfiles.ryan, - 'ryan.id', metadata, undefined, appPrivateKey, @@ -387,7 +384,6 @@ test('handlePendingSignIn with existing user session', async () => { const authResponse = await makeAuthResponse( privateKey, sampleProfiles.ryan, - 'ryan.id', metadata, undefined, appPrivateKey, @@ -453,7 +449,6 @@ test('handlePendingSignIn with authResponseToken, transit key and custom Blockst const authResponse = await makeAuthResponse( privateKey, sampleProfiles.ryan, - 'ryan.id', metadata, undefined, appPrivateKey, @@ -503,7 +498,6 @@ test( const authResponse = await makeAuthResponse( privateKey, sampleProfiles.ryan, - 'ryan.id', metadata, undefined, appPrivateKey, diff --git a/packages/keychain/src/identity.ts b/packages/keychain/src/identity.ts index 16be96952..f006ba791 100644 --- a/packages/keychain/src/identity.ts +++ b/packages/keychain/src/identity.ts @@ -96,7 +96,6 @@ export class Identity implements IdentifyInterface { ...(this.profile || {}), stxAddress, }, - this.defaultUsername || '', { profileUrl, }, diff --git a/packages/wallet-sdk/src/models/account.ts b/packages/wallet-sdk/src/models/account.ts index 71bf6d61e..ca185a7b8 100644 --- a/packages/wallet-sdk/src/models/account.ts +++ b/packages/wallet-sdk/src/models/account.ts @@ -118,7 +118,6 @@ export const makeAuthResponse = async ({ mainnet: getStxAddress({ account, transactionVersion: TransactionVersion.Mainnet }), }, }, - account.username || '', { profileUrl, }, From ed8caee33703a9cfbbb2dc453b2f3929ab1c24a7 Mon Sep 17 00:00:00 2001 From: janniks Date: Mon, 18 Apr 2022 21:25:41 +0200 Subject: [PATCH 4/5] docs: fix docstring --- packages/auth/src/verification.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/auth/src/verification.ts b/packages/auth/src/verification.ts index 878314772..cb708a13d 100644 --- a/packages/auth/src/verification.ts +++ b/packages/auth/src/verification.ts @@ -203,8 +203,6 @@ export async function verifyAuthRequestAndLoadManifest(token: string): Promise Date: Tue, 19 Apr 2022 14:53:26 +0200 Subject: [PATCH 5/5] chore: bump payload version --- packages/auth/src/messages.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/auth/src/messages.ts b/packages/auth/src/messages.ts index eccf8ff19..6f424a9b9 100644 --- a/packages/auth/src/messages.ts +++ b/packages/auth/src/messages.ts @@ -12,7 +12,7 @@ import { } from '@stacks/encryption'; import { DEFAULT_SCOPE, AuthScope } from './constants'; -const VERSION = '1.3.1'; +const VERSION = '1.4.0'; type AuthMetadata = { email?: string;