From 6a45a691c19003c15ceee3e14a321a90dfef5183 Mon Sep 17 00:00:00 2001 From: M Hickford Date: Wed, 28 Sep 2022 23:19:55 +0100 Subject: [PATCH] Ignore port for loopback redirect URIs (#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 ad1d80e25a..73c250d4af 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 3b2ba8c8f1..3815cb3b2c 100644 --- a/models/auth/oauth2_test.go +++ b/models/auth/oauth2_test.go @@ -43,6 +43,26 @@ func TestOAuth2Application_ContainsRedirectURI(t *testing.T) { assert.False(t, app.ContainsRedirectURI("d")) } +func TestOAuth2Application_ContainsRedirectURI_WithPort(t *testing.T) { + app := &auth_model.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, &auth_model.OAuth2Application{ID: 1})