fix: correctly send links to login v2 in email notifications (#10711)

# Which Problems Are Solved There were still some emails (passkey registration and domain claimed) sent with links pointing to login v1 even when the login v2 was enabled for the instance. Also while looking into the issue, it was discovered that some links pointing to login V2 were not correctly generated. # How the Problems Are Solved - Added default paths for passkey registration and domain claimed notifications - Fixed the existing paths to properly handle concatenation (resp. use `url.ResolveReference`) - Change their go types (from string) to `*url.URL` - Added a mapstructure hook for string to url - Removed unnecessary `InstanceSetupFeatures` and corresponding conversions - Refactored the methods on the `login.DefaultPaths` struct and added an interface to the `Commands` to only need to pass a single config (and not every method) - Added an `OriginURL` method to the `DomainCtx` to prevent going from url to string and back - Added the use of the templates in case of enabled login v2 for passkey registration and domain claimed) # Additional Changes None # Additional Context closes #10643 --------- Co-authored-by: Max Peintner <[email protected]> Co-authored-by: Livio Spring <[email protected]> Co-authored-by: Livio Spring <[email protected]> Co-authored-by: Max Peintner <[email protected]> Co-authored-by: Gayathri Vijayan <[email protected]>
2026-10-28 01:52:16 +00:00 · 2026-02-13 13:06:56 +00:00
parent 382aec2d61
commit 2a2d5392a3
48 changed files with 1323 additions and 450 deletions
@@ -4,6 +4,7 @@ import (
 	"context"
 	"net/http"
 	"net/url"
+	"slices"
 	"strings"
 	"time"

@@ -56,39 +57,125 @@ type Config struct {
 }

 type DefaultPaths struct {
-	BasePath        string
-	PasswordSetPath string
-	EmailCodePath   string
-	OTPEmailPath    string
+	BasePath          *url.URL
+	PasswordSetPath   *url.URL
+	EmailCodePath     *url.URL
+	OTPEmailPath      *url.URL
+	PasskeySetPath    *url.URL
+	DomainClaimedPath *url.URL
 }

-func (c *Config) defaultBaseURL(ctx context.Context) string {
+func (c *DefaultPaths) defaultBaseURL(ctx context.Context) *url.URL {
 	loginV2 := authz.GetInstance(ctx).Features().LoginV2
-	if loginV2.Required {
-		// use the origin as default
-		baseURI := http_utils.DomainContext(ctx).Origin()
-		// use custom base URI if defined
-		if loginV2.BaseURI != nil && loginV2.BaseURI.String() != "" {
-			baseURI = loginV2.BaseURI.String()
-		}
-		return baseURI + c.DefaultPaths.BasePath
+	// In case login v1 is still active, we don't want to return a base URL, as the templates will not be used
+	if !loginV2.Required {
+		return nil
 	}
-	return ""
+	origin := http_utils.DomainContext(ctx).OriginURL()
+	if loginV2.BaseURI == nil || loginV2.BaseURI.String() == "" {
+		// In case the login v2 is enabled without a custom BaseURI,
+		// we use the request origin plus the default base path as the base URL for the templates.
+		if c.BasePath == nil {
+			return origin
+		}
+		return origin.ResolveReference(c.BasePath)
+	}
+	// In case a custom BaseURI is set for login v2, we use it as the base URL for the templates.
+	if loginV2.BaseURI.IsAbs() {
+		return loginV2.BaseURI
+	}
+	// If the custom BaseURI is a relative URL, we join the request origin with the custom BaseURI to form the base URL for the templates.
+	return origin.ResolveReference(loginV2.BaseURI)
 }

-func (c *Config) DefaultEmailCodeURLTemplate(ctx context.Context) string {
-	basePath := c.defaultBaseURL(ctx)
-	if basePath == "" {
+// mergeURLs will merge two URLs, where the path is joined and query parameters are combined.
+// It uses (*url.URL).JoinPath on the base URL and the second URL's Path field to build the resulting path,
+// and then merges the query parameters from both URLs using [mergeQueries], preserving existing values and placeholders.
+// Fragments are not modified or combined by this function.
+// For example, merging "https://example.com/base" with a URL whose path is "/path/to/resource" will result in "https://example.com/base/path/to/resource".
+func mergeURLs(base, path *url.URL) string {
+	if base == nil && path == nil {
 		return ""
 	}
-	return basePath + c.DefaultPaths.EmailCodePath
+	if base == nil {
+		return path.String()
+	}
+	if path == nil {
+		return base.String()
+	}
+	u := base.JoinPath(path.Path)
+	u.RawQuery = mergeQueries(u.Query(), path.Query())
+	return u.String()
 }
-func (c *Config) DefaultPasswordSetURLTemplate(ctx context.Context) string {
+
+// mergeQueries will merge two sets of query parameters, preserving existing values and placeholders.
+// It takes two url.Values, where the first one is considered the base and the second one is merged into it.
+// The resulting query string will contain all unique key-value pairs from both sets of query parameters.
+// If a value contains placeholders in the format "{{placeholder}}", they are preserved in the final query string without being URL-encoded.
+func mergeQueries(base, path url.Values) string {
+	placeholders := map[string]string{}
+	for _, value := range base {
+		for _, v := range value {
+			if strings.Contains(v, "{{") && strings.Contains(v, "}}") {
+				placeholders[url.QueryEscape(v)] = v
+			}
+		}
+	}
+	for key, value := range path {
+		baseValues, ok := base[key]
+		for _, v := range value {
+			if !ok || !slices.Contains(baseValues, v) {
+				base.Add(key, v)
+			}
+			if strings.Contains(v, "{{") && strings.Contains(v, "}}") {
+				placeholders[url.QueryEscape(v)] = v
+			}
+		}
+	}
+	raw := base.Encode()
+	for esc, orig := range placeholders {
+		raw = strings.ReplaceAll(raw, esc, orig)
+	}
+	return raw
+}
+
+func (c *DefaultPaths) DefaultEmailCodeURLTemplate(ctx context.Context) string {
 	basePath := c.defaultBaseURL(ctx)
-	if basePath == "" {
+	if basePath == nil {
 		return ""
 	}
-	return c.defaultBaseURL(ctx) + c.DefaultPaths.PasswordSetPath
+	return mergeURLs(basePath, c.EmailCodePath)
+}
+
+func (c *DefaultPaths) DefaultPasswordSetURLTemplate(ctx context.Context) string {
+	basePath := c.defaultBaseURL(ctx)
+	if basePath == nil {
+		return ""
+	}
+	return mergeURLs(basePath, c.PasswordSetPath)
+}
+
+func (c *DefaultPaths) DefaultPasskeySetURLTemplate(ctx context.Context) string {
+	basePath := c.defaultBaseURL(ctx)
+	if basePath == nil {
+		return ""
+	}
+	return mergeURLs(basePath, c.PasskeySetPath)
+}
+
+func (c *DefaultPaths) DefaultDomainClaimedURLTemplate(ctx context.Context) string {
+	basePath := c.defaultBaseURL(ctx)
+	if basePath == nil {
+		return ""
+	}
+	return mergeURLs(basePath, c.DomainClaimedPath)
+}
+
+func (c *DefaultPaths) DefaultOTPEmailURLTemplate(origin *url.URL) string {
+	if c.BasePath == nil {
+		return mergeURLs(origin, c.OTPEmailPath)
+	}
+	return mergeURLs(origin.ResolveReference(c.BasePath), c.OTPEmailPath)
 }

 const (
@@ -16,12 +16,10 @@ import (
 func TestConfig_defaultBaseURL(t *testing.T) {
 	t.Parallel()

-	config := &Config{
-		DefaultPaths: &DefaultPaths{BasePath: "/basepath"},
-	}
+	config := &DefaultPaths{BasePath: &url.URL{Path: "/basepath"}}

 	baseCustomURI, err := url.Parse("https://custom")
-	require.Nil(t, err)
+	require.NoError(t, err)

 	tt := []struct {
 		name     string
@@ -45,15 +43,14 @@ func TestConfig_defaultBaseURL(t *testing.T) {
 			expected: "https://origin/basepath",
 		},
 		{
-			name: "LoginV2 required, custom BaseURI",
+			name: "LoginV2 required, absolute custom BaseURI",
 			inputCtx: http.WithDomainContext(
 				authz.NewMockContext("instance1", "org1", "user1",
 					authz.WithMockFeatures(feature.Features{LoginV2: feature.LoginV2{Required: true, BaseURI: baseCustomURI}}),
 				),
 				&http.DomainCtx{Protocol: "https", PublicHost: "origin"},
 			),
-
-			expected: "https://custom/basepath",
+			expected: "https://custom",
 		},
 		{
 			name: "LoginV2 required, custom BaseURI empty string",
@@ -65,13 +62,27 @@ func TestConfig_defaultBaseURL(t *testing.T) {
 			),
 			expected: "https://origin/basepath",
 		},
+		{
+			name: "LoginV2 required, relative custom BaseURI",
+			inputCtx: http.WithDomainContext(
+				authz.NewMockContext("instance1", "org1", "user1",
+					authz.WithMockFeatures(feature.Features{LoginV2: feature.LoginV2{Required: true, BaseURI: &url.URL{Path: "/custom"}}}),
+				),
+				&http.DomainCtx{Protocol: "https", PublicHost: "origin"},
+			),
+			expected: "https://origin/custom",
+		},
 	}

 	for _, tc := range tt {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 			result := config.defaultBaseURL(tc.inputCtx)
-			assert.Equal(t, tc.expected, result)
+			if tc.expected == "" {
+				assert.Nil(t, result)
+				return
+			}
+			assert.Equal(t, tc.expected, result.String())
 		})
 	}
 }
@@ -111,10 +122,9 @@ func TestConfig_DefaultEmailCodeURLTemplate(t *testing.T) {
 			t.Parallel()

 			// Given
-			c := &Config{
-				DefaultPaths: &DefaultPaths{
-					BasePath:      "/basepath",
-					EmailCodePath: "/email-code-path"},
+			c := &DefaultPaths{
+				BasePath:      &url.URL{Path: "/basepath"},
+				EmailCodePath: &url.URL{Path: "/email-code-path"},
 			}

 			// Test
@@ -161,11 +171,9 @@ func TestConfig_DefaultPasswordSetURLTemplate(t *testing.T) {
 			t.Parallel()

 			// Given
-			c := &Config{
-				DefaultPaths: &DefaultPaths{
-					BasePath:        "/basepath",
-					PasswordSetPath: "/password-set-path",
-				},
+			c := &DefaultPaths{
+				BasePath:        &url.URL{Path: "/basepath"},
+				PasswordSetPath: &url.URL{Path: "/password-set-path"},
 			}

 			// Test
@@ -176,3 +184,220 @@ func TestConfig_DefaultPasswordSetURLTemplate(t *testing.T) {
 		})
 	}
 }
+
+func TestConfig_DefaultPasskeySetURLTemplate(t *testing.T) {
+	t.Parallel()
+
+	tt := []struct {
+		testName                 string
+		inputCtx                 context.Context
+		expectedEmailURLTemplate string
+	}{
+		{
+			testName: "when base path is empty should return empty email url template",
+			inputCtx: http.WithDomainContext(
+				authz.NewMockContext("instance1", "org1", "user1",
+					authz.WithMockFeatures(feature.Features{LoginV2: feature.LoginV2{Required: false, BaseURI: &url.URL{}}}),
+				),
+				&http.DomainCtx{Protocol: "https", PublicHost: "origin"},
+			),
+			expectedEmailURLTemplate: "",
+		},
+		{
+			testName: "when base path is not empty should return expected url template",
+			inputCtx: http.WithDomainContext(
+				authz.NewMockContext("instance1", "org1", "user1",
+					authz.WithMockFeatures(feature.Features{LoginV2: feature.LoginV2{Required: true, BaseURI: &url.URL{}}}),
+				),
+				&http.DomainCtx{Protocol: "https", PublicHost: "origin"},
+			),
+			expectedEmailURLTemplate: "https://origin/basepath/passkey-set-path",
+		},
+	}
+
+	for _, tc := range tt {
+		t.Run(tc.testName, func(t *testing.T) {
+			t.Parallel()
+
+			// Given
+			c := &DefaultPaths{
+				BasePath:       &url.URL{Path: "/basepath"},
+				PasskeySetPath: &url.URL{Path: "/passkey-set-path"},
+			}
+
+			// Test
+			res := c.DefaultPasskeySetURLTemplate(tc.inputCtx)
+
+			// Verify
+			assert.Equal(t, tc.expectedEmailURLTemplate, res)
+		})
+	}
+}
+
+func TestConfig_DefaultDomainClaimedURLTemplate(t *testing.T) {
+	t.Parallel()
+
+	tt := []struct {
+		testName                 string
+		inputCtx                 context.Context
+		expectedEmailURLTemplate string
+	}{
+		{
+			testName: "when base path is empty should return empty email url template",
+			inputCtx: http.WithDomainContext(
+				authz.NewMockContext("instance1", "org1", "user1",
+					authz.WithMockFeatures(feature.Features{LoginV2: feature.LoginV2{Required: false, BaseURI: &url.URL{}}}),
+				),
+				&http.DomainCtx{Protocol: "https", PublicHost: "origin"},
+			),
+			expectedEmailURLTemplate: "",
+		},
+		{
+			testName: "when base path is not empty should return expected url template",
+			inputCtx: http.WithDomainContext(
+				authz.NewMockContext("instance1", "org1", "user1",
+					authz.WithMockFeatures(feature.Features{LoginV2: feature.LoginV2{Required: true, BaseURI: &url.URL{}}}),
+				),
+				&http.DomainCtx{Protocol: "https", PublicHost: "origin"},
+			),
+			expectedEmailURLTemplate: "https://origin/basepath/loginname",
+		},
+	}
+
+	for _, tc := range tt {
+		t.Run(tc.testName, func(t *testing.T) {
+			t.Parallel()
+
+			// Given
+			c := &DefaultPaths{
+				BasePath:          &url.URL{Path: "/basepath"},
+				DomainClaimedPath: &url.URL{Path: "/loginname"},
+			}
+
+			// Test
+			res := c.DefaultDomainClaimedURLTemplate(tc.inputCtx)
+
+			// Verify
+			assert.Equal(t, tc.expectedEmailURLTemplate, res)
+		})
+	}
+}
+
+func Test_mergeURLs(t *testing.T) {
+	type args struct {
+		base *url.URL
+		path *url.URL
+	}
+	tests := []struct {
+		name string
+		args args
+		want string
+	}{
+		{
+			name: "both base and path are nil, return empty string",
+			args: args{
+				base: nil,
+				path: nil,
+			},
+			want: "",
+		},
+		{
+			name: "base is nil, path is set, return path",
+			args: args{
+				base: nil,
+				path: &url.URL{Path: "/path"},
+			},
+			want: "/path",
+		},
+		{
+			name: "base is set, path is nil, return base",
+			args: args{
+				base: &url.URL{Path: "/base"},
+				path: nil,
+			},
+			want: "/base",
+		},
+		{
+			name: "both base and path are set, return merged path",
+			args: args{
+				base: &url.URL{Path: "/base"},
+				path: &url.URL{Path: "/path"},
+			},
+			want: "/base/path",
+		},
+		{
+			name: "both base and path are set incl. placeholders, return merged path",
+			args: args{
+				base: &url.URL{Path: "/base", RawQuery: "query={{.Placeholder}}"},
+				path: &url.URL{Path: "/path", RawQuery: "query={{.Placeholder1}}&query2={{.Placeholder2}}"},
+			},
+			want: "/base/path?query={{.Placeholder}}&query={{.Placeholder1}}&query2={{.Placeholder2}}",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := mergeURLs(tt.args.base, tt.args.path)
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}
+
+func Test_mergeQueries(t *testing.T) {
+	type args struct {
+		base url.Values
+		path url.Values
+	}
+	tests := []struct {
+		name string
+		args args
+		want string
+	}{
+		{
+			name: "both base and path are empty, return empty string",
+			args: args{
+				base: map[string][]string{},
+				path: map[string][]string{},
+			},
+			want: "",
+		},
+		{
+			name: "base is empty, path is set, return path",
+			args: args{
+				base: map[string][]string{},
+				path: map[string][]string{
+					"query": {"{{.Placeholder1}}", "{{.Placeholder2}}"},
+				},
+			},
+			want: "query={{.Placeholder1}}&query={{.Placeholder2}}",
+		},
+		{
+			name: "base is set, path is empty, return base",
+			args: args{
+				base: map[string][]string{
+					"query": {"{{.Placeholder}}"},
+				},
+				path: map[string][]string{},
+			},
+			want: "query={{.Placeholder}}",
+		},
+		{
+			name: "both base and path are set, return merged query",
+			args: args{
+				base: map[string][]string{
+					"query":  {"{{.Placeholder}}"},
+					"query2": {"{{.Placeholder}}"},
+				},
+				path: map[string][]string{
+					"query":  {"{{.Placeholder1}}"},
+					"query2": {"{{.Placeholder}}", "{{.Placeholder2}}"},
+				},
+			},
+			want: "query={{.Placeholder}}&query={{.Placeholder1}}&query2={{.Placeholder}}&query2={{.Placeholder2}}",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equalf(t, tt.want, mergeQueries(tt.args.base, tt.args.path), "mergeQueries(%v, %v)", tt.args.base, tt.args.path)
+		})
+	}
+}