Merge branch 'naman/auth-constructor-options-pattern' into 'master'

Refactor auth constructor to use options struct

See merge request https://gitlab.com/gitlab-org/gitlab-pages/-/merge_requests/846

Merged-by: Jaime Martinez <jmartinez@gitlab.com>
Approved-by: James Fargher <proglottis@gmail.com>
Approved-by: Jaime Martinez <jmartinez@gitlab.com>
Reviewed-by: James Fargher <proglottis@gmail.com>
Co-authored-by: ngala <ngala@gitlab.com>

3 files changed, 50 insertions, 25 deletions

diff --git a/app.go b/app.go
index 911df905..f35df26c 100644
--- a/app.go
+++ b/app.go
@@ -390,9 +390,18 @@ func (a *theApp) setAuth(config *cfg.Config) error {
 	}
 
 	var err error
-	a.Auth, err = auth.New(config.General.Domain, config.Authentication.Secret, config.Authentication.ClientID, config.Authentication.ClientSecret,
-		config.Authentication.RedirectURI, config.GitLab.InternalServer, config.GitLab.PublicServer, config.Authentication.Scope,
-		config.Authentication.Timeout, config.Authentication.CookieSessionTimeout)
+	a.Auth, err = auth.New(&auth.Options{
+		PagesDomain:          config.General.Domain,
+		StoreSecret:          config.Authentication.Secret,
+		ClientID:             config.Authentication.ClientID,
+		ClientSecret:         config.Authentication.ClientSecret,
+		RedirectURI:          config.Authentication.RedirectURI,
+		InternalGitlabServer: config.GitLab.InternalServer,
+		PublicGitlabServer:   config.GitLab.PublicServer,
+		AuthScope:            config.Authentication.Scope,
+		AuthTimeout:          config.Authentication.Timeout,
+		CookieSessionTimeout: config.Authentication.CookieSessionTimeout,
+	})
 	if err != nil {
 		return fmt.Errorf("could not initialize auth package: %w", err)
 	}
diff --git a/internal/auth/auth.go b/internal/auth/auth.go
index 8df98178..dcc81eee 100644
--- a/internal/auth/auth.go
+++ b/internal/auth/auth.go
@@ -652,31 +652,45 @@ func generateKeys(secret string, count int) ([][]byte, error) {
 	return keys, nil
 }
 
+// Options carry required auth parameters used to populate Auth struct
+type Options struct {
+	PagesDomain          string
+	StoreSecret          string
+	ClientID             string
+	ClientSecret         string
+	RedirectURI          string
+	InternalGitlabServer string
+	PublicGitlabServer   string
+	AuthScope            string
+	AuthTimeout          time.Duration
+	CookieSessionTimeout time.Duration
+}
+
 // New when authentication supported this will be used to create authentication handler
-func New(pagesDomain, storeSecret, clientID, clientSecret, redirectURI, internalGitlabServer, publicGitlabServer, authScope string, authTimeout, cookieSessionTimeout time.Duration) (*Auth, error) {
+func New(options *Options) (*Auth, error) {
 	// generate 3 keys, 2 for the cookie store and 1 for JWT signing
-	keys, err := generateKeys(storeSecret, 3)
+	keys, err := generateKeys(options.StoreSecret, 3)
 	if err != nil {
 		return nil, err
 	}
 
 	return &Auth{
-		pagesDomain:          pagesDomain,
-		clientID:             clientID,
-		clientSecret:         clientSecret,
-		redirectURI:          redirectURI,
-		internalGitlabServer: strings.TrimRight(internalGitlabServer, "/"),
-		publicGitlabServer:   strings.TrimRight(publicGitlabServer, "/"),
+		pagesDomain:          options.PagesDomain,
+		clientID:             options.ClientID,
+		clientSecret:         options.ClientSecret,
+		redirectURI:          options.RedirectURI,
+		internalGitlabServer: strings.TrimRight(options.InternalGitlabServer, "/"),
+		publicGitlabServer:   strings.TrimRight(options.PublicGitlabServer, "/"),
 		apiClient: &http.Client{
-			Timeout:   authTimeout,
+			Timeout:   options.AuthTimeout,
 			Transport: httptransport.DefaultTransport,
 		},
 		store:                sessions.NewCookieStore(keys[0], keys[1]),
-		authSecret:           storeSecret,
-		authScope:            authScope,
+		authSecret:           options.StoreSecret,
+		authScope:            options.AuthScope,
 		jwtSigningKey:        keys[2],
 		jwtExpiry:            time.Minute,
 		now:                  time.Now,
-		cookieSessionTimeout: cookieSessionTimeout,
+		cookieSessionTimeout: options.CookieSessionTimeout,
 	}, nil
 }
diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go
index 9226f847..40c6db15 100644
--- a/internal/auth/auth_test.go
+++ b/internal/auth/auth_test.go
@@ -22,16 +22,18 @@ import (
 func createTestAuth(t *testing.T, internalServer string, publicServer string) *Auth {
 	t.Helper()
 
-	a, err := New("pages.gitlab-example.com",
-		"something-very-secret",
-		"id",
-		"secret",
-		"http://pages.gitlab-example.com/auth",
-		internalServer,
-		publicServer,
-		"scope",
-		5*time.Second,
-		10*time.Minute)
+	a, err := New(&Options{
+		PagesDomain:          "pages.gitlab-example.com",
+		StoreSecret:          "something-very-secret",
+		ClientID:             "id",
+		ClientSecret:         "secret",
+		RedirectURI:          "http://pages.gitlab-example.com/auth",
+		InternalGitlabServer: internalServer,
+		PublicGitlabServer:   publicServer,
+		AuthScope:            "scope",
+		AuthTimeout:          5 * time.Second,
+		CookieSessionTimeout: 10 * time.Minute,
+	})
 
 	require.NoError(t, err)