Skip to content

Commit 6c9d8ff

Browse files
committed
Duplicate GitHub OAuth info to a linked_accounts table (deploy 1)
That has a `provider` column that will (for now) always be set to 0, which corresponds to `AccountProvider::Github`. The table's primary key is (provider, account_id), which corresponds to (0, gh_id). This constraint will mean a particular GitHub/GitLab/etc account, identified from the provider by an ID, may only be associated with one crates.io user record, but a crates.io user record could (eventually) have *both* a GitHub *and* a GitLab account associated with it (or two GitHub accounts, even!) This is the first step of many to eventually allow for crates.io accounts linked to other OAuth providers in addition/instead of GitHub. No code aside from one test is reading from the linked accounts table at this time. No backfill has been done yet. No handling of creating/associating multiple OAuth accounts with one crates.io account has been done yet.
1 parent c517934 commit 6c9d8ff

File tree

11 files changed

+217
-5
lines changed

11 files changed

+217
-5
lines changed

crates/crates_io_database/src/models/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ pub use self::krate::{Crate, CrateName, NewCrate, RecentCrateDownloads};
1414
pub use self::owner::{CrateOwner, Owner, OwnerKind};
1515
pub use self::team::{NewTeam, Team};
1616
pub use self::token::ApiToken;
17-
pub use self::user::{NewUser, User};
17+
pub use self::user::{AccountProvider, LinkedAccount, NewLinkedAccount, NewUser, User};
1818
pub use self::version::{NewVersion, TopVersions, Version};
1919

2020
pub mod helpers;

crates/crates_io_database/src/models/user.rs

+77-2
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ use diesel_async::{AsyncPgConnection, RunQueryDsl};
88
use secrecy::SecretString;
99

1010
use crate::models::{Crate, CrateOwner, Email, Owner, OwnerKind};
11-
use crate::schema::{crate_owners, emails, users};
12-
use crates_io_diesel_helpers::lower;
11+
use crate::schema::{crate_owners, emails, linked_accounts, users};
12+
use crates_io_diesel_helpers::{lower, pg_enum};
1313

1414
/// The model representing a row in the `users` database table.
1515
#[derive(Clone, Debug, Queryable, Identifiable, Selectable)]
@@ -122,3 +122,78 @@ impl NewUser<'_> {
122122
.await
123123
}
124124
}
125+
126+
// Supported OAuth providers. Currently only GitHub.
127+
pg_enum! {
128+
pub enum AccountProvider {
129+
Github = 0,
130+
}
131+
}
132+
133+
/// Represents an OAuth account record linked to a user record.
134+
#[derive(Associations, Identifiable, Selectable, Queryable, Debug, Clone)]
135+
#[diesel(
136+
table_name = linked_accounts,
137+
check_for_backend(diesel::pg::Pg),
138+
primary_key(provider, account_id),
139+
belongs_to(User),
140+
)]
141+
pub struct LinkedAccount {
142+
pub user_id: i32,
143+
pub provider: AccountProvider,
144+
pub account_id: i32, // corresponds to user.gh_id
145+
#[diesel(deserialize_as = String)]
146+
pub access_token: SecretString, // corresponds to user.gh_access_token
147+
pub login: String, // corresponds to user.gh_login
148+
pub avatar: Option<String>, // corresponds to user.gh_avatar
149+
}
150+
151+
/// Represents a new linked account record insertable to the `linked_accounts` table
152+
#[derive(Insertable, Debug, Builder)]
153+
#[diesel(
154+
table_name = linked_accounts,
155+
check_for_backend(diesel::pg::Pg),
156+
primary_key(provider, account_id),
157+
belongs_to(User),
158+
)]
159+
pub struct NewLinkedAccount<'a> {
160+
pub user_id: i32,
161+
pub provider: AccountProvider,
162+
pub account_id: i32, // corresponds to user.gh_id
163+
pub access_token: &'a str, // corresponds to user.gh_access_token
164+
pub login: &'a str, // corresponds to user.gh_login
165+
pub avatar: Option<&'a str>, // corresponds to user.gh_avatar
166+
}
167+
168+
impl NewLinkedAccount<'_> {
169+
/// Inserts the linked account into the database, or updates an existing one.
170+
///
171+
/// This is to be used for logging in when there is no currently logged-in user, as opposed to
172+
/// adding another linked account to a currently-logged-in user. The logic for adding another
173+
/// linked account (when that ability gets added) will need to ensure that a particular
174+
/// (provider, account_id) combo (ex: GitHub account with GitHub ID 1234) is only associated
175+
/// with one crates.io account, so that we know what crates.io account to log in when we get an
176+
/// oAuth request from GitHub ID 1234. In other words, we should NOT be updating the user_id on
177+
/// an existing (provider, account_id) row when starting from a currently-logged-in crates.io \
178+
/// user because that would mean that oAuth account has already been associated with a
179+
/// different crates.io account.
180+
///
181+
/// This function should be called if there is no current user and should update, say, the
182+
/// access token, login, or avatar if those have changed.
183+
pub async fn insert_or_update(
184+
&self,
185+
conn: &mut AsyncPgConnection,
186+
) -> QueryResult<LinkedAccount> {
187+
diesel::insert_into(linked_accounts::table)
188+
.values(self)
189+
.on_conflict((linked_accounts::provider, linked_accounts::account_id))
190+
.do_update()
191+
.set((
192+
linked_accounts::access_token.eq(excluded(linked_accounts::access_token)),
193+
linked_accounts::login.eq(excluded(linked_accounts::login)),
194+
linked_accounts::avatar.eq(excluded(linked_accounts::avatar)),
195+
))
196+
.get_result(conn)
197+
.await
198+
}
199+
}

