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
Show file tree
Hide file tree
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
Expand Up @@ -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}}
Expand Down
4 changes: 2 additions & 2 deletions app/components/pending-owner-invite-row.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
4 changes: 2 additions & 2 deletions app/components/user-avatar.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
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
Expand Up @@ -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}}
Expand Down
6 changes: 3 additions & 3 deletions app/controllers/crate/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 4 additions & 3 deletions app/models/team.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
1 change: 1 addition & 0 deletions app/models/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion app/services/session.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
14 changes: 7 additions & 7 deletions app/templates/crate/settings.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand All @@ -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">
Expand Down
2 changes: 1 addition & 1 deletion app/templates/settings/profile.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand Down
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"}}
Expand Down
2 changes: 1 addition & 1 deletion crates/crates_io_database/src/models/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
111 changes: 109 additions & 2 deletions crates/crates_io_database/src/models/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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,
}
Expand Down Expand Up @@ -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)),
Expand All @@ -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
Loading