Skip to content

Commit ddabcf7

Browse files
EmyrkEmyrk
committed
Extract oauth convert into a helper function
1 parent 9cfb69c commit ddabcf7

File tree

1 file changed

+102
-67
lines changed

1 file changed

+102
-67
lines changed

‎coderd/userauth.go

+102-67
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ import (
3232
"github.com/coder/coder/cryptorand"
3333
)
3434

35+
const mergeStateStringPrefix = "convert-"
36+
3537
// postConvertLoginType replies with an oauth state token capable of converting
3638
// the user to an oauth user.
3739
//
@@ -105,6 +107,9 @@ func (api *API) postConvertLoginType(rw http.ResponseWriter, r *http.Request) {
105107
})
106108
return
107109
}
110+
// The prefix is used to identify this state string as a conversion state
111+
// without needing to hit the database. The random string is the CSRF protection.
112+
stateString = fmt.Sprintf("%s%s", mergeStateStringPrefix, stateString)
108113

109114
now := time.Now()
110115
var mergeState database.OauthMergeState
@@ -1024,74 +1029,12 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook
10241029
}
10251030
}
10261031

1027-
// If this is not a new user and the login type is different,
1028-
// we need to check if the user is trying to change their login type.
1029-
if user.ID != uuid.Nil && user.LoginType != params.LoginType {
1030-
wrongLoginTypeErr := httpError{
1031-
code: http.StatusForbidden,
1032-
msg: fmt.Sprintf("Incorrect login type, attempting to use %q but user is of login type %q",
1033-
params.LoginType,
1034-
user.LoginType,
1035-
),
1036-
}
1037-
// If we do not allow converting to oauth, return an error.
1038-
if !params.OauthConversionEnabled {
1039-
return wrongLoginTypeErr
1040-
}
1041-
1042-
// At this point, this request could be an attempt to convert from
1043-
// password auth to oauth auth.
1044-
var (
1045-
auditor = *api.Auditor.Load()
1046-
oauthConvertAudit, commitOauthConvertAudit = params.InitAuditRequest(&audit.RequestParams{
1047-
Audit: auditor,
1048-
Log: api.Logger,
1049-
Request: r,
1050-
Action: database.AuditActionLogin,
1051-
})
1052-
)
1053-
defer commitOauthConvertAudit()
1054-
1055-
// nolint:gocritic // Required to auth the oidc convert
1056-
mergeState, err := tx.GetUserOauthMergeState(dbauthz.AsSystemRestricted(ctx), database.GetUserOauthMergeStateParams{
1057-
UserID: user.ID,
1058-
StateString: params.State.StateString,
1059-
})
1060-
if xerrors.Is(err, sql.ErrNoRows) {
1061-
return wrongLoginTypeErr
1062-
}
1063-
1064-
failedMsg := fmt.Sprintf("Request to convert login type from %s to %s failed", user.LoginType, params.LoginType)
1032+
// If you do a convert to OIDC and your email does not match, we need to
1033+
// catch this and not make a new account.
1034+
if isMergeStateString(params.State.StateString) {
1035+
err := api.convertUserToOauth(ctx, r, tx, params)
10651036
if err != nil {
1066-
return httpError{
1067-
code: http.StatusForbidden,
1068-
msg: failedMsg,
1069-
}
1070-
}
1071-
oauthConvertAudit.Old = mergeState
1072-
// Make sure the merge state generated matches this OIDC login request.
1073-
// It needs to have the correct login type information for this
1074-
// user.
1075-
if user.ID != mergeState.UserID || user.LoginType != mergeState.FromLoginType || params.LoginType != mergeState.ToLoginType {
1076-
return httpError{
1077-
code: http.StatusForbidden,
1078-
msg: failedMsg,
1079-
}
1080-
}
1081-
1082-
// Convert the user and default to the normal login flow.
1083-
// If the login succeeds, this transaction will commit and the user
1084-
// will be converted.
1085-
// nolint:gocritic // system query to update user login type
1086-
user, err = tx.UpdateUserLoginType(dbauthz.AsSystemRestricted(ctx), database.UpdateUserLoginTypeParams{
1087-
LoginType: params.LoginType,
1088-
UserID: user.ID,
1089-
})
1090-
if err != nil {
1091-
return httpError{
1092-
code: http.StatusInternalServerError,
1093-
msg: "Failed to convert user to new login type",
1094-
}
1037+
return err
10951038
}
10961039
}
10971040

@@ -1258,6 +1201,91 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook
12581201
return cookie, *key, nil
12591202
}
12601203

1204+
// convertUserToOauth will convert a user from password base loginType to
1205+
// an oauth login type. If it fails, it will return a httpError
1206+
func (api *API) convertUserToOauth(ctx context.Context, r *http.Request, db database.Store, params oauthLoginParams) error {
1207+
user := params.User
1208+
1209+
// Trying to convert to OIDC, but the email does not match.
1210+
// So do not make a new user, just block the request.
1211+
if user.ID == uuid.Nil {
1212+
return httpError{
1213+
code: http.StatusBadRequest,
1214+
msg: fmt.Sprintf("The oidc account with the email %q does not match the email of the account you are trying to convert. Contact your administrator to resolve this issue.", params.Email),
1215+
}
1216+
}
1217+
1218+
// nolint:gocritic // Required to auth the oidc convert
1219+
mergeState, err := db.GetUserOauthMergeState(dbauthz.AsSystemRestricted(ctx), database.GetUserOauthMergeStateParams{
1220+
UserID: user.ID,
1221+
StateString: params.State.StateString,
1222+
})
1223+
if xerrors.Is(err, sql.ErrNoRows) {
1224+
return httpError{
1225+
code: http.StatusBadRequest,
1226+
msg: "No convert login request found with given state. Restart the convert process and try again.",
1227+
}
1228+
}
1229+
if err != nil {
1230+
return httpError{
1231+
code: http.StatusInternalServerError,
1232+
msg: err.Error(),
1233+
}
1234+
}
1235+
1236+
// At this point, this request could be an attempt to convert from
1237+
// password auth to oauth auth. Always log these attempts.
1238+
var (
1239+
auditor = *api.Auditor.Load()
1240+
oauthConvertAudit, commitOauthConvertAudit = params.InitAuditRequest(&audit.RequestParams{
1241+
Audit: auditor,
1242+
Log: api.Logger,
1243+
Request: r,
1244+
Action: database.AuditActionLogin,
1245+
})
1246+
)
1247+
oauthConvertAudit.Old = mergeState
1248+
defer commitOauthConvertAudit()
1249+
1250+
// If we do not allow converting to oauth, return an error.
1251+
if !params.OauthConversionEnabled {
1252+
return httpError{
1253+
code: http.StatusForbidden,
1254+
msg: fmt.Sprintf("Incorrect login type, attempting to use %q but user is of login type %q",
1255+
params.LoginType,
1256+
user.LoginType,
1257+
),
1258+
}
1259+
}
1260+
1261+
// Make sure the merge state generated matches this OIDC login request.
1262+
// It needs to have the correct login type information for this
1263+
// user.
1264+
if user.ID != mergeState.UserID || user.LoginType != mergeState.FromLoginType || params.LoginType != mergeState.ToLoginType {
1265+
return httpError{
1266+
code: http.StatusForbidden,
1267+
msg: fmt.Sprintf("Request to convert login type from %s to %s failed", user.LoginType, params.LoginType),
1268+
}
1269+
}
1270+
1271+
// Convert the user and default to the normal login flow.
1272+
// If the login succeeds, this transaction will commit and the user
1273+
// will be converted.
1274+
// nolint:gocritic // system query to update user login type
1275+
user, err = db.UpdateUserLoginType(dbauthz.AsSystemRestricted(ctx), database.UpdateUserLoginTypeParams{
1276+
LoginType: params.LoginType,
1277+
UserID: user.ID,
1278+
})
1279+
if err != nil {
1280+
return httpError{
1281+
code: http.StatusInternalServerError,
1282+
msg: "Failed to convert user to new login type",
1283+
}
1284+
}
1285+
return nil
1286+
1287+
}
1288+
12611289
// githubLinkedID returns the unique ID for a GitHub user.
12621290
func githubLinkedID(u *github.User) string {
12631291
return strconv.FormatInt(u.GetID(), 10)
@@ -1324,3 +1352,10 @@ func findLinkedUser(ctx context.Context, db database.Store, linkedID string, ema
13241352

13251353
return user, link, nil
13261354
}
1355+
1356+
func isMergeStateString(state string) bool {
1357+
if strings.HasPrefix(state, mergeStateStringPrefix) {
1358+
return true
1359+
}
1360+
return false
1361+
}

0 commit comments

Comments
 (0)