crates/crates_io_database/src/schema.rs

+46
Original file line numberDiff line numberDiff line change
@@ -593,6 +593,50 @@ diesel::table! {
593593
}
594594
}
595595

596+
diesel::table! {
597+
/// Representation of the `linked_accounts` table.
598+
///
599+
/// (Automatically generated by Diesel.)
600+
linked_accounts (provider, account_id) {
601+
/// The `user_id` column of the `linked_accounts` table.
602+
///
603+
/// Its SQL type is `Int4`.
604+
///
605+
/// (Automatically generated by Diesel.)
606+
user_id -> Int4,
607+
/// The `provider` column of the `linked_accounts` table.
608+
///
609+
/// Its SQL type is `Int4`.
610+
///
611+
/// (Automatically generated by Diesel.)
612+
provider -> Int4,
613+
/// The `account_id` column of the `linked_accounts` table.
614+
///
615+
/// Its SQL type is `Int4`.
616+
///
617+
/// (Automatically generated by Diesel.)
618+
account_id -> Int4,
619+
/// The `access_token` column of the `linked_accounts` table.
620+
///
621+
/// Its SQL type is `Varchar`.
622+
///
623+
/// (Automatically generated by Diesel.)
624+
access_token -> Varchar,
625+
/// The `login` column of the `linked_accounts` table.
626+
///
627+
/// Its SQL type is `Varchar`.
628+
///
629+
/// (Automatically generated by Diesel.)
630+
login -> Varchar,
631+
/// The `avatar` column of the `linked_accounts` table.
632+
///
633+
/// Its SQL type is `Nullable<Varchar>`.
634+
///
635+
/// (Automatically generated by Diesel.)
636+
avatar -> Nullable<Varchar>,
637+
}
638+
}
639+
596640
diesel::table! {
597641
/// Representation of the `metadata` table.
598642
///
@@ -1064,6 +1108,7 @@ diesel::joinable!(dependencies -> versions (version_id));
10641108
diesel::joinable!(emails -> users (user_id));
10651109
diesel::joinable!(follows -> crates (crate_id));
10661110
diesel::joinable!(follows -> users (user_id));
1111+
diesel::joinable!(linked_accounts -> users (user_id));
10671112
diesel::joinable!(publish_limit_buckets -> users (user_id));
10681113
diesel::joinable!(publish_rate_overrides -> users (user_id));
10691114
diesel::joinable!(readme_renderings -> versions (version_id));
@@ -1092,6 +1137,7 @@ diesel::allow_tables_to_appear_in_same_query!(
10921137
emails,
10931138
follows,
10941139
keywords,
1140+
linked_accounts,
10951141
metadata,
10961142
processed_log_files,
10971143
publish_limit_buckets,

crates/crates_io_database_dump/src/dump-db.toml

+10
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,16 @@ keyword = "public"
154154
crates_cnt = "public"
155155
created_at = "public"
156156

157+
[linked_accounts.columns]
158+
user_id = "public"
159+
provider = "public"
160+
account_id = "public"
161+
access_token = "private"
162+
login = "public"
163+
avatar = "public"
164+
[linked_accounts.column_defaults]
165+
access_token = "''"
166+
157167
[metadata.columns]
158168
total_downloads = "public"
159169

crates/crates_io_database_dump/src/snapshots/[email protected]

+2
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
---
22
source: crates/crates_io_database_dump/src/lib.rs
33
expression: content
4+
snapshot_kind: text
45
---
56
BEGIN ISOLATION LEVEL REPEATABLE READ, READ ONLY;
67

78
\copy "categories" ("category", "crates_cnt", "created_at", "description", "id", "path", "slug") TO 'data/categories.csv' WITH CSV HEADER
89
\copy "crate_downloads" ("crate_id", "downloads") TO 'data/crate_downloads.csv' WITH CSV HEADER
910
\copy "crates" ("created_at", "description", "documentation", "homepage", "id", "max_features", "max_upload_size", "name", "readme", "repository", "updated_at") TO 'data/crates.csv' WITH CSV HEADER
1011
\copy "keywords" ("crates_cnt", "created_at", "id", "keyword") TO 'data/keywords.csv' WITH CSV HEADER
12+
\copy "linked_accounts" ("account_id", "avatar", "login", "provider", "user_id") TO 'data/linked_accounts.csv' WITH CSV HEADER
1113
\copy "metadata" ("total_downloads") TO 'data/metadata.csv' WITH CSV HEADER
1214
\copy "reserved_crate_names" ("name") TO 'data/reserved_crate_names.csv' WITH CSV HEADER
1315
\copy "teams" ("avatar", "github_id", "id", "login", "name", "org_id") TO 'data/teams.csv' WITH CSV HEADER

crates/crates_io_database_dump/src/snapshots/[email protected]

+7
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
---
22
source: crates/crates_io_database_dump/src/lib.rs
33
expression: content
4+
snapshot_kind: text
45
---
56
BEGIN;
67
-- Disable triggers on each table.
@@ -9,6 +10,7 @@ BEGIN;
910
ALTER TABLE "crate_downloads" DISABLE TRIGGER ALL;
1011
ALTER TABLE "crates" DISABLE TRIGGER ALL;
1112
ALTER TABLE "keywords" DISABLE TRIGGER ALL;
13+
ALTER TABLE "linked_accounts" DISABLE TRIGGER ALL;
1214
ALTER TABLE "metadata" DISABLE TRIGGER ALL;
1315
ALTER TABLE "reserved_crate_names" DISABLE TRIGGER ALL;
1416
ALTER TABLE "teams" DISABLE TRIGGER ALL;
@@ -23,6 +25,7 @@ BEGIN;
2325

2426
-- Set defaults for non-nullable columns not included in the dump.
2527

28+
ALTER TABLE "linked_accounts" ALTER COLUMN "access_token" SET DEFAULT '';
2629
ALTER TABLE "users" ALTER COLUMN "gh_access_token" SET DEFAULT '';
2730

2831
-- Truncate all tables.
@@ -31,6 +34,7 @@ BEGIN;
3134
TRUNCATE "crate_downloads" RESTART IDENTITY CASCADE;
3235
TRUNCATE "crates" RESTART IDENTITY CASCADE;
3336
TRUNCATE "keywords" RESTART IDENTITY CASCADE;
37+
TRUNCATE "linked_accounts" RESTART IDENTITY CASCADE;
3438
TRUNCATE "metadata" RESTART IDENTITY CASCADE;
3539
TRUNCATE "reserved_crate_names" RESTART IDENTITY CASCADE;
3640
TRUNCATE "teams" RESTART IDENTITY CASCADE;
@@ -52,6 +56,7 @@ BEGIN;
5256
\copy "crate_downloads" ("crate_id", "downloads") FROM 'data/crate_downloads.csv' WITH CSV HEADER
5357
\copy "crates" ("created_at", "description", "documentation", "homepage", "id", "max_features", "max_upload_size", "name", "readme", "repository", "updated_at") FROM 'data/crates.csv' WITH CSV HEADER
5458
\copy "keywords" ("crates_cnt", "created_at", "id", "keyword") FROM 'data/keywords.csv' WITH CSV HEADER
59+
\copy "linked_accounts" ("account_id", "avatar", "login", "provider", "user_id") FROM 'data/linked_accounts.csv' WITH CSV HEADER
5560
\copy "metadata" ("total_downloads") FROM 'data/metadata.csv' WITH CSV HEADER
5661
\copy "reserved_crate_names" ("name") FROM 'data/reserved_crate_names.csv' WITH CSV HEADER
5762
\copy "teams" ("avatar", "github_id", "id", "login", "name", "org_id") FROM 'data/teams.csv' WITH CSV HEADER
@@ -66,6 +71,7 @@ BEGIN;
6671

6772
-- Drop the defaults again.
6873

74+
ALTER TABLE "linked_accounts" ALTER COLUMN "access_token" DROP DEFAULT;
6975
ALTER TABLE "users" ALTER COLUMN "gh_access_token" DROP DEFAULT;
7076

7177
-- Reenable triggers on each table.
@@ -74,6 +80,7 @@ BEGIN;
7480
ALTER TABLE "crate_downloads" ENABLE TRIGGER ALL;
7581
ALTER TABLE "crates" ENABLE TRIGGER ALL;
7682
ALTER TABLE "keywords" ENABLE TRIGGER ALL;
83+
ALTER TABLE "linked_accounts" ENABLE TRIGGER ALL;
7784
ALTER TABLE "metadata" ENABLE TRIGGER ALL;
7885
ALTER TABLE "reserved_crate_names" ENABLE TRIGGER ALL;
7986
ALTER TABLE "teams" ENABLE TRIGGER ALL;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
DROP TABLE linked_accounts;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
CREATE TABLE linked_accounts (
2+
user_id INTEGER NOT NULL REFERENCES users (id) ON DELETE CASCADE,
3+
provider INTEGER NOT NULL,
4+
account_id INTEGER NOT NULL,
5+
access_token VARCHAR NOT NULL,
6+
login VARCHAR NOT NULL,
7+
avatar VARCHAR,
8+
PRIMARY KEY (provider, account_id)
9+
);

src/controllers/session.rs

+16-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use crate::app::AppState;
1010
use crate::controllers::user::update::UserConfirmEmail;
1111
use crate::email::Emails;
1212
use crate::middleware::log_request::RequestLogExt;
13-
use crate::models::{NewEmail, NewUser, User};
13+
use crate::models::{AccountProvider, NewEmail, NewLinkedAccount, NewUser, User};
1414
use crate::schema::users;
1515
use crate::util::diesel::is_read_only_error;
1616
use crate::util::errors::{AppResult, bad_request, server_error};
@@ -162,6 +162,21 @@ async fn create_or_update_user(
162162
async move {
163163
let user = new_user.insert_or_update(conn).await?;
164164

165+
// To assist in eventually someday allowing OAuth with more than GitHub, also
166+
// write the GitHub info to the `linked_accounts` table. Nothing currently reads
167+
// from this table. Only log errors but don't fail login if this writing fails.
168+
let new_linked_account = NewLinkedAccount::builder()
169+
.user_id(user.id)
170+
.provider(AccountProvider::Github)
171+
.account_id(user.gh_id)
172+
.access_token(new_user.gh_access_token)
173+
.login(&user.gh_login)
174+
.maybe_avatar(user.gh_avatar.as_deref())
175+
.build();
176+
if let Err(e) = new_linked_account.insert_or_update(conn).await {
177+
info!("Error inserting or updating linked_accounts record: {e}");
178+
}
179+
165180
// To send the user an account verification email
166181
if let Some(user_email) = email {
167182
let new_email = NewEmail::builder()

src/tests/dump_db.rs

+2
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ async fn test_dump_db_job() -> anyhow::Result<()> {
5252
"YYYY-MM-DD-HHMMSS/data/crate_downloads.csv",
5353
"YYYY-MM-DD-HHMMSS/data/crates.csv",
5454
"YYYY-MM-DD-HHMMSS/data/keywords.csv",
55+
"YYYY-MM-DD-HHMMSS/data/linked_accounts.csv",
5556
"YYYY-MM-DD-HHMMSS/data/metadata.csv",
5657
"YYYY-MM-DD-HHMMSS/data/reserved_crate_names.csv",
5758
"YYYY-MM-DD-HHMMSS/data/teams.csv",
@@ -84,6 +85,7 @@ async fn test_dump_db_job() -> anyhow::Result<()> {
8485
"data/crate_downloads.csv",
8586
"data/crates.csv",
8687
"data/keywords.csv",
88+
"data/linked_accounts.csv",
8789
"data/metadata.csv",
8890
"data/reserved_crate_names.csv",
8991
"data/teams.csv",

src/tests/user.rs

+46-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use crate::controllers::session;
2-
use crate::models::{ApiToken, Email, User};
2+
use crate::models::{AccountProvider, ApiToken, Email, LinkedAccount, User};
3+
use crate::schema::linked_accounts;
34
use crate::tests::TestApp;
45
use crate::tests::util::github::next_gh_id;
56
use crate::tests::util::{MockCookieUser, RequestHelper};
@@ -265,3 +266,47 @@ async fn test_existing_user_email() -> anyhow::Result<()> {
265266

266267
Ok(())
267268
}
269+
270+
// To assist in eventually someday allowing OAuth with more than GitHub, verify that we're starting
271+
// to also write the GitHub info to the `linked_accounts` table. Nothing currently reads from this
272+
// table other than this test.
273+
#[tokio::test(flavor = "multi_thread")]
274+
async fn also_write_to_linked_accounts() -> anyhow::Result<()> {
275+
let (app, _) = TestApp::init().empty().await;
276+
let mut conn = app.db_conn().await;
277+
278+
// Simulate logging in via GitHub. Don't use app.db_new_user because it inserts a user record
279+
// directly into the database and we want to test the OAuth flow here.
280+
let email = "[email protected]";
281+
282+
let emails = &app.as_inner().emails;
283+
284+
let gh_user = GithubUser {
285+
id: next_gh_id(),
286+
login: "arbitrary_username".to_string(),
287+
name: None,
288+
email: Some(email.to_string()),
289+
avatar_url: None,
290+
};
291+
292+
let u =
293+
session::save_user_to_database(&gh_user, "some random token", emails, &mut conn).await?;
294+
295+
let linked_accounts = linked_accounts::table
296+
.filter(linked_accounts::provider.eq(AccountProvider::Github))
297+
.filter(linked_accounts::account_id.eq(u.gh_id))
298+
.load::<LinkedAccount>(&mut conn)
299+
.await
300+
.unwrap();
301+
302+
assert_eq!(linked_accounts.len(), 1);
303+
let linked_account = &linked_accounts[0];
304+
assert_eq!(linked_account.user_id, u.id);
305+
assert_eq!(linked_account.login, u.gh_login);
306+
assert_eq!(
307+
linked_account.access_token.expose_secret(),
308+
u.gh_access_token.expose_secret()
309+
);
310+
311+
Ok(())
312+
}

0 commit comments

Comments
 (0)