Skip to content

Commit 0146dbe

Browse files
novacuumnovacuum
authored
fix: missing login provider value (#90)
* fix: missing login provider value * fix: styleci
1 parent 145ae1a commit 0146dbe

File tree

2 files changed

+177
-1
lines changed

2 files changed

+177
-1
lines changed

‎src/Api/CurrentUserAttributes.php‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace FoF\OAuth\Api;
1313

1414
use Flarum\Api\Serializer\CurrentUserSerializer;
15+
use Flarum\User\LoginProvider;
1516
use Flarum\User\User;
1617
use FoF\Extend\Controllers\AbstractOAuthController;
1718
use Illuminate\Contracts\Cache\Store as Cache;
@@ -33,7 +34,22 @@ public function __invoke(CurrentUserSerializer $serializer, User $user, array $a
3334
$session = $serializer->getRequest()->getAttribute('session');
3435

3536
if ($session !== null) {
36-
$attributes['loginProvider'] = $this->cache->get(AbstractOAuthController::SESSION_OAUTH2PROVIDER.'_'.$session->getId());
37+
$loginProvider = $this->cache->get(AbstractOAuthController::SESSION_OAUTH2PROVIDER.'_'.$session->getId());
38+
39+
if ($loginProvider === null) {
40+
// This solution is not optimal, if someone uses multiple login providers at the same time, this could be lead to wrong results
41+
$loginProvider = LoginProvider::query()
42+
->where('user_id', $user->id)
43+
->orderBy('last_login_at', 'desc')
44+
->pluck('provider')
45+
->first();
46+
47+
$loginProvider = $loginProvider ?: false;
48+
$this->cache->forever(AbstractOAuthController::SESSION_OAUTH2PROVIDER.'_'.$session->getId(), $loginProvider);
49+
}
50+
51+
// We don't want return false when the provider is not set, map it back to null
52+
$attributes['loginProvider'] = $loginProvider ?: null;
3753
}
3854

3955
return $attributes;

‎tests/integration/AuthenticationFlowTest.php‎

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
<?php
2+
3+
/*
4+
* This file is part of fof/oauth.
5+
*
6+
* Copyright (c) FriendsOfFlarum.
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace FoF\OAuth\Tests\integration;
13+
14+
use Dflydev\FigCookies\SetCookies;
15+
use Flarum\Settings\SettingsRepositoryInterface;
16+
use Flarum\Testing\integration\RetrievesAuthorizedUsers;
17+
use Flarum\Testing\integration\TestCase;
18+
use League\OAuth2\Client\Token\AccessToken;
19+
use Omines\OAuth2\Client\Provider\Gitlab;
20+
use Omines\OAuth2\Client\Provider\GitlabResourceOwner;
21+
use Psr\Http\Message\ResponseInterface;
22+
23+
class AuthenticationFlowTest extends TestCase
24+
{
25+
use RetrievesAuthorizedUsers;
26+
27+
protected function setUp(): void
28+
{
29+
parent::setUp();
30+
31+
$this->extension('fof-oauth');
32+
33+
$this->prepareDatabase([
34+
'users' => [
35+
$this->normalUser(),
36+
[
37+
'id' => 3, 'username' => 'Seboubeach',
38+
'is_email_confirmed' => 1, 'email' => 'Seboubeach1@machine.local',
39+
'joined_at' => '2021-01-01 00:00:00',
40+
],
41+
[
42+
'id' => 4, 'username' => 'Hephoica',
43+
'is_email_confirmed' => 1, 'email' => 'Hephoica@machine.local',
44+
'joined_at' => '2021-01-01 00:00:00',
45+
],
46+
],
47+
'login_providers' => [
48+
['id' => 1, 'user_id' => 3, 'provider' => 'gitlab', 'identifier' => '123456'],
49+
],
50+
'group_permission' => [
51+
['permission' => 'user.editOwnNickname', 'group_id' => 4],
52+
],
53+
]);
54+
55+
$this->setting('fof-oauth.gitlab.client_id', 'test');
56+
$this->setting('fof-oauth.gitlab.client_secret', 'test');
57+
$this->setting('fof-oauth.gitlab', 1);
58+
}
59+
60+
public function test_loginProvider_is_set_with_correct_value_after_oauth_login(): void
61+
{
62+
$this->mockProvider('123456', 'Seboubeach1@machine.local');
63+
64+
$response = $this->send($this->request('GET', '/auth/gitlab'));
65+
66+
// get query params from location url in the header
67+
$location = $response->getHeaderLine('location');
68+
parse_str(parse_url($location, PHP_URL_QUERY), $query);
69+
70+
$request = $this->request('GET', '/auth/gitlab')
71+
->withQueryParams([
72+
'code' => 'code:123456',
73+
'state' => $query['state'],
74+
])
75+
->withCookieParams($this->toRequestCookies($response));
76+
77+
$response = $this->send($request);
78+
$content = $response->getBody()->getContents();
79+
80+
// check if the content contains is_loggedIn
81+
$this->assertStringContainsString(
82+
'window.opener.app.authenticationComplete(',
83+
$content
84+
);
85+
86+
preg_match('/window.opener.app.authenticationComplete\((.*)\)/', $content, $matches);
87+
$json = json_decode($matches[1], true);
88+
89+
$this->assertArrayHasKey('loggedIn', $json);
90+
91+
$response = $this->send($this->request('GET', '/')->withCookieParams($this->toRequestCookies($response)));
92+
$this->assertEquals(200, $response->getStatusCode());
93+
94+
$this->checkOauthProviderIsSerialized($this->toRequestCookies($response), 'gitlab');
95+
}
96+
97+
protected function checkOauthProviderIsSerialized(array $cookies, ?string $value = null): void
98+
{
99+
$response = $this->send(
100+
$this->request('GET', '/api')->withCookieParams($cookies)
101+
);
102+
103+
$this->assertEquals(200, $response->getStatusCode());
104+
105+
$body = json_decode($response->getBody()->getContents(), true);
106+
107+
// get user from included
108+
$user = array_filter($body['included'], function ($item) {
109+
return $item['type'] === 'users';
110+
});
111+
112+
$user = array_values($user)[0];
113+
$this->assertArrayHasKey('loginProvider', $user['attributes']);
114+
$this->assertEquals($value, $user['attributes']['loginProvider']);
115+
}
116+
117+
private function mockProvider(string $identifier, string $email): void
118+
{
119+
$container = $this->app()->getContainer();
120+
121+
$mockProvider = $this->getMockBuilder(Gitlab::class)
122+
->setConstructorArgs([
123+
'options' => [
124+
'clientId' => 'test',
125+
'clientSecret' => 'test',
126+
'redirectUri' => 'http://localhost/auth/gitlab',
127+
],
128+
])
129+
->onlyMethods(['getAccessToken', 'getResourceOwner'])
130+
->getMock();
131+
132+
$accessToken = new AccessToken(['access_token' => '123456', 'expires' => time() + 3600]);
133+
$mockProvider->method('getAccessToken')->willReturn(
134+
$accessToken
135+
);
136+
$mockProvider->method('getResourceOwner')->willReturn(
137+
new GitlabResourceOwner(['id' => $identifier, 'email' => $email], $accessToken)
138+
);
139+
140+
$mockFofProvider = $this->getMockBuilder(\FoF\OAuth\Providers\GitLab::class)
141+
->setConstructorArgs([
142+
'settings' => $container->make(SettingsRepositoryInterface::class),
143+
])
144+
->onlyMethods(['provider'])
145+
->getMock();
146+
$mockFofProvider->method('provider')->willReturn($mockProvider);
147+
148+
$this->app()->getContainer()->instance(\FoF\OAuth\Providers\GitLab::class, $mockFofProvider);
149+
}
150+
151+
protected function toRequestCookies(ResponseInterface $response): array
152+
{
153+
$responseCookies = [];
154+
foreach (SetCookies::fromResponse($response)->getAll() as $cookie) {
155+
$responseCookies[$cookie->getName()] = $cookie->getValue();
156+
}
157+
158+
return $responseCookies;
159+
}
160+
}

0 commit comments

Comments
 (0)