Open jack-h-wang opened 1 year ago
Hey team! Please add your planning poker estimate with Zenhub @arnejduranovic @jack-h-wang @JessicaWNava @JFU-NAVA-PBC @thetaurean @luis-pabon-tf
Please add your planning poker estimate with Zenhub @mkalish
Need to discuss this ticket - @arnejduranovic to schedule a call/discussion on this during sprint 79
Implementation Proposal:
Need to provision similar for LOCAL DEV for same secret / sensitive data access experience.
List of environment variables, per receiver SFTP credentials PPK, PEM already provisioned in azure key vault, here only demo user/password:
PROD:
STAGING:
TEST:
./docs/docs-deprecated/getting-started/Using-an-apple-silicon-mac.md:RS_OKTA_clientId=0oa8uvan2i07YXJLk1d7 ./docker-compose.yml: - RS_OKTA_clientId=0oa8uvan2i07YXJLk1d7
OAUTH2 client ID is considered as app credential, like user name in user name / password
Report Stream OAUTH2 uses a flow that is OK between Parteners (STLTs), so it is good practice to keep it between parteners, showing up in public repo is open it up to general github users.
Note,
External SFTP settings in PROD/STAGING involve sender, receiver on boarding process as described in:
./docs/onboarding-users/transport/sftp.md
For this POC, the sensitive info is manually provisioned in key vault:
PROTOTYPE done, pushed the PR (no merge POC only):
smoke tests:
STEPS TO RUN THE PROTOTYPE:
prerequisite: provision Azure Account with Key Vault
Check the source code for the following:
SFTP credentials are no longer fetched from Hashicorp API, but just from ENV vars (which come from AZ Key Vault) This also apply to other credentials including AZ Storage connection string
As a result:
Hashicorp vault and Hashicorp secret service code can be removed.
AzureSecretService might need to stay for per receiver transportation credentials access (since these per receiver creds are not ppopulated as ENV vars as in step 4, they are fetched at run time.
Move review feedbacks and conversations here from PR (which is POC and will closed and deleted after research done).
=========== PR review feedbacks: I think this is an interesting idea, though I'm not sure this is 100% the right path.
Some thoughts:
there is the intention that this be an open source project, so we need to avoid adding local configuration that would prevent anyone from cloning and working on this repo For local dev, I'm not seeing why we would need to fetch the credentials from azure rather than using the local vault implementation (this is how it currently works for fetching the blob transport params) I think we need to work more closely with devops since ultimately where these credentials live is there domain. Taking a look in azure, it looks like value like the DB password and blob connection string exist as configuration on the function app I think the change in SftpTransport is actually undoing a working version of what we want to see where we dynamically load the credential out of the vault My read on the scope of this work was to work with the devops team to see if it could make sense to make sensitive values no longer be an app config setting and fetch them out the existing azure vault, whether that be via a change to the application code or as a change in how the azure function works (i.e. using managed identities rather than passwords.)
Yea, the research went beyond the original ask.
below are the considerations:
"there is the intention that this be an open source project, so we need to avoid adding local configuration that would prevent anyone from cloning and working on this repo"
JF: there are OAUTH client IDs (credentials) for prod and staging in the repo, they should be relocated to vaults, the community developers should plug in their environment specific OAUTH creds instead of report streams
"I think we need to work more closely with devops since ultimately where these credentials live is there domain. Taking a look in azure, it looks like value like the DB password and blob connection string exist as configuration on the function app"
JF: this is a original AC, and sure I can proceed on that, I got access to the vaults recently so I can check that
" I think the change in SftpTransport is actually undoing a working version of what we want to see where we dynamically load the credential out of the vault"
JF: So this is something broken? could you point me to the code? so a bug can be logged to track
"My read on the scope of this work was to work with the devops team to see if it could make sense to make sensitive values no longer be an app config setting and fetch them out the existing azure vault, whether that be via a change to the application code or as a change in how the azure function works (i.e. using managed identities rather than passwords.) "
JF: will checkout the contents of current appconfig, and keyvaults, and BTW, pdhstaging-appconfig and pdhstaging-keyvault are all Azure Key Vaults, any value for moving blob conn str from one to the other?
See this PR for a proof of concept and conversation that is tangential to this ticket but does not meet the AC: https://github.com/CDCgov/prime-reportstream/pull/12760
Clarification from Michael on what work this ticket should consist of:
work with the devops team to see if it could make sense to make sensitive values no longer be an app config setting and fetch them out the existing azure vault, whether that be via a change to the application code or as a change in how the azure function works (i.e. using managed identities rather than passwords.)
User Story
As a ReportStream engineer, I want to follow best practices for storage of sensitive data so that I can be sure sensitive data remains secure.
Description/Use Case
ReportStream stores connection settings to Azure blob stores for both accessing RS specific blob stores as well as connecting to non-RS blob stores when sending pipeline data. We should evaluate whether the storage of these settings is sufficiently secure or if we need to consider migrating these settings to a new location.
Risks/Impacts/Considerations
Dev Notes
Acceptance Criteria