From b41995a13969b2926ad265bcc769f473e48166cb Mon Sep 17 00:00:00 2001 From: Ercan Ucan Date: Mon, 15 Feb 2021 00:28:29 +0000 Subject: fix(auth): make authentication scope for Pages configurable This MR makes required authentication permission scope for Pages configurable. By default, Pages will use `api` scope to authenticate with Pages Application registered on GitLab. With this MR, the scope is configurable and can be set to `read_api` by providing the `auth-scope` variable in the arguments or in the `gitlab-pages.conf` /label ~security Changelog: added --- app.go | 2 +- app_config.go | 1 + internal/auth/auth.go | 9 +++++---- internal/auth/auth_test.go | 3 ++- internal/validateargs/validateargs.go | 2 +- main.go | 6 ++++++ test/acceptance/auth_test.go | 3 ++- test/acceptance/helpers_test.go | 4 +++- 8 files changed, 21 insertions(+), 9 deletions(-) diff --git a/app.go b/app.go index 096a1c28..0a6f0cd4 100644 --- a/app.go +++ b/app.go @@ -537,7 +537,7 @@ func (a *theApp) setAuth(config appConfig) { var err error a.Auth, err = auth.New(config.Domain, config.StoreSecret, config.ClientID, config.ClientSecret, - config.RedirectURI, config.GitLabServer) + config.RedirectURI, config.GitLabServer, config.AuthScope) if err != nil { log.WithError(err).Fatal("could not initialize auth package") } diff --git a/app_config.go b/app_config.go index dbd349ad..5b6c547a 100644 --- a/app_config.go +++ b/app_config.go @@ -39,6 +39,7 @@ type appConfig struct { ClientID string ClientSecret string RedirectURI string + AuthScope string SentryDSN string SentryEnvironment string CustomHeaders []string diff --git a/internal/auth/auth.go b/internal/auth/auth.go index cbbc720e..2fdcbeb3 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -32,7 +32,7 @@ import ( const ( apiURLUserTemplate = "%s/api/v4/user" apiURLProjectTemplate = "%s/api/v4/projects/%d/pages_access" - authorizeURLTemplate = "%s/oauth/authorize?client_id=%s&redirect_uri=%s&response_type=code&state=%s" + authorizeURLTemplate = "%s/oauth/authorize?client_id=%s&redirect_uri=%s&response_type=code&state=%s&scope=%s" tokenURLTemplate = "%s/oauth/token" tokenContentTemplate = "client_id=%s&client_secret=%s&code=%s&grant_type=authorization_code&redirect_uri=%s" callbackPath = "/auth" @@ -59,6 +59,7 @@ type Auth struct { redirectURI string gitLabServer string authSecret string + authScope string jwtSigningKey []byte jwtExpiry time.Duration apiClient *http.Client @@ -266,7 +267,7 @@ func (a *Auth) handleProxyingAuth(session *sessions.Session, w http.ResponseWrit return true } - url := fmt.Sprintf(authorizeURLTemplate, a.gitLabServer, a.clientID, a.redirectURI, state) + url := fmt.Sprintf(authorizeURLTemplate, a.gitLabServer, a.clientID, a.redirectURI, state, a.authScope) logRequest(r).WithFields(log.Fields{ "gitlab_server": a.gitLabServer, @@ -645,8 +646,7 @@ func generateKeys(secret string, count int) ([][]byte, error) { } // New when authentication supported this will be used to create authentication handler -func New(pagesDomain string, storeSecret string, clientID string, clientSecret string, - redirectURI string, gitLabServer string) (*Auth, error) { +func New(pagesDomain, storeSecret, clientID, clientSecret, redirectURI, gitLabServer, authScope string) (*Auth, error) { // generate 3 keys, 2 for the cookie store and 1 for JWT signing keys, err := generateKeys(storeSecret, 3) if err != nil { @@ -665,6 +665,7 @@ func New(pagesDomain string, storeSecret string, clientID string, clientSecret s }, store: sessions.NewCookieStore(keys[0], keys[1]), authSecret: storeSecret, + authScope: authScope, jwtSigningKey: keys[2], jwtExpiry: time.Minute, now: time.Now, diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index ce7d8320..1bd52d09 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -25,7 +25,8 @@ func createTestAuth(t *testing.T, url string) *Auth { "id", "secret", "http://pages.gitlab-example.com/auth", - url) + url, + "scope") require.NoError(t, err) diff --git a/internal/validateargs/validateargs.go b/internal/validateargs/validateargs.go index 3b75b69b..ff7484c8 100644 --- a/internal/validateargs/validateargs.go +++ b/internal/validateargs/validateargs.go @@ -11,7 +11,7 @@ const ( ) var deprecatedArgs = []string{"-sentry-dsn"} -var notAllowedArgs = []string{"-auth-client-id", "-auth-client-secret", "-auth-secret"} +var notAllowedArgs = []string{"-auth-client-id", "-auth-client-secret", "-auth-secret", "-auth-scope"} // Deprecated checks if deprecated params have been used func Deprecated(args []string) error { diff --git a/main.go b/main.go index 9921bc96..582d963e 100644 --- a/main.go +++ b/main.go @@ -73,6 +73,7 @@ var ( clientID = flag.String("auth-client-id", "", "GitLab application Client ID") clientSecret = flag.String("auth-client-secret", "", "GitLab application Client Secret") redirectURI = flag.String("auth-redirect-uri", "", "GitLab application redirect URI") + authScope = flag.String("auth-scope", "api", "Scope to be used for authentication (must match GitLab Pages OAuth application settings)") maxConns = flag.Uint("max-conns", 5000, "Limit on the number of concurrent connections to the HTTP, HTTPS or proxy listeners") insecureCiphers = flag.Bool("insecure-ciphers", false, "Use default list of cipher suites, may contain insecure ones like 3DES and RC4") tlsMinVersion = flag.String("tls-min-version", "tls1.2", tlsconfig.FlagUsage("min")) @@ -205,6 +206,7 @@ func configFromFlags() appConfig { config.ClientID = *clientID config.ClientSecret = *clientSecret config.RedirectURI = *redirectURI + config.AuthScope = *authScope config.SentryDSN = *sentryDSN config.SentryEnvironment = *sentryEnvironment @@ -242,6 +244,9 @@ func assertAuthConfig(config appConfig) { if config.RedirectURI == "" { log.Fatal("auth-redirect-uri must be defined if authentication is supported") } + if config.AuthScope == "" { + log.Fatal("auth-scope must be defined if authentication is supported") + } } func initErrorReporting(sentryDSN, sentryEnvironment string) { @@ -297,6 +302,7 @@ func loadConfig() appConfig { "api-secret-key": *gitLabAPISecretKey, "domain-config-source": config.DomainConfigurationSource, "auth-redirect-uri": config.RedirectURI, + "auth-scope": config.AuthScope, "zip-cache-expiration": config.ZipCacheExpiry, "zip-cache-cleanup": config.ZipCacheCleanup, "zip-cache-refresh": config.ZipCacheRefresh, diff --git a/test/acceptance/auth_test.go b/test/acceptance/auth_test.go index fa2d768d..1e2ec481 100644 --- a/test/acceptance/auth_test.go +++ b/test/acceptance/auth_test.go @@ -54,7 +54,8 @@ func TestWhenAuthIsEnabledPrivateWillRedirectToAuthorize(t *testing.T) { require.Equal(t, "/oauth/authorize", url.Path) require.Equal(t, "clientID", url.Query().Get("client_id")) require.Equal(t, "https://projects.gitlab-example.com/auth", url.Query().Get("redirect_uri")) - require.NotEqual(t, "", url.Query().Get("state")) + require.NotEmpty(t, url.Query().Get("scope")) + require.NotEmpty(t, url.Query().Get("state")) } func TestWhenAuthDeniedWillCauseUnauthorized(t *testing.T) { diff --git a/test/acceptance/helpers_test.go b/test/acceptance/helpers_test.go index 5c380938..d3b4f7b9 100644 --- a/test/acceptance/helpers_test.go +++ b/test/acceptance/helpers_test.go @@ -628,7 +628,9 @@ func defaultConfigFileWith(t *testing.T, configs ...string) (string, func()) { configs = append(configs, "auth-client-id=clientID", "auth-client-secret=clientSecret", - "auth-secret=authSecret") + "auth-secret=authSecret", + "auth-scope=authScope", + ) name := newConfigFile(t, configs...) -- cgit v1.2.3