Skip to content

Commit c3eea98

Browse files
sreyasreya
authored
fix: use unique ID for linked accounts (#3441)
- move OAuth-related fields off of api_keys into a new user_links table - restrict users to single form of login - process updates to user email/usernames for OIDC - added a login_type column to users
1 parent 53d1fb3 commit c3eea98

29 files changed

+909
-244
lines changed

‎coderd/audit/table.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ var AuditableResources = auditMap(map[any]map[string]Action{
9494
"updated_at": ActionIgnore, // Changes, but is implicit and not helpful in a diff.
9595
"status": ActionTrack,
9696
"rbac_roles": ActionTrack,
97+
"login_type": ActionIgnore,
9798
},
9899
&database.Workspace{}: {
99100
"id": ActionTrack,

‎coderd/database/databasefake/databasefake.go

Lines changed: 89 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ type data struct {
7373
organizations []database.Organization
7474
organizationMembers []database.OrganizationMember
7575
users []database.User
76+
userLinks []database.UserLink
7677

7778
// New tables
7879
auditLogs []database.AuditLog
@@ -1454,20 +1455,16 @@ func (q *fakeQuerier) InsertAPIKey(_ context.Context, arg database.InsertAPIKeyP
14541455

14551456
//nolint:gosimple
14561457
key := database.APIKey{
1457-
ID: arg.ID,
1458-
LifetimeSeconds: arg.LifetimeSeconds,
1459-
HashedSecret: arg.HashedSecret,
1460-
IPAddress: arg.IPAddress,
1461-
UserID: arg.UserID,
1462-
ExpiresAt: arg.ExpiresAt,
1463-
CreatedAt: arg.CreatedAt,
1464-
UpdatedAt: arg.UpdatedAt,
1465-
LastUsed: arg.LastUsed,
1466-
LoginType: arg.LoginType,
1467-
OAuthAccessToken: arg.OAuthAccessToken,
1468-
OAuthRefreshToken: arg.OAuthRefreshToken,
1469-
OAuthIDToken: arg.OAuthIDToken,
1470-
OAuthExpiry: arg.OAuthExpiry,
1458+
ID: arg.ID,
1459+
LifetimeSeconds: arg.LifetimeSeconds,
1460+
HashedSecret: arg.HashedSecret,
1461+
IPAddress: arg.IPAddress,
1462+
UserID: arg.UserID,
1463+
ExpiresAt: arg.ExpiresAt,
1464+
CreatedAt: arg.CreatedAt,
1465+
UpdatedAt: arg.UpdatedAt,
1466+
LastUsed: arg.LastUsed,
1467+
LoginType: arg.LoginType,
14711468
}
14721469
q.apiKeys = append(q.apiKeys, key)
14731470
return key, nil
@@ -1744,6 +1741,7 @@ func (q *fakeQuerier) InsertUser(_ context.Context, arg database.InsertUserParam
17441741
Username: arg.Username,
17451742
Status: database.UserStatusActive,
17461743
RBACRoles: arg.RBACRoles,
1744+
LoginType: arg.LoginType,
17471745
}
17481746
q.users = append(q.users, user)
17491747
return user, nil
@@ -1899,9 +1897,6 @@ func (q *fakeQuerier) UpdateAPIKeyByID(_ context.Context, arg database.UpdateAPI
18991897
apiKey.LastUsed = arg.LastUsed
19001898
apiKey.ExpiresAt = arg.ExpiresAt
19011899
apiKey.IPAddress = arg.IPAddress
1902-
apiKey.OAuthAccessToken = arg.OAuthAccessToken
1903-
apiKey.OAuthRefreshToken = arg.OAuthRefreshToken
1904-
apiKey.OAuthExpiry = arg.OAuthExpiry
19051900
q.apiKeys[index] = apiKey
19061901
return nil
19071902
}
@@ -2260,3 +2255,80 @@ func (q *fakeQuerier) GetDeploymentID(_ context.Context) (string, error) {
22602255

22612256
return q.deploymentID, nil
22622257
}
2258+
2259+
func (q *fakeQuerier) GetUserLinkByLinkedID(_ context.Context, id string) (database.UserLink, error) {
2260+
q.mutex.RLock()
2261+
defer q.mutex.RUnlock()
2262+
2263+
for _, link := range q.userLinks {
2264+
if link.LinkedID == id {
2265+
return link, nil
2266+
}
2267+
}
2268+
return database.UserLink{}, sql.ErrNoRows
2269+
}
2270+
2271+
func (q *fakeQuerier) GetUserLinkByUserIDLoginType(_ context.Context, params database.GetUserLinkByUserIDLoginTypeParams) (database.UserLink, error) {
2272+
q.mutex.RLock()
2273+
defer q.mutex.RUnlock()
2274+
2275+
for _, link := range q.userLinks {
2276+
if link.UserID == params.UserID && link.LoginType == params.LoginType {
2277+
return link, nil
2278+
}
2279+
}
2280+
return database.UserLink{}, sql.ErrNoRows
2281+
}
2282+
2283+
func (q *fakeQuerier) InsertUserLink(_ context.Context, args database.InsertUserLinkParams) (database.UserLink, error) {
2284+
q.mutex.RLock()
2285+
defer q.mutex.RUnlock()
2286+
2287+
//nolint:gosimple
2288+
link := database.UserLink{
2289+
UserID: args.UserID,
2290+
LoginType: args.LoginType,
2291+
LinkedID: args.LinkedID,
2292+
OAuthAccessToken: args.OAuthAccessToken,
2293+
OAuthRefreshToken: args.OAuthRefreshToken,
2294+
OAuthExpiry: args.OAuthExpiry,
2295+
}
2296+
2297+
q.userLinks = append(q.userLinks, link)
2298+
2299+
return link, nil
2300+
}
2301+
2302+
func (q *fakeQuerier) UpdateUserLinkedID(_ context.Context, params database.UpdateUserLinkedIDParams) (database.UserLink, error) {
2303+
q.mutex.RLock()
2304+
defer q.mutex.RUnlock()
2305+
2306+
for i, link := range q.userLinks {
2307+
if link.UserID == params.UserID && link.LoginType == params.LoginType {
2308+
link.LinkedID = params.LinkedID
2309+
2310+
q.userLinks[i] = link
2311+
return link, nil
2312+
}
2313+
}
2314+
2315+
return database.UserLink{}, sql.ErrNoRows
2316+
}
2317+
2318+
func (q *fakeQuerier) UpdateUserLink(_ context.Context, params database.UpdateUserLinkParams) (database.UserLink, error) {
2319+
q.mutex.RLock()
2320+
defer q.mutex.RUnlock()
2321+
2322+
for i, link := range q.userLinks {
2323+
if link.UserID == params.UserID && link.LoginType == params.LoginType {
2324+
link.OAuthAccessToken = params.OAuthAccessToken
2325+
link.OAuthRefreshToken = params.OAuthRefreshToken
2326+
link.OAuthExpiry = params.OAuthExpiry
2327+
2328+
q.userLinks[i] = link
2329+
return link, nil
2330+
}
2331+
}
2332+
2333+
return database.UserLink{}, sql.ErrNoRows
2334+
}

‎coderd/database/db_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ func TestNestedInTx(t *testing.T) {
3737
CreatedAt: database.Now(),
3838
UpdatedAt: database.Now(),
3939
RBACRoles: []string{},
40+
LoginType: database.LoginTypeGithub,
4041
})
4142
return err
4243
})

‎coderd/database/dump.sql

Lines changed: 17 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/generate.sh

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ SCRIPT_DIR=$(dirname "${BASH_SOURCE[0]}")
1313
(
1414
cd "$SCRIPT_DIR"
1515

16+
# Dump the updated schema.
17+
go run dump/main.go
1618
# The logic below depends on the exact version being correct :(
1719
go run github.com/kyleconroy/sqlc/cmd/sqlc@v1.13.0 generate
1820

‎coderd/database/migrations/000035_linked_user_id.down.sql

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
-- This migration makes no attempt to try to populate
2+
-- the oauth_access_token, oauth_refresh_token, and oauth_expiry
3+
-- columns of api_key rows with the values from the dropped user_links
4+
-- table.
5+
BEGIN;
6+
7+
DROP TABLE IF EXISTS user_links;
8+
9+
ALTER TABLE
10+
api_keys
11+
ADD COLUMN oauth_access_token text DEFAULT ''::text NOT NULL;
12+
13+
ALTER TABLE
14+
api_keys
15+
ADD COLUMN oauth_refresh_token text DEFAULT ''::text NOT NULL;
16+
17+
ALTER TABLE
18+
api_keys
19+
ADD COLUMN oauth_expiry timestamp with time zone DEFAULT '0001-01-01 00:00:00+00'::timestamp with time zone NOT NULL;
20+
21+
ALTER TABLE users DROP COLUMN login_type;
22+
23+
COMMIT;

‎coderd/database/migrations/000035_linked_user_id.up.sql

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
BEGIN;
2+
3+
CREATE TABLE IF NOT EXISTS user_links (
4+
user_id uuid NOT NULL,
5+
login_type login_type NOT NULL,
6+
linked_id text DEFAULT ''::text NOT NULL,
7+
oauth_access_token text DEFAULT ''::text NOT NULL,
8+
oauth_refresh_token text DEFAULT ''::text NOT NULL,
9+
oauth_expiry timestamp with time zone DEFAULT '0001-01-01 00:00:00+00'::timestamp with time zone NOT NULL,
10+
PRIMARY KEY(user_id, login_type),
11+
FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE
12+
);
13+
14+
-- This migrates columns on api_keys to the new user_links table.
15+
-- It does this by finding all the API keys for each user, choosing
16+
-- the most recently updated for each one and then assigning its relevant
17+
-- values to the user_links table.
18+
-- A user should at most have a row for an OIDC account and a Github account.
19+
-- 'password' login types are ignored.
20+
21+
INSERT INTO user_links
22+
(
23+
user_id,
24+
login_type,
25+
linked_id,
26+
oauth_access_token,
27+
oauth_refresh_token,
28+
oauth_expiry
29+
)
30+
SELECT
31+
keys.user_id,
32+
keys.login_type,
33+
'',
34+
keys.oauth_access_token,
35+
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,
41+
api_keys.* FROM api_keys
42+
) as keys
43+
WHERE x=1 AND keys.login_type != 'password';
44+
45+
-- Drop columns that have been migrated to user_links.
46+
-- It appears the 'oauth_id_token' was unused and so it has
47+
-- been dropped here as well to avoid future confusion.
48+
ALTER TABLE api_keys
49+
DROP COLUMN oauth_access_token,
50+
DROP COLUMN oauth_refresh_token,
51+
DROP COLUMN oauth_id_token,
52+
DROP COLUMN oauth_expiry;
53+
54+
ALTER TABLE users ADD COLUMN login_type login_type NOT NULL DEFAULT 'password';
55+
56+
UPDATE
57+
users
58+
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+
)
69+
FROM
70+
user_links
71+
WHERE
72+
user_links.user_id = users.id;
73+
74+
COMMIT;

‎coderd/database/models.go

Lines changed: 20 additions & 14 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/querier.go

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)