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

[DDO-2885] Remove claim verification from local development site.conf template #2408

Merged
merged 3 commits into from
Jun 23, 2023

Conversation

choover-broad
Copy link
Contributor

@choover-broad choover-broad commented Jun 22, 2023

Ticket: DDO-2885

The local dev site.conf template I merged last week is missing some claim allowlist clauses that are present in Terra's dev environment, so that eg. @broadinstitute.org identities get a 401. These claim checks as a security feature aren't necessary for local development so let's remove them.

Testing

I spun up a local Rawls off of this PR and verified that I could:

  • authenticate via Swagger with my BI account and execute GET /api/workspaces
  • execute
curl -H "content-type: application/json" --header "Authorization: Bearer `gcloud auth print-access-token`" https://local.broadinstitute.org:20443/api/workspaces

(which did not work prior to this PR)


PR checklist

  • Include the JIRA issue number in the PR description and title
  • Make sure Swagger is updated if API changes
    • ...and Orchestration's Swagger too!
  • If you changed anything in model/, then you should publish a new official rawls-model and update rawls-model in Orchestration's dependencies.
  • Get two thumbsworth of PR review
  • Verify all tests go green, including CI tests
  • Squash commits and merge to develop (branches are automatically deleted after merging)
  • Inform other teams of any substantial changes via Slack and/or email

@choover-broad choover-broad marked this pull request as ready for review June 22, 2023 20:31
Copy link
Contributor

@jyang-broad jyang-broad left a comment

Choose a reason for hiding this comment

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

removes the audienceAllowList from local-dev, doesn't touch prod so should be safe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants