Skip to content

Commit 134f152

Browse files
BrunoQuaresmaBrunoQuaresma
committed
Merge branch 'stevenmasley/merge_oidc_account' into bq/oidc-ui
2 parents a353043 + 57b3605 commit 134f152

File tree

7 files changed

+120
-73
lines changed

7 files changed

+120
-73
lines changed

‎coderd/apidoc/docs.go

+3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

+3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/userauth.go

+103-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
@@ -406,6 +411,7 @@ func (api *API) userAuthMethods(rw http.ResponseWriter, r *http.Request) {
406411

407412
httpapi.Write(r.Context(), rw, http.StatusOK, codersdk.AuthMethods{
408413
UserAuthenticationType: currentUserLoginType,
414+
ConvertToOIDCEnabled: api.Options.DeploymentValues.EnableOauthAccountConversion.Value(),
409415
Password: codersdk.AuthMethod{
410416
Enabled: !api.DeploymentValues.DisablePasswordAuth.Value(),
411417
},
@@ -1024,74 +1030,12 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook
10241030
}
10251031
}
10261032

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)
1033+
// If you do a convert to OIDC and your email does not match, we need to
1034+
// catch this and not make a new account.
1035+
if isMergeStateString(params.State.StateString) {
1036+
err := api.convertUserToOauth(ctx, r, tx, params)
10651037
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-
}
1038+
return err
10951039
}
10961040
}
10971041

@@ -1258,6 +1202,91 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook
12581202
return cookie, *key, nil
12591203
}
12601204

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

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

‎codersdk/users.go

+1
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ type AuthMethods struct {
126126
// UserAuthenticationType returns the authentication method for the given
127127
// caller if the request is an authenticated request. Otherwise it is empty.
128128
UserAuthenticationType LoginType `json:"me_login_type,omitempty"`
129+
ConvertToOIDCEnabled bool `json:"convert_to_oidc_enabled"`
129130
Password AuthMethod `json:"password"`
130131
Github AuthMethod `json:"github"`
131132
OIDC OIDCAuthMethod `json:"oidc"`

‎docs/api/schemas.md

+8-6
Original file line numberDiff line numberDiff line change
@@ -1107,6 +1107,7 @@
11071107

11081108
```json
11091109
{
1110+
"convert_to_oidc_enabled": true,
11101111
"github": {
11111112
"enabled": true
11121113
},
@@ -1124,12 +1125,13 @@
11241125

11251126
### Properties
11261127

1127-
| Name | Type | Required | Restrictions | Description |
1128-
| --------------- | -------------------------------------------------- | -------- | ------------ | --------------------------------------------------------------------------------------------------------------------------------------- |
1129-
| `github` | [codersdk.AuthMethod](#codersdkauthmethod) | false | | |
1130-
| `me_login_type` | [codersdk.LoginType](#codersdklogintype) | false | | Me login type returns the authentication method for the given caller if the request is an authenticated request. Otherwise it is empty. |
1131-
| `oidc` | [codersdk.OIDCAuthMethod](#codersdkoidcauthmethod) | false | | |
1132-
| `password` | [codersdk.AuthMethod](#codersdkauthmethod) | false | | |
1128+
| Name | Type | Required | Restrictions | Description |
1129+
| ------------------------- | -------------------------------------------------- | -------- | ------------ | --------------------------------------------------------------------------------------------------------------------------------------- |
1130+
| `convert_to_oidc_enabled` | boolean | false | | |
1131+
| `github` | [codersdk.AuthMethod](#codersdkauthmethod) | false | | |
1132+
| `me_login_type` | [codersdk.LoginType](#codersdklogintype) | false | | Me login type returns the authentication method for the given caller if the request is an authenticated request. Otherwise it is empty. |
1133+
| `oidc` | [codersdk.OIDCAuthMethod](#codersdkoidcauthmethod) | false | | |
1134+
| `password` | [codersdk.AuthMethod](#codersdkauthmethod) | false | | |
11331135

11341136
## codersdk.AuthorizationCheck
11351137

‎docs/api/users.md

+1
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ curl -X GET http://coder-server:8080/api/v2/users/authmethods \
140140
141141
```json
142142
{
143+
"convert_to_oidc_enabled": true,
143144
"github": {
144145
"enabled": true
145146
},

‎site/src/api/typesGenerated.ts

+1
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ export interface AuthMethod {
105105
// From codersdk/users.go
106106
export interface AuthMethods {
107107
readonly me_login_type?: LoginType
108+
readonly convert_to_oidc_enabled: boolean
108109
readonly password: AuthMethod
109110
readonly github: AuthMethod
110111
readonly oidc: OIDCAuthMethod

0 commit comments

Comments
 (0)