feat: resource-scoped server access permissions for Viewer/Demo users#259
feat: resource-scoped server access permissions for Viewer/Demo users#259kayvanaarssen wants to merge 5 commits intoDavid-Crty:mainfrom
Conversation
Closes David-Crty#257 Implements per-server and per-database access control for Viewer/Demo users, allowing admins to grant clients access to only the servers and databases they need — without promoting them to the Member role. New features: - user_server_accesses pivot table with per-server grants - Optional allowed_databases JSON column (null = all databases on server) - Per-grant action flags: can_download, can_backup, can_restore - Server Access admin UI embedded in the user edit page - Live database autocomplete from past snapshot history - Add/remove individual database restrictions - Scoped query filtering across server, snapshot, and job listings - Restore button on backup job rows (opens modal pre-selecting snapshot) - Worker container now runs as application user (fixes backup file permissions) Architecture: - Admin/Member roles are unaffected and always see everything - Viewer with no grants: unchanged behaviour (read-only, all servers) - Viewer with at least one grant: becomes a scoped user, visibility restricted to granted servers/databases across all index pages and policies - User::isScopedUser() and applyScopedSnapshotFilter() are reusable entry points for future scoping requirements
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a per-user server access system with grant records, migrations, factory, model relations and helpers, updates policies and query builders to enforce scoped visibility/permissions, introduces an admin Livewire UI to manage grants, and wires restore flows to respect scoped access and database-type restrictions. ChangesUser-Scoped Access Control & Restore Wiring
Sequence DiagramssequenceDiagram
actor Admin
participant ServerAccess as ServerAccess Component
participant DB as Database
participant Policy as Authorization Policy
Admin->>ServerAccess: openGrantModal()
ServerAccess->>Policy: authorize('update', User)
Policy-->>ServerAccess: authorized
Admin->>ServerAccess: grantAccess(selectedServer, allowedDatabases, flags)
ServerAccess->>DB: UserServerAccess::updateOrCreate(...)
DB-->>ServerAccess: saved
ServerAccess-->>Admin: success toast
sequenceDiagram
actor ScopedUser as Scoped User
participant BackupJob as BackupJob Index
participant Modal as RestoreModal
participant Query as SnapshotQuery
participant Policy as SnapshotPolicy
participant DB as Database
ScopedUser->>BackupJob: click "Restore" on job
BackupJob->>Modal: confirmRestoreFromJob(serverId, snapshotId)
Modal->>Policy: authorize('restore', server)
Policy-->>Modal: authorized/denied
alt authorized
Modal->>Query: buildFromParams(..., scopedUser)
Query->>DB: fetch snapshot filtered by grants
DB-->>Query: snapshot
Query-->>Modal: snapshot found
Modal-->>ScopedUser: show restore modal pre-selected
else denied
Modal-->>ScopedUser: authorization denied
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 6/8 reviews remaining, refill in 7 minutes and 32 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/Queries/DatabaseServerQuery.php (1)
49-58:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftScope the server-level counts for database-restricted users too.
The row filter hides ungranted servers, but
withCount('snapshots')andrestores_countstill aggregate all activity on a granted server. For a user restricted to specificallowed_databases, that can expose counts for other databases on the same server.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Queries/DatabaseServerQuery.php` around lines 49 - 58, The counts are not being filtered by a database-restricted user's allowed databases; update the snapshot and restore counts to be scoped when $scopedUser is database-restricted: replace withCount('snapshots') with withCount(['snapshots as snapshots_count' => function (Builder $q) use ($scopedUser) { if ($scopedUser->isDatabaseRestricted()) $q->whereIn('database_id', $scopedUser->getAllowedDatabaseIds()); }]) and change the restores_count subquery (Restore::selectRaw(...)) to add the same database filter (e.g., ->whereIn('database_id', $scopedUser->getAllowedDatabaseIds()) when $scopedUser->isDatabaseRestricted()) inside the when($scopedUser, ...) scope so both counts only include databases the user is allowed to see.
🧹 Nitpick comments (5)
docker-compose.yml (1)
31-31: Both app and worker services have hardcoded UID/GID; consider making them truly configurable for portability.Line 31 hardcodes the worker to
1000:1000, but the app service (lines 18–19) also hardcodes these same values. While comments suggest they should be customizable, neither service currently uses environment variable interpolation. On hosts where the effective developer UID/GID differs, this can cause permission issues for backup/restore operations.Consider updating both services to use interpolated values:
Suggested approach
worker: - user: "1000:1000" + user: "${UID:-1000}:${GID:-1000}" app: - UID: 1000 - GID: 1000 + UID: ${UID:-1000} + GID: ${GID:-1000}Then set
UIDandGIDin a.envfile or pass them viadocker-compose -e.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-compose.yml` at line 31, The docker-compose services "app" and "worker" currently hardcode user: "1000:1000"; change both service definitions to use environment-variable interpolation (e.g., user: "${UID}:${GID}") so the UID/GID become configurable, and document that UID/GID should be provided via a .env file or docker-compose environment (or passed on the CLI) so backup/restore permission issues are avoided.tests/Feature/User/ServerAccessTest.php (1)
94-98: ⚡ Quick winUse the new
UserServerAccessFactorystates here.These cases hand-write
allowed_databases,can_restore, andcan_downloadeven though the factory now exposeswithDatabases()/canRestore(), andcan_download = trueis already the default. Switching to the states will keep these tests aligned with the factory contract and make intent clearer.Example
- UserServerAccess::factory()->create([ - 'user_id' => $viewer->id, - 'database_server_id' => $server->id, - 'allowed_databases' => ['client_db'], - 'can_download' => true, - ]); + UserServerAccess::factory() + ->for($viewer) + ->for($server, 'databaseServer') + ->withDatabases('client_db') + ->create();As per coding guidelines, "When creating models for tests, use factories. Check if the factory has custom states before manually setting up the model."
Also applies to: 107-111, 123-127, 138-143, 154-159, 170-175
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Feature/User/ServerAccessTest.php` around lines 94 - 98, Replace direct attribute assignments on UserServerAccess::factory() with the factory's provided states: use withDatabases([...]) to set allowed_databases and use canRestore() (or a negated state if available, e.g., withoutRestore()/cannotRestore()) to set can_restore instead of manually passing those attributes; omit setting can_download because it's true by default. Update the instances at the shown locations (the factory calls around the symbols UserServerAccess::factory() in this test and the other occurrences listed) to call withDatabases(...) and the appropriate canRestore/cannotRestore state so tests follow the factory contract and intent.app/Livewire/User/ServerAccess.php (3)
20-21: ⚡ Quick win
#[Locked]is redundant on Eloquent model properties.Livewire already protects model properties from client-side ID tampering; keep
#[Locked]for scalar props only.Based on learnings, "In Livewire components, Eloquent model properties ... are automatically locked ... The
#[Locked]attribute is only needed for scalar properties."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Livewire/User/ServerAccess.php` around lines 20 - 21, The #[Locked] attribute on the Eloquent model property should be removed: edit the Livewire component class ServerAccess and delete the #[Locked] annotation from the public User $user property (leave the property declaration intact); retain #[Locked] only for scalar properties if needed elsewhere, since Livewire already auto-locks Eloquent model properties against client-side ID tampering.
109-114: ⚡ Quick winMove validation rules to
#[Validate]attributes (or a Form object).This component validates correctly, but the repository standard for Livewire is attribute/form-based rules with
$this->validate()at execution time.Suggested direction
+ use Livewire\Attributes\Validate; ... - public string $selectedServerId = ''; + #[Validate('required|exists:database_servers,id')] + public string $selectedServerId = ''; - public bool $canDownload = true; + #[Validate('boolean')] + public bool $canDownload = true;As per coding guidelines, "In Livewire components, use
#[Validate]attributes or Form objects for validation. Call$this->validate()before processing data."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Livewire/User/ServerAccess.php` around lines 109 - 114, Move the inline rules from the $this->validate() call into Livewire attribute-based validation or a Form object: add #[Validate] attributes (or define a Form with rules) on the component properties selectedServerId, canDownload, canBackup, and canRestore in the ServerAccess Livewire component, then remove the hard-coded array from the current validate() call and ensure $this->validate() is invoked immediately before any processing (e.g., before saving or dispatching) so runtime validation still occurs; reference the properties selectedServerId, canDownload, canBackup, canRestore and the ServerAccess component when making the change.
69-79: ⚡ Quick winReactive database search should be bounded for large snapshot tables.
This query is called during live typing and currently does a full distinct+order path without a cap. Add a small
limit(...)(and pair with debounce in the input binding) to avoid heavy interactive queries.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Livewire/User/ServerAccess.php` around lines 69 - 79, The Snapshot query in the Livewire component (ServerAccess) is unbounded and can be expensive during reactive typing; add a small limit (e.g., limit(50)) to the query chain (place it after orderBy('database_name') and before pluck('database_name')) so distinct/order/limit happen in SQL, and keep the existing filter/values/all() logic; also ensure the Livewire input binding for $databaseSearch uses a debounce (e.g., wire:model.debounce.300ms) to reduce query frequency while typing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/Livewire/DatabaseServer/RestoreModal.php`:
- Around line 99-109: openFromSnapshot currently trusts the client-supplied
$snapshotId after only authorizing the target server; instead, load and
authorize the snapshot using the same scoped query path used later (the query
around Snapshot::findOrFail in the restore/select logic) before preselecting it
— e.g., fetch the snapshot via the target server's scoped relationship or the
controller's existing scoped query (matching the logic in the code around lines
264-270), verify it belongs to/is compatible with $this->targetServer, then set
selectedSnapshotId and call selectSnapshot; update openFromSnapshot to use that
scoped fetch (and fail/abort if not found or incompatible) rather than trusting
the raw ID.
In `@app/Livewire/User/ServerAccess.php`:
- Around line 151-155: availableServerOptions() currently excludes
already-granted servers via whereNotIn('id', $grantedIds), which prevents the UI
from selecting an existing server when editing a grant; change
availableServerOptions (and its callers like grantAccess()) to accept an
optional $currentServerId (nullable) and when present remove that id from
$grantedIds before applying whereNotIn so the server tied to the grant being
edited remains selectable (i.e., only exclude other granted ids); ensure
grantAccess() passes the current grant's database_server_id when editing so
updateOrCreate() can update existing grants.
In `@app/Policies/DatabaseServerPolicy.php`:
- Around line 91-99: The policy's restore(User $user, DatabaseServer
$databaseServer) currently only checks role/grant flags and must also enforce
the same agent/Redis constraints the UI uses; update restore to refuse restores
for servers that the UI blocks by calling the same predicate(s) the
UI/controller uses (e.g. reuse or call the existing logic in
DatabaseServerController::restore or a DatabaseServer model method such as
isRestorable()/isAgentManaged()/isRedisOnly() that mirrors the UI checks) in
addition to the existing scoped-user and demo/canPerformActions checks so API
restores are prevented for non-restorable servers.
In `@app/Queries/BackupJobQuery.php`:
- Around line 97-109: The restore branch only filters by target_server_id and
ignores $access->allowed_databases, leaking jobs; update the
orWhereHas('restore', ...) closure in BackupJobQuery to also apply the allowlist
when $access->allowed_databases !== null (e.g. add a whereIn on the restore's
database column using $access->allowed_databases), keeping the existing
target_server_id check and preserving behavior when allowed_databases is null;
reference the scopedUser->serverAccesses(), the snapshot and restore relations,
and the $access->allowed_databases property when making the change.
In `@resources/views/livewire/backup-job/index.blade.php`:
- Around line 146-150: The x-button component's tooltip attribute is using Blade
interpolation which double-encodes translated text; update the attribute on the
<x-button> that calls confirmRestoreFromJob (the button with icon="o-arrow-path"
and wire:click="confirmRestoreFromJob(...)") to use dynamic binding by changing
tooltip="{{ __('Restore') }}" to :tooltip="__('Restore')" so the translated
string is passed as a raw attribute.
In `@resources/views/livewire/user/server-access.blade.php`:
- Line 17: Replace Blade interpolation of translated strings inside component
attributes with dynamic :attr bindings to avoid double-encoding; e.g., change
label="{{ __('Grant Access') }}" to use :label="__('Grant Access')", and apply
the same change for all other translated attributes in this view
(titles/labels/placeholders such as "Revoke Access", "Search", "Save", "Cancel",
and the other occurrences noted). Update each component attribute in
server-access.blade.php that currently uses {{ __('...') }} so it uses the
colon-prefixed dynamic syntax :attribute="__('...')" while preserving the same
translation keys/function calls and existing attribute names.
---
Outside diff comments:
In `@app/Queries/DatabaseServerQuery.php`:
- Around line 49-58: The counts are not being filtered by a database-restricted
user's allowed databases; update the snapshot and restore counts to be scoped
when $scopedUser is database-restricted: replace withCount('snapshots') with
withCount(['snapshots as snapshots_count' => function (Builder $q) use
($scopedUser) { if ($scopedUser->isDatabaseRestricted())
$q->whereIn('database_id', $scopedUser->getAllowedDatabaseIds()); }]) and change
the restores_count subquery (Restore::selectRaw(...)) to add the same database
filter (e.g., ->whereIn('database_id', $scopedUser->getAllowedDatabaseIds())
when $scopedUser->isDatabaseRestricted()) inside the when($scopedUser, ...)
scope so both counts only include databases the user is allowed to see.
---
Nitpick comments:
In `@app/Livewire/User/ServerAccess.php`:
- Around line 20-21: The #[Locked] attribute on the Eloquent model property
should be removed: edit the Livewire component class ServerAccess and delete the
#[Locked] annotation from the public User $user property (leave the property
declaration intact); retain #[Locked] only for scalar properties if needed
elsewhere, since Livewire already auto-locks Eloquent model properties against
client-side ID tampering.
- Around line 109-114: Move the inline rules from the $this->validate() call
into Livewire attribute-based validation or a Form object: add #[Validate]
attributes (or define a Form with rules) on the component properties
selectedServerId, canDownload, canBackup, and canRestore in the ServerAccess
Livewire component, then remove the hard-coded array from the current validate()
call and ensure $this->validate() is invoked immediately before any processing
(e.g., before saving or dispatching) so runtime validation still occurs;
reference the properties selectedServerId, canDownload, canBackup, canRestore
and the ServerAccess component when making the change.
- Around line 69-79: The Snapshot query in the Livewire component (ServerAccess)
is unbounded and can be expensive during reactive typing; add a small limit
(e.g., limit(50)) to the query chain (place it after orderBy('database_name')
and before pluck('database_name')) so distinct/order/limit happen in SQL, and
keep the existing filter/values/all() logic; also ensure the Livewire input
binding for $databaseSearch uses a debounce (e.g., wire:model.debounce.300ms) to
reduce query frequency while typing.
In `@docker-compose.yml`:
- Line 31: The docker-compose services "app" and "worker" currently hardcode
user: "1000:1000"; change both service definitions to use environment-variable
interpolation (e.g., user: "${UID}:${GID}") so the UID/GID become configurable,
and document that UID/GID should be provided via a .env file or docker-compose
environment (or passed on the CLI) so backup/restore permission issues are
avoided.
In `@tests/Feature/User/ServerAccessTest.php`:
- Around line 94-98: Replace direct attribute assignments on
UserServerAccess::factory() with the factory's provided states: use
withDatabases([...]) to set allowed_databases and use canRestore() (or a negated
state if available, e.g., withoutRestore()/cannotRestore()) to set can_restore
instead of manually passing those attributes; omit setting can_download because
it's true by default. Update the instances at the shown locations (the factory
calls around the symbols UserServerAccess::factory() in this test and the other
occurrences listed) to call withDatabases(...) and the appropriate
canRestore/cannotRestore state so tests follow the factory contract and intent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eb999027-d924-4ff1-9350-3e83922efe56
📒 Files selected for processing (20)
app/Livewire/BackupJob/Index.phpapp/Livewire/DatabaseServer/Index.phpapp/Livewire/DatabaseServer/RestoreModal.phpapp/Livewire/User/ServerAccess.phpapp/Models/DatabaseServer.phpapp/Models/User.phpapp/Models/UserServerAccess.phpapp/Policies/DatabaseServerPolicy.phpapp/Policies/SnapshotPolicy.phpapp/Queries/BackupJobQuery.phpapp/Queries/DatabaseServerQuery.phpapp/Queries/SnapshotQuery.phpdatabase/factories/UserServerAccessFactory.phpdatabase/migrations/2026_05_02_000001_create_user_server_accesses_table.phpdatabase/migrations/2026_05_02_000002_add_can_backup_to_user_server_accesses_table.phpdocker-compose.ymlresources/views/livewire/backup-job/index.blade.phpresources/views/livewire/user/edit.blade.phpresources/views/livewire/user/server-access.blade.phptests/Feature/User/ServerAccessTest.php
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.php: In PHP files, always use curly braces for control structures, even for single-line bodies.
Use PHP 8 constructor property promotion:public function __construct(public GitHub $github) { }. Do not leave empty zero-parameter__construct()methods unless the constructor is private.
Use explicit return type declarations and type hints for all method parameters in PHP:function isAccessible(User $user, ?string $path = null): bool
Prefer PHPDoc blocks over inline comments. Only add inline comments for exceptionally complex logic.
Use array shape type definitions in PHPDoc blocks for PHP.
Files:
app/Livewire/DatabaseServer/Index.phpdatabase/migrations/2026_05_02_000002_add_can_backup_to_user_server_accesses_table.phpresources/views/livewire/user/edit.blade.phpdatabase/migrations/2026_05_02_000001_create_user_server_accesses_table.phpapp/Models/UserServerAccess.phpapp/Policies/DatabaseServerPolicy.phpapp/Queries/DatabaseServerQuery.phpdatabase/factories/UserServerAccessFactory.phpapp/Livewire/DatabaseServer/RestoreModal.phpapp/Queries/BackupJobQuery.phpapp/Queries/SnapshotQuery.phpresources/views/livewire/backup-job/index.blade.phpapp/Policies/SnapshotPolicy.phpapp/Livewire/BackupJob/Index.phpresources/views/livewire/user/server-access.blade.phpapp/Livewire/User/ServerAccess.phpapp/Models/User.phptests/Feature/User/ServerAccessTest.phpapp/Models/DatabaseServer.php
app/Livewire/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
app/Livewire/**/*.php: In Livewire components, use#[Validate]attributes or Form objects for validation. Call$this->validate()before processing data.
In Livewire components, useSession::flash()for one-time messages and show them via@if (session('success'))in Blade templates.
In Livewire components, return$this->redirect()withnavigate: truefor SPA-like navigation.
In Mary UI modals, add a boolean property to the component class and usewire:modelin the Blade template.
Keep state server-side in Livewire components so the UI reflects it. Validate and authorize in actions as you would in HTTP requests.
Files:
app/Livewire/DatabaseServer/Index.phpapp/Livewire/DatabaseServer/RestoreModal.phpapp/Livewire/BackupJob/Index.phpapp/Livewire/User/ServerAccess.php
**/*.{php,blade.php}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{php,blade.php}: Always use named routes and theroute()function when generating links to other pages.
Translations should use__('...')helper function and be stored in JSON translation files inlang/{locale}.json. Keep technical terms like 'Backup', 'Restore', 'Snapshot(s)' in English across all locales.
Files:
app/Livewire/DatabaseServer/Index.phpdatabase/migrations/2026_05_02_000002_add_can_backup_to_user_server_accesses_table.phpresources/views/livewire/user/edit.blade.phpdatabase/migrations/2026_05_02_000001_create_user_server_accesses_table.phpapp/Models/UserServerAccess.phpapp/Policies/DatabaseServerPolicy.phpapp/Queries/DatabaseServerQuery.phpdatabase/factories/UserServerAccessFactory.phpapp/Livewire/DatabaseServer/RestoreModal.phpapp/Queries/BackupJobQuery.phpapp/Queries/SnapshotQuery.phpresources/views/livewire/backup-job/index.blade.phpapp/Policies/SnapshotPolicy.phpapp/Livewire/BackupJob/Index.phpresources/views/livewire/user/server-access.blade.phpapp/Livewire/User/ServerAccess.phpapp/Models/User.phptests/Feature/User/ServerAccessTest.phpapp/Models/DatabaseServer.php
resources/views/livewire/**/*.blade.php
📄 CodeRabbit inference engine (CLAUDE.md)
All Livewire component blade files should contain only view markup; all PHP logic must be in the component class.
Files:
resources/views/livewire/user/edit.blade.phpresources/views/livewire/backup-job/index.blade.phpresources/views/livewire/user/server-access.blade.php
resources/views/**/*.blade.php
📄 CodeRabbit inference engine (CLAUDE.md)
resources/views/**/*.blade.php: All UI components in Mary UI should be prefixed withx-(e.g.,<x-button>,<x-input>,<x-card>) and use Heroicons for icons (e.g.,icon="o-user"for outline,icon="s-user"for solid).
In Mary UI select components, use the:optionsprop with array format[['id' => 'value', 'name' => 'Label']].
In Mary UI alerts, useclass="alert-success",class="alert-error", etc. (notvariantprop).
Use Alpine.js for client-side interactions in Livewire instead of JavaScript frameworks.
In Blade templates, use:attrbinding (dynamic syntax) instead of{{ }}interpolation when passing translated strings to component attributes to avoid double-encoding special characters.
Files:
resources/views/livewire/user/edit.blade.phpresources/views/livewire/backup-job/index.blade.phpresources/views/livewire/user/server-access.blade.php
tests/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.php: When creating models for tests, use factories. Check if the factory has custom states before manually setting up the model.
Use faker methods such as$this->faker->word()orfake()->randomDigit()when generating test data. Follow existing conventions on whether to use$this->fakerorfake().
Create Pest tests usingphp artisan make:test --pest {name}without including the test suite directory. Usephp artisan make:test --pest SomeFeatureTestinstead ofphp artisan make:test --pest Feature/SomeFeatureTest.
Files:
tests/Feature/User/ServerAccessTest.php
🧠 Learnings (7)
📚 Learning: 2026-02-18T09:45:52.485Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 116
File: app/Livewire/DatabaseServer/ConnectionStatus.php:18-18
Timestamp: 2026-02-18T09:45:52.485Z
Learning: In Livewire components, Eloquent model properties (e.g., public DatabaseServer $server) are automatically locked by the framework to prevent client-side ID tampering. The #[Locked] attribute is only needed for scalar properties (int, string, bool, etc.) that require protection from client-side mutation. Apply this guidance to all Livewire PHP components; use #[Locked] only on primitive properties that you want to shield from client manipulation, and rely on automatic locking for Eloquent model properties.
Applied to files:
app/Livewire/DatabaseServer/Index.phpapp/Livewire/DatabaseServer/RestoreModal.phpapp/Livewire/BackupJob/Index.phpapp/Livewire/User/ServerAccess.php
📚 Learning: 2026-02-25T10:48:17.811Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 132
File: app/Console/Commands/RecoverAgentLeasesCommand.php:44-48
Timestamp: 2026-02-25T10:48:17.811Z
Learning: When reviewing PHP code, especially with foreign keys that use cascadeOnDelete and are non-nullable, assume child relations exist at runtime (the database will delete children when the parent is deleted). Do not rely on null-safe operators for these relations, as PHPStan already models them as non-null. This guideline applies broadly to PHP files that define models with foreign keys using cascade delete; verify there are no unnecessary null checks or optional chaining on such relations.
Applied to files:
app/Livewire/DatabaseServer/Index.phpdatabase/migrations/2026_05_02_000002_add_can_backup_to_user_server_accesses_table.phpresources/views/livewire/user/edit.blade.phpdatabase/migrations/2026_05_02_000001_create_user_server_accesses_table.phpapp/Models/UserServerAccess.phpapp/Policies/DatabaseServerPolicy.phpapp/Queries/DatabaseServerQuery.phpdatabase/factories/UserServerAccessFactory.phpapp/Livewire/DatabaseServer/RestoreModal.phpapp/Queries/BackupJobQuery.phpapp/Queries/SnapshotQuery.phpresources/views/livewire/backup-job/index.blade.phpapp/Policies/SnapshotPolicy.phpapp/Livewire/BackupJob/Index.phpresources/views/livewire/user/server-access.blade.phpapp/Livewire/User/ServerAccess.phpapp/Models/User.phptests/Feature/User/ServerAccessTest.phpapp/Models/DatabaseServer.php
📚 Learning: 2026-04-10T08:46:55.182Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 205
File: database/migrations/2026_04_09_000001_add_notification_channels.php:143-156
Timestamp: 2026-04-10T08:46:55.182Z
Learning: In David-Crty/databasement, treat Laravel migrations as forward-only for production. For migrations in database/migrations/*.php, it is acceptable to delete or transform legacy data in the migration’s up() (e.g., removing app_configs rows) without restoring it in down(), even if down() cannot fully reverse the state. During code review, do not flag missing rollback restoration of migrated/deleted data as a production risk for this repo’s migrations.
Applied to files:
database/migrations/2026_05_02_000002_add_can_backup_to_user_server_accesses_table.phpdatabase/migrations/2026_05_02_000001_create_user_server_accesses_table.php
📚 Learning: 2026-01-30T22:27:46.107Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 61
File: resources/views/livewire/volume/connectors/s3-config.blade.php:1-13
Timestamp: 2026-01-30T22:27:46.107Z
Learning: In Blade template files (any .blade.php) within the databasement project, allow using alert-info for informational content inside <x-alert> components. The guideline that permits alert-success and alert-error does not exclude using alert-info for informational purposes. Apply this consistently to all Blade components that render alerts; ensure semantic usage and accessibility.
Applied to files:
resources/views/livewire/user/edit.blade.phpresources/views/livewire/backup-job/index.blade.phpresources/views/livewire/user/server-access.blade.php
📚 Learning: 2026-02-06T10:34:43.585Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 75
File: resources/views/livewire/backup-job/_filters.blade.php:36-40
Timestamp: 2026-02-06T10:34:43.585Z
Learning: In Blade template files, when creating compact inline filter controls, prefer using native <input type="checkbox"> elements with daisyUI classes (e.g., checkbox checkbox-warning checkbox-xs) over the Mary UI <x-checkbox> component. The <x-checkbox> component adds wrapper markup (e.g., <div><fieldset><label> with gap-3) that can break tight inline flex layouts. Use the native input approach for compact inline controls, but reserve <x-checkbox> for form fields that require labels, hints, and errors.
Applied to files:
resources/views/livewire/user/edit.blade.phpresources/views/livewire/backup-job/index.blade.phpresources/views/livewire/user/server-access.blade.php
📚 Learning: 2026-02-13T11:05:37.072Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 105
File: tests/Feature/Services/Backup/Databases/SqliteDatabaseTest.php:8-11
Timestamp: 2026-02-13T11:05:37.072Z
Learning: Adopt a global afterEach hook in tests/Pest.php (or equivalent Pest bootstrap) to clean up temporary directories created during tests. Specifically handle temp dirs named with the prefixes sqlite-db-test-*, backup-task-test-*, restore-task-test-*, and volume-test-*, so individual test files don’t need their own cleanup logic. This applies to all PHP test files under the tests directory.
Applied to files:
tests/Feature/User/ServerAccessTest.php
📚 Learning: 2026-04-09T13:59:25.873Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 205
File: tests/Feature/ConfigurationTest.php:165-176
Timestamp: 2026-04-09T13:59:25.873Z
Learning: In this repository, `Notification::fake()` is already performed globally in the base test class (`tests/TestCase.php`) within `setUp()` before each test. When reviewing individual test files under `tests/`, do not flag missing `Notification::fake()` calls, since they are handled by the shared base test setup.
Applied to files:
tests/Feature/User/ServerAccessTest.php
|
Hey @kayvanaarssen thanks for your suggestion and PR, one thing that I was thinking in order to simplify this would be to create a new "global" model named "Organization". Current models would be impacted and attach to an organization:
Would this organzation feature match your needs ? Not sure if I made my self clear about this approach. |
TL;DR: Built a PoC showing it's feasible. Need a backup tool that lets devs access/restore specific client DBs — current mysqldump + restic + S3 setup doesn't support that. Found a project that fits. Yeah that's maybe even better - i've just created this with a little help of Ai as a PoC to see if it could be done within the current scope and it is. We are facing questions from time to time from devs working on clients sites if they can make extra backups / restore or download them. So came across your project and its exactly what we are looking for. Back to your reply. For origanisation i would still opt for;
And then with the options - got some screenshots here. Happy to work on this feature with you to facilitate the extra options 
|
|
We would need to this in 2 separated steps and PR because thouse changes have a big impact
Feel free to explore the options I will check this deeper next week. |
Amazing! Thanks for the positive feedback on this! |
- Verify snapshot accessibility via scoped SnapshotQuery in RestoreModal::openFromSnapshot() before pre-selecting, preventing a scoped user from opening a snapshot they cannot access by supplying an arbitrary ULID through the Livewire event - Apply allowed_databases filter to restore jobs in BackupJobQuery so scoped users cannot see restore jobs for databases outside their grants - Scope snapshot and restore counts in DatabaseServerQuery to the union of allowed databases for scoped users, preventing count leakage on the server index - Support editing existing grants in ServerAccess: openGrantModal() now accepts an optional access ID, pre-fills the form, and keeps the server selectable by excluding it from the already-granted exclusion list; pencil edit button added alongside revoke - Use :tooltip binding (dynamic syntax) on the restore button in the jobs table to avoid HTML double-encoding
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/Queries/DatabaseServerQuery.php`:
- Around line 56-79: The bug is that $allAllowedDbs flattens grants across all
servers and then reuses that global list for every database_servers row; instead
build per-server correlated filters so each database_server uses only its own
serverAccess row(s). Replace the flattened $allAllowedDbs logic with correlated
subqueries/conditions: for snapshots use withCount(['snapshots' => fn(Builder
$q) => $q->whereExists(...)]) and for restores replace the
Restore::selectRaw(...) with a correlated subselect that joins/filters
server_accesses for the current database_server (match database_servers.id to
server_accesses.database_server_id and scoped user id), and enforce
(server_accesses.allowed_databases IS NULL) OR (the snapshot/schema name is
contained in server_accesses.allowed_databases) so unrestricted grants count all
DBs while restricted grants only count DB names for that specific server; keep
using serverAccesses(), allowed_databases, snapshots, and Restore as the
reference points when implementing these correlated filters.
- Around line 51-55: The when() is applied for any non-null User and will hide
all servers when getAccessibleServerIds() is empty; change the predicate so the
whereIn('id', $scopedUser->getAccessibleServerIds()) is only applied for
actually scoped users (e.g. replace the condition with $scopedUser !== null &&
$scopedUser->isScoped() or the project’s equivalent scoped-user predicate),
keeping the closure using Builder and getAccessibleServerIds(); if your
predicate name differs, call that predicate method inside the when() first
argument so Admin/Member and un-granted Viewers keep the global listing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d566a11c-3f7e-44cc-bab3-bd69c488055c
📒 Files selected for processing (6)
app/Livewire/DatabaseServer/RestoreModal.phpapp/Livewire/User/ServerAccess.phpapp/Queries/BackupJobQuery.phpapp/Queries/DatabaseServerQuery.phpresources/views/livewire/backup-job/index.blade.phpresources/views/livewire/user/server-access.blade.php
✅ Files skipped from review due to trivial changes (1)
- resources/views/livewire/user/server-access.blade.php
🚧 Files skipped from review as they are similar to previous changes (4)
- resources/views/livewire/backup-job/index.blade.php
- app/Livewire/DatabaseServer/RestoreModal.php
- app/Livewire/User/ServerAccess.php
- app/Queries/BackupJobQuery.php
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.php: In PHP files, always use curly braces for control structures, even for single-line bodies.
Use PHP 8 constructor property promotion:public function __construct(public GitHub $github) { }. Do not leave empty zero-parameter__construct()methods unless the constructor is private.
Use explicit return type declarations and type hints for all method parameters in PHP:function isAccessible(User $user, ?string $path = null): bool
Prefer PHPDoc blocks over inline comments. Only add inline comments for exceptionally complex logic.
Use array shape type definitions in PHPDoc blocks for PHP.
Files:
app/Queries/DatabaseServerQuery.php
**/*.{php,blade.php}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{php,blade.php}: Always use named routes and theroute()function when generating links to other pages.
Translations should use__('...')helper function and be stored in JSON translation files inlang/{locale}.json. Keep technical terms like 'Backup', 'Restore', 'Snapshot(s)' in English across all locales.
Files:
app/Queries/DatabaseServerQuery.php
🧠 Learnings (1)
📚 Learning: 2026-02-25T10:48:17.811Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 132
File: app/Console/Commands/RecoverAgentLeasesCommand.php:44-48
Timestamp: 2026-02-25T10:48:17.811Z
Learning: When reviewing PHP code, especially with foreign keys that use cascadeOnDelete and are non-nullable, assume child relations exist at runtime (the database will delete children when the parent is deleted). Do not rely on null-safe operators for these relations, as PHPStan already models them as non-null. This guideline applies broadly to PHP files that define models with foreign keys using cascade delete; verify there are no unnecessary null checks or optional chaining on such relations.
Applied to files:
app/Queries/DatabaseServerQuery.php
- Block agent-backed server restores at the policy level; add Redis and
agent guards to the API restore endpoint so automated restore cannot be
triggered via API for unsupported server types (Redis manual-instructions
UX in the UI is preserved since the policy now only blocks agents)
- Gate the scoped-user query branch in DatabaseServerQuery on
isScopedUser() so Admin/Member users passed as scopedUser never receive
a restricted listing; replace the flat allowed-databases union with
per-server correlated OR-WHERE conditions so snapshot and restore counts
are filtered by each server's own grant rather than a merged global list
- Convert {{ __('...') }} interpolation to :attr="__('...')" dynamic
bindings throughout server-access.blade.php to avoid double-encoding of
translated component attributes
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/Http/Controllers/Api/V1/DatabaseServerController.php`:
- Around line 199-205: The controller currently rejects Redis/Valkey restores in
DatabaseServerController (restore path) but DatabaseServerPolicy::restore()
still permits them, causing authorize('restore') to succeed then fail with 422;
update DatabaseServerPolicy::restore() to include the same check (deny when
$databaseServer->database_type === DatabaseType::REDIS or Valkey-equivalent) so
authorization and controller behavior match, and then remove the duplicate
Redis/Valkey check from DatabaseServerController to keep logic centralized and
consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4799d170-cbf4-4f27-a0ae-e8e8f26addea
📒 Files selected for processing (4)
app/Http/Controllers/Api/V1/DatabaseServerController.phpapp/Policies/DatabaseServerPolicy.phpapp/Queries/DatabaseServerQuery.phpresources/views/livewire/user/server-access.blade.php
✅ Files skipped from review due to trivial changes (1)
- resources/views/livewire/user/server-access.blade.php
🚧 Files skipped from review as they are similar to previous changes (1)
- app/Policies/DatabaseServerPolicy.php
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.php: In PHP files, always use curly braces for control structures, even for single-line bodies.
Use PHP 8 constructor property promotion:public function __construct(public GitHub $github) { }. Do not leave empty zero-parameter__construct()methods unless the constructor is private.
Use explicit return type declarations and type hints for all method parameters in PHP:function isAccessible(User $user, ?string $path = null): bool
Prefer PHPDoc blocks over inline comments. Only add inline comments for exceptionally complex logic.
Use array shape type definitions in PHPDoc blocks for PHP.
Files:
app/Http/Controllers/Api/V1/DatabaseServerController.phpapp/Queries/DatabaseServerQuery.php
**/*.{php,blade.php}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{php,blade.php}: Always use named routes and theroute()function when generating links to other pages.
Translations should use__('...')helper function and be stored in JSON translation files inlang/{locale}.json. Keep technical terms like 'Backup', 'Restore', 'Snapshot(s)' in English across all locales.
Files:
app/Http/Controllers/Api/V1/DatabaseServerController.phpapp/Queries/DatabaseServerQuery.php
🧠 Learnings (2)
📚 Learning: 2026-02-25T10:48:17.811Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 132
File: app/Console/Commands/RecoverAgentLeasesCommand.php:44-48
Timestamp: 2026-02-25T10:48:17.811Z
Learning: When reviewing PHP code, especially with foreign keys that use cascadeOnDelete and are non-nullable, assume child relations exist at runtime (the database will delete children when the parent is deleted). Do not rely on null-safe operators for these relations, as PHPStan already models them as non-null. This guideline applies broadly to PHP files that define models with foreign keys using cascade delete; verify there are no unnecessary null checks or optional chaining on such relations.
Applied to files:
app/Http/Controllers/Api/V1/DatabaseServerController.phpapp/Queries/DatabaseServerQuery.php
📚 Learning: 2026-03-23T11:45:15.846Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 178
File: app/Http/Controllers/Api/V1/VolumeController.php:116-120
Timestamp: 2026-03-23T11:45:15.846Z
Learning: For the API v1 `testConnection` endpoints in this repo (e.g., `VolumeController::testConnection`, `DatabaseServerController::testConnection`), keep using `$this->authorize('view', $model)` for authorization. This is an intentional maintainer design choice; do not flag it as a bug or as insufficiently strict authorization in future reviews for these `testConnection` actions.
Applied to files:
app/Http/Controllers/Api/V1/DatabaseServerController.php
🔇 Additional comments (4)
app/Queries/DatabaseServerQuery.php (4)
51-55: Scoped gating now correctly protects Admin/Member and ungranted Viewer behavior.Good fix on Line 52: the branch only applies access restriction when
isScopedUser()is true, which matches the PR contract.
61-72: Snapshot counting is now properly grant-correlated per server.The nested OR structure on Line 62-Line 71 keeps snapshot counting tied to each explicit access grant instead of using a global merged DB list.
74-86:restores_countcorrelation is aligned with per-server access rules.Nice update: Line 75 plus the scoped OR branches on Line 76-Line 85 keep restore counts constrained to server grant + optional schema allowlist.
88-93: Fallback branch preserves existing global behavior for non-scoped users.Line 88-Line 93 cleanly retains the previous unscoped counting behavior, which keeps this change isolated to scoped users.
Move the Redis/Valkey automated-restore restriction from the API controller into DatabaseServerPolicy::restore() alongside the existing agent check, so authorization is the single source of truth for both the UI and the API. The Redis manual-instructions flow is preserved by checking for Redis before the authorize() call in Index::confirmRestore(), since showing documentation does not dispatch a restore job. The blade button is similarly split so Redis servers show a "View Restore Instructions" button outside the @can('restore') gate, while all other servers remain policy-gated as before. The now-redundant 422 guards in the API controller are removed.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/Livewire/DatabaseServer/Index.php`:
- Around line 115-126: The Redis branch currently returns before authorization
and before setting component state, allowing unauthorized access and leaving
$this->restoreId stale; call $this->authorize('restore', $server) first, then
set $this->restoreId = $id, and only after those do the DatabaseType::REDIS
check and set $this->showRedisRestoreModal = true with an early return;
reference the authorize('restore', $server) call, $this->restoreId assignment,
$this->showRedisRestoreModal flag and $server->database_type to locate and
update the code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bc296389-5792-4281-a44a-5bb4ec5861d5
📒 Files selected for processing (3)
app/Livewire/DatabaseServer/Index.phpapp/Policies/DatabaseServerPolicy.phpresources/views/livewire/database-server/index.blade.php
🚧 Files skipped from review as they are similar to previous changes (1)
- app/Policies/DatabaseServerPolicy.php
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.php: In PHP files, always use curly braces for control structures, even for single-line bodies.
Use PHP 8 constructor property promotion:public function __construct(public GitHub $github) { }. Do not leave empty zero-parameter__construct()methods unless the constructor is private.
Use explicit return type declarations and type hints for all method parameters in PHP:function isAccessible(User $user, ?string $path = null): bool
Prefer PHPDoc blocks over inline comments. Only add inline comments for exceptionally complex logic.
Use array shape type definitions in PHPDoc blocks for PHP.
Files:
resources/views/livewire/database-server/index.blade.phpapp/Livewire/DatabaseServer/Index.php
resources/views/livewire/**/*.blade.php
📄 CodeRabbit inference engine (CLAUDE.md)
All Livewire component blade files should contain only view markup; all PHP logic must be in the component class.
Files:
resources/views/livewire/database-server/index.blade.php
resources/views/**/*.blade.php
📄 CodeRabbit inference engine (CLAUDE.md)
resources/views/**/*.blade.php: All UI components in Mary UI should be prefixed withx-(e.g.,<x-button>,<x-input>,<x-card>) and use Heroicons for icons (e.g.,icon="o-user"for outline,icon="s-user"for solid).
In Mary UI select components, use the:optionsprop with array format[['id' => 'value', 'name' => 'Label']].
In Mary UI alerts, useclass="alert-success",class="alert-error", etc. (notvariantprop).
Use Alpine.js for client-side interactions in Livewire instead of JavaScript frameworks.
In Blade templates, use:attrbinding (dynamic syntax) instead of{{ }}interpolation when passing translated strings to component attributes to avoid double-encoding special characters.
Files:
resources/views/livewire/database-server/index.blade.php
**/*.{php,blade.php}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{php,blade.php}: Always use named routes and theroute()function when generating links to other pages.
Translations should use__('...')helper function and be stored in JSON translation files inlang/{locale}.json. Keep technical terms like 'Backup', 'Restore', 'Snapshot(s)' in English across all locales.
Files:
resources/views/livewire/database-server/index.blade.phpapp/Livewire/DatabaseServer/Index.php
app/Livewire/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
app/Livewire/**/*.php: In Livewire components, use#[Validate]attributes or Form objects for validation. Call$this->validate()before processing data.
In Livewire components, useSession::flash()for one-time messages and show them via@if (session('success'))in Blade templates.
In Livewire components, return$this->redirect()withnavigate: truefor SPA-like navigation.
In Mary UI modals, add a boolean property to the component class and usewire:modelin the Blade template.
Keep state server-side in Livewire components so the UI reflects it. Validate and authorize in actions as you would in HTTP requests.
Files:
app/Livewire/DatabaseServer/Index.php
🧠 Learnings (4)
📚 Learning: 2026-01-30T22:27:46.107Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 61
File: resources/views/livewire/volume/connectors/s3-config.blade.php:1-13
Timestamp: 2026-01-30T22:27:46.107Z
Learning: In Blade template files (any .blade.php) within the databasement project, allow using alert-info for informational content inside <x-alert> components. The guideline that permits alert-success and alert-error does not exclude using alert-info for informational purposes. Apply this consistently to all Blade components that render alerts; ensure semantic usage and accessibility.
Applied to files:
resources/views/livewire/database-server/index.blade.php
📚 Learning: 2026-02-06T10:34:43.585Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 75
File: resources/views/livewire/backup-job/_filters.blade.php:36-40
Timestamp: 2026-02-06T10:34:43.585Z
Learning: In Blade template files, when creating compact inline filter controls, prefer using native <input type="checkbox"> elements with daisyUI classes (e.g., checkbox checkbox-warning checkbox-xs) over the Mary UI <x-checkbox> component. The <x-checkbox> component adds wrapper markup (e.g., <div><fieldset><label> with gap-3) that can break tight inline flex layouts. Use the native input approach for compact inline controls, but reserve <x-checkbox> for form fields that require labels, hints, and errors.
Applied to files:
resources/views/livewire/database-server/index.blade.php
📚 Learning: 2026-02-25T10:48:17.811Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 132
File: app/Console/Commands/RecoverAgentLeasesCommand.php:44-48
Timestamp: 2026-02-25T10:48:17.811Z
Learning: When reviewing PHP code, especially with foreign keys that use cascadeOnDelete and are non-nullable, assume child relations exist at runtime (the database will delete children when the parent is deleted). Do not rely on null-safe operators for these relations, as PHPStan already models them as non-null. This guideline applies broadly to PHP files that define models with foreign keys using cascade delete; verify there are no unnecessary null checks or optional chaining on such relations.
Applied to files:
resources/views/livewire/database-server/index.blade.phpapp/Livewire/DatabaseServer/Index.php
📚 Learning: 2026-02-18T09:45:52.485Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 116
File: app/Livewire/DatabaseServer/ConnectionStatus.php:18-18
Timestamp: 2026-02-18T09:45:52.485Z
Learning: In Livewire components, Eloquent model properties (e.g., public DatabaseServer $server) are automatically locked by the framework to prevent client-side ID tampering. The #[Locked] attribute is only needed for scalar properties (int, string, bool, etc.) that require protection from client-side mutation. Apply this guidance to all Livewire PHP components; use #[Locked] only on primitive properties that you want to shield from client manipulation, and rely on automatic locking for Eloquent model properties.
Applied to files:
app/Livewire/DatabaseServer/Index.php
🔇 Additional comments (1)
app/Livewire/DatabaseServer/Index.php (1)
203-210: Nice centralization of scoped listing logic.Passing the scoped user into
DatabaseServerQuery::buildFromParams()keeps the visibility rules in one place instead of reimplementing them in the component.
…estore
Add authorize('view', \$server) before the Redis early-return so any
logged-in user cannot invoke confirmRestore with an arbitrary Redis server
ID without passing an authorization check. Move \$this->restoreId = \$id
to the same position so the modal's backup-jobs link always reflects the
correct server, even when the Redis instructions path is taken.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
app/Livewire/DatabaseServer/Index.php (1)
115-125:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftAlign the Redis branch with the restore policy.
Line 121 returns after only a
viewcheck, so any user who can see a Redis server can still open this restore-related flow even thoughDatabaseServerPolicy::restore()explicitly denies Redis servers. That leaves the Livewire action enforcing a weaker permission than the policy. Either gate this branch with the same restore ability or introduce a dedicated ability for viewing manual restore instructions and use that consistently in both the component and UI.As per coding guidelines, "Keep state server-side in Livewire components so the UI reflects it. Validate and authorize in actions as you would in HTTP requests."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Livewire/DatabaseServer/Index.php` around lines 115 - 125, The Redis branch currently returns after only authorizing 'view', so allow-listing differs from the policy; call $this->authorize('restore', $server) (the same ability used later) before setting $this->restoreId and $this->showRedisRestoreModal when $server->database_type === DatabaseType::REDIS, or alternatively introduce and use a new ability name (e.g. 'viewManualRestore') consistently in the Livewire component and UI; ensure you reference the DatabaseType::REDIS check, the $this->restoreId and $this->showRedisRestoreModal assignments, and the existing $this->authorize('restore', $server) usage so authorization is enforced server-side before mutating component state.
🧹 Nitpick comments (1)
app/Livewire/DatabaseServer/Index.php (1)
205-209: ⚡ Quick winAvoid calling
isScopedUser()twice per render.
render()now checksisScopedUser()before callingDatabaseServerQuery::buildFromParams(), but that query builder already repeats the same check internally. Passing$userthrough and letting the query layer decide avoids an extra grant lookup for scoped users on every Livewire refresh.♻️ Proposed simplification
$servers = DatabaseServerQuery::buildFromParams( search: $this->search, sortColumn: $this->sortBy['column'], sortDirection: $this->sortBy['direction'], - scopedUser: $user->isScopedUser() ? $user : null, + scopedUser: $user, )->paginate(10);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Livewire/DatabaseServer/Index.php` around lines 205 - 209, Avoid calling isScopedUser() here; pass the $user object to DatabaseServerQuery::buildFromParams() and let the query layer decide scope. Replace scopedUser: $user->isScopedUser() ? $user : null with scopedUser: $user (keeping the existing earlier render() check intact), so the repeated grant lookup in buildFromParams()/isScopedUser() is eliminated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@app/Livewire/DatabaseServer/Index.php`:
- Around line 115-125: The Redis branch currently returns after only authorizing
'view', so allow-listing differs from the policy; call
$this->authorize('restore', $server) (the same ability used later) before
setting $this->restoreId and $this->showRedisRestoreModal when
$server->database_type === DatabaseType::REDIS, or alternatively introduce and
use a new ability name (e.g. 'viewManualRestore') consistently in the Livewire
component and UI; ensure you reference the DatabaseType::REDIS check, the
$this->restoreId and $this->showRedisRestoreModal assignments, and the existing
$this->authorize('restore', $server) usage so authorization is enforced
server-side before mutating component state.
---
Nitpick comments:
In `@app/Livewire/DatabaseServer/Index.php`:
- Around line 205-209: Avoid calling isScopedUser() here; pass the $user object
to DatabaseServerQuery::buildFromParams() and let the query layer decide scope.
Replace scopedUser: $user->isScopedUser() ? $user : null with scopedUser: $user
(keeping the existing earlier render() check intact), so the repeated grant
lookup in buildFromParams()/isScopedUser() is eliminated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: af19d91c-52fe-499c-9472-e3ad727889ef
📒 Files selected for processing (1)
app/Livewire/DatabaseServer/Index.php
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.php: In PHP files, always use curly braces for control structures, even for single-line bodies.
Use PHP 8 constructor property promotion:public function __construct(public GitHub $github) { }. Do not leave empty zero-parameter__construct()methods unless the constructor is private.
Use explicit return type declarations and type hints for all method parameters in PHP:function isAccessible(User $user, ?string $path = null): bool
Prefer PHPDoc blocks over inline comments. Only add inline comments for exceptionally complex logic.
Use array shape type definitions in PHPDoc blocks for PHP.
Files:
app/Livewire/DatabaseServer/Index.php
app/Livewire/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
app/Livewire/**/*.php: In Livewire components, use#[Validate]attributes or Form objects for validation. Call$this->validate()before processing data.
In Livewire components, useSession::flash()for one-time messages and show them via@if (session('success'))in Blade templates.
In Livewire components, return$this->redirect()withnavigate: truefor SPA-like navigation.
In Mary UI modals, add a boolean property to the component class and usewire:modelin the Blade template.
Keep state server-side in Livewire components so the UI reflects it. Validate and authorize in actions as you would in HTTP requests.
Files:
app/Livewire/DatabaseServer/Index.php
**/*.{php,blade.php}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{php,blade.php}: Always use named routes and theroute()function when generating links to other pages.
Translations should use__('...')helper function and be stored in JSON translation files inlang/{locale}.json. Keep technical terms like 'Backup', 'Restore', 'Snapshot(s)' in English across all locales.
Files:
app/Livewire/DatabaseServer/Index.php
🧠 Learnings (2)
📚 Learning: 2026-02-18T09:45:52.485Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 116
File: app/Livewire/DatabaseServer/ConnectionStatus.php:18-18
Timestamp: 2026-02-18T09:45:52.485Z
Learning: In Livewire components, Eloquent model properties (e.g., public DatabaseServer $server) are automatically locked by the framework to prevent client-side ID tampering. The #[Locked] attribute is only needed for scalar properties (int, string, bool, etc.) that require protection from client-side mutation. Apply this guidance to all Livewire PHP components; use #[Locked] only on primitive properties that you want to shield from client manipulation, and rely on automatic locking for Eloquent model properties.
Applied to files:
app/Livewire/DatabaseServer/Index.php
📚 Learning: 2026-02-25T10:48:17.811Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 132
File: app/Console/Commands/RecoverAgentLeasesCommand.php:44-48
Timestamp: 2026-02-25T10:48:17.811Z
Learning: When reviewing PHP code, especially with foreign keys that use cascadeOnDelete and are non-nullable, assume child relations exist at runtime (the database will delete children when the parent is deleted). Do not rely on null-safe operators for these relations, as PHPStan already models them as non-null. This guideline applies broadly to PHP files that define models with foreign keys using cascade delete; verify there are no unnecessary null checks or optional chaining on such relations.
Applied to files:
app/Livewire/DatabaseServer/Index.php
2 participants
Context
This is a proof-of-concept implementation for #257.
The use case comes from hosting companies and MSPs running Databasement as a shared platform: they need to give individual clients (or teams) access to only their own database servers and databases — without promoting them to the Member role, which grants global access.
Currently the only options are:
There is no middle ground for "this user may only see Server A and only the
proddatabase within it, and may download/restore but not delete."This PR implements exactly that. It has been built and tested locally against the full test suite. It is presented as a starting point for the maintainers to validate, polish, and adapt to their standards before merging.
What this adds
user_server_accessestable (new migration)A pivot table linking users to database servers with fine-grained control:
user_iddatabase_server_idallowed_databasesnullfor all databases on the servercan_downloadtrue)can_backupfalse)can_restorefalse)Scoping behaviour
The logic follows a simple principle: Admin and Member roles are completely unaffected. Only Viewer/Demo users with at least one grant become "scoped users". A Viewer with no grants retains the existing read-only-everything behaviour.
When a user is scoped, every listing query is filtered:
allowed_databaseswhen setPer-grant action permissions
Policy checks for
backup,restore, anddownloadnow consult the grant for scoped users:Server Access UI (admin side)
A new
livewire:user.server-accesscomponent is embedded at the bottom of the user edit page. Admins can:Restore from jobs list
A restore button has been added to each completed backup job row (gated by
@can('restore', $server)). Clicking it opens the restore modal with the snapshot pre-selected, skipping the snapshot-selection step.Docker worker user fix
The queue worker was running as
root, causing backup files to be created with700 rootpermissions in/data/backups. The PHP process (running asapplication) could not read them, resulting in 404 on download. Fixed by addinguser: "1000:1000"to the worker service indocker-compose.yml.Files changed
New files:
database/migrations/2026_05_02_000001_create_user_server_accesses_table.phpdatabase/migrations/2026_05_02_000002_add_can_backup_to_user_server_accesses_table.phpapp/Models/UserServerAccess.phpdatabase/factories/UserServerAccessFactory.phpapp/Livewire/User/ServerAccess.phpresources/views/livewire/user/server-access.blade.phptests/Feature/User/ServerAccessTest.phpModified files:
app/Models/User.php—serverAccesses(),isScopedUser(),getAccessibleServerIds(),getServerAccess(),applyScopedSnapshotFilter()app/Models/DatabaseServer.php—userAccesses()relationshipapp/Policies/DatabaseServerPolicy.php—view(),backup(),restore()honour grants for scoped usersapp/Policies/SnapshotPolicy.php—view(),download()honour grants andallowed_databasesfor scoped usersapp/Queries/DatabaseServerQuery.php—scopedUserparameterapp/Queries/SnapshotQuery.php—scopedUserparameterapp/Queries/BackupJobQuery.php—scopedUserparameter with per-database filteringapp/Livewire/DatabaseServer/Index.php— passesscopedUserto queryapp/Livewire/DatabaseServer/RestoreModal.php—open-restore-from-snapshotevent listener; snapshot/server lists respect grantsapp/Livewire/BackupJob/Index.php—confirmRestoreFromJob(), passesscopedUserto queryresources/views/livewire/backup-job/index.blade.php— restore button per job row, includes restore modalresources/views/livewire/user/edit.blade.php— embedsserver-accesscomponentdocker-compose.yml— worker runs asuser: "1000:1000"Testing
All 914 existing tests pass. A dedicated test file covers:
allowed_databases)Notes for maintainers
This is a PoC and may need polishing before merge:
date('Y_m_d_His')format before merging to avoid ordering conflictsdocker-compose.ymlchange (worker user) is a local dev fix — may warrant a separate PRUserServerAccesshas no soft-deletes; revoking a grant is a hard deleteisScopedUser()method makes a DB query on every call — a cached version may be desirable for high-traffic installscan_backupshould default totrueor remainfalse(currentlyfalseto be conservative)Summary by CodeRabbit
New Features
Improvements
Tests