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.

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

[CP-9960] appcheck improvements #159

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@
"@avalabs/core-wallets-sdk": "3.1.0-alpha.37",
"@avalabs/evm-module": "1.4.3",
"@avalabs/glacier-sdk": "3.1.0-alpha.37",
"@avalabs/hw-app-avalanche": "0.14.1",
"@avalabs/hvm-module": "1.4.3",
"@avalabs/hw-app-avalanche": "0.14.1",
"@avalabs/types": "3.1.0-alpha.37",
"@avalabs/vm-module-types": "1.4.3",
"@blockaid/client": "0.10.0",
Expand Down Expand Up @@ -73,6 +73,7 @@
"bitcoinjs-lib": "5.2.0",
"bn.js": "5.2.1",
"date-fns": "2.28.0",
"detect-browser": "5.3.0",
"eth-json-rpc-middleware": "8.0.1",
"eth-rpc-errors": "4.0.3",
"ethers": "6.13.5",
Expand Down
2 changes: 1 addition & 1 deletion src/background/services/appcheck/AppCheckService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ describe('AppCheckService', () => {
.mocked(CustomProvider)
.mock.calls[0]?.[0].getToken()
.catch((err) => {
expect(err).toBe('timeout');
expect(err).toBe('[AppCheck] challenge solution timeout');
});

for (let i = 0; i <= WAIT_FOR_CHALLENGE_ATTEMPT_COUNT; i++) {
Expand Down
4 changes: 2 additions & 2 deletions src/background/services/appcheck/AppCheckService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import registerForChallenge from './utils/registerForChallenge';
import verifyChallenge from './utils/verifyChallenge';
import solveChallenge from './utils/solveChallenge';

export const WAIT_FOR_CHALLENGE_ATTEMPT_COUNT = 10;
export const WAIT_FOR_CHALLENGE_ATTEMPT_COUNT = 20;
export const WAIT_FOR_CHALLENGE_DELAY_MS = 500;

// Implementation based on https://github.com/ava-labs/core-id-service/blob/main/docs/extension-appcheck-attestation.md
Expand Down Expand Up @@ -97,7 +97,7 @@ export class AppCheckService {
res({ registrationId, solution });
} else if (remainingAttempts < 1) {
clearInterval(timer);
rej('timeout');
rej('[AppCheck] challenge solution timeout');
}
}, WAIT_FOR_CHALLENGE_DELAY_MS);
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ describe('registerForChallenge', () => {
} as Response);

expect(registerForChallenge(params)).rejects.toThrow(
'challenge registration error: "internal error"',
'[AppCheck] challenge registration error "internal error"',
);
});

Expand All @@ -53,7 +53,7 @@ describe('registerForChallenge', () => {
expect(registerForChallenge(params)).resolves.toBeUndefined();
expect(global.fetch).toHaveBeenCalledWith(
'https://id.com/v1/ext/register',
{
expect.objectContaining({
method: 'POST',
headers: {
'Content-Type': 'application/json',
Expand All @@ -64,7 +64,7 @@ describe('registerForChallenge', () => {
token: 'token',
requestId: 'requestId',
}),
},
}),
);
});
});
35 changes: 21 additions & 14 deletions src/background/services/appcheck/utils/registerForChallenge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,29 +3,36 @@ type Params = {
requestId: string;
};

const REGISTER_TIMEOUT_MS = 10_000;

const registerForChallenge = async ({ token, requestId }: Params) => {
const url = process.env.ID_SERVICE_URL;

if (!url) {
throw new Error('ID_SERVICE_URL is missing');
}

const registerResponse = await fetch(`${url}/v1/ext/register`, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
'X-App-Type': 'extension',
'X-App-Version': chrome.runtime.getManifest().version,
},
body: JSON.stringify({
token,
requestId,
}),
});
try {
const registerResponse = await fetch(`${url}/v1/ext/register`, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
'X-App-Type': 'extension',
'X-App-Version': chrome.runtime.getManifest().version,
},
body: JSON.stringify({
token,
requestId,
}),
signal: AbortSignal.timeout(REGISTER_TIMEOUT_MS),
});

