Skip to content

Commit

Permalink
refactor(core-api): keychain plugin get/set generics
Browse files Browse the repository at this point in the history
Fixes #1135

Signed-off-by: Tommesha Wiggins <[email protected]>
Signed-off-by: Peter Somogyvari <[email protected]>
  • Loading branch information
twiggins120 authored and petermetz committed Aug 3, 2021
1 parent b40c837 commit a340765
Show file tree
Hide file tree
Showing 48 changed files with 280 additions and 153 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ export class CarbonAccountingPlugin
enrollmentSecret,
);

this.keychain.set(identityId, identity);
this.keychain.set(identityId, JSON.stringify(identity));
this.log.debug(`Stored Fabric admin identity on keychain as ${identityId}`);

const res: EnrollAdminV1Response = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,11 +172,11 @@ export class SupplyChainAppDummyInfrastructure {

await this.keychain.set(
BookshelfRepositoryJSON.contractName,
BookshelfRepositoryJSON,
BookshelfRepositoryJSON.contractName,
);
await this.keychain.set(
BambooHarvestRepositoryJSON.contractName,
BambooHarvestRepositoryJSON,
BambooHarvestRepositoryJSON.contractName,
);
{
this._quorumAccount = await this.quorum.createEthTestAccount(2000000);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export interface IPluginKeychain extends ICactusPlugin {
getKeychainId(): string;

has(key: string): Promise<boolean>;
get<T>(key: string): Promise<T>;
set<T>(key: string, value: T): Promise<void>;
get(key: string): Promise<string>;
set(key: string, value: string): Promise<void>;
delete(key: string): Promise<void>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export class PluginKeychainAwsSm
private endpoints: IWebServiceEndpoint[] | undefined;
private awsCredentialType: AwsCredentialType;

public get className() {
public get className(): string {
return PluginKeychainAwsSm.CLASS_NAME;
}

Expand Down Expand Up @@ -191,18 +191,18 @@ export class PluginKeychainAwsSm
}

public getEncryptionAlgorithm(): string {
return null as any;
return (null as unknown) as string;
}

async get<T>(key: string): Promise<T> {
async get(key: string): Promise<string> {
const fnTag = `${this.className}#get(key: string)`;
const awsClient = this.getAwsClient();
try {
const res = (await awsClient
const res = await awsClient
.getSecretValue({
SecretId: key,
})
.promise()) as any;
.promise();
if (res.SecretString) {
return res.SecretString;
} else {
Expand All @@ -215,7 +215,8 @@ export class PluginKeychainAwsSm
SECRETMANAGER_STATUS_KEY_NOT_FOUND,
);
if (errorStatus) {
return (null as unknown) as T;
// FIXME: Throw if the value was not present instead of returning null
return (null as unknown) as string;
} else {
this.log.error(`Error retriving secret value for the key "${key}"`);
throw ex;
Expand All @@ -227,11 +228,11 @@ export class PluginKeychainAwsSm
const fnTag = `${this.className}#has(key: string)`;
const awsClient = this.getAwsClient();
try {
(await awsClient
await awsClient
.describeSecret({
SecretId: key,
})
.promise()) as any;
.promise();
return true;
} catch (ex) {
const errorStatus = (ex as Error).message.includes(
Expand All @@ -246,16 +247,16 @@ export class PluginKeychainAwsSm
}
}

async set<T>(key: string, value: T): Promise<void> {
async set(key: string, value: string): Promise<void> {
const fnTag = `${this.className}#set(key: string)`;
const awsClient = this.getAwsClient();
try {
(await awsClient
await awsClient
.createSecret({
Name: key,
SecretString: value as any,
SecretString: value,
})
.promise()) as any;
.promise();
} catch (ex) {
this.log.error(` ${fnTag}: Error writing secret "${key}"`);
throw ex;
Expand All @@ -266,12 +267,12 @@ export class PluginKeychainAwsSm
const fnTag = `${this.className}#delete(key: string)`;
const awsClient = this.getAwsClient();
try {
(await awsClient
await awsClient
.deleteSecret({
SecretId: key,
ForceDeleteWithoutRecovery: true,
})
.promise()) as any;
.promise();
} catch (ex) {
this.log.error(`${fnTag} Error deleting secret "${key}"`);
throw ex;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export class PluginKeychainAzureKv
private endpoints: IWebServiceEndpoint[] | undefined;
private azureKvClient: SecretClient;

public get className() {
public get className(): string {
return PluginKeychainAzureKv.CLASS_NAME;
}

Expand Down Expand Up @@ -187,20 +187,20 @@ export class PluginKeychainAzureKv
}

public getEncryptionAlgorithm(): string {
return null as any;
return (null as unknown) as string;
}

public getAzureKvClient(): SecretClient {
return this.azureKvClient;
}

async get<T>(key: string): Promise<T> {
async get(key: string): Promise<string> {
const keyVaultSecret: KeyVaultSecret = await this.azureKvClient.getSecret(
key,
);
if (keyVaultSecret) {
const result = keyVaultSecret.value;
return (result as unknown) as T;
return result as string;
} else {
throw new Error(`${key} secret not found`);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export class PluginKeychainGoogleSm
private endpoints: IWebServiceEndpoint[] | undefined;
private googleSmClient: SecretManagerServiceClient;

public get className() {
public get className(): string {
return PluginKeychainGoogleSm.CLASS_NAME;
}

Expand Down Expand Up @@ -118,7 +118,7 @@ export class PluginKeychainGoogleSm
}

public getEncryptionAlgorithm(): string {
return "AES-256" as any;
return "AES-256";
}

public async onPluginInit(): Promise<unknown> {
Expand All @@ -129,13 +129,15 @@ export class PluginKeychainGoogleSm
return this.googleSmClient;
}

async get<T>(key: string): Promise<T> {
async get(key: string): Promise<string> {
const accessResponse = await this.googleSmClient.getSecret({
name: key,
});
if (accessResponse[0]) {
const result = accessResponse[0];
return (result as unknown) as T;
// FIXME: We need to verify if this actually works because
// based on the type definitions of the underlying library it should not.
return (result as unknown) as string;
} else {
throw new Error(`${key} secret not found.`);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,15 @@ import {

export interface IPluginKeychainMemoryOptions extends ICactusPluginOptions {
logLevel?: LogLevelDesc;
backend?: Map<string, unknown>;
backend?: Map<string, string>;
keychainId: string;
prometheusExporter?: PrometheusExporter;
}

export class PluginKeychainMemory {
public static readonly CLASS_NAME = "PluginKeychainMemory";

private readonly backend: Map<string, unknown>;
private readonly backend: Map<string, string>;
private readonly log: Logger;
private readonly instanceId: string;
public prometheusExporter: PrometheusExporter;
Expand Down Expand Up @@ -116,15 +116,20 @@ export class PluginKeychainMemory {
return;
}

async get<T>(key: string): Promise<T> {
return this.backend.get(key) as T;
async get(key: string): Promise<string> {
const value = this.backend.get(key);
if (value) {
return value;
} else {
throw new Error(`Keychain entry for "${key}" not found.`);
}
}

async has(key: string): Promise<boolean> {
return this.backend.has(key);
}

async set<T>(key: string, value: T): Promise<void> {
async set(key: string, value: string): Promise<void> {
this.backend.set(key, value);
this.prometheusExporter.setTotalKeyCounter(this.backend.size);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,7 @@ test("PluginKeychainMemory", (t1: Test) => {
const hasAfterDelete1 = await plugin.has(key1);
t.false(hasAfterDelete1, "hasAfterDelete === false OK");

const valueAfterDelete1 = await plugin.get(key1);
t.notok(valueAfterDelete1, "valueAfterDelete falsy OK");
await t.rejects(plugin.get(key1), key1);
{
const res = await apiClient.getPrometheusMetricsV1();
const promMetricsOutput =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,20 +119,17 @@ export class PluginKeychainVaultRemoteAdapter
}
}

public async get<T>(key: string): Promise<T> {
public async get(key: string): Promise<string> {
const { data } = await this.backend.getKeychainEntryV1({ key });
// FIXME what to do here? Does it make any sense to have the get() method
// of the keychain be generically parameterized when we know we can only
// return a string anyway?
return (data.value as unknown) as T;
return data.value;
}

public async set<T>(key: string, value: T): Promise<void> {
public async set(key: string, value: string): Promise<void> {
// FIXME Does it make any sense to have the set() method be generic?
await this.backend.setKeychainEntryV1({
key,
value: (value as unknown) as string,
});
await this.backend.setKeychainEntryV1({ key, value });
}

public async delete(key: string): Promise<void> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ export class PluginKeychainVault implements IPluginWebService, IPluginKeychain {
return `${this.kvSecretsMountPath}${key}`;
}

async get<T>(key: string): Promise<T> {
async get(key: string): Promise<string> {
const fnTag = `${this.className}#get(key: string)`;
const path = this.pathFor(key);
try {
Expand All @@ -233,8 +233,9 @@ export class PluginKeychainVault implements IPluginWebService, IPluginKeychain {
);
}
} catch (ex) {
// FIXME: Throw if not found, detect it in the endpoint code, status=404
if (ex?.response?.statusCode === HttpStatus.NOT_FOUND) {
return (null as unknown) as T;
return (null as unknown) as string;
} else {
this.log.error(`Retrieval of "${key}" crashed:`, ex);
throw ex;
Expand Down Expand Up @@ -269,7 +270,7 @@ export class PluginKeychainVault implements IPluginWebService, IPluginKeychain {
}
}

async set<T>(key: string, value: T): Promise<void> {
async set(key: string, value: string): Promise<void> {
const path = this.pathFor(key);
await this.backend.write(path, { data: { value } });
this.prometheusExporter.setTotalKeyCounter(key, "set");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import type { Express } from "express";
import { promisify } from "util";
import { Optional } from "typescript-optional";
import Web3 from "web3";
import { AbiItem } from "web3-utils";

import { Contract, ContractSendMethod } from "web3-eth-contract";
import { TransactionReceipt } from "web3-eth";
Expand Down Expand Up @@ -321,7 +320,8 @@ export class PluginLedgerConnectorBesu
`${fnTag} Cannot create an instance of the contract because the contractName and the contractName of the JSON doesn't match`,
);
}
const contractJSON = (await keychainPlugin.get(contractName)) as any;
const contractStr = await keychainPlugin.get(contractName);
const contractJSON = JSON.parse(contractStr);
if (
contractJSON.networks === undefined ||
contractJSON.networks[networkId] === undefined ||
Expand Down Expand Up @@ -566,7 +566,7 @@ export class PluginLedgerConnectorBesu

// Now use the found keychain plugin to actually perform the lookup of
// the private key that we need to run the transaction.
const privateKeyHex = await keychainPlugin?.get<string>(keychainEntryKey);
const privateKeyHex = await keychainPlugin?.get(keychainEntryKey);

return this.transactPrivateKey({
transactionConfig,
Expand Down Expand Up @@ -675,12 +675,8 @@ export class PluginLedgerConnectorBesu
if (status && contractAddress) {
const networkInfo = { address: contractAddress };

type SolcJson = {
abi: AbiItem[];
networks: unknown;
};
const contractJSON = await keychainPlugin.get<SolcJson>(contractName);

const contractStr = await keychainPlugin.get(contractName);
const contractJSON = JSON.parse(contractStr);
this.log.debug("Contract JSON: \n%o", JSON.stringify(contractJSON));

const contract = new this.web3.eth.Contract(
Expand All @@ -692,7 +688,7 @@ export class PluginLedgerConnectorBesu
const network = { [networkId]: networkInfo };
contractJSON.networks = network;

keychainPlugin.set(contractName, contractJSON);
await keychainPlugin.set(contractName, JSON.stringify(contractJSON));
}
} else {
throw new Error(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ test("can get past logs of an account", async (t: Test) => {
});
keychainPlugin.set(
HelloWorldContractJson.contractName,
HelloWorldContractJson,
JSON.stringify(HelloWorldContractJson),
);
const factory = new PluginFactoryLedgerConnector({
pluginImportType: PluginImportType.Local,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ test(testCase, async (t: Test) => {
});
keychainPlugin.set(
HelloWorldContractJson.contractName,
HelloWorldContractJson,
JSON.stringify(HelloWorldContractJson),
);
const factory = new PluginFactoryLedgerConnector({
pluginImportType: PluginImportType.Local,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ test("can get balance of an account", async (t: Test) => {
});
keychainPlugin.set(
HelloWorldContractJson.contractName,
HelloWorldContractJson,
JSON.stringify(HelloWorldContractJson),
);
const factory = new PluginFactoryLedgerConnector({
pluginImportType: PluginImportType.Local,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ test("can get block from blockchain", async (t: Test) => {
});
keychainPlugin.set(
HelloWorldContractJson.contractName,
HelloWorldContractJson,
JSON.stringify(HelloWorldContractJson),
);
const factory = new PluginFactoryLedgerConnector({
pluginImportType: PluginImportType.Local,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ test("can get past logs of an account", async (t: Test) => {
});
keychainPlugin.set(
HelloWorldContractJson.contractName,
HelloWorldContractJson,
JSON.stringify(HelloWorldContractJson),
);
const factory = new PluginFactoryLedgerConnector({
pluginImportType: PluginImportType.Local,
Expand Down
Loading

0 comments on commit a340765

Please sign in to comment.