Skip to content

WIP sekrit project #10610

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions app/components/owners-list.hbs
Original file line number Diff line number Diff line change
@@ -7,12 +7,12 @@
<li local-class="{{if (eq owner.kind "team") "team"}}">
<LinkTo
@route={{owner.kind}}
@model={{owner.login}}
@model={{owner.username}}
local-class="link"
data-test-owner-link={{owner.login}}
data-test-owner-link={{owner.username}}
>
<UserAvatar @user={{owner}} @size="medium-small" local-class="avatar" aria-hidden="true" />
<span local-class="name {{unless this.showDetailedList "hidden-name"}}">{{or owner.display_name owner.name owner.login}}</span>
<span local-class="name {{unless this.showDetailedList "hidden-name"}}">{{or owner.display_name owner.name owner.username}}</span>
</LinkTo>
</li>
{{/each}}
4 changes: 2 additions & 2 deletions app/components/pending-owner-invite-row.hbs
Original file line number Diff line number Diff line change
@@ -19,8 +19,8 @@
</div>
<div>
Invited by:
<LinkTo @route="user" @model={{@invite.inviter.login}} data-test-inviter-link>
{{@invite.inviter.login}}
<LinkTo @route="user" @model={{@invite.inviter.username}} data-test-inviter-link>
{{@invite.inviter.username}}
</LinkTo>
</div>
<div local-class="date-column" data-test-date>
4 changes: 2 additions & 2 deletions app/components/user-avatar.js
Original file line number Diff line number Diff line change
@@ -13,8 +13,8 @@ export default class UserAvatar extends Component {

get alt() {
return this.args.user.name === null
? `(${this.args.user.login})`
: `${this.args.user.name} (${this.args.user.login})`;
? `(${this.args.user.username})`
: `${this.args.user.name} (${this.args.user.username})`;
}

get title() {
2 changes: 1 addition & 1 deletion app/components/user-link.hbs
Original file line number Diff line number Diff line change
@@ -1 +1 @@
<a href={{@user.url}} title={{@user.login}} ...attributes>{{yield}}</a>
<a href={{@user.url}} title={{@user.username}} ...attributes>{{yield}}</a>
4 changes: 2 additions & 2 deletions app/components/version-list/row.hbs
Original file line number Diff line number Diff line change
@@ -40,9 +40,9 @@
{{#if @version.published_by}}
<span local-class="publisher">
by
<LinkTo @route="user" @model={{@version.published_by.login}}>
<LinkTo @route="user" @model={{@version.published_by.username}}>
<UserAvatar @user={{@version.published_by}} local-class="avatar" />
{{or @version.published_by.name @version.published_by.login}}
{{or @version.published_by.name @version.published_by.username}}
</LinkTo>
</span>
{{/if}}
6 changes: 3 additions & 3 deletions app/controllers/crate/settings.js
Original file line number Diff line number Diff line change
@@ -32,19 +32,19 @@ export default class CrateSettingsController extends Controller {

removeOwnerTask = task(async owner => {
try {
await this.crate.removeOwner(owner.get('login'));
await this.crate.removeOwner(owner.get('username'));

if (owner.kind === 'team') {
this.notifications.success(`Team ${owner.get('display_name')} removed as crate owner`);
let owner_team = await this.crate.owner_team;
removeOwner(owner_team, owner);
} else {
this.notifications.success(`User ${owner.get('login')} removed as crate owner`);
this.notifications.success(`User ${owner.get('username')} removed as crate owner`);
let owner_user = await this.crate.owner_user;
removeOwner(owner_user, owner);
}
} catch (error) {
let subject = owner.kind === 'team' ? `team ${owner.get('display_name')}` : `user ${owner.get('login')}`;
let subject = owner.kind === 'team' ? `team ${owner.get('display_name')}` : `user ${owner.get('username')}`;
let message = `Failed to remove the ${subject} as crate owner`;

let detail = error.errors?.[0]?.detail;
7 changes: 4 additions & 3 deletions app/models/team.js
Original file line number Diff line number Diff line change
@@ -4,15 +4,16 @@ export default class Team extends Model {
@attr email;
@attr name;
@attr login;
@attr username;
@attr api_token;
@attr avatar;
@attr url;
@attr kind;

get org_name() {
let login = this.login;
let login_split = login.split(':');
return login_split[1];
let username = this.username;
let username_split = username.split(':');
return username_split[1];
}

get display_name() {
1 change: 1 addition & 0 deletions app/models/user.js
Original file line number Diff line number Diff line change
@@ -13,6 +13,7 @@ export default class User extends Model {
@attr name;
@attr is_admin;
@attr login;
@attr username;
@attr avatar;
@attr url;
@attr kind;
2 changes: 1 addition & 1 deletion app/services/session.js
Original file line number Diff line number Diff line change
@@ -194,7 +194,7 @@ export default class SessionService extends Service {
}

let currentUser = this.store.push(this.store.normalize('user', response.user));
debug(`User found: ${currentUser.login}`);
debug(`User found: ${currentUser.username}`);
let ownedCrates = response.owned_crates.map(c => this.store.push(this.store.normalize('owned-crate', c)));

let { id } = currentUser;
14 changes: 7 additions & 7 deletions app/templates/crate/settings.hbs
Original file line number Diff line number Diff line change
@@ -18,11 +18,11 @@

<div local-class='list' data-test-owners>
{{#each this.crate.owner_team as |team|}}
<div local-class='row' data-test-owner-team={{team.login}}>
<LinkTo @route={{team.kind}} @model={{team.login}}>
<div local-class='row' data-test-owner-team={{team.username}}>
<LinkTo @route={{team.kind}} @model={{team.username}}>
<UserAvatar @user={{team}} @size="medium-small" />
</LinkTo>
<LinkTo @route={{team.kind}} @model={{team.login}}>
<LinkTo @route={{team.kind}} @model={{team.username}}>
{{team.display_name}}
</LinkTo>
<div local-class="email-column">
@@ -32,15 +32,15 @@
</div>
{{/each}}
{{#each this.crate.owner_user as |user|}}
<div local-class='row' data-test-owner-user={{user.login}}>
<LinkTo @route={{user.kind}} @model={{user.login}}>
<div local-class='row' data-test-owner-user={{user.username}}>
<LinkTo @route={{user.kind}} @model={{user.username}}>
<UserAvatar @user={{user}} @size="medium-small" />
</LinkTo>
<LinkTo @route={{user.kind}} @model={{user.login}}>
<LinkTo @route={{user.kind}} @model={{user.username}}>
{{#if user.name}}
{{user.name}}
{{else}}
{{user.login}}
{{user.username}}
{{/if}}
</LinkTo>
<div local-class="email-column">
2 changes: 1 addition & 1 deletion app/templates/settings/profile.hbs
Original file line number Diff line number Diff line change
@@ -13,7 +13,7 @@
<dt>Name</dt>
<dd>{{ this.model.user.name }}</dd>
<dt>GitHub Account</dt>
<dd>{{ this.model.user.login }}</dd>
<dd>{{ this.model.user.username }}</dd>
</dl>
</div>

2 changes: 1 addition & 1 deletion app/templates/user.hbs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<PageHeader local-class="header" data-test-heading>
<UserAvatar @user={{this.model.user}} @size="medium" data-test-avatar />
<h1 data-test-username>
{{ this.model.user.login }}
{{ this.model.user.username }}
</h1>
<UserLink @user={{this.model.user}} local-class="github-link" data-test-user-link>
{{svg-jar "github" alt="GitHub profile"}}
2 changes: 1 addition & 1 deletion crates/crates_io_database/src/models/mod.rs
Original file line number Diff line number Diff line change
@@ -14,7 +14,7 @@ pub use self::krate::{Crate, CrateName, NewCrate, RecentCrateDownloads};
pub use self::owner::{CrateOwner, Owner, OwnerKind};
pub use self::team::{NewTeam, Team};
pub use self::token::ApiToken;
pub use self::user::{NewUser, User};
pub use self::user::{AccountProvider, LinkedAccount, NewLinkedAccount, NewUser, User};
pub use self::version::{NewVersion, TopVersions, Version};

pub mod helpers;
111 changes: 109 additions & 2 deletions crates/crates_io_database/src/models/user.rs
Original file line number Diff line number Diff line change
@@ -8,8 +8,10 @@ use diesel_async::{AsyncPgConnection, RunQueryDsl};
use secrecy::SecretString;

use crate::models::{Crate, CrateOwner, Email, Owner, OwnerKind};
use crate::schema::{crate_owners, emails, users};
use crates_io_diesel_helpers::lower;
use crate::schema::{crate_owners, emails, linked_accounts, users};
use crates_io_diesel_helpers::{lower, pg_enum};

use std::fmt::{Display, Formatter};

/// The model representing a row in the `users` database table.
#[derive(Clone, Debug, Queryable, Identifiable, Selectable)]
@@ -25,6 +27,7 @@ pub struct User {
pub account_lock_until: Option<DateTime<Utc>>,
pub is_admin: bool,
pub publish_notifications: bool,
pub username: Option<String>,
}

impl User {
@@ -76,6 +79,17 @@ impl User {
.await
.optional()
}

/// Queries for the linked accounts belonging to a particular user
pub async fn linked_accounts(
&self,
conn: &mut AsyncPgConnection,
) -> QueryResult<Vec<LinkedAccount>> {
LinkedAccount::belonging_to(self)
.select(LinkedAccount::as_select())
.load(conn)
.await
}
}

/// Represents a new user record insertable to the `users` table
@@ -85,6 +99,7 @@ pub struct NewUser<'a> {
pub gh_id: i32,
pub gh_login: &'a str,
pub name: Option<&'a str>,
pub username: Option<&'a str>,
pub gh_avatar: Option<&'a str>,
pub gh_access_token: &'a str,
}
@@ -114,6 +129,7 @@ impl NewUser<'_> {
.do_update()
.set((
users::gh_login.eq(excluded(users::gh_login)),
users::username.eq(excluded(users::username)),
users::name.eq(excluded(users::name)),
users::gh_avatar.eq(excluded(users::gh_avatar)),
users::gh_access_token.eq(excluded(users::gh_access_token)),
@@ -122,3 +138,94 @@ impl NewUser<'_> {
.await
}
}

// Supported OAuth providers. Currently only GitHub.
pg_enum! {
pub enum AccountProvider {
Github = 0,
}
}

impl Display for AccountProvider {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
Self::Github => write!(f, "GitHub"),
}
}
}

impl AccountProvider {
pub fn url(&self, login: &str) -> String {
match self {
Self::Github => format!("https://github.com/{login}"),
}
}
}

/// Represents an OAuth account record linked to a user record.
#[derive(Associations, Identifiable, Selectable, Queryable, Debug, Clone)]
#[diesel(
table_name = linked_accounts,
check_for_backend(diesel::pg::Pg),
primary_key(provider, account_id),
belongs_to(User),
)]
pub struct LinkedAccount {
pub user_id: i32,
pub provider: AccountProvider,
pub account_id: i32, // corresponds to user.gh_id
#[diesel(deserialize_as = String)]
pub access_token: SecretString, // corresponds to user.gh_access_token
pub login: String, // corresponds to user.gh_login
pub avatar: Option<String>, // corresponds to user.gh_avatar
}

/// Represents a new linked account record insertable to the `linked_accounts` table
#[derive(Insertable, Debug, Builder)]
#[diesel(
table_name = linked_accounts,
check_for_backend(diesel::pg::Pg),
primary_key(provider, account_id),
belongs_to(User),
)]
pub struct NewLinkedAccount<'a> {
pub user_id: i32,
pub provider: AccountProvider,
pub account_id: i32, // corresponds to user.gh_id
pub access_token: &'a str, // corresponds to user.gh_access_token
pub login: &'a str, // corresponds to user.gh_login
pub avatar: Option<&'a str>, // corresponds to user.gh_avatar
}

impl NewLinkedAccount<'_> {
/// Inserts the linked account into the database, or updates an existing one.
///
/// This is to be used for logging in when there is no currently logged-in user, as opposed to
/// adding another linked account to a currently-logged-in user. The logic for adding another
/// linked account (when that ability gets added) will need to ensure that a particular
/// (provider, account_id) combo (ex: GitHub account with GitHub ID 1234) is only associated
/// with one crates.io account, so that we know what crates.io account to log in when we get an
/// oAuth request from GitHub ID 1234. In other words, we should NOT be updating the user_id on
/// an existing (provider, account_id) row when starting from a currently-logged-in crates.io \
/// user because that would mean that oAuth account has already been associated with a
/// different crates.io account.
///
/// This function should be called if there is no current user and should update, say, the
/// access token, login, or avatar if those have changed.
pub async fn insert_or_update(
&self,
conn: &mut AsyncPgConnection,
) -> QueryResult<LinkedAccount> {
diesel::insert_into(linked_accounts::table)
.values(self)
.on_conflict((linked_accounts::provider, linked_accounts::account_id))
.do_update()
.set((
linked_accounts::access_token.eq(excluded(linked_accounts::access_token)),
linked_accounts::login.eq(excluded(linked_accounts::login)),
linked_accounts::avatar.eq(excluded(linked_accounts::avatar)),
))
.get_result(conn)
.await
}
}
Loading