Commit de270d18 authored by Rohith Jayawardene's avatar Rohith Jayawardene Committed by GitHub
Browse files

Normalize URL (#202)

- normal the url before we apply the protection middleware against it
- adding additional unit tests
- adding the extra dependencies
- updated the CHANGELOG for later release
parent a67c80a0
#### **2.0.5 (unreleased)**
FIXES:
* We normalize all urls before the protection middleware is applied [#PR202](https://github.com/gambol99/keycloak-proxy/pull/202)
#### **2.0.4**
FIXES:
......
{
"ImportPath": "github.com/gambol99/keycloak-proxy",
"GoVersion": "go1.7",
"GoVersion": "go1.8",
"GodepVersion": "v79",
"Deps": [
{
"ImportPath": "github.com/PuerkitoBio/purell",
"Comment": "v1.1.0",
"Rev": "0bcb03f4b4d0a9428594752bd2a3b9aa0a9d4bd4"
},
{
"ImportPath": "github.com/PuerkitoBio/urlesc",
"Rev": "5bd2802263f21d8788851d5305584c82a5c75d7e"
},
{
"ImportPath": "github.com/Sirupsen/logrus",
"Comment": "v0.10.0-14-g081307d",
......@@ -41,16 +50,6 @@
"ImportPath": "github.com/coreos/go-oidc/oidc",
"Rev": "dedb650fb29c39c2f21aa88c1e4cec66da8754d1"
},
{
"ImportPath": "github.com/coreos/go-systemd/journal",
"Comment": "v14-2-gefa0506",
"Rev": "efa0506b99d3d9cfade280f5c7a1c1980d8c6c48"
},
{
"ImportPath": "github.com/coreos/pkg/capnslog",
"Comment": "v3-2-g447b7ec",
"Rev": "447b7ec906e523386d9c53be15b55a8ae86ea944"
},
{
"ImportPath": "github.com/coreos/pkg/health",
"Comment": "v3-2-g447b7ec",
......@@ -70,16 +69,16 @@
"ImportPath": "github.com/davecgh/go-spew/spew",
"Rev": "5215b55f46b2b919f50a1df0eaa5886afe4e3b3d"
},
{
"ImportPath": "github.com/gambol99/goproxy",
"Comment": "v1.0-87-gc3b6ff1",
"Rev": "c3b6ff1178d68ec9e93f7c996f41a3df89931d9f"
},
{
"ImportPath": "github.com/fsnotify/fsnotify",
"Comment": "v1.4.2-2-gfd9ec7d",
"Rev": "fd9ec7deca8bf46ecd2a795baaacf2b3a9be1197"
},
{
"ImportPath": "github.com/gambol99/goproxy",
"Comment": "v1.0-87-gc3b6ff1",
"Rev": "c3b6ff1178d68ec9e93f7c996f41a3df89931d9f"
},
{
"ImportPath": "github.com/gin-gonic/gin",
"Comment": "v1.1.4",
......@@ -112,10 +111,6 @@
"ImportPath": "github.com/manucorporat/sse",
"Rev": "ee05b128a739a0fb76c7ebd3ae4810c1de808d6d"
},
{
"ImportPath": "github.com/mattn/go-colorable",
"Rev": "9cbef7c35391cca05f15f8181dc0b18bc9736dbb"
},
{
"ImportPath": "github.com/mattn/go-isatty",
"Rev": "56b76bdf51f7708750eac80fa38b952bb9f32639"
......@@ -173,6 +168,10 @@
"ImportPath": "golang.org/x/net/context",
"Rev": "bc3663df0ac92f928d419e31e0d2af22e683a5a2"
},
{
"ImportPath": "golang.org/x/net/idna",
"Rev": "bc3663df0ac92f928d419e31e0d2af22e683a5a2"
},
{
"ImportPath": "golang.org/x/net/publicsuffix",
"Rev": "bc3663df0ac92f928d419e31e0d2af22e683a5a2"
......@@ -182,9 +181,16 @@
"Rev": "833a04a10549a95dc34458c195cbad61bbb6cb4d"
},
{
"ImportPath": "gopkg.in/bluesuncorp/validator.v5",
"Comment": "v5.12",
"Rev": "d5acf1dac43705f8bfbb71d878e290e2bed3950b"
"ImportPath": "golang.org/x/text/transform",
"Rev": "f28f36722d5ef2f9655ad3de1f248e3e52ad5ebd"
},
{
"ImportPath": "golang.org/x/text/unicode/norm",
"Rev": "f28f36722d5ef2f9655ad3de1f248e3e52ad5ebd"
},
{
"ImportPath": "golang.org/x/text/width",
"Rev": "f28f36722d5ef2f9655ad3de1f248e3e52ad5ebd"
},
{
"ImportPath": "gopkg.in/bsm/ratelimit.v1",
......
......@@ -29,6 +29,9 @@ import (
// reverseProxyMiddleware is responsible for handles reverse proxy request to the upstream endpoint
func (r *oauthProxy) reverseProxyMiddleware() gin.HandlerFunc {
return func(cx *gin.Context) {
// step: continue the flow
cx.Next()
// step: check its cool to continue
if cx.IsAborted() {
return
}
......
......@@ -262,8 +262,8 @@ func TestAuthorizationURL(t *testing.T) {
ExpectedCode: http.StatusTemporaryRedirect,
},
{
URL: "/admin/../",
ExpectedURL: "/oauth/authorize?state=L2FkbWluLy4uLw==",
URL: "/help/../admin",
ExpectedURL: "/oauth/authorize?state=L2FkbWlu",
ExpectedCode: http.StatusTemporaryRedirect,
},
{
......
......@@ -16,12 +16,12 @@ limitations under the License.
package main
import (
"bytes"
"fmt"
"regexp"
"strings"
"time"
"github.com/PuerkitoBio/purell"
log "github.com/Sirupsen/logrus"
"github.com/coreos/go-oidc/jose"
"github.com/gin-gonic/gin"
......@@ -34,19 +34,19 @@ const (
cxEnforce = "Enforcing"
)
const normalizeFlags purell.NormalizationFlags = purell.FlagRemoveDotSegments | purell.FlagRemoveDuplicateSlashes
// 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()
// step: keep a copy of the original
orig := cx.Request.URL.Path
// step: normalize the url
purell.NormalizeURL(cx.Request.URL, normalizeFlags)
// step: continue the flow
cx.Next()
// step: place back the original
cx.Request.URL.Path = orig
}
}
......
......@@ -26,60 +26,62 @@ import (
"github.com/stretchr/testify/assert"
)
func TestRolePermissionsMiddleware(t *testing.T) {
cfg := newFakeKeycloakConfig()
type fakeRequest struct {
URI string
Method string
Redirects bool
HasToken bool
NotSigned bool
Expires time.Duration
Roles []string
Expects int
}
func makeFakesRequests(t *testing.T, reqs []fakeRequest, cfg *Config) {
cfg.SkipTokenVerification = false
cfg.Resources = []*Resource{
{
URL: fakeAdminRoleURL,
Methods: []string{"ANY"},
Roles: []string{fakeAdminRole},
},
{
URL: fakeTestRoleURL,
Methods: []string{"GET"},
Roles: []string{fakeTestRole},
},
{
URL: fakeTestAdminRolesURL,
Methods: []string{"GET"},
Roles: []string{fakeAdminRole, fakeTestRole},
},
{
URL: fakeTestWhitelistedURL,
WhiteListed: true,
Methods: []string{},
Roles: []string{},
},
{
URL: "/",
Methods: []string{"ANY"},
Roles: []string{fakeTestRole},
},
}
px, idp, svc := newTestProxyService(cfg)
for i, c := range reqs {
px.config.NoRedirects = !c.Redirects
// step: make the client
hc := resty.New().SetRedirectPolicy(resty.NoRedirectPolicy())
if c.HasToken {
token := newTestToken(idp.getLocation())
if len(c.Roles) > 0 {
token.setRealmsRoles(c.Roles)
}
if c.Expires > 0 {
token.setExpiration(time.Now().Add(c.Expires))
}
if !c.NotSigned {
signed, err := idp.signToken(token.claims)
if !assert.NoError(t, err, "case %d, unable to sign the token, error: %s", i, err) {
continue
}
hc.SetAuthToken(signed.Encode())
} else {
jwt := token.getToken()
hc.SetAuthToken(jwt.Encode())
}
}
// step: make the request
resp, err := hc.R().Execute(c.Method, svc+c.URI)
if err != nil {
if !strings.Contains(err.Error(), "Auto redirect is disable") {
assert.NoError(t, err, "case %d, unable to make request, error: %s", i, err)
continue
}
}
// step: check against the expected
assert.Equal(t, c.Expects, resp.StatusCode(), "case %d, uri: %s, expected: %d, got: %d",
i, c.URI, c.Expects, resp.StatusCode())
}
}
// test cases
cs := []struct {
URI string
Method string
Redirects bool
HasToken bool
NotSigned bool
Expires time.Duration
Roles []string
Expects int
}{
func TestOauthRequests(t *testing.T) {
cfg := newFakeKeycloakConfig()
requests := []fakeRequest{
{
URI: "/",
Expects: http.StatusUnauthorized,
},
{ // check whitelisted is passed
URI: fakeTestWhitelistedURL,
Expects: http.StatusOK,
},
{ // check for redirect
URI: "/",
URI: "/oauth/authorize",
Redirects: true,
Expects: http.StatusTemporaryRedirect,
},
......@@ -93,6 +95,20 @@ func TestRolePermissionsMiddleware(t *testing.T) {
Redirects: true,
Expects: http.StatusOK,
},
}
makeFakesRequests(t, requests, cfg)
}
func TestStrangeAdminRequests(t *testing.T) {
cfg := newFakeKeycloakConfig()
cfg.Resources = []*Resource{
{
URL: "/admin",
Methods: []string{"ANY"},
Roles: []string{fakeAdminRole},
},
}
requests := []fakeRequest{
{ // check for escaping
URI: "//admin%2Ftest",
Redirects: true,
......@@ -124,11 +140,6 @@ func TestRolePermissionsMiddleware(t *testing.T) {
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,
......@@ -141,6 +152,94 @@ func TestRolePermissionsMiddleware(t *testing.T) {
Roles: []string{fakeAdminRole},
Expects: http.StatusOK,
},
{
URI: "/help/../admin/test/21",
Redirects: false,
Expects: http.StatusUnauthorized,
},
}
makeFakesRequests(t, requests, cfg)
}
func TestWhiteListedRequests(t *testing.T) {
cfg := newFakeKeycloakConfig()
cfg.Resources = []*Resource{
{
URL: "/whitelist",
WhiteListed: true,
Methods: []string{"GET"},
Roles: []string{},
},
{
URL: "/",
Methods: []string{"ANY"},
Roles: []string{fakeTestRole},
},
{
URL: "/whitelisted",
WhiteListed: true,
Methods: []string{"ANY"},
Roles: []string{fakeTestRole},
},
}
requests := []fakeRequest{
{ // check whitelisted is passed
URI: "/whitelist",
Expects: http.StatusOK,
},
{ // check whitelisted is passed
URI: "/whitelist/test",
Expects: http.StatusOK,
},
{
URI: "/",
Expects: http.StatusUnauthorized,
},
}
makeFakesRequests(t, requests, cfg)
}
func TestRolePermissionsMiddleware(t *testing.T) {
cfg := newFakeKeycloakConfig()
cfg.Resources = []*Resource{
{
URL: "/admin",
Methods: []string{"ANY"},
Roles: []string{fakeAdminRole},
},
{
URL: "/test",
Methods: []string{"GET"},
Roles: []string{fakeTestRole},
},
{
URL: "/test_admin_role",
Methods: []string{"GET"},
Roles: []string{fakeAdminRole, fakeTestRole},
},
{
URL: "/whitelist",
WhiteListed: true,
Methods: []string{"GET"},
Roles: []string{},
},
{
URL: "/",
Methods: []string{"ANY"},
Roles: []string{fakeTestRole},
},
}
// test cases
requests := []fakeRequest{
{
URI: "/",
Expects: http.StatusUnauthorized,
},
{ // check for redirect
URI: "/",
Redirects: true,
Expects: http.StatusTemporaryRedirect,
},
{ // check with a token
URI: "/",
Redirects: false,
......@@ -155,20 +254,27 @@ func TestRolePermissionsMiddleware(t *testing.T) {
Expects: http.StatusForbidden,
},
{ // token, wrong roles
URI: fakeTestRoleURL,
URI: "/test",
Redirects: false,
HasToken: true,
Roles: []string{"bad_role"},
Expects: http.StatusForbidden,
},
{ // token, wrong roles, no 'get' method
URI: fakeTestRoleURL,
URI: "/test",
Method: http.MethodPost,
Redirects: false,
HasToken: true,
Roles: []string{"bad_role"},
Expects: http.StatusOK,
},
{ // check with correct token
URI: "/test",
Redirects: false,
HasToken: true,
Roles: []string{fakeTestRole},
Expects: http.StatusOK,
},
{ // check with correct token
URI: "/",
Redirects: false,
......@@ -185,7 +291,7 @@ func TestRolePermissionsMiddleware(t *testing.T) {
Expects: http.StatusForbidden,
},
{ // check with correct token, signed
URI: fakeAdminRoleURL,
URI: "/admin/page",
Method: http.MethodPost,
Redirects: false,
HasToken: true,
......@@ -193,26 +299,26 @@ func TestRolePermissionsMiddleware(t *testing.T) {
Expects: http.StatusForbidden,
},
{ // check with correct token, signed, wrong roles
URI: fakeAdminRoleURL,
URI: "/admin/page",
Redirects: false,
HasToken: true,
Roles: []string{fakeTestRole},
Expects: http.StatusForbidden,
},
{ // check with correct token, signed, wrong roles
URI: fakeAdminRoleURL,
URI: "/admin/page",
Redirects: false,
HasToken: true,
Roles: []string{fakeTestRole, fakeAdminRole},
Expects: http.StatusOK,
},
{ // strange url
URI: fakeAdminRoleURL + "/.." + fakeAdminRoleURL,
URI: "/admin/..//admin/page",
Redirects: false,
Expects: http.StatusUnauthorized,
},
{ // strange url, token
URI: fakeAdminRoleURL + "/.." + fakeAdminRoleURL,
URI: "/admin/../admin",
Redirects: false,
HasToken: true,
Roles: []string{"hehe"},
......@@ -229,51 +335,24 @@ func TestRolePermissionsMiddleware(t *testing.T) {
Redirects: false,
HasToken: true,
Roles: []string{fakeAdminRole},
Expects: http.StatusForbidden,
Expects: http.StatusOK,
},
{ // strange url, token, wrong roles
URI: "/test/.." + fakeTestAdminRolesURL,
URI: "/test/../admin",
Redirects: false,
HasToken: true,
Roles: []string{fakeAdminRole},
Expects: http.StatusOK,
},
{ // strange url, token, wrong roles
URI: "/test/../admin",
Redirects: false,
HasToken: true,
Roles: []string{fakeTestRole},
Expects: http.StatusForbidden,
},
}
for i, c := range cs {
px.config.NoRedirects = !c.Redirects
// step: make the client
hc := resty.New().SetRedirectPolicy(resty.NoRedirectPolicy())
if c.HasToken {
token := newTestToken(idp.getLocation())
if len(c.Roles) > 0 {
token.setRealmsRoles(c.Roles)
}
if c.Expires > 0 {
token.setExpiration(time.Now().Add(c.Expires))
}
if !c.NotSigned {
signed, err := idp.signToken(token.claims)
if !assert.NoError(t, err, "case %d, unable to sign the token, error: %s", i, err) {
continue
}
hc.SetAuthToken(signed.Encode())
} else {
jwt := token.getToken()
hc.SetAuthToken(jwt.Encode())
}
}
// step: make the request
resp, err := hc.R().Execute(c.Method, svc+c.URI)
if err != nil {
if !strings.Contains(err.Error(), "Auto redirect is disable") {
assert.NoError(t, err, "case %d, unable to make request, error: %s", i, err)
continue
}
}
// step: check against the expected
assert.Equal(t, c.Expects, resp.StatusCode(), "case %d, uri: %s, expected: %d, got: %d",
i, c.URI, c.Expects, resp.StatusCode())
}
makeFakesRequests(t, requests, cfg)
}
func TestCrossSiteHandler(t *testing.T) {
......
......@@ -155,7 +155,7 @@ func (r *oauthProxy) createReverseProxy() error {
engine := gin.New()
engine.Use(gin.Recovery())
// step: custom filtering
engine.Use(r.filterMiddleware())
engine.Use(r.filterMiddleware(), r.reverseProxyMiddleware())
// step: is profiling enabled?
if r.config.EnableProfiling {
......@@ -205,8 +205,10 @@ func (r *oauthProxy) createReverseProxy() error {
}
// step: add the middleware
engine.Use(r.entrypointMiddleware(), r.authenticationMiddleware(), r.admissionMiddleware(),
r.headersMiddleware(r.config.AddClaims), r.reverseProxyMiddleware())
engine.Use(r.entrypointMiddleware(),
r.authenticationMiddleware(),
r.admissionMiddleware(),
r.headersMiddleware(r.config.AddClaims))
// step: set the handler
r.router = engine
......@@ -473,9 +475,7 @@ func (r *oauthProxy) createUpstreamProxy(upstream *url.URL) error {
return nil
}
//
// createTemplates loads the custom template
//
func (r *oauthProxy) createTemplates() error {
var list []string
......
*.sublime-*
.DS_Store
*.swp
*.swo
tags
language: go
go:
- 1.4
- 1.5
- 1.6
- tip
Copyright (c) 2012, Martin Angers
All rights reserved.
Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met:
* Redistributions of source code must retain the above copyright notice, this list of conditions and the following disclaimer.
* Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the following disclaimer in the documentation and/or other materials provided with the distribution.