Skip to content

Commit fabc854

Browse files
ssddanbrownssddanbrown
committed
My Account: Updated and started adding to tests
- Updated existing tests now affected by my-account changes. - Updated some existing tests to more accuractly check the scenario. - Updated some code styling in SocialController. - Fixed redirects for social account flows to fit my-account. - Added test for social account attaching. - Added test for api token redirect handling.
1 parent 1294641 commit fabc854

File tree

14 files changed

+302
-227
lines changed

14 files changed

+302
-227
lines changed

‎app/Access/Controllers/SocialController.php‎

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,22 +16,12 @@
1616

1717
class SocialController extends Controller
1818
{
19-
protected SocialAuthService $socialAuthService;
20-
protected RegistrationService $registrationService;
21-
protected LoginService $loginService;
22-
23-
/**
24-
* SocialController constructor.
25-
*/
2619
public function __construct(
27-
SocialAuthService $socialAuthService,
28-
RegistrationService $registrationService,
29-
LoginService $loginService
20+
protected SocialAuthService $socialAuthService,
21+
protected RegistrationService $registrationService,
22+
protected LoginService $loginService,
3023
) {
3124
$this->middleware('guest')->only(['register']);
32-
$this->socialAuthService = $socialAuthService;
33-
$this->registrationService = $registrationService;
34-
$this->loginService = $loginService;
3525
}
3626

3727
/**
@@ -112,7 +102,7 @@ public function detach(string $socialDriver)
112102
$this->socialAuthService->detachSocialAccount($socialDriver);
113103
session()->flash('success', trans('settings.users_social_disconnected', ['socialAccount' => Str::title($socialDriver)]));
114104

115-
return redirect(user()->getEditUrl());
105+
return redirect('/my-account/auth#social-accounts');
116106
}
117107

118108
/**

‎app/Access/SocialAuthService.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -154,21 +154,21 @@ public function handleLoginCallback(string $socialDriver, SocialUser $socialUser
154154
$currentUser->socialAccounts()->save($account);
155155
session()->flash('success', trans('settings.users_social_connected', ['socialAccount' => $titleCaseDriver]));
156156

157-
return redirect($currentUser->getEditUrl());
157+
return redirect('/my-account/auth#social_accounts');
158158
}
159159

160160
// When a user is logged in and the social account exists and is already linked to the current user.
161161
if ($isLoggedIn && $socialAccount !== null && $socialAccount->user->id === $currentUser->id) {
162162
session()->flash('error', trans('errors.social_account_existing', ['socialAccount' => $titleCaseDriver]));
163163

164-
return redirect($currentUser->getEditUrl());
164+
return redirect('/my-account/auth#social_accounts');
165165
}
166166

167167
// When a user is logged in, A social account exists but the users do not match.
168168
if ($isLoggedIn && $socialAccount !== null && $socialAccount->user->id != $currentUser->id) {
169169
session()->flash('error', trans('errors.social_account_already_used_existing', ['socialAccount' => $titleCaseDriver]));
170170

171-
return redirect($currentUser->getEditUrl());
171+
return redirect('/my-account/auth#social_accounts');
172172
}
173173

174174
// Otherwise let the user know this social account is not used by anyone.

‎app/Api/UserApiTokenController.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ protected function updateContext(Request $request): void
166166
protected function getRedirectPath(User $relatedUser): string
167167
{
168168
$context = session()->get('api-token-context');
169-
if ($context === 'settings') {
169+
if ($context === 'settings' || user()->id !== $relatedUser->id) {
170170
return $relatedUser->getEditUrl('#api_tokens');
171171
}
172172

‎lang/en/settings.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,8 @@
214214
'users_social_accounts_info' => 'Here you can connect your other accounts for quicker and easier login. Disconnecting an account here does not revoke previously authorized access. Revoke access from your profile settings on the connected social account.',
215215
'users_social_connect' => 'Connect Account',
216216
'users_social_disconnect' => 'Disconnect Account',
217+
'users_social_status_connected' => 'Connected',
218+
'users_social_status_disconnected' => 'Disconnected',
217219
'users_social_connected' => ':socialAccount account was successfully attached to your profile.',
218220
'users_social_disconnected' => ':socialAccount account was successfully disconnected from your profile.',
219221
'users_api_tokens' => 'API Tokens',

‎resources/views/users/account/layout.blade.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@
1212
<a href="{{ url('/my-account/profile') }}" class="{{ $category === 'profile' ? 'active' : '' }}">@icon('user') {{ trans('preferences.profile') }}</a>
1313
<a href="{{ url('/my-account/auth') }}" class="{{ $category === 'auth' ? 'active' : '' }}">@icon('security') {{ trans('preferences.auth') }}</a>
1414
<a href="{{ url('/my-account/shortcuts') }}" class="{{ $category === 'shortcuts' ? 'active' : '' }}">@icon('shortcuts') {{ trans('preferences.shortcuts_interface') }}</a>
15-
<a href="{{ url('/my-account/notifications') }}" class="{{ $category === 'notifications' ? 'active' : '' }}">@icon('notifications') {{ trans('preferences.notifications') }}</a>
15+
@if(userCan('receive-notifications'))
16+
<a href="{{ url('/my-account/notifications') }}" class="{{ $category === 'notifications' ? 'active' : '' }}">@icon('notifications') {{ trans('preferences.notifications') }}</a>
17+
@endif
1618
</nav>
1719
</div>
1820
</div>

‎resources/views/users/edit.blade.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,9 @@ class="button outline">{{ trans('settings.users_mfa_configure') }}</a>
8989
<div role="presentation">@icon('auth/'. $driver, ['style' => 'width: 56px;height: 56px;'])</div>
9090
<p class="my-none bold">{{ $driverName }}</p>
9191
@if($user->hasSocialAccount($driver))
92-
<p class="text-pos bold text-small my-none">Connected</p>
92+
<p class="text-pos bold text-small my-none">{{ trans('settings.users_social_status_connected') }}</p>
9393
@else
94-
<p class="text-neg bold text-small my-none">Disconnected</p>
94+
<p class="text-neg bold text-small my-none">{{ trans('settings.users_social_status_disconnected') }}</p>
9595
@endif
9696
</div>
9797
@endforeach

‎routes/web.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,8 @@
244244
Route::put('/my-account/auth/password', [UserControllers\UserAccountController::class, 'updatePassword']);
245245
Route::get('/my-account/delete', [UserControllers\UserAccountController::class, 'delete']);
246246
Route::delete('/my-account', [UserControllers\UserAccountController::class, 'destroy']);
247+
248+
// User Preference Endpoints
247249
Route::patch('/preferences/change-view/{type}', [UserControllers\UserPreferencesController::class, 'changeView']);
248250
Route::patch('/preferences/change-sort/{type}', [UserControllers\UserPreferencesController::class, 'changeSort']);
249251
Route::patch('/preferences/change-expansion/{type}', [UserControllers\UserPreferencesController::class, 'changeExpansion']);

‎tests/Actions/WebhookCallTest.php‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public function test_webhook_runs_for_delete_actions()
5151
{
5252
// This test must not fake the queue/bus since this covers an issue
5353
// around handling and serialization of items now deleted from the database.
54-
$this->newWebhook(['active' => true, 'endpoint' => 'https://wh.example.com'], ['all']);
54+
$webhook = $this->newWebhook(['active' => true, 'endpoint' => 'https://wh.example.com'], ['all']);
5555
$this->mockHttpClient([new Response(500)]);
5656

5757
$user = $this->users->newUser();
@@ -61,8 +61,10 @@ public function test_webhook_runs_for_delete_actions()
6161
/** @var ApiToken $apiToken */
6262
$editor = $this->users->editor();
6363
$apiToken = ApiToken::factory()->create(['user_id' => $editor]);
64-
$resp = $this->delete($editor->getEditUrl('/api-tokens/' . $apiToken->id));
65-
$resp->assertRedirect($editor->getEditUrl('#api_tokens'));
64+
$this->delete($apiToken->getUrl())->assertRedirect();
65+
66+
$webhook->refresh();
67+
$this->assertEquals('Response status from endpoint was 500', $webhook->last_error);
6668
}
6769

