Prevent creating empty sessions (#6677)

* Prevent creating empty sessions

Signed-off-by: Andrew Thornton <art27@cantab.net>

* Update modules/setting/session.go

* Remove unnecessary option

Signed-off-by: Andrew Thornton <art27@cantab.net>

* Add destory to list of ignored misspellings

* rename cookie.go -> virtual.go

* Delete old file

* Add test to ensure that sessions are not created without being logged in

Signed-off-by: Andrew Thornton <art27@cantab.net>

* fix tests

Signed-off-by: Andrew Thornton <art27@cantab.net>

* Update integrations/create_no_session_test.go

Makefile

@@ -154,7 +154,7 @@ misspell-check:
 	@hash misspell > /dev/null 2>&1; if [ $$? -ne 0 ]; then \
 		$(GO) get -u github.com/client9/misspell/cmd/misspell; \
 	fi
-	misspell -error -i unknwon $(GOFILES)
+	misspell -error -i unknwon,destory $(GOFILES)
 
 .PHONY: misspell
 misspell:

integrations/create_no_session_test.go

@@ -0,0 +1,119 @@
+// Copyright 2019 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package integrations
+
+import (
+	"encoding/json"
+	"io/ioutil"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/routers/routes"
+
+	"github.com/go-macaron/session"
+	"github.com/stretchr/testify/assert"
+)
+
+func getSessionID(t *testing.T, resp *httptest.ResponseRecorder) string {
+	cookies := resp.Result().Cookies()
+	found := false
+	sessionID := ""
+	for _, cookie := range cookies {
+		if cookie.Name == setting.SessionConfig.CookieName {
+			sessionID = cookie.Value
+			found = true
+		}
+	}
+	assert.True(t, found)
+	assert.NotEmpty(t, sessionID)
+	return sessionID
+}
+
+func sessionFile(tmpDir, sessionID string) string {
+	return filepath.Join(tmpDir, sessionID[0:1], sessionID[1:2], sessionID)
+}
+
+func sessionFileExist(t *testing.T, tmpDir, sessionID string) bool {
+	sessionFile := sessionFile(tmpDir, sessionID)
+	_, err := os.Lstat(sessionFile)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return false
+		}
+		assert.NoError(t, err)
+	}
+	return true
+}
+
+func TestSessionFileCreation(t *testing.T) {
+	prepareTestEnv(t)
+
+	oldSessionConfig := setting.SessionConfig.ProviderConfig
+	defer func() {
+		setting.SessionConfig.ProviderConfig = oldSessionConfig
+		mac = routes.NewMacaron()
+		routes.RegisterRoutes(mac)
+	}()
+
+	var config session.Options
+	err := json.Unmarshal([]byte(oldSessionConfig), &config)
+	assert.NoError(t, err)
+
+	config.Provider = "file"
+
+	// Now create a temporaryDirectory
+	tmpDir, err := ioutil.TempDir("", "sessions")
+	assert.NoError(t, err)
+	defer func() {
+		if _, err := os.Stat(tmpDir); !os.IsNotExist(err) {
+			_ = os.RemoveAll(tmpDir)
+		}
+	}()
+	config.ProviderConfig = tmpDir
+
+	newConfigBytes, err := json.Marshal(config)
+	assert.NoError(t, err)
+
+	setting.SessionConfig.ProviderConfig = string(newConfigBytes)
+
+	mac = routes.NewMacaron()
+	routes.RegisterRoutes(mac)
+
+	t.Run("NoSessionOnViewIssue", func(t *testing.T) {
+		PrintCurrentTest(t)
+
+		req := NewRequest(t, "GET", "/user2/repo1/issues/1")
+		resp := MakeRequest(t, req, http.StatusOK)
+		sessionID := getSessionID(t, resp)
+
+		// We're not logged in so there should be no session
+		assert.False(t, sessionFileExist(t, tmpDir, sessionID))
+	})
+	t.Run("CreateSessionOnLogin", func(t *testing.T) {
+		PrintCurrentTest(t)
+
+		req := NewRequest(t, "GET", "/user/login")
+		resp := MakeRequest(t, req, http.StatusOK)
+		sessionID := getSessionID(t, resp)
+
+		// We're not logged in so there should be no session
+		assert.False(t, sessionFileExist(t, tmpDir, sessionID))
+
+		doc := NewHTMLParser(t, resp.Body)
+		req = NewRequestWithValues(t, "POST", "/user/login", map[string]string{
+			"_csrf":     doc.GetCSRF(),
+			"user_name": "user2",
+			"password":  userPassword,
+		})
+		resp = MakeRequest(t, req, http.StatusFound)
+		sessionID = getSessionID(t, resp)
+
+		assert.FileExists(t, sessionFile(tmpDir, sessionID))
+	})
+}

