Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make redirect uri validation case-insensitve #282

Merged
merged 5 commits into from
Mar 13, 2024
Merged

make redirect uri validation case-insensitve #282

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fffbc6c
make redirect uri validation case-insensitve
fairclothjm Mar 12, 2024
ed6ec0d
improve loopback check; add comment
fairclothjm Mar 13, 2024
2467031
update test loop var name
fairclothjm Mar 13, 2024
d5fd40d
update helper func name
fairclothjm Mar 13, 2024
af5fe32
simplify return in helper func
fairclothjm Mar 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion path_oidc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"encoding/json"
"errors"
"fmt"
"net"
"net/http"
"net/url"
"strings"
Expand Down Expand Up @@ -537,6 +538,17 @@ func (b *jwtAuthBackend) verifyOIDCRequest(stateID string) *oidcRequest {
return nil
}

func isLoopbackAddress(hostname string) bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! One minor nit, I would maybe name this function isLocalAddr() or isLocalhost() since it also compares against a hostname.

ip := net.ParseIP(hostname)
if ip != nil {
return ip.IsLoopback()
} else {
// localhost is not guaranteed to map back to a loopback interface address
// however, this is historically how the plugin has behaved
return hostname == "localhost"
}
}

// validRedirect checks whether uri is in allowed using special handling for loopback uris.
// Ref: https://tools.ietf.org/html/rfc8252#section-7.3
func validRedirect(uri string, allowed []string) bool {
Expand All @@ -546,7 +558,7 @@ func validRedirect(uri string, allowed []string) bool {
}

// if uri isn't a loopback, just string search the allowed list
if !strutil.StrListContains([]string{"localhost", "127.0.0.1", "::1"}, inputURI.Hostname()) {
if !isLoopbackAddress(inputURI.Hostname()) {
return strutil.StrListContainsCaseInsensitive(allowed, uri)
}

Expand Down
1 change: 1 addition & 0 deletions path_oidc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1563,6 +1563,7 @@ func TestOIDC_ValidRedirect(t *testing.T) {
{"https://example.com/a/b/c", []string{"a", "b", "https://example.com/a/b/c"}, true},
{"https://localhost:9000", []string{"a", "b", "https://localhost:5000"}, true},
{"https://127.0.0.1:9000", []string{"a", "b", "https://127.0.0.1:5000"}, true},
{"https://127.0.0.2:9000", []string{"a", "b", "https://127.0.0.2:5000"}, true},
{"https://[::1]:9000", []string{"a", "b", "https://[::1]:5000"}, true},
{"https://[::1]:9000/x/y?r=42", []string{"a", "b", "https://[::1]:5000/x/y?r=42"}, true},
{"https://EXAMPLE.com:5000", []string{"a", "b", "https://example.com:5000"}, true},
Expand Down