Move auth functionality into separate package

Signed-off-by: Arunprasad Rajkumar <[email protected]>

Author: arajkumar

CHANGELOG.md

@@ -18,7 +18,7 @@ We use the following categories for changes:
   reader and writer [#1020].
 - Run timescaledb-tune with the promscale profile [#1615]
 - Propagate the context from received HTTP read requests downstream to database
-  requests [#1205].
+  requests [#1205]
 - Add cmd flag `web.auth.ignore-path` to skip http paths from authentication [#1637]
 
 ### Changed

docs/configuration.md

@@ -154,7 +154,7 @@ The following subsections cover all CLI flags which promscale supports. You can
 | web.auth.password          | string  |      ""       | Authentication password used for web endpoint authentication. This flag should be set together with auth-username. It is mutually exclusive with auth-password-file and bearer-token methods.                               |
 | web.auth.password-file     | string  |      ""       | Path for auth password file containing the actual password used for web endpoint authentication. This flag should be set together with auth-username. It is mutually exclusive with auth-password and bearer-token methods. |
 | web.auth.username          | string  |      ""       | Authentication username used for web endpoint authentication. Disabled by default.                                                                                                                                          |
-| web.auth.ignore-path       | string  |      ""       | Http paths which has to be skipped from authentication. This flag shall be repeated and each one would be appended to the ignore list.                                                                                      |
+| web.auth.ignore-path       | string  |      ""       | HTTP paths which has to be skipped from authentication. This flag shall be repeated and each one would be appended to the ignore list.                                                                                      |
 | web.cors-origin            | string  |     `.*`      | Regex for CORS origin. It is fully anchored. Example: 'https?://(domain1                                                                                                                                                    |
 | web.enable-admin-api       | boolean |     false     | Allow operations via API that are for advanced users. Currently, these operations are limited to deletion of series.                                                                                                        |
 | web.listen-address         | string  |    `:9201`    | Address to listen on for web endpoints.                                                                                                                                                                                     |

pkg/api/common.go

@@ -11,10 +11,7 @@ import (
 	"io"
 	"math"
 	"net/http"
-	"os"
-	"path"
 	"strconv"
-	"strings"
 	"time"
 
 	"github.com/grafana/regexp"
@@ -33,121 +30,30 @@ import (
 var (
 	minTimeFormatted = pgmodel.MinTime.Format(time.RFC3339Nano)
 	maxTimeFormatted = pgmodel.MaxTime.Format(time.RFC3339Nano)
-
-	usernameAndTokenFlagsSetError = fmt.Errorf("at most one of basic-auth-username, bearer-token & bearer-token-file must be set")
-	noUsernameFlagSetError        = fmt.Errorf("invalid auth setup, cannot enable authorization with password only (username required)")
-	noPasswordFlagsSetError       = fmt.Errorf("one of basic-auth-password & basic-auth-password-file must be configured")
-	multiplePasswordFlagsSetError = fmt.Errorf("at most one of basic-auth-password & basic-auth-password-file must be configured")
-	multipleTokenFlagsSetError    = fmt.Errorf("at most one of bearer-token & bearer-token-file must be set")
 )
 
-type arrayOfIgnorePaths []string
-
-type Auth struct {
-	BasicAuthUsername     string
-	BasicAuthPassword     string
-	BasicAuthPasswordFile string
-
-	BearerToken     string
-	BearerTokenFile string
-
-	IgnorePaths arrayOfIgnorePaths
-}
-
-func (p *arrayOfIgnorePaths) String() string {
-	return fmt.Sprintf("auth ignored paths: %#v", p)
-}
-
-func (p *arrayOfIgnorePaths) Set(path string) error {
-	*p = append(*p, path)
-	return nil
-}
-
-func (a *Auth) Validate() error {
-	switch {
-	case a.BasicAuthUsername != "":
-		if a.BearerToken != "" || a.BearerTokenFile != "" {
-			return usernameAndTokenFlagsSetError
-		}
-		if a.BasicAuthPassword == "" && a.BasicAuthPasswordFile == "" {
-			return noPasswordFlagsSetError
-		}
-		if a.BasicAuthPassword != "" && a.BasicAuthPasswordFile != "" {
-			return multiplePasswordFlagsSetError
-		}
-		pwd, err := readFromFile(a.BasicAuthPasswordFile, a.BasicAuthPassword)
-		if err != nil {
-			return fmt.Errorf("error reading password file: %w", err)
-		}
-		a.BasicAuthPassword = pwd
-	case a.BasicAuthPassword != "" || a.BasicAuthPasswordFile != "":
-		// At this point, if we have password set with no username, throw
-		// error to warn the user this is an invalid auth setup.
-		return noUsernameFlagSetError
-	case a.BearerToken != "" || a.BearerTokenFile != "":
-		if a.BearerToken != "" && a.BearerTokenFile != "" {
-			return multipleTokenFlagsSetError
-		}
-		token, err := readFromFile(a.BearerTokenFile, a.BearerToken)
-		if err != nil {
-			return fmt.Errorf("error reading bearer token file: %w", err)
-		}
-		a.BearerToken = token
-	case a.IgnorePaths != nil:
-		for _, p := range a.IgnorePaths {
-			_, err := path.Match(p, "")
-			if err != nil {
-				return fmt.Errorf("invalid ignore path pattern: %w", err)
-			}
-		}
-	}
-
-	return nil
-}
-
 type Config struct {
 	AllowedOrigin    *regexp.Regexp
 	ReadOnly         bool
 	HighAvailability bool
 	AdminAPIEnabled  bool
 	TelemetryPath    string
 
-	Auth         *Auth
 	MultiTenancy tenancy.Authorizer
 	Rules        *rules.Manager
 }
 
 func ParseFlags(fs *flag.FlagSet, cfg *Config) *Config {
-	cfg.Auth = &Auth{}
-
 	fs.BoolVar(&cfg.ReadOnly, "db.read-only", false, "Read-only mode for the connector. Operations related to writing or updating the database are disallowed. It is used when pointing the connector to a TimescaleDB read replica.")
 	fs.BoolVar(&cfg.HighAvailability, "metrics.high-availability", false, "Enable external_labels based HA.")
 	fs.BoolVar(&cfg.AdminAPIEnabled, "web.enable-admin-api", false, "Allow operations via API that are for advanced users. Currently, these operations are limited to deletion of series.")
 	fs.StringVar(&cfg.TelemetryPath, "web.telemetry-path", "/metrics", "Web endpoint for exposing Promscale's Prometheus metrics.")
 
-	fs.StringVar(&cfg.Auth.BasicAuthUsername, "web.auth.username", "", "Authentication username used for web endpoint authentication. Disabled by default.")
-	fs.StringVar(&cfg.Auth.BasicAuthPassword, "web.auth.password", "", "Authentication password used for web endpoint authentication. This flag should be set together with auth-username. It is mutually exclusive with auth-password-file and bearer-token flags.")
-	fs.StringVar(&cfg.Auth.BasicAuthPasswordFile, "web.auth.password-file", "", "Path for auth password file containing the actual password used for web endpoint authentication. This flag should be set together with auth-username. It is mutually exclusive with auth-password and bearer-token methods.")
-	fs.StringVar(&cfg.Auth.BearerToken, "web.auth.bearer-token", "", "Bearer token (JWT) used for web endpoint authentication. Disabled by default. Mutually exclusive with bearer-token-file and basic auth methods.")
-	fs.StringVar(&cfg.Auth.BearerTokenFile, "web.auth.bearer-token-file", "", "Path of the file containing the bearer token (JWT) used for web endpoint authentication. Disabled by default. Mutually exclusive with bearer-token and basic auth methods.")
-	fs.Var(&cfg.Auth.IgnorePaths, "web.auth.ignore-path", "Http paths which has to be skipped from authentication. This flag shall be repeated and each one would be appended to the ignore list.")
 	return cfg
 }
 
 func Validate(cfg *Config) error {
-	return cfg.Auth.Validate()
-}
-
-func readFromFile(path string, defaultValue string) (string, error) {
-	if path == "" {
-		return defaultValue, nil
-	}
-	bs, err := os.ReadFile(path) // #nosec G304
-	if err != nil {
-		return "", fmt.Errorf("unable to read file %s: %w", path, err)
-	}
-
-	return strings.TrimSpace(string(bs)), nil
+	return nil
 }
 
 func corsWrapper(conf *Config, f http.HandlerFunc) http.HandlerFunc {

pkg/api/common_test.go

@@ -6,11 +6,8 @@ package api
 
 import (
 	"context"
-	"errors"
 	"net/http"
 	"net/http/httptest"
-	"os"
-	"path"
 	"reflect"
 	"strings"
 	"testing"
@@ -122,160 +119,6 @@ func doCORSWrapperRequest(t *testing.T, queryHandler http.Handler, url, origin s
 	return w
 }
 
-func TestValidateConfig(t *testing.T) {
-	fileContents, err := os.ReadFile("common_test.go")
-	if err != nil {
-		t.Fatal("error reading file contents common_test.go")
-	}
-	fileContentsString := strings.TrimSpace(string(fileContents))
-	testCases := []struct {
-		name      string
-		cfg       *Auth
-		returnErr error
-		passSet   string
-		tokenSet  string
-	}{
-		{
-			name: "empty config",
-			cfg:  &Auth{},
-		},
-		{
-			name: "basic auth and bearer token set",
-			cfg: &Auth{
-				BasicAuthUsername: "foo",
-				BearerToken:       "foo",
-			},
-			returnErr: usernameAndTokenFlagsSetError,
-		},
-		{
-			name: "basic auth missing password",
-			cfg: &Auth{
-				BasicAuthUsername: "foo",
-			},
-			returnErr: noPasswordFlagsSetError,
-		},
-		{
-			name: "basic auth password and password file set",
-			cfg: &Auth{
-				BasicAuthUsername:     "foo",
-				BasicAuthPassword:     "foo",
-				BasicAuthPasswordFile: "foo",
-			},
-			returnErr: multiplePasswordFlagsSetError,
-		},
-		{
-			name: "basic auth invalid password file",
-			cfg: &Auth{
-				BasicAuthUsername:     "foo",
-				BasicAuthPasswordFile: "invalid filename",
-			},
-			returnErr: os.ErrNotExist,
-		},
-		{
-			name: "basic auth password set",
-			cfg: &Auth{
-				BasicAuthUsername: "foo",
-				BasicAuthPassword: "pass",
-			},
-			passSet: "pass",
-		},
-		{
-			name: "basic auth no username set",
-			cfg: &Auth{
-				BasicAuthPassword: "pass",
-			},
-			returnErr: noUsernameFlagSetError,
-		},
-		{
-			name: "basic auth password file set",
-			cfg: &Auth{
-				BasicAuthUsername:     "foo",
-				BasicAuthPasswordFile: "common_test.go",
-			},
-			passSet: fileContentsString,
-		},
-		{
-			name: "bearer token and token file set",
-			cfg: &Auth{
-				BearerToken:     "foo",
-				BearerTokenFile: "foo",
-			},
-			returnErr: multipleTokenFlagsSetError,
-		},
-		{
-			name: "bearer token set",
-			cfg: &Auth{
-				BearerToken: "foo",
-			},
-			tokenSet: "foo",
-		},
-		{
-			name: "bearer token file set",
-			cfg: &Auth{
-				BearerTokenFile: "common_test.go",
-			},
-			tokenSet: fileContentsString,
-		},
-		{
-			name: "bearer token file invalid file set",
-			cfg: &Auth{
-				BearerTokenFile: "invalid file",
-			},
-			returnErr: os.ErrNotExist,
-		},
-		{
-			name: "all config options set",
-			cfg: &Auth{
-				BasicAuthUsername:     "set",
-				BasicAuthPassword:     "set",
-				BasicAuthPasswordFile: "set",
-				BearerToken:           "set",
-				BearerTokenFile:       "set",
-			},
-			returnErr: usernameAndTokenFlagsSetError,
-		},
-		{
-			name: "invalid ignore path",
-			cfg: &Auth{
-				IgnorePaths: []string{
-					"[",
-				},
-			},
-			returnErr: path.ErrBadPattern,
-		},
-		{
-			name: "valid ignore path",
-			cfg: &Auth{
-				IgnorePaths: []string{
-					"/hello",
-				},
-			},
-		},
-	}
-
-	for _, c := range testCases {
-		t.Run(c.name, func(t *testing.T) {
-			err := Validate(&Config{
-				Auth: c.cfg,
-			})
-			if c.returnErr != nil {
-				if !errors.Is(err, c.returnErr) {
-					t.Errorf("unexpected error received: %s", err)
-				}
-			} else if err != nil {
-				t.Errorf("unexpected error received: %s", err)
-			}
-
-			if c.passSet != "" && c.cfg.BasicAuthPassword != c.passSet {
-				t.Errorf("unexpected password set: got %s wanted %s", c.cfg.BasicAuthPassword, c.passSet)
-			}
-			if c.tokenSet != "" && c.cfg.BearerToken != c.tokenSet {
-				t.Errorf("unexpected bearer token set: got %s wanted %s", c.cfg.BearerToken, c.tokenSet)
-			}
-		})
-	}
-}
-
 func TestMarshalExemplar(t *testing.T) {
 	tcs := []struct {
 		name        string