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

PXP-6406 Feat/ga4gh scope #799

Merged
merged 6 commits into from
Jul 23, 2020
Merged

PXP-6406 Feat/ga4gh scope #799

merged 6 commits into from
Jul 23, 2020

Conversation

vpsx
Copy link
Contributor

@vpsx vpsx commented Jul 15, 2020

Also introduces a techdebt.md

New Features

  • Add new 'scopes' claim to access tokens (for GA4GH AAI); add ga4gh_passport_v1 to user/session scopes
  • Make (for now only one of the) claims returned in userinfo responsive to scopes present in access token

Deployment changes

  • If the fence config already has [CLIENT_, USER_, SESSION_]ALLOWED_SCOPES fields, and GA4GH AAI integration is desired, then add ga4gh_passport_v1 to all those lists. If default config values are being used then no change required.

@github-actions
Copy link

The style in this PR agrees with black. ✔️

This formatting comment was generated automatically by a script in uc-cdis/wool.

@coveralls
Copy link

coveralls commented Jul 15, 2020

Pull Request Test Coverage Report for Build 9346

  • 2 of 6 (33.33%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.04%) to 69.31%

Changes Missing Coverage Covered Lines Changed/Added Lines %
fence/resources/user/init.py 2 6 33.33%
Totals Coverage Status
Change from base Build 9321: -0.04%
Covered Lines: 5314
Relevant Lines: 7667

💛 - Coveralls

@vpsx vpsx added the test-google-googleDataAccessTest setting label to retry specific feature label Jul 16, 2020
@vpsx vpsx added test-google-googleDataAccessTest setting label to retry specific feature and removed test-google-googleDataAccessTest setting label to retry specific feature labels Jul 23, 2020
TECHDEBT.md Outdated
Comment on lines 4 to 6
Observed: July 2020
Impact: (If this tech debt affected your work somehow, add a +1 here with a date and note)
+1 Zoe 2020 July 15 This is an example of a +1
Copy link
Contributor

Choose a reason for hiding this comment

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

markdown displays paragraphs not separated by newlines as a single paragraph

TECHDEBT.md Outdated
##### What the solution might be:
GA4GH AAI already requires that a 'scope' claim be included in access tokens issued by Passport Brokers:
https://github.com/ga4gh/data-security/blob/master/AAI/AAIConnectProfile.md#access_token-issued-by-broker
So as of July 2020 we will put scopes in the 'scope' claim. However, this is in addition to keeping them in the 'aud' claim. Ideally we would only have the scopes in the 'scope' claim.
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

@vpsx vpsx merged commit f032491 into master Jul 23, 2020
@vpsx vpsx deleted the feat/ga4gh-scope branch July 23, 2020 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test-google-googleDataAccessTest setting label to retry specific feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants