Skip to content

[auth] Fix update to user's last active time#15318

Open
kush-chandra wants to merge 1 commit intohail-is:mainfrom
kush-chandra:kchandra-user-reactivate
Open

[auth] Fix update to user's last active time#15318
kush-chandra wants to merge 1 commit intohail-is:mainfrom
kush-chandra:kchandra-user-reactivate

Conversation

@kush-chandra
Copy link
Contributor

Change Description

Currently, we only update a user's last active time if they have an existing active session. However, this excludes some scenarios (ex. calls through the CLI) where the user might not have created a session with a given auth token. This change makes it so when authorizing a call, we will always update the last active time as long as the user is authorized & currently active.

Security Assessment

  • This change potentially impacts the Hail Batch instance as deployed by Broad Institute in GCP

Impact Rating

  • This change has a low security impact

Impact Description

Refactoring where the update to a user's last active time happens. This adds one database update on some authorized calls which didn't have it before.

Appsec Review

  • Required: The impact has been assessed and approved by appsec

@kush-chandra kush-chandra requested a review from a team as a code owner March 3, 2026 18:52
@kush-chandra kush-chandra requested a review from grohli March 5, 2026 20:46
Copy link
Contributor

@grohli grohli left a comment

Choose a reason for hiding this comment

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

Looks good! One minor question/comment but other than that it should be good to go.

userdata = await get_userinfo_from_login_id_or_hail_identity_id(request, uid)

if userdata:
if userdata['state'] == 'active':
Copy link
Contributor

Choose a reason for hiding this comment

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

If we hit this then I assume we got some non-None thing back from either get_userinfo_from_login_id_or_hail_identity_id() or get_userinfo_from_hail_session_id()? It looks like both of those functions already check/query for users.state = 'active', so it may be alright to drop this check.

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.

2 participants