From 7efc162aaa1a8cbd540fe1d2dab2106ddd8278d4 Mon Sep 17 00:00:00 2001
From: Mitsuo Heijo <mitsuo.heijo@gmail.com>
Date: Thu, 9 Apr 2020 23:39:07 +0900
Subject: [PATCH] Prevent browser caching during auth flow (#453)

* Prevent browser caching during auth flow

* simplify no-cache logic, add tests and update changelog

* checking noCacheHeaders does not exists in response headers from upstream

* remove unnecessary codes

* add no-cache headers in SignInPage and OAuthStart for proxy mode

https://github.com/oauth2-proxy/oauth2-proxy/pull/453#discussion_r405072222
---
 CHANGELOG.md       |  3 +++
 oauthproxy.go      | 21 +++++++++++++++++++++
 oauthproxy_test.go | 43 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 86b894a3..7ed07159 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,8 @@
 
 ## Important Notes
 
+- [#453](https://github.com/oauth2-proxy/oauth2-proxy/pull/453) Responses to endpoints with a proxy prefix will now return headers for preventing browser caching.
+
 ## Breaking Changes
 
 - Migration from Pusher to independent org may have introduced breaking changes for your environment.
@@ -12,6 +14,7 @@
 
 ## Changes since v5.1.0
 
+- [#453](https://github.com/oauth2-proxy/oauth2-proxy/pull/453) Prevent browser caching during auth flow (@johejo)
 - [#481](https://github.com/oauth2-proxy/oauth2-proxy/pull/481) Update Okta docs (@trevorbox)
 - [#474](https://github.com/oauth2-proxy/oauth2-proxy/pull/474) Always log hasMember request error object (@jbielick)
 - [#468](https://github.com/oauth2-proxy/oauth2-proxy/pull/468) Implement graceful shutdown and propagate request context (@johejo)
diff --git a/oauthproxy.go b/oauthproxy.go
index 1b3e3fbe..c5470544 100644
--- a/oauthproxy.go
+++ b/oauthproxy.go
@@ -453,6 +453,7 @@ func (p *OAuthProxy) ErrorPage(rw http.ResponseWriter, code int, title string, m
 
 // SignInPage writes the sing in template to the response
 func (p *OAuthProxy) SignInPage(rw http.ResponseWriter, req *http.Request, code int) {
+	prepareNoCache(rw)
 	p.ClearSessionCookie(rw, req)
 	rw.WriteHeader(code)
 
@@ -633,7 +634,26 @@ func getRemoteAddr(req *http.Request) (s string) {
 	return
 }
 
+// See https://developers.google.com/web/fundamentals/performance/optimizing-content-efficiency/http-caching?hl=en
+var noCacheHeaders = map[string]string{
+	"Expires":         time.Unix(0, 0).Format(time.RFC1123),
+	"Cache-Control":   "no-cache, no-store, must-revalidate, max-age=0",
+	"X-Accel-Expires": "0", // https://www.nginx.com/resources/wiki/start/topics/examples/x-accel/
+}
+
+// prepareNoCache prepares headers for preventing browser caching.
+func prepareNoCache(w http.ResponseWriter) {
+	// Set NoCache headers
+	for k, v := range noCacheHeaders {
+		w.Header().Set(k, v)
+	}
+}
+
 func (p *OAuthProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
+	if strings.HasPrefix(req.URL.Path, p.ProxyPrefix) {
+		prepareNoCache(rw)
+	}
+
 	switch path := req.URL.Path; {
 	case path == p.RobotsPath:
 		p.RobotsTxt(rw)
@@ -715,6 +735,7 @@ func (p *OAuthProxy) SignOut(rw http.ResponseWriter, req *http.Request) {
 
 // OAuthStart starts the OAuth2 authentication flow
 func (p *OAuthProxy) OAuthStart(rw http.ResponseWriter, req *http.Request) {
+	prepareNoCache(rw)
 	nonce, err := encryption.Nonce()
 	if err != nil {
 		logger.Printf("Error obtaining nonce: %s", err.Error())
diff --git a/oauthproxy_test.go b/oauthproxy_test.go
index eebad11e..ec46bdd8 100644
--- a/oauthproxy_test.go
+++ b/oauthproxy_test.go
@@ -1524,3 +1524,46 @@ func TestFindJwtBearerToken(t *testing.T) {
 
 	fmt.Printf("%s", token)
 }
+
+func Test_prepareNoCache(t *testing.T) {
+	handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		prepareNoCache(w)
+	})
+	mux := http.NewServeMux()
+	mux.Handle("/", handler)
+
+	rec := httptest.NewRecorder()
+	req := httptest.NewRequest(http.MethodGet, "/", nil)
+	mux.ServeHTTP(rec, req)
+
+	for k, v := range noCacheHeaders {
+		assert.Equal(t, rec.Header().Get(k), v)
+	}
+}
+
+func Test_noCacheHeadersDoesNotExistsInResponseHeadersFromUpstream(t *testing.T) {
+	upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Write([]byte("upstream"))
+	}))
+	t.Cleanup(upstream.Close)
+
+	opts := NewOptions()
+	opts.Upstreams = []string{upstream.URL}
+	opts.SkipAuthRegex = []string{".*"}
+	_ = opts.Validate()
+	proxy := NewOAuthProxy(opts, func(email string) bool {
+		return true
+	})
+
+	rec := httptest.NewRecorder()
+	req := httptest.NewRequest(http.MethodGet, "/upstream", nil)
+	proxy.ServeHTTP(rec, req)
+
+	assert.Equal(t, http.StatusOK, rec.Code)
+	assert.Equal(t, "upstream", rec.Body.String())
+
+	// checking noCacheHeaders does not exists in response headers from upstream
+	for k := range noCacheHeaders {
+		assert.Equal(t, "", rec.Header().Get(k))
+	}
+}