From e9601d4bf2b184bb8d41ff8435ff918e1db054c4 Mon Sep 17 00:00:00 2001 From: Eela Nagaraj <7308464+eelanagaraj@users.noreply.github.com> Date: Fri, 21 Oct 2022 20:20:52 +0200 Subject: [PATCH] Add enable config parameter for legacy PNP signer endpoints (#9967) * Add separate enable config value for legacy PNP signer endpoint * Driveby change: remove unnecessary jest line from test utils * Add legacy enabled param to celotool and helm charts * Remove rpc-wallet dep --- .env | 1 + dependency-graph.json | 3 +- packages/celotool/src/lib/env-utils.ts | 2 + packages/celotool/src/lib/odis.ts | 1 + .../odis/templates/signer-deployment.yaml | 1 + .../combiner/test/integration/domain.test.ts | 4 ++ .../test/integration/legacypnp.test.ts | 8 +++- .../combiner/test/integration/pnp.test.ts | 4 ++ .../common/src/test/utils.ts | 2 - .../phone-number-privacy/signer/package.json | 1 - .../phone-number-privacy/signer/src/config.ts | 8 ++++ .../phone-number-privacy/signer/src/server.ts | 8 ++-- .../end-to-end/domain/domain.service.test.ts | 2 +- .../signer/test/integration/legacypnp.test.ts | 40 +++++++++++++++---- 14 files changed, 65 insertions(+), 20 deletions(-) diff --git a/.env b/.env index 4765fe2f390..73dd42eabc6 100644 --- a/.env +++ b/.env @@ -209,6 +209,7 @@ ODIS_SIGNER_DOCKER_IMAGE_TAG=oblivious-decentralized-identifier-service-1.1.10 ODIS_SIGNER_BLOCKCHAIN_PROVIDER=https://alfajores-forno.celo-testnet.org ODIS_SIGNER_DOMAINS_API_ENABLED=true ODIS_SIGNER_PNP_API_ENABLED=true +ODIS_SIGNER_LEGACY_PNP_API_ENABLED=true # ODIS signer 0 Azure info AZURE_ODIS0_CENTRALUS_AZURE_SUBSCRIPTION_ID=97e2b592-255b-4f92-bce0-127257163c36 diff --git a/dependency-graph.json b/dependency-graph.json index b064f90957f..1a3092ab3c4 100644 --- a/dependency-graph.json +++ b/dependency-graph.json @@ -135,8 +135,7 @@ "@celo/identity", "@celo/phone-number-privacy-common", "@celo/utils", - "@celo/wallet-hsm-azure", - "@celo/wallet-rpc" + "@celo/wallet-hsm-azure" ] }, "@celo/base": { diff --git a/packages/celotool/src/lib/env-utils.ts b/packages/celotool/src/lib/env-utils.ts index 910423b90f6..ccb68093e75 100644 --- a/packages/celotool/src/lib/env-utils.ts +++ b/packages/celotool/src/lib/env-utils.ts @@ -121,6 +121,7 @@ export enum envVar { ODIS_SIGNER_BLOCKCHAIN_PROVIDER = 'ODIS_SIGNER_BLOCKCHAIN_PROVIDER', ODIS_SIGNER_DOMAINS_API_ENABLED = 'ODIS_SIGNER_DOMAINS_API_ENABLED', ODIS_SIGNER_PNP_API_ENABLED = 'ODIS_SIGNER_PNP_API_ENABLED', + ODIS_SIGNER_LEGACY_PNP_API_ENABLED = 'ODIS_SIGNER_LEGACY_PNP_API_ENABLED', ORACLE_DOCKER_IMAGE_REPOSITORY = 'ORACLE_DOCKER_IMAGE_REPOSITORY', ORACLE_DOCKER_IMAGE_TAG = 'ORACLE_DOCKER_IMAGE_TAG', ORACLE_UNUSED_ORACLE_ADDRESSES = 'ORACLE_UNUSED_ORACLE_ADDRESSES', @@ -226,6 +227,7 @@ export enum DynamicEnvVar { ODIS_SIGNER_AZURE_KEYVAULT_DOMAINS_KEY_LATEST_VERSION = '{{ context }}_ODIS_SIGNER_AZURE_KEYVAULT_DOMAINS_KEY_LATEST_VERSION', ODIS_SIGNER_DOMAINS_API_ENABLED = '{{ context }}_ODIS_SIGNER_DOMAINS_API_ENABLED', ODIS_SIGNER_PHONE_NUMBER_PRIVACY_API_ENABLED = '{{ context }}_ODIS_SIGNER_PNP_API_ENABLED', + ODIS_SIGNER_LEGACY_PHONE_NUMBER_PRIVACY_API_ENABLED = '{{ context }}_ODIS_SIGNER_LEGACY_PNP_API_ENABLED', ODIS_SIGNER_DB_HOST = '{{ context }}_ODIS_SIGNER_DB_HOST', ODIS_SIGNER_DB_PORT = '{{ context }}_ODIS_SIGNER_DB_PORT', ODIS_SIGNER_DB_USERNAME = '{{ context }}_ODIS_SIGNER_DB_USERNAME', diff --git a/packages/celotool/src/lib/odis.ts b/packages/celotool/src/lib/odis.ts index 0e0953b8bf9..b309fa1898e 100644 --- a/packages/celotool/src/lib/odis.ts +++ b/packages/celotool/src/lib/odis.ts @@ -172,6 +172,7 @@ async function helmParameters(celoEnv: string, context: string) { `--set keystore.pnpKeyLatestVersion=${keyVaultConfig.pnpKeyLatestVersion}`, `--set keystore.domainsKeyLatestVersion=${keyVaultConfig.domainsKeyLatestVersion}`, `--set api.pnpAPIEnabled=${fetchEnv(envVar.ODIS_SIGNER_PNP_API_ENABLED)}`, + `--set api.legacyPnpAPIEnabled=${fetchEnv(envVar.ODIS_SIGNER_LEGACY_PNP_API_ENABLED)}`, `--set api.domainsAPIEnabled=${fetchEnv(envVar.ODIS_SIGNER_DOMAINS_API_ENABLED)}`, `--set blockchainProvider=${fetchEnv(envVar.ODIS_SIGNER_BLOCKCHAIN_PROVIDER)}`, `--set blockchainApiKey=${blockchainConfig.blockchainApiKey}`, diff --git a/packages/helm-charts/odis/templates/signer-deployment.yaml b/packages/helm-charts/odis/templates/signer-deployment.yaml index a9ef72b87c4..ec10c4181ea 100644 --- a/packages/helm-charts/odis/templates/signer-deployment.yaml +++ b/packages/helm-charts/odis/templates/signer-deployment.yaml @@ -53,6 +53,7 @@ spec: {{ include "common.env-var" (dict "name" "DOMAINS_LATEST_KEY_VERSION" "dict" .Values.keystore "value_name" "domainsKeyLatestVersion") | indent 12 }} {{ include "common.env-var" (dict "name" "DOMAINS_API_ENABLED" "dict" .Values.api "value_name" "domainsAPIEnabled") | indent 12 }} {{ include "common.env-var" (dict "name" "PHONE_NUMBER_PRIVACY_API_ENABLED" "dict" .Values.api "value_name" "pnpAPIEnabled") | indent 12 }} +{{ include "common.env-var" (dict "name" "LEGACY_PHONE_NUMBER_PRIVACY_API_ENABLED" "dict" .Values.api "value_name" "legacyPnpAPIEnabled") | indent 12 }} - name: DB_PASSWORD valueFrom: secretKeyRef: diff --git a/packages/phone-number-privacy/combiner/test/integration/domain.test.ts b/packages/phone-number-privacy/combiner/test/integration/domain.test.ts index f9cca121464..cfebd876c66 100644 --- a/packages/phone-number-privacy/combiner/test/integration/domain.test.ts +++ b/packages/phone-number-privacy/combiner/test/integration/domain.test.ts @@ -87,6 +87,10 @@ const signerConfig: SignerConfig = { enabled: false, shouldFailOpen: true, }, + legacyPhoneNumberPrivacy: { + enabled: false, + shouldFailOpen: true, + }, }, attestations: { numberAttestationsRequired: 3, diff --git a/packages/phone-number-privacy/combiner/test/integration/legacypnp.test.ts b/packages/phone-number-privacy/combiner/test/integration/legacypnp.test.ts index 8eba868383d..c93ab0601d6 100644 --- a/packages/phone-number-privacy/combiner/test/integration/legacypnp.test.ts +++ b/packages/phone-number-privacy/combiner/test/integration/legacypnp.test.ts @@ -89,6 +89,10 @@ const signerConfig: SignerConfig = { enabled: false, }, phoneNumberPrivacy: { + enabled: false, + shouldFailOpen: true, + }, + legacyPhoneNumberPrivacy: { enabled: true, shouldFailOpen: true, }, @@ -729,7 +733,7 @@ describe(`legacyPnpService: ${CombinerEndpoint.LEGACY_PNP_SIGN}`, () => { describe('when 2/3 of signers are disabled', () => { beforeEach(async () => { const configWithApiDisabled: SignerConfig = JSON.parse(JSON.stringify(signerConfig)) - configWithApiDisabled.api.phoneNumberPrivacy.enabled = false + configWithApiDisabled.api.legacyPhoneNumberPrivacy.enabled = false signer1 = startSigner(signerConfig, signerDB1, keyProvider1).listen(3001) signer2 = startSigner(configWithApiDisabled, signerDB2, keyProvider2).listen(3002) signer3 = startSigner(configWithApiDisabled, signerDB3, keyProvider3).listen(3003) @@ -750,7 +754,7 @@ describe(`legacyPnpService: ${CombinerEndpoint.LEGACY_PNP_SIGN}`, () => { describe('when 1/3 of signers are disabled', () => { beforeEach(async () => { const configWithApiDisabled: SignerConfig = JSON.parse(JSON.stringify(signerConfig)) - configWithApiDisabled.api.phoneNumberPrivacy.enabled = false + configWithApiDisabled.api.legacyPhoneNumberPrivacy.enabled = false signer1 = startSigner(signerConfig, signerDB1, keyProvider1).listen(3001) signer2 = startSigner(signerConfig, signerDB2, keyProvider2).listen(3002) signer3 = startSigner(configWithApiDisabled, signerDB3, keyProvider3).listen(3003) diff --git a/packages/phone-number-privacy/combiner/test/integration/pnp.test.ts b/packages/phone-number-privacy/combiner/test/integration/pnp.test.ts index 6c1b5eb26a9..36603bc0fbf 100644 --- a/packages/phone-number-privacy/combiner/test/integration/pnp.test.ts +++ b/packages/phone-number-privacy/combiner/test/integration/pnp.test.ts @@ -96,6 +96,10 @@ const signerConfig: SignerConfig = { enabled: true, shouldFailOpen: true, }, + legacyPhoneNumberPrivacy: { + enabled: false, + shouldFailOpen: true, + }, }, attestations: { numberAttestationsRequired: 3, diff --git a/packages/phone-number-privacy/common/src/test/utils.ts b/packages/phone-number-privacy/common/src/test/utils.ts index b1079808079..0d6c2505e2b 100644 --- a/packages/phone-number-privacy/common/src/test/utils.ts +++ b/packages/phone-number-privacy/common/src/test/utils.ts @@ -16,8 +16,6 @@ import { import { signWithRawKey } from '../utils/authentication' import { genSessionID } from '../utils/logger' -jest.fn() - export function createMockAttestation(getVerifiedStatus: jest.Mock) { return { getVerifiedStatus, diff --git a/packages/phone-number-privacy/signer/package.json b/packages/phone-number-privacy/signer/package.json index d12478ee95b..c5e98af562b 100644 --- a/packages/phone-number-privacy/signer/package.json +++ b/packages/phone-number-privacy/signer/package.json @@ -56,7 +56,6 @@ "promise.allsettled": "^1.0.2" }, "devDependencies": { - "@celo/wallet-rpc": "2.2.1-dev", "@types/btoa": "^1.2.3", "@types/express": "^4.17.6", "@types/supertest": "^2.0.12", diff --git a/packages/phone-number-privacy/signer/src/config.ts b/packages/phone-number-privacy/signer/src/config.ts index 89e56d05dec..89dc0158fb3 100644 --- a/packages/phone-number-privacy/signer/src/config.ts +++ b/packages/phone-number-privacy/signer/src/config.ts @@ -47,6 +47,10 @@ export interface SignerConfig { enabled: boolean shouldFailOpen: boolean } + legacyPhoneNumberPrivacy: { + enabled: boolean + shouldFailOpen: boolean + } } attestations: { numberAttestationsRequired: number @@ -125,6 +129,10 @@ export const config: SignerConfig = { enabled: toBool(env.PHONE_NUMBER_PRIVACY_API_ENABLED, false), shouldFailOpen: toBool(env.FULL_NODE_ERRORS_SHOULD_FAIL_OPEN, true), }, + legacyPhoneNumberPrivacy: { + enabled: toBool(env.LEGACY_PHONE_NUMBER_PRIVACY_API_ENABLED, false), + shouldFailOpen: toBool(env.LEGACY_FULL_NODE_ERRORS_SHOULD_FAIL_OPEN, true), + }, }, attestations: { numberAttestationsRequired: Number(env.ATTESTATIONS_NUMBER_ATTESTATIONS_REQUIRED ?? 3), diff --git a/packages/phone-number-privacy/signer/src/server.ts b/packages/phone-number-privacy/signer/src/server.ts index af7b5b69f15..5078dbac3d2 100644 --- a/packages/phone-number-privacy/signer/src/server.ts +++ b/packages/phone-number-privacy/signer/src/server.ts @@ -122,8 +122,8 @@ export function startSigner( legacyPnpQuotaService, keyProvider, new LegacyPnpSignIO( - config.api.phoneNumberPrivacy.enabled, - config.api.phoneNumberPrivacy.shouldFailOpen, + config.api.legacyPhoneNumberPrivacy.enabled, + config.api.legacyPhoneNumberPrivacy.shouldFailOpen, kit ) ) @@ -133,8 +133,8 @@ export function startSigner( config, legacyPnpQuotaService, new LegacyPnpQuotaIO( - config.api.phoneNumberPrivacy.enabled, - config.api.phoneNumberPrivacy.shouldFailOpen, + config.api.legacyPhoneNumberPrivacy.enabled, + config.api.legacyPhoneNumberPrivacy.shouldFailOpen, kit ) ) diff --git a/packages/phone-number-privacy/signer/test/end-to-end/domain/domain.service.test.ts b/packages/phone-number-privacy/signer/test/end-to-end/domain/domain.service.test.ts index e9750c6ba3f..411598c0e06 100644 --- a/packages/phone-number-privacy/signer/test/end-to-end/domain/domain.service.test.ts +++ b/packages/phone-number-privacy/signer/test/end-to-end/domain/domain.service.test.ts @@ -4,8 +4,8 @@ import { SequentialDelayDomain, SignerEndpoint as Endpoint, } from '@celo/phone-number-privacy-common' +import { ACCOUNT_ADDRESS2 } from '@celo/phone-number-privacy-common/lib/test/values' import { defined, noBool, noString } from '@celo/utils/lib/sign-typed-data-utils' -import { ACCOUNT_ADDRESS2 } from '@celo/wallet-rpc/lib/rpc-wallet.test' import 'isomorphic-fetch' import { contractKit } from '../../../../combiner/test/end-to-end/resources' diff --git a/packages/phone-number-privacy/signer/test/integration/legacypnp.test.ts b/packages/phone-number-privacy/signer/test/integration/legacypnp.test.ts index cb83628fc50..32927684406 100644 --- a/packages/phone-number-privacy/signer/test/integration/legacypnp.test.ts +++ b/packages/phone-number-privacy/signer/test/integration/legacypnp.test.ts @@ -120,7 +120,7 @@ describe('legacyPNP', () => { const _config: typeof config = JSON.parse(JSON.stringify(config)) _config.db.type = SupportedDatabase.Sqlite _config.keystore.type = SupportedKeystore.MOCK_SECRET_MANAGER - _config.api.phoneNumberPrivacy.enabled = true + _config.api.legacyPhoneNumberPrivacy.enabled = true const expectedSignature = expectedSignatures[_config.keystore.keys.phoneNumberPrivacy.latest - 1] @@ -569,7 +569,7 @@ describe('legacyPNP', () => { it('Should respond with 503 on disabled api', async () => { const configWithApiDisabled: typeof _config = JSON.parse(JSON.stringify(_config)) - configWithApiDisabled.api.phoneNumberPrivacy.enabled = false + configWithApiDisabled.api.legacyPhoneNumberPrivacy.enabled = false const appWithApiDisabled = startSigner(configWithApiDisabled, db, keyProvider) const req = getLegacyPnpQuotaRequest(ACCOUNT_ADDRESS1, IDENTIFIER) const authorization = getPnpRequestAuthorization(req, PRIVATE_KEY1) @@ -1148,7 +1148,7 @@ describe('legacyPNP', () => { it('Should respond with 503 on disabled api', async () => { const configWithApiDisabled: typeof _config = JSON.parse(JSON.stringify(_config)) - configWithApiDisabled.api.phoneNumberPrivacy.enabled = false + configWithApiDisabled.api.legacyPhoneNumberPrivacy.enabled = false const appWithApiDisabled = startSigner(configWithApiDisabled, db, keyProvider) const req = getLegacyPnpSignRequest( @@ -1174,6 +1174,14 @@ describe('legacyPNP', () => { }) describe('interactions between legacy and new endpoints', () => { + const configWithPNPEnabled: typeof _config = JSON.parse(JSON.stringify(_config)) + configWithPNPEnabled.api.phoneNumberPrivacy.enabled = true + let appWithPNPEnabled: any + + beforeEach(() => { + appWithPNPEnabled = startSigner(configWithPNPEnabled, db, keyProvider) + }) + // Keep both of these cases with the legacy test suite // since once this endpoint is deprecated, these tests will no longer be needed it('Should not be affected by requests and queries from the new endpoint', async () => { @@ -1185,7 +1193,13 @@ describe('legacyPNP', () => { AuthenticationMethod.WALLET_KEY ) const authorization = getPnpRequestAuthorization(req, PRIVATE_KEY1) - const res = await sendRequest(req, authorization, SignerEndpoint.PNP_SIGN) + const res = await sendRequest( + req, + authorization, + SignerEndpoint.PNP_SIGN, + undefined, + appWithPNPEnabled + ) expect(res.status).toBe(200) expect(res.body).toStrictEqual({ success: true, @@ -1209,7 +1223,9 @@ describe('legacyPNP', () => { const legacyRes = await sendRequest( legacyReq, legacyAuthorization, - SignerEndpoint.LEGACY_PNP_SIGN + SignerEndpoint.LEGACY_PNP_SIGN, + undefined, + appWithPNPEnabled ) expect(legacyRes.status).toBe(200) expect(legacyRes.body).toStrictEqual({ @@ -1226,7 +1242,7 @@ describe('legacyPNP', () => { ) }) - it('Should affect the requests and queries to the new endpoint', async () => { + it('Should not affect the requests and queries to the new endpoint', async () => { const legacyReq = getLegacyPnpSignRequest( ACCOUNT_ADDRESS1, BLINDED_PHONE_NUMBER, @@ -1237,7 +1253,9 @@ describe('legacyPNP', () => { const legacyRes = await sendRequest( legacyReq, legacyAuthorization, - SignerEndpoint.LEGACY_PNP_SIGN + SignerEndpoint.LEGACY_PNP_SIGN, + undefined, + appWithPNPEnabled ) expect(legacyRes.status).toBe(200) expect(legacyRes.body).toStrictEqual({ @@ -1260,7 +1278,13 @@ describe('legacyPNP', () => { AuthenticationMethod.WALLET_KEY ) const authorization = getPnpRequestAuthorization(req, PRIVATE_KEY1) - const res = await sendRequest(req, authorization, SignerEndpoint.PNP_SIGN) + const res = await sendRequest( + req, + authorization, + SignerEndpoint.PNP_SIGN, + undefined, + appWithPNPEnabled + ) expect(res.status).toBe(200) expect(res.body).toStrictEqual({ success: true,