PXP-6617 Remove scopes from aud claim in tokens #839

vpsx · 2020-09-23T21:30:43Z

Remove scopes from aud claim in Fence-issued tokens. Add custom scope claim for scopes, along with custom validation. Use and validate aud claim as originally intended. (Previously, the aud claim was being used for both audiences and scopes, and Fence/authutils had custom validation logic for the aud claim.)

Greater detail in the commit messages.

For a short definition of the aud claim see here.

Requires these changes to authutils: uc-cdis/authutils#47

New Features

Breaking Changes

Remove scopes from "aud" claim in Fence-issued tokens. This means that in general, old tokens (minted by previous Fence releases) will not pass validation by this Fence.
Access tokens will still include scopes for compatibility with other microservices, but scopes are no longer guaranteed and should no longer be expected in the aud claim from this version on. ID tokens, refresh tokens, and API keys will not have scopes in the aud claim.
Add custom "scope" claim in tokens and put scopes there instead. Custom validation for scopes claim added in Authutils 6.0.1
Add issuer to "aud" claim (alongside other aud values--so when there is a Fence client involved, this means the aud claim will include both Fence's BASE_URL and the client_id)
Remove custom "aud" claim validation and use normal validation instead (by bumping to Authutils 6.0.1). This means checking that the aud claim contains a value that identifies the consuming service, i.e. Fence's BASE_URL.

Bug Fixes

Improvements

Dependency updates

Requires authutils 6.0.1.

Deployment changes

This commit includes patches to allow old style refresh tokens and API keys to pass new style validation, while issuing new style refresh tokens and API keys. Since refresh tokens and API keys have a default TTL of 30 days, stay on this tag for 30 days if possible.
Old access tokens will NOT work once this is deployed so active sessions may be temporarily interrupted.

github-actions · 2020-09-23T21:31:41Z

The style in this PR agrees with black. ✔️

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

coveralls · 2020-09-23T21:35:35Z

Pull Request Test Coverage Report for Build 11137

32 of 55 (58.18%) changed or added relevant lines in 9 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.09%) to 70.684%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
fence/oidc/grants/refresh_token_grant.py	4	7	57.14%
fence/resources/storage/cdis_jwt.py	3	7	42.86%
fence/jwt/validate.py	3	19	15.79%

Totals
Change from base Build 11098:	-0.09%
Covered Lines:	6047
Relevant Lines:	8555

💛 - Coveralls

lgtm-com · 2020-09-23T22:04:18Z

This pull request fixes 1 alert when merging 69da6dd into ed8b837 - view on LGTM.com

fixed alerts:

1 for Unused import

lgtm-com · 2020-10-09T21:15:09Z

This pull request fixes 1 alert when merging 363a4a5 into 50ff568 - view on LGTM.com

fixed alerts:

1 for Unused import

* Don't pass scope to audiences arg in generate_token_response * In generate_id_token itself: * Don't append to audiences itself; instead make a copy * Check if client_id is None * Don't include aud claim in token if aud is empty

* New scope arg in validate_jwt defaults to {'openid'} but allows None * No longer use aud claim for scopes

* add separate scope claim to id, refresh tkns (already in access tkns) * rm scopes from aud claim in all tokens * set aud claim to client_id in all oidc tokens * also do all of the above for user API key * update docstrings some fence.jwt.token functions

…esh token

* see TECHDEBT.md for context * also rm unused has_oauth import

…not aud arg

* previously passed 'openid' only to appease validation code, bc using aud for scopes

vpsx mentioned this pull request Sep 23, 2020

PXP-6617 Add custom scopes validation and revert aud validation to default uc-cdis/authutils#47

Merged

vpsx force-pushed the fix/jwt-aud branch 2 times, most recently from b46f5cb to 69da6dd Compare September 23, 2020 21:43

vpsx force-pushed the fix/jwt-aud branch from a78a3ac to c382775 Compare October 9, 2020 20:38

vpsx force-pushed the fix/jwt-aud branch from 363a4a5 to 2b5aa17 Compare February 16, 2021 18:50

vpsx force-pushed the fix/jwt-aud branch 2 times, most recently from de3694a to 9416236 Compare April 29, 2021 22:00

This was referenced May 26, 2021

(PXP-6617): remove scopes from jwt aud claim uc-cdis/go-authutils#3

Merged

(PXP-6617): remove scopes from jwt aud claim uc-cdis/arborist#130

Merged

vpsx force-pushed the fix/jwt-aud branch from 734a3db to e3d026c Compare May 26, 2021 19:42

PlanXCyborg added the test-google-googleServiceAccountRemovalTest label May 26, 2021

vpsx added 15 commits May 27, 2021 09:47

fix(jwt-aud): Rm scopes from aud claim in id tokens

7a51d60

* Don't pass scope to audiences arg in generate_token_response * In generate_id_token itself: * Don't append to audiences itself; instead make a copy * Check if client_id is None * Don't include aud claim in token if aud is empty

feat(scope): Add JWT scope validation

e9548d8

* New scope arg in validate_jwt defaults to {'openid'} but allows None * No longer use aud claim for scopes

fix(aud): Pass aud to validate_jwt as string, not set

b510bfc

feat(scope): pass scope=None when validating session tokens

04f7da8

feat(scope): pass scopes to generate_[implicit,token]_response

9de4879

fix(aud): Validate aud claim in refresh token grant flow

6df3762

