Skip to content

Commit e3a6587

Browse files
Kira-PilotKira-Pilot
committed
audit Github oauth and write tests
1 parent e5dbe17 commit e3a6587

File tree

3 files changed

+161
-14
lines changed

3 files changed

+161
-14
lines changed

‎coderd/userauth.go

Lines changed: 69 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"golang.org/x/oauth2"
1818
"golang.org/x/xerrors"
1919

20+
"github.com/coder/coder/coderd/audit"
2021
"github.com/coder/coder/coderd/database"
2122
"github.com/coder/coder/coderd/httpapi"
2223
"github.com/coder/coder/coderd/httpmw"
@@ -66,9 +67,18 @@ func (api *API) userAuthMethods(rw http.ResponseWriter, r *http.Request) {
6667
// @Router /users/oauth2/github/callback [get]
6768
func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
6869
var (
69-
ctx = r.Context()
70-
state = httpmw.OAuth2(r)
70+
ctx = r.Context()
71+
state = httpmw.OAuth2(r)
72+
auditor = api.Auditor.Load()
73+
aReq, commitAudit = audit.InitRequest[database.APIKey](rw, &audit.RequestParams{
74+
Audit: *auditor,
75+
Log: api.Logger,
76+
Request: r,
77+
Action: database.AuditActionLogin,
78+
})
7179
)
80+
aReq.Old = database.APIKey{}
81+
defer commitAudit()
7282

7383
oauthClient := oauth2.NewClient(ctx, oauth2.StaticTokenSource(state.Token))
7484

@@ -81,6 +91,9 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
8191
Message: "Internal error fetching authenticated Github user organizations.",
8292
Detail: err.Error(),
8393
})
94+
// We pass a disposable user ID just to force an audit diff
95+
// and generate a log for a failed login
96+
aReq.New = database.APIKey{UserID: uuid.New()}
8497
return
8598
}
8699

@@ -101,6 +114,9 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
101114
httpapi.Write(ctx, rw, http.StatusUnauthorized, codersdk.Response{
102115
Message: "You aren't a member of the authorized Github organizations!",
103116
})
117+
// We pass a disposable user ID just to force an audit diff
118+
// and generate a log for a failed login
119+
aReq.New = database.APIKey{UserID: uuid.New()}
104120
return
105121
}
106122
}
@@ -111,6 +127,9 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
111127
Message: "Internal error fetching authenticated Github user.",
112128
Detail: err.Error(),
113129
})
130+
// We pass a disposable user ID just to force an audit diff
131+
// and generate a log for a failed login
132+
aReq.New = database.APIKey{UserID: uuid.New()}
114133
return
115134
}
116135

@@ -139,6 +158,9 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
139158
httpapi.Write(ctx, rw, http.StatusUnauthorized, codersdk.Response{
140159
Message: fmt.Sprintf("You aren't a member of an authorized team in the %v Github organization(s)!", organizationNames),
141160
})
161+
// We pass a disposable user ID just to force an audit diff
162+
// and generate a log for a failed login
163+
aReq.New = database.APIKey{UserID: uuid.New()}
142164
return
143165
}
144166
}
@@ -149,6 +171,9 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
149171
Message: "Internal error fetching personal Github user.",
150172
Detail: err.Error(),
151173
})
174+
// We pass a disposable user ID just to force an audit diff
175+
// and generate a log for a failed login
176+
aReq.New = database.APIKey{UserID: uuid.New()}
152177
return
153178
}
154179

@@ -164,10 +189,28 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
164189
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
165190
Message: "Your primary email must be verified on GitHub!",
166191
})
192+
// We pass a disposable user ID just to force an audit diff
193+
// and generate a log for a failed login
194+
aReq.New = database.APIKey{UserID: uuid.New()}
167195
return
168196
}
169197

