Add support for previewing artifacts that are not public

Remove some duplicate logic on Auth module
Separate handling artifact to own handlers package
Unit test handlers by mocking auth and artifact modules
Add generate-mock step to Makefile
Use additional handler func to simplify TryMakeRequest return type
Always try with token if exists
Do not log RequestURI, log path only
Remove not used logRequest func

2 files changed, 228 insertions, 0 deletions

diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go
new file mode 100644
index 00000000..5791e793
--- /dev/null
+++ b/internal/handlers/handlers.go
@@ -0,0 +1,67 @@
+package handlers
+
+import (
+	"net/http"
+
+	"gitlab.com/gitlab-org/gitlab-pages/internal"
+	"gitlab.com/gitlab-org/gitlab-pages/internal/logging"
+)
+
+// Handlers take care of handling specific requests
+type Handlers struct {
+	Auth     internal.Auth
+	Artifact internal.Artifact
+}
+
+// New when provided the arguments defined herein, returns a pointer to an
+// Handlers that is used to handle requests.
+func New(auth internal.Auth, artifact internal.Artifact) *Handlers {
+	return &Handlers{
+		Auth:     auth,
+		Artifact: artifact,
+	}
+}
+
+func (a *Handlers) checkIfLoginRequiredOrInvalidToken(w http.ResponseWriter, r *http.Request, token string) func(*http.Response) bool {
+	return func(resp *http.Response) bool {
+
+		if resp.StatusCode == http.StatusNotFound {
+
+			if token == "" {
+				if !a.Auth.IsAuthSupported() {
+					// Auth is not supported, probably means no access or does not exist but we cannot try with auth
+					return false
+				}
+
+				logging.LogRequest(r).Debug("Artifact API response was 404 without token, try with authentication")
+
+				// Authenticate user
+				if a.Auth.RequireAuth(w, r) {
+					return true
+				}
+			} else {
+				logging.LogRequest(r).Debug("Artifact API response was 404 with authentication")
+			}
+		}
+
+		if a.Auth.CheckResponseForInvalidToken(w, r, resp) {
+			return true
+		}
+
+		return false
+	}
+}
+
+// HandleArtifactRequest handles all artifact related requests, will return true if request was handled here
+func (a *Handlers) HandleArtifactRequest(host string, w http.ResponseWriter, r *http.Request) bool {
+	// In the event a host is prefixed with the artifact prefix an artifact
+	// value is created, and an attempt to proxy the request is made
+
+	// Always try to add token to the request if it exists
+	token, err := a.Auth.GetTokenIfExists(w, r)
+	if err != nil {
+		return true
+	}
+
+	return a.Artifact.TryMakeRequest(host, w, r, token, a.checkIfLoginRequiredOrInvalidToken(w, r, token))
+}
diff --git a/internal/handlers/handlers_test.go b/internal/handlers/handlers_test.go
new file mode 100644
index 00000000..aa664266
--- /dev/null
+++ b/internal/handlers/handlers_test.go
@@ -0,0 +1,161 @@
+package handlers
+
+import (
+	"errors"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"testing"
+
+	"gitlab.com/gitlab-org/gitlab-pages/internal/mocks"
+
+	"github.com/golang/mock/gomock"
+	"github.com/stretchr/testify/require"
+)
+
+func TestNotHandleArtifactRequestReturnsFalse(t *testing.T) {
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+
+	mockArtifact := mocks.NewMockArtifact(mockCtrl)
+	mockArtifact.EXPECT().
+		TryMakeRequest(gomock.Any(), gomock.Any(), gomock.Any(), "", gomock.Any()).
+		Return(false).
+		Times(1)
+
+	mockAuth := mocks.NewMockAuth(mockCtrl)
+	mockAuth.EXPECT().
+		GetTokenIfExists(gomock.Any(), gomock.Any()).
+		Return("", nil).
+		Times(1)
+
+	handlers := New(mockAuth, mockArtifact)
+
+	result := httptest.NewRecorder()
+	reqURL, err := url.Parse("/something")
+	require.NoError(t, err)
+	r := &http.Request{URL: reqURL}
+
+	require.Equal(t, false, handlers.HandleArtifactRequest("host", result, r))
+}
+
+func TestHandleArtifactRequestedReturnsTrue(t *testing.T) {
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+
+	mockArtifact := mocks.NewMockArtifact(mockCtrl)
+	mockArtifact.EXPECT().
+		TryMakeRequest(gomock.Any(), gomock.Any(), gomock.Any(), "", gomock.Any()).
+		Return(true).
+		Times(1)
+
+	mockAuth := mocks.NewMockAuth(mockCtrl)
+	mockAuth.EXPECT().
+		GetTokenIfExists(gomock.Any(), gomock.Any()).
+		Return("", nil).
+		Times(1)
+
+	handlers := New(mockAuth, mockArtifact)
+
+	result := httptest.NewRecorder()
+	reqURL, err := url.Parse("/something")
+	require.NoError(t, err)
+	r := &http.Request{URL: reqURL}
+
+	require.Equal(t, true, handlers.HandleArtifactRequest("host", result, r))
+}
+
+func TestNotFoundWithTokenIsNotHandled(t *testing.T) {
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+
+	mockAuth := mocks.NewMockAuth(mockCtrl)
+	mockAuth.EXPECT().CheckResponseForInvalidToken(gomock.Any(), gomock.Any(), gomock.Any()).
+		Return(false)
+
+	handlers := New(mockAuth, nil)
+
+	w := httptest.NewRecorder()
+	reqURL, _ := url.Parse("/")
+	r := &http.Request{URL: reqURL}
+	response := &http.Response{StatusCode: http.StatusNotFound}
+	handled := handlers.checkIfLoginRequiredOrInvalidToken(w, r, "token")(response)
+
+	require.False(t, handled)
+}
+
+func TestNotFoundWithoutTokenIsNotHandledWhenNotAuthSupport(t *testing.T) {
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+
+	mockAuth := mocks.NewMockAuth(mockCtrl)
+	mockAuth.EXPECT().IsAuthSupported().Return(false)
+
+	handlers := New(mockAuth, nil)
+
+	w := httptest.NewRecorder()
+	reqURL, _ := url.Parse("/")
+	r := &http.Request{URL: reqURL}
+	response := &http.Response{StatusCode: http.StatusNotFound}
+	handled := handlers.checkIfLoginRequiredOrInvalidToken(w, r, "")(response)
+
+	require.False(t, handled)
+}
+func TestNotFoundWithoutTokenIsHandled(t *testing.T) {
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+
+	mockAuth := mocks.NewMockAuth(mockCtrl)
+	mockAuth.EXPECT().IsAuthSupported().Return(true)
+	mockAuth.EXPECT().RequireAuth(gomock.Any(), gomock.Any()).Times(1).Return(true)
+
+	handlers := New(mockAuth, nil)
+
+	w := httptest.NewRecorder()
+	reqURL, _ := url.Parse("/")
+	r := &http.Request{URL: reqURL}
+	response := &http.Response{StatusCode: http.StatusNotFound}
+	handled := handlers.checkIfLoginRequiredOrInvalidToken(w, r, "")(response)
+
+	require.True(t, handled)
+}
+func TestInvalidTokenResponseIsHandled(t *testing.T) {
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+
+	mockAuth := mocks.NewMockAuth(mockCtrl)
+	mockAuth.EXPECT().CheckResponseForInvalidToken(gomock.Any(), gomock.Any(), gomock.Any()).
+		Return(true)
+
+	handlers := New(mockAuth, nil)
+
+	w := httptest.NewRecorder()
+	reqURL, _ := url.Parse("/")
+	r := &http.Request{URL: reqURL}
+	response := &http.Response{StatusCode: http.StatusUnauthorized}
+	handled := handlers.checkIfLoginRequiredOrInvalidToken(w, r, "token")(response)
+
+	require.True(t, handled)
+}
+
+func TestHandleArtifactRequestButGetTokenFails(t *testing.T) {
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+
+	mockArtifact := mocks.NewMockArtifact(mockCtrl)
+	mockArtifact.EXPECT().
+		TryMakeRequest(gomock.Any(), gomock.Any(), gomock.Any(), "", gomock.Any()).
+		Times(0)
+
+	mockAuth := mocks.NewMockAuth(mockCtrl)
+	mockAuth.EXPECT().GetTokenIfExists(gomock.Any(), gomock.Any()).Return("", errors.New("error when retrieving token"))
+
+	handlers := New(mockAuth, mockArtifact)
+
+	result := httptest.NewRecorder()
+	reqURL, err := url.Parse("/something")
+	require.NoError(t, err)
+	r := &http.Request{URL: reqURL}
+
+	require.Equal(t, true, handlers.HandleArtifactRequest("host", result, r))
+}