update before actions in scratch controller#887
Conversation
Test coverage91.85% line coverage reported by SimpleCov. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6843e7b. Configure here.
| class ProjectsController < ScratchController | ||
| include RemixSelection | ||
|
|
||
| before_action :authorize_user, except: %i[show] |
There was a problem hiding this comment.
Remix checks create not show
Medium Severity
After dropping the school-only gate, non-school users can hit scratch remix create, but authorize_resource :original_project authorizes :create on the original. Anonymous starters allow :show but not :create, so remixing a public template can return forbidden despite the new spec expecting success.
Reviewed by Cursor Bugbot for commit 6843e7b. Configure here.
There was a problem hiding this comment.
Pull request overview
This PR relaxes authentication requirements for the Scratch API so that show endpoints for Scratch projects/assets can be accessed without requiring a logged-in user, while keeping other Scratch actions authenticated/authorized.
Changes:
- Removes the “only schools can use Scratch” gate from
Api::Scratch::ScratchController. - Makes
ProjectsController#showandAssetsController#showskipauthorize_user(authentication), while retaining CanCan authorization. - Updates request specs and adds a
:scratch_projectfactory to support anonymous Scratch-project scenarios.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/features/scratch/showing_a_scratch_project_spec.rb | Adds coverage for anonymous Scratch project show, but currently has an “authenticated” test that doesn’t actually send auth headers. |
| spec/features/scratch/creating_and_showing_a_scratch_asset_spec.rb | Adds coverage for asset show when unauthenticated/anonymous; includes minor wording typos. |
| spec/features/scratch/creating_a_scratch_project_spec.rb | Adjusts expected status codes for non-school users and adds an anonymous-original remix case. |
| spec/factories/project.rb | Adds :scratch_project factory for Scratch projects with a scratch component. |
| app/controllers/api/scratch/scratch_controller.rb | Removes authorize_user and only_allow_schools_to_use_scratch from the base Scratch controller. |
| app/controllers/api/scratch/projects_controller.rb | Skips authentication for show; all other actions still require authentication. |
| app/controllers/api/scratch/assets_controller.rb | Skips authentication for show; all other actions still require authentication. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| before_action :authorize_user, except: %i[show] | ||
| before_action :load_project, except: %i[create] | ||
| authorize_resource :project, except: %i[create] |
| class ProjectsController < ScratchController | ||
| include RemixSelection | ||
|
|
||
| before_action :authorize_user, except: %i[show] |
| before_action :authorize_user, except: %i[show] | ||
| prepend_before_action :load_project_from_header, only: %i[show create] | ||
| authorize_resource :project_from_header |
| it 'returns a 200 ok if logged in for an anonymous scratch project' do | ||
| authenticated_in_hydra_as(student) | ||
| project = create(:scratch_project, locale: 'en', user_id: nil) | ||
| get "/api/scratch/projects/#{project.identifier}" | ||
|
|
||
| expect(response).to have_http_status(:ok) | ||
| end |
There was a problem hiding this comment.
If we add the headers: in here, the test fails, i.e. students are blocked.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
showroutes in assets/projects scratch controllers to run without authorising user