Skip to content

Commit 9cfb69c

Browse files
EmyrkEmyrk
committed
Add from_login_type
1 parent 368fa2b commit 9cfb69c

File tree

12 files changed

+55
-38
lines changed

12 files changed

+55
-38
lines changed

‎coderd/coderd.go

+3
Original file line numberDiff line numberDiff line change
@@ -595,6 +595,9 @@ func New(options *Options) *API {
595595
r.Get("/first", api.firstUser)
596596
r.Post("/first", api.postFirstUser)
597597
r.Route("/authmethods", func(r chi.Router) {
598+
// The API Key allows this method to return the auth method
599+
// for the logged-in user. This information is useful for the
600+
// caller. If not authenticated, this information is omitted.
598601
r.Use(apiKeyMiddlewareOptional)
599602
r.Get("/", api.userAuthMethods)
600603
})

‎coderd/database/dbfake/dbfake.go

+6-5
Original file line numberDiff line numberDiff line change
@@ -4044,11 +4044,12 @@ func (q *fakeQuerier) InsertUserOauthMergeState(_ context.Context, arg database.
40444044
}
40454045

40464046
s := database.OauthMergeState{
4047-
StateString: arg.StateString,
4048-
CreatedAt: arg.CreatedAt,
4049-
ExpiresAt: arg.ExpiresAt,
4050-
ToLoginType: arg.ToLoginType,
4051-
UserID: arg.UserID,
4047+
StateString: arg.StateString,
4048+
CreatedAt: arg.CreatedAt,
4049+
ExpiresAt: arg.ExpiresAt,
4050+
FromLoginType: arg.FromLoginType,
4051+
ToLoginType: arg.ToLoginType,
4052+
UserID: arg.UserID,
40524053
}
40534054
q.oauthMergeStates = append(q.oauthMergeStates, s)
40544055
return s, nil

‎coderd/database/dump.sql

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

‎coderd/database/migrations/000131_merge_oidc_account.up.sql

+1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ CREATE TABLE IF NOT EXISTS oauth_merge_state (
44
state_string text NOT NULL,
55
created_at timestamptz NOT NULL,
66
expires_at timestamptz NOT NULL,
7+
from_login_type login_type NOT NULL,
78
to_login_type login_type NOT NULL,
89
user_id uuid NOT NULL
910
REFERENCES users (id) ON DELETE CASCADE,

‎coderd/database/migrations/testdata/fixtures/000131_oauth_merge_state.up.sql

+2
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,14 @@ INSERT INTO oauth_merge_state(
22
state_string,
33
created_at,
44
expires_at,
5+
from_login_type,
56
to_login_type,
67
user_id
78
) VALUES (
89
gen_random_uuid()::text,
910
now(),
1011
now() + interval '24 hour',
12+
'password',
1113
'oidc',
1214
(SELECT id FROM users LIMIT 1)
1315
)

‎coderd/database/models.go

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

‎coderd/database/queries.sql.go

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

‎coderd/database/queries/users.sql

+2-1
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,13 @@ INSERT INTO
1212
oauth_merge_state (
1313
user_id,
1414
state_string,
15+
from_login_type,
1516
to_login_type,
1617
created_at,
1718
expires_at
1819
)
1920
VALUES
20-
($1, $2, $3, $4, $5) RETURNING *;
21+
($1, $2, $3, $4, $5, $6) RETURNING *;
2122

2223
-- name: DeleteUserOauthMergeStates :exec
2324
DELETE FROM oauth_merge_state WHERE user_id = @user_id;

‎coderd/httpmw/oauth2.go

-9
Original file line numberDiff line numberDiff line change
@@ -178,12 +178,3 @@ func ExtractOAuth2(config OAuth2Config, client *http.Client, authURLOpts map[str
178178
})
179179
}
180180
}
181-
182-
func RemoveOauthStateCookie(rw http.ResponseWriter) {
183-
http.SetCookie(rw, &http.Cookie{
184-
Name: codersdk.OAuth2StateCookie,
185-
Value: "",
186-
Path: "/",
187-
MaxAge: -1,
188-
})
189-
}

‎coderd/userauth.go

