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-8992): Expire user policies and update Arborist username #986

Merged
merged 12 commits into from
Dec 1, 2021

Conversation

johnfrancismccann
Copy link
Contributor

Jira Ticket: PXP-8992

New Features

  • Set expiration for created user policies when a passport is POSTed to DRS endpoint
  • Change username in Arborist when a user POSTs to DRS endpoint and then logs in

Dependency updates

  • Update gen3authz to 1.4.0

@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 Nov 29, 2021

Pull Request Test Coverage Report for Build 12056

  • 19 of 21 (90.48%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.03%) to 72.433%

Changes Missing Coverage Covered Lines Changed/Added Lines %
fence/resources/openid/ras_oauth2.py 16 18 88.89%
Totals Coverage Status
Change from base Build 12046: 0.03%
Covered Lines: 6653
Relevant Lines: 9185

💛 - Coveralls

@@ -1664,6 +1666,9 @@ def _update_authz_in_arborist(
policy_id_list = []
policies = []

if expires is not None:
expires = datetime.datetime.utcfromtimestamp(expires)
Copy link
Contributor

Choose a reason for hiding this comment

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

@@ -206,7 +206,11 @@ def map_iss_sub_pair_to_user(self, issuer, subject_id, username, email):
"from the DRS endpoint. Changing said user's username"
f' to "{username}".'
)
# TODO also change username in Arborist
flask.current_app.arborist.update_user(
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to handle the error here? or, I see you raise exception in this function. maybe there's not much we can actually do if this fails... but we should at least catch the exception and log useful information (and not implicitly rely on the library to do that). If it's relatively simple, we could try some simple backoff to retry one more time maybe?

@Avantol13 Avantol13 merged commit 699a483 into feat/passport Dec 1, 2021
@Avantol13 Avantol13 deleted the chore/expire-arborist-policies branch December 1, 2021 17:32
@Avantol13 Avantol13 mentioned this pull request Dec 1, 2021
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants