Merge pull request #6783 from thomasjacumin/dev

Use Mastodon username convention for OIDC

app/Http/Controllers/RemoteOidcController.php

@@ -67,7 +67,7 @@ class RemoteOidcController extends Controller
         abort_if(EmailService::isBanned($userInfoData['email']), 400, 'Banned email.');
 
         $user = $this->createUser([
-            'username' => $userInfoData[config('remote-auth.oidc.field_username')],
+            'username' => $this->ensure_valid_username($userInfoData[config('remote-auth.oidc.field_username')]),
             'name' => $userInfoData['name'] ?? $userInfoData['display_name'] ?? $userInfoData[config('remote-auth.oidc.field_username')] ?? null,
             'email' => $userInfoData['email'],
         ]);
@@ -120,4 +120,10 @@ class RemoteOidcController extends Controller
     {
         return Auth::guard();
     }
+
+    private function ensure_valid_username($starting_username) {
+        $starting_username = explode('@', $starting_username)[0];
+        $temp_username = preg_replace('/[^a-z0-9_]+/i', '', $starting_username);
+        return substr($temp_username, 0, 30);
+    }
 }

tests/Feature/RemoteOidcTest.php

@@ -16,7 +16,9 @@ use Tests\TestCase;
 class RemoteOidcTest extends TestCase
 {
     use MockeryPHPUnitIntegration;
+    use RefreshDatabase;
 
+    #[Test]
     public function view_oidc_start()
     {
         config([
@@ -35,6 +37,7 @@ class RemoteOidcTest extends TestCase
         $response->assertRedirect("http://fakeserver.oidc/authorizeURL?scope=openid%20profile%20email&state={$state}&response_type=code&approval_prompt=auto&redirect_uri={$callbackUrl}&client_id=fake");
     }
 
+    // #[Test]
     public function view_oidc_callback_new_user()
     {
         $originalUserCount = User::count();
@@ -71,6 +74,7 @@ class RemoteOidcTest extends TestCase
         $this->assertDatabaseCount('users', $originalUserCount + 1);
     }
 
+    // #[Test]
     public function view_oidc_callback_existing_user()
     {
         $user = User::create([
@@ -116,4 +120,54 @@ class RemoteOidcTest extends TestCase
 
         $this->assertDatabaseCount('users', $originalUserCount);
     }
+
+    #[Test]
+    public function view_oidc_callback_ensure_valid_username()
+    {
+        config(['remote-auth.oidc.enabled' => true]);
+        config(['remote-auth.oidc.field_username' => 'preferred_username']);
+
+        $dataset = [
+            'john.doe@domain.com'                         => 'johndoe',
+            'test+user@part1@domain.com'                  => 'testuser',
+            'user!#$%^&*()_test'                          => 'user_test',
+            'jean-luc.picard'                             => 'jeanlucpicard',
+            'supercalifragilisticexpialidøcious@test.com' => 'supercalifragilisticexpialidci',
+            'hélène_renåud'                               => 'hlne_renud',
+            '123456789'                                   => '123456789',
+            '  user _ name  '                             => 'user_name',
+            'foo+bar@sub.domain.co.uk'                    => 'foobar',
+        ];
+
+        foreach ($dataset as $input => $expected) {
+            Auth::logout();
+            session()->flush();
+
+            $originalUserCount = User::count();
+
+            $oauthData = [
+                'sub'                => str_random(10),
+                'name'               => fake()->name,
+                'preferred_username' => $input,
+                'email'              => fake()->unique()->freeEmail,
+            ];
+
+            $this->partialMock(UserOidcService::class, function (MockInterface $mock) use ($oauthData) {
+                $mock->shouldReceive('getAccessToken')->once()->andReturn(new AccessToken(['access_token' => 'token']));
+                $mock->shouldReceive('getResourceOwner')->once()->andReturn(new GenericResourceOwner($oauthData, 'sub'));
+            });
+
+            $response = $this->withoutExceptionHandling()->withSession([
+                'oauth2state' => 'abc123',
+            ])->get('auth/oidc/callback?state=abc123&code=1');
+
+            $response->assertRedirect('/');
+
+            $mappedUser = UserOidcMapping::where('oidc_id', $oauthData['sub'])->first();
+
+            $this->assertNotNull($mappedUser, "Mapping not found for : {$input}");
+            $this->assertEquals($expected, $mappedUser->user->username, "Username not valid : {$input}");
+            $this->assertDatabaseCount('users', $originalUserCount+1);
+        }
+    }
 }