Add DelAllSession method

In order to prevent leaking of session values (and to avoid the mess of
deleting the entire session cookie which could still have values we want
in it) this nuclear method is now called by expire/logout with a
whitelist of keys to keep (passed in from
Config.Storage.SessionWhitelistKeys).

CHANGELOG.md

@@ -3,6 +3,23 @@
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+## [Unreleased]
+
+### Added
+
+- DelAllSession is a new method called both by Expire and Logout (in addition
+  to still calling DelKnownSession etc. as they do now) to ensure that
+  conforming implementations of ClientStateReadWriter's delete all keys
+  in the session.
+- Config.Storage.SessionWhitelistKeys has been added in order to allow users
+  to persist session variables past logout/expire.
+
+### Deprecated
+
+- Deprecated DelKnownSession for DelAllSession. DelAllSession should be
+  implemented by existing ClientStateReadWriters in order to prevent session
+  values from leaking to a different user post-logout/expire.
+
 ## [2.2.0] - 2018-12-16
 
 ### Added

README.md

@@ -382,9 +382,11 @@ method. This ensures the above facets are taken care of which the exception of t
 If it's also desirable to have the user logged out, please use the following methods to erase
 all known sessions and cookies from the user.
 
-* [authboss.DelKnownSession](https://godoc.org//github.com/volatiletech/authboss/#DelKnownSession)
+* [authboss.DelAllSession](https://godoc.org//github.com/volatiletech/authboss/#DelAllSession)
 * [authboss.DelKnownCookie](https://godoc.org//github.com/volatiletech/authboss/#DelKnownCookie)
 
+*Note: DelKnownSession has been deprecated for security reasons*
+
 ## User Auth via Password
 
 | Info and Requirements |          |

client_state.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"net/http"
+	"strings"
 )
 
 const (
@@ -45,11 +46,21 @@ type ClientStateEventKind int
 
 // ClientStateEvent kinds
 const (
+	// ClientStateEventPut means you should put the key-value pair into the
+	// client state.
 	ClientStateEventPut ClientStateEventKind = iota
+	// ClientStateEventPut means you should delete the key-value pair from the
+	// client state.
 	ClientStateEventDel
+	// ClientStateEventDelAll means you should delete EVERY key-value pair from
+	// the client state - though a whitelist of keys that should not be deleted
+	// may be passed through as a comma separated list of keys in
+	// the ClientStateEvent.Key field.
+	ClientStateEventDelAll
 )
 
 // ClientStateEvent are the different events that can be recorded during
+// a request.
 type ClientStateEvent struct {
 	Kind  ClientStateEventKind
 	Key   string
@@ -247,8 +258,21 @@ func IsTwoFactored(r *http.Request) bool {
 	return has2fa
 }
 
-// DelKnownSession deletes all known session variables, effectively
-// logging a user out.
+// DelAllSession deletes all variables in the session except for those on
+// the whitelist.
+//
+// The whitelist is typically provided directly from the authboss config.
+//
+// This is the best way to ensure the session is cleaned up after use for
+// a given user. An example is when a user is expired or logged out this method
+// is called.
+func DelAllSession(w http.ResponseWriter, whitelist []string) {
+	delAllState(w, CTXKeySessionState, whitelist)
+}
+
+// DelKnownSession is deprecated. See DelAllSession for an alternative.
+// DelKnownSession deletes all known session variables,
+// effectively logging a user out.
 func DelKnownSession(w http.ResponseWriter) {
 	DelSession(w, SessionKey)
 	DelSession(w, SessionHalfAuthKey)
@@ -299,6 +323,10 @@ func delState(w http.ResponseWriter, CTXKey contextKey, key string) {
 	setState(w, CTXKey, ClientStateEventDel, key, "")
 }
 
+func delAllState(w http.ResponseWriter, CTXKey contextKey, whitelist []string) {
+	setState(w, CTXKey, ClientStateEventDelAll, strings.Join(whitelist, ","), "")
+}
+
 func setState(w http.ResponseWriter, ctxKey contextKey, op ClientStateEventKind, key, val string) {
 	csrw := MustClientStateResponseWriter(w)
 	ev := ClientStateEvent{

client_state_test.go

@@ -160,6 +160,23 @@ func TestFlashClearer(t *testing.T) {
 	}
 }
 
+func TestDelAllSession(t *testing.T) {
+	t.Parallel()
+
+	csrw := &ClientStateResponseWriter{}
+
+	DelAllSession(csrw, []string{"notthisone", "orthis"})
+
+	if len(csrw.sessionStateEvents) != 1 {
+		t.Error("should have one delete all")
+	}
+	if ev := csrw.sessionStateEvents[0]; ev.Kind != ClientStateEventDelAll {
+		t.Error("it should be a delete all event:", ev.Kind)
+	} else if ev.Key != "notthisone,orthis" {
+		t.Error("the whitelist should be passed through as CSV:", ev.Key)
+	}
+}
+
 func TestDelKnown(t *testing.T) {
 	t.Parallel()
 

config.go

@@ -189,6 +189,12 @@ type Config struct {
 		// storing session-only values for the given response, and reading them
 		// from the request.
 		SessionState ClientStateReadWriter
+
+		// SessionStateWhitelistKeys are set to preserve keys in the session
+		// when authboss.DelAllSession is called. A correct implementation
+		// of ClientStateReadWriter will delete ALL session key-value pairs
+		// unless that key is whitelisted here.
+		SessionStateWhitelistKeys []string
 	}
 
 	Core struct {

expire/expire.go

@@ -47,8 +47,9 @@ func refreshExpiry(w http.ResponseWriter) {
 }
 
 type expireMiddleware struct {
-	expireAfter time.Duration
-	next        http.Handler
+	expireAfter      time.Duration
+	next             http.Handler
+	sessionWhitelist []string
 }
 
 // Middleware ensures that the user's expiry information is kept up-to-date
@@ -58,7 +59,11 @@ type expireMiddleware struct {
 // at the same time.
 func Middleware(ab *authboss.Authboss) func(http.Handler) http.Handler {
 	return func(next http.Handler) http.Handler {
-		return expireMiddleware{ab.Config.Modules.ExpireAfter, next}
+		return expireMiddleware{
+			expireAfter:      ab.Config.Modules.ExpireAfter,
+			next:             next,
+			sessionWhitelist: ab.Config.Storage.SessionStateWhitelistKeys,
+		}
 	}
 }
 
@@ -68,6 +73,7 @@ func (m expireMiddleware) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	if _, ok := authboss.GetSession(r, authboss.SessionKey); ok {
 		ttl := timeToExpiry(r, m.expireAfter)
 		if ttl == 0 {
+			authboss.DelAllSession(w, m.sessionWhitelist)
 			authboss.DelSession(w, authboss.SessionKey)
 			authboss.DelSession(w, authboss.SessionLastAction)
 			ctx := context.WithValue(r.Context(), authboss.CTXKeyPID, nil)

logout/logout.go

@@ -49,6 +49,7 @@ func (l *Logout) Logout(w http.ResponseWriter, r *http.Request) error {
 		logger.Info("user (unknown) logged out")
 	}
 
+	authboss.DelAllSession(w, l.Config.Storage.SessionStateWhitelistKeys)
 	authboss.DelKnownSession(w)
 	authboss.DelKnownCookie(w)