feat(scope): add scope claim to claims_supported

5e731d0

fix(aud): Look for scopes in scope not aud claim when validating refr…

67fd871

…esh token

fix(aud): Skip aud validation in login_required fn

44b977e

* see TECHDEBT.md for context * also rm unused has_oauth import

fix(aud-to-scope): authutils require_auth_header now takes scope arg …

7760681

…not aud arg

fix(aud-scope): Don't include aud in API keys; fix API key validation

72796f8

fix(scope): change scope param to set in require_auth_header

315d470

fix(aud-scope): Fix validate_request argument

5d70405

fix(aud-scope): Don't validate aud when blacklisting tokens

42539f3

* previously passed 'openid' only to appease validation code, bc using aud for scopes

This was referenced Aug 31, 2021

PXP-7947 KidsFirst prod 2021.06 + fence:5.2.1 uc-cdis/cdis-manifest#3436

Merged

Gen3 Monthly Release 2021.09 vpodc.org 1631031602 uc-cdis/cdis-manifest#3454

Closed

Gen3 Monthly Release 2021.09 genomel.bionimbus.org 1631040160 uc-cdis/cdis-manifest#3458

Closed

PlanXCyborg mentioned this pull request Sep 14, 2021

Updating CI env with integration202109 qa-midrc uc-cdis/gitops-qa#1623

Merged

This was referenced Sep 23, 2021

Gen3 Monthly Release 2021.09 vpodc.org 1632434141 uc-cdis/cdis-manifest#3548

Closed

Gen3 Monthly Release 2021.06 vpodc.org 1623270258 uc-cdis/cdis-manifest#3059

Closed

This was referenced Oct 19, 2021

feat: update to 2021.10 uc-cdis/gitops-qa#1701

Merged

feat: committing changes from qa-mickey uc-cdis/gitops-qa#1702

Merged

Feat/midrc staging uc-cdis/cdis-manifest#3668

Merged

PlanXCyborg mentioned this pull request Nov 17, 2021

feat(ga4gh-prep): make more similar to BDCat env for testing uc-cdis/gitops-qa#1769

Merged

paulineribeyre mentioned this pull request Dec 13, 2021

update lock uc-cdis/workspace-token-service#39

Merged

PlanXCyborg mentioned this pull request Dec 14, 2021

HEAL External Data - update services to 2021.12 uc-cdis/cdis-manifest#3917

Merged

This was referenced Mar 18, 2022

neuro 2022.03 2021.06 gen3-neuro.datacommons.io 1647626488 uc-cdis/cdis-manifest#4335

Closed

icgc pcawg 2022.03 2021.06 icgc.bionimbus.org 1647626526 uc-cdis/cdis-manifest#4336

Merged

This was referenced Mar 29, 2022

Gen3 Monthly Release 2022.04 tbi.datacommons.io 1648529943 uc-cdis/cdis-manifest#4376

Closed

Gen3 Monthly Release 2022.04 gen3-neuro.datacommons.io 1648533542 uc-cdis/cdis-manifest#4377

Closed

This was referenced Apr 30, 2022

Gen3 Monthly Release 2022.05 tbi.datacommons.io 1651309735 uc-cdis/cdis-manifest#4576

Closed

Gen3 Monthly Release 2022.05 gen3-neuro.datacommons.io 1651313335 uc-cdis/cdis-manifest#4577

Closed

This was referenced Jun 23, 2022

Gen3 Monthly Release 2022.06 tbi.datacommons.io 1655955061 uc-cdis/cdis-manifest#4792

Closed

Gen3 Monthly Release 2022.06 gen3-neuro.datacommons.io 1655958659 uc-cdis/cdis-manifest#4793

Closed

PlanXCyborg mentioned this pull request Sep 14, 2022

chore/UpdateBloodpac: update servaces to match production versions uc-cdis/gitops-qa#2187

Merged

PlanXCyborg mentioned this pull request Oct 21, 2022

Gen3 Monthly Release 2022.10 dcf-interop.kidsfirstdrc.org 1666320713 uc-cdis/cdis-manifest#5281

Closed

github-actions bot mentioned this pull request Aug 15, 2024

Updated fence to expose metrics uc-cdis/cdis-manifest#7742

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PXP-6617 Remove scopes from aud claim in tokens #839

PXP-6617 Remove scopes from aud claim in tokens #839

vpsx commented Sep 23, 2020 •

edited

Loading

github-actions bot commented Sep 23, 2020 •

edited

Loading

coveralls commented Sep 23, 2020 •

edited

Loading

lgtm-com bot commented Sep 23, 2020

lgtm-com bot commented Oct 9, 2020

PXP-6617 Remove scopes from aud claim in tokens #839

PXP-6617 Remove scopes from aud claim in tokens #839

Conversation

vpsx commented Sep 23, 2020 • edited Loading

New Features

Breaking Changes

Bug Fixes

Improvements

Dependency updates

Deployment changes

github-actions bot commented Sep 23, 2020 • edited Loading

coveralls commented Sep 23, 2020 • edited Loading

Pull Request Test Coverage Report for Build 11137

💛 - Coveralls

lgtm-com bot commented Sep 23, 2020

lgtm-com bot commented Oct 9, 2020

vpsx commented Sep 23, 2020 •

edited

Loading

github-actions bot commented Sep 23, 2020 •

edited

Loading

coveralls commented Sep 23, 2020 •

edited

Loading