Handle the blacklisted IP error in the Go middleware
🌲 Context
The go middleware adds support of the go-get=1
parameter. This parameter is used when a user $ go get
, This middleware will reply the proper text response accordingly: in particular, the response will have the project path so that $ go
can clone the repository.
The problem is when authenticated users try to access projects/groups that don't exist: the middleware will reply back with simple_path
which is defined as the first two path segments. $ go get
will then build the repository url but since the project/group doesn't exist, the repository url will be wrong and $ git clone
will fail.
The above scenario is executed by scripts that run that on a loop. So basically, gitlab.com will receive n
requests with wrong paths on this middleware. This leads to the core issue of #220561 (closed): authenticated user are rate limited. These scripts will thus trigger a GitLab::Auth::IpBlacklisted
error.
Since this error is not handled by the middleware, this will end up in a 500 error response and this counts towards our production SLAs.
🔬 What does this MR do?
- Handle the
GitLab::Auth::IpBlacklisted
error in the go middleware. - The handling is similar to what is done in the application controller. A
403 Forbidden
will be replied.
🖼 Screenshots
Note that the project used in these examples doesn't exist. In other words, the project path doesn't lead anywhere.
Before this MR
$ go get -v gdk.test/group/subgroup/noproject/subpackage
package gdk.test/group/subgroup/noproject/subpackage: unrecognized import path "gdk.test/group/subgroup/noproject/subpackage": reading https://gdk.test/group/subgroup/noproject/subpackage?go-get=1: 500 Internal Server Error
server response:
Gitlab::Auth::IpBlacklisted at group/subgroup/noproject/subpackage
==================================================================
-> Notice the 500 Internal Server Error
With this MR
$ go get -v gdk.test/group/subgroup/noproject/subpackage
package gdk.test/group/subgroup/noproject/subpackage: unrecognized import path "gdk.test/group/subgroup/noproject/subpackage": reading https://gdk.test/group/subgroup/noproject/subpackage?go-get=1: 403 Forbidden
-> we have now 403 Forbidden
🔮 Other considerations
To be complete here, I don't fully understand this explanation. I also don't get why we reply a successful response when the project is not found. My guess is that unauthenticated access to private projects are replied with the simple_path
so that $ go get
can build the repository path and $ git clone
will use its own saved credentials against the private repository which works if the user has the git access to the private repository.
To me, if the project is not found, we should return bad request
and that's it. If users want to use the go get
feature on private projects, they should properly set up their credentials first.
Thoughts for a follow up MR I guess.
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry -
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers -
Informed Infrastructure department of a default or new setting change, if applicable per definition of done
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
-
Label as security and @ mention @gitlab-com/gl-security/appsec
-
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods -
Security reports checked/validated by a reviewer from the AppSec team