Skip to content

Commit

Permalink
feat(atlas-service): migrate from keytar COMPASS-7491 (#5174)
Browse files Browse the repository at this point in the history
  • Loading branch information
mabaasit authored Dec 5, 2023
1 parent 7781e4a commit 027f8a3
Show file tree
Hide file tree
Showing 10 changed files with 111 additions and 142 deletions.
4 changes: 2 additions & 2 deletions package-lock.json

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

2 changes: 1 addition & 1 deletion packages/atlas-service/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
"typescript": "^5.0.4"
},
"dependencies": {
"@mongodb-js/compass-user-data": "^0.1.10",
"@mongodb-js/compass-components": "^1.20.0",
"@mongodb-js/compass-logging": "^1.2.7",
"@mongodb-js/compass-utils": "^0.5.6",
Expand All @@ -81,7 +82,6 @@
"compass-preferences-model": "^2.16.0",
"electron": "^25.9.7",
"hadron-ipc": "^3.2.5",
"keytar": "^7.9.0",
"node-fetch": "^2.6.7",
"react": "^17.0.2",
"react-redux": "^8.0.5",
Expand Down
11 changes: 4 additions & 7 deletions packages/atlas-service/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ import nodeFetch from 'node-fetch';
import type { SimplifiedSchema } from 'mongodb-schema';
import type { Document } from 'mongodb';
import type {
AtlasUserConfig,
AIAggregation,
AIFeatureEnablement,
AIQuery,
IntrospectInfo,
AtlasUserInfo,
} from './util';
import type { AtlasUserConfig } from './user-config-store';
import {
validateAIQueryResponse,
validateAIAggregationResponse,
Expand All @@ -36,7 +36,7 @@ import {
mongoLogId,
} from '@mongodb-js/compass-logging';
import preferences from 'compass-preferences-model';
import { SecretStore, SECRET_STORE_KEY } from './secret-store';
import { SecretStore } from './secret-store';
import { AtlasUserConfigStore } from './user-config-store';
import { OidcPluginLogger } from './oidc-plugin-logger';
import { getActiveUser, isAIFeatureEnabled } from 'compass-preferences-model';
Expand Down Expand Up @@ -251,7 +251,7 @@ export class AtlasService {
'Atlas service initialized',
{ config: this.config }
);
const serializedState = await this.secretStore.getItem(SECRET_STORE_KEY);
const serializedState = await this.secretStore.getState();
this.setupPlugin(serializedState);
await this.setupAIAccess();
// Whether or not we got the state, try requesting user info. If there was
Expand Down Expand Up @@ -791,10 +791,7 @@ export class AtlasService {
return;
}
try {
await this.secretStore.setItem(
SECRET_STORE_KEY,
await this.plugin.serialize()
);
await this.secretStore.setState(await this.plugin.serialize());
} catch (err) {
log.warn(
mongoLogId(1_001_000_221),
Expand Down
2 changes: 1 addition & 1 deletion packages/atlas-service/src/oidc-plugin-logger.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { EventEmitter } from 'events';
import type { MongoDBOIDCPluginOptions } from '@mongodb-js/oidc-plugin';
import type { AtlasUserConfig } from './util';
import type { AtlasUserConfig } from './user-config-store';

type MongoDBOIDCPluginLogger = Required<MongoDBOIDCPluginOptions>['logger'];

Expand Down
5 changes: 3 additions & 2 deletions packages/atlas-service/src/renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import {
userConfigChanged,
} from './store/atlas-signin-reducer';
import { getStore } from './store/atlas-signin-store';
import type { AtlasUserConfig, AtlasUserInfo } from './util';
import type { AtlasUserInfo } from './util';
import type { AtlasUserConfig } from './user-config-store';

let atlasServiceInstanceSingleton: AtlasService;

Expand Down Expand Up @@ -175,9 +176,9 @@ export class AtlasService {
export { AtlasSignIn } from './components/atlas-signin';

export { AtlasServiceError } from './util';
export type { AtlasUserConfig } from './user-config-store';
export type {
AtlasUserInfo,
AtlasUserConfig,
IntrospectInfo,
Token,
AIQuery,
Expand Down
30 changes: 21 additions & 9 deletions packages/atlas-service/src/secret-store.ts
Original file line number Diff line number Diff line change
@@ -1,28 +1,40 @@
import keytar from 'keytar';
import { app } from 'electron';
import { UserData, z } from '@mongodb-js/compass-user-data';
import { safeStorage } from 'electron';

export const SECRET_STORE_KEY = 'AtlasLoginOIDCPluginState';
const AtlasPluginStateSchema = z.string().optional();

export class SecretStore {
async getItem(key: string): Promise<string | undefined> {
private readonly userData: UserData<typeof AtlasPluginStateSchema>;
private readonly fileName = 'AtlasPluginState';
constructor(basePath?: string) {
this.userData = new UserData(AtlasPluginStateSchema, {
subdir: 'AtlasState',
basePath,
});
}

async getState(): Promise<string | undefined> {
try {
if (process.env.COMPASS_E2E_DISABLE_KEYCHAIN_USAGE === 'true') {
throw new Error('Unsupported environment');
}
const appName = app.getName();
return (await keytar.getPassword(appName, key)) ?? undefined;
const data = await this.userData.readOne(this.fileName);
if (!data) {
return undefined;
}
return safeStorage.decryptString(Buffer.from(data, 'base64'));
} catch {
return undefined;
}
}

async setItem(key: string, value: string): Promise<void> {
async setState(value: string): Promise<void> {
try {
if (process.env.COMPASS_E2E_DISABLE_KEYCHAIN_USAGE === 'true') {
throw new Error('Unsupported environment');
}
const appName = app.getName();
return await keytar.setPassword(appName, key, value);
const data = safeStorage.encryptString(value).toString('base64');
await this.userData.write(this.fileName, data);
} catch {
return;
}
Expand Down
3 changes: 2 additions & 1 deletion packages/atlas-service/src/store/atlas-signin-reducer.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import type { AnyAction, Reducer } from 'redux';
import type { ThunkAction } from 'redux-thunk';
import { openToast } from '@mongodb-js/compass-components';
import type { AtlasUserConfig, AtlasUserInfo } from '../util';
import type { AtlasUserInfo } from '../util';
import type { AtlasUserConfig } from '../user-config-store';
import type { AtlasService } from '../renderer';
import { throwIfAborted } from '@mongodb-js/compass-utils';
import { showOptInConfirmation } from '../components/ai-opt-in-confirmation';
Expand Down
132 changes: 58 additions & 74 deletions packages/atlas-service/src/user-config-store.spec.ts
Original file line number Diff line number Diff line change
@@ -1,59 +1,69 @@
import Sinon from 'sinon';
import os from 'os';
import fs from 'fs/promises';
import path from 'path';
import { AtlasUserConfigStore } from './user-config-store';
import { expect } from 'chai';

describe('AtlasUserConfigStore', function () {
const sandbox = Sinon.createSandbox();
let tmpDir: string;
let userConfigStore: AtlasUserConfigStore;

beforeEach(async function () {
tmpDir = await fs.mkdtemp(
path.join(os.tmpdir(), 'atlas-user-config-tests')
);
await fs.mkdir(path.join(tmpDir, 'AtlasUserConfig'), { recursive: true });
userConfigStore = new AtlasUserConfigStore(tmpDir);
});

beforeEach(function () {
sandbox.reset();
new AtlasUserConfigStore().clearCache();
afterEach(async function () {
await fs.rm(tmpDir, { recursive: true });
userConfigStore.clearCache();
});

const createAtlasUserConfig = async (userId: string, data: any) => {
const filePath = path.join(tmpDir, 'AtlasUserConfig', `${userId}.json`);
await fs.writeFile(filePath, JSON.stringify(data));
};

describe('getUserConfig', function () {
it('should throw if config shape is unexpected', async function () {
const store = new AtlasUserConfigStore({
readFile: sandbox
.stub()
.onFirstCall()
.resolves('"meow"')
.onSecondCall()
.resolves('{"enabledAIFeature": "yes, totally"}'),
} as any);

store['getConfigDir'] = () => '/tmp';

try {
await store.getUserConfig('1234');
await createAtlasUserConfig('1234', 'meow');
await userConfigStore.getUserConfig('1234');
expect.fail('Expected getUserConfig to throw');
} catch (err) {
expect(err)
.to.have.property('message')
.match(/Expected AtlasUserConfig to be an object/);
expect((err as Error).message).to.match(
/Expected object, received string/
);
}

try {
await store.getUserConfig('1234');
await createAtlasUserConfig('1234', {
enabledAIFeature: 'yes, totally',
});
await userConfigStore.getUserConfig('1234');
expect.fail('Expected getUserConfig to throw');
} catch (err) {
expect(err)
.to.have.property('message')
.match(/Unexpected values in AtlasUserConfig/);
expect((err as Error).message).to.match(
/Expected boolean, received string/
);
}
});

it('should throw if reading file failed for any reason other than file not existing', async function () {
const err = new Error('EACCESS');
(err as any).code = 'EACCESS';

const store = new AtlasUserConfigStore({
readFile: sandbox.stub().rejects(err),
} as any);

store['getConfigDir'] = () => '/tmp';
const readOneStub = Sinon.stub(
(userConfigStore as any).userData,
'readOne'
);
readOneStub.throws(err);

try {
await store.getUserConfig('1234');
await userConfigStore.getUserConfig('1234');
expect.fail('Expected getUserConfig to throw');
} catch (err) {
expect(err)
Expand All @@ -63,83 +73,57 @@ describe('AtlasUserConfigStore', function () {
});

it("should return default user config if config file doesn't exist", async function () {
const err = new Error('ENOENT');
(err as any).code = 'ENOENT';

const store = new AtlasUserConfigStore({
readFile: sandbox.stub().rejects(err),
} as any);

store['getConfigDir'] = () => '/tmp';

expect(await store.getUserConfig('1234')).to.deep.eq({
expect(await userConfigStore.getUserConfig('1234')).to.deep.eq({
enabledAIFeature: false,
});
});

it('should return default user config if config file is missing required keys', async function () {
const store = new AtlasUserConfigStore({
readFile: sandbox.stub().resolves('{}'),
} as any);

store['getConfigDir'] = () => '/tmp';

expect(await store.getUserConfig('1234')).to.deep.eq({
await createAtlasUserConfig('1234', {});
expect(await userConfigStore.getUserConfig('1234')).to.deep.eq({
enabledAIFeature: false,
});
});

it('should return config if it exists', async function () {
const store = new AtlasUserConfigStore({
readFile: sandbox.stub().resolves('{"enabledAIFeature": true}'),
} as any);

store['getConfigDir'] = () => '/tmp';

expect(await store.getUserConfig('1234')).to.deep.eq({
await createAtlasUserConfig('1234', { enabledAIFeature: true });
expect(await userConfigStore.getUserConfig('1234')).to.deep.eq({
enabledAIFeature: true,
});
});

it('should return config from in-memory cache after first read', async function () {
const mockFs = {
readFile: sandbox.stub().resolves('{"enabledAIFeature": true}'),
};
const store = new AtlasUserConfigStore(mockFs as any);

store['getConfigDir'] = () => '/tmp';
const readSpy = Sinon.spy((userConfigStore as any).userData, 'readOne');

expect(await store.getUserConfig('1234')).to.deep.eq({
await createAtlasUserConfig('1234', { enabledAIFeature: true });
expect(await userConfigStore.getUserConfig('1234')).to.deep.eq({
enabledAIFeature: true,
});

expect(await store.getUserConfig('1234')).to.deep.eq({
expect(await userConfigStore.getUserConfig('1234')).to.deep.eq({
enabledAIFeature: true,
});

expect(mockFs.readFile).to.have.been.calledOnce;
expect(readSpy).to.have.been.calledOnce;
});
});

describe('updateUserConfig', function () {
it('should update user config value in in-memory cache', async function () {
const mockFs = {
readFile: sandbox.stub().resolves('{"enabledAIFeature": true}'),
writeFile: sandbox.stub().resolves(),
mkdir: sandbox.stub().resolves(),
};

const store = new AtlasUserConfigStore(mockFs);
const readSpy = Sinon.spy((userConfigStore as any).userData, 'readOne');
const updateSpy = Sinon.spy((userConfigStore as any).userData, 'write');

store['getConfigDir'] = () => '/tmp';
await createAtlasUserConfig('1234', { enabledAIFeature: false });

await store.updateUserConfig('1234', { enabledAIFeature: true });
expect(await store.getUserConfig('1234')).to.deep.eq({
await userConfigStore.updateUserConfig('1234', {
enabledAIFeature: true,
});
expect(await userConfigStore.getUserConfig('1234')).to.deep.eq({
enabledAIFeature: true,
});

expect(mockFs.readFile).to.have.been.calledOnce;
expect(mockFs.writeFile).to.have.been.calledOnce;
expect(readSpy).to.have.been.calledOnce;
expect(updateSpy).to.have.been.calledOnce;
});
});
});
Loading

0 comments on commit 027f8a3

Please sign in to comment.