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

PXP-6617 Add custom scopes validation and revert aud validation to default #47

Merged
merged 27 commits into from
Apr 29, 2021
Merged

PXP-6617 Add custom scopes validation and revert aud validation to default #47

merged 27 commits into from
Apr 29, 2021

Conversation

vpsx
Copy link
Contributor

@vpsx vpsx commented Sep 23, 2020 •
edited
Loading

Add custom scopes validation and revert aud validation to default; services (eg Fence) will stop using aud claim for scopes. Matching Fence PR here uc-cdis/fence#839

Greater detail in the commit messages.

Breaking Changes

  • Add custom scopes validation and revert aud validation to default; services (eg Fence) will stop using the aud claim for scopes. Changes are in token.validate_jwt and core.validate_jwt as well as the require_auth_header decorator.

Improvements

  • Update some docstrings
  • Allow passthrough of 'options' arg to pyjwt (via token.validate_jwt and core.validate_jwt)

Dependency updates

Deployment changes

@github-actions
Copy link

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

The style in this PR agrees with black. ✔️

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

@vpsx vpsx mentioned this pull request Sep 23, 2020
Merged
vpsx added 21 commits February 9, 2021 11:31
@vpsx
feat(scope): Add class JWTScopeError(JWTError)
c723d4b
@vpsx
fix(aud): Validate aud claim the normal way, validate custom scopes c…
d2a4a77 
…laim

* import new JWTScopeError
* add new scope arg to core.validate_jwt
* add new scope validation to core.validate_jwt
* remove custom aud validation from core.validate_jwt
* remove random_aud hack in core.validate_jwt; type(aud) now string-or-None, not set-or-list
* pass aud through to PyJWT for normal validation
* update docstring for core.validate_jwt
* allow empty aud arg in validate.validate_jwt; cease raising ValueError
* add new optional scope arg in validate.validate_jwt; pass through to core.validate_jwt
* update docstring for validate.validate_jwt
@vpsx
fix(aud): allow passthrough of options arg to pyjwt
397d062
@vpsx
fix(aud-scope): switch require_auth_header to checking scopes not aud
66dd781
@vpsx
fix(aud-scope): Skip aud validation in require_auth_header/validate_r…
93cfe40 
…equest
@vpsx
test(aud-scope): Change default_audiences fixture to default_scopes; …
29f8420 
…rm aud from generic claims
@vpsx
fix(aud-scope): chg aud to scope in FastAPI access_token dependency
ee69572
@vpsx
test(aud-scope): Upd tests to reflect new aud/scope usage
950a1aa
@vpsx
fix(aud-scope): chg aud to scope in CurrentUser call to validate_request
e404a59
@vpsx
test(aud): add happy-path test for aud validation
c64b190
@vpsx
style(black): Blacken, and update black rev in precommit config
99b1531
@vpsx
test(aud): Explicitly pass None instead of default_audiences
13d8a3b 
* because default_audience may change to not None in future
* and because this better reflects the intention of the test
@vpsx
fix(aud): Re-enable aud claim validation in require_auth_header
e25302d
@vpsx
fix(aud): Expect iss in aud claim by default in token.validate_jwt...
40ef1a4 
* ...if a value for iss is avbl, from app cfg BASE_URL or USER_API.
* Also clarify core.validate_jwt docstring.
@vpsx
test(app-fixture): Set app.config['BASE_URL'] as well as ['USER_API']
71e5bba
@vpsx
fix(aud): Allow passing expected audience to FastAPI access_token dep…
25a3162 
…endency
@vpsx
test(aud): Include aud claim in default claims test fixture
57f9bd0 
* Update default_audience fixture accordingly
* Update tests to account for new default claims
@vpsx
fix(aud): Allow passing expected audience to require_auth_header and …
38326bf 
…validate_request

* Also let scope={} by default
* Update calls to require_auth_header
@vpsx
fix(aud): Update set_current_user proxy fn to pass in expected aud
0178eb4 
* based on flask.current_app.config
* Since this already assumes Flask request ctx, I think OK to look in Flask app cfg in this case
@vpsx
test(aud): Add test: no aud arg provided and no aud claim in token
e0be0f1
@vpsx
test(aud): Rename fixture default_audiences to default_audience
ba5ffbf
paulineribeyre
paulineribeyre reviewed Mar 18, 2021
View reviewed changes
.pre-commit-config.yaml Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
src/authutils/token/fastapi.py Show resolved Hide resolved
src/authutils/token/core.py Outdated Show resolved Hide resolved
src/authutils/token/validate.py Outdated Show resolved Hide resolved
src/authutils/user.py Outdated Show resolved Hide resolved
vpsx and others added 2 commits April 23, 2021 09:23
@vpsx @paulineribeyre
fix(aud-scope): error message
5272cfc 
Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>
@vpsx @paulineribeyre
docs(aud-scope): Fix docstring
229f586 
Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>
@vpsx vpsx merged commit 2b7538e into master Apr 29, 2021
@vpsx vpsx deleted the fix/aud branch April 29, 2021 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@paulineribeyre paulineribeyre paulineribeyre approved these changes

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
@vpsx @paulineribeyre