diff --git a/CHANGELOG.md b/CHANGELOG.md index 90ee419b8..743ef453a 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 020b8357f..47cd03dac 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 13e4cc22e..f2d76810e 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) { @@ -51,7 +68,7 @@ func (r *oauthProxy) loggingMiddleware() gin.HandlerFunc { } } -// metricsMiddleware is responsible for collecting metrics +// metricsMiddleware is responsiblie for collecting metrics func (r *oauthProxy) metricsMiddleware() gin.HandlerFunc { log.Infof("enabled the service metrics middleware, available on %s%s", oauthURL, metricsURL) diff --git a/middleware_test.go b/middleware_test.go index c476107ad..c858e6ff4 100644 --- a/middleware_test.go +++ b/middleware_test.go @@ -93,6 +93,49 @@ 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 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 87f10280d..f267998da 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: remove the slashs + engine.Use(r.filterMiddleware()) + // step: is profiling enabled? if r.config.EnableProfiling { log.Warn("Enabling the debug profiling on /debug/pprof")