Skip to content

Commit 083d256

Browse files
committed
pr comments
1 parent 5c7cbae commit 083d256

File tree

4 files changed

+59
-57
lines changed

4 files changed

+59
-57
lines changed

coderd/database/migrations/000034_linked_user_id.up.sql

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ CREATE TABLE IF NOT EXISTS user_links (
77
oauth_access_token text DEFAULT ''::text NOT NULL,
88
oauth_refresh_token text DEFAULT ''::text NOT NULL,
99
oauth_expiry timestamp with time zone DEFAULT '0001-01-01 00:00:00+00'::timestamp with time zone NOT NULL,
10-
UNIQUE(user_id, login_type),
11-
FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE
10+
PRIMARY KEY(user_id, login_type),
11+
FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE
1212
);
1313

1414
-- This migrates columns on api_keys to the new user_links table.
@@ -18,34 +18,34 @@ CREATE TABLE IF NOT EXISTS user_links (
1818
-- A user should at most have a row for an OIDC account and a Github account.
1919
-- 'password' login types are ignored.
2020

21-
INSERT INTO user_links
22-
(
21+
INSERT INTO user_links
22+
(
2323
user_id,
2424
login_type,
2525
linked_id,
2626
oauth_access_token,
2727
oauth_refresh_token,
2828
oauth_expiry
2929
)
30-
SELECT
31-
keys.user_id,
30+
SELECT
31+
keys.user_id,
3232
keys.login_type,
3333
'',
3434
keys.oauth_access_token,
3535
keys.oauth_refresh_token,
36-
keys.oauth_expiry
37-
FROM
38-
(
39-
SELECT
40-
row_number() OVER (partition by user_id, login_type ORDER BY last_used DESC) AS x,
36+
keys.oauth_expiry
37+
FROM
38+
(
39+
SELECT
40+
row_number() OVER (partition by user_id, login_type ORDER BY last_used DESC) AS x,
4141
api_keys.* FROM api_keys
4242
) as keys
43-
WHERE x=1 AND keys.login_type != 'password';
43+
WHERE x=1 AND keys.login_type != 'password';
4444

4545
-- Drop columns that have been migrated to user_links.
4646
-- It appears the 'oauth_id_token' was unused and so it has
4747
-- been dropped here as well to avoid future confusion.
48-
ALTER TABLE api_keys
48+
ALTER TABLE api_keys
4949
DROP COLUMN oauth_access_token,
5050
DROP COLUMN oauth_refresh_token,
5151
DROP COLUMN oauth_id_token,
@@ -54,18 +54,18 @@ ALTER TABLE api_keys
5454
ALTER TABLE users ADD COLUMN login_type login_type NOT NULL DEFAULT 'password';
5555

5656
UPDATE
57-
users
57+
users
5858
SET
59-
login_type = (
60-
SELECT
61-
login_type
62-
FROM
63-
user_links
64-
WHERE
65-
user_links.user_id = users.id
66-
ORDER BY oauth_expiry DESC
67-
LIMIT 1
68-
)
59+
login_type = (
60+
SELECT
61+
login_type
62+
FROM
63+
user_links
64+
WHERE
65+
user_links.user_id = users.id
66+
ORDER BY oauth_expiry DESC
67+
LIMIT 1
68+
)
6969
FROM
7070
user_links
7171
WHERE
Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,46 @@
11
-- name: GetUserLinkByLinkedID :one
22
SELECT
3-
*
3+
*
44
FROM
5-
user_links
5+
user_links
66
WHERE
7-
linked_id = $1;
7+
linked_id = $1;
88

99
-- name: GetUserLinkByUserIDLoginType :one
1010
SELECT
11-
*
11+
*
1212
FROM
13-
user_links
13+
user_links
1414
WHERE
15-
user_id = $1 AND login_type = $2;
15+
user_id = $1 AND login_type = $2;
1616

1717
-- name: InsertUserLink :one
1818
INSERT INTO
19-
user_links (
20-
user_id,
21-
login_type,
22-
linked_id,
23-
oauth_access_token,
24-
oauth_refresh_token,
25-
oauth_expiry
26-
)
19+
user_links (
20+
user_id,
21+
login_type,
22+
linked_id,
23+
oauth_access_token,
24+
oauth_refresh_token,
25+
oauth_expiry
26+
)
2727
VALUES
28-
( $1, $2, $3, $4, $5, $6 ) RETURNING *;
28+
( $1, $2, $3, $4, $5, $6 ) RETURNING *;
2929

3030
-- name: UpdateUserLinkedID :one
3131
UPDATE
32-
user_links
32+
user_links
3333
SET
34-
linked_id = $1
34+
linked_id = $1
3535
WHERE
36-
user_id = $2 AND login_type = $3 RETURNING *;
36+
user_id = $2 AND login_type = $3 RETURNING *;
3737

3838
-- name: UpdateUserLink :one
3939
UPDATE
40-
user_links
40+
user_links
4141
SET
42-
oauth_access_token = $1,
43-
oauth_refresh_token = $2,
44-
oauth_expiry = $3
42+
oauth_access_token = $1,
43+
oauth_refresh_token = $2,
44+
oauth_expiry = $3
4545
WHERE
46-
user_id = $4 AND login_type = $5 RETURNING *;
46+
user_id = $4 AND login_type = $5 RETURNING *;

coderd/database/queries/users.sql

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ INSERT INTO
3838
created_at,
3939
updated_at,
4040
rbac_roles,
41-
login_type
41+
login_type
4242
)
4343
VALUES
4444
($1, $2, $3, $4, $5, $6, $7, $8) RETURNING *;
@@ -55,12 +55,12 @@ WHERE
5555

5656
-- name: UpdateUserRoles :one
5757
UPDATE
58-
users
58+
users
5959
SET
6060
-- Remove all duplicates from the roles.
6161
rbac_roles = ARRAY(SELECT DISTINCT UNNEST(@granted_roles :: text[]))
6262
WHERE
63-
id = @id
63+
id = @id
6464
RETURNING *;
6565

6666
-- name: UpdateUserHashedPassword :exec
@@ -123,8 +123,8 @@ WHERE
123123
END
124124
-- End of filters
125125
ORDER BY
126-
-- Deterministic and consistent ordering of all users, even if they share
127-
-- a timestamp. This is to ensure consistent pagination.
126+
-- Deterministic and consistent ordering of all users, even if they share
127+
-- a timestamp. This is to ensure consistent pagination.
128128
(created_at, id) ASC OFFSET @offset_opt
129129
LIMIT
130130
-- A null limit means "no limit", so 0 means return all
@@ -153,10 +153,10 @@ SELECT
153153
array_append(users.rbac_roles, 'member'),
154154
-- All org_members get the org-member role for their orgs
155155
array_append(organization_members.roles, 'organization-member:'||organization_members.organization_id::text)) :: text[]
156-
AS roles
156+
AS roles
157157
FROM
158158
users
159159
LEFT JOIN organization_members
160160
ON id = user_id
161161
WHERE
162-
id = @user_id;
162+
id = @user_id;

coderd/userauth.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
148148

149149
if user.ID != uuid.Nil && user.LoginType != database.LoginTypeGithub {
150150
httpapi.Write(rw, http.StatusForbidden, codersdk.Response{
151-
Message: fmt.Sprintf("Incorrect login type, attempting to use %q but user is of login type %q", database.LoginTypeOIDC, user.LoginType),
151+
Message: fmt.Sprintf("Incorrect login type, attempting to use %q but user is of login type %q", database.LoginTypeGithub, user.LoginType),
152152
})
153153
return
154154
}
@@ -215,7 +215,7 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
215215
if err != nil {
216216
httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{
217217
Message: "A database error occurred.",
218-
Detail: xerrors.Errorf("insert user link: %w", err.Error).Error(),
218+
Detail: fmt.Sprintf("insert user link: %s", err.Error()),
219219
})
220220
return
221221
}
@@ -358,6 +358,8 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
358358
return
359359
}
360360

361+
// This can happen if a user is a built-in user but is signing in
362+
// with OIDC for the first time.
361363
if user.ID == uuid.Nil {
362364
var organizationID uuid.UUID
363365
organizations, _ := api.Database.GetOrganizations(ctx)
@@ -404,7 +406,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
404406
if err != nil {
405407
httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{
406408
Message: "A database error occurred.",
407-
Detail: xerrors.Errorf("insert user link: %w", err.Error).Error(),
409+
Detail: fmt.Sprintf("insert user link: %s", err.Error()),
408410
})
409411
return
410412
}

0 commit comments

Comments
 (0)