Move CICO provider url composition server-side #74

tarikbellamine · 2021-03-02T00:14:50Z

Description

Moving all provider API keys server-side for better security and ease of future key rotation.

Other changes

Formatting asset codes on ProviderOptionsScreen to dry up code
Added Ramp API key to enable debit card purchases

Tested

Manually

Related issues

Fixes Enable card payments on Ramp #68
Fixes Move CICO provider keys server-side #71

Backwards compatibility

Yes

jeanregisser

Thanks! Great changes overall.
However biggest security concern is about returning some private keys to the client.

packages/mobile/src/config.ts

packages/mobile/src/fiatExchanges/MoonPayScreen.tsx

packages/mobile/src/fiatExchanges/ProviderOptionsScreen.tsx

packages/mobile/src/fiatExchanges/RampScreen.tsx

packages/mobile/src/fiatExchanges/TransakScreen.tsx

packages/valora-auth/package.json

packages/valora-auth/src/index.ts

gnardini

Looks great, the code on valora-auth is very tidy :)

packages/mobile/src/fiatExchanges/utils.tsx

packages/valora-auth/.firebaserc

gnardini · 2021-03-05T01:15:39Z

packages/valora-auth/firebase-env-config.sh

+#!/usr/bin/env bash
+
+firebase functions:config:set \
+transak.url_staging=$(grep TRANSAK_URL_STAGING .env | cut -d '=' -f 2-) \


Not really necessary but we could have a bash function that receives the env variable name as parameter and runs grep $1 .env | cut -d '=' -f 2- and call it in all these lines to make them slightly more readable.

packages/valora-auth/src/index.ts

Addressed

jeanregisser

Cool! Couple of additional comments.

Also wondering if we should find a different name for valora-auth that doesn't contain valora.
i.e. to be more in line with the other services names we have and given we've put all Valora branding into a private repo.

packages/valora-auth/.firebaserc

packages/valora-auth/src/config.ts

tarikbellamine · 2021-03-06T00:52:24Z

Also wondering if we should find a different name for valora-auth that doesn't contain valora.
i.e. to be more in line with the other services names we have and given we've put all Valora branding into a private repo.

@jeanregisser I've renamed it to provider-url-service. I'm flexible on naming though so lmk if you have a suggestion you like more.

jeanregisser

Thanks! 👍
Left some optional comments, but since this is working and looks good relative to security, I'm approving 😄

jeanregisser · 2021-03-10T11:32:44Z

packages/provider-url-service/firebase-alfajores-env-config.sh

+firebase functions:config:set \
+transak.public_key=$(grep TRANSAK_PUBLIC_KEY_STAGING .env | cut -d '=' -f 2-) \
+transak.private_key=$(grep TRANSAK_PRIVATE_KEY_STAGING .env | cut -d '=' -f 2-) \
+ramp.public_key=$(grep RAMP_PUBLIC_KEY_STAGING .env | cut -d '=' -f 2-) \
+moonpay.public_key=$(grep MOONPAY_PUBLIC_KEY_STAGING .env | cut -d '=' -f 2-) \
+moonpay.private_key=$(grep MOONPAY_PRIVATE_KEY_STAGING .env | cut -d '=' -f 2-) \


nit: there's a way to import dotenv variables in a single command and also slightly more robustly (handles no quotes, single and double quotes) that we're using in our run_app.sh script https://github.com/celo-org/wallet/blob/4e6cc3a42d654fa072eccb79e6b836a44b123f7d/packages/mobile/scripts/run_app.sh#L42-L44

Leaving this somewhat manual for now because I couldn't figure out how to properly nest provider specific variables (e.g., do moonpay.public_key instead of moonpay_public_key) and didn't want to crowd the env namesapce

packages/provider-url-service/firebase-alfajores-env-config.sh

packages/provider-url-service/src/config.ts

packages/provider-url-service/src/index.ts

ValoraQA · 2021-03-23T12:53:53Z

Hi @tarikbellamine can you give more information to test this task. Thanks!

tarikbellamine · 2021-03-24T05:41:29Z

Hi @Celoqa! The way to test this is to see if the WebView or InAppBrowser loads correctly for each cash-in option. For example, if you select "MoonPay", then it should load the MoonPay url in app.

If there is a failure, you will see that there is a loading indicator going on forever when you select an option.

ValoraQA · 2021-03-24T09:26:41Z

Hi @tarikbellamine Thank you so much for giving more information about this task.
Here we have verified this task using latest Android Internal Build V1.12.0(1004294340) & Test flight build V1.12.0(49) and observe the followings.

Selecting MoonPay:

iOS: 1) Loading indicator is shown for 2seconds & "celo wants to use moonpay.io to signin popup is shown after 2 seconds.
2) User should redirected to Buy.moonpay.io inAppBrowser by tapping on continue button.
Android: 1) Loading indicator is shown for 2seconds & User should redirected to Buy.moonpay.io inAppBrowser after 2 seconds.

Selecting Simplex:

iOS: User should redirect to Valoraapp.com Webview by tapping on Simplex after 4 seconds
Android: User should redirect to Valoraapp.com Webview by tapping on Simplex after 4 seconds

Selecting Ramp:

iOS: 1) Loading indicator is shown for 2seconds & "celo wants to use ramp.network to signin popup is shown after 2 seconds.
2) User should redirected to Buy.ramp.network inAppBrowser by tapping on continue button.

