From 672d54fafa5c5a805886b4125daca8ba1d490af9 Mon Sep 17 00:00:00 2001 From: M Hickford Date: Sat, 8 Oct 2022 02:52:35 +0100 Subject: [PATCH] Ignore port for OAuth2 loopback redirect URIs (#21293) (#21373) Backport #21293 Following https://datatracker.ietf.org/doc/html/rfc8252#section-7.3 Fixes #21285 --- models/auth/oauth2.go | 13 +++++++++++++ models/auth/oauth2_test.go | 20 ++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/models/auth/oauth2.go b/models/auth/oauth2.go index 9c479becd9..92c3eb96e9 100644 --- a/models/auth/oauth2.go +++ b/models/auth/oauth2.go @@ -10,6 +10,7 @@ import ( "encoding/base32" "encoding/base64" "fmt" + "net" "net/url" "strings" @@ -56,6 +57,18 @@ func (app *OAuth2Application) PrimaryRedirectURI() string { // ContainsRedirectURI checks if redirectURI is allowed for app func (app *OAuth2Application) ContainsRedirectURI(redirectURI string) bool { + uri, err := url.Parse(redirectURI) + // ignore port for http loopback uris following https://datatracker.ietf.org/doc/html/rfc8252#section-7.3 + if err == nil && uri.Scheme == "http" && uri.Port() != "" { + ip := net.ParseIP(uri.Hostname()) + if ip != nil && ip.IsLoopback() { + // strip port + uri.Host = uri.Hostname() + if util.IsStringInSlice(uri.String(), app.RedirectURIs, true) { + return true + } + } + } return util.IsStringInSlice(redirectURI, app.RedirectURIs, true) } diff --git a/models/auth/oauth2_test.go b/models/auth/oauth2_test.go index cb8c4aeb6a..19cab9a3be 100644 --- a/models/auth/oauth2_test.go +++ b/models/auth/oauth2_test.go @@ -42,6 +42,26 @@ func TestOAuth2Application_ContainsRedirectURI(t *testing.T) { assert.False(t, app.ContainsRedirectURI("d")) } +func TestOAuth2Application_ContainsRedirectURI_WithPort(t *testing.T) { + app := &OAuth2Application{ + RedirectURIs: []string{"http://127.0.0.1/", "http://::1/", "http://192.168.0.1/", "http://intranet/", "https://127.0.0.1/"}, + } + + // http loopback uris should ignore port + // https://datatracker.ietf.org/doc/html/rfc8252#section-7.3 + assert.True(t, app.ContainsRedirectURI("http://127.0.0.1:3456/")) + assert.True(t, app.ContainsRedirectURI("http://127.0.0.1/")) + assert.True(t, app.ContainsRedirectURI("http://[::1]:3456/")) + + // not http + assert.False(t, app.ContainsRedirectURI("https://127.0.0.1:3456/")) + // not loopback + assert.False(t, app.ContainsRedirectURI("http://192.168.0.1:9954/")) + assert.False(t, app.ContainsRedirectURI("http://intranet:3456/")) + // unparseable + assert.False(t, app.ContainsRedirectURI(":")) +} + func TestOAuth2Application_ValidateClientSecret(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) app := unittest.AssertExistsAndLoadBean(t, &OAuth2Application{ID: 1}).(*OAuth2Application)