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.

Sign up for GitHub

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

Jump to bottom

authn: Allow override of Cognito user pool and client ids by environment variables #273

Merged
merged 2 commits into from
Apr 25, 2023
Merged

authn: Allow override of Cognito user pool and client ids by environment variables #273

merged 2 commits into from
Apr 25, 2023

Conversation

tsibley
Copy link
Member

@tsibley tsibley commented Apr 24, 2023 •
edited
Loading

Useful for development/testing against non-production infrastructure. These aren't ergonomic to use, but they at least make it possible to test without changing the CLI's source code. Developer ergonomics can come later if warranted.

Related-to: nextstrain/nextstrain.org#656

Testing

I ran a local nextstrain.org server (with the default "testing" env) and then set

export NEXTSTRAIN_COGNITO_USER_POOL_ID="$(jq -r .COGNITO_USER_POOL_ID ../nextstrain.org/env/testing/config.json)"
export NEXTSTRAIN_COGNITO_CLI_CLIENT_ID="$(jq -r .COGNITO_CLI_CLIENT_ID ../nextstrain.org/env/testing/config.json)"
export NEXTSTRAIN_DOT_ORG=http://localhost:5000
nextstrain login
nextstrain whoami
nextstrain remote ls groups/test-private
  • Works locally
  • Checks pass

@tsibley
authn: Allow override of Cognito user pool and client ids by environm…
c5bb583 
…ent variables

Useful for development/testing against non-production infrastructure.
These aren't ergonomic to use, but they at least make it possible to
test without changing the CLI's source code.  Developer ergonomics can
come later if warranted.

Related-to: <nextstrain/nextstrain.org#656>
@tsibley tsibley requested a review from a team April 24, 2023 23:05
@tsibley tsibley force-pushed the trs/authn-for-dev branch from a3d88d4 to c5bb583 Compare April 24, 2023 23:05
jameshadfield
jameshadfield reviewed Apr 24, 2023
View reviewed changes
Copy link
Member

@jameshadfield jameshadfield left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept seems good 👍 Could you add example usage to the README and also clarify what's meant by the (nextstrain.org) testing environment (or link out to docs which clarify that).

@tsibley
Copy link
Member Author

tsibley commented Apr 25, 2023 •
edited
Loading

Added to the development doc in 81dd925.

CI is failing due to #274, which is unrelated, so I'll merge without it passing.

@tsibley tsibley merged commit c6998b4 into master Apr 25, 2023
@tsibley tsibley deleted the trs/authn-for-dev branch April 25, 2023 23:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jameshadfield jameshadfield jameshadfield left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tsibley @jameshadfield