From c7fbd7f30ca270ef9d22d58c231c1e5b1c801d33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timoth=C3=A9e=20Isnard?= Date: Sun, 7 Jan 2024 21:16:19 +0100 Subject: [PATCH 1/4] feat: Add Sentry tags with Tanker info on outbound exceptions We now take an (optional) Sentry Hub object as Tanker option. When an exception bubbles up through a running Session, we add tags to the current Sentry Hub before re-throwing it. If the option is left undefined, we try to get Sentry from globalThis (but it can be explicitly set to null to disable our Sentry usage) --- package-lock.json | 12 +++++++++++- packages/core/package.json | 1 + packages/core/src/Network/Client.ts | 4 ++++ packages/core/src/Session/Session.ts | 21 ++++++++++++++++++--- packages/core/src/Tanker.ts | 19 ++++++++++++++++--- packages/global-this/package.json | 3 ++- packages/global-this/src/index.ts | 5 ++++- 7 files changed, 56 insertions(+), 9 deletions(-) diff --git a/package-lock.json b/package-lock.json index 4db2c9c7..55cc0500 100644 --- a/package-lock.json +++ b/package-lock.json @@ -888,6 +888,14 @@ "node": ">= 8" } }, + "node_modules/@sentry/types": { + "version": "7.76.0", + "resolved": "https://registry.npmjs.org/@sentry/types/-/types-7.76.0.tgz", + "integrity": "sha512-vj6z+EAbVrKAXmJPxSv/clpwS9QjPqzkraMFk2hIdE/kii8s8kwnkBwTSpIrNc8GnzV3qYC4r3qD+BXDxAGPaw==", + "engines": { + "node": ">=8" + } + }, "node_modules/@sinonjs/commons": { "version": "1.8.6", "resolved": "https://registry.npmjs.org/@sinonjs/commons/-/commons-1.8.6.tgz", @@ -10109,7 +10117,7 @@ "@tanker/datastore-pouchdb-memory": "0.0.1" }, "engines": { - "node": ">=14" + "node": ">=18" } }, "packages/core": { @@ -10117,6 +10125,7 @@ "version": "0.0.1", "license": "Apache-2.0", "dependencies": { + "@sentry/types": "7.76.0", "@tanker/crypto": "0.0.1", "@tanker/datastore-base": "0.0.1", "@tanker/errors": "0.0.1", @@ -10317,6 +10326,7 @@ "version": "0.0.1", "license": "Apache-2.0", "dependencies": { + "@sentry/types": "7.76.0", "tslib": "^2.3.1" }, "devDependencies": { diff --git a/packages/core/package.json b/packages/core/package.json index 63d8b15d..e303b509 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -32,6 +32,7 @@ "@tanker/stream-cloud-storage": "0.0.1", "@tanker/types": "0.0.1", "@types/varint": "^6.0.0", + "@sentry/types": "7.76.0", "tslib": "^2.3.1", "varint": "^6.0.0" }, diff --git a/packages/core/src/Network/Client.ts b/packages/core/src/Network/Client.ts index 63972623..635ef68b 100644 --- a/packages/core/src/Network/Client.ts +++ b/packages/core/src/Network/Client.ts @@ -81,6 +81,10 @@ export class Client { this._userId = userId; } + get instanceId(): string { + return this._instanceId; + } + _cancelable = (fun: (...args: Array) => Promise) => (...args: Array) => { // cancelationHandle.promise always rejects. Its returned type doesn't matter if (this._cancelationHandle.settled) { diff --git a/packages/core/src/Session/Session.ts b/packages/core/src/Session/Session.ts index ebfac03d..f816382b 100644 --- a/packages/core/src/Session/Session.ts +++ b/packages/core/src/Session/Session.ts @@ -1,6 +1,8 @@ import EventEmitter from 'events'; +import type { Hub } from '@sentry/types'; import { TankerError, ExpiredVerification, InternalError, InvalidArgument, InvalidVerification, NetworkError, OperationCanceled, PreconditionFailed, TooManyAttempts } from '@tanker/errors'; +import { utils } from '@tanker/crypto'; import type { DataStoreOptions } from './Storage'; import { Storage } from './Storage'; @@ -22,6 +24,7 @@ import { SessionManager } from '../TransparentSession/Manager'; export class Session extends EventEmitter { _storage: Storage; _client: Client; + _sentry: Hub | null; _localUserManager: LocalUserManager; _userManager: UserManager; @@ -34,12 +37,13 @@ export class Session extends EventEmitter { _status: Status; - constructor(userData: UserData, storage: Storage, oidcNonceManagerGetter: () => Promise, client: Client) { + constructor(userData: UserData, storage: Storage, oidcNonceManagerGetter: () => Promise, client: Client, sentry: Hub | null) { super(); this._storage = storage; this._client = client; this._status = Status.STOPPED; + this._sentry = sentry; this._localUserManager = new LocalUserManager(userData, oidcNonceManagerGetter, client, storage.keyStore); this._localUserManager.on('error', async (e: Error) => { @@ -72,6 +76,10 @@ export class Session extends EventEmitter { return this._status; } + get statusName(): string { + return Status[this.status]; + } + set status(nextStatus: Status) { if (nextStatus !== this._status) { this._status = nextStatus; @@ -79,13 +87,13 @@ export class Session extends EventEmitter { } } - static init = async (userData: UserData, oidcNonceManagerGetter: () => Promise, storeOptions: DataStoreOptions, clientOptions: ClientOptions): Promise => { + static init = async (userData: UserData, oidcNonceManagerGetter: () => Promise, storeOptions: DataStoreOptions, clientOptions: ClientOptions, sentry: Hub | null): Promise => { const client = new Client(userData.trustchainId, userData.userId, clientOptions); const storage = new Storage(storeOptions); await storage.open(userData.userId, userData.userSecret); - return new Session(userData, storage, oidcNonceManagerGetter, client); + return new Session(userData, storage, oidcNonceManagerGetter, client, sentry); }; start = async (): Promise => { @@ -123,6 +131,13 @@ export class Session extends EventEmitter { try { return await manager[func].call(manager, ...args); } catch (e) { + const localUser = this._localUserManager.localUser; + this._sentry?.setTag('tanker_app_id', utils.toBase64(localUser.trustchainId)); + this._sentry?.setTag('tanker_user_id', utils.toBase64(localUser.userId)); + this._sentry?.setTag('tanker_device_id', utils.toBase64(localUser.deviceId)); + this._sentry?.setTag('tanker_instance_id', this._client.instanceId); + this._sentry?.setTag('tanker_status', this.statusName); + await this._handleUnrecoverableError(e); throw e; } diff --git a/packages/core/src/Tanker.ts b/packages/core/src/Tanker.ts index 1b5c4cfe..506abb0f 100644 --- a/packages/core/src/Tanker.ts +++ b/packages/core/src/Tanker.ts @@ -2,8 +2,10 @@ import EventEmitter from 'events'; import type { b64string, EncryptionStream, DecryptionStream } from '@tanker/crypto'; import { randomBase64Token, ready as cryptoReady, tcrypto, utils, extractEncryptionFormat, SAFE_EXTRACTION_LENGTH, assertResourceId } from '@tanker/crypto'; import { InternalError, InvalidArgument } from '@tanker/errors'; +import { globalThis } from '@tanker/global-this'; import { assertDataType, assertInteger, assertNotEmptyString, castData } from '@tanker/types'; import type { Data, ResourceMetadata } from '@tanker/types'; +import type { Hub } from '@sentry/types'; import { _deserializeProvisionalIdentity, isSecretProvisionalIdentity } from './Identity'; import type { ClientOptions } from './Network/Client'; @@ -60,6 +62,9 @@ export type TankerCoreOptions = { url?: string; dataStore: DataStoreOptions; sdkType: string; + /// Setting this to null explicitly turns off Sentry integration. + /// If left undefined, we try to find Sentry ourselves. + sentryHub?: Hub | null, }; export type TankerOptions = Partial & { dataStore: Partial; }>; @@ -94,6 +99,7 @@ export class Tanker extends EventEmitter { _clientOptions: ClientOptions; _dataStoreOptions: DataStoreOptions; _localDeviceLock: Lock; + _sentry: Hub | null; static version = TANKER_SDK_VERSION; static statuses = statuses; @@ -118,6 +124,12 @@ export class Tanker extends EventEmitter { throw new InvalidArgument('options.dataStore.adapter', 'function', options.dataStore.adapter); } + if (options.sentryHub === undefined) { + this._sentry = globalThis.Sentry?.getCurrentHub() || null; + } else { + this._sentry = options.sentryHub; + } + assertNotEmptyString(options.sdkType, 'options.sdkType'); this._localDeviceLock = new Lock(); this._options = options; @@ -172,8 +184,9 @@ export class Tanker extends EventEmitter { } get statusName(): string { - const def = statusDefs[this.status]; - return def && def.name || `invalid status: ${this.status}`; + if (!this._session) + return statusDefs[statuses.STOPPED]!.name; + return this._session.statusName; } override addListener(eventName: string, listener: (...args: Array) => void) { @@ -271,7 +284,7 @@ export class Tanker extends EventEmitter { const session = await Session.init(userData, async () => { await this._initUnauthSession(); return this._unauthSession!.getOidcNonceManager(); - }, this._dataStoreOptions, this._clientOptions); + }, this._dataStoreOptions, this._clientOptions, this._sentry); // Watch and start the session session.on('status_change', s => this.emit('statusChange', s)); await session.start(); diff --git a/packages/global-this/package.json b/packages/global-this/package.json index c41a059f..b838bc16 100644 --- a/packages/global-this/package.json +++ b/packages/global-this/package.json @@ -23,7 +23,8 @@ "test": "cd ../..; npm run test:global-this" }, "dependencies": { - "tslib": "^2.3.1" + "tslib": "^2.3.1", + "@sentry/types": "7.76.0" }, "devDependencies": { "@tanker/test-utils": "0.0.1" diff --git a/packages/global-this/src/index.ts b/packages/global-this/src/index.ts index acfeab19..58e70637 100644 --- a/packages/global-this/src/index.ts +++ b/packages/global-this/src/index.ts @@ -1,5 +1,8 @@ import { getGlobalThis } from './global-this'; +import type { Hub } from '@sentry/types'; -const myGlobalThis: typeof globalThis = getGlobalThis(); +const myGlobalThis: typeof globalThis & { + Sentry?: { getCurrentHub: () => Hub }; +} = getGlobalThis(); export { myGlobalThis as globalThis }; From bf88db03b49d06ea97e9357837792f3c08ee6319 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timoth=C3=A9e=20Isnard?= Date: Sun, 7 Jan 2024 21:16:26 +0100 Subject: [PATCH 2/4] feat: Add Sentry breadcrumbs for resource key cache operations We leave a warning for resources not found at all, and info loglevel for success, so this is filterable later in case the logs create too much noise (the Sentry UI has a filter, or even using sentry filter hooks which lets people adjust the amount of information sent without having to release a new sdk-js) Note that the resource IDs we log are _simple_ resource IDs, so in the case of a composite resource that failed to decrypt, you will see two warnings for key not found with different simple IDs, while the 'could not find key for resource' will show the composite ID. --- packages/core/src/Resources/Manager.ts | 21 +++++++++++++++++++++ packages/core/src/Session/Session.ts | 2 +- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/packages/core/src/Resources/Manager.ts b/packages/core/src/Resources/Manager.ts index 41d23d71..6adef0df 100644 --- a/packages/core/src/Resources/Manager.ts +++ b/packages/core/src/Resources/Manager.ts @@ -2,6 +2,7 @@ import type { Key, b64string } from '@tanker/crypto'; import { utils } from '@tanker/crypto'; import { UpgradeRequired } from '@tanker/errors'; import { errors as dbErrors } from '@tanker/datastore-base'; +import type { Hub } from '@sentry/types'; import { getKeyPublishEntryFromBlock } from './Serialize'; import { KeyDecryptor } from './KeyDecryptor'; @@ -23,6 +24,7 @@ export class ResourceManager { declare _keyDecryptor: KeyDecryptor; declare _keyLookupCoalescer: TaskCoalescer; declare _resourceStore: ResourceStore; + declare _sentry: Hub | null; constructor( client: Client, @@ -30,11 +32,13 @@ export class ResourceManager { localUserManager: LocalUserManager, groupManager: GroupManager, provisionalIdentityManager: ProvisionalIdentityManager, + sentry: Hub | null, ) { this._client = client; this._keyDecryptor = new KeyDecryptor(localUserManager, groupManager, provisionalIdentityManager); this._keyLookupCoalescer = new TaskCoalescer(); this._resourceStore = resourceStore; + this._sentry = sentry; } async findKeyFromResourceId(resourceId: Uint8Array): Promise { @@ -52,11 +56,28 @@ export class ResourceManager { if (!resourceKey) { const keyPublishBlock = await this._client.getResourceKey(resourceId); if (!keyPublishBlock) { + this._sentry?.addBreadcrumb({ + category: 'tanker_keystore', + level: 'warning', + message: `Key not found in either cache or server for ${b64resourceId}`, + }); return { id: b64resourceId, key: null }; } + this._sentry?.addBreadcrumb({ + category: 'tanker_keystore', + level: 'debug', + message: `Tanker key not found in cache, but fetched from server for ${b64resourceId}`, + }); + const keyPublish = getKeyPublishEntryFromBlock(keyPublishBlock); resourceKey = await this._keyDecryptor.keyFromKeyPublish(keyPublish); await this._resourceStore.saveResourceKey(resourceId, resourceKey); + } else { + this._sentry?.addBreadcrumb({ + category: 'tanker_keystore', + level: 'debug', + message: `Tanker key found in cache for ${b64resourceId}`, + }); } return { id: b64resourceId, key: resourceKey }; diff --git a/packages/core/src/Session/Session.ts b/packages/core/src/Session/Session.ts index f816382b..68e4df73 100644 --- a/packages/core/src/Session/Session.ts +++ b/packages/core/src/Session/Session.ts @@ -66,7 +66,7 @@ export class Session extends EventEmitter { this._userManager = new UserManager(client, this._localUserManager.localUser); this._provisionalIdentityManager = new ProvisionalIdentityManager(client, storage.keyStore, this._localUserManager, this._userManager); this._groupManager = new GroupManager(client, storage.groupStore, this._localUserManager.localUser, this._userManager, this._provisionalIdentityManager); - this._resourceManager = new ResourceManager(client, storage.resourceStore, this._localUserManager, this._groupManager, this._provisionalIdentityManager); + this._resourceManager = new ResourceManager(client, storage.resourceStore, this._localUserManager, this._groupManager, this._provisionalIdentityManager, this._sentry); this._sessionManager = new SessionManager(storage.sessionStore); this._dataProtector = new DataProtector(client, this._localUserManager.localUser, this._userManager, this._provisionalIdentityManager, this._groupManager, this._resourceManager, this._sessionManager); this._cloudStorageManager = new CloudStorageManager(client, this._dataProtector); From d966da5d8b359c0b92943c195a7b0b4a350635e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timoth=C3=A9e=20Isnard?= Date: Fri, 5 Jan 2024 09:10:01 +0100 Subject: [PATCH 3/4] test: Sentry integration adding breadcrumbs and tags with a mock --- package-lock.json | 1 + packages/functional-tests/package.json | 1 + .../src/__tests__/client.spec.node.ts | 5 +- .../src/__tests__/client.spec.web.ts | 5 +- .../functional-tests/src/helpers/Device.ts | 7 +- .../functional-tests/src/helpers/TestArgs.ts | 4 +- packages/functional-tests/src/index.ts | 8 +- packages/functional-tests/src/sentry.ts | 105 ++++++++++++++++++ 8 files changed, 124 insertions(+), 12 deletions(-) create mode 100644 packages/functional-tests/src/sentry.ts diff --git a/package-lock.json b/package-lock.json index 55cc0500..2ee06507 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10308,6 +10308,7 @@ "version": "0.0.1", "license": "UNLICENSED", "dependencies": { + "@sentry/types": "7.76.0", "@tanker/client-browser": "0.0.1", "@tanker/client-node": "0.0.1", "@tanker/core": "0.0.1", diff --git a/packages/functional-tests/package.json b/packages/functional-tests/package.json index e8269c5f..6e5d4dc4 100644 --- a/packages/functional-tests/package.json +++ b/packages/functional-tests/package.json @@ -12,6 +12,7 @@ "package.json" ], "dependencies": { + "@sentry/types": "7.76.0", "@tanker/client-browser": "0.0.1", "@tanker/client-node": "0.0.1", "@tanker/core": "0.0.1", diff --git a/packages/functional-tests/src/__tests__/client.spec.node.ts b/packages/functional-tests/src/__tests__/client.spec.node.ts index d809d46a..022b7cfe 100644 --- a/packages/functional-tests/src/__tests__/client.spec.node.ts +++ b/packages/functional-tests/src/__tests__/client.spec.node.ts @@ -1,18 +1,19 @@ import Tanker from '@tanker/client-node'; import { pouchDBMemory } from '@tanker/datastore-pouchdb-memory'; -import type { b64string } from '@tanker/core'; +import type { TankerOptions, b64string } from '@tanker/core'; import type { DefaultDownloadType, TestResources } from '../helpers'; import { appdUrl, makeRandomUint8Array } from '../helpers'; import { generateFunctionalTests } from '..'; -const makeTanker = (appId: b64string, storagePrefix: string): Tanker => { +const makeTanker = (appId: b64string, storagePrefix: string, extraOpts: TankerOptions): Tanker => { const tanker = new Tanker({ appId, dataStore: { adapter: pouchDBMemory, prefix: storagePrefix }, sdkType: 'js-functional-tests-node', url: appdUrl, + ...extraOpts, }); return tanker; diff --git a/packages/functional-tests/src/__tests__/client.spec.web.ts b/packages/functional-tests/src/__tests__/client.spec.web.ts index eb62d398..3f345351 100644 --- a/packages/functional-tests/src/__tests__/client.spec.web.ts +++ b/packages/functional-tests/src/__tests__/client.spec.web.ts @@ -1,17 +1,18 @@ import Tanker from '@tanker/client-browser'; -import type { b64string } from '@tanker/core'; +import type { TankerOptions, b64string } from '@tanker/core'; import type { DefaultDownloadType, TestResources } from '../helpers'; import { appdUrl, makeRandomUint8Array } from '../helpers'; import { generateFunctionalTests } from '..'; -const makeTanker = (appId: b64string, storagePrefix: string): Tanker => { +const makeTanker = (appId: b64string, storagePrefix: string, extraOpts: TankerOptions): Tanker => { const tanker = new Tanker({ appId, dataStore: { prefix: storagePrefix }, sdkType: 'js-functional-tests-web', url: appdUrl, + ...extraOpts, }); return tanker; diff --git a/packages/functional-tests/src/helpers/Device.ts b/packages/functional-tests/src/helpers/Device.ts index d75e1002..30583247 100644 --- a/packages/functional-tests/src/helpers/Device.ts +++ b/packages/functional-tests/src/helpers/Device.ts @@ -1,7 +1,7 @@ -import { Tanker } from '@tanker/core'; +import { Tanker, type TankerOptions } from '@tanker/core'; import { uuid } from '@tanker/test-utils'; -export type TankerFactory = (appId: string, storagePrefix: string) => Tanker; +export type TankerFactory = (appId: string, storagePrefix: string, extraOpts: TankerOptions) => Tanker; const VERIFICATION = { passphrase: 'passphrase' }; @@ -18,7 +18,7 @@ export class Device { this.storagePrefix = storagePrefix; } - static async create(makeTanker: (appId: string, storagePrefix: string) => Tanker, appId: string, identity: string): Promise { + static async create(makeTanker: TankerFactory, appId: string, identity: string): Promise { return new Device(makeTanker, appId, identity, uuid.v4()); } @@ -26,6 +26,7 @@ export class Device { const tanker = this.makeTanker( this.appId, this.storagePrefix, + {}, ); const status = await tanker.start(this.identity); if (status === Tanker.statuses.IDENTITY_REGISTRATION_NEEDED) diff --git a/packages/functional-tests/src/helpers/TestArgs.ts b/packages/functional-tests/src/helpers/TestArgs.ts index ef3fd4ae..fe020f53 100644 --- a/packages/functional-tests/src/helpers/TestArgs.ts +++ b/packages/functional-tests/src/helpers/TestArgs.ts @@ -1,4 +1,4 @@ -import type { Tanker } from '@tanker/core'; +import type { Tanker, TankerOptions } from '@tanker/core'; import type { Class, Data } from '@tanker/types'; import type { AppHelper } from './AppHelper'; @@ -12,5 +12,5 @@ export type TestArgs = { appHelper: AppHelper; resources: TestResources; defaultDownloadType: DefaultDownloadType; - makeTanker: (b64AppId?: string) => Tanker; + makeTanker: (b64AppId?: string, extraOpts?: TankerOptions) => Tanker; }; diff --git a/packages/functional-tests/src/index.ts b/packages/functional-tests/src/index.ts index 4e82b0d1..abc52628 100644 --- a/packages/functional-tests/src/index.ts +++ b/packages/functional-tests/src/index.ts @@ -1,5 +1,5 @@ import { ready as cryptoReady, utils } from '@tanker/crypto'; -import type { Tanker, b64string } from '@tanker/core'; +import type { Tanker, TankerOptions, b64string } from '@tanker/core'; import { silencer } from '@tanker/test-utils'; import { makePrefix, AppHelper, appdUrl, managementSettings, oidcSettings, trustchaindUrl } from './helpers'; @@ -15,11 +15,12 @@ import { generateSessionTests } from './session'; import { generateUploadTests } from './upload'; import { generateVerificationTests } from './verification'; import { generateConcurrencyTests } from './concurrency'; +import { generateSentryTests } from './sentry'; import { generateSessionTokenTests } from './sessionToken'; export function generateFunctionalTests( name: string, - makeTanker: (appId: b64string, storagePrefix: string) => Tanker, + makeTanker: (appId: b64string, storagePrefix: string, extraOpts: TankerOptions) => Tanker, generateTestResources: () => { resources: TestResources; defaultDownloadType: DefaultDownloadType }, ) { if (!appdUrl || !managementSettings || !oidcSettings || !trustchaindUrl) { @@ -50,7 +51,7 @@ export function generateFunctionalTests( args.appHelper = await AppHelper.newApp(makeTanker); const b64DefaultAppId = utils.toBase64(args.appHelper.appId); - args.makeTanker = (b64AppId = b64DefaultAppId) => makeTanker(b64AppId, makePrefix()); + args.makeTanker = (b64AppId = b64DefaultAppId, extraOpts = {}) => makeTanker(b64AppId, makePrefix(), extraOpts); silencer.silence('warn', /deprecated/); }); @@ -73,6 +74,7 @@ export function generateFunctionalTests( generateUploadTests(args); generateNetworkTests(args); generateConcurrencyTests(args); + generateSentryTests(args); generateSessionTokenTests(args); }); } diff --git a/packages/functional-tests/src/sentry.ts b/packages/functional-tests/src/sentry.ts new file mode 100644 index 00000000..a2437790 --- /dev/null +++ b/packages/functional-tests/src/sentry.ts @@ -0,0 +1,105 @@ +import { Tanker, type b64string } from '@tanker/core'; +import { expect } from '@tanker/test-utils'; +import { errors } from '@tanker/core'; +import { utils } from '@tanker/crypto'; + +import type { TestArgs, AppHelper } from './helpers'; +import type { Breadcrumb, BreadcrumbHint, Primitive } from '@sentry/types'; +import { getPublicIdentity } from '@tanker/identity'; + +class MockSentryHub { + breadcrumbs: Array; + tags: { [key: string]: Primitive }; + + constructor() { + this.breadcrumbs = []; + this.tags = {}; + } + + addBreadcrumb = (breadcrumb: Breadcrumb, _?: BreadcrumbHint) => { + this.breadcrumbs.push(breadcrumb); + }; + + setTag = (key: string, value: Primitive) => { + this.tags[key] = value; + }; +} + +export const generateSentryTests = (args: TestArgs) => { + const fakeMissingResource = utils.fromBase64('CrrQdawRM9/icauwqmrgFiHal4v3uMQnqptJcz4nOCV1Lag+RKvttOr6XAzfQSQai9PGtoi5hLcELy+e'); + + describe('Sentry integration', () => { + let hub: MockSentryHub; + let alice: Tanker; + let aliceIdentity: b64string; + let appHelper: AppHelper; + + before(async () => { + ({ appHelper } = args); + }); + + beforeEach(async () => { + hub = new MockSentryHub(); + // @ts-expect-error Mock doesn't implement the full interface + alice = args.makeTanker(undefined, { sentryHub: hub }); + aliceIdentity = await appHelper.generateIdentity(); + await alice.start(aliceIdentity); + await alice.registerIdentity({ passphrase: 'passphrase' }); + }); + + afterEach(async () => { + await alice.stop(); + }); + + it("doesn't set tags when everything goes well", async () => { + const encrypted = await alice.encrypt('foo'); + await alice.decrypt(encrypted); + expect(hub.tags).to.deep.equal({}); + }); + + it('sets tags when decryption fails', async () => { + await expect(alice.decrypt(fakeMissingResource)).to.be.rejectedWith(errors.InvalidArgument); + + const aliceUserId = utils.fromB64Json(aliceIdentity)['value']; + + expect(hub.tags['tanker_app_id']).to.equal(utils.toBase64(appHelper.appId)); + expect(hub.tags['tanker_user_id']).to.equal(aliceUserId); + expect(hub.tags['tanker_status']).to.equal('READY'); + }); + + it('logs a breadcrumb when decryption fails', async () => { + await expect(alice.decrypt(fakeMissingResource)).to.be.rejectedWith(errors.InvalidArgument); + + expect(hub.breadcrumbs).to.have.lengthOf(2); + expect(hub.breadcrumbs[0]?.message).to.contain('Key not found'); // Transparent session key + expect(hub.breadcrumbs[1]?.message).to.contain('Key not found'); // Individual resource key + }); + + it('keeps breadcrumbs of previous operations', async () => { + const encryptedGood = await alice.encrypt('good'); + await alice.decrypt(encryptedGood); + + await expect(alice.decrypt(fakeMissingResource)).to.be.rejectedWith(errors.InvalidArgument); + + expect(hub.breadcrumbs).to.have.lengthOf(3); + expect(hub.breadcrumbs[0]?.message).to.contain('Tanker key found in cache'); // 1st decrypt key found + expect(hub.breadcrumbs[1]?.message).to.contain('Key not found'); // 2nd transparent session key + expect(hub.breadcrumbs[2]?.message).to.contain('Key not found'); // 2nd individual resource key + }); + + it('logs a breadcrumb when decrypting with a key fetched from the server', async () => { + const bob = args.makeTanker(); + await bob.start(await appHelper.generateIdentity()); + await bob.registerIdentity({ passphrase: 'passphrase' }); + const options = { + shareWithUsers: [await getPublicIdentity(aliceIdentity)], + }; + const encrypted = await bob.encrypt('foo', options); + + await alice.decrypt(encrypted); + + expect(hub.breadcrumbs).to.have.lengthOf(1); + expect(hub.breadcrumbs[0]?.message).to.contain('Tanker key not found in cache, but fetched from server'); + }); + }); +}; From 8040eb5c091e2d60009da7ce8ff5ef06a1922de3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timoth=C3=A9e=20Isnard?= Date: Fri, 5 Jan 2024 16:45:30 +0100 Subject: [PATCH 4/4] feat: Sentry limiter: max 20 Tanker breadcrumbs per Sentry event This avoids too much flood, in case there is an exception on a page where the user decrypts a lot of small values (e.g. agenda) --- packages/core/src/Resources/Manager.ts | 6 +-- packages/core/src/SentryLimiter.ts | 29 ++++++++++++ packages/core/src/Session/Session.ts | 6 ++- .../core/src/__tests__/SentryLimiter.spec.ts | 44 +++++++++++++++++++ packages/functional-tests/src/sentry.ts | 7 ++- 5 files changed, 86 insertions(+), 6 deletions(-) create mode 100644 packages/core/src/SentryLimiter.ts create mode 100644 packages/core/src/__tests__/SentryLimiter.spec.ts diff --git a/packages/core/src/Resources/Manager.ts b/packages/core/src/Resources/Manager.ts index 6adef0df..ae10f6cd 100644 --- a/packages/core/src/Resources/Manager.ts +++ b/packages/core/src/Resources/Manager.ts @@ -2,7 +2,6 @@ import type { Key, b64string } from '@tanker/crypto'; import { utils } from '@tanker/crypto'; import { UpgradeRequired } from '@tanker/errors'; import { errors as dbErrors } from '@tanker/datastore-base'; -import type { Hub } from '@sentry/types'; import { getKeyPublishEntryFromBlock } from './Serialize'; import { KeyDecryptor } from './KeyDecryptor'; @@ -13,6 +12,7 @@ import type { ResourceStore } from './ResourceStore'; import type { LocalUserManager } from '../LocalUser/Manager'; import type { GroupManager } from '../Groups/Manager'; import type { ProvisionalIdentityManager } from '../ProvisionalIdentity/Manager'; +import { SentryLimiter } from '../SentryLimiter'; export type KeyResult = { id: b64string; @@ -24,7 +24,7 @@ export class ResourceManager { declare _keyDecryptor: KeyDecryptor; declare _keyLookupCoalescer: TaskCoalescer; declare _resourceStore: ResourceStore; - declare _sentry: Hub | null; + declare _sentry: SentryLimiter | null; constructor( client: Client, @@ -32,7 +32,7 @@ export class ResourceManager { localUserManager: LocalUserManager, groupManager: GroupManager, provisionalIdentityManager: ProvisionalIdentityManager, - sentry: Hub | null, + sentry: SentryLimiter | null, ) { this._client = client; this._keyDecryptor = new KeyDecryptor(localUserManager, groupManager, provisionalIdentityManager); diff --git a/packages/core/src/SentryLimiter.ts b/packages/core/src/SentryLimiter.ts new file mode 100644 index 00000000..900212f7 --- /dev/null +++ b/packages/core/src/SentryLimiter.ts @@ -0,0 +1,29 @@ +import type { Breadcrumb, Hub } from '@sentry/types'; + +export const BREADCRUMB_LIMIT = 20; + +export class SentryLimiter { + breadcrumbs: Array; + sentryHub: Hub; + + constructor(sentryHub: Hub) { + this.breadcrumbs = []; + this.sentryHub = sentryHub; + } + + addBreadcrumb = (breadcrumb: Breadcrumb) => { + if (this.breadcrumbs.length == BREADCRUMB_LIMIT) + this.breadcrumbs.shift(); + + this.breadcrumbs.push({ + timestamp: Math.floor(Date.now() / 1000), + ...breadcrumb, + }); + }; + + flush = () => { + for (const breadcrumb of this.breadcrumbs) + this.sentryHub.addBreadcrumb(breadcrumb, undefined); + this.breadcrumbs = []; + }; +} diff --git a/packages/core/src/Session/Session.ts b/packages/core/src/Session/Session.ts index 68e4df73..63196aa4 100644 --- a/packages/core/src/Session/Session.ts +++ b/packages/core/src/Session/Session.ts @@ -20,11 +20,13 @@ import { ResourceManager } from '../Resources/Manager'; import { DataProtector } from '../DataProtection/DataProtector'; import type { OidcNonceManager } from '../OidcNonce/Manager'; import { SessionManager } from '../TransparentSession/Manager'; +import { SentryLimiter } from '../SentryLimiter'; export class Session extends EventEmitter { _storage: Storage; _client: Client; _sentry: Hub | null; + _sentryLimiter: SentryLimiter | null; _localUserManager: LocalUserManager; _userManager: UserManager; @@ -44,6 +46,7 @@ export class Session extends EventEmitter { this._client = client; this._status = Status.STOPPED; this._sentry = sentry; + this._sentryLimiter = sentry ? new SentryLimiter(sentry) : null; this._localUserManager = new LocalUserManager(userData, oidcNonceManagerGetter, client, storage.keyStore); this._localUserManager.on('error', async (e: Error) => { @@ -66,7 +69,7 @@ export class Session extends EventEmitter { this._userManager = new UserManager(client, this._localUserManager.localUser); this._provisionalIdentityManager = new ProvisionalIdentityManager(client, storage.keyStore, this._localUserManager, this._userManager); this._groupManager = new GroupManager(client, storage.groupStore, this._localUserManager.localUser, this._userManager, this._provisionalIdentityManager); - this._resourceManager = new ResourceManager(client, storage.resourceStore, this._localUserManager, this._groupManager, this._provisionalIdentityManager, this._sentry); + this._resourceManager = new ResourceManager(client, storage.resourceStore, this._localUserManager, this._groupManager, this._provisionalIdentityManager, this._sentryLimiter); this._sessionManager = new SessionManager(storage.sessionStore); this._dataProtector = new DataProtector(client, this._localUserManager.localUser, this._userManager, this._provisionalIdentityManager, this._groupManager, this._resourceManager, this._sessionManager); this._cloudStorageManager = new CloudStorageManager(client, this._dataProtector); @@ -137,6 +140,7 @@ export class Session extends EventEmitter { this._sentry?.setTag('tanker_device_id', utils.toBase64(localUser.deviceId)); this._sentry?.setTag('tanker_instance_id', this._client.instanceId); this._sentry?.setTag('tanker_status', this.statusName); + this._sentryLimiter?.flush(); await this._handleUnrecoverableError(e); throw e; diff --git a/packages/core/src/__tests__/SentryLimiter.spec.ts b/packages/core/src/__tests__/SentryLimiter.spec.ts new file mode 100644 index 00000000..d253fc9a --- /dev/null +++ b/packages/core/src/__tests__/SentryLimiter.spec.ts @@ -0,0 +1,44 @@ +import { expect } from '@tanker/test-utils'; +import { BREADCRUMB_LIMIT, SentryLimiter } from '../SentryLimiter'; +import type { Breadcrumb } from '@sentry/types'; + +describe('SentryLimiter', () => { + it('adds timestamp on breadcrumbs', async () => { + // @ts-expect-error Not using a real Hub object + const limiter = new SentryLimiter({ + addBreadcrumb: (breadcrumb: Breadcrumb) => { + expect(breadcrumb.timestamp).to.be.a('number'); + expect(breadcrumb.message).to.equal('plop'); + }, + }); + + limiter.addBreadcrumb({ + message: 'plop', + // no timestamp + }); + }); + + it('only keeps the last BREADCRUMB_LIMIT breadcrumbs', async () => { + let result: Array = []; + // @ts-expect-error Not using a real Hub object + const limiter = new SentryLimiter({ + addBreadcrumb: (b: Breadcrumb) => result.push(b), + }); + + const NUM_TO_DROP = 10; + for (let i = 0; i < BREADCRUMB_LIMIT + NUM_TO_DROP; i += 1) { + limiter.addBreadcrumb({ + level: 'info', + message: `${i}`, + }); + } + expect(result.length).to.equal(0); + + limiter.flush(); + expect(result.length).to.equal(BREADCRUMB_LIMIT); + result.forEach((b, idx) => { + expect(b.level).to.equal('info'); + expect(b.message).to.equal(`${NUM_TO_DROP + idx}`); + }); + }); +}); diff --git a/packages/functional-tests/src/sentry.ts b/packages/functional-tests/src/sentry.ts index a2437790..86b823b0 100644 --- a/packages/functional-tests/src/sentry.ts +++ b/packages/functional-tests/src/sentry.ts @@ -81,7 +81,7 @@ export const generateSentryTests = (args: TestArgs) => { await expect(alice.decrypt(fakeMissingResource)).to.be.rejectedWith(errors.InvalidArgument); - expect(hub.breadcrumbs).to.have.lengthOf(3); + expect(hub.breadcrumbs).to.have.lengthOf(1 + 2); expect(hub.breadcrumbs[0]?.message).to.contain('Tanker key found in cache'); // 1st decrypt key found expect(hub.breadcrumbs[1]?.message).to.contain('Key not found'); // 2nd transparent session key expect(hub.breadcrumbs[2]?.message).to.contain('Key not found'); // 2nd individual resource key @@ -97,9 +97,12 @@ export const generateSentryTests = (args: TestArgs) => { const encrypted = await bob.encrypt('foo', options); await alice.decrypt(encrypted); + await expect(alice.decrypt(fakeMissingResource)).to.be.rejectedWith(errors.InvalidArgument); - expect(hub.breadcrumbs).to.have.lengthOf(1); + expect(hub.breadcrumbs).to.have.lengthOf(1 + 2); expect(hub.breadcrumbs[0]?.message).to.contain('Tanker key not found in cache, but fetched from server'); + expect(hub.breadcrumbs[1]?.message).to.contain('Key not found'); // 2nd transparent session key + expect(hub.breadcrumbs[2]?.message).to.contain('Key not found'); // 2nd individual resource key }); }); };