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

Feat/refresh expires in #848

Merged
merged 25 commits into from
Feb 24, 2021
Merged

Feat/refresh expires in #848

merged 25 commits into from
Feb 24, 2021

Conversation

mattgarvin1
Copy link
Contributor

@mattgarvin1 mattgarvin1 commented Nov 17, 2020

Ticket: https://ctds-planx.atlassian.net/browse/PXP-6563

Allow user/client to specify a lifetime of a refresh token - specified in seconds as param expires_in at the /authorize endpoint - must be a positive integer, and any request greater than the config default value (30 days) results in a refresh_token lifetime of the default 30 days.

Primarily this feature is for testing purposes, for interop, for developers of other systems and apps who want to test what happens in the case of an expired refresh token.

New Features

  • Implemented support for specifying lifetime of refresh token at the /authorize endpoint via param expires_in

Deployment changes

  • REQUIRES A FENCE DB MIGRATION, since with this update there is now an additional column in the authorization_code table "refresh_token_expires_in"

@github-actions
Copy link

github-actions bot commented Nov 17, 2020

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 Nov 17, 2020

Pull Request Test Coverage Report for Build 10445

  • 34 of 39 (87.18%) changed or added relevant lines in 9 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 69.418%

Changes Missing Coverage Covered Lines Changed/Added Lines %
fence/blueprints/storage_creds/google.py 0 2 0.0%
fence/scripting/fence_create.py 1 4 25.0%
Totals Coverage Status
Change from base Build 10440: 0.1%
Covered Lines: 5702
Relevant Lines: 8214

💛 - Coveralls

Copy link
Contributor

@vpsx vpsx left a comment

Choose a reason for hiding this comment

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

The db migration and the new flow work swimmingly in my local tests 🎉 I have left only nitpicks made with the long term in mind--in both cases, do feel free to disagree and dismiss 👯

fence/oidc/grants/oidc_code_grant.py Outdated Show resolved Hide resolved
fence/oidc/grants/oidc_code_grant.py Outdated Show resolved Hide resolved
@vpsx vpsx merged commit 9417655 into master Feb 24, 2021
@vpsx vpsx deleted the feat/refresh-expires-in branch February 24, 2021 20:11
rolinge added a commit to rolinge/fence that referenced this pull request Mar 10, 2021
* annotate spot for fix

* try overwrite roles, if fail, then create

* test exception handling

* catch arborist error

* log info failed put role

* test gen3authz patch

* fix git requirement line

* debug requirements load from git

* specify test gen3authz by commit

* test no git

* debug gen3authz tag

* clear cache in quay

* debug import test gen3authz lib

* test

* test

* revert dockerfile and requirements to master

* hack test poetry install gen3authz

* debug poetry install

* debug test dockerfile

* clear quay cache

* comment out gen3authz from setup.py

* tell poetry we don't need a virtual env thanks

* add gen3authz name to setup.py package list

* bump cdiserrors version

* add cdiserrors to setup.py

* bump gen3config

* fix ArboristError import path

* await get users from arborist

* don't await; catch attribute error

* h4ck dockerfile

* revert dockerfile

* use new release of gen3authz lib

* update httplib2

* remove comment

* fix conftest mock arborist patch method

* fix conftest request import path

* async magic mock arborist client

* add asynctest to dev-requirements

* revert testing stuff..

* fix tests - replace gen3authz requests.request with httpx.request

* revert

* fix requests import path in tests

* revert

* update dep versions

* bump authutils..

* revert deps..

* update dep versions

* bump authutils

* delete old poetry lock file

* test import cdispyutils branch

* add test lock file

* update tests

* update tests

* update tests

* debug tests

* revert

* reset

* Parse RAS visas

