Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
shreyas-goenka committed Nov 4, 2024
1 parent 04d5d72 commit 1517d0a
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 20 deletions.
12 changes: 7 additions & 5 deletions libs/auth/oauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,19 +144,21 @@ func (a *PersistentAuth) Challenge(ctx context.Context) error {
return nil
}

// Best effort to remove url query args and fragments from the host
// Remove url query and path args and fragments from the host
func (a *PersistentAuth) cleanHost() {
parsedHost, err := url.Parse(a.Host)
if err != nil {
return
}
// when either host or scheme is empty, we don't want to clean it. This is because
// the Go url library parses a raw "abc" string as the path of a URL and cleaning
// it will return thus return an empty string.
if parsedHost.Host == "" || parsedHost.Scheme == "" {
return
}
host := url.URL{
Scheme: parsedHost.Scheme,
Host: parsedHost.Host,

// We retain the path, because it may contain the account id for account
// logins.
Path: parsedHost.Path,
}
a.Host = host.String()
}
Expand Down
34 changes: 19 additions & 15 deletions libs/auth/oauth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,22 +234,26 @@ func TestPersistentAuthCleanHost(t *testing.T) {
in string
out string
}{

{"https://example.com", "https://example.com"},
{"https://example.com/path", "https://example.com/path"},
{"https://example.com/path/subpath", "https://example.com/path/subpath"},
{"https://example.com/path?query=1", "https://example.com/path"},
{"https://example.com/path?query=1&other=2", "https://example.com/path"},
{"https://example.com/path#fragment", "https://example.com/path"},
{"https://example.com/path?query=1#fragment", "https://example.com/path"},
{"https://example.com/path?query=1&other=2#fragment", "https://example.com/path"},
{"https://example.com/path/subpath?query=1", "https://example.com/path/subpath"},
{"https://example.com/path/subpath?query=1&other=2", "https://example.com/path/subpath"},
{"https://example.com/path/subpath#fragment", "https://example.com/path/subpath"},
{"https://example.com/path/subpath?query=1#fragment", "https://example.com/path/subpath"},
{"https://example.com/path/subpath?query=1&other=2#fragment", "https://example.com/path/subpath"},
{"https://example.com/path?query=1%20value&other=2%20value", "https://example.com/path"},
{"https://example.com/path/subpath?query=1%20value&other=2%20value", "https://example.com/path/subpath"},
{"https://example.com/", "https://example.com"},
{"https://example.com/path", "https://example.com"},
{"https://example.com/path/subpath", "https://example.com"},
{"https://example.com/path?query=1", "https://example.com"},
{"https://example.com/path?query=1&other=2", "https://example.com"},
{"https://example.com/path#fragment", "https://example.com"},
{"https://example.com/path?query=1#fragment", "https://example.com"},
{"https://example.com/path?query=1&other=2#fragment", "https://example.com"},
{"https://example.com/path/subpath?query=1", "https://example.com"},
{"https://example.com/path/subpath?query=1&other=2", "https://example.com"},
{"https://example.com/path/subpath#fragment", "https://example.com"},
{"https://example.com/path/subpath?query=1#fragment", "https://example.com"},
{"https://example.com/path/subpath?query=1&other=2#fragment", "https://example.com"},
{"https://example.com/path?query=1%20value&other=2%20value", "https://example.com"},
{"http://example.com/path/subpath?query=1%20value&other=2%20value", "http://example.com"},

// URLs without scheme should be left as is
{"abc", "abc"},
{"abc.com/def", "abc.com/def"},
} {
p := &PersistentAuth{
Host: tcases.in,
Expand Down

0 comments on commit 1517d0a

Please sign in to comment.