hook: Fix allowed errors not propagating correctly anymore

In 773668f55 (hook: Fix prereceive returning AllowedError for generic
errors, 2021-08-13), we have refactored our allowed checks to return a
generic error in case the call to `Allowed()` itself failed. The
assumption here was that because the call returns an `allowed` boolean,
it would not raise an error if the call wasn't allowed.

As it turns out, this assumption is wrong: the GitlabNetClient in
gitlab-shell will always return an error if the HTTP status code is not
between 200 and 399, and access checks do return an error code in case
access was denied. As a result, the error we returned to the Gitaly
client wouldn't have its "GitLab: " prefix anymore and thus GitLab
wouldn't recognize this error as an error that shall be returned to the
user.

Ideally, we'd properly fix this by inspecting errors returned by the
GitLabNetClient. But this isn't possible without parsing the error
messages given that the client will only return generic errors. So this
commit just reverts the state to what we had before, where we simply
treat all errors returned by the client as user-facing errors. This
logic is broken and will reveal messages to the user which aren't
intended for him in the first place, but we cannot help this for now.

Changelog: fixed

1 files changed, 8 insertions, 1 deletions

diff --git a/internal/gitaly/hook/prereceive_test.go b/internal/gitaly/hook/prereceive_test.go
index eafeb5657..caeda1659 100644
--- a/internal/gitaly/hook/prereceive_test.go
+++ b/internal/gitaly/hook/prereceive_test.go
@@ -321,7 +321,14 @@ func TestPrereceive_gitlab(t *testing.T) {
 				return false, "", errors.New("oops")
 			},
 			expectHookCall: false,
-			expectedErr:    fmt.Errorf("invoking access checks: %w", errors.New("oops")),
+			// This really is wrong, but we cannot fix it without adapting gitlab-shell
+			// to return proper errors from its GitlabNetClient.
+			expectedErr: NotAllowedError{
+				Message:  "oops",
+				Protocol: "web",
+				UserID:   "1234",
+				Changes:  []byte("changes\n"),
+			},
 		},
 		{
 			desc:    "prereceive rejects",