From 45207a5697deac544dc0e5822ad82b49c5a92d2f Mon Sep 17 00:00:00 2001
From: Frederic BIDON <frederic@oneconcern.com>
Date: Thu, 21 Feb 2019 12:20:14 +0100
Subject: [PATCH] [KEYCLOAK-9786] Secure token and logout endpoint

* now token validity is checked to reach those endpoints, even though a valid cookie is presented
* previous BadRequest responses on malformed tokens now yield Unauthorized

Signed-off-by: Frederic BIDON <frederic@oneconcern.com>
---
 handlers_test.go | 17 +++++++++++------
 middleware.go    |  2 +-
 server.go        |  6 +++---
 3 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/handlers_test.go b/handlers_test.go
index b4aa6c9..be97996 100644
--- a/handlers_test.go
+++ b/handlers_test.go
@@ -138,7 +138,10 @@ func TestLoginHandler(t *testing.T) {
 
 func TestLogoutHandlerBadRequest(t *testing.T) {
 	requests := []fakeRequest{
-		{URI: newFakeKeycloakConfig().WithOAuthURI(logoutURL), ExpectedCode: http.StatusBadRequest},
+		{
+			URI:          newFakeKeycloakConfig().WithOAuthURI(logoutURL),
+			ExpectedCode: http.StatusUnauthorized,
+		},
 	}
 	newFakeProxy(nil).RunTests(t, requests)
 }
@@ -148,18 +151,18 @@ func TestLogoutHandlerBadToken(t *testing.T) {
 	requests := []fakeRequest{
 		{
 			URI:          c.WithOAuthURI(logoutURL),
-			ExpectedCode: http.StatusBadRequest,
+			ExpectedCode: http.StatusUnauthorized,
 		},
 		{
 			URI:            c.WithOAuthURI(logoutURL),
 			HasCookieToken: true,
 			RawToken:       "this.is.a.bad.token",
-			ExpectedCode:   http.StatusBadRequest,
+			ExpectedCode:   http.StatusUnauthorized,
 		},
 		{
 			URI:          c.WithOAuthURI(logoutURL),
 			RawToken:     "this.is.a.bad.token",
-			ExpectedCode: http.StatusBadRequest,
+			ExpectedCode: http.StatusUnauthorized,
 		},
 	}
 	newFakeProxy(nil).RunTests(t, requests)
@@ -185,20 +188,22 @@ func TestLogoutHandlerGood(t *testing.T) {
 
 func TestTokenHandler(t *testing.T) {
 	uri := newFakeKeycloakConfig().WithOAuthURI(tokenURL)
+	goodToken := newTestToken("example").getToken()
 	requests := []fakeRequest{
 		{
 			URI:          uri,
 			HasToken:     true,
+			RawToken:     (&goodToken).Encode(),
 			ExpectedCode: http.StatusOK,
 		},
 		{
 			URI:          uri,
-			ExpectedCode: http.StatusBadRequest,
+			ExpectedCode: http.StatusUnauthorized,
 		},
 		{
 			URI:          uri,
 			RawToken:     "niothing",
-			ExpectedCode: http.StatusBadRequest,
+			ExpectedCode: http.StatusUnauthorized,
 		},
 		{
 			URI:            uri,
diff --git a/middleware.go b/middleware.go
index 74be12c..7adcde9 100644
--- a/middleware.go
+++ b/middleware.go
@@ -98,7 +98,7 @@ func (r *oauthProxy) loggingMiddleware(next http.Handler) http.Handler {
 }
 
 // authenticationMiddleware is responsible for verifying the access token
-func (r *oauthProxy) authenticationMiddleware(resource *Resource) func(http.Handler) http.Handler {
+func (r *oauthProxy) authenticationMiddleware() func(http.Handler) http.Handler {
 	return func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
 			clientIP := req.RemoteAddr
diff --git a/server.go b/server.go
index b169579..fa6d79e 100644
--- a/server.go
+++ b/server.go
@@ -204,8 +204,8 @@ func (r *oauthProxy) createReverseProxy() error {
 		e.Get(callbackURL, r.oauthCallbackHandler)
 		e.Get(expiredURL, r.expirationHandler)
 		e.Get(healthURL, r.healthHandler)
-		e.Get(logoutURL, r.logoutHandler)
-		e.Get(tokenURL, r.tokenHandler)
+		e.With(r.authenticationMiddleware()).Get(logoutURL, r.logoutHandler)
+		e.With(r.authenticationMiddleware()).Get(tokenURL, r.tokenHandler)
 		e.Post(loginURL, r.loginHandler)
 		if r.config.EnableMetrics {
 			r.log.Info("enabled the service metrics middleware", zap.String("path", r.config.WithOAuthURI(metricsURL)))
@@ -260,7 +260,7 @@ func (r *oauthProxy) createReverseProxy() error {
 	for _, x := range r.config.Resources {
 		r.log.Info("protecting resource", zap.String("resource", x.String()))
 		e := engine.With(
-			r.authenticationMiddleware(x),
+			r.authenticationMiddleware(),
 			r.admissionMiddleware(x),
 			r.identityHeadersMiddleware(r.config.AddClaims))
 
-- 
GitLab