170-
cookie, err := api.oauthLogin(r, oauthLoginParams{
198+
user, link, err := findLinkedUser(ctx, api.Database, githubLinkedID(ghUser), verifiedEmail.GetEmail())
199+
if err != nil {
200+
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
201+
Message: "Failed to find linked user.",
202+
Detail: err.Error(),
203+
})
204+
// We pass a disposable user ID just to force an audit diff
205+
// and generate a log for a failed login
206+
aReq.New = database.APIKey{UserID: uuid.New()}
207+
return
208+
}
209+
aReq.UserID = user.ID
210+
211+
cookie, key, err := api.oauthLogin(r, oauthLoginParams{
212+
User: user,
213+
Link: link,
171214
State: state,
172215
LinkedID: githubLinkedID(ghUser),
173216
LoginType: database.LoginTypeGithub,
@@ -182,16 +225,24 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
182225
Message: httpErr.msg,
183226
Detail: httpErr.detail,
184227
})
228+
// We pass a disposable user ID just to force an audit diff
229+
// and generate a log for a failed login
230+
aReq.New = database.APIKey{UserID: uuid.New()}
185231
return
186232
}
187233
if err != nil {
188234
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
189235
Message: "Failed to process OAuth login.",
190236
Detail: err.Error(),
191237
})
238+
// We pass a disposable user ID just to force an audit diff
239+
// and generate a log for a failed login
240+
aReq.New = database.APIKey{UserID: uuid.New()}
192241
return
193242
}
194243

244+
aReq.New = key
245+
195246
http.SetCookie(rw, cookie)
196247

197248
redirect := state.Redirect
@@ -362,7 +413,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
362413
picture, _ = pictureRaw.(string)
363414
}
364415

365-
cookie, err := api.oauthLogin(r, oauthLoginParams{
416+
cookie, _, err := api.oauthLogin(r, oauthLoginParams{
366417
State: state,
367418
LinkedID: oidcLinkedID(idToken),
368419
LoginType: database.LoginTypeOIDC,
@@ -397,6 +448,8 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
397448
}
398449

399450
type oauthLoginParams struct {
451+
User database.User
452+
Link database.UserLink
400453
State httpmw.OAuth2State
401454
LinkedID string
402455
LoginType database.LoginType
@@ -423,7 +476,7 @@ func (e httpError) Error() string {
423476
return e.msg
424477
}
425478

426-
func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cookie, error) {
479+
func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cookie, database.APIKey, error) {
427480
var (
428481
ctx = r.Context()
429482
user database.User
@@ -435,10 +488,13 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook
435488
err error
436489
)
437490

438-
user, link, err = findLinkedUser(ctx, tx, params.LinkedID, params.Email)
439-
if err != nil {
440-
return xerrors.Errorf("find linked user: %w", err)
441-
}
491+
user = params.User
492+
link = params.Link
493+
//
494+
// user, link, err = findLinkedUser(ctx, tx, params.LinkedID, params.Email)
495+
// if err != nil {
496+
// return xerrors.Errorf("find linked user: %w", err)
497+
// }
442498

443499
if user.ID == uuid.Nil && !params.AllowSignups {
444500
return httpError{
@@ -599,19 +655,19 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook
599655
return nil
600656
}, nil)
601657
if err != nil {
602-
return nil, xerrors.Errorf("in tx: %w", err)
658+
return nil, database.APIKey{}, xerrors.Errorf("in tx: %w", err)
603659
}
604660

605-
cookie, _, err := api.createAPIKey(ctx, createAPIKeyParams{
661+
cookie, key, err := api.createAPIKey(ctx, createAPIKeyParams{
606662
UserID: user.ID,
607663
LoginType: params.LoginType,
608664
RemoteAddr: r.RemoteAddr,
609665
})
610666
if err != nil {
611-
return nil, xerrors.Errorf("create API key: %w", err)
667+
return nil, database.APIKey{}, xerrors.Errorf("create API key: %w", err)
612668
}
613669

614-
return cookie, nil
670+
return cookie, *key, nil
615671
}
616672

617673
// githubLinkedID returns the unique ID for a GitHub user.

0 commit comments

Comments
 (0)