Remove unnecessary checking in git controller
What does this MR do?
In b0bf9214 we introduce the checking Guest.can?(:download_code, project) to ensure that users weren't able to clone disabled public repositories (gitlab-foss#23788 (closed)).
Nevertheless, that line generates some unintentional bugs and also seems not to be necessary. At first, when we implemented this, we needed to implement something else for the SSH workflow and that would go inside the GitAccess class, because the SSH workflow doesn't go through the git controllers. So why don't we remove the checking and let GitAccess handle both workflows?.
Regarding unintentional bugs, it happens to occur one when the repository is disabled and a guest user wants to clone a public wiki. If the repository is disabled the download_code ability is prevented, but the download_wiki_code might be enabled. Nevertheless, when we get to https://gitlab.com/gitlab-org/gitlab/blob/master/app/controllers/repositories/git_http_client_controller.rb#L120 the user is denied to the access even though they have access to the wiki.
That statement also makes more difficult to reuse the code, because download_code is an ability tight to projects, while the repository might not belong to a project but a snippet or a group.
Another issue is a small leak of information. When the user is a guest, and the project is private, for example, we end up calling Guest.can?(:download_code, project) and then failing with a 401. This basically says that the project exists.
Nevertheless, if we let the request be handled by GitAccess, it will check if Guest.can?(:read_project, project) and returning a 404 instead.