feat: allow admins to access suspended servers in client API - #2507
feat: allow admins to access suspended servers in client API#2507lotus64yt wants to merge 2 commits into
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe server access middleware now recognizes administrator file routes as exceptions for suspended or maintenance-affected servers. The resources-route exemption and existing update-permission checks remain enforced. ChangesServer access authorization
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/Http/Middleware/Api/Client/Server/AuthenticateServerAccess.php`:
- Around line 53-54: Update the authorization flow around the two `throw_if`
calls in `AuthenticateServerAccess` so administrators are also bypassed for file
endpoints. Ensure unnamed `/files/*` routes are recognized as allowed alongside
the existing `api:client:server.ws` exception, while preserving the current
route restrictions for non-administrators.
🪄 Autofix
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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: dc40bb30-5777-408b-b9db-ee5c251af9db
📒 Files selected for processing (1)
app/Http/Middleware/Api/Client/Server/AuthenticateServerAccess.php
|
I have read the CLA Document and I hereby sign the CLA |
|
What is the use case behind this? |
|
I have a host powered by Pelican, and we have illegal servers. We suspend them, but we want to investigate to prevent other servers like this by finding repetitive code blocks without downloading all the files. |
| if (!$request->routeIs('api:client:server.view')) { | ||
| throw_if(($server->isSuspended() || $server->node->isUnderMaintenance()) && !$request->routeIs('api:client:server.resources'), $exception); | ||
| throw_if($user->cannot('update server', $server) || !$request->routeIs($this->except), $exception); | ||
| $isExcepted = $request->routeIs($this->except) || ($user->isAdmin() && $request->is('api/client/servers/*/files*')); |
This allows an admin user to bypass the suspended and maintenance checks in the client API, giving them access to the server's files even when the server is suspended.