From a67c80a02eeab2a18a2971ac0e327dff7a112ee8 Mon Sep 17 00:00:00 2001 From: Rohith Jayawardene <gambol99@gmail.com> Date: Fri, 17 Mar 2017 20:19:45 +0000 Subject: [PATCH] FIXES: (#201) * Fixes a bug in authentication, which permitted double slashed url entry [#PR200](https://github.com/gambol99/keycloak-proxy/pull/200) FEATURES: * Grabbing the revocation-url from the idp config if user override is not specified [#PR193](https://github.com/gambol99/keycloak-proxy/pull/193) --- CHANGELOG.md | 3 +++ doc.go | 2 +- middleware.go | 17 ++++++++++++++++ middleware_test.go | 48 ++++++++++++++++++++++++++++++++++++++++++++++ server.go | 3 +++ 5 files changed, 72 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 90ee419..743ef45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,9 @@ #### **2.0.4** +FIXES: + * Fixes a bug in authentication, which permitted double slashed url entry [#PR200](https://github.com/gambol99/keycloak-proxy/pull/200) + FEATURES: * Grabbing the revocation-url from the idp config if user override is not specified [#PR193](https://github.com/gambol99/keycloak-proxy/pull/193) diff --git a/doc.go b/doc.go index 020b835..47cd03d 100644 --- a/doc.go +++ b/doc.go @@ -24,7 +24,7 @@ import ( ) var ( - release = "v2.0.3" + release = "v2.0.4" gitsha = "no gitsha provided" version = release + " (git+sha: " + gitsha + ")" ) diff --git a/middleware.go b/middleware.go index 13e4cc2..2a0fa21 100644 --- a/middleware.go +++ b/middleware.go @@ -16,6 +16,7 @@ limitations under the License. package main import ( + "bytes" "fmt" "regexp" "strings" @@ -33,6 +34,22 @@ const ( cxEnforce = "Enforcing" ) +// filterMiddleware is custom filtering for incoming requests +func (r *oauthProxy) filterMiddleware() gin.HandlerFunc { + return func(cx *gin.Context) { + var p rune + var b bytes.Buffer + for _, c := range cx.Request.URL.Path { + if c == '/' && p == '/' { + continue + } + p = c + b.WriteRune(c) + } + cx.Request.URL.Path = b.String() + } +} + // loggingMiddleware is a custom http logger func (r *oauthProxy) loggingMiddleware() gin.HandlerFunc { return func(cx *gin.Context) { diff --git a/middleware_test.go b/middleware_test.go index c476107..2a8e93c 100644 --- a/middleware_test.go +++ b/middleware_test.go @@ -93,6 +93,54 @@ func TestRolePermissionsMiddleware(t *testing.T) { Redirects: true, Expects: http.StatusOK, }, + { // check for escaping + URI: "//admin%2Ftest", + Redirects: true, + Expects: http.StatusTemporaryRedirect, + }, + { // check for escaping + URI: "/admin%2Ftest", + Redirects: true, + Expects: http.StatusTemporaryRedirect, + }, + { // check for prefix slashs + URI: "//admin/test", + Redirects: true, + Expects: http.StatusTemporaryRedirect, + }, + { // check for prefix slashs + URI: "/admin//test", + Redirects: true, + Expects: http.StatusTemporaryRedirect, + }, + { // check for prefix slashs + URI: "/admin//test", + Redirects: false, + HasToken: true, + Expects: http.StatusForbidden, + }, + { // check for dodgy url + URI: "//admin/../admin/test", + Redirects: true, + Expects: http.StatusTemporaryRedirect, + }, + { // check for dodgy url + URI: "/help/../admin/test", + Redirects: true, + Expects: http.StatusTemporaryRedirect, + }, + { // check for it works + URI: "//admin/test", + HasToken: true, + Roles: []string{fakeAdminRole}, + Expects: http.StatusOK, + }, + { // check for it works + URI: "//admin//test", + HasToken: true, + Roles: []string{fakeAdminRole}, + Expects: http.StatusOK, + }, { // check with a token URI: "/", Redirects: false, diff --git a/server.go b/server.go index 87f1028..9dda7bf 100644 --- a/server.go +++ b/server.go @@ -154,6 +154,9 @@ func (r *oauthProxy) createReverseProxy() error { // step: create the gin router engine := gin.New() engine.Use(gin.Recovery()) + // step: custom filtering + engine.Use(r.filterMiddleware()) + // step: is profiling enabled? if r.config.EnableProfiling { log.Warn("Enabling the debug profiling on /debug/pprof") -- GitLab