modules/session/virtual.go

@@ -0,0 +1,193 @@
+// Copyright 2019 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package session
+
+import (
+	"encoding/json"
+	"fmt"
+	"sync"
+
+	"github.com/go-macaron/session"
+	couchbase "github.com/go-macaron/session/couchbase"
+	memcache "github.com/go-macaron/session/memcache"
+	mysql "github.com/go-macaron/session/mysql"
+	nodb "github.com/go-macaron/session/nodb"
+	postgres "github.com/go-macaron/session/postgres"
+	redis "github.com/go-macaron/session/redis"
+)
+
+// VirtualSessionProvider represents a shadowed session provider implementation.
+type VirtualSessionProvider struct {
+	lock        sync.RWMutex
+	maxlifetime int64
+	rootPath    string
+	provider    session.Provider
+}
+
+// Init initializes the cookie session provider with given root path.
+func (o *VirtualSessionProvider) Init(gclifetime int64, config string) error {
+	var opts session.Options
+	if err := json.Unmarshal([]byte(config), &opts); err != nil {
+		return err
+	}
+	// Note that these options are unprepared so we can't just use NewManager here.
+	// Nor can we access the provider map in session.
+	// So we will just have to do this by hand.
+	// This is only slightly more wrong than modules/setting/session.go:23
+	switch opts.Provider {
+	case "memory":
+		o.provider = &session.MemProvider{}
+	case "file":
+		o.provider = &session.FileProvider{}
+	case "redis":
+		o.provider = &redis.RedisProvider{}
+	case "mysql":
+		o.provider = &mysql.MysqlProvider{}
+	case "postgres":
+		o.provider = &postgres.PostgresProvider{}
+	case "couchbase":
+		o.provider = &couchbase.CouchbaseProvider{}
+	case "memcache":
+		o.provider = &memcache.MemcacheProvider{}
+	case "nodb":
+		o.provider = &nodb.NodbProvider{}
+	default:
+		return fmt.Errorf("VirtualSessionProvider: Unknown Provider: %s", opts.Provider)
+	}
+	return o.provider.Init(gclifetime, opts.ProviderConfig)
+}
+
+// Read returns raw session store by session ID.
+func (o *VirtualSessionProvider) Read(sid string) (session.RawStore, error) {
+	o.lock.RLock()
+	defer o.lock.RUnlock()
+	if o.provider.Exist(sid) {
+		return o.provider.Read(sid)
+	}
+	kv := make(map[interface{}]interface{})
+	kv["_old_uid"] = "0"
+	return NewVirtualStore(o, sid, kv), nil
+}
+
+// Exist returns true if session with given ID exists.
+func (o *VirtualSessionProvider) Exist(sid string) bool {
+	return true
+}
+
+// Destory deletes a session by session ID.
+func (o *VirtualSessionProvider) Destory(sid string) error {
+	o.lock.Lock()
+	defer o.lock.Unlock()
+	return o.provider.Destory(sid)
+}
+
+// Regenerate regenerates a session store from old session ID to new one.
+func (o *VirtualSessionProvider) Regenerate(oldsid, sid string) (session.RawStore, error) {
+	o.lock.Lock()
+	defer o.lock.Unlock()
+	return o.provider.Regenerate(oldsid, sid)
+}
+
+// Count counts and returns number of sessions.
+func (o *VirtualSessionProvider) Count() int {
+	o.lock.RLock()
+	defer o.lock.RUnlock()
+	return o.provider.Count()
+}
+
+// GC calls GC to clean expired sessions.
+func (o *VirtualSessionProvider) GC() {
+	o.provider.GC()
+}
+
+func init() {
+	session.Register("VirtualSession", &VirtualSessionProvider{})
+}
+
+// VirtualStore represents a virtual session store implementation.
+type VirtualStore struct {
+	p    *VirtualSessionProvider
+	sid  string
+	lock sync.RWMutex
+	data map[interface{}]interface{}
+}
+
+// NewVirtualStore creates and returns a virtual session store.
+func NewVirtualStore(p *VirtualSessionProvider, sid string, kv map[interface{}]interface{}) *VirtualStore {
+	return &VirtualStore{
+		p:    p,
+		sid:  sid,
+		data: kv,
+	}
+}
+
+// Set sets value to given key in session.
+func (s *VirtualStore) Set(key, val interface{}) error {
+	s.lock.Lock()
+	defer s.lock.Unlock()
+
+	s.data[key] = val
+	return nil
+}
+
+// Get gets value by given key in session.
+func (s *VirtualStore) Get(key interface{}) interface{} {
+	s.lock.RLock()
+	defer s.lock.RUnlock()
+
+	return s.data[key]
+}
+
+// Delete delete a key from session.
+func (s *VirtualStore) Delete(key interface{}) error {
+	s.lock.Lock()
+	defer s.lock.Unlock()
+
+	delete(s.data, key)
+	return nil
+}
+
+// ID returns current session ID.
+func (s *VirtualStore) ID() string {
+	return s.sid
+}
+
+// Release releases resource and save data to provider.
+func (s *VirtualStore) Release() error {
+	s.lock.Lock()
+	defer s.lock.Unlock()
+	// Now need to lock the provider
+	s.p.lock.Lock()
+	defer s.p.lock.Unlock()
+	if oldUID, ok := s.data["_old_uid"]; (ok && (oldUID != "0" || len(s.data) > 1)) || (!ok && len(s.data) > 0) {
+		// Now ensure that we don't exist!
+		realProvider := s.p.provider
+
+		if realProvider.Exist(s.sid) {
+			// This is an error!
+			return fmt.Errorf("new sid '%s' already exists", s.sid)
+		}
+		realStore, err := realProvider.Read(s.sid)
+		if err != nil {
+			return err
+		}
+		for key, value := range s.data {
+			if err := realStore.Set(key, value); err != nil {
+				return err
+			}
+		}
+		return realStore.Release()
+	}
+	return nil
+}
+
+// Flush deletes all session data.
+func (s *VirtualStore) Flush() error {
+	s.lock.Lock()
+	defer s.lock.Unlock()
+
+	s.data = make(map[interface{}]interface{})
+	return nil
+}

modules/setting/session.go

@@ -5,11 +5,15 @@
 package setting
 
 import (
+	"encoding/json"
 	"path"
 	"path/filepath"
 	"strings"
 
 	"code.gitea.io/gitea/modules/log"
+	// This ensures that VirtualSessionProvider is available
+	_ "code.gitea.io/gitea/modules/session"
+
 	"github.com/go-macaron/session"
 )
 
@@ -31,5 +35,12 @@ func newSessionService() {
 	SessionConfig.Gclifetime = Cfg.Section("session").Key("GC_INTERVAL_TIME").MustInt64(86400)
 	SessionConfig.Maxlifetime = Cfg.Section("session").Key("SESSION_LIFE_TIME").MustInt64(86400)
 
+	shadowConfig, err := json.Marshal(SessionConfig)
+	if err != nil {
+		log.Fatal("Can't shadow session config: %v", err)
+	}
+	SessionConfig.ProviderConfig = string(shadowConfig)
+	SessionConfig.Provider = "VirtualSession"
+
 	log.Info("Session Service Enabled")
 }