+19-9
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ func (api *API) postConvertLoginType(rw http.ResponseWriter, r *http.Request) {
6565

6666
switch req.ToLoginType {
6767
case codersdk.LoginTypeGithub, codersdk.LoginTypeOIDC:
68+
// Allowed!
6869
case codersdk.LoginTypeNone, codersdk.LoginTypePassword, codersdk.LoginTypeToken:
6970
// These login types are not allowed to be converted to at this time.
7071
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
@@ -78,6 +79,7 @@ func (api *API) postConvertLoginType(rw http.ResponseWriter, r *http.Request) {
7879
return
7980
}
8081

82+
// This handles the email/pass checking.
8183
user, _, ok := api.loginRequest(ctx, rw, req.LoginWithPasswordRequest)
8284
if !ok {
8385
return
@@ -116,11 +118,12 @@ func (api *API) postConvertLoginType(rw http.ResponseWriter, r *http.Request) {
116118
}
117119

118120
mergeState, err = store.InsertUserOauthMergeState(ctx, database.InsertUserOauthMergeStateParams{
119-
UserID: user.ID,
120-
StateString: stateString,
121-
ToLoginType: database.LoginType(req.ToLoginType),
122-
CreatedAt: now,
123-
ExpiresAt: now.Add(time.Minute * 5),
121+
UserID: user.ID,
122+
StateString: stateString,
123+
FromLoginType: user.LoginType,
124+
ToLoginType: database.LoginType(req.ToLoginType),
125+
CreatedAt: now,
126+
ExpiresAt: now.Add(time.Minute * 5),
124127
})
125128
if err != nil {
126129
return err
@@ -175,7 +178,8 @@ func (api *API) postLogin(rw http.ResponseWriter, r *http.Request) {
175178
}
176179

177180
user, roles, ok := api.loginRequest(ctx, rw, loginWithPassword)
178-
// 'user.ID' will be empty, or will be an actual value.
181+
// 'user.ID' will be empty, or will be an actual value. Either is correct
182+
// here.
179183
aReq.UserID = user.ID
180184
if !ok {
181185
// user failed to login
@@ -1030,9 +1034,13 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook
10301034
user.LoginType,
10311035
),
10321036
}
1037+
// If we do not allow converting to oauth, return an error.
10331038
if !params.OauthConversionEnabled {
10341039
return wrongLoginTypeErr
10351040
}
1041+
1042+
// At this point, this request could be an attempt to convert from
1043+
// password auth to oauth auth.
10361044
var (
10371045
auditor = *api.Auditor.Load()
10381046
oauthConvertAudit, commitOauthConvertAudit = params.InitAuditRequest(&audit.RequestParams{
@@ -1052,7 +1060,6 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook
10521060
if xerrors.Is(err, sql.ErrNoRows) {
10531061
return wrongLoginTypeErr
10541062
}
1055-
oauthConvertAudit.Old = mergeState
10561063

10571064
failedMsg := fmt.Sprintf("Request to convert login type from %s to %s failed", user.LoginType, params.LoginType)
10581065
if err != nil {
@@ -1061,8 +1068,11 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook
10611068
msg: failedMsg,
10621069
}
10631070
}
1064-
if user.ID != mergeState.UserID {
1065-
// User tried to use someone else's merge state?
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 {
10661076
return httpError{
10671077
code: http.StatusForbidden,
10681078
msg: failedMsg,

‎docs/admin/audit-logs.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ We track the following resources:
1515
| Group<br><i>create, write, delete</i> | <table><thead><tr><th>Field</th><th>Tracked</th></tr></thead><tbody><tr><td>avatar_url</td><td>true</td></tr><tr><td>id</td><td>true</td></tr><tr><td>members</td><td>true</td></tr><tr><td>name</td><td>true</td></tr><tr><td>organization_id</td><td>false</td></tr><tr><td>quota_allowance</td><td>true</td></tr></tbody></table> |
1616
| GitSSHKey<br><i>create</i> | <table><thead><tr><th>Field</th><th>Tracked</th></tr></thead><tbody><tr><td>created_at</td><td>false</td></tr><tr><td>private_key</td><td>true</td></tr><tr><td>public_key</td><td>true</td></tr><tr><td>updated_at</td><td>false</td></tr><tr><td>user_id</td><td>true</td></tr></tbody></table> |
1717
| License<br><i>create, delete</i> | <table><thead><tr><th>Field</th><th>Tracked</th></tr></thead><tbody><tr><td>exp</td><td>true</td></tr><tr><td>id</td><td>false</td></tr><tr><td>jwt</td><td>false</td></tr><tr><td>uploaded_at</td><td>true</td></tr><tr><td>uuid</td><td>true</td></tr></tbody></table> |
18-
| OauthMergeState<br><i></i> | <table><thead><tr><th>Field</th><th>Tracked</th></tr></thead><tbody><tr><td>created_at</td><td>true</td></tr><tr><td>expires_at</td><td>true</td></tr><tr><td>state_string</td><td>true</td></tr><tr><td>to_login_type</td><td>true</td></tr><tr><td>user_id</td><td>true</td></tr></tbody></table> |
18+
| OauthMergeState<br><i></i> | <table><thead><tr><th>Field</th><th>Tracked</th></tr></thead><tbody><tr><td>created_at</td><td>true</td></tr><tr><td>expires_at</td><td>true</td></tr><tr><td>from_login_type</td><td>true</td></tr><tr><td>state_string</td><td>true</td></tr><tr><td>to_login_type</td><td>true</td></tr><tr><td>user_id</td><td>true</td></tr></tbody></table> |
1919
| Template<br><i>write, delete</i> | <table><thead><tr><th>Field</th><th>Tracked</th></tr></thead><tbody><tr><td>active_version_id</td><td>true</td></tr><tr><td>allow_user_autostart</td><td>true</td></tr><tr><td>allow_user_autostop</td><td>true</td></tr><tr><td>allow_user_cancel_workspace_jobs</td><td>true</td></tr><tr><td>created_at</td><td>false</td></tr><tr><td>created_by</td><td>true</td></tr><tr><td>default_ttl</td><td>true</td></tr><tr><td>deleted</td><td>false</td></tr><tr><td>description</td><td>true</td></tr><tr><td>display_name</td><td>true</td></tr><tr><td>failure_ttl</td><td>true</td></tr><tr><td>group_acl</td><td>true</td></tr><tr><td>icon</td><td>true</td></tr><tr><td>id</td><td>true</td></tr><tr><td>inactivity_ttl</td><td>true</td></tr><tr><td>locked_ttl</td><td>true</td></tr><tr><td>max_ttl</td><td>true</td></tr><tr><td>name</td><td>true</td></tr><tr><td>organization_id</td><td>false</td></tr><tr><td>provisioner</td><td>true</td></tr><tr><td>updated_at</td><td>false</td></tr><tr><td>user_acl</td><td>true</td></tr></tbody></table> |
2020
| TemplateVersion<br><i>create, write</i> | <table><thead><tr><th>Field</th><th>Tracked</th></tr></thead><tbody><tr><td>created_at</td><td>false</td></tr><tr><td>created_by</td><td>true</td></tr><tr><td>git_auth_providers</td><td>false</td></tr><tr><td>id</td><td>true</td></tr><tr><td>job_id</td><td>false</td></tr><tr><td>name</td><td>true</td></tr><tr><td>organization_id</td><td>false</td></tr><tr><td>readme</td><td>true</td></tr><tr><td>template_id</td><td>true</td></tr><tr><td>updated_at</td><td>false</td></tr></tbody></table> |
2121
| User<br><i>create, write, delete</i> | <table><thead><tr><th>Field</th><th>Tracked</th></tr></thead><tbody><tr><td>avatar_url</td><td>false</td></tr><tr><td>created_at</td><td>false</td></tr><tr><td>deleted</td><td>true</td></tr><tr><td>email</td><td>true</td></tr><tr><td>hashed_password</td><td>true</td></tr><tr><td>id</td><td>true</td></tr><tr><td>last_seen_at</td><td>false</td></tr><tr><td>login_type</td><td>false</td></tr><tr><td>rbac_roles</td><td>true</td></tr><tr><td>status</td><td>true</td></tr><tr><td>updated_at</td><td>false</td></tr><tr><td>username</td><td>true</td></tr></tbody></table> |

‎enterprise/audit/table.go

+6-5
Original file line numberDiff line numberDiff line change
@@ -157,11 +157,12 @@ var auditableResourcesTypes = map[any]map[string]Action{
157157
"token_name": ActionIgnore,
158158
},
159159
&database.OauthMergeState{}: {
160-
"state_string": ActionSecret,
161-
"created_at": ActionTrack,
162-
"expires_at": ActionTrack,
163-
"to_login_type": ActionTrack,
164-
"user_id": ActionTrack,
160+
"state_string": ActionSecret,
161+
"created_at": ActionTrack,
162+
"expires_at": ActionTrack,
163+
"from_login_type": ActionTrack,
164+
"to_login_type": ActionTrack,
165+
"user_id": ActionTrack,
165166
},
166167
// TODO: track an ID here when the below ticket is completed:
167168
// https://github.com/coder/coder/pull/6012

0 commit comments

Comments
 (0)