From ab4404590b2e335e33fd2021d403adaa6b83057e Mon Sep 17 00:00:00 2001 From: clayton Date: Sun, 6 Nov 2022 12:24:17 -0800 Subject: [PATCH 01/49] Clean up GithubUser model and transformer --- app/Models/GithubUser.php | 52 ++++++++++++++++------ app/Transformers/GithubUserTransformer.php | 8 ++-- 2 files changed, 44 insertions(+), 16 deletions(-) diff --git a/app/Models/GithubUser.php b/app/Models/GithubUser.php index b293e7fbf83..5fa43465bb0 100644 --- a/app/Models/GithubUser.php +++ b/app/Models/GithubUser.php @@ -3,21 +3,28 @@ // Copyright (c) ppy Pty Ltd . Licensed under the GNU Affero General Public License v3.0. // See the LICENCE file in the repository root for full licence text. +declare(strict_types=1); + namespace App\Models; +use Illuminate\Database\Eloquent\Relations\BelongsTo; +use Illuminate\Database\Eloquent\Relations\HasMany; + /** * @property int|null $canonical_id - * @property \Illuminate\Database\Eloquent\Collection $changelogEntries ChangelogEntry + * @property-read \Illuminate\Database\Eloquent\Collection $changelogEntries * @property \Carbon\Carbon|null $created_at + * @property-read string|null $created_at_json * @property int $id * @property \Carbon\Carbon|null $updated_at - * @property User $user + * @property-read string|null $updated_at_json + * @property-read User|null $user * @property int|null $user_id * @property string|null $username */ class GithubUser extends Model { - public static function importFromGithub($data) + public static function importFromGithub(array $data): static { $githubUser = static::where('canonical_id', '=', $data['id'])->first(); @@ -39,39 +46,58 @@ public static function importFromGithub($data) return $githubUser; } - public function user() + public function changelogEntries(): HasMany { - return $this->belongsTo(User::class, 'user_id'); + return $this->hasMany(ChangelogEntry::class); } - public function changelogEntries() + public function user(): BelongsTo { - return $this->hasMany(ChangelogEntry::class); + return $this->belongsTo(User::class, 'user_id'); } - public function displayName() + public function displayName(): string { return presence($this->username) - ?? optional($this->user)->username + ?? $this->osuUsername() ?? '[no name]'; } - public function githubUrl() + public function githubUrl(): ?string { if (present($this->username)) { return "https://github.com/{$this->username}"; } } - public function userUrl() + public function osuUsername(): ?string + { + return $this->user?->username; + } + + public function userUrl(): ?string { if ($this->user_id !== null) { return route('users.show', $this->user_id); } } - public function url() + public function getAttribute($key) { - return $this->githubUrl() ?? $this->userUrl(); + return match ($key) { + 'canonical_id', + 'id', + 'user_id', + 'username' => $this->getRawAttribute($key), + + 'created_at', + 'updated_at' => $this->getTimeFast($key), + + 'created_at_json', + 'updated_at_json' => $this->getJsonTimeFast($key), + + 'changelogEntries', + 'user' => $this->getRelationValue($key), + }; } } diff --git a/app/Transformers/GithubUserTransformer.php b/app/Transformers/GithubUserTransformer.php index 14470631888..291743f891a 100644 --- a/app/Transformers/GithubUserTransformer.php +++ b/app/Transformers/GithubUserTransformer.php @@ -3,19 +3,21 @@ // Copyright (c) ppy Pty Ltd . Licensed under the GNU Affero General Public License v3.0. // See the LICENCE file in the repository root for full licence text. +declare(strict_types=1); + namespace App\Transformers; use App\Models\GithubUser; class GithubUserTransformer extends TransformerAbstract { - public function transform(GithubUser $githubUser) + public function transform(GithubUser $githubUser): array { return [ - 'id' => $githubUser->getKey(), 'display_name' => $githubUser->displayName(), 'github_url' => $githubUser->githubUrl(), - 'osu_username' => optional($githubUser->user)->username, + 'id' => $githubUser->getKey(), + 'osu_username' => $githubUser->osuUsername(), 'user_id' => $githubUser->user_id, 'user_url' => $githubUser->userUrl(), ]; From d478ea9078a9c803a40a533cb118e63bdd821fb8 Mon Sep 17 00:00:00 2001 From: clayton Date: Sun, 6 Nov 2022 12:24:18 -0800 Subject: [PATCH 02/49] Add User associate option to `GithubUser::importFromGithub()` --- app/Models/GithubUser.php | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/app/Models/GithubUser.php b/app/Models/GithubUser.php index 5fa43465bb0..764206d569d 100644 --- a/app/Models/GithubUser.php +++ b/app/Models/GithubUser.php @@ -24,26 +24,25 @@ */ class GithubUser extends Model { - public static function importFromGithub(array $data): static + public static function importFromGithub(array $apiUser, ?User $user = null): static { - $githubUser = static::where('canonical_id', '=', $data['id'])->first(); + $params = [ + 'canonical_id' => $apiUser['id'], + 'username' => $apiUser['login'], + ]; + if ($user !== null) { + $params['user_id'] = $user->getKey(); + } + + $githubUser = static::where('canonical_id', $params['canonical_id'])->first() + ?? static::where('username', $params['username'])->last(); - if (isset($githubUser)) { - $githubUser->update(['username' => $data['login']]); + if ($githubUser === null) { + return static::create($params); } else { - $githubUser = static::where('username', '=', $data['login'])->last(); - - if (isset($githubUser)) { - $githubUser->update(['canonical_id' => $data['id']]); - } else { - $githubUser = static::create([ - 'canonical_id' => $data['id'], - 'username' => $data['login'], - ]); - } + $githubUser->update($params); + return $githubUser; } - - return $githubUser; } public function changelogEntries(): HasMany From 72dd9ce34e7c017c1861fd29e125433cb8520bd2 Mon Sep 17 00:00:00 2001 From: clayton Date: Sun, 6 Nov 2022 12:24:18 -0800 Subject: [PATCH 03/49] Add endpoints to link and unlink GitHub accounts from users --- .env.example | 4 + .../Account/GithubUsersController.php | 100 ++++++++++++++++++ config/osu.php | 4 + routes/web.php | 3 + 4 files changed, 111 insertions(+) create mode 100644 app/Http/Controllers/Account/GithubUsersController.php diff --git a/.env.example b/.env.example index ab6adaadaee..197e21ac5ca 100644 --- a/.env.example +++ b/.env.example @@ -104,6 +104,10 @@ PUSHER_SECRET= # GITHUB_TOKEN= +# GitHub client for users to associate their GitHub accounts +# GITHUB_CLIENT_ID= +# GITHUB_CLIENT_SECRET= + # DATADOG_ENABLED=true # DATADOG_PREFIX=osu.web # DATADOG_API_KEY= diff --git a/app/Http/Controllers/Account/GithubUsersController.php b/app/Http/Controllers/Account/GithubUsersController.php new file mode 100644 index 00000000000..b09d30d940b --- /dev/null +++ b/app/Http/Controllers/Account/GithubUsersController.php @@ -0,0 +1,100 @@ +. Licensed under the GNU Affero General Public License v3.0. +// See the LICENCE file in the repository root for full licence text. + +declare(strict_types=1); + +namespace App\Http\Controllers\Account; + +use App\Http\Controllers\Controller; +use App\Models\GithubUser; +use Github\Client as GithubClient; +use GuzzleHttp\Client as HttpClient; + +class GithubUsersController extends Controller +{ + public function __construct() + { + $this->middleware('auth'); + $this->middleware('verify-user'); + + parent::__construct(); + } + + public function callback() + { + $params = get_params(request()->all(), null, [ + 'code:string', + 'state:string', + ]); + + abort_unless(isset($params['code']), 422, 'Invalid code.'); + abort_unless( + isset($params['state']) && $params['state'] === session()->pull('github_auth_state'), + 403, + 'Invalid state.', + ); + + $tokenResponseBody = (new HttpClient()) + ->request('POST', 'https://github.com/login/oauth/access_token', [ + 'query' => [ + 'client_id' => config('osu.github.client_id'), + 'client_secret' => config('osu.github.client_secret'), + 'code' => $params['code'], + ], + ]) + ->getBody() + ->getContents(); + parse_str($tokenResponseBody, $tokenResponseBodyParams); + $token = $tokenResponseBodyParams['access_token'] ?? null; + + abort_if($token === null, 500, 'Invalid response from GitHub API.'); + + $githubClient = new GithubClient(); + $githubClient->authenticate($token, null, GithubClient::AUTH_ACCESS_TOKEN); + $githubApiUser = $githubClient->currentUser()->show(); + + GithubUser::importFromGithub($githubApiUser, auth()->user()); + + return redirect(route('account.edit').'#github'); + } + + public function create() + { + abort_if( + config('osu.github.client_id') === null || config('osu.github.client_secret') === null, + 404, + ); + abort_if( + auth()->user()->githubUsers()->count() >= 10, + 403, + 'Too many GitHub accounts.', + ); + + $state = bin2hex(random_bytes(32)); + session()->put('github_auth_state', $state); + + return redirect('https://github.com/login/oauth/authorize?'.http_build_query([ + 'allow_signup' => 'false', + 'client_id' => config('osu.github.client_id'), + 'scope' => '', + 'state' => $state, + ])); + } + + public function destroy(int $id) + { + $githubUser = auth()->user()->githubUsers()->findOrFail($id); + + abort_if( + $githubUser->canonical_id === null || $githubUser->username === null, + 422, + 'Cannot unassociate user from GitHub user without a valid ID or username.', + ); + + $githubUser->update(['user_id' => null]); + + return response(null, 204); + } +} diff --git a/config/osu.php b/config/osu.php index cfd52460fb0..2f3c33c7e50 100644 --- a/config/osu.php +++ b/config/osu.php @@ -123,6 +123,10 @@ 'git-sha' => presence(env('GIT_SHA')) ?? (file_exists(__DIR__.'/../version') ? trim(file_get_contents(__DIR__.'/../version')) : null) ?? 'unknown-version', + 'github' => [ + 'client_id' => presence(env('GITHUB_CLIENT_ID')), + 'client_secret' => presence(env('GITHUB_CLIENT_SECRET')), + ], 'is_development_deploy' => get_bool(env('IS_DEVELOPMENT_DEPLOY')) ?? true, 'landing' => [ 'video_url' => env('LANDING_VIDEO_URL', 'https://assets.ppy.sh/media/landing.mp4'), diff --git a/routes/web.php b/routes/web.php index 70def786b13..ec1bd4d906a 100644 --- a/routes/web.php +++ b/routes/web.php @@ -214,6 +214,9 @@ Route::get('verify', 'AccountController@verifyLink'); Route::post('verify', 'AccountController@verify')->name('verify'); Route::put('/', 'AccountController@update')->name('update'); + + Route::get('github-users/callback', 'Account\GithubUsersController@callback'); + Route::resource('github-users', 'Account\GithubUsersController', ['only' => ['create', 'destroy']]); }); Route::get('quick-search', 'HomeController@quickSearch')->name('quick-search'); From 63178a8cf4c28967886c833d4218e3b9a22631d3 Mon Sep 17 00:00:00 2001 From: clayton Date: Sun, 6 Nov 2022 12:24:18 -0800 Subject: [PATCH 04/49] Add GitHub account options to account edit page --- app/Http/Controllers/AccountController.php | 9 +++ app/Transformers/GithubUserTransformer.php | 1 + resources/assets/less/bem-index.less | 1 + resources/assets/less/bem/github-user.less | 15 +++++ .../assets/lib/account-edit/github-user.tsx | 64 +++++++++++++++++++ .../assets/lib/account-edit/github-users.tsx | 53 +++++++++++++++ .../assets/lib/entrypoints/account-edit.tsx | 7 +- .../assets/lib/interfaces/github-user-json.ts | 17 +++++ resources/lang/en/accounts.php | 7 ++ .../accounts/_edit_github_users.blade.php | 24 +++++++ resources/views/accounts/edit.blade.php | 8 +++ 11 files changed, 205 insertions(+), 1 deletion(-) create mode 100644 resources/assets/less/bem/github-user.less create mode 100644 resources/assets/lib/account-edit/github-user.tsx create mode 100644 resources/assets/lib/account-edit/github-users.tsx create mode 100644 resources/assets/lib/interfaces/github-user-json.ts create mode 100644 resources/views/accounts/_edit_github_users.blade.php diff --git a/app/Http/Controllers/AccountController.php b/app/Http/Controllers/AccountController.php index 27635db31e0..37184dddc96 100644 --- a/app/Http/Controllers/AccountController.php +++ b/app/Http/Controllers/AccountController.php @@ -113,10 +113,19 @@ public function edit() $notificationOptions = $user->notificationOptions->keyBy('name'); + $githubUsers = json_collection( + $user + ->githubUsers() + ->whereNotNull(['canonical_id', 'username']) + ->get(), + 'GithubUser', + ); + return ext_view('accounts.edit', compact( 'authorizedClients', 'blocks', 'currentSessionId', + 'githubUsers', 'notificationOptions', 'ownClients', 'sessions' diff --git a/app/Transformers/GithubUserTransformer.php b/app/Transformers/GithubUserTransformer.php index 291743f891a..cb8bec41606 100644 --- a/app/Transformers/GithubUserTransformer.php +++ b/app/Transformers/GithubUserTransformer.php @@ -16,6 +16,7 @@ public function transform(GithubUser $githubUser): array return [ 'display_name' => $githubUser->displayName(), 'github_url' => $githubUser->githubUrl(), + 'github_username' => $githubUser->username, 'id' => $githubUser->getKey(), 'osu_username' => $githubUser->osuUsername(), 'user_id' => $githubUser->user_id, diff --git a/resources/assets/less/bem-index.less b/resources/assets/less/bem-index.less index e5e911710b8..9146f5ffd32 100644 --- a/resources/assets/less/bem-index.less +++ b/resources/assets/less/bem-index.less @@ -182,6 +182,7 @@ @import "bem/gallery-thumbnails"; @import "bem/game-mode"; @import "bem/game-mode-link"; +@import "bem/github-user"; @import "bem/grid"; @import "bem/grid-cell"; @import "bem/grid-items"; diff --git a/resources/assets/less/bem/github-user.less b/resources/assets/less/bem/github-user.less new file mode 100644 index 00000000000..c3bf787d90f --- /dev/null +++ b/resources/assets/less/bem/github-user.less @@ -0,0 +1,15 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the GNU Affero General Public License v3.0. +// See the LICENCE file in the repository root for full licence text. + +.github-user { + display: flex; + align-items: center; + justify-content: space-between; + border-bottom: 1px solid @osu-colour-b5; + margin-bottom: 10px; + padding-bottom: 10px; + + &__name { + font-size: @font-size--title-small-3; + } +} diff --git a/resources/assets/lib/account-edit/github-user.tsx b/resources/assets/lib/account-edit/github-user.tsx new file mode 100644 index 00000000000..fd5dc4e924f --- /dev/null +++ b/resources/assets/lib/account-edit/github-user.tsx @@ -0,0 +1,64 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the GNU Affero General Public License v3.0. +// See the LICENCE file in the repository root for full licence text. + +import BigButton from 'components/big-button'; +import { GithubUserJsonForAccountEdit } from 'interfaces/github-user-json'; +import { route } from 'laroute'; +import { action, makeObservable, observable } from 'mobx'; +import { observer } from 'mobx-react'; +import * as React from 'react'; +import { onErrorWithCallback } from 'utils/ajax'; + +interface Props { + onDelete: (id: number) => void; + user: GithubUserJsonForAccountEdit; +} + +@observer +export default class GithubUser extends React.Component { + @observable private deleting = false; + private xhr?: JQuery.jqXHR; + + constructor(props: Props) { + super(props); + makeObservable(this); + } + + componentWillUnmount() { + this.xhr?.abort(); + } + + render() { + return ( + + ); + } + + @action + private onDeleteButtonClick = () => { + this.xhr?.abort(); + this.deleting = true; + + this.xhr = $.ajax( + route('account.github-users.destroy', { github_user: this.props.user.id }), + { method: 'DELETE' }, + ) + .done(() => this.props.onDelete(this.props.user.id)) + .fail(onErrorWithCallback(this.onDeleteButtonClick)) + .always(action(() => this.deleting = false)); + }; +} diff --git a/resources/assets/lib/account-edit/github-users.tsx b/resources/assets/lib/account-edit/github-users.tsx new file mode 100644 index 00000000000..74e99689746 --- /dev/null +++ b/resources/assets/lib/account-edit/github-users.tsx @@ -0,0 +1,53 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the GNU Affero General Public License v3.0. +// See the LICENCE file in the repository root for full licence text. + +import BigButton from 'components/big-button'; +import { GithubUserJsonForAccountEdit } from 'interfaces/github-user-json'; +import { route } from 'laroute'; +import { action, makeObservable, observable } from 'mobx'; +import { observer } from 'mobx-react'; +import * as React from 'react'; +import GithubUser from './github-user'; + +interface Props { + users: GithubUserJsonForAccountEdit[]; +} + +@observer +export default class GithubUsers extends React.Component { + @observable private users: GithubUserJsonForAccountEdit[]; + + constructor(props: Props) { + super(props); + + this.users = props.users; + + makeObservable(this); + } + + render() { + return ( + <> + {this.users.length === 0 + ?
{osu.trans('accounts.github_users.none')}
+ : this.users.map((user) => ( + + ))} + + + ); + } + + @action + private onDelete = (id: number) => { + this.users = this.users.filter((user) => user.id !== id); + }; +} diff --git a/resources/assets/lib/entrypoints/account-edit.tsx b/resources/assets/lib/entrypoints/account-edit.tsx index 919250dfd24..d4ca2d06b65 100644 --- a/resources/assets/lib/entrypoints/account-edit.tsx +++ b/resources/assets/lib/entrypoints/account-edit.tsx @@ -1,13 +1,14 @@ // Copyright (c) ppy Pty Ltd . Licensed under the GNU Affero General Public License v3.0. // See the LICENCE file in the repository root for full licence text. +import GithubUsers from 'account-edit/github-users'; import { ClientJson } from 'interfaces/client-json'; import { OwnClientJson } from 'interfaces/own-client-json'; import { AuthorizedClients } from 'oauth/authorized-clients'; import { OwnClients } from 'oauth/own-clients'; import core from 'osu-core-singleton'; import * as React from 'react'; -import { parseJsonNullable } from 'utils/json'; +import { parseJson, parseJsonNullable } from 'utils/json'; core.reactTurbolinks.register('authorized-clients', () => { const json = parseJsonNullable('json-authorized-clients', true); @@ -18,6 +19,10 @@ core.reactTurbolinks.register('authorized-clients', () => { return ; }); +core.reactTurbolinks.register('github-users', () => ( + +)); + core.reactTurbolinks.register('own-clients', () => { const json = parseJsonNullable('json-own-clients', true); if (json != null) { diff --git a/resources/assets/lib/interfaces/github-user-json.ts b/resources/assets/lib/interfaces/github-user-json.ts new file mode 100644 index 00000000000..f6293aeadf9 --- /dev/null +++ b/resources/assets/lib/interfaces/github-user-json.ts @@ -0,0 +1,17 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the GNU Affero General Public License v3.0. +// See the LICENCE file in the repository root for full licence text. + +export type GithubUserJsonForAccountEdit = GithubUserJson & { + github_url: string; + github_username: string; +}; + +export default interface GithubUserJson { + display_name: string; + github_url: string | null; + github_username: string | null; + id: number; + osu_username: string | null; + user_id: number | null; + user_url: string | null; +} diff --git a/resources/lang/en/accounts.php b/resources/lang/en/accounts.php index 627d07d9fa9..dd534823187 100644 --- a/resources/lang/en/accounts.php +++ b/resources/lang/en/accounts.php @@ -46,6 +46,13 @@ ], ], + 'github_users' => [ + 'accounts' => 'accounts', + 'link' => 'Link GitHub account', + 'none' => 'No GitHub accounts', + 'title' => 'GitHub', + ], + 'notifications' => [ 'beatmapset_discussion_qualified_problem' => 'receive notifications for new problems on qualified beatmaps of the following modes', 'beatmapset_disqualify' => 'receive notifications for when beatmaps of the following modes are disqualified', diff --git a/resources/views/accounts/_edit_github_users.blade.php b/resources/views/accounts/_edit_github_users.blade.php new file mode 100644 index 00000000000..6f75f11fe02 --- /dev/null +++ b/resources/views/accounts/_edit_github_users.blade.php @@ -0,0 +1,24 @@ +{{-- + Copyright (c) ppy Pty Ltd . Licensed under the GNU Affero General Public License v3.0. + See the LICENCE file in the repository root for full licence text. +--}} + diff --git a/resources/views/accounts/edit.blade.php b/resources/views/accounts/edit.blade.php index d642afc39b9..184f54f8e89 100644 --- a/resources/views/accounts/edit.blade.php +++ b/resources/views/accounts/edit.blade.php @@ -163,6 +163,10 @@ class="js-account-edit-avatar__button fileupload"
@include('accounts._edit_oauth')
+ +
+ @include('accounts._edit_github_users') +
@endsection @section("script") @@ -170,6 +174,10 @@ class="js-account-edit-avatar__button fileupload" {!! json_encode($authorizedClients) !!} + + From 7581e68537b51a6515d664db664c467987e4bf32 Mon Sep 17 00:00:00 2001 From: clayton Date: Sun, 6 Nov 2022 12:24:18 -0800 Subject: [PATCH 05/49] Only show GitHub menu if client info is set --- .../Account/GithubUsersController.php | 5 +---- app/Http/Controllers/AccountController.php | 19 ++++++++++++------- app/Models/GithubUser.php | 6 ++++++ resources/views/accounts/edit.blade.php | 16 ++++++++++------ 4 files changed, 29 insertions(+), 17 deletions(-) diff --git a/app/Http/Controllers/Account/GithubUsersController.php b/app/Http/Controllers/Account/GithubUsersController.php index b09d30d940b..16b1a1b07cc 100644 --- a/app/Http/Controllers/Account/GithubUsersController.php +++ b/app/Http/Controllers/Account/GithubUsersController.php @@ -62,10 +62,7 @@ public function callback() public function create() { - abort_if( - config('osu.github.client_id') === null || config('osu.github.client_secret') === null, - 404, - ); + abort_unless(GithubUser::canAuthenticate(), 404); abort_if( auth()->user()->githubUsers()->count() >= 10, 403, diff --git a/app/Http/Controllers/AccountController.php b/app/Http/Controllers/AccountController.php index 37184dddc96..f8effb7b328 100644 --- a/app/Http/Controllers/AccountController.php +++ b/app/Http/Controllers/AccountController.php @@ -11,6 +11,7 @@ use App\Libraries\UserVerificationState; use App\Mail\UserEmailUpdated; use App\Mail\UserPasswordUpdated; +use App\Models\GithubUser; use App\Models\OAuth\Client; use App\Models\UserAccountHistory; use App\Models\UserNotificationOption; @@ -113,13 +114,17 @@ public function edit() $notificationOptions = $user->notificationOptions->keyBy('name'); - $githubUsers = json_collection( - $user - ->githubUsers() - ->whereNotNull(['canonical_id', 'username']) - ->get(), - 'GithubUser', - ); + if (GithubUser::canAuthenticate()) { + $githubUsers = json_collection( + $user + ->githubUsers() + ->whereNotNull(['canonical_id', 'username']) + ->get(), + 'GithubUser', + ); + } else { + $githubUsers = null; + } return ext_view('accounts.edit', compact( 'authorizedClients', diff --git a/app/Models/GithubUser.php b/app/Models/GithubUser.php index 764206d569d..d648f4a08dd 100644 --- a/app/Models/GithubUser.php +++ b/app/Models/GithubUser.php @@ -24,6 +24,12 @@ */ class GithubUser extends Model { + public static function canAuthenticate(): bool + { + return config('osu.github.client_id') !== null + && config('osu.github.client_secret') !== null; + } + public static function importFromGithub(array $apiUser, ?User $user = null): static { $params = [ diff --git a/resources/views/accounts/edit.blade.php b/resources/views/accounts/edit.blade.php index 184f54f8e89..52939d6823e 100644 --- a/resources/views/accounts/edit.blade.php +++ b/resources/views/accounts/edit.blade.php @@ -164,9 +164,11 @@ class="js-account-edit-avatar__button fileupload" @include('accounts._edit_oauth') -
- @include('accounts._edit_github_users') -
+ @if (\App\Models\GithubUser::canAuthenticate()) +
+ @include('accounts._edit_github_users') +
+ @endif @endsection @section("script") @@ -174,9 +176,11 @@ class="js-account-edit-avatar__button fileupload" {!! json_encode($authorizedClients) !!} - + @if (\App\Models\GithubUser::canAuthenticate()) + + @endif @if (\App\Models\GithubUser::canAuthenticate()) - @endif From 465c6151ee59d359fde5e093de249ba0de1bd440 Mon Sep 17 00:00:00 2001 From: clayton Date: Fri, 19 May 2023 19:32:27 -0700 Subject: [PATCH 18/49] Fix file path for github user component --- resources/js/entrypoints/account-edit.tsx | 2 +- .../account-edit/github-user.tsx => js/github-user/index.tsx} | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename resources/{assets/lib/account-edit/github-user.tsx => js/github-user/index.tsx} (96%) diff --git a/resources/js/entrypoints/account-edit.tsx b/resources/js/entrypoints/account-edit.tsx index 9f701094755..ab9ded7e745 100644 --- a/resources/js/entrypoints/account-edit.tsx +++ b/resources/js/entrypoints/account-edit.tsx @@ -1,7 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the GNU Affero General Public License v3.0. // See the LICENCE file in the repository root for full licence text. -import GithubUser from 'account-edit/github-user'; +import GithubUser from 'github-user'; import { ClientJson } from 'interfaces/client-json'; import { OwnClientJson } from 'interfaces/own-client-json'; import LegacyApiKey from 'legacy-api-key'; diff --git a/resources/assets/lib/account-edit/github-user.tsx b/resources/js/github-user/index.tsx similarity index 96% rename from resources/assets/lib/account-edit/github-user.tsx rename to resources/js/github-user/index.tsx index 177f19908a4..773cdbc7636 100644 --- a/resources/assets/lib/account-edit/github-user.tsx +++ b/resources/js/github-user/index.tsx @@ -15,7 +15,7 @@ interface Props { } @observer -export default class GithubUsers extends React.Component { +export default class GithubUser extends React.Component { @observable private deleting = false; @observable private user: GithubUserJson | null | undefined; private xhr?: JQuery.jqXHR; From a90f572e658faa8179bf444deb136d5fefe5baf0 Mon Sep 17 00:00:00 2001 From: clayton Date: Fri, 19 May 2023 19:32:27 -0700 Subject: [PATCH 19/49] Prevent re-assigning same GitHub account --- app/Http/Controllers/Account/GithubUsersController.php | 4 +++- resources/lang/en/accounts.php | 6 +++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/app/Http/Controllers/Account/GithubUsersController.php b/app/Http/Controllers/Account/GithubUsersController.php index 97d58a99f7b..8612fbc22b8 100644 --- a/app/Http/Controllers/Account/GithubUsersController.php +++ b/app/Http/Controllers/Account/GithubUsersController.php @@ -45,7 +45,9 @@ public function callback() $apiUser = $client->currentUser()->show(); $user = GithubUser::firstWhere('canonical_id', $apiUser['id']); - abort_if($user === null, 422, osu_trans('accounts.github_user.error_no_contribution')); + abort_if($user === null, 422, osu_trans('accounts.github_user.error.no_contribution')); + + abort_if($user->user_id !== null, 422, osu_trans('accounts.github_user.error.already_linked')); $user->update([ 'user_id' => auth()->id(), diff --git a/resources/lang/en/accounts.php b/resources/lang/en/accounts.php index 45680798d66..96c1e280564 100644 --- a/resources/lang/en/accounts.php +++ b/resources/lang/en/accounts.php @@ -54,9 +54,13 @@ 'github_user' => [ 'account' => 'account', - 'error_no_contribution' => 'Cannot link GitHub account without any contribution history in osu! repositories.', 'link' => 'Link GitHub account', 'title' => 'GitHub', + + 'error' => [ + 'already_linked' => 'This GitHub account is already linked to a different user.', + 'no_contribution' => 'Cannot link GitHub account without any contribution history in osu! repositories.', + ], ], 'notifications' => [ From 17fe25a1f72756cde5e49eb3c3c9597940f68244 Mon Sep 17 00:00:00 2001 From: clayton Date: Fri, 19 May 2023 19:32:27 -0700 Subject: [PATCH 20/49] Fix display of legacy changelog entries basically revert some of the code that assumed GitHub username would never be null. --- app/Models/GithubUser.php | 20 ++++++++++++++----- app/Transformers/GithubUserTransformer.php | 2 +- .../js/components/changelog-entry.coffee | 17 ++++++++++------ resources/js/interfaces/github-user-json.ts | 4 ++++ 4 files changed, 31 insertions(+), 12 deletions(-) diff --git a/app/Models/GithubUser.php b/app/Models/GithubUser.php index 687c4e2287a..336c4e57d5a 100644 --- a/app/Models/GithubUser.php +++ b/app/Models/GithubUser.php @@ -11,6 +11,9 @@ use Illuminate\Database\Eloquent\Relations\HasMany; /** + * Note: `$canonical_id`, `$id`, and `$username` are null when this model is + * created for use in legacy changelog entries. + * * @property int $canonical_id * @property-read \Illuminate\Database\Eloquent\Collection $changelogEntries * @property \Carbon\Carbon|null $created_at @@ -57,23 +60,30 @@ public function user(): BelongsTo return $this->belongsTo(User::class, 'user_id'); } - public function githubUrl(): string + public function displayUsername(): string { - return "https://github.com/{$this->username}"; + return $this->username ?? $this->osuUsername() ?? '[no name]'; } - public function osuUsername(): ?string + public function githubUrl(): ?string { - return $this->user?->username; + return $this->username !== null + ? "https://github.com/{$this->username}" + : null; } - public function userUrl(): ?string + public function osuUrl(): ?string { return $this->user_id !== null ? route('users.show', $this->user_id) : null; } + public function osuUsername(): ?string + { + return $this->user?->username; + } + public function getAttribute($key) { return match ($key) { diff --git a/app/Transformers/GithubUserTransformer.php b/app/Transformers/GithubUserTransformer.php index b491ed17e05..a1159d4ba09 100644 --- a/app/Transformers/GithubUserTransformer.php +++ b/app/Transformers/GithubUserTransformer.php @@ -14,7 +14,7 @@ class GithubUserTransformer extends TransformerAbstract public function transform(GithubUser $githubUser): array { return [ - 'display_name' => $githubUser->username, // TODO: can be removed + 'display_name' => $githubUser->displayUsername(), 'github_url' => $githubUser->githubUrl(), 'github_username' => $githubUser->username, 'id' => $githubUser->getKey(), diff --git a/resources/js/components/changelog-entry.coffee b/resources/js/components/changelog-entry.coffee index a5ebc75e265..4a2d18c9866 100644 --- a/resources/js/components/changelog-entry.coffee +++ b/resources/js/components/changelog-entry.coffee @@ -42,13 +42,18 @@ export ChangelogEntry = ({entry}) => "#{entry.repository.replace /^.*\//, ''}##{entry.github_pull_request_id}" ')' do => - user = _.escape(entry.github_user.github_username) + user = _.escape(entry.github_user.display_name) + url = entry.github_user.github_url ? entry.github_user.user_url + link = - "#{user}" + if url? + "#{user}" + else + user span className: 'changelog-entry__user' diff --git a/resources/js/interfaces/github-user-json.ts b/resources/js/interfaces/github-user-json.ts index 429de6635f7..87f381926d1 100644 --- a/resources/js/interfaces/github-user-json.ts +++ b/resources/js/interfaces/github-user-json.ts @@ -1,7 +1,11 @@ // Copyright (c) ppy Pty Ltd . Licensed under the GNU Affero General Public License v3.0. // See the LICENCE file in the repository root for full licence text. +// Note: `github_url`, `github_username`, and `id` are null when this model is +// created for use in legacy changelog entries. Typings don't reflect this +// because changelogs are only CoffeeScript for now. export default interface GithubUserJson { + display_name: string; github_url: string; github_username: string; id: number; From 708c5d9fdb7d33d6d7cf817216b32f6d084b0857 Mon Sep 17 00:00:00 2001 From: clayton Date: Fri, 19 May 2023 19:32:28 -0700 Subject: [PATCH 21/49] Redate migration --- ...=> 2023_05_20_023000_require_github_users_id_and_username.php} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename database/migrations/{2022_12_09_120041_require_github_users_id_and_username.php => 2023_05_20_023000_require_github_users_id_and_username.php} (100%) diff --git a/database/migrations/2022_12_09_120041_require_github_users_id_and_username.php b/database/migrations/2023_05_20_023000_require_github_users_id_and_username.php similarity index 100% rename from database/migrations/2022_12_09_120041_require_github_users_id_and_username.php rename to database/migrations/2023_05_20_023000_require_github_users_id_and_username.php From a06ed1ce66fdb10e7c79bf62fa548f1ee4fd0451 Mon Sep 17 00:00:00 2001 From: clayton Date: Fri, 19 May 2023 19:39:57 -0700 Subject: [PATCH 22/49] Fix url method name --- app/Models/GithubUser.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/Models/GithubUser.php b/app/Models/GithubUser.php index 336c4e57d5a..d0823c616a9 100644 --- a/app/Models/GithubUser.php +++ b/app/Models/GithubUser.php @@ -72,16 +72,16 @@ public function githubUrl(): ?string : null; } - public function osuUrl(): ?string + public function osuUsername(): ?string { - return $this->user_id !== null - ? route('users.show', $this->user_id) - : null; + return $this->user?->username; } - public function osuUsername(): ?string + public function userUrl(): ?string { - return $this->user?->username; + return $this->user_id !== null + ? route('users.show', $this->user_id) + : null; } public function getAttribute($key) From 03f19dc4bc08498a31829c2722e8c309d11620e8 Mon Sep 17 00:00:00 2001 From: clayton Date: Tue, 12 Sep 2023 15:46:49 -0700 Subject: [PATCH 23/49] Fix state issues on navigation --- resources/js/entrypoints/account-edit.tsx | 4 ++-- resources/js/github-user/index.tsx | 14 ++++++++++---- .../views/accounts/_edit_github_user.blade.php | 5 ++++- resources/views/accounts/edit.blade.php | 6 ------ 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/resources/js/entrypoints/account-edit.tsx b/resources/js/entrypoints/account-edit.tsx index ab9ded7e745..e134e8825d2 100644 --- a/resources/js/entrypoints/account-edit.tsx +++ b/resources/js/entrypoints/account-edit.tsx @@ -21,8 +21,8 @@ core.reactTurbolinks.register('authorized-clients', () => { return ; }); -core.reactTurbolinks.register('github-user', () => ( - +core.reactTurbolinks.register('github-user', (container: HTMLElement) => ( + )); core.reactTurbolinks.register('legacy-api-key', (container: HTMLElement) => ( diff --git a/resources/js/github-user/index.tsx b/resources/js/github-user/index.tsx index 773cdbc7636..ded9a65dded 100644 --- a/resources/js/github-user/index.tsx +++ b/resources/js/github-user/index.tsx @@ -4,31 +4,37 @@ import BigButton from 'components/big-button'; import GithubUserJson from 'interfaces/github-user-json'; import { route } from 'laroute'; -import { action, makeObservable, observable } from 'mobx'; +import { action, makeObservable, observable, reaction } from 'mobx'; import { observer } from 'mobx-react'; import * as React from 'react'; import { onErrorWithCallback } from 'utils/ajax'; import { trans } from 'utils/lang'; interface Props { - user: GithubUserJson | null | undefined; + container: HTMLElement; } @observer export default class GithubUser extends React.Component { @observable private deleting = false; - @observable private user: GithubUserJson | null | undefined; + @observable private user: GithubUserJson | null; + private userDatasetSyncDisposer; private xhr?: JQuery.jqXHR; constructor(props: Props) { super(props); - this.user = props.user; + this.user = JSON.parse(this.props.container.dataset.user ?? '') as GithubUserJson | null; + this.userDatasetSyncDisposer = reaction( + () => JSON.stringify(this.user), + (githubUserJson) => this.props.container.dataset.user = githubUserJson, + ); makeObservable(this); } componentWillUnmount() { + this.userDatasetSyncDisposer(); this.xhr?.abort(); } diff --git a/resources/views/accounts/_edit_github_user.blade.php b/resources/views/accounts/_edit_github_user.blade.php index cfddc4a1bd0..16ab5e4fcdc 100644 --- a/resources/views/accounts/_edit_github_user.blade.php +++ b/resources/views/accounts/_edit_github_user.blade.php @@ -16,7 +16,10 @@ {{ osu_trans('accounts.github_user.account')}} diff --git a/resources/views/accounts/edit.blade.php b/resources/views/accounts/edit.blade.php index 3ce0ec548c8..86b06a1a408 100644 --- a/resources/views/accounts/edit.blade.php +++ b/resources/views/accounts/edit.blade.php @@ -162,12 +162,6 @@ class="js-account-edit-avatar__button fileupload" {!! json_encode($authorizedClients) !!} - @if (\App\Models\GithubUser::canAuthenticate()) - - @endif - From a1464947ae683d29b3c3cc93628561e3737f890e Mon Sep 17 00:00:00 2001 From: clayton Date: Tue, 12 Sep 2023 15:46:49 -0700 Subject: [PATCH 24/49] Continue xhr instead of aborting, remove redundant `deleting` state --- resources/js/github-user/index.tsx | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/resources/js/github-user/index.tsx b/resources/js/github-user/index.tsx index ded9a65dded..2ebc7116167 100644 --- a/resources/js/github-user/index.tsx +++ b/resources/js/github-user/index.tsx @@ -16,10 +16,9 @@ interface Props { @observer export default class GithubUser extends React.Component { - @observable private deleting = false; @observable private user: GithubUserJson | null; private userDatasetSyncDisposer; - private xhr?: JQuery.jqXHR; + @observable private xhr: JQuery.jqXHR | null = null; constructor(props: Props) { super(props); @@ -51,7 +50,7 @@ export default class GithubUser extends React.Component { { @action private onDeleteButtonClick = () => { - this.xhr?.abort(); - this.deleting = true; + if (this.xhr != null) return; this.xhr = $.ajax( route('account.github-users.destroy', { github_user: this.user?.id }), @@ -79,6 +77,6 @@ export default class GithubUser extends React.Component { ) .done(() => this.user = null) .fail(onErrorWithCallback(this.onDeleteButtonClick)) - .always(action(() => this.deleting = false)); + .always(action(() => this.xhr = null)); }; } From 179ebed3d12ee14d3a8beac4c924bae202b3024f Mon Sep 17 00:00:00 2001 From: clayton Date: Tue, 12 Sep 2023 15:46:49 -0700 Subject: [PATCH 25/49] Missing action --- resources/js/github-user/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/js/github-user/index.tsx b/resources/js/github-user/index.tsx index 2ebc7116167..b5b30f52b43 100644 --- a/resources/js/github-user/index.tsx +++ b/resources/js/github-user/index.tsx @@ -75,7 +75,7 @@ export default class GithubUser extends React.Component { route('account.github-users.destroy', { github_user: this.user?.id }), { method: 'DELETE' }, ) - .done(() => this.user = null) + .done(action(() => this.user = null)) .fail(onErrorWithCallback(this.onDeleteButtonClick)) .always(action(() => this.xhr = null)); }; From eeaf389ba1a28a4a78eaccf475ec513d9cebc1da Mon Sep 17 00:00:00 2001 From: clayton Date: Tue, 12 Sep 2023 15:46:49 -0700 Subject: [PATCH 26/49] Fix div id removed by accident --- resources/views/accounts/_edit_github_user.blade.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/views/accounts/_edit_github_user.blade.php b/resources/views/accounts/_edit_github_user.blade.php index 16ab5e4fcdc..a28b73a9543 100644 --- a/resources/views/accounts/_edit_github_user.blade.php +++ b/resources/views/accounts/_edit_github_user.blade.php @@ -2,7 +2,7 @@ Copyright (c) ppy Pty Ltd . Licensed under the GNU Affero General Public License v3.0. See the LICENCE file in the repository root for full licence text. --}} -