fix(authorization): final slash in url matches ignored (#3717)

This fixes an issue with the URL matching machinery which ignores the final slash of a URL. Introduced in 664d65d7fb.

Fixes #3692

internal/authorization/authorizer_test.go

@@ -669,16 +669,22 @@ func (s *AuthorizerSuite) TestShouldCheckResourceMatching() {
 			Policy:    twoFactor,
 			Resources: createSliceRegexRule(s.T(), []string{"^/a/longer/rule/.*$"}),
 		}).
+		WithRule(schema.ACLRule{
+			Domains:   []string{"resource.example.com"},
+			Policy:    twoFactor,
+			Resources: createSliceRegexRule(s.T(), []string{"^/an/exact/path/$"}),
+		}).
 		Build()
 
 	tester.CheckAuthorizations(s.T(), John, "https://resource.example.com/", "GET", Bypass)
 	tester.CheckAuthorizations(s.T(), John, "https://resource.example.com/bypass/abc", "GET", Bypass)
-	tester.CheckAuthorizations(s.T(), John, "https://resource.example.com/bypass/", "GET", Denied)
+	tester.CheckAuthorizations(s.T(), John, "https://resource.example.com/bypass/", "GET", Bypass)
 	tester.CheckAuthorizations(s.T(), John, "https://resource.example.com/one_factor/abc", "GET", OneFactor)
 	tester.CheckAuthorizations(s.T(), John, "https://resource.example.com/xyz/embedded/abc", "GET", Bypass)
 	tester.CheckAuthorizations(s.T(), John, "https://resource.example.com/a/longer/rule/abc", "GET", TwoFactor)
 	tester.CheckAuthorizations(s.T(), John, "https://resource.example.com/case/abc", "GET", Bypass)
 	tester.CheckAuthorizations(s.T(), John, "https://resource.example.com/case/ABC", "GET", Denied)
+	tester.CheckAuthorizations(s.T(), John, "https://resource.example.com/an/exact/path/", "GET", TwoFactor)
 	tester.CheckAuthorizations(s.T(), John, "https://resource.example.com/bypass/../a/longer/rule/abc", "GET", TwoFactor)
 	tester.CheckAuthorizations(s.T(), John, "https://resource.example.com/bypass/..//a/longer/rule/abc", "GET", TwoFactor)
 	tester.CheckAuthorizations(s.T(), John, "https://resource.example.com/bypass/..%2f/a/longer/rule/abc", "GET", TwoFactor)
@@ -697,6 +703,7 @@ func (s *AuthorizerSuite) TestShouldCheckResourceMatching() {
 	tester.CheckAuthorizations(s.T(), John, "https://resource.example.com/bypass/%2E%2E%2fa/longer/rule/abc", "GET", TwoFactor)
 	tester.CheckAuthorizations(s.T(), John, "https://resource.example.com/bypass/%2E%2E%2F/a/longer/rule/abc", "GET", TwoFactor)
 	tester.CheckAuthorizations(s.T(), John, "https://resource.example.com/bypass/%2E%2E%2Fa/longer/rule/abc", "GET", TwoFactor)
+	tester.CheckAuthorizations(s.T(), John, "https://resource.example.com/bypass/%2E%2E%2Fan/exact/path/", "GET", TwoFactor)
 }
 
 // This test assures that rules without domains (not allowed by schema validator at this time) will pass validation correctly.

internal/utils/url.go

@@ -7,11 +7,23 @@ import (
 
 // URLPathFullClean returns a URL path with the query parameters appended (full path) with the path portion parsed
 // through path.Clean given a *url.URL.
-func URLPathFullClean(u *url.URL) (p string) {
+func URLPathFullClean(u *url.URL) (output string) {
-	switch len(u.RawQuery) {
+	lengthPath := len(u.Path)
-	case 0:
+	lengthQuery := len(u.RawQuery)
-		return path.Clean(u.Path)
+	appendForwardSlash := lengthPath > 1 && u.Path[lengthPath-1] == '/'
-	default:
+
+	switch {
+	case lengthPath == 1 && lengthQuery == 0:
+		return u.Path
+	case lengthPath == 1:
 		return path.Clean(u.Path) + "?" + u.RawQuery
+	case lengthQuery != 0 && appendForwardSlash:
+		return path.Clean(u.Path) + "/?" + u.RawQuery
+	case lengthQuery != 0:
+		return path.Clean(u.Path) + "?" + u.RawQuery
+	case appendForwardSlash:
+		return path.Clean(u.Path) + "/"
+	default:
+		return path.Clean(u.Path)
 	}
 }

internal/utils/url_test.go

@@ -14,13 +14,17 @@ func TestURLPathFullClean(t *testing.T) {
 		have     string
 		expected string
 	}{
+		{"ShouldReturnFullPathSingleSlash", "https://example.com/", "/"},
+		{"ShouldReturnFullPathSingleSlashWithQuery", "https://example.com/?query=1&alt=2", "/?query=1&alt=2"},
 		{"ShouldReturnFullPathNormal", "https://example.com/test", "/test"},
-		{"ShouldReturnFullPathWithQuery", "https://example.com/test?query=1", "/test?query=1"},
+		{"ShouldReturnFullPathNormalWithSlashSuffix", "https://example.com/test/", "/test/"},
-		{"ShouldReturnCleanedPath", "https://example.com/five/../test?query=1", "/test?query=1"},
+		{"ShouldReturnFullPathNormalWithSlashSuffixAndQuery", "https://example.com/test/?query=1&alt=2", "/test/?query=1&alt=2"},
-		{"ShouldReturnCleanedPathEscaped", "https://example.com/five/..%2ftest?query=1", "/test?query=1"},
+		{"ShouldReturnFullPathWithQuery", "https://example.com/test?query=1&alt=2", "/test?query=1&alt=2"},
-		{"ShouldReturnCleanedPathEscapedExtra", "https://example.com/five/..%2ftest?query=1", "/test?query=1"},
+		{"ShouldReturnCleanedPath", "https://example.com/five/../test?query=1&alt=2", "/test?query=1&alt=2"},
-		{"ShouldReturnCleanedPathEscapedExtraSurrounding", "https://example.com/five/%2f..%2f/test?query=1", "/test?query=1"},
+		{"ShouldReturnCleanedPathEscaped", "https://example.com/five/..%2ftest?query=1&alt=2", "/test?query=1&alt=2"},
-		{"ShouldReturnCleanedPathEscapedPeriods", "https://example.com/five/%2f%2e%2e%2f/test?query=1", "/test?query=1"},
+		{"ShouldReturnCleanedPathEscapedExtra", "https://example.com/five/..%2ftest?query=1&alt=2", "/test?query=1&alt=2"},
+		{"ShouldReturnCleanedPathEscapedExtraSurrounding", "https://example.com/five/%2f..%2f/test?query=1&alt=2", "/test?query=1&alt=2"},
+		{"ShouldReturnCleanedPathEscapedPeriods", "https://example.com/five/%2f%2e%2e%2f/test?query=1&alt=2", "/test?query=1&alt=2"},
 	}
 
 	for _, tc := range testCases {