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

Clarify experimental variables should not be changed for prod #108

Closed
surchs opened this issue Feb 3, 2025 · 1 comment · Fixed by #106
Closed

Clarify experimental variables should not be changed for prod #108

surchs opened this issue Feb 3, 2025 · 1 comment · Fixed by #106
Assignees
Labels
documentation Changes only affect the documentation quick fix Minimal planning and/or implementation work required. released This issue/pull request has been released. usability Issue affecting user or developer experience.

Comments

@surchs
Copy link
Contributor

surchs commented Feb 3, 2025

We have had questions come up about this section:

recipes/template.env

Lines 78 to 86 in ef1dcc5

# ---- SECURITY CONFIGURATION ----
# NOTE: EXPERIMENTAL, THESE SETTINGS ARE UNDER ACTIVE DEVELOPMENT AND CURRENTLY SHOULD BE MODIFIED FOR DEV DEPLOYMENTS ONLY.
# The below settings will be used for both the query tool and the f-API.
# NB_ENABLE_AUTH=false
# If NB_ENABLE_AUTH is set to true, you MUST provide a valid OAuth client ID for your query tool instance.
# To obtain an OAuth client ID to enable login with Google, see https://developers.google.com/identity/openid-connect/openid-connect#appsetup.
# NB_QUERY_CLIENT_ID=XXXX
# --------------------------------

Specifically, folks were wondering how to make "Auth" work in the tool. In other words: the warning in the .env file above these settings is not a strong enough signal for folks to not try anyway to enable auth in production. That's not good. We should quickly find a better way to handle these variables.

We should remove the reference to Google Auth since we are no longer using that.

Either:

  • make it unambiguously clear, that these variables are not to be changed in prod
  • rename the variables to more clearly demonstrate that they are experimental and only for AuthN, not AuthZ
  • remove these variables entirely from the template.env to avoid people ignoring the warnings
  • introduce a "development context" variable somewhere that, if set to "production", either yells about these values or just strips them directly
@surchs surchs added documentation Changes only affect the documentation usability Issue affecting user or developer experience. flag:schedule Flag issue that should go on the roadmap or backlog. labels Feb 3, 2025
@alyssadai alyssadai added the quick fix Minimal planning and/or implementation work required. label Feb 4, 2025
@alyssadai alyssadai moved this to Backlog in Neurobagel Feb 4, 2025
@alyssadai alyssadai removed the flag:schedule Flag issue that should go on the roadmap or backlog. label Feb 4, 2025
@alyssadai alyssadai moved this from Backlog to Specify - Done in Neurobagel Feb 5, 2025
@alyssadai alyssadai moved this from Specify - Done to Implement - Track in Neurobagel Feb 6, 2025
@alyssadai alyssadai moved this from Implement - Track to Implement - Active in Neurobagel Feb 6, 2025
@alyssadai alyssadai self-assigned this Feb 6, 2025
@alyssadai alyssadai moved this from Implement - Active to Implement - Track in Neurobagel Feb 6, 2025
@alyssadai alyssadai moved this from Implement - Track to Implement - Active in Neurobagel Feb 12, 2025
@alyssadai alyssadai moved this from Implement - Active to Review - Active in Neurobagel Feb 12, 2025
@github-project-automation github-project-automation bot moved this from Review - Active to Review - Done in Neurobagel Feb 14, 2025
Copy link
Contributor

🚀 Issue was released in v0.5.0 🚀

@neurobagel-bot neurobagel-bot bot added the released This issue/pull request has been released. label Feb 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Changes only affect the documentation quick fix Minimal planning and/or implementation work required. released This issue/pull request has been released. usability Issue affecting user or developer experience.
Projects
Status: Review - Done
2 participants