From 72ce41af6200cf6ca5509080c62547367edd5216 Mon Sep 17 00:00:00 2001 From: Rohith Jayawardene Date: Sun, 5 Feb 2017 22:54:40 +0000 Subject: [PATCH] Access Token Duration (#188) * Access Token Duration - fixed the access token expiring too quickly; the cookie expiration needed to be extended. Not sure when this was broken, but I changed the duration a while back. Needs a test added. * - switching to gambol99/goproxy until a fix upstream --- CHANGELOG.md | 2 + Godeps/Godeps.json | 4 +- config.go | 1 + doc.go | 2 + forwarding.go | 11 +-- handlers.go | 45 +++++------- middleware.go | 69 ++++++++----------- misc.go | 15 ++++ server.go | 3 +- .../{elazarl => gambol99}/goproxy/.gitignore | 0 .../{elazarl => gambol99}/goproxy/LICENSE | 0 .../{elazarl => gambol99}/goproxy/README.md | 0 .../{elazarl => gambol99}/goproxy/actions.go | 0 .../{elazarl => gambol99}/goproxy/all.bash | 0 .../{elazarl => gambol99}/goproxy/ca.pem | 0 .../{elazarl => gambol99}/goproxy/certs.go | 0 .../{elazarl => gambol99}/goproxy/chunked.go | 0 .../goproxy/counterecryptor.go | 0 .../{elazarl => gambol99}/goproxy/ctx.go | 0 .../goproxy/dispatcher.go | 0 .../{elazarl => gambol99}/goproxy/doc.go | 0 .../{elazarl => gambol99}/goproxy/https.go | 0 .../{elazarl => gambol99}/goproxy/key.pem | 0 .../{elazarl => gambol99}/goproxy/proxy.go | 3 - .../goproxy/responses.go | 0 .../{elazarl => gambol99}/goproxy/signer.go | 0 26 files changed, 73 insertions(+), 82 deletions(-) rename vendor/github.com/{elazarl => gambol99}/goproxy/.gitignore (100%) rename vendor/github.com/{elazarl => gambol99}/goproxy/LICENSE (100%) rename vendor/github.com/{elazarl => gambol99}/goproxy/README.md (100%) rename vendor/github.com/{elazarl => gambol99}/goproxy/actions.go (100%) rename vendor/github.com/{elazarl => gambol99}/goproxy/all.bash (100%) rename vendor/github.com/{elazarl => gambol99}/goproxy/ca.pem (100%) rename vendor/github.com/{elazarl => gambol99}/goproxy/certs.go (100%) rename vendor/github.com/{elazarl => gambol99}/goproxy/chunked.go (100%) rename vendor/github.com/{elazarl => gambol99}/goproxy/counterecryptor.go (100%) rename vendor/github.com/{elazarl => gambol99}/goproxy/ctx.go (100%) rename vendor/github.com/{elazarl => gambol99}/goproxy/dispatcher.go (100%) rename vendor/github.com/{elazarl => gambol99}/goproxy/doc.go (100%) rename vendor/github.com/{elazarl => gambol99}/goproxy/https.go (100%) rename vendor/github.com/{elazarl => gambol99}/goproxy/key.pem (100%) rename vendor/github.com/{elazarl => gambol99}/goproxy/proxy.go (99%) rename vendor/github.com/{elazarl => gambol99}/goproxy/responses.go (100%) rename vendor/github.com/{elazarl => gambol99}/goproxy/signer.go (100%) diff --git a/CHANGELOG.md b/CHANGELOG.md index b9b2270f..96b3fc81 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,10 +12,12 @@ CHANGES: * Fixed up some spelling mistakes [#PR177](https://github.com/gambol99/keycloak-proxy/pull/177) * Changed the CLI to use reflection of the config struct [#PR176](https://github.com/gambol99/keycloak-proxy/pull/176) * Updated the docker base image to alpine:3.5 [#PR184](https://github.com/gambol99/keycloak-proxy/pull/184) + * Added a new options to control the access token duration [#PR188](https://github.com/gambol99/keycloak-proxy/pull/188) BUGS: * Fixed the time.Duration flags in the reflection code [#PR173](https://github.com/gambol99/keycloak-proxy/pull/173) * Fixed the environment variable type [#PR176](https://github.com/gambol99/keycloak-proxy/pull/176) + * Fixed the refresh tokens, the access token cookie was timing out too quickly ([#PR188](https://github.com/gambol99/keycloak-proxy/pull/188) #### **2.0.1** diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index 50f85430..e5f883bf 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -71,9 +71,9 @@ "Rev": "5215b55f46b2b919f50a1df0eaa5886afe4e3b3d" }, { - "ImportPath": "github.com/elazarl/goproxy", + "ImportPath": "github.com/gambol99/goproxy", "Comment": "v1.0-96-g6016d23", - "Rev": "6016d2397298b0750eeb06f4d24e03cfd7e45c77" + "Rev": "16403edea88bc5b716703dca58a5e94ad8b11879" }, { "ImportPath": "github.com/fsnotify/fsnotify", diff --git a/config.go b/config.go index 1a50c693..a765cf31 100644 --- a/config.go +++ b/config.go @@ -27,6 +27,7 @@ import ( // newDefaultConfig returns a initialized config func newDefaultConfig() *Config { return &Config{ + AccessTokenDuration: time.Duration(720) * time.Hour, Tags: make(map[string]string, 0), MatchClaims: make(map[string]string, 0), Headers: make(map[string]string, 0), diff --git a/doc.go b/doc.go index a9712de7..7b31f84b 100644 --- a/doc.go +++ b/doc.go @@ -160,6 +160,8 @@ type Config struct { // LocalhostMetrics indicated the metrics can only be consume via localhost LocalhostMetrics bool `json:"localhost-metrics" yaml:"localhost-metrics" usage:"enforces the metrics page can only been requested from 127.0.0.1"` + // AccessTokenDuration is default duration applied to the access token cookie + AccessTokenDuration time.Duration `json:"access-token-duration" yaml:"access-token-duration" usage:"fallback cookie duration for the access token when using refresh tokens"` // CookieDomain is a list of domains the cookie is available to CookieDomain string `json:"cookie-domain" yaml:"cookie-domain" usage:"domain the access cookie is available to, defaults host header"` // CookieAccessName is the name of the access cookie holding the access token diff --git a/forwarding.go b/forwarding.go index 73defadf..df0ff265 100644 --- a/forwarding.go +++ b/forwarding.go @@ -26,9 +26,7 @@ import ( "github.com/gin-gonic/gin" ) -// // reverseProxyMiddleware is responsible for handles reverse proxy request to the upstream endpoint -// func (r *oauthProxy) reverseProxyMiddleware() gin.HandlerFunc { return func(cx *gin.Context) { if cx.IsAborted() { @@ -46,10 +44,9 @@ func (r *oauthProxy) reverseProxyMiddleware() gin.HandlerFunc { cx.Abort() return } - /* - By default goproxy only provides a forwarding proxy, thus all requests have to be absolute - and we must update the host headers - */ + + // By default goproxy only provides a forwarding proxy, thus all requests have to be absolute + // and we must update the host headers cx.Request.URL.Host = r.endpoint.Host cx.Request.URL.Scheme = r.endpoint.Scheme cx.Request.Host = r.endpoint.Host @@ -58,9 +55,7 @@ func (r *oauthProxy) reverseProxyMiddleware() gin.HandlerFunc { } } -// // forwardProxyHandler is responsible for signing outbound requests -// func (r *oauthProxy) forwardProxyHandler() func(*http.Request, *http.Response) { // step: create oauth client client, err := r.client.OAuthClient() diff --git a/handlers.go b/handlers.go index b28e5839..c14f551b 100644 --- a/handlers.go +++ b/handlers.go @@ -125,7 +125,7 @@ func (r *oauthProxy) oauthCallbackHandler(cx *gin.Context) { } // step: exchange the authorization for a access token - response, err := exchangeAuthenticationCode(client, code) + resp, err := exchangeAuthenticationCode(client, code) if err != nil { log.WithFields(log.Fields{"error": err.Error()}).Errorf("unable to exchange code for access token") @@ -134,7 +134,7 @@ func (r *oauthProxy) oauthCallbackHandler(cx *gin.Context) { } // step: parse decode the identity token - session, identity, err := parseToken(response.IDToken) + token, identity, err := parseToken(resp.IDToken) if err != nil { log.WithFields(log.Fields{"error": err.Error()}).Errorf("unable to parse id token for identity") @@ -143,7 +143,7 @@ func (r *oauthProxy) oauthCallbackHandler(cx *gin.Context) { } // step: verify the token is valid - if err = verifyToken(r.client, session); err != nil { + if err = verifyToken(r.client, token); err != nil { log.WithFields(log.Fields{"error": err.Error()}).Errorf("unable to verify the id token") r.accessForbidden(cx) @@ -151,11 +151,11 @@ func (r *oauthProxy) oauthCallbackHandler(cx *gin.Context) { } // step: attempt to decode the access token else we default to the id token - accessToken, id, err := parseToken(response.AccessToken) + access, id, err := parseToken(resp.AccessToken) if err != nil { log.WithFields(log.Fields{"error": err.Error()}).Errorf("unable to parse the access token, using id token only") } else { - session = accessToken + token = access identity = id } @@ -163,16 +163,12 @@ func (r *oauthProxy) oauthCallbackHandler(cx *gin.Context) { "email": identity.Email, "expires": identity.ExpiresAt.Format(time.RFC3339), "duration": identity.ExpiresAt.Sub(time.Now()).String(), - }).Infof("issuing a new access token for user, email: %s", identity.Email) - - // step: drop's a session cookie with the access token - duration := identity.ExpiresAt.Sub(time.Now()) - r.dropAccessTokenCookie(cx, session.Encode(), duration) + }).Infof("issuing access token for user, email: %s", identity.Email) // step: does the response has a refresh token and we are NOT ignore refresh tokens? - if r.config.EnableRefreshTokens && response.RefreshToken != "" { + if r.config.EnableRefreshTokens && resp.RefreshToken != "" { // step: encrypt the refresh token - encrypted, err := encodeText(response.RefreshToken, r.config.EncryptionKey) + encrypted, err := encodeText(resp.RefreshToken, r.config.EncryptionKey) if err != nil { log.WithFields(log.Fields{"error": err.Error()}).Errorf("failed to encrypt the refresh token") @@ -180,27 +176,25 @@ func (r *oauthProxy) oauthCallbackHandler(cx *gin.Context) { return } - // step: create and inject the state session + // drop in the access token - cookie expiration = access token + r.dropAccessTokenCookie(cx, token.Encode(), r.getAccessCookieExpiration(token, resp.RefreshToken)) + switch r.useStore() { case true: - if err := r.StoreRefreshToken(session, encrypted); err != nil { + if err := r.StoreRefreshToken(token, encrypted); err != nil { log.WithFields(log.Fields{"error": err.Error()}).Warnf("failed to save the refresh token in the store") } - // step: get expiration of the refresh token if we can - _, ident, err := parseToken(response.RefreshToken) - if err != nil { - r.dropAccessTokenCookie(cx, session.Encode(), time.Duration(72)*time.Hour) - } else { - r.dropAccessTokenCookie(cx, session.Encode(), ident.ExpiresAt.Sub(time.Now())) - } default: - // step: attempt to decode the refresh token (not all refresh tokens are jwt tokens; google for instance. - if _, ident, err := parseToken(response.RefreshToken); err != nil { - r.dropRefreshTokenCookie(cx, encrypted, time.Duration(72)*time.Hour) + // 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(cx, encrypted, time.Duration(240)*time.Hour) } else { r.dropRefreshTokenCookie(cx, encrypted, ident.ExpiresAt.Sub(time.Now())) } } + } else { + r.dropAccessTokenCookie(cx, token.Encode(), identity.ExpiresAt.Sub(time.Now())) } // step: decode the state variable @@ -231,7 +225,6 @@ func (r *oauthProxy) loginHandler(cx *gin.Context) { // step: parse the client credentials username := cx.Request.PostFormValue("username") password := cx.Request.PostFormValue("password") - if username == "" || password == "" { return "request does not have both username and password", http.StatusBadRequest, errors.New("no credentials") } @@ -285,7 +278,7 @@ func (r *oauthProxy) loginHandler(cx *gin.Context) { // - optionally, the user can be redirected by to a url // func (r *oauthProxy) logoutHandler(cx *gin.Context) { - // the user can specify a url to redirect the back to + // the user can specify a url to redirect the back redirectURL := cx.Request.URL.Query().Get("redirect") // step: drop the access token diff --git a/middleware.go b/middleware.go index ffb30f57..13e4cc22 100644 --- a/middleware.go +++ b/middleware.go @@ -77,7 +77,7 @@ func (r *oauthProxy) metricsMiddleware() gin.HandlerFunc { // entrypointMiddleware checks to see if the request requires authentication func (r *oauthProxy) entrypointMiddleware() gin.HandlerFunc { return func(cx *gin.Context) { - // step: we can skip the + // step: we can skip if under oauth prefix if strings.HasPrefix(cx.Request.URL.Path, oauthURL) { return } @@ -128,7 +128,7 @@ func (r *oauthProxy) authenticationMiddleware() gin.HandlerFunc { // step: inject the user into the context cx.Set(userContextName, user) - // step: verify the access token + // step: skipif we are running skip-token-verification if r.config.SkipTokenVerification { log.Warnf("skip token verification enabled, skipping verification process - FOR TESTING ONLY") @@ -145,7 +145,6 @@ func (r *oauthProxy) authenticationMiddleware() gin.HandlerFunc { return } - // step: verify the access token if err := verifyToken(r.client, user.token); err != nil { // step: if the error post verification is anything other than a token expired error // we immediately throw an access forbidden - as there is something messed up in the token @@ -174,78 +173,68 @@ func (r *oauthProxy) authenticationMiddleware() gin.HandlerFunc { log.WithFields(log.Fields{ "email": user.email, "client_ip": clientIP, - }).Infof("accces token for user: %s has expired, attemping to refresh the token", user.email) + }).Infof("accces token for user has expired, attemping to refresh the token") // step: check if the user has refresh token - rToken, err := r.retrieveRefreshToken(cx.Request, user) + refresh, err := r.retrieveRefreshToken(cx.Request, user) if err != nil { log.WithFields(log.Fields{ "email": user.email, "error": err.Error(), "client_ip": clientIP, - }).Errorf("unable to find a refresh token for the client: %s", user.email) + }).Errorf("unable to find a refresh token for user") r.redirectToAuthorization(cx) return } - log.WithFields(log.Fields{ - "email": user.email, - "client_ip": clientIP, - }).Info("attempting to refresh access token for user") - - token, expires, err := getRefreshedToken(r.client, rToken) + // attempt to refresh the access token + token, _, err := getRefreshedToken(r.client, refresh) if err != nil { - // step: has the refresh token expired? switch err { case ErrRefreshTokenExpired: log.WithFields(log.Fields{ "email": user.email, "client_ip": clientIP, - }).Warningf("refresh token has expired for user") + }).Warningf("refresh token has expired, cannot retrieve access token") r.clearAllCookies(cx) default: - log.WithFields(log.Fields{ - "error": err.Error(), - }).Errorf("failed to refresh the access token") + log.WithFields(log.Fields{"error": err.Error()}).Errorf("failed to refresh the access token") } r.redirectToAuthorization(cx) return } - // step: inject the refreshed access token - accessDuration := expires.Sub(time.Now()) + // get the expiration of the new access token + expiresIn := r.getAccessCookieExpiration(token, refresh) log.WithFields(log.Fields{ - "email": user.email, - "expires_in": accessDuration.String(), - "client_ip": clientIP, - }).Infof("injecting refreshed access token, expires on: %s", expires.Format(time.RFC3339)) + "client_ip": clientIP, + "cookie_name": r.config.CookieAccessName, + "email": user.email, + "expires_in": expiresIn.String(), + }).Infof("injecting the refreshed access token cookie") - // step: drop's a session cookie with the access token - duration := expires.Sub(time.Now()) - if r.useStore() { - duration = duration * 10 - } - - r.dropAccessTokenCookie(cx, token.Encode(), duration) + // step: inject the refreshed access token + r.dropAccessTokenCookie(cx, token.Encode(), expiresIn) if r.useStore() { - go func(t jose.JWT, rt string) { - // step: store the new refresh token - if err := r.StoreRefreshToken(t, rt); err != nil { - log.WithFields(log.Fields{ - "error": err.Error(), - }).Errorf("failed to store refresh token") - + go func(old, new jose.JWT, state string) { + if err := r.DeleteRefreshToken(old); err != nil { + log.WithFields(log.Fields{"error": err.Error()}).Errorf("failed to remove old token") + } + if err := r.StoreRefreshToken(new, state); err != nil { + log.WithFields(log.Fields{"error": err.Error()}).Errorf("failed to store refresh token") return } - }(user.token, rToken) + }(user.token, token, refresh) } + // step: update the with the new access token user.token = token + // step: inject the user into the context cx.Set(userContextName, user) } @@ -438,9 +427,7 @@ func (r *oauthProxy) securityMiddleware() gin.HandlerFunc { return func(cx *gin.Context) { if err := secure.Process(cx.Writer, cx.Request); err != nil { - log.WithFields(log.Fields{ - "error": err.Error(), - }).Errorf("failed security middleware") + log.WithFields(log.Fields{"error": err.Error()}).Errorf("failed security middleware") cx.Abort() } diff --git a/misc.go b/misc.go index 51a91801..06ce7ba1 100644 --- a/misc.go +++ b/misc.go @@ -20,8 +20,10 @@ import ( "fmt" "net/http" "path" + "time" log "github.com/Sirupsen/logrus" + "github.com/coreos/go-oidc/jose" "github.com/gin-gonic/gin" ) @@ -62,3 +64,16 @@ func (r *oauthProxy) redirectToAuthorization(cx *gin.Context) { r.redirectToURL(oauthURL+authorizationURL+authQuery, cx) } + +// getAccessCookieExpiration calucates the expiration of the access token cookie +func (r *oauthProxy) getAccessCookieExpiration(token jose.JWT, refresh string) time.Duration { + // notes: by default the duration of the access token will be the configuration option, if + // however we can decode the refresh token, we will set the duration to the duraction of the + // refresh token + duration := r.config.AccessTokenDuration + if _, ident, err := parseToken(refresh); err == nil { + duration = ident.ExpiresAt.Sub(time.Now()) + } + + return duration +} diff --git a/server.go b/server.go index 73f325ba..87f10280 100644 --- a/server.go +++ b/server.go @@ -33,7 +33,7 @@ import ( log "github.com/Sirupsen/logrus" "github.com/armon/go-proxyproto" "github.com/coreos/go-oidc/oidc" - "github.com/elazarl/goproxy" + "github.com/gambol99/goproxy" "github.com/gin-gonic/gin" "github.com/prometheus/client_golang/prometheus" ) @@ -238,7 +238,6 @@ func (r *oauthProxy) createForwardingProxy() error { // step: setup the tls configuration if r.config.TLSCaCertificate != "" && r.config.TLSCaPrivateKey != "" { - // step: read in the ca ca, err := loadCA(r.config.TLSCaCertificate, r.config.TLSCaPrivateKey) if err != nil { return fmt.Errorf("unable to load certificate authority, error: %s", err) diff --git a/vendor/github.com/elazarl/goproxy/.gitignore b/vendor/github.com/gambol99/goproxy/.gitignore similarity index 100% rename from vendor/github.com/elazarl/goproxy/.gitignore rename to vendor/github.com/gambol99/goproxy/.gitignore diff --git a/vendor/github.com/elazarl/goproxy/LICENSE b/vendor/github.com/gambol99/goproxy/LICENSE similarity index 100% rename from vendor/github.com/elazarl/goproxy/LICENSE rename to vendor/github.com/gambol99/goproxy/LICENSE diff --git a/vendor/github.com/elazarl/goproxy/README.md b/vendor/github.com/gambol99/goproxy/README.md similarity index 100% rename from vendor/github.com/elazarl/goproxy/README.md rename to vendor/github.com/gambol99/goproxy/README.md diff --git a/vendor/github.com/elazarl/goproxy/actions.go b/vendor/github.com/gambol99/goproxy/actions.go similarity index 100% rename from vendor/github.com/elazarl/goproxy/actions.go rename to vendor/github.com/gambol99/goproxy/actions.go diff --git a/vendor/github.com/elazarl/goproxy/all.bash b/vendor/github.com/gambol99/goproxy/all.bash similarity index 100% rename from vendor/github.com/elazarl/goproxy/all.bash rename to vendor/github.com/gambol99/goproxy/all.bash diff --git a/vendor/github.com/elazarl/goproxy/ca.pem b/vendor/github.com/gambol99/goproxy/ca.pem similarity index 100% rename from vendor/github.com/elazarl/goproxy/ca.pem rename to vendor/github.com/gambol99/goproxy/ca.pem diff --git a/vendor/github.com/elazarl/goproxy/certs.go b/vendor/github.com/gambol99/goproxy/certs.go similarity index 100% rename from vendor/github.com/elazarl/goproxy/certs.go rename to vendor/github.com/gambol99/goproxy/certs.go diff --git a/vendor/github.com/elazarl/goproxy/chunked.go b/vendor/github.com/gambol99/goproxy/chunked.go similarity index 100% rename from vendor/github.com/elazarl/goproxy/chunked.go rename to vendor/github.com/gambol99/goproxy/chunked.go diff --git a/vendor/github.com/elazarl/goproxy/counterecryptor.go b/vendor/github.com/gambol99/goproxy/counterecryptor.go similarity index 100% rename from vendor/github.com/elazarl/goproxy/counterecryptor.go rename to vendor/github.com/gambol99/goproxy/counterecryptor.go diff --git a/vendor/github.com/elazarl/goproxy/ctx.go b/vendor/github.com/gambol99/goproxy/ctx.go similarity index 100% rename from vendor/github.com/elazarl/goproxy/ctx.go rename to vendor/github.com/gambol99/goproxy/ctx.go diff --git a/vendor/github.com/elazarl/goproxy/dispatcher.go b/vendor/github.com/gambol99/goproxy/dispatcher.go similarity index 100% rename from vendor/github.com/elazarl/goproxy/dispatcher.go rename to vendor/github.com/gambol99/goproxy/dispatcher.go diff --git a/vendor/github.com/elazarl/goproxy/doc.go b/vendor/github.com/gambol99/goproxy/doc.go similarity index 100% rename from vendor/github.com/elazarl/goproxy/doc.go rename to vendor/github.com/gambol99/goproxy/doc.go diff --git a/vendor/github.com/elazarl/goproxy/https.go b/vendor/github.com/gambol99/goproxy/https.go similarity index 100% rename from vendor/github.com/elazarl/goproxy/https.go rename to vendor/github.com/gambol99/goproxy/https.go diff --git a/vendor/github.com/elazarl/goproxy/key.pem b/vendor/github.com/gambol99/goproxy/key.pem similarity index 100% rename from vendor/github.com/elazarl/goproxy/key.pem rename to vendor/github.com/gambol99/goproxy/key.pem diff --git a/vendor/github.com/elazarl/goproxy/proxy.go b/vendor/github.com/gambol99/goproxy/proxy.go similarity index 99% rename from vendor/github.com/elazarl/goproxy/proxy.go rename to vendor/github.com/gambol99/goproxy/proxy.go index fefb3bb0..5c69597a 100644 --- a/vendor/github.com/elazarl/goproxy/proxy.go +++ b/vendor/github.com/gambol99/goproxy/proxy.go @@ -32,9 +32,6 @@ type ProxyHttpServer struct { var hasPort = regexp.MustCompile(`:\d+$`) func copyHeaders(dst, src http.Header) { - for k, _ := range dst { - dst.Del(k) - } for k, vs := range src { for _, v := range vs { dst.Add(k, v) diff --git a/vendor/github.com/elazarl/goproxy/responses.go b/vendor/github.com/gambol99/goproxy/responses.go similarity index 100% rename from vendor/github.com/elazarl/goproxy/responses.go rename to vendor/github.com/gambol99/goproxy/responses.go diff --git a/vendor/github.com/elazarl/goproxy/signer.go b/vendor/github.com/gambol99/goproxy/signer.go similarity index 100% rename from vendor/github.com/elazarl/goproxy/signer.go rename to vendor/github.com/gambol99/goproxy/signer.go