Fixing permission problems with applications #571
Conversation
WalkthroughWalkthroughThis update refines the permission checks within the application management system. The revision focuses on ensuring the correct abilities are verified for viewing, editing, and finalising applications. These changes affect both the backend logic and the frontend Blade templates, enhancing the accuracy and security of permission handling. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ApplicationController
participant ApplicationModel
participant BladeTemplate
User->>ApplicationController: View Applications
ApplicationController->>ApplicationModel: Check permissions for viewUnfinished
ApplicationModel->>ApplicationController: Permissions verified
ApplicationController->>BladeTemplate: Render applications view
Note right of BladeTemplate: Application.blade.php
BladeTemplate->>BladeTemplate: @can('view', $user->application)
User-->>BladeTemplate: Interacts with the view
loop Edit Application Status
User->>ApplicationController: Edit Application Status
ApplicationController->>ApplicationModel: Check permissions for editStatus
ApplicationModel->>ApplicationController: Permissions verified
ApplicationController->>BladeTemplate: Render status form
end
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
app/Http/Controllers/Auth/ApplicationController.php (1)
166-166: MissingviewUnfinishedmethod inApplicationFormPolicyThe
viewUnfinishedpermission check inApplicationControllerreferences a method that does not exist in theApplicationFormPolicyclass. Please ensure that theviewUnfinishedmethod is correctly defined and implemented in theApplicationFormPolicyclass.
- File:
app/Http/Controllers/Auth/ApplicationController.php- Line: 166
- Class:
ApplicationFormPolicy- Missing Method:
viewUnfinishedAnalysis chain
Confirm correct implementation of the updated permission check.
The updated permission check using
@can('viewUnfinished', \App\Models\ApplicationForm::class)appears to be aligned with the PR objectives. However, confirm that this permission is correctly defined and implemented in the backend.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Confirm the correct implementation of the 'viewUnfinished' permission. ast-grep --lang php --pattern $'class ApplicationFormPolicy { $$$ viewUnfinished($_, $_) { $$$ } $$$ }'Length of output: 225
Script:
#!/bin/bash # Description: Confirm the correct implementation of the 'viewUnfinished' permission. rg -A 5 "class ApplicationFormPolicy" --glob "**/*.php" | rg -A 5 "function viewUnfinished"Length of output: 93
Script:
#!/bin/bash # Description: List all methods in policy classes to confirm the existence of the 'viewUnfinished' method. rg -A 10 "class .*Policy" --glob "**/*.php"Length of output: 14046
|
(I don't know anything about PHP...) Is there a static analyzer/type checker that could catch problems like this one before broken code is deployed? @kdmnk maybe? |
|
I have this in progress: #553 |
AFAIK the problematic code was introduced in #532 which was deployed to production by #551 because #542 depended on it (although I advised against deploying it in #542 (comment)). |
Update: I found https://github.com/larastan/larastan which allegedly checks Laravel's "beautiful magic" 😄 |
The problem was that old names of policies were used instead of the new ones in ApplicationFormPolicy. Therefore, applications did not appear correctly and unfinished applications were hidden.