fix(config): Validate OIDC configuration for login and callback path

Author: oxyno-zeta

pkg/config/validation.go

@@ -4,6 +4,8 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"net/url"
+	"path"
 	"strings"
 
 	"github.com/thoas/go-funk"
@@ -83,6 +85,37 @@ func validateBusinessConfig(out *Config) error {
 		}
 	}
 
+	// Validate authentication providers
+	if out.AuthProviders != nil && out.AuthProviders.OIDC != nil {
+		for prov, authProviderCfg := range out.AuthProviders.OIDC {
+			// Build redirect url
+			u, err := url.Parse(authProviderCfg.RedirectURL)
+			// Check if error exists
+			if err != nil {
+				return err
+			}
+			// Continue to build redirect url
+			u.Path = path.Join(u.Path, authProviderCfg.CallbackPath)
+
+			// Now, full callback path is generated
+
+			// Check if new path is "/"
+			if u.Path == "/" {
+				return fmt.Errorf("provider %s can't have a callback path equal to / (to avoid redirect loop)", prov)
+			}
+
+			// Check login path
+			if authProviderCfg.LoginPath == "/" {
+				return fmt.Errorf("provider %s can't have a login path equal to / (to avoid redirect loop)", prov)
+			}
+
+			// Check that they are different
+			if authProviderCfg.LoginPath == u.Path {
+				return fmt.Errorf("provider %s can't have same login and callback path (to avoid redirect loop)", prov)
+			}
+		}
+	}
+
 	return nil
 }
 

pkg/config/validation_test.go

@@ -1,5 +1,3 @@
-// +build unit
-
 package config
 
 import (
@@ -522,6 +520,130 @@ func Test_validateBusinessConfig(t *testing.T) {
 			wantErr:     true,
 			errorString: "path 0 in list targets must ends with /",
 		},
+		{
+			name: "OIDC provider with wrong callback path",
+			args: args{
+				out: &Config{
+					AuthProviders: &AuthProviderConfig{
+						OIDC: map[string]*OIDCAuthConfig{
+							"provider1": {
+								CallbackPath: "/",
+							},
+						},
+					},
+					Targets: []*TargetConfig{
+						{
+							Name: "test1",
+							Bucket: &BucketConfig{
+								Name:   "bucket1",
+								Region: "region1",
+							},
+							Mount: &MountConfig{
+								Path: []string{"/mount1/"},
+							},
+							Resources: nil,
+							Actions: &ActionsConfig{
+								GET:    &GetActionConfig{Enabled: true},
+								PUT:    &PutActionConfig{Enabled: false},
+								DELETE: &DeleteActionConfig{Enabled: false},
+							},
+						},
+					},
+					ListTargets: &ListTargetsConfig{
+						Enabled: true,
+						Mount: &MountConfig{
+							Path: []string{"/"},
+						},
+						Resource: nil,
+					},
+				},
+			},
+			wantErr:     true,
+			errorString: "provider provider1 can't have a callback path equal to / (to avoid redirect loop)",
+		},
+		{
+			name: "OIDC provider with wrong login path",
+			args: args{
+				out: &Config{
+					AuthProviders: &AuthProviderConfig{
+						OIDC: map[string]*OIDCAuthConfig{
+							"provider1": {
+								LoginPath: "/",
+							},
+						},
+					},
+					Targets: []*TargetConfig{
+						{
+							Name: "test1",
+							Bucket: &BucketConfig{
+								Name:   "bucket1",
+								Region: "region1",
+							},
+							Mount: &MountConfig{
+								Path: []string{"/mount1/"},
+							},
+							Resources: nil,
+							Actions: &ActionsConfig{
+								GET:    &GetActionConfig{Enabled: true},
+								PUT:    &PutActionConfig{Enabled: false},
+								DELETE: &DeleteActionConfig{Enabled: false},
+							},
+						},
+					},
+					ListTargets: &ListTargetsConfig{
+						Enabled: true,
+						Mount: &MountConfig{
+							Path: []string{"/"},
+						},
+						Resource: nil,
+					},
+				},
+			},
+			wantErr:     true,
+			errorString: "provider provider1 can't have a login path equal to / (to avoid redirect loop)",
+		},
+		{
+			name: "OIDC provider with same login and callback path",
+			args: args{
+				out: &Config{
+					AuthProviders: &AuthProviderConfig{
+						OIDC: map[string]*OIDCAuthConfig{
+							"provider1": {
+								LoginPath:    "/fake",
+								CallbackPath: "/fake",
+							},
+						},
+					},
+					Targets: []*TargetConfig{
+						{
+							Name: "test1",
+							Bucket: &BucketConfig{
+								Name:   "bucket1",
+								Region: "region1",
+							},
+							Mount: &MountConfig{
+								Path: []string{"/mount1/"},
+							},
+							Resources: nil,
+							Actions: &ActionsConfig{
+								GET:    &GetActionConfig{Enabled: true},
+								PUT:    &PutActionConfig{Enabled: false},
+								DELETE: &DeleteActionConfig{Enabled: false},
+							},
+						},
+					},
+					ListTargets: &ListTargetsConfig{
+						Enabled: true,
+						Mount: &MountConfig{
+							Path: []string{"/"},
+						},
+						Resource: nil,
+					},
+				},
+			},
+			wantErr:     true,
+			errorString: "provider provider1 can't have same login and callback path (to avoid redirect loop)",
+		},
 		{
 			name: "Configuration with list target and target is valid",
 			args: args{