if (!registerResponse.ok) {
if (!registerResponse.ok) {
throw new Error(registerResponse.statusText);
}
} catch (err) {
throw new Error(
`challenge registration error: "${registerResponse.statusText}"`,
`[AppCheck] challenge registration error "${(err as Error).message}"`,
);
}
};
Expand Down
27 changes: 15 additions & 12 deletions src/background/services/appcheck/utils/verifyChallenge.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ describe('verifyChallenge', () => {
} as Response);

expect(verifyChallenge(params)).rejects.toThrow(
'challenge verification error: "internal error"',
'[AppCheck] challenge verification error "internal error"',
);
});

Expand All @@ -52,17 +52,20 @@ describe('verifyChallenge', () => {
} as unknown as Response);

expect(verifyChallenge(params)).resolves.toStrictEqual({ foo: 'bar' });
expect(global.fetch).toHaveBeenCalledWith('https://id.com/v1/ext/verify', {
method: 'POST',
headers: {
'Content-Type': 'application/json',
'X-App-Type': 'extension',
'X-App-Version': '0.0.1',
},
body: JSON.stringify({
registrationId: 'registrationId',
solution: 'solution',
expect(global.fetch).toHaveBeenCalledWith(
'https://id.com/v1/ext/verify',
expect.objectContaining({
method: 'POST',
headers: {
'Content-Type': 'application/json',
'X-App-Type': 'extension',
'X-App-Version': '0.0.1',
},
body: JSON.stringify({
registrationId: 'registrationId',
solution: 'solution',
}),
}),
});
);
});
});
39 changes: 23 additions & 16 deletions src/background/services/appcheck/utils/verifyChallenge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,33 +3,40 @@ type Params = {
solution: string;
};

const VERIFY_TIMEOUT_MS = 10_000;

const verifyChallenge = async ({ registrationId, solution }: Params) => {
const url = process.env.ID_SERVICE_URL;

if (!url) {
throw new Error('ID_SERVICE_URL is missing');
}

const verifyChallengeResponse = await fetch(`${url}/v1/ext/verify`, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
'X-App-Type': 'extension',
'X-App-Version': chrome.runtime.getManifest().version,
},
body: JSON.stringify({
registrationId,
solution,
}),
});
try {
const verifyChallengeResponse = await fetch(`${url}/v1/ext/verify`, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
'X-App-Type': 'extension',
'X-App-Version': chrome.runtime.getManifest().version,
},
body: JSON.stringify({
registrationId,
solution,
}),
signal: AbortSignal.timeout(VERIFY_TIMEOUT_MS),
});

if (!verifyChallengeResponse.ok) {
throw new Error(verifyChallengeResponse.statusText);
}

if (!verifyChallengeResponse.ok) {
return verifyChallengeResponse.json();
} catch (err) {
throw new Error(
`challenge verification error: "${verifyChallengeResponse.statusText}"`,
`[AppCheck] challenge verification error "${(err as Error).message}"`,
);
}

return verifyChallengeResponse.json();
};

export default verifyChallenge;
11 changes: 11 additions & 0 deletions src/background/services/firebase/FirebaseService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@ import {
} from '../featureFlags/models';
import { deleteToken, getToken, MessagePayload } from 'firebase/messaging';
import { FcmMessageEvents, FirebaseEvents } from './models';
import { isSupportedBrowser } from '@src/utils/isSupportedBrowser';

jest.mock('firebase/app');
jest.mock('firebase/messaging');
jest.mock('firebase/messaging/sw');
jest.mock('@src/utils/isSupportedBrowser');

describe('FirebaseService', () => {
const realEnv = process.env;
Expand All @@ -31,6 +33,7 @@ describe('FirebaseService', () => {
jest.resetAllMocks();
jest.mocked(initializeApp).mockReturnValue(appMock);
jest.mocked(getMessaging).mockReturnValue(messagingMock);
jest.mocked(isSupportedBrowser).mockReturnValue(true);

process.env = {
...realEnv,
Expand All @@ -52,6 +55,14 @@ describe('FirebaseService', () => {
);
});

it('does not initialize when the browser is not supported', () => {
jest.mocked(isSupportedBrowser).mockReturnValueOnce(false);

const firebaseService = new FirebaseService(featureFlagService);
expect(firebaseService.getFirebaseApp()).toBeUndefined();
expect(initializeApp).not.toHaveBeenCalled();
});

it('initializes correctly', () => {
const firebaseService = new FirebaseService(featureFlagService);

Expand Down
7 changes: 6 additions & 1 deletion src/background/services/firebase/FirebaseService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@ import sentryCaptureException, {
} from '@src/monitoring/sentryCaptureException';
import { FeatureFlagService } from '../featureFlags/FeatureFlagService';
import { FeatureFlagEvents, FeatureGates } from '../featureFlags/models';
import { isSupportedBrowser } from '@src/utils/isSupportedBrowser';

@singleton()
export class FirebaseService {
#app: FirebaseApp;
#app?: FirebaseApp;
#isFcmInitialized = false;
#fcmToken?: string;
#firebaseEventEmitter = new EventEmitter();
Expand All @@ -27,6 +28,10 @@ export class FirebaseService {
throw new Error('FIREBASE_CONFIG is missing');
}

if (!isSupportedBrowser()) {
return;
}

this.#app = initializeApp(
JSON.parse(Buffer.from(process.env.FIREBASE_CONFIG, 'base64').toString()),
);
Expand Down
2 changes: 2 additions & 0 deletions src/monitoring/sharedSentryConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ const sharedSentryConfig = {
/^.*The user aborted a request\.$/, // ignore errors caused by chrome's throttling
/^.*could not detect network.*$/, // ignore ethers provider connection errors
/^.*Failed to fetch$/, // ignore network errors
/AbortError: Registration failed - push service error/, // ignore push service FCM registration failures
/^.*NotAllowedError: Registration failed - permission denied/, // ignore push service FCM registration failures
],
};

Expand Down
10 changes: 10 additions & 0 deletions src/tests/setupTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,16 @@ import crypto from 'node:crypto';
global.TextEncoder = MockTextEncoder;
global.TextDecoder = TextDecoder as any;

// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
global.AbortSignal = {
timeout(ms: number) {
const controller = new AbortController();
setTimeout(() => controller.abort(new DOMException('TimeoutError')), ms);
return controller.signal;
},
};

Object.defineProperties(global.crypto, {
randomUUID: {
value: jest.fn().mockReturnValue('00000000-0000-0000-0000-000000000000'),
Expand Down
30 changes: 30 additions & 0 deletions src/utils/isSupportedBrowser.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { BrowserInfo, detect } from 'detect-browser';
import { isSupportedBrowser, supportedBrowsers } from './isSupportedBrowser';

jest.mock('detect-browser');

describe('isSupportedBrowser', () => {
beforeEach(() => {
jest.resetAllMocks();
});

it('returns false for unsupported browser', () => {
jest
.mocked(detect)
.mockReturnValueOnce({ name: 'unusupported' } as unknown as BrowserInfo);

expect(isSupportedBrowser()).toBe(false);
});

it.each(
supportedBrowsers.map((supportedBrowser) => ({
type: supportedBrowser,
})),
)('returns true for $type', ({ type }) => {
jest
.mocked(detect)
.mockReturnValueOnce({ name: type } as unknown as BrowserInfo);

expect(isSupportedBrowser()).toBe(true);
});
});
11 changes: 11 additions & 0 deletions src/utils/isSupportedBrowser.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { Browser, detect } from 'detect-browser';

export const supportedBrowsers: Browser[] = ['chrome'];
export const isSupportedBrowser = () => {
const browser = detect();
const isSupported = supportedBrowsers.includes(
(browser?.name ?? '') as Browser,
);

return isSupported;
};
Loading