* chore(fence): New parent image pybase3-1.4.2 (uc-cdis#853)

* parse all users

* feat(userinfo): include identity provider name in userinfo response for clients

* feat(userinfo): safely graph idp name even if relationship doesn't exist

* docs(openapi): add idp to userinfo response docs

* visa update cron job

* initialize

* notes

* fix(dependencies): Fix poetry lock command (uc-cdis#858)

* fix(dependencies): Fix poetry lock

* new lock file

* Update pyproject.toml

Cool

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* draft flow

* Dependency Fix New

* chore(builds): Speed up poetry commands by setting proper botocore version (uc-cdis#859)

* fix(dependencies): Fix poetry lock

* new lock file

* chore(build): Speed up docker build nby setting proper botocore version

* add new lock file

* removing line from the other PR

* more notes

* gen3authz use branch

* pkg

* async

* new fix

* new authz

* print

* stuff

* print

* new endpoint

* async

* prints

* updated

* remove prints

* update

* authutils

* testing

* testing

* update dependencies

* poetry update

* account for empty url list

* add window method

* updated as designed

* chore(google-manage-keys): Handle key deletion attempt that returns 400 FAILED_PRECONDITION (uc-cdis#868)

* chore(google-manage-keys): Handle key deletion with FAILED_PRECONDITION

* somem logging

* fence-create+logging

* fix asyncio

* fix asyncio for python<3.6

* db session fixes

* small fixes

* fixes

* try

* remove grant type

* remove try

* add not

* return for when no urls are present

* add variables

* fix

* check update

* int

* fix

* change small things

* add single visa processing

* loop through all urls

* rerun

* lock update

* add unit test

* fix parse_consent_code

* remove test

* finishing up + fence create update

* fix user yaml

* fix get

* add test cases

* comments

* feat(cookies): always set httponly flag on cookies (also update deps)

* add test case reformat

* remove redundent code

* adding fallback logic

* fix things

* add help

* feat(cookies): add httponly flag to missing spots

* test and such

* lock update

* fix things

* cahnge config

* lgos + black

* remove invalid visas

* db_session

* smoll fix

* add logs

* add logs

* #):
fix(no_force_sign): add regression test for urls with no_force_sign=False

* fix(no_force_sign): parse truthiness from no_force_sign param, rather than check if exists

* ras login

* fixes

* small fixes

* stop revoke

* revoke

* black

* small change

* sess

* init sync

* blargh

* add session

* revert

* add to config

* black + spaces

* fix tests

* add to config

* Update bin/fence_create.py

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* Update fence/scripting/fence_create.py

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* Update fence/sync/sync_users.py

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* Update fence/sync/sync_users.py

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* Update fence/job/visa_update_cronjob.py

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* review changes

* black

* resolve again

* remove comments

* black

* small fix

* login

* revoke control

* close session

* refactor

* PXP-7696 Fix/renew access token (uc-cdis#874)

* ix: always mint new token

* fix: wrong logic

* fix: add config

* run the hook

* set default to false

* resolve comments

* Update tests/dbgap_sync/test_user_sync.py

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* Update tests/dbgap_sync/test_user_sync.py

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* fix(empty idp): always commit user/idp to db

* Update fence/sync/sync_users.py

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* update black

* fix typo

* Feat/refresh expires in (uc-cdis#848)

* store expires_in param from authz request in flask session

* take expires_in value from session if present for generating refresh tokens

* fix key check

* debug

* revert

* add exp col to auth code db table model

* extract expires_in param from authorize request and put in db record

* add refresh_token_expires_in logic to token response

* add code for db migration authorization_code table add col

* remove comment

* reformat

* handle pre-db migration case

* undo last change

* handle pre-db-migration case

* fix init auth code record

* undo last change

* revert

* try reflecting db to enable backwards compatibility with previous db schema

* add ref to relevant doc

* fix db reflection

* add scope handling

* reflect auth code table at token endpoint

* revert

* Accommodate variously named query params for expiration times

* expiry does not always refer to access token

* Refactor get-and-validate-requested-expiration pattern into util fn

* Clearer intentions, better names, less repetition, less cognitive load, etc

Co-authored-by: vpsx <19900057+vpsx@users.noreply.github.com>

* test(login_user): ensure logged in users have idp

* docs(login_user): add a docstring

* format black

* fix(login_user): dont log in if already configured

* chore(login_user): remove unused flask.request arg

* chore(login_user): trigger build

* fix(oauth): allow fence oauth w/o LOGIN_OPTIONS

* docs(Accessing Data): Fixed broken link to #create-user-access-file (uc-cdis#881)

Co-authored-by: mattgarvin1 <mattgarvin1@gmail.com>
Co-authored-by: Alexander VanTol <Avantol13@users.noreply.github.com>
Co-authored-by: BinamB <numb.baj@gmail.com>
Co-authored-by: Marcelo R Costa <marceloc@uchicago.edu>
Co-authored-by: Alexander VT <alexander.m.vantol@gmail.com>
Co-authored-by: Binam Bajracharya <44302895+BinamB@users.noreply.github.com>
Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>
Co-authored-by: mattreex <mattreex.9@gail.com>
Co-authored-by: Mattreex <45504048+mattreex@users.noreply.github.com>
Co-authored-by: Matthew Cannalte <mcannalte@uchicago.edu>
Co-authored-by: Mingfei Shao <2475897+mfshao@users.noreply.github.com>
Co-authored-by: John McCann <johnmccann@uchicago.edu>
Co-authored-by: vpsx <19900057+vpsx@users.noreply.github.com>
rolinge added a commit to rolinge/fence that referenced this pull request Mar 10, 2021
* annotate spot for fix

* try overwrite roles, if fail, then create

* test exception handling

* catch arborist error

* log info failed put role

* test gen3authz patch

* fix git requirement line

* debug requirements load from git

* specify test gen3authz by commit

* test no git

* debug gen3authz tag

* clear cache in quay

* debug import test gen3authz lib

* test

* test

* revert dockerfile and requirements to master

* hack test poetry install gen3authz

* debug poetry install

* debug test dockerfile

* clear quay cache

* comment out gen3authz from setup.py

* tell poetry we don't need a virtual env thanks

* add gen3authz name to setup.py package list

* bump cdiserrors version

* add cdiserrors to setup.py

* bump gen3config

* fix ArboristError import path

* await get users from arborist

* don't await; catch attribute error

* h4ck dockerfile

* revert dockerfile

* use new release of gen3authz lib

* update httplib2

* remove comment

* fix conftest mock arborist patch method

* fix conftest request import path

* async magic mock arborist client

* add asynctest to dev-requirements

* revert testing stuff..

* fix tests - replace gen3authz requests.request with httpx.request

* revert

* fix requests import path in tests

* revert

* update dep versions

* bump authutils..

* revert deps..

* update dep versions

* bump authutils

* delete old poetry lock file

* test import cdispyutils branch

* add test lock file

* update tests

* update tests

* update tests

* debug tests

* revert

* reset

* Parse RAS visas

* chore(fence): New parent image pybase3-1.4.2 (uc-cdis#853)

* parse all users

* feat(userinfo): include identity provider name in userinfo response for clients

* feat(userinfo): safely graph idp name even if relationship doesn't exist

* docs(openapi): add idp to userinfo response docs

* visa update cron job

* initialize

* notes

* fix(dependencies): Fix poetry lock command (uc-cdis#858)

* fix(dependencies): Fix poetry lock

* new lock file

* Update pyproject.toml

Cool

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* draft flow

* Dependency Fix New

* chore(builds): Speed up poetry commands by setting proper botocore version (uc-cdis#859)

* fix(dependencies): Fix poetry lock

* new lock file

* chore(build): Speed up docker build nby setting proper botocore version

* add new lock file

* removing line from the other PR

* more notes

* gen3authz use branch

* pkg

* async

* new fix

* new authz

* print

* stuff

* print

* new endpoint

* async

* prints

* updated

* remove prints

* update

* authutils

* testing

* testing

* update dependencies

* poetry update

* account for empty url list

* add window method

* updated as designed

* chore(google-manage-keys): Handle key deletion attempt that returns 400 FAILED_PRECONDITION (uc-cdis#868)

* chore(google-manage-keys): Handle key deletion with FAILED_PRECONDITION

* somem logging

* fence-create+logging

* fix asyncio

* fix asyncio for python<3.6

* db session fixes

* small fixes

* fixes

* try

* remove grant type

* remove try

* add not

* return for when no urls are present

* add variables

* fix

* check update

* int

* fix

* change small things

* add single visa processing

* loop through all urls

* rerun

* lock update

* add unit test

* fix parse_consent_code

* remove test

* finishing up + fence create update

* fix user yaml

* fix get

* add test cases

* comments

* feat(cookies): always set httponly flag on cookies (also update deps)

* add test case reformat

* remove redundent code

* adding fallback logic

* fix things

* add help

* feat(cookies): add httponly flag to missing spots

* test and such

* lock update

* fix things

* cahnge config

* lgos + black

* remove invalid visas

* db_session

* smoll fix

* add logs

* add logs

* #):
fix(no_force_sign): add regression test for urls with no_force_sign=False

* fix(no_force_sign): parse truthiness from no_force_sign param, rather than check if exists

* ras login

* fixes

* small fixes

* stop revoke

* revoke

* black

* small change

* sess

* init sync

* blargh

* add session

* revert

* add to config

* black + spaces

* fix tests

* add to config

* Update bin/fence_create.py

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* Update fence/scripting/fence_create.py

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* Update fence/sync/sync_users.py

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* Update fence/sync/sync_users.py

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* Update fence/job/visa_update_cronjob.py

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* review changes

* black

* resolve again

* remove comments

* black

* small fix

* login

* revoke control

* close session

* refactor

* PXP-7696 Fix/renew access token (uc-cdis#874)

* ix: always mint new token

* fix: wrong logic

* fix: add config

* run the hook

* set default to false

* resolve comments

* Update tests/dbgap_sync/test_user_sync.py

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* Update tests/dbgap_sync/test_user_sync.py

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* fix(empty idp): always commit user/idp to db

* Update fence/sync/sync_users.py

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* update black

* fix typo

* Feat/refresh expires in (uc-cdis#848)

* store expires_in param from authz request in flask session

* take expires_in value from session if present for generating refresh tokens

* fix key check

* debug

* revert

* add exp col to auth code db table model

* extract expires_in param from authorize request and put in db record

* add refresh_token_expires_in logic to token response

* add code for db migration authorization_code table add col

* remove comment

* reformat

* handle pre-db migration case

* undo last change

* handle pre-db-migration case

* fix init auth code record

* undo last change

* revert

* try reflecting db to enable backwards compatibility with previous db schema

* add ref to relevant doc

* fix db reflection

* add scope handling

* reflect auth code table at token endpoint

* revert

* Accommodate variously named query params for expiration times

* expiry does not always refer to access token

* Refactor get-and-validate-requested-expiration pattern into util fn

* Clearer intentions, better names, less repetition, less cognitive load, etc

Co-authored-by: vpsx <19900057+vpsx@users.noreply.github.com>

* test(login_user): ensure logged in users have idp

* docs(login_user): add a docstring

* format black

* fix(login_user): dont log in if already configured

* chore(login_user): remove unused flask.request arg

* chore(login_user): trigger build

* fix(oauth): allow fence oauth w/o LOGIN_OPTIONS

* docs(Accessing Data): Fixed broken link to #create-user-access-file (uc-cdis#881)

Co-authored-by: mattgarvin1 <mattgarvin1@gmail.com>
Co-authored-by: Alexander VanTol <Avantol13@users.noreply.github.com>
Co-authored-by: BinamB <numb.baj@gmail.com>
Co-authored-by: Marcelo R Costa <marceloc@uchicago.edu>
Co-authored-by: Alexander VT <alexander.m.vantol@gmail.com>
Co-authored-by: Binam Bajracharya <44302895+BinamB@users.noreply.github.com>
Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>
Co-authored-by: mattreex <mattreex.9@gail.com>
Co-authored-by: Mattreex <45504048+mattreex@users.noreply.github.com>
Co-authored-by: Matthew Cannalte <mcannalte@uchicago.edu>
Co-authored-by: Mingfei Shao <2475897+mfshao@users.noreply.github.com>
Co-authored-by: John McCann <johnmccann@uchicago.edu>
Co-authored-by: vpsx <19900057+vpsx@users.noreply.github.com>
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