6870
public function test_failed_webhook_call_logs_error()

‎tests/Auth/SocialAuthTest.php‎

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ public function test_social_registration()
1818
$user = User::factory()->make();
1919

2020
$this->setSettings(['registration-enabled' => 'true']);
21-
config(['GOOGLE_APP_ID' => 'abc123', 'GOOGLE_APP_SECRET' => '123abc', 'APP_URL' => 'http://localhost']);
21+
config(['GOOGLE_APP_ID' => 'abc123', 'GOOGLE_APP_SECRET' => '123abc']);
2222

2323
$mockSocialite = $this->mock(Factory::class);
2424
$mockSocialDriver = Mockery::mock(Provider::class);
@@ -45,7 +45,6 @@ public function test_social_login()
4545
config([
4646
'GOOGLE_APP_ID' => 'abc123', 'GOOGLE_APP_SECRET' => '123abc',
4747
'GITHUB_APP_ID' => 'abc123', 'GITHUB_APP_SECRET' => '123abc',
48-
'APP_URL' => 'http://localhost',
4948
]);
5049

5150
$mockSocialite = $this->mock(Factory::class);
@@ -86,12 +85,41 @@ public function test_social_login()
8685
$this->assertActivityExists(ActivityType::AUTH_LOGIN, null, 'github; (' . $this->users->admin()->id . ') ' . $this->users->admin()->name);
8786
}
8887

