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

[logging] Log identity as part of API call logging #14793

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

cjllanwarne
Copy link
Collaborator

Change Description

  • Adds the username dict to requests during auth checks
  • Adds logging of that username dict (if available) during call logging

Security Assessment

  • This change has a medium security impact

Impact Description

  • Makes usernames explicitly visible in the system logs, so admins are now able to track what individual users are doing or looking at.
  • The userdata in request objects should be handled carefully, though it does not include anything secret (ie nothing which would allow impersonation if leaked)

(Reviewers: please confirm the security impact before approving)

Copy link
Collaborator

@patrick-schultz patrick-schultz 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, just a minor code style suggestion.

Comment on lines 67 to 75
if 'username' in userdata_maybe:
extra['username'] = userdata_maybe['username']
if 'login_id' in userdata_maybe:
extra['login_id'] = userdata_maybe['login_id']
if 'is_developer' in userdata_maybe:
extra['is_developer'] = userdata_maybe['is_developer']
if 'hail_identity' in userdata_maybe:
extra['hail_identity'] = userdata_maybe['hail_identity']

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this can be simplified to something more pythonic like (untested)

id_keys = ['username', 'login_id', 'is_developer', 'hail_identity']
extra.update((k, userdata_maybe[k]) for k in id_keys if k in userdata_maybe)

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