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

Hide External API Keys #1054

Closed
Davinkjohnson opened this issue Jul 2, 2024 · 9 comments · Fixed by NCI-C4CP/connectApp#833, NCI-C4CP/biospecimen#733 or NCI-C4CP/studyManagerDashboard#12
Assignees
Labels
CCC Priority 1 Issues to be addressed in the current release

Comments

@Davinkjohnson
Copy link
Collaborator

Creating an issue to track work and progress.

@sonyekere sonyekere added the CCC Priority 1 Issues to be addressed in the current release label Jul 2, 2024
@Davinkjohnson
Copy link
Collaborator Author

From Amber 7/18:
After extensive research and consultation with Brian, I think that we're ultimately going about this API Key work all wrong. I don't really see a way to remove it from Git entirely which does not require adding the use of a builder like webpack, which would change a lot of people's workflow for no real gain. (I'm also not 100% sure this would work for the dev environment at all.) Moreover, I don't think that's really the appropriate way to restrict use of this key. The appropriate way (as laid out in this article) would be to restrict the URLs that each API key is authorized on.

The one exception here is the use of local development on localhost:5000, which allows much more arbitrary usage than any of the dev/stage/prod apps. My suggestion for this, which I believe is currently viable, would be to:

Remove localhost from the list of permitted URLs for the dev app (should ideally already not be allowed for stage and prod)
Add an additional key to the dev project which is set to permit localhost usage
Send that key out via Box or some other method to developers who need it and have developers update some kind of local-dev/config.js file with the key, with the file itself being in .gitignore to prevent it being committed.

@Davinkjohnson
Copy link
Collaborator Author

@anthonypetersen @we-ai @JoeArmani Any thoughts on Amber's proposed solution for our external keys? (i.e. those keys which are not possible to put in a secret.)

@anthonypetersen
Copy link
Contributor

sounds like a plan to me

@JoeArmani
Copy link
Collaborator

I agree this sounds like a good plan for Firebase keys.

@we-ai
Copy link
Collaborator

we-ai commented Jul 22, 2024

I agree that it's fine to keep Firebase API keys in repos, based on reasons mentioned.
Also, it's doable to hide those keys by removing those config.js files from our repos. Those files can be added back, manually or programmatically, to site files in CGP.

@amber-emmes
Copy link
Collaborator

I'm not entirely sure that's the case for the dev environment, which is hosted as a GitHub site. While it's possible to provide environment variables to GitHub, the file built with them would, in my research, still need to be uploaded to the GitHub branch from which the site is built.

@we-ai
Copy link
Collaborator

we-ai commented Jul 22, 2024

I'm not entirely sure that's the case for the dev environment, which is hosted as a GitHub site. While it's possible to provide environment variables to GitHub, the file built with them would, in my research, still need to be uploaded to the GitHub branch from which the site is built.

Yes, dev site files are in GitHub. Not sure whether they'll be moved to GCP or not.
Stage and prod site files are in GCP, and some files (like config files) can be removed from repos if needed.

@anthonypetersen
Copy link
Contributor

The long term plan is to move dev into a GCP environment

@amber-emmes
Copy link
Collaborator

The local dev key work is now done. I'm planning to time revocation of localhost access for the dev key for the next prod release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CCC Priority 1 Issues to be addressed in the current release
Projects
None yet
6 participants