Add GitHub/GitLab logins (fail/success) to security events#43886
Conversation
|
Codenotify: Notifying subscribers in CODENOTIFY files for diff 79ed283...fe7899b.
|
| http.Error(w, safeErrMsg, http.StatusInternalServerError) | ||
| db.SecurityEventLogs().LogEvent(ctx, &database.SecurityEvent{ | ||
| Name: s.AuthFailedEventName(), | ||
| URL: r.URL.Path, |
There was a problem hiding this comment.
Note that we don't want the entire URL as it contains sensitive OAuth query parameters
There was a problem hiding this comment.
Could you put the above as a code comment, I can imagine someone unaware of this making a PR to "fix missing full URL" and approved by someone who is also unware of this 🤣
unknwon
left a comment
There was a problem hiding this comment.
Approve to unblock 👍
(Please also remove the period (.) in the PR title, as it would probably become the commit title 😄 )
| http.Error(w, safeErrMsg, http.StatusInternalServerError) | ||
| db.SecurityEventLogs().LogEvent(ctx, &database.SecurityEvent{ | ||
| Name: s.AuthFailedEventName(), | ||
| URL: r.URL.Path, |
There was a problem hiding this comment.
Could you put the above as a code comment, I can imagine someone unaware of this making a PR to "fix missing full URL" and approved by someone who is also unware of this 🤣
| Name: s.AuthFailedEventName(), | ||
| URL: r.URL.Path, | ||
| AnonymousUserID: anonymousId, | ||
| Source: "BACKEND", |
There was a problem hiding this comment.
Have lost context on how we define our source, but this one seems to be "WEB"?
There was a problem hiding this comment.
I actually tried to dig it up, and it all seems just "BACKEND". We only use "WEB" in the tests. I agree that "WEB" actually sounds reasonable, though 🤔 I'll leave it be but if you have any additional context I'm happy to follow up with additional PR.
Co-authored-by: Joe Chen <joe@sourcegraph.com>
Co-authored-by: Joe Chen <joe@sourcegraph.com>
Description
Add GitHub and GitLab logins to the security events (so that they automatically appear in the audit log as well).
The following events are added:
GithubAuthSucceededGithubAuthFailedGitlabAuthSucceededGitlabAuthFailedI decided not to add
GitHubAuthAttempted/GitLabAuthAttemptedbecause these can’t be tied to a specific actor, and we don’t want to log the only bit of identity (the access token) to the logs.Test plan
GitHub successful login:
GitHub failed login
GitLab works, too
