Skip to content

fix: remove username matching #1230

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

Merged
merged 5 commits into from
Apr 19, 2022
Merged

fix: remove username matching #1230

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
93683a3
fix: remove username checking
janniks Mar 21, 2022
2e95a32
fix: remove doPublicKeysMatchUsername
janniks Apr 14, 2022
98bff3f
feat: remove username from payload and userdata
janniks Apr 18, 2022
ed8caee
docs: fix docstring
janniks Apr 18, 2022
b25b3c4
chore: bump payload version
janniks Apr 19, 2022
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
2 changes: 0 additions & 2 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion packages/auth/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
1 change: 0 additions & 1 deletion packages/auth/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ export {
verifyAuthResponse,
isExpirationDateValid,
isIssuanceDateValid,
doPublicKeysMatchUsername,
doPublicKeysMatchIssuer,
doSignaturesMatchPublicKeys,
isManifestUriValid,
Expand Down
24 changes: 2 additions & 22 deletions packages/auth/src/userSession.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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.');
}
Expand Down
85 changes: 5 additions & 80 deletions packages/auth/src/verification.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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<Boolean>} 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<boolean> {
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.
Expand Down Expand Up @@ -275,22 +210,12 @@ export async function verifyAuthRequestAndLoadManifest(token: string): Promise<a
* @private
* @ignore
*/
export async function verifyAuthResponse(
token: string,
nameLookupURL: string,
fallbackLookupURLs?: string[]
): Promise<boolean> {
const values = await Promise.all([
export async function verifyAuthResponse(token: string): Promise<boolean> {
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);
}
25 changes: 5 additions & 20 deletions packages/auth/tests/auth.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
isIssuanceDateValid,
doSignaturesMatchPublicKeys,
doPublicKeysMatchIssuer,
doPublicKeysMatchUsername,
isManifestUriValid,
isRedirectUriValid,
verifyAuthRequestAndLoadManifest,
Expand All @@ -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');
Expand Down Expand Up @@ -183,18 +181,14 @@ 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);
});

expect(isExpirationDateValid(authResponse)).toBe(true);
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 () => {
Expand Down Expand Up @@ -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, 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(0);
});

test('auth response with invalid private key', async () => {
Expand Down Expand Up @@ -308,8 +298,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();
Expand Down Expand Up @@ -338,12 +326,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();
Expand Down Expand Up @@ -371,8 +357,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 () => {
Expand Down