Merge pull request #1575 from linuxserver/chore/laravel-13-upgrade

Fix to try and mitigate any disappearing tiles
This commit is contained in:
KodeStar
2026-07-09 11:30:47 +01:00
committed by GitHub
13 changed files with 391 additions and 11 deletions
+9 -2
View File
@@ -328,11 +328,17 @@ class ItemController extends Controller
$config = json_encode($configObject);
}
$current_user = User::currentUser();
$request->merge([
'description' => $config,
]);
// Only assign ownership when creating; updates must keep the existing owner.
if ($id === null) {
$current_user = User::currentUser();
$request->merge([
'user_id' => $current_user->getId(),
]);
}
if ($request->input('appid') === 'null' || $request->input('appid') === null) {
$request->merge([
@@ -348,7 +354,8 @@ class ItemController extends Controller
$item = Item::create($request->all());
} else {
$item = Item::find($id);
$item->update($request->all());
// Exclude user_id so an update can never reassign ownership
$item->update($request->except(['user_id']));
}
$item->parents()->sync($request->tags);
+2 -1
View File
@@ -142,7 +142,8 @@ class TagController extends Controller
'url' => $slug,
]);
Item::find($id)->update($request->all());
// Exclude user_id so an update can never reassign ownership
Item::find($id)->update($request->except(['user_id']));
$route = route('dash', []);
+6
View File
@@ -2,6 +2,7 @@
namespace App\Http\Controllers;
use App\Item;
use App\User;
use Illuminate\Contracts\View\View;
use Illuminate\Http\RedirectResponse;
@@ -154,6 +155,11 @@ class UserController extends Controller
public function destroy(User $user): RedirectResponse
{
if ($user->id !== 1) {
// Hard-delete this user's items (tiles and tags) so they don't become
// orphaned; item_tag pivot rows cascade via the existing FK. Shared
// items (user_id = 0) are left untouched.
Item::withoutGlobalScopes()->where('user_id', $user->id)->forceDelete();
$user->delete();
$route = route('dash', []);
+3 -1
View File
@@ -86,7 +86,9 @@ class Item extends Model
static::addGlobalScope('user_id', function (Builder $builder) {
$current_user = User::currentUser();
if ($current_user) {
$builder->where('user_id', $current_user->getId())->orWhere('user_id', 0);
$builder->where(function ($query) use ($current_user) {
$query->where('user_id', $current_user->getId())->orWhere('user_id', 0);
});
} else {
$builder->where('user_id', 0);
}
+3 -2
View File
@@ -150,9 +150,10 @@ class AppServiceProvider extends ServiceProvider
$db_type = config()->get('database.default');
if ($db_type == 'sqlite') {
$db_file = database_path(env('DB_DATABASE', 'app.sqlite'));
$db_file = config()->get('database.connections.sqlite.database');
Log::debug('SQLite Database Path: ' . $db_file);
if (! is_file($db_file)) {
// Do not create a file for the in-memory database identifier.
if ($db_file !== ':memory:' && ! is_file($db_file)) {
touch($db_file);
}
}
+5 -1
View File
@@ -7,7 +7,11 @@ return [
'connections' => [
'sqlite' => [
'driver' => 'sqlite',
'database' => database_path(env('DB_DATABASE', 'app.sqlite')), // Make sure to use the correct path
// Use the correct path, but let the special in-memory identifier
// pass through untouched so tests can run against ':memory:'.
'database' => env('DB_DATABASE', 'app.sqlite') === ':memory:'
? ':memory:'
: database_path(env('DB_DATABASE', 'app.sqlite')),
'prefix' => '',
'foreign_key_constraints' => env('DB_FOREIGN_KEYS', true), // Enable foreign key constraints
],
@@ -0,0 +1,46 @@
<?php
use Illuminate\Database\Migrations\Migration;
use Illuminate\Support\Facades\DB;
return new class extends Migration
{
/**
* Run the migrations.
*
* Reassign items whose user_id references a user that no longer exists to a
* surviving user, so previously orphaned tiles and tags become visible again.
* user_id = 0 means "shared with all users" and is never treated as orphaned.
*
* The DB query builder is used directly (not the Item model) so the global
* scope and soft-delete constraints don't interfere.
*/
public function up(): void
{
// Prefer user id 1 if it still exists, otherwise the lowest existing id.
$target = DB::table('users')->where('id', 1)->value('id')
?? DB::table('users')->min('id');
// No users left: nothing to reassign to.
if ($target === null) {
return;
}
DB::table('items')
->where('user_id', '!=', 0)
->whereNotIn('user_id', function ($query) {
$query->select('id')->from('users');
})
->update(['user_id' => $target]);
}
/**
* Reverse the migrations.
*
* This is a data migration; the original ownership cannot be recovered.
*/
public function down(): void
{
//
}
};
+2 -2
View File
@@ -12,8 +12,8 @@
<env name="APP_ENV" value="testing"/>
<env name="BCRYPT_ROUNDS" value="4"/>
<env name="CACHE_STORE" value="array"/>
<!-- <env name="DB_CONNECTION" value="sqlite"/> -->
<!-- <env name="DB_DATABASE" value=":memory:"/> -->
<env name="DB_CONNECTION" value="sqlite"/>
<env name="DB_DATABASE" value=":memory:"/>
<env name="MAIL_MAILER" value="array"/>
<env name="PULSE_ENABLED" value="false"/>
<env name="QUEUE_CONNECTION" value="sync"/>
+13
View File
@@ -5,6 +5,7 @@ namespace Tests\Feature;
use App\Item;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Illuminate\Support\Facades\Date;
use Illuminate\Support\Facades\DB;
use Tests\TestCase;
class ItemExportTest extends TestCase
@@ -39,6 +40,18 @@ class ItemExportTest extends TestCase
public function test_exports_assigned_tag_titles_excluding_the_root_tag(): void
{
// Mirror the root/default dashboard row that production seeds (id 0),
// so the item_tag pivot's foreign key to items.id is satisfied on a
// fresh in-memory database.
DB::table('items')->insert([
'id' => 0,
'title' => 'app.dashboard',
'url' => '',
'type' => 1,
'user_id' => 0,
'pinned' => 0,
]);
$item = Item::factory()
->create([
'title' => 'Tagged Item',
+132
View File
@@ -0,0 +1,132 @@
<?php
namespace Tests\Feature;
use App\Item;
use App\User;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Tests\TestCase;
class ItemOwnershipTest extends TestCase
{
use RefreshDatabase;
/**
* Create a passwordless user (so the "allowed" middleware lets the
* request through) and make it the current session user.
*/
private function actAsCurrentUser(array $attributes = []): User
{
$user = User::factory()->create(array_merge([
'password' => null,
'public_front' => 1,
], $attributes));
$this->withSession(['current_user' => $user]);
return $user;
}
public function test_creating_an_item_assigns_the_creator_as_owner(): void
{
$this->seed();
$creator = $this->actAsCurrentUser();
$response = $this->post('/items', [
'pinned' => 1,
'appid' => 'null',
'website' => null,
'title' => 'Owned Item',
'colour' => '#00f',
'url' => 'http://10.0.1.1',
'tags' => [0],
]);
$response->assertStatus(302);
$item = Item::withoutGlobalScopes()->where('title', 'Owned Item')->first();
$this->assertNotNull($item);
$this->assertSame($creator->id, (int) $item->user_id);
}
public function test_creating_an_item_ignores_a_crafted_user_id(): void
{
$this->seed();
$creator = $this->actAsCurrentUser();
$other = User::factory()->create();
$response = $this->post('/items', [
'pinned' => 1,
'appid' => 'null',
'title' => 'Crafted Owner Item',
'colour' => '#00f',
'url' => 'http://10.0.1.2',
'user_id' => $other->id, // attempt to create on behalf of another user
'tags' => [0],
]);
$response->assertStatus(302);
$item = Item::withoutGlobalScopes()->where('title', 'Crafted Owner Item')->first();
$this->assertNotNull($item);
$this->assertSame($creator->id, (int) $item->user_id);
}
public function test_updating_a_shared_item_does_not_change_its_owner(): void
{
$this->seed();
// Attacker is a different logged-in user.
$attacker = $this->actAsCurrentUser();
// A shared item (user_id = 0) is visible to every user.
$item = Item::factory()->create([
'title' => 'Shared Item',
'user_id' => 0,
]);
$response = $this->patch('/items/'.$item->id, [
'appid' => 'null',
'title' => 'Shared Item Edited',
'url' => 'http://example.test',
'user_id' => $attacker->id, // crafted mass-assignment attempt
'tags' => [0],
]);
$response->assertRedirect(route('dash'));
$fresh = Item::withoutGlobalScopes()->find($item->id);
// Ownership is unchanged despite the crafted user_id field...
$this->assertSame(0, (int) $fresh->user_id);
// ...but the rest of the edit still applied.
$this->assertSame('Shared Item Edited', $fresh->title);
}
public function test_updating_an_owned_item_does_not_change_its_owner(): void
{
$this->seed();
$owner = $this->actAsCurrentUser();
$item = Item::factory()->create([
'title' => 'Owned Item',
'user_id' => $owner->id,
]);
$response = $this->patch('/items/'.$item->id, [
'appid' => 'null',
'title' => 'Owned Item Edited',
'url' => 'http://example.test',
'user_id' => 999, // crafted mass-assignment attempt
'tags' => [0],
]);
$response->assertRedirect(route('dash'));
$fresh = Item::withoutGlobalScopes()->find($item->id);
$this->assertSame($owner->id, (int) $fresh->user_id);
$this->assertSame('Owned Item Edited', $fresh->title);
}
}
+52
View File
@@ -0,0 +1,52 @@
<?php
namespace Tests\Feature;
use App\Item;
use App\User;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Tests\TestCase;
class OrphanItemRecoveryTest extends TestCase
{
use RefreshDatabase;
/**
* RefreshDatabase already runs every migration up-front, so the orphans
* are created afterwards and the data migration is invoked directly.
*/
public function test_reassigns_orphaned_items_but_leaves_shared_and_valid_items(): void
{
$this->seed();
// A surviving, valid owner.
$validOwner = User::factory()->create();
// Orphan: user_id references a user that does not exist.
$orphan = Item::factory()->create([
'title' => 'Orphaned Item',
'user_id' => 999,
]);
// Shared items (user_id = 0) must never be reassigned.
$shared = Item::factory()->create([
'title' => 'Shared Item',
'user_id' => 0,
]);
// A validly-owned item must be left untouched.
$valid = Item::factory()->create([
'title' => 'Valid Item',
'user_id' => $validOwner->id,
]);
$migration = include database_path('migrations/2026_07_09_120000_reassign_orphaned_items.php');
$migration->up();
// Orphan reassigned to the surviving admin (id 1).
$this->assertSame(1, (int) Item::withoutGlobalScopes()->find($orphan->id)->user_id);
// Shared and valid rows unchanged.
$this->assertSame(0, (int) Item::withoutGlobalScopes()->find($shared->id)->user_id);
$this->assertSame($validOwner->id, (int) Item::withoutGlobalScopes()->find($valid->id)->user_id);
}
}
+79
View File
@@ -0,0 +1,79 @@
<?php
namespace Tests\Feature;
use App\Item;
use App\User;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Tests\TestCase;
class UserDeleteItemsTest extends TestCase
{
use RefreshDatabase;
public function test_deleting_a_user_hard_deletes_only_that_users_items(): void
{
$this->seed();
// Admin (id 1, passwordless) is the only user allowed to manage users.
$this->withSession(['current_user' => User::find(1)]);
$victim = User::factory()->create();
// The victim's own tile and tag should both be hard-deleted.
$victimTile = Item::factory()->create([
'title' => 'Victim Tile',
'type' => 0,
'user_id' => $victim->id,
]);
$victimTag = Item::factory()->create([
'title' => 'Victim Tag',
'type' => 1,
'user_id' => $victim->id,
]);
// A shared item and another user's item must be left untouched.
$sharedItem = Item::factory()->create([
'title' => 'Shared Item',
'type' => 0,
'user_id' => 0,
]);
$adminItem = Item::factory()->create([
'title' => 'Admin Item',
'type' => 0,
'user_id' => 1,
]);
$response = $this->delete(route('users.destroy', $victim->id));
$response->assertRedirect(route('dash'));
// Victim and their items are gone entirely (force-deleted, not soft-deleted).
$this->assertDatabaseMissing('users', ['id' => $victim->id]);
$this->assertNull(Item::withoutGlobalScopes()->withTrashed()->find($victimTile->id));
$this->assertNull(Item::withoutGlobalScopes()->withTrashed()->find($victimTag->id));
// Shared and other users' items survive.
$this->assertNotNull(Item::withoutGlobalScopes()->find($sharedItem->id));
$this->assertNotNull(Item::withoutGlobalScopes()->find($adminItem->id));
}
public function test_user_id_one_cannot_be_deleted(): void
{
$this->seed();
$this->withSession(['current_user' => User::find(1)]);
$adminItem = Item::factory()->create([
'title' => 'Admin Item',
'type' => 0,
'user_id' => 1,
]);
$this->delete(route('users.destroy', 1));
// The admin and their items remain.
$this->assertDatabaseHas('users', ['id' => 1]);
$this->assertNotNull(Item::withoutGlobalScopes()->find($adminItem->id));
}
}
+38 -1
View File
@@ -6,5 +6,42 @@ use Illuminate\Foundation\Testing\TestCase as BaseTestCase;
abstract class TestCase extends BaseTestCase
{
//
protected function setUp(): void
{
parent::setUp();
$this->guardAgainstRealDatabase();
}
/**
* Refuse to run the test suite against anything other than an
* in-memory SQLite database.
*
* The suite uses RefreshDatabase (migrate:fresh), which would wipe
* whatever database it is pointed at. The only supported test
* configuration for this repo is sqlite + ':memory:'. Any other
* connection (a real sqlite file, mysql, pgsql, ...) is rejected so
* we can never destroy real data.
*/
private function guardAgainstRealDatabase(): void
{
$default = config('database.default');
$driver = config("database.connections.{$default}.driver");
$database = config("database.connections.{$default}.database");
if ($driver === 'sqlite' && $database === ':memory:') {
return;
}
throw new \RuntimeException(sprintf(
'Refusing to run tests: the default database connection (%s) is '
. 'driver "%s" pointing at "%s". Tests only run against an '
. 'in-memory SQLite database (driver "sqlite", database ":memory:") '
. 'to avoid wiping real data via RefreshDatabase. Check phpunit.xml '
. 'DB_CONNECTION/DB_DATABASE overrides.',
$default,
$driver,
is_scalar($database) ? (string) $database : gettype($database)
));
}
}