feat: audit login #5925
feat: audit login #5925
Conversation
coderd/users.go
Outdated
| var loginWithPassword codersdk.LoginWithPasswordRequest | ||
| if !httpapi.Read(ctx, rw, r, &loginWithPassword) { | ||
| // We pass a disposable user ID just to force an audit diff | ||
| // and generate a log for a failed login | ||
| aReq.New = database.APIKey{UserID: uuid.New()} |
There was a problem hiding this comment.
Down for another approach here. The auditor will skip audit log generation if there's not a valid diff. We don't have a proper UUID on APIKey and anyways, we bail before we actually create the model. I figured generating a garbage UUID was the fastest path forward but admittedly it's kinda gross.
coderd/users.go
Outdated
| @@ -995,9 +995,25 @@ func (api *API) organizationByUserAndName(rw http.ResponseWriter, r *http.Reques | |||
| // @Success 201 {object} codersdk.LoginWithPasswordResponse | |||
| // @Router /users/login [post] | |||
| func (api *API) postLogin(rw http.ResponseWriter, r *http.Request) { | |||
| ctx := r.Context() | |||
| var ( | |||
There was a problem hiding this comment.
Does this handler handle all authentication paths? What if a user isn't logging with a password?
There was a problem hiding this comment.
There's a file called userauth.go with all of the other ways to login. This endpoint should probably be in there anyways.
There was a problem hiding this comment.
Alright, added auditing for OAuth and OIDC. I am not sure how best to smoke-test locally, but I made sure to update all the auth tests.
| return xerrors.Errorf("find linked user: %w", err) | ||
| } | ||
| user = params.User | ||
| link = params.Link |
There was a problem hiding this comment.
I thought it was safe to lift finding the user outside of the transaction and into the callers so we can access User.ID in those places. If, in the callers, we don't find a user, we bail early, never entering oauthLogin.
Co-authored-by: Steven Masley <Emyrk@users.noreply.github.com>
Part of #4538
Logout coming next