Forráskód Böngészése

Fixes for items highlighted by review.ai

* Consider using `hash_equals()` instead of `==` when comparing the state values to prevent timing attacks:
`abort_unless(hash_equals($request->input('state'), $request->session()->pull('oauth2state')), 400, 'invalid
state');`
* For better data integrity, consider adding a foreign key constraint to the user_id column: `$table-
>foreign('user_id')->references('id')->on('users')->onDelete('cascade');`
* Does the OIDC provider guarantee that the username field exists in the userInfo data? Consider adding a
null check or fallback: `$userInfoData[config('remote-auth.oidc.field_username')] ?? null`
Gavin Mogan 5 hónapja
szülő
commit
70584b47c5

+ 2 - 2
app/Http/Controllers/RemoteOidcController.php

@@ -46,7 +46,7 @@ class RemoteOidcController extends Controller
         abort_unless($request->input("state"), 400);
         abort_unless($request->input("code"), 400);
 
-        abort_unless($request->input("state") == $request->session()->pull('oauth2state'), 400, "invalid state");
+        abort_unless(hash_equals($request->session()->pull('oauth2state'), $request->input("state")), 400, "invalid state");
 
         $accessToken = $provider->getAccessToken('authorization_code', [
             'code' => $request->get('code')
@@ -66,7 +66,7 @@ class RemoteOidcController extends Controller
 
         $user = $this->createUser([
             'username' => $userInfoData[config('remote-auth.oidc.field_username')],
-            'name' => $userInfoData["name"] ?? $userInfoData["display_name"] ?? $userInfoData[config('remote-auth.oidc.field_username')],
+            'name' => $userInfoData["name"] ?? $userInfoData["display_name"] ?? $userInfoData[config('remote-auth.oidc.field_username')] ?? null,
             'email' => $userInfoData["email"],
         ]);
 

+ 3 - 2
database/migrations/2025_01_30_061434_create_user_oidc_mapping_table.php

@@ -12,9 +12,10 @@ return new class extends Migration
     public function up(): void
     {
         Schema::create('user_oidc_mappings', function (Blueprint $table) {
-            $table->id();
-            $table->unsignedInteger('user_id')->index();
+            $table->bigIncrements('id');
+            $table->bigInteger('user_id')->unsigned()->index();
             $table->string('oidc_id')->unique()->index();
+            $table->foreign('user_id')->references('id')->on('users')->onDelete('cascade');
             $table->timestamps();
         });
     }