Skip to content

Commit

Permalink
Improve keytar handling
Browse files Browse the repository at this point in the history
  • Loading branch information
msujew committed Jun 18, 2023
1 parent 01716cc commit 13b48d5
Show file tree
Hide file tree
Showing 11 changed files with 131 additions and 49 deletions.
1 change: 0 additions & 1 deletion packages/core/src/electron-main/electron-api-main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ export class TheiaMainApi implements ElectronMainApplicationContribution {

// cookies
ipcMain.on(CHANNEL_SET_COOKIE, (event, endpoint: string, name: string, value: string) => {
console.log(`Setting cookie on ${endpoint}: ${name}=${value}`);
session.defaultSession.cookies.set({
url: endpoint,
name,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,19 @@
// *****************************************************************************

import * as http from 'http';
import { injectable } from 'inversify';
import { inject, injectable } from 'inversify';
import { BackendRemoteService } from '../../node/remote/backend-remote-service';
import { WsRequestValidatorContribution } from '../../node/ws-request-validators';

@injectable()
export class ElectronWsOriginValidator implements WsRequestValidatorContribution {

enabled = true;
@inject(BackendRemoteService)
protected readonly backendRemoteService: BackendRemoteService;

allowWsUpgrade(request: http.IncomingMessage): boolean {
if (!this.enabled) {
// If we are running as a remote server, requests will come from an http endpoint
if (this.backendRemoteService.isRemoteServer()) {
return true;
}
// On Electron the main page is served from the `file` protocol.
Expand Down
78 changes: 75 additions & 3 deletions packages/core/src/node/keytar-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,23 @@
// code copied and modified from https://github.com/microsoft/vscode/blob/1.55.2/src/vs/platform/native/electron-main/nativeHostMainService.ts#L679-L771

import { KeytarService } from '../common/keytar-protocol';
import { injectable } from 'inversify';
import { inject, injectable } from 'inversify';
import { isWindows } from '../common';
import * as keytar from 'keytar';
import { BackendRemoteService } from './remote/backend-remote-service';

@injectable()
export class KeytarServiceImpl implements KeytarService {

@inject(BackendRemoteService)
protected readonly backendRemoteService: BackendRemoteService;

private static readonly MAX_PASSWORD_LENGTH = 2500;
private static readonly PASSWORD_CHUNK_SIZE = KeytarServiceImpl.MAX_PASSWORD_LENGTH - 100;

protected cache?: typeof import('keytar');

async setPassword(service: string, account: string, password: string): Promise<void> {
const keytar = await this.getKeytar();
if (isWindows && password.length > KeytarServiceImpl.MAX_PASSWORD_LENGTH) {
let index = 0;
let chunk = 0;
Expand All @@ -54,11 +61,13 @@ export class KeytarServiceImpl implements KeytarService {
}
}

deletePassword(service: string, account: string): Promise<boolean> {
async deletePassword(service: string, account: string): Promise<boolean> {
const keytar = await this.getKeytar();
return keytar.deletePassword(service, account);
}

async getPassword(service: string, account: string): Promise<string | undefined> {
const keytar = await this.getKeytar();
const password = await keytar.getPassword(service, account);
if (password) {
try {
Expand All @@ -81,15 +90,78 @@ export class KeytarServiceImpl implements KeytarService {
}
}
}

async findPassword(service: string): Promise<string | undefined> {
const keytar = await this.getKeytar();
const password = await keytar.findPassword(service);
if (password) {
return password;
}
}

async findCredentials(service: string): Promise<Array<{ account: string, password: string }>> {
const keytar = await this.getKeytar();
return keytar.findCredentials(service);
}

protected async getKeytar(): Promise<typeof import('keytar')> {
if (this.cache) {
return this.cache;
}
try {
return (this.cache = await import('keytar'));
} catch (err) {
if (this.backendRemoteService.isRemoteServer()) {
// When running as a remote server, the necessary prerequisites might not be installed on the system
// Just use an in-memory cache for credentials
return (this.cache = new InMemoryCredentialsProvider());
} else {
throw err;
}
}
}
}

export class InMemoryCredentialsProvider {
private secretVault: Record<string, Record<string, string> | undefined> = {};

async getPassword(service: string, account: string): Promise<string | null> {
// eslint-disable-next-line no-null/no-null
return this.secretVault[service]?.[account] ?? null;
}

async setPassword(service: string, account: string, password: string): Promise<void> {
this.secretVault[service] = this.secretVault[service] ?? {};
this.secretVault[service]![account] = password;
}

async deletePassword(service: string, account: string): Promise<boolean> {
if (!this.secretVault[service]?.[account]) {
return false;
}
delete this.secretVault[service]![account];
if (Object.keys(this.secretVault[service]!).length === 0) {
delete this.secretVault[service];
}
return true;
}

async findPassword(service: string): Promise<string | null> {
// eslint-disable-next-line no-null/no-null
return JSON.stringify(this.secretVault[service]) ?? null;
}

async findCredentials(service: string): Promise<Array<{ account: string; password: string }>> {
const credentials: { account: string; password: string }[] = [];
for (const account of Object.keys(this.secretVault[service] || {})) {
credentials.push({ account, password: this.secretVault[service]![account] });
}
return credentials;
}

async clear(): Promise<void> {
this.secretVault = {};
}
}

interface ChunkedPassword {
Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/node/remote/backend-remote-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@ import { CoreCopyContribution } from './core-copy-contribution';
import { RemoteCopyContribution, RemoteCopyRegistry } from './remote-copy-contribution';
import { RemoteNativeDependencyContribution } from './remote-native-dependency-contribution';
import { AppNativeDependencyContribution } from './app-native-dependency-contribution';
import { BackendRemoteService } from './backend-remote-service';

export default new ContainerModule(bind => {
bind(BackendRemoteService).toSelf().inSingletonScope();
bindContributionProvider(bind, RemoteCopyContribution);
bind(RemoteCopyRegistry).toSelf().inSingletonScope();
bind(CoreCopyContribution).toSelf().inSingletonScope();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,12 @@
// SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0
// *****************************************************************************

import { ContainerModule } from '@theia/core/shared/inversify';
import { CliContribution } from '@theia/core/lib/node';
import { RemoteCliContribution } from './remote-cli-contribution';
import { injectable } from 'inversify';

export default new ContainerModule(bind => {
bind(RemoteCliContribution).toSelf().inSingletonScope();
bind(CliContribution).toService(RemoteCliContribution);
});
@injectable()
export class BackendRemoteService {

isRemoteServer(): boolean {
return false;
}
}
3 changes: 0 additions & 3 deletions packages/remote/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@
{
"frontend": "lib/browser/remote-frontend-module",
"backend": "lib/node/remote-backend-module"
},
{
"backendElectron": "lib/electron-node/remote-electron-backend-module"
}
],
"keywords": [
Expand Down
2 changes: 1 addition & 1 deletion packages/remote/src/browser/remote-ssh-contribution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export class RemoteSSHContribution extends AbstractRemoteRegistryContribution {
const remoteId = await this.sendSSHConnect(host!, user!);
this.openRemote(remoteId, newWindow);
} catch (err) {
this.messageService.error(`${nls.localize('theia/remote/sshFailure', 'Could not open SSH connection to remote.')} ${String(err)}`);
this.messageService.error(`${nls.localize('theia/remote/sshFailure', 'Could not open SSH connection to remote.')} ${err.message ?? String(err)}`);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,16 @@
// *****************************************************************************

import { CliContribution } from '@theia/core/lib/node';
import { injectable, inject } from '@theia/core/shared/inversify';
import { injectable } from '@theia/core/shared/inversify';
import { Arguments, Argv } from '@theia/core/shared/yargs';
import { ElectronWsOriginValidator } from '@theia/core/lib/electron-node/hosting/electron-ws-origin-validator';
import { BackendRemoteService } from '@theia/core/lib/node/remote/backend-remote-service';

export const REMOTE_START = 'remote';

@injectable()
export class RemoteCliContribution implements CliContribution {
export class BackendRemoteServiceImpl extends BackendRemoteService implements CliContribution {

@inject(ElectronWsOriginValidator)
protected readonly wsOriginValidator: ElectronWsOriginValidator;
protected isRemote: boolean = false;

configure(conf: Argv): void {
conf.option(REMOTE_START, {
Expand All @@ -36,10 +35,11 @@ export class RemoteCliContribution implements CliContribution {
}

setArguments(args: Arguments): void {
const remote = args[REMOTE_START];
if (remote) {
this.wsOriginValidator.enabled = false;
}
this.isRemote = Boolean(args[REMOTE_START]);
}

override isRemoteServer(): boolean {
return this.isRemote;
}

}
10 changes: 8 additions & 2 deletions packages/remote/src/node/remote-backend-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
// *****************************************************************************

import { ContainerModule } from '@theia/core/shared/inversify';
import { BackendApplicationContribution, MessagingService } from '@theia/core/lib/node';
import { BackendApplicationContribution, CliContribution, MessagingService } from '@theia/core/lib/node';
import { RemoteExpressProxyContribution } from './remote-express-proxy-contribution';
import { RemoteConnectionService } from './remote-connection-service';
import { RemoteProxyServerProvider } from './remote-proxy-server-provider';
Expand All @@ -29,14 +29,16 @@ import { SSHIdentityFileCollector } from './ssh/ssh-identity-file-collector';
import { RemoteCopyService } from './remote-copy-service';
import { RemoteSetupService } from './remote-setup-service';
import { RemoteNativeDependencyService } from './remote-native-dependency-service';
import { BackendRemoteServiceImpl } from './backend-remote-service-impl';
import { BackendRemoteService } from '@theia/core/lib/node/remote/backend-remote-service';

export const remoteConnectionModule = ConnectionContainerModule.create(({ bind, bindBackendService }) => {
bind(RemoteSSHConnectionProviderImpl).toSelf().inSingletonScope();
bind(RemoteSSHConnectionProvider).toService(RemoteSSHConnectionProviderImpl);
bindBackendService(RemoteSSHConnectionProviderPath, RemoteSSHConnectionProvider);
});

export default new ContainerModule(bind => {
export default new ContainerModule((bind, _unbind, _isBound, rebind) => {
bind(RemoteCopyService).toSelf().inSingletonScope();
bind(RemoteSetupService).toSelf().inSingletonScope();
bind(RemoteNativeDependencyService).toSelf().inSingletonScope();
Expand All @@ -51,5 +53,9 @@ export default new ContainerModule(bind => {
bind(MessagingService.RedirectContribution).toService(RemoteRedirectContribution);
bind(ConnectionContainerModule).toConstantValue(remoteConnectionModule);

bind(BackendRemoteServiceImpl).toSelf().inSingletonScope();
rebind(BackendRemoteService).toService(BackendRemoteServiceImpl);
bind(CliContribution).toService(BackendRemoteServiceImpl);

bind(SSHIdentityFileCollector).toSelf().inSingletonScope();
});
10 changes: 5 additions & 5 deletions packages/remote/src/node/remote-setup-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,9 @@ export class RemoteSetupService {
const nodeExecutable = this.joinRemotePath(platform, nodeDir, 'bin', platform === 'windows' ? 'node.exe' : 'node');
const mainJsFile = this.joinRemotePath(platform, remotePath, 'lib', 'backend', 'main.js');
const localAddressRegex = /listening on http:\/\/localhost:(\d+)/;
const result = await connection.execPartial(nodeExecutable,
// Change to the remote application path and start a node process with the copied main.js file
// This way, our current working directory is set as expected
const result = await connection.execPartial(`cd "${remotePath}";${nodeExecutable}`,
stdout => localAddressRegex.test(stdout),
[mainJsFile, '--port=0', '--remote']);

Expand All @@ -97,7 +99,7 @@ export class RemoteSetupService {
const result = await this.retry(() => connection.exec('uname -s'));

if (result.stderr) {
// Only Windows systems can error out here
// Only Windows systems return an error stream here
return 'windows';
} else if (result.stdout) {
if (result.stdout.includes('windows32') || result.stdout.includes('MINGW64')) {
Expand Down Expand Up @@ -182,9 +184,7 @@ export class RemoteSetupService {

protected async dirExistsRemote(connection: RemoteConnection, remotePath: string): Promise<boolean> {
const cdResult = await connection.exec('cd', [remotePath]);
const result = !Boolean(cdResult.stderr);
console.log('Result direxists: ' + remotePath + ' :: ' + result);
return result;
return !Boolean(cdResult.stderr);
}

protected async unzipRemote(connection: RemoteConnection, remoteFile: string, remoteDirectory: string): Promise<void> {
Expand Down
32 changes: 17 additions & 15 deletions packages/remote/src/node/ssh/remote-ssh-connection-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,20 +62,23 @@ export class RemoteSSHConnectionProviderImpl implements RemoteSSHConnectionProvi
});
const report: RemoteStatusReport = message => progress.report({ message });
report('Connecting to remote system...');
const remote = await this.establishSSHConnection(host, user);
await this.remoteSetup.setup(remote, report);
const registration = this.remoteConnectionService.register(remote);
const server = await this.serverProvider.getProxyServer(socket => {
remote.forwardOut(socket);
});
const proxyRouter = this.remoteExpressProxy.setupProxyRouter(remote, server);
remote.onDidDisconnect(() => {
server.close();
proxyRouter.dispose();
registration.dispose();
});
progress.cancel();
return remote.id;
try {
const remote = await this.establishSSHConnection(host, user);
await this.remoteSetup.setup(remote, report);
const registration = this.remoteConnectionService.register(remote);
const server = await this.serverProvider.getProxyServer(socket => {
remote.forwardOut(socket);
});
const proxyRouter = this.remoteExpressProxy.setupProxyRouter(remote, server);
remote.onDidDisconnect(() => {
server.close();
proxyRouter.dispose();
registration.dispose();
});
return remote.id;
} finally {
progress.cancel();
}
}

async establishSSHConnection(host: string, user: string): Promise<RemoteSSHConnection> {
Expand Down Expand Up @@ -328,7 +331,6 @@ export class RemoteSSHConnection implements RemoteConnection {
protected buildCmd(cmd: string, args?: string[]): string {
const escapedArgs = args?.map(arg => `"${arg.replace(/"/g, '\\"')}"`) || [];
const fullCmd = cmd + (escapedArgs.length > 0 ? (' ' + escapedArgs.join(' ')) : '');
console.log(`Build full cmd: '${fullCmd}'`);
return fullCmd;
}

Expand Down

0 comments on commit 13b48d5

Please sign in to comment.