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
Append OIDC logins (success/failure) to security events. #44467
Conversation
Codenotify: Notifying subscribers in CODENOTIFY files for diff 6334795...fbdb723.
|
enterprise/cmd/frontend/internal/auth/openidconnect/middleware.go
Outdated
Show resolved
Hide resolved
db.SecurityEventLogs().LogEvent(r.Context(), &database.SecurityEvent{ | ||
Name: database.SecurityEventOIDCLoginFailed, | ||
URL: r.URL.Path, // don't log OIDC query params | ||
AnonymousUserID: fmt.Sprintf("unknown OIDC @ %s", time.Now()), // we don't know have a reliable user identify at the time of the failure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AnonymousUserID: fmt.Sprintf("unknown OIDC @ %s", time.Now()), // we don't know have a reliable user identify at the time of the failure | |
AnonymousUserID: fmt.Sprintf("unknown OIDC @ %s", time.Now()), // We don't have a reliable user identity at the time of the failure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is time.Now() necessary as we also have a Timestamp
field?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think no, but I wanted to make that unique (more or less); we'd normally use a cookie value for the AnonymousUserID
, but we haven't got anything in this case.
enterprise/cmd/frontend/internal/auth/openidconnect/middleware.go
Outdated
Show resolved
Hide resolved
Co-authored-by: Will Dollman <will.dollman@sourcegraph.com>
Co-authored-by: Will Dollman <will.dollman@sourcegraph.com>
Description
Expand security events with a new
SecurityEventOIDCLoginSucceeded
andSecurityEventOIDCLoginFailed
pair and hook it up to the correct place.Test plan