Selecting Transak:

iOS: Loading indicator is shown for 4 seconds & user redirected to global.transak.com after 4 seconds
Android: Loading indicator is shown for 4 seconds & user redirected to global.transak.com after 4 seconds

Can you please let us know if we need to test anything else in this task.

tarikbellamine · 2021-03-24T21:42:18Z

That's all. Thanks!

tarikbellamine added 3 commits March 1, 2021 15:51

move provider key managment to cloud function

b1be909

add new file to encryption script

a24c934

drying up code

3f34a21

tarikbellamine requested review from asaj, cmcewen and timmoreton as code owners March 2, 2021 00:14

tarikbellamine added 2 commits March 1, 2021 16:25

fix tests

f0a5b5d

adding deprecation notice to moonpay-auth package

6043144

tarikbellamine requested a review from i1skn as a code owner March 2, 2021 00:29

tarikbellamine requested review from gnardini and removed request for timmoreton, asaj and i1skn March 2, 2021 00:29

tarikbellamine mentioned this pull request Mar 2, 2021

Enable card payments on Ramp #69

Merged

tarikbellamine added 3 commits March 1, 2021 16:40

remove irrelevant Moonpay snapshot test

52d5b97

linting

a7d6386

remove old snapshot

536863d

jeanregisser previously requested changes Mar 2, 2021

View reviewed changes

tarikbellamine added 4 commits March 2, 2021 13:00

removing private key returning from endpoint

678d896

Merge branch 'main' into tarikbellamine/cico-provider-keys

3144f55

pr feedback

1d41e43

cleanup

c1c816e

tarikbellamine requested a review from jeanregisser March 3, 2021 00:27

gnardini reviewed Mar 3, 2021

View reviewed changes

packages/valora-auth/src/index.ts Outdated Show resolved Hide resolved

tarikbellamine added 3 commits March 3, 2021 17:02

fix merge conflicts

496d86f

remove references to Ramp API keys

80e9af8

serve urls instead of api keys

a27208a

tarikbellamine changed the title ~~Move CICO provider keys server-side~~ Move CICO provider url composition server-side Mar 5, 2021

add minor url validation

e03e34e

gnardini approved these changes Mar 5, 2021

View reviewed changes

jeanregisser reviewed Mar 5, 2021

View reviewed changes

packages/valora-auth/.firebaserc Outdated Show resolved Hide resolved

packages/valora-auth/src/config.ts Outdated Show resolved Hide resolved

tarikbellamine added 2 commits March 5, 2021 16:47

more pr feedback

db04b98

rename service

824f79c

tarikbellamine added 3 commits March 8, 2021 14:10

split prod and staging key configs

69e123b

linting

bcf268b

more pr feedback

909feb9

jeanregisser approved these changes Mar 10, 2021

View reviewed changes

tarikbellamine added 2 commits March 10, 2021 16:46

refining deployment setup

a0a803e

fix gitignore

1f2aeb3

tarikbellamine added the automerge Have PR merge automatically when checks pass label Mar 11, 2021

Merge branch 'main' into tarikbellamine/cico-provider-keys

f2824d6

tarikbellamine merged commit b1c478f into main Mar 11, 2021

tarikbellamine deleted the tarikbellamine/cico-provider-keys branch March 11, 2021 08:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move CICO provider url composition server-side #74

Move CICO provider url composition server-side #74

tarikbellamine commented Mar 2, 2021 •

edited

Loading

jeanregisser left a comment •

edited

Loading

gnardini left a comment •

edited

Loading

gnardini Mar 5, 2021

jeanregisser left a comment •

edited

Loading

tarikbellamine commented Mar 6, 2021

jeanregisser left a comment

jeanregisser Mar 10, 2021

tarikbellamine Mar 11, 2021 •

edited

Loading

ValoraQA commented Mar 23, 2021 •

edited

Loading

tarikbellamine commented Mar 24, 2021

ValoraQA commented Mar 24, 2021

tarikbellamine commented Mar 24, 2021

Move CICO provider url composition server-side #74

Move CICO provider url composition server-side #74

Conversation

tarikbellamine commented Mar 2, 2021 • edited Loading

Description

Other changes

Tested

Related issues

Backwards compatibility

jeanregisser left a comment • edited Loading

Choose a reason for hiding this comment

gnardini left a comment • edited Loading

Choose a reason for hiding this comment

gnardini Mar 5, 2021

Choose a reason for hiding this comment

jeanregisser left a comment • edited Loading

Choose a reason for hiding this comment

tarikbellamine commented Mar 6, 2021

jeanregisser left a comment

Choose a reason for hiding this comment

jeanregisser Mar 10, 2021

Choose a reason for hiding this comment

tarikbellamine Mar 11, 2021 • edited Loading

Choose a reason for hiding this comment

ValoraQA commented Mar 23, 2021 • edited Loading

tarikbellamine commented Mar 24, 2021

ValoraQA commented Mar 24, 2021

tarikbellamine commented Mar 24, 2021

tarikbellamine commented Mar 2, 2021 •

edited

Loading

jeanregisser left a comment •

edited

Loading

gnardini left a comment •

edited

Loading

jeanregisser left a comment •

edited

Loading

tarikbellamine Mar 11, 2021 •

edited

Loading

ValoraQA commented Mar 23, 2021 •

edited

Loading