Backend: Remove check_resource_access from catalog_controller and add feature specs
Summary
Currently there is a check_resource_access
authorization in catalog_controller.rb
for the catalog resource details page. This method uses ::Ci::Catalog::Listing#find_resource
to check if the resource is retrievable and then returns a 404 page if not.
The resolver for ciCatalogResource
will be updated (in #432043 (closed)) to use the same find_resource
method, which means it will be functionally equivalent to check_resource_access
. Since the FE already makes a call to the GQL endpoint ciCatalogResource
and returns a "not found" page if the resource cannot be retrieved, we can remove the redundant access check from catalog_controller.rb
.
For further context (from Slack thread):
We already have a mechanism on the FE to show a local “not found” page instead of the browser default 404 when the graphql query fails to find the resource, so on our end, not having the check in the controller would be fine!
Proposal
Remove check_resource_access
from catalog_controller.rb
after #432043 (closed) is complete. I believe catalog_resource
could also be removed from the controller as it doesn't appear to be used outside of the access check (please double check).
Also add feature integration specs (if not already present) to ensure that the catalog resource details page renders correctly and returns a "not found" page when applicable.
Update [2024-02-16]
There was a miscommunication with FE team. The FE actually doesn't redirect to 404 if the resource cannot be fetched. It shows an error message instead (see below). So we cannot remove this access check from the controller as it would change the current behaviour. Setting issue to VerifyCancel.