Skip to content
This repository has been archived by the owner on Dec 7, 2020. It is now read-only.

Commit

Permalink
Session Only Cookie (#357)
Browse files Browse the repository at this point in the history
- adding a command line parameter to override the expiration on the cookie to make them session only
- updated the CHANGELOG to reflect the various changes
  • Loading branch information
gambol99 authored May 12, 2018
1 parent c1e8399 commit ab5ef86
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 20 deletions.
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,20 @@ FEATURES:
* Force configuration to use the wildcard [#PR338](https://github.com/gambol99/keycloak-proxy/pull/338)
* Updated the docker base image alpine 3.7 [#PR313](https://github.com/gambol99/keycloak-proxy/pull/313)
* Updated to Golang version 1.10 [#PR316](https://github.com/gambol99/keycloak-proxy/pull/316)
* Adding the `--enable-session-cookies` to enable session only cookies [#PR357](https://github.com/gambol99/keycloak-proxy/pull/357)
* Imported the fix to the cache headers from upstream go-oidc [#PR341](https://github.com/gambol99/keycloak-proxy/pull/341)
* Switching to using SHA256 from MD5 for the token hash [#PR350](https://github.com/gambol99/keycloak-proxy/pull/350)

FIXES:
* Fixed a redirection bug [#PR337](https://github.com/gambol99/keycloak-proxy/pull/337)
* Updated the go-oidc to fix the cache header [issues](https://github.com/gambol99/keycloak-proxy/issues/340)[#PR339](https://github.com/gambol99/keycloak-proxy/pull/339)
* Fixed up the readme indicating we can run without client secret [#PR342](https://github.com/gambol99/keycloak-proxy/pull/342)
* Fixed up the redirect url in the logout handler [#PR345](https://github.com/gambol99/keycloak-proxy/pull/345)
* Switched to using the upstream stream goproxy [#PR349](https://github.com/gambol99/keycloak-proxy/pull/349)
* Removing the unused code [#PR352](https://github.com/gambol99/keycloak-proxy/pull/352)
* Reducing the aggressive timeouts on the upstream [#PR354](https://github.com/gambol99/keycloak-proxy/pull/354)
* Fixed the issue with a zero exp claim [#PR355](https://github.com/gambol99/keycloak-proxy/pull/355)
* Added a method check for the hijacker [#PR302](https://github.com/gambol99/keycloak-proxy/pull/302)

#### **2.1.1**

Expand Down
13 changes: 8 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ USAGE:
keycloak-proxy [options]

VERSION:
v2.1.1 (git+sha: 35e834a, built: 02-03-2018)
v2.1.1 (git+sha: c1e8399-dirty, built: 12-05-2018)

AUTHOR:
Rohith <gambol99@gmail.com>
Expand All @@ -54,18 +54,21 @@ GLOBAL OPTIONS:
--skip-openid-provider-tls-verify skip the verification of any TLS communication with the openid provider (default: false)
--openid-provider-proxy value proxy for communication with the openid provider
--openid-provider-timeout value timeout for openid configuration on .well-known/openid-configuration (default: 30s)
--oauth-uri value the uri for proxy oauth endpoints (default: "/oauth") [$PROXY_OAUTH_URI]
--scopes value list of scopes requested when authenticating the user
--upstream-url value url for the upstream endpoint you wish to proxy [$PROXY_UPSTREAM_URL]
--upstream-ca value the path to a file container a CA certificate to validate the upstream tls endpoint
--resources value list of resources 'uri=/admin*|methods=GET,PUT|roles=role1,role2'
--headers value custom headers to the upstream request, key=value
--enable-logout-redirect indicates we should redirect to the identity provider for logging out (default: false)
--enable-default-deny enables a default denial on all requests, you have to explicitly say what is permitted (recommended) (default: false)
--enable-encrypted-token enable encryption for the access tokens (default: false)
--enable-logging enable http logging of the requests (default: false)
--enable-json-logging switch on json logging rather than text (default: false)
--enable-forwarding enables the forwarding proxy mode, signing outbound request (default: false)
--enable-security-filter enables the security filter handler (default: false) [$PROXY_ENABLE_SECURITY_FILTER]
--enable-refresh-tokens enables the handling of the refresh tokens (default: false) [$PROXY_ENABLE_REFRESH_TOKEN]
--enable-session-cookies access and refresh tokens are session only i.e. removed browser close (default: false)
--enable-login-handler enables the handling of the refresh tokens (default: false) [$PROXY_ENABLE_LOGIN_HANDLER]
--enable-token-header enables the token authentication header X-Auth-Token to upstream (default: true)
--enable-authorization-header adds the authorization header to the proxy request (default: true) [$PROXY_ENABLE_AUTHORIZATION_HEADER]
Expand Down Expand Up @@ -108,20 +111,20 @@ GLOBAL OPTIONS:
--upstream-timeout value maximum amount of time a dial will wait for a connect to complete (default: 10s)
--upstream-keepalive-timeout value specifies the keep-alive period for an active network connection (default: 10s)
--upstream-tls-handshake-timeout value the timeout placed on the tls handshake for upstream (default: 10s)
--upstream-response-header-timeout value the timeout placed on the response header for upstream (default: 1s)
--upstream-response-header-timeout value the timeout placed on the response header for upstream (default: 10s)
--upstream-expect-continue-timeout value the timeout placed on the expect continue for upstream (default: 10s)
--verbose switch on debug / verbose logging (default: false)
--enabled-proxy-protocol enable proxy protocol (default: false)
--server-read-timeout value the server read timeout on the http server (default: 5s)
--server-read-timeout value the server read timeout on the http server (default: 10s)
--server-write-timeout value the server write timeout on the http server (default: 10s)
--server-idle-timeout value the server idle timeout on the http server (default: 2m0s)
--use-letsencrypt use letsencrypt for certificates (default: false)
--letsencrypt-cache-dir value path where cached letsencrypt certificates are stored (default: "./cache/")
--sign-in-page value path to custom template displayed for signin
--forbidden-page value path to custom template used for access forbidden
--tags value keypairs passed to the templates at render,e.g title=Page
--forwarding-username value username to use when logging into the openid provider
--forwarding-password value password to use when logging into the openid provider
--forwarding-username value username to use when logging into the openid provider [$PROXY_FORWARDING_USERNAME]
--forwarding-password value password to use when logging into the openid provider [$PROXY_FORWARDING_PASSWORD]
--forwarding-domains value list of domains which should be signed; everything else is relayed unsigned
--disable-all-logging disables all logging to stdout and stderr (default: false)
--help, -h show help
Expand Down
28 changes: 14 additions & 14 deletions cookies.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,14 @@ func (r *oauthProxy) dropCookie(w http.ResponseWriter, host, name, value string,
domain = r.config.CookieDomain
}
cookie := &http.Cookie{
Name: name,
Domain: domain,
HttpOnly: r.config.HTTPOnlyCookie,
Name: name,
Path: "/",
Secure: r.config.SecureCookie,
Value: value,
}
if duration != 0 {
if !r.config.EnableSessionCookies && duration != 0 {
cookie.Expires = time.Now().Add(duration)
}

Expand All @@ -47,39 +47,39 @@ func (r *oauthProxy) dropCookie(w http.ResponseWriter, host, name, value string,
// dropAccessTokenCookie drops a access token cookie into the response
func (r *oauthProxy) dropAccessTokenCookie(req *http.Request, w http.ResponseWriter, value string, duration time.Duration) {
// also cookie name is included in the cookie length; cookie name suffix "-xxx"
maxCookieLenght := 4089 - len(r.config.CookieAccessName)
maxCookieLength := 4089 - len(r.config.CookieAccessName)

if len(value) <= maxCookieLenght {
if len(value) <= maxCookieLength {
r.dropCookie(w, req.Host, r.config.CookieAccessName, value, duration)
} else {
// write divided cookies because payload is too long for single cookie
r.dropCookie(w, req.Host, r.config.CookieAccessName, value[0:maxCookieLenght], duration)
for i := maxCookieLenght; i < len(value); i += maxCookieLenght {
end := i + maxCookieLenght
r.dropCookie(w, req.Host, r.config.CookieAccessName, value[0:maxCookieLength], duration)
for i := maxCookieLength; i < len(value); i += maxCookieLength {
end := i + maxCookieLength
if end > len(value) {
end = len(value)
}
r.dropCookie(w, req.Host, r.config.CookieAccessName+"-"+strconv.Itoa(i/maxCookieLenght), value[i:end], duration)
r.dropCookie(w, req.Host, r.config.CookieAccessName+"-"+strconv.Itoa(i/maxCookieLength), value[i:end], duration)
}
}
}

// dropRefreshTokenCookie drops a refresh token cookie into the response
func (r *oauthProxy) dropRefreshTokenCookie(req *http.Request, w http.ResponseWriter, value string, duration time.Duration) {
// also cookie name is included in the cookie length; cookie name suffix "-xxx"
maxCookieLenght := 4089 - len(r.config.CookieRefreshName)
maxCookieLength := 4089 - len(r.config.CookieRefreshName)

if len(value) <= maxCookieLenght {
if len(value) <= maxCookieLength {
r.dropCookie(w, req.Host, r.config.CookieRefreshName, value, duration)
} else {
// write divided cookies because payload is too long for single cookie
r.dropCookie(w, req.Host, r.config.CookieRefreshName, value[0:maxCookieLenght], duration)
for i := maxCookieLenght; i < len(value); i += maxCookieLenght {
end := i + maxCookieLenght
r.dropCookie(w, req.Host, r.config.CookieRefreshName, value[0:maxCookieLength], duration)
for i := maxCookieLength; i < len(value); i += maxCookieLength {
end := i + maxCookieLength
if end > len(value) {
end = len(value)
}
r.dropCookie(w, req.Host, r.config.CookieRefreshName+"-"+strconv.Itoa(i/maxCookieLenght), value[i:end], duration)
r.dropCookie(w, req.Host, r.config.CookieRefreshName+"-"+strconv.Itoa(i/maxCookieLength), value[i:end], duration)
}
}
}
Expand Down
14 changes: 14 additions & 0 deletions cookies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"net/http"
"net/http/httptest"
"testing"
"time"

"github.com/stretchr/testify/assert"
)
Expand Down Expand Up @@ -104,6 +105,19 @@ func TestDropRefreshCookie(t *testing.T) {
"we have not set the cookie, headers: %v", resp.Header())
}

func TestSessionOnlyCookie(t *testing.T) {
p, _, _ := newTestProxyService(nil)
p.config.EnableSessionCookies = true

req := newFakeHTTPRequest("GET", "/admin")
resp := httptest.NewRecorder()
p.dropCookie(resp, req.Host, "test-cookie", "test-value", 1*time.Hour)

assert.Equal(t, resp.Header().Get("Set-Cookie"),
"test-cookie=test-value; Path=/; Domain=127.0.0.1",
"we have not set the cookie, headers: %v", resp.Header())
}

func TestHTTPOnlyCookie(t *testing.T) {
p, _, _ := newTestProxyService(nil)

Expand Down
2 changes: 2 additions & 0 deletions doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,8 @@ type Config struct {
EnableSecurityFilter bool `json:"enable-security-filter" yaml:"enable-security-filter" usage:"enables the security filter handler" env:"ENABLE_SECURITY_FILTER"`
// EnableRefreshTokens indicate's you wish to ignore using refresh tokens and re-auth on expiration of access token
EnableRefreshTokens bool `json:"enable-refresh-tokens" yaml:"enable-refresh-tokens" usage:"enables the handling of the refresh tokens" env:"ENABLE_REFRESH_TOKEN"`
// EnableSessionCookies indicates the cookies, both token and refresh should not be persisted
EnableSessionCookies bool `json:"enable-session-cookies" yaml:"enable-session-cookies" usage:"access and refresh tokens are session only i.e. removed browser close"`
// EnableLoginHandler indicates we want the login handler enabled
EnableLoginHandler bool `json:"enable-login-handler" yaml:"enable-login-handler" usage:"enables the handling of the refresh tokens" env:"ENABLE_LOGIN_HANDLER"`
// EnableTokenHeader adds the JWT token to the upstream authentication headers
Expand Down
2 changes: 1 addition & 1 deletion handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ func (r *oauthProxy) oauthCallbackHandler(w http.ResponseWriter, req *http.Reque
// notes: not all idp refresh tokens are readable, google for example, so we attempt to decode into
// a jwt and if possible extract the expiration, else we default to 10 days
if _, ident, err := parseToken(resp.RefreshToken); err != nil {
r.dropRefreshTokenCookie(req, w, encrypted, time.Duration(240)*time.Hour)
r.dropRefreshTokenCookie(req, w, encrypted, 0)
} else {
r.dropRefreshTokenCookie(req, w, encrypted, time.Until(ident.ExpiresAt))
}
Expand Down
4 changes: 4 additions & 0 deletions server.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,10 @@ func (r *oauthProxy) createReverseProxy() error {
})
}

if r.config.EnableSessionCookies {
r.log.Info("using session cookies only for access and refresh tokens")
}

// step: load the templates if any
if err := r.createTemplates(); err != nil {
return err
Expand Down

0 comments on commit ab5ef86

Please sign in to comment.