88+
public function test_social_account_attach()
89+
{
90+
config([
91+
'GOOGLE_APP_ID' => 'abc123', 'GOOGLE_APP_SECRET' => '123abc',
92+
]);
93+
$editor = $this->users->editor();
94+
95+
$mockSocialite = $this->mock(Factory::class);
96+
$mockSocialDriver = Mockery::mock(Provider::class);
97+
$mockSocialUser = Mockery::mock(\Laravel\Socialite\Contracts\User::class);
98+
99+
$mockSocialUser->shouldReceive('getId')->twice()->andReturn('logintest123');
100+
$mockSocialUser->shouldReceive('getAvatar')->andReturn(null);
101+
102+
$mockSocialite->shouldReceive('driver')->twice()->with('google')->andReturn($mockSocialDriver);
103+
$mockSocialDriver->shouldReceive('redirect')->once()->andReturn(redirect('/login/service/google/callback'));
104+
$mockSocialDriver->shouldReceive('user')->once()->andReturn($mockSocialUser);
105+
106+
// Test login routes
107+
$resp = $this->actingAs($editor)->followingRedirects()->get('/login/service/google');
108+
$resp->assertSee('Access & Security');
109+
110+
// Test social callback with matching social account
111+
$this->assertDatabaseHas('social_accounts', [
112+
'user_id' => $editor->id,
113+
'driver' => 'google',
114+
'driver_id' => 'logintest123',
115+
]);
116+
}
117+
89118
public function test_social_account_detach()
90119
{
91120
$editor = $this->users->editor();
92121
config([
93122
'GITHUB_APP_ID' => 'abc123', 'GITHUB_APP_SECRET' => '123abc',
94-
'APP_URL' => 'http://localhost',
95123
]);
96124

97125
$socialAccount = SocialAccount::query()->forceCreate([
@@ -100,11 +128,11 @@ public function test_social_account_detach()
100128
'driver_id' => 'logintest123',
101129
]);
102130

103-
$resp = $this->actingAs($editor)->get($editor->getEditUrl());
131+
$resp = $this->actingAs($editor)->get('/my-account/auth');
104132
$this->withHtml($resp)->assertElementContains('form[action$="/login/service/github/detach"]', 'Disconnect Account');
105133

106134
$resp = $this->post('/login/service/github/detach');
107-
$resp->assertRedirect($editor->getEditUrl());
135+
$resp->assertRedirect('/my-account/auth#social-accounts');
108136
$resp = $this->followRedirects($resp);
109137
$resp->assertSee('Github account was successfully disconnected from your profile.');
110138

@@ -115,7 +143,6 @@ public function test_social_autoregister()
115143
{
116144
config([
117145
'services.google.client_id' => 'abc123', 'services.google.client_secret' => '123abc',
118-
'APP_URL' => 'http://localhost',
119146
]);
120147

121148
$user = User::factory()->make();
@@ -153,7 +180,7 @@ public function test_social_auto_email_confirm()
153180
{
154181
config([
155182
'services.google.client_id' => 'abc123', 'services.google.client_secret' => '123abc',
156-
'APP_URL' => 'http://localhost', 'services.google.auto_register' => true, 'services.google.auto_confirm' => true,
183+
'services.google.auto_register' => true, 'services.google.auto_confirm' => true,
157184
]);
158185

159186
$user = User::factory()->make();
@@ -191,7 +218,7 @@ public function test_social_registration_with_no_name_uses_email_as_name()
191218
$user = User::factory()->make(['email' => 'nonameuser@example.com']);
192219

193220
$this->setSettings(['registration-enabled' => 'true']);
194-
config(['GITHUB_APP_ID' => 'abc123', 'GITHUB_APP_SECRET' => '123abc', 'APP_URL' => 'http://localhost']);
221+
config(['GITHUB_APP_ID' => 'abc123', 'GITHUB_APP_SECRET' => '123abc']);
195222

196223
$mockSocialite = $this->mock(Factory::class);
197224
$mockSocialDriver = Mockery::mock(Provider::class);

‎tests/Permissions/RolePermissionsTest.php‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,14 +44,12 @@ public function test_manage_users_permission_shows_link_in_header_if_does_not_ha
4444

4545
public function test_user_cannot_change_email_unless_they_have_manage_users_permission()
4646
{
47-
$userProfileUrl = '/settings/users/' . $this->user->id;
4847
$originalEmail = $this->user->email;
4948
$this->actingAs($this->user);
5049

51-
$resp = $this->get($userProfileUrl)
52-
->assertOk();
50+
$resp = $this->get('/my-account/profile')->assertOk();
5351
$this->withHtml($resp)->assertElementExists('input[name=email][disabled]');
54-
$this->put($userProfileUrl, [
52+
$this->put('/my-account/profile', [
5553
'name' => 'my_new_name',
5654
'email' => 'new_email@example.com',
5755
]);
@@ -63,11 +61,12 @@ public function test_user_cannot_change_email_unless_they_have_manage_users_perm
6361

6462
$this->permissions->grantUserRolePermissions($this->user, ['users-manage']);
6563

66-
$resp = $this->get($userProfileUrl)
67-
->assertOk();
68-
$this->withHtml($resp)->assertElementNotExists('input[name=email][disabled]')
64+
$resp = $this->get('/my-account/profile')->assertOk();
65+
$this->withHtml($resp)
66+
->assertElementNotExists('input[name=email][disabled]')
6967
->assertElementExists('input[name=email]');
70-
$this->put($userProfileUrl, [
68+
69+
$this->put('/my-account/profile', [
7170
'name' => 'my_new_name_2',
7271
'email' => 'new_email@example.com',
7372
]);

0 commit comments

Comments
 (0)