Implement proxy uri, url config validation, redis minors (#14)

internal/authz/oidc.go

@@ -67,8 +67,14 @@ func NewOIDCHandler(cfg *oidcv1.OIDCConfig, jwks oidc.JWKSProvider,
 
 	transport := http.DefaultTransport.(*http.Transport).Clone()
 	transport.TLSClientConfig = &tls.Config{InsecureSkipVerify: cfg.SkipVerifyPeerCert}
+
+	if cfg.ProxyUri != "" {
+		// config validation ensures that the proxy uri is valid
+		proxyURL, _ := url.Parse(cfg.ProxyUri)
+		transport.Proxy = http.ProxyURL(proxyURL)
+	}
+
 	client := &http.Client{Transport: transport}
-	// TODO (sergicastro) use proxy uri
 
 	return &oidcHandler{
 		log:        internal.Logger(internal.Authz).With("type", "oidc"),

internal/config.go

@@ -17,6 +17,7 @@ package internal
 import (
 	"errors"
 	"fmt"
+	"net/url"
 	"os"
 
 	"github.com/redis/go-redis/v9"
@@ -35,7 +36,7 @@ var (
 	ErrInvalidOIDCOverride = errors.New("invalid OIDC override")
 	ErrDuplicateOIDCConfig = errors.New("duplicate OIDC configuration")
 	ErrMultipleOIDCConfig  = errors.New("multiple OIDC configurations")
-	ErrInvalidRedisURL     = errors.New("invalid Redis URL")
+	ErrInvalidURL          = errors.New("invalid URL")
 )
 
 // LocalConfigFile is a run.Config that loads the configuration file.
@@ -70,6 +71,11 @@ func (l *LocalConfigFile) Validate() error {
 		return err
 	}
 
+	// Validate the URLs before merging the OIDC configurations
+	if err = validateURLs(&l.Config); err != nil {
+		return err
+	}
+
 	// Validate OIDC configuration overrides
 	for _, fc := range l.Config.Chains {
 		hasOidc := false
@@ -117,12 +123,6 @@ func mergeAndValidateOIDCConfigs(cfg *configv1.Config) error {
 				proto.Merge(oidc, f.GetOidcOverride())
 				f.Type = &configv1.Filter_Oidc{Oidc: oidc}
 			}
-
-			if redisURL := f.GetOidc().GetRedisSessionStoreConfig().GetServerUri(); redisURL != "" {
-				if _, err := redis.ParseURL(redisURL); err != nil {
-					return fmt.Errorf("%w: invalid redis URL in chain %q", ErrInvalidRedisURL, fc.Name)
-				}
-			}
 		}
 	}
 	// Clear the default config as it has already been merged. This way there is only one
@@ -136,3 +136,62 @@ func ConfigToJSONString(c *configv1.Config) string {
 	b, _ := protojson.Marshal(c)
 	return string(b)
 }
+
+func validateURLs(config *configv1.Config) error {
+	if err := validateOIDCConfigURLs(config.DefaultOidcConfig); err != nil {
+		return fmt.Errorf("invalid default OIDC config: %w", err)
+	}
+
+	for _, fc := range config.Chains {
+		for fi, f := range fc.Filters {
+			if f.GetOidc() != nil {
+				err := validateOIDCConfigURLs(f.GetOidc())
+				if err != nil {
+					return fmt.Errorf("invalid OIDC config from chain[%s].filter[%d]: %w", fc.GetName(), fi, err)
+				}
+			}
+			if f.GetOidcOverride() != nil {
+				err := validateOIDCConfigURLs(f.GetOidcOverride())
+				if err != nil {
+					return fmt.Errorf("invalid OIDC override from chain[%s].filter[%d]: %w", fc.GetName(), fi, err)
+				}
+			}
+		}
+	}
+
+	return nil
+}
+
+func validateOIDCConfigURLs(c *oidcv1.OIDCConfig) error {
+	if err := validateURL(c.GetProxyUri()); err != nil {
+		return fmt.Errorf("%w: invalid proxy URL: %w", ErrInvalidURL, err)
+	}
+	if err := validateURL(c.GetTokenUri()); err != nil {
+		return fmt.Errorf("%w: invalid token URL: %w", ErrInvalidURL, err)
+	}
+	if err := validateURL(c.GetAuthorizationUri()); err != nil {
+		return fmt.Errorf("%w: invalid authorization URL: %w", ErrInvalidURL, err)
+	}
+	if err := validateURL(c.GetCallbackUri()); err != nil {
+		return fmt.Errorf("%w: invalid callback URL: %w", ErrInvalidURL, err)
+	}
+	if err := validateURL(c.GetJwksFetcher().GetJwksUri()); err != nil {
+		return fmt.Errorf("%w: invalid JWKS Fetcher URL: %w", ErrInvalidURL, err)
+	}
+	if redisURL := c.GetRedisSessionStoreConfig().GetServerUri(); redisURL != "" {
+		if _, err := redis.ParseURL(redisURL); err != nil {
+			return fmt.Errorf("%w: invalid Redis session store URL: %w", ErrInvalidURL, err)
+		}
+	}
+	return nil
+}
+
+func validateURL(u string) error {
+	if u == "" {
+		return nil
+	}
+	fmt.Println("u: ", u)
+	_, err := url.Parse(u)
+	fmt.Println("err: ", err)
+	return err
+}

internal/config_test.go

@@ -59,7 +59,7 @@ func TestValidateConfig(t *testing.T) {
 		{"duplicate-oidc", "testdata/duplicate-oidc.json", errCheck{is: ErrDuplicateOIDCConfig}},
 		{"invalid-oidc-override", "testdata/invalid-oidc-override.json", errCheck{is: ErrInvalidOIDCOverride}},
 		{"multiple-oidc", "testdata/multiple-oidc.json", errCheck{is: ErrMultipleOIDCConfig}},
-		{"invalid-redis", "testdata/invalid-redis.json", errCheck{is: ErrInvalidRedisURL}},
+		{"invalid-redis", "testdata/invalid-redis.json", errCheck{is: ErrInvalidURL}},
 		{"valid", "testdata/mock.json", errCheck{is: nil}},
 	}
 
@@ -71,6 +71,88 @@ func TestValidateConfig(t *testing.T) {
 	}
 }
 
+func TestValidateURLs(t *testing.T) {
+	const (
+		validURL      = "http://fake"
+		invalidURL    = "ht tp://invalid"
+		validRedisURL = "redis://localhost:6379/0"
+	)
+
+	urlTests := []struct {
+		name    string
+		oidCCfg *oidcv1.OIDCConfig
+		check   errCheck
+	}{
+		{"empty", &oidcv1.OIDCConfig{}, errCheck{is: nil}},
+		{
+			"invalid-redis",
+			&oidcv1.OIDCConfig{RedisSessionStoreConfig: &oidcv1.RedisConfig{ServerUri: invalidURL}},
+			errCheck{is: ErrInvalidURL},
+		},
+		{
+			"invalid-jwks-fetcher",
+			&oidcv1.OIDCConfig{
+				JwksConfig: &oidcv1.OIDCConfig_JwksFetcher{
+					JwksFetcher: &oidcv1.OIDCConfig_JwksFetcherConfig{JwksUri: invalidURL},
+				},
+			},
+			errCheck{is: ErrInvalidURL},
+		},
+		{"invalid-proxy-uri", &oidcv1.OIDCConfig{ProxyUri: invalidURL}, errCheck{is: ErrInvalidURL}},
+		{"invalid-token-uri", &oidcv1.OIDCConfig{TokenUri: invalidURL}, errCheck{is: ErrInvalidURL}},
+		{"invalid-authorization-uri", &oidcv1.OIDCConfig{AuthorizationUri: invalidURL}, errCheck{is: ErrInvalidURL}},
+		{"invalid-callback-uri", &oidcv1.OIDCConfig{CallbackUri: invalidURL}, errCheck{is: ErrInvalidURL}},
+		{
+			"valid",
+			&oidcv1.OIDCConfig{
+				ProxyUri: validURL, AuthorizationUri: validURL, TokenUri: validURL, CallbackUri: validURL,
+				JwksConfig:              &oidcv1.OIDCConfig_JwksFetcher{JwksFetcher: &oidcv1.OIDCConfig_JwksFetcherConfig{JwksUri: validURL}},
+				RedisSessionStoreConfig: &oidcv1.RedisConfig{ServerUri: validRedisURL},
+			},
+			errCheck{is: nil},
+		},
+	}
+
+	configTests := []struct {
+		name string
+		cfg  func(*oidcv1.OIDCConfig) *configv1.Config
+	}{
+		{
+			"default",
+			func(oidcCfg *oidcv1.OIDCConfig) *configv1.Config {
+				return &configv1.Config{DefaultOidcConfig: oidcCfg}
+			},
+		},
+		{
+			"chain-oidc",
+			func(oidcCfg *oidcv1.OIDCConfig) *configv1.Config {
+				return &configv1.Config{
+					Chains: []*configv1.FilterChain{{Filters: []*configv1.Filter{{Type: &configv1.Filter_Oidc{Oidc: oidcCfg}}}}},
+				}
+			},
+		},
+		{
+			"chain-oidc-override",
+			func(oidcCfg *oidcv1.OIDCConfig) *configv1.Config {
+				return &configv1.Config{
+					Chains: []*configv1.FilterChain{{Filters: []*configv1.Filter{{Type: &configv1.Filter_OidcOverride{OidcOverride: oidcCfg}}}}},
+				}
+			},
+		},
+	}
+
+	for _, ct := range configTests {
+		t.Run(ct.name, func(t *testing.T) {
+			for _, tt := range urlTests {
+				t.Run(tt.name, func(t *testing.T) {
+					cfg := ct.cfg(tt.oidCCfg)
+					tt.check.Check(t, validateURLs(cfg))
+				})
+			}
+		})
+	}
+}
+
 func TestLoadMock(t *testing.T) {
 	want := &configv1.Config{
 		ListenAddress: "0.0.0.0",

internal/oidc/redis.go

@@ -259,7 +259,12 @@ func (r *redisStore) refreshExpiration(ctx context.Context, sessionID string, ti
 		}
 	}
 
-	log.Debug("updating session expiration", "expire_at", expireAt)
+	log.Debug("updating session expiration",
+		"expire_at", expireAt.Format(time.DateTime),
+		"time_added", timeAdded.Format(time.DateTime),
+		"absolute_session_timeout", r.absoluteSessionTimeout,
+		"idle_session_timeout", r.idleSessionTimeout,
+	)
 
 	return r.client.ExpireAt(ctx, sessionID, expireAt).Err()
 }

internal/oidc/session.go

@@ -99,8 +99,8 @@ func (s *sessionStoreFactory) PreRun() error {
 				opts, _ := redis.ParseURL(redisServer)
 				client := redis.NewClient(opts)
 				r, err := NewRedisStore(clock, client,
-					time.Duration(f.GetOidc().GetAbsoluteSessionTimeout()),
-					time.Duration(f.GetOidc().GetIdleSessionTimeout()),
+					time.Duration(f.GetOidc().GetAbsoluteSessionTimeout())*time.Second,
+					time.Duration(f.GetOidc().GetIdleSessionTimeout())*time.Second,
 				)
 				if err != nil {
 					return err
@@ -109,8 +109,8 @@ func (s *sessionStoreFactory) PreRun() error {
 			} else if s.memory == nil { // Use a shared in-memory store for all OIDC configurations
 				log.Info("initializing in-memory session store")
 				s.memory = NewMemoryStore(clock,
-					time.Duration(f.GetOidc().GetAbsoluteSessionTimeout()),
-					time.Duration(f.GetOidc().GetIdleSessionTimeout()),
+					time.Duration(f.GetOidc().GetAbsoluteSessionTimeout())*time.Second,
+					time.Duration(f.GetOidc().GetIdleSessionTimeout())*time.Second,
 				)
 			}
 		}