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`
pull/5608/head
Gavin Mogan 3 weeks ago
parent 59843d2751
commit 02972a337d

@ -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"],
]);

@ -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();
});
}

Loading…
Cancel
Save