Merge branch 'fix/domain-allowed' into 'master'

fix(auth): check suffix correctly in domainAllowed

See merge request gitlab-org/gitlab-pages!619

2 files changed, 51 insertions, 1 deletions

diff --git a/internal/auth/auth.go b/internal/auth/auth.go
index 23a2fc68..07af99fe 100644
--- a/internal/auth/auth.go
+++ b/internal/auth/auth.go
@@ -219,7 +219,7 @@ func (a *Auth) checkAuthenticationResponse(session *sessions.Session, w http.Res
 }
 
 func (a *Auth) domainAllowed(ctx context.Context, name string, domains source.Source) bool {
-	isConfigured := (name == a.pagesDomain) || strings.HasSuffix("."+name, a.pagesDomain)
+	isConfigured := (name == a.pagesDomain) || strings.HasSuffix(name, "."+a.pagesDomain)
 
 	if isConfigured {
 		return true
diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go
index 74c0370c..d55c5a46 100644
--- a/internal/auth/auth_test.go
+++ b/internal/auth/auth_test.go
@@ -2,6 +2,7 @@ package auth
 
 import (
 	"bytes"
+	"context"
 	"fmt"
 	"io"
 	"net/http"
@@ -513,3 +514,52 @@ func TestCheckResponseForInvalidTokenWhenNotInvalidToken(t *testing.T) {
 
 	require.False(t, auth.CheckResponseForInvalidToken(result, r, resp))
 }
+
+func TestDomainAllowed(t *testing.T) {
+	auth := createTestAuth(t, "", "")
+	mockCtrl := gomock.NewController(t)
+	mockSource := mocks.NewMockSource(mockCtrl)
+
+	testCases := []struct {
+		name     string
+		expected bool
+	}{
+		{
+			name:     "pages.unrelated-site.com",
+			expected: false,
+		},
+		{
+			name:     "prepended-pages.gitlab-example.com",
+			expected: false,
+		},
+		{
+			name:     "pages.gitlab-example.com.unrelated-site.com",
+			expected: false,
+		},
+		{
+			name:     "pages.gitlab-example.com",
+			expected: true,
+		},
+		{
+			name:     "subdomain.pages.gitlab-example.com",
+			expected: true,
+		},
+		{
+			name:     "multi.sub.domain.pages.gitlab-example.com",
+			expected: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			ctx := context.Background()
+
+			if !tc.expected {
+				mockSource.EXPECT().GetDomain(ctx, tc.name).Return(nil, nil)
+			}
+
+			actual := auth.domainAllowed(ctx, tc.name, mockSource)
+			require.Equal(t, tc.expected, actual)
+		})
+	}
+}