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

Feature/remote key manager #202

Open
wants to merge 17 commits into
base: feature/p2pmessaging
Choose a base branch
from
Open
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
fixed PR comments
indramalav committed Jun 4, 2020
commit 65fea2060c80acf844575f9098b210e80ba672a2
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -6,7 +6,7 @@
"main": "lib/index.js",
"types": "lib/index.d.ts",
"scripts": {
"test": "TS_NODE_PROJECT='./src/test/tsconfig.commonjs.json' TS_NODE_TRANSPILE_ONLY=true ./node_modules/.bin/mocha --exit --require ts-node/register --require mock-local-storage --require jsdom-global/register --allow-uncaught --colors --reporter mocha-reporter --timeout 50000 src/test/*.ts src/test/crux-messenger/*.ts",
"test": "TS_NODE_PROJECT='./src/test/tsconfig.commonjs.json' TS_NODE_TRANSPILE_ONLY=true ./node_modules/.bin/mocha --exit --require ts-node/register --require mock-local-storage --require jsdom-global/register --allow-uncaught --colors --reporter mocha-reporter --timeout 5000 src/test/*.ts src/test/crux-messenger/*.ts",
"start-bridge-server": "node dist/crux-gateway-bridge-without-auth.js &",
"stop-bridge-server": "pkill -- signal SIGINT crux-gateway-bridge-server-without-auth",
"integration-test": "TS_NODE_PROJECT='./src/test/tsconfig.commonjs.json' TS_NODE_TRANSPILE_ONLY=true ./node_modules/.bin/mocha --exit --require ts-node/register --require mock-local-storage --require jsdom-global/register --allow-uncaught --colors --reporter mocha-reporter --timeout 80000 src/test/integration-tests/crux-messenger/*.ts",
4 changes: 2 additions & 2 deletions src/application/clients/crux-service-client.ts
Original file line number Diff line number Diff line change
@@ -7,9 +7,9 @@ export class CruxServiceClient {
private selfIdClaim: any;
private cruxProtocolMessenger: any;

constructor(selfIdClaim: any, secureCruxNetwork: any, protocolSchema?: any) {
constructor(selfIdClaim: any, secureCruxNetwork: any, protocolSchema: any) {
this.selfIdClaim = selfIdClaim;
this.cruxProtocolMessenger = new CruxProtocolMessenger(secureCruxNetwork, protocolSchema ? protocolSchema : keyManagementProtocol);
this.cruxProtocolMessenger = new CruxProtocolMessenger(secureCruxNetwork, protocolSchema);
}

public async getWalletClientForUser(remoteUserId: CruxId) {
12 changes: 2 additions & 10 deletions src/application/clients/crux-wallet-client.ts
Original file line number Diff line number Diff line change
@@ -206,14 +206,10 @@ export class CruxWalletClient {

@throwCruxClientError
public getAddressMap = async (): Promise<IAddressMapping> => {
console.log("CruxWalletClient::getAddressMap::keyManager: ", this.getKeyManager());
await this.initPromise;
console.log("CruxWalletClient::getAddressMap::publickKey: ", await this.getKeyManager().getPubKey());
const cruxUser = await this.cruxUserRepository.getWithKey(this.getKeyManager());
console.log("CruxWalletClient::getAddressMap::cruxUser: ", cruxUser);
if (cruxUser) {
const assetIdAddressMap = cruxUser.getAddressMap();
console.log("CruxWalletClient::getAddressMap::assetIdAddressMap");
return this.cruxAssetTranslator.assetIdAddressMapToSymbolAddressMap(assetIdAddressMap);
}
return {};
@@ -259,7 +255,6 @@ export class CruxWalletClient {

@throwCruxClientError
public putAddressMap = async (newAddressMap: IAddressMapping): Promise<{success: IPutAddressMapSuccess, failures: IPutAddressMapFailures}> => {
console.log("+++==", newAddressMap);
await this.initPromise;
const cruxUser = await this.getCruxUserByKey();
if (!cruxUser) {
@@ -448,7 +443,7 @@ export class CruxWalletClient {
const selfIdClaim = await this.getSelfClaim();
if (selfIdClaim) {
try {
await this.setupCruxMessenger(selfIdClaim, options);
await this.setupCruxMessenger(selfIdClaim);
} catch (err) {
console.log(err);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of this? Remove if it was for your personal debugging only.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually it was for catching errors from setupCruxMessenger but now we can remove it(options are being used till here only)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'catching error' is not a valid purpose. 'catching error to _________' is a valid purpose

  1. Catching error to silence/suppress it
  2. Catching error to transform it and throw it again as another error
  3. Catching error to retry
  4. etc

Here you've suppressed it and logged it. Any particular cases/errors you're trying to catch here? If this function is too failure prone then it may block other functionalities of SDK. So then it may be a good idea to suppress error here like youve done
But in that case we should figure out why its failing and fix that.

}
@@ -471,13 +466,10 @@ export class CruxWalletClient {
}
return selfClaim;
}
private setupCruxMessenger = async (selfIdClaim: ICruxIdClaim | undefined, options: ICruxWalletClientOptions) => {
private setupCruxMessenger = async (selfIdClaim: ICruxIdClaim | undefined) => {
if (!selfIdClaim) {
throw Error("Self ID Claim is required to setup messenger");
}
if (options && options.disableCruxMessenger) {
throw Error("Secure Crux Network hasn't been initiated for Wallet");
}
const pubsubClientFactory = getPubsubClientFactory();
this.secureCruxNetwork = new SecureCruxNetwork(this.cruxUserRepository, pubsubClientFactory, selfIdClaim);
this.paymentProtocolMessenger = new CruxProtocolMessenger(this.secureCruxNetwork, cruxPaymentProtocol);
3 changes: 0 additions & 3 deletions src/core/domain-services/crux-messenger.ts
Original file line number Diff line number Diff line change
@@ -252,9 +252,6 @@ export class SecureContext {
data,
};
const serializedSecurePacket = JSON.stringify(securePacket);
if (typeof recipientId === "string") {
recipientId = CruxId.fromString(recipientId);
}
const recipientCruxUser: CruxUser | undefined = await this.cruxUserRepo.getByCruxId(recipientId);
if (!recipientCruxUser) {
throw Error("No Such CRUX User Found");
80 changes: 18 additions & 62 deletions src/core/domain-services/remote-key-service.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {makeUUID4} from "blockstack/lib";
import { createNanoEvents, DefaultEvents, Emitter } from "nanoevents";
import { CruxId } from "src/packages";
import { CruxId } from "../../packages";
import { IKeyManager } from "../interfaces";
import { CruxProtocolMessenger, SecureCruxNetwork } from "./crux-messenger";

@@ -71,7 +71,7 @@ export class RemoteKeyHost {
console.log("Inside RemoteKeyHost::in::Msg, senderId: ", msg, senderId);
const data = await this.handleMessage(msg);
console.log("Inside RemoteKeyHost::initialize::Data(handleMessage): ", data);
this.sendInvocationResult(data, senderId!);
await this.sendInvocationResult(data, CruxId.fromString(senderId!));
});
}
private async sendInvocationResult(result: any, receiverId: CruxId) {
@@ -80,40 +80,19 @@ export class RemoteKeyHost {
}
const resultData = this.generateInvocationResponse(result);
console.log("RemoteKeyHost::Inside sendInvocationResult::resultData: ", resultData);
// @ts-ignore
await this.cruxProtocolMessenger.send({
content: resultData,
type: "KEY_MANAGER_RESPONSE",
}, receiverId);
}

private async handleMessage(message: any) {
if (!VALID_METHODS.includes(message.method)) {
throw new Error("Invalid key manager method");
}
if (!this.keyManager) {
throw new Error("Key Manager not available");
}
let data;
if (message.method === "signWebToken") {
console.log("HandleMessage entrance:signWebToken");
data = await this.keyManager.signWebToken(message.args[0]);
console.log("HandleMessage exit:signWebToken", data);
} else if (message.method === "getPubKey") {
console.log("HandleMessage entrance:getPubKey");
data = await this.keyManager.getPubKey();
console.log("HandleMessage exit:getPubKey", data);
} else if (message.method === "deriveSharedSecret") {
console.log("HandleMessage entrance:deriveSharedSecret");
// @ts-ignore
data = await this.keyManager.deriveSharedSecret(message.args[0]);
console.log("HandleMessage exit:deriveSharedSecret", data);
} else if (message.method === "decryptMessage") {
console.log("HandleMessage entrance:decryptMessage");
// @ts-ignore
data = await this.keyManager.decryptMessage(message.args[0]);
console.log("HandleMessage exit:decryptMessage", data);
}
// @ts-ignore
data = await this.keyManager[message.method](message.args[0]);
return {
data,
invocationId: message.invocationId,
@@ -141,52 +120,29 @@ export class RemoteKeyManager implements IKeyManager {
await this.remoteKeyClient.initialize();
}
// @ts-ignore
public signWebToken = async (token: any) => {
return new Promise(async (resolve, reject) => {
const invocationId = await this.remoteKeyClient.invoke("signWebToken", [token]);
console.log("RemoteKeyManager::signWebToken::invokationId: ", invocationId);
this.remoteKeyClient.listenToInvocation(invocationId, (msg, senderId) => {
console.log("RemoteKeyManager::signWebToken::msg: ", msg);
resolve(msg.result.data);
}, (err) => {
reject(err);
});
});
public signWebToken = async (args: any) => {
return this.makeRemoteMessageCall("signWebToken", [args]);
}
// @ts-ignore
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove ts ignore and fix errors. Let me know if you're getting confused with 'args' and how to make it generic for arguments.

public getPubKey = async () => {
return new Promise(async (resolve, reject) => {
const invocationId = await this.remoteKeyClient.invoke("getPubKey", []);
console.log("RemoteKeyManager::getPubKey::invokationId: ", invocationId);
this.remoteKeyClient.listenToInvocation(invocationId, (msg, senderId) => {
console.log("RemoteKeyManager::getPubKey::msg: ", msg);
resolve(msg.result.data);
}, (err) => {
reject(err);
});
});
return this.makeRemoteMessageCall("getPubKey");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can do an optimization here -
We know the Crux ID. And we're doing secure communication on the sole basis of the fact that we know the ID's Public Key. So we don't need to do makeRemoteMessageCall to getPubKey. You can get a CruxUser from CruxId and get Public Key from there. How to get CruxUser? We need UserRepository. SecureCruxNetwork already has a userRepository, Just that it's a private variable at the moment. You can make it public, and access it via CruxProtocolMessenger->SecureCruxNetwork->userRepository

}
// @ts-ignore
public deriveSharedSecret = async (publicKey: string) => {
const invocationId = await this.remoteKeyClient.invoke("deriveSharedSecret", [publicKey]);
console.log("RemoteKeyManager::deriveSharedSecret::invokationId: ", invocationId);
return new Promise(async (resolve, reject) => {
this.remoteKeyClient.listenToInvocation(invocationId, (msg, senderId) => {
console.log("RemoteKeyManager::deriveSharedSecret::msg: ", msg);
resolve(msg.result.data);
}, (err) => {
reject(err);
});
});
public deriveSharedSecret = async (args: string) => {
return this.makeRemoteMessageCall("deriveSharedSecret", [args]);
}
// @ts-ignore
public decryptMessage = async (encryptedMessage: string) => {
console.log("RemoteKeyManager::decryptMessage::entry: encryptedMessage: ", encryptedMessage);
const invocationId = await this.remoteKeyClient.invoke("decryptMessage", [encryptedMessage]);
console.log("RemoteKeyManager::decryptMessage::invokationId: ", invocationId);
public decryptMessage = async (args: string) => {
Copy link
Member

@quixote911 quixote911 Jun 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong naming of parameter in all methods - look at other KeyManager implementation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry will take care of it

return this.makeRemoteMessageCall("decryptMessage", [args]);
}

private makeRemoteMessageCall = async (method: string, args: any = []) => {
console.log("makeRemoteMessageCall::", method, args);
const invocationId = await this.remoteKeyClient.invoke(method, args);
console.log("RemoteKeyManager::makeRemoteMessageCall::invokationId: ", invocationId);
return new Promise(async (resolve, reject) => {
this.remoteKeyClient.listenToInvocation(invocationId, (msg, senderId) => {
console.log("RemoteKeyManager::decryptMessage::msg: ", msg);
console.log("RemoteKeyManager::deriveSharedSecret::msg: ", msg);
resolve(msg.result.data);
}, (err) => {
reject(err);
Original file line number Diff line number Diff line change
@@ -98,7 +98,7 @@ export class BlockstackCruxUserRepository implements ICruxUserRepository {
}
return new CruxUser(cruxID.components.subdomain, await this.getUserCruxDomain(cruxID) as CruxDomain, addressMap, cruxUserInformation, cruxUserData, cruxpayPubKey);
}
public getWithKey = async (keyManager: IKeyManager): Promise<CruxUser|undefined> => {
public getWithKey = async (keyManager: IKeyManager): Promise<CruxUser|undefined> => {
const cruxID = await this.blockstackService.getCruxIdWithKeyManager(keyManager, this.getCruxDomain().id);
if (!cruxID) {
return;
9 changes: 5 additions & 4 deletions src/infrastructure/implementations/crux-messenger.ts
Original file line number Diff line number Diff line change
@@ -186,19 +186,20 @@ export class CruxNetPubSubClientFactory implements IPubSubClientFactory {
};
}
public getClient = (from: CruxId, keyManager: IKeyManager, to?: CruxId): IPubSubClient => {
if (this.bufferPahoClient[from.toString()]) { return this.bufferPahoClient[from.toString()]; }
const fromCruxIdString = from.toString();
if (this.bufferPahoClient[fromCruxIdString]) { return this.bufferPahoClient[fromCruxIdString]; }
const overrideOpts = this.getDomainLevelClientOptions(to ? to : from);
this.bufferPahoClient[from.toString()] = new PahoClient({
this.bufferPahoClient[fromCruxIdString] = new PahoClient({
clientOptions: {
clientId: from.toString(),
clientId: fromCruxIdString,
host: overrideOpts ? overrideOpts.host : this.options.defaultLinkServer.host,
path: overrideOpts ? overrideOpts.path : this.options.defaultLinkServer.path,
port: overrideOpts ? overrideOpts.port : this.options.defaultLinkServer.port,
// tslint:disable-next-line: object-literal-sort-keys
},
subscribeOptions: this.defaultSubscribeOptions,
});
return this.bufferPahoClient[from.toString()];
return this.bufferPahoClient[fromCruxIdString];
}
private getDomainLevelClientOptions = (cruxId: CruxId): {host: string, port: number, path: string} | undefined => {
// TODO Implement
Original file line number Diff line number Diff line change
@@ -52,7 +52,6 @@ describe('Test CruxServiceClient - Prod', function() {
const cruxWalletClient = new CruxWalletClient({
privateKey: user1PvtKey,
walletClientName: "cruxdev",
isHost: true,
cacheStorage: new InMemStorage(),
});
const cruxServiceClient = new CruxServiceClient({
@@ -69,7 +68,6 @@ describe('Test CruxServiceClient - Prod', function() {
const cruxWalletClient = new CruxWalletClient({
privateKey: user1PvtKey,
walletClientName: "cruxdev",
isHost: true,
cacheStorage: new InMemStorage(),
});
const cruxServiceClient = new CruxServiceClient({