From 1380a2c411e04582ace1c056bf89e73c8b29ea67 Mon Sep 17 00:00:00 2001 From: Jaime Martinez Date: Wed, 25 May 2022 15:33:18 +1000 Subject: [PATCH 1/2] Disable custom 404 redirection to auth when resource does not exist Related to https://gitlab.com/gitlab-com/gl-infra/production/-/issues/7126 Changelog: changed --- app.go | 10 +++++++--- internal/domain/domain.go | 5 ++++- test/acceptance/auth_test.go | 15 ++++++++------- 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/app.go b/app.go index 6f7537c7e..2330c83ba 100644 --- a/app.go +++ b/app.go @@ -104,9 +104,13 @@ func (a *theApp) domain(ctx context.Context, host string) (*domain.Domain, error // by behaving the same if user has no access to the project or if project simply does not exists func (a *theApp) checkAuthAndServeNotFound(domain *domain.Domain, w http.ResponseWriter, r *http.Request) { // To avoid user knowing if pages exist, we will force user to login and authorize pages - if a.Auth.CheckAuthenticationWithoutProject(w, r, domain) { - return - } + + // Temporarily disable auth redirection for domain not found, this may break custom 404 page redirection + // for private projects https://gitlab.com/gitlab-org/gitlab-pages/-/issues/183. + // TODO: refactor and handle auth properly https://gitlab.com/gitlab-org/gitlab-pages/-/issues/765 + //if a.Auth.CheckAuthenticationWithoutProject(w, r, domain) { + // return + //} // auth succeeded try to serve the correct 404 page domain.ServeNotFoundAuthFailed(w, r) diff --git a/internal/domain/domain.go b/internal/domain/domain.go index 1cfee100b..2b21499c3 100644 --- a/internal/domain/domain.go +++ b/internal/domain/domain.go @@ -194,7 +194,10 @@ func (d *Domain) ServeNamespaceNotFound(w http.ResponseWriter, r *http.Request) // 404 page is served. func (d *Domain) ServeNotFoundAuthFailed(w http.ResponseWriter, r *http.Request) { lookupPath, err := d.GetLookupPath(r) - if err != nil { + // Temporarily handle all access controlled pages as a generic 404 to avoid leaking + // project existence to everyone. This may trigger https://gitlab.com/gitlab-org/gitlab-pages/-/issues/183 again. + // TODO: https://gitlab.com/gitlab-org/gitlab-pages/-/issues/765 + if err != nil || lookupPath.HasAccessControl { httperrors.Serve404(w) return } diff --git a/test/acceptance/auth_test.go b/test/acceptance/auth_test.go index 022afcbd1..31b5cea5b 100644 --- a/test/acceptance/auth_test.go +++ b/test/acceptance/auth_test.go @@ -247,13 +247,14 @@ func TestCustomErrorPageWithAuth(t *testing.T) { path: "/private_project/unknown", expectedErrorPage: "Private custom 404 error page", }, - { - name: "public_namespace_with_private_unauthorized_project", - domain: "group.404.gitlab-example.com", - // /private_unauthorized/config.json resolves project ID to 2000 which will cause a 401 from the mock GitLab testServer - path: "/private_unauthorized/unknown", - expectedErrorPage: "Custom 404 group page", - }, + // TODO: enable test https://gitlab.com/gitlab-org/gitlab-pages/-/issues/765 + //{ + // name: "public_namespace_with_private_unauthorized_project", + // domain: "group.404.gitlab-example.com", + // // /private_unauthorized/config.json resolves project ID to 2000 which will cause a 401 from the mock GitLab testServer + // path: "/private_unauthorized/unknown", + // expectedErrorPage: "Custom 404 group page", + //}, { name: "private_namespace_authorized", domain: "group.auth.gitlab-example.com", -- GitLab From e3de9ffea99f7c46939ec22df979caaf4c5c3a7e Mon Sep 17 00:00:00 2001 From: Jaime Martinez Date: Wed, 25 May 2022 16:03:39 +1000 Subject: [PATCH 2/2] Revert check if project is private --- internal/domain/domain.go | 5 +---- test/acceptance/auth_test.go | 15 +++++++-------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/internal/domain/domain.go b/internal/domain/domain.go index 2b21499c3..1cfee100b 100644 --- a/internal/domain/domain.go +++ b/internal/domain/domain.go @@ -194,10 +194,7 @@ func (d *Domain) ServeNamespaceNotFound(w http.ResponseWriter, r *http.Request) // 404 page is served. func (d *Domain) ServeNotFoundAuthFailed(w http.ResponseWriter, r *http.Request) { lookupPath, err := d.GetLookupPath(r) - // Temporarily handle all access controlled pages as a generic 404 to avoid leaking - // project existence to everyone. This may trigger https://gitlab.com/gitlab-org/gitlab-pages/-/issues/183 again. - // TODO: https://gitlab.com/gitlab-org/gitlab-pages/-/issues/765 - if err != nil || lookupPath.HasAccessControl { + if err != nil { httperrors.Serve404(w) return } diff --git a/test/acceptance/auth_test.go b/test/acceptance/auth_test.go index 31b5cea5b..022afcbd1 100644 --- a/test/acceptance/auth_test.go +++ b/test/acceptance/auth_test.go @@ -247,14 +247,13 @@ func TestCustomErrorPageWithAuth(t *testing.T) { path: "/private_project/unknown", expectedErrorPage: "Private custom 404 error page", }, - // TODO: enable test https://gitlab.com/gitlab-org/gitlab-pages/-/issues/765 - //{ - // name: "public_namespace_with_private_unauthorized_project", - // domain: "group.404.gitlab-example.com", - // // /private_unauthorized/config.json resolves project ID to 2000 which will cause a 401 from the mock GitLab testServer - // path: "/private_unauthorized/unknown", - // expectedErrorPage: "Custom 404 group page", - //}, + { + name: "public_namespace_with_private_unauthorized_project", + domain: "group.404.gitlab-example.com", + // /private_unauthorized/config.json resolves project ID to 2000 which will cause a 401 from the mock GitLab testServer + path: "/private_unauthorized/unknown", + expectedErrorPage: "Custom 404 group page", + }, { name: "private_namespace_authorized", domain: "group.auth.gitlab-example.com", -- GitLab