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

Add extra information to the impersonation logs #743

Merged
merged 1 commit into from
Aug 12, 2024

Conversation

michelletran-codecov
Copy link
Contributor

@michelletran-codecov michelletran-codecov commented Aug 9, 2024

Purpose/Motivation

We want to improve impersonation logs for the security team.

Links to relevant tickets

codecov/engineering-team#1850

What does this PR do?

Adds extra email field for the staff user to the Panther impersonation logs.

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

@codecov-notifications
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

Copy link

codecov bot commented Aug 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.05%. Comparing base (c0110f8) to head (9c08519).

Changes have been made to critical files, which contain lines commonly executed in production. Learn more

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff             @@
##               main       #743   +/-   ##
===========================================
  Coverage   96.05000   96.05000           
===========================================
  Files           814        814           
  Lines         18469      18469           
===========================================
  Hits          17740      17740           
  Misses          729        729           
Flag Coverage Δ
unit 91.77% <100.00%> (ø)
unit-latest-uploader 91.77% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@michelletran-codecov michelletran-codecov force-pushed the 1850_add_more_impersonation_logs branch from 3516864 to 8063b0a Compare August 9, 2024 15:35
@michelletran-codecov michelletran-codecov requested a review from a team August 9, 2024 15:39
@jeff-a-holland-codecov
Copy link

Update: Please do the following:

  • Log ID, username, email for the person impersonating (e.g. codecov employee). If all you can do it the ID, that's fine.
  • Only log the ID for the customer being impersonated.

Thanks

@michelletran-codecov michelletran-codecov force-pushed the 1850_add_more_impersonation_logs branch from 8063b0a to 28981f6 Compare August 9, 2024 18:51
In particular, we want to add the email and username for the staff user
and the impersonated user. It is not easy to get the staff username,
so only logging the staff email for now.
@michelletran-codecov michelletran-codecov force-pushed the 1850_add_more_impersonation_logs branch from 28981f6 to 9c08519 Compare August 9, 2024 18:52
@michelletran-codecov
Copy link
Contributor Author

I've removed the impersonating user information and am only adding the staff's email to the logged fields. The username is a bit tricky to get because it's service dependent (and requires another query). @jeff-a-holland-codecov

@jeff-a-holland-codecov
Copy link

Thanks, @michelletran-codecov !

"""Log and ensure that the impersonating user is authenticated.
The `current user` is the staff user that is impersonating the
user owner at `impersonating_ownerid`.
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

ty for this docstr 💟

@michelletran-codecov michelletran-codecov added this pull request to the merge queue Aug 12, 2024
Merged via the queue into main with commit d515dc3 Aug 12, 2024
18 checks passed
@michelletran-codecov michelletran-codecov deleted the 1850_add_more_impersonation_logs branch August 12, 2024 18:35
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