Store User ID in session instead of the whole user model (#12984)

This commit makes a change so that the user id will be stored in sessoin
after user login instead of user model to avoid data inconsistency when
user model changes.

Fixes #12934

Signed-off-by: Daniel Jiang <jiangd@vmware.com>

src/core/api/base.go

@@ -35,11 +35,11 @@ import (
 	"github.com/goharbor/harbor/src/pkg/repository"
 	"github.com/goharbor/harbor/src/pkg/retention"
 	"github.com/goharbor/harbor/src/pkg/scheduler"
+	sec "github.com/goharbor/harbor/src/server/middleware/security"
 )
 
 const (
 	yamlFileContentType = "application/x-yaml"
-	userSessionKey      = "user"
 )
 
 // the managers/controllers used globally
@@ -176,7 +176,7 @@ func (b *BaseController) WriteYamlData(object interface{}) {
 // PopulateUserSession generates a new session ID and fill the user model in parm to the session
 func (b *BaseController) PopulateUserSession(u models.User) {
 	b.SessionRegenerateID()
-	b.SetSession(userSessionKey, u)
+	b.SetSession(sec.UserIDSessionKey, u.UserID)
 }
 
 // Init related objects/configurations for the API controllers

src/server/middleware/security/auth_proxy_test.go

@@ -17,25 +17,23 @@ package security
 import (
 	"encoding/json"
 	"fmt"
+	"io/ioutil"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"testing"
+
 	"github.com/goharbor/harbor/src/common"
 	"github.com/goharbor/harbor/src/common/models"
-	"github.com/goharbor/harbor/src/common/utils/test"
 	_ "github.com/goharbor/harbor/src/core/auth/authproxy"
 	"github.com/goharbor/harbor/src/core/config"
 	"github.com/goharbor/harbor/src/lib"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"io/ioutil"
 	"k8s.io/api/authentication/v1beta1"
-	"net/http"
-	"net/http/httptest"
-	"net/url"
-	"testing"
 )
 
 func TestAuthProxy(t *testing.T) {
-	config.Init()
-	test.InitDatabaseFromEnv()
 	authProxy := &authProxy{}
 
 	server, err := newAuthProxyTestServer()
@@ -49,7 +47,7 @@ func TestAuthProxy(t *testing.T) {
 		common.HTTPAuthProxyTokenReviewEndpoint: server.URL,
 		common.AUTHMode:                         common.HTTPAuth,
 	}
-	config.Upload(c)
+	config.InitWithSettings(c)
 	v, e := config.HTTPAuthProxySetting()
 	require.Nil(t, e)
 	assert.Equal(t, *v, models.HTTPAuthProxy{

src/server/middleware/security/basic_auth_test.go

@@ -15,18 +15,39 @@
 package security
 
 import (
+	"fmt"
+
+	"github.com/goharbor/harbor/src/common"
+	"github.com/goharbor/harbor/src/common/dao"
+	"github.com/goharbor/harbor/src/common/models"
 	_ "github.com/goharbor/harbor/src/core/auth/db"
+	"github.com/goharbor/harbor/src/core/config"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
 	"net/http"
 	"testing"
 )
 
 func TestBasicAuth(t *testing.T) {
+	c := map[string]interface{}{
+		common.AUTHMode: common.DBAuth,
+	}
+	config.InitWithSettings(c)
+
+	user := models.User{
+		Username: "tester",
+		Password: "Harbor12345",
+	}
+	uid, err := dao.Register(user)
+	defer func(id int64) {
+		sql := fmt.Sprintf("DELETE FROM harbor_user WHERE user_id=%d", id)
+		dao.ExecuteBatchSQL([]string{sql})
+	}(uid)
 	basicAuth := &basicAuth{}
 	req, err := http.NewRequest(http.MethodGet, "http://127.0.0.1/api/projects/", nil)
 	require.Nil(t, err)
-	req.SetBasicAuth("admin", "Harbor12345")
+	req.SetBasicAuth("tester", "Harbor12345")
 	ctx := basicAuth.Generate(req)
 	assert.NotNil(t, ctx)
 }

src/server/middleware/security/security_test.go

@@ -15,13 +15,21 @@
 package security
 
 import (
+	"github.com/goharbor/harbor/src/common/dao"
 	"github.com/goharbor/harbor/src/common/security"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
 	"net/http"
+	"os"
 	"testing"
 )
 
+func TestMain(m *testing.M) {
+	dao.PrepareTestForPostgresSQL()
+	os.Exit(m.Run())
+}
+
 func TestSecurity(t *testing.T) {
 	var ctx security.Context
 	var exist bool

src/server/middleware/security/session.go

@@ -19,6 +19,7 @@ import (
 	"net/http/httptest"
 
 	"github.com/astaxie/beego"
+	"github.com/goharbor/harbor/src/common/dao"
 	"github.com/goharbor/harbor/src/common/models"
 	"github.com/goharbor/harbor/src/common/security"
 	"github.com/goharbor/harbor/src/common/security/local"
@@ -26,6 +27,9 @@ import (
 	"github.com/goharbor/harbor/src/lib/log"
 )
 
+// UserIDSessionKey is the key for storing user id in session
+const UserIDSessionKey = "user_id"
+
 type session struct{}
 
 func (s *session) Generate(req *http.Request) security.Context {
@@ -35,15 +39,24 @@ func (s *session) Generate(req *http.Request) security.Context {
 		log.Errorf("failed to get the session store for request: %v", err)
 		return nil
 	}
-	userInterface := store.Get("user")
+	userInterface := store.Get(UserIDSessionKey)
 	if userInterface == nil {
 		return nil
 	}
-	user, ok := userInterface.(models.User)
+	uid, ok := userInterface.(int)
 	if !ok {
-		log.Warning("can not convert the user in session to user model")
+		log.Warning("can not convert the data in session to user id")
+		return nil
+	}
+	user, err := dao.GetUser(models.User{UserID: uid})
+	if err != nil {
+		log.Errorf("failed to get user info, error: %v, skip generating security context via session", err)
+		return nil
+	}
+	if user == nil {
+		log.Errorf("the user id from session: %d, not exist in DB", uid)
 		return nil
 	}
 	log.Debugf("a session security context generated for request %s %s", req.Method, req.URL.Path)
-	return local.NewSecurityContext(&user, config.GlobalProjectMgr)
+	return local.NewSecurityContext(user, config.GlobalProjectMgr)
 }

src/server/middleware/security/session_test.go

@@ -15,11 +15,15 @@
 package security
 
 import (
+	"fmt"
+
 	"github.com/astaxie/beego"
 	beegosession "github.com/astaxie/beego/session"
+	"github.com/goharbor/harbor/src/common/dao"
 	"github.com/goharbor/harbor/src/common/models"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
 	"net/http"
 	"net/http/httptest"
 	"path/filepath"
@@ -41,20 +45,31 @@ func TestSession(t *testing.T) {
 	beego.GlobalSessions, err = beegosession.NewManager("memory", conf)
 	require.Nil(t, err)
 
-	user := models.User{
+	user := &models.User{
-		Username:     "admin",
+		Username:     "session_test",
-		UserID:       1,
+		Email:        "session_test@example.com",
-		Email:        "admin@example.com",
 		SysAdminFlag: true,
 	}
+	err = dao.OnBoardUser(user)
+	require.Nil(t, err)
+	defer func(id int) {
+		sql := fmt.Sprintf("DELETE FROM harbor_user WHERE user_id=%d", id)
+		dao.ExecuteBatchSQL([]string{sql})
+	}(user.UserID)
+
 	req, err := http.NewRequest(http.MethodGet, "http://127.0.0.1/api/projects/", nil)
 	require.Nil(t, err)
 	store, err := beego.GlobalSessions.SessionStart(httptest.NewRecorder(), req)
 	require.Nil(t, err)
-	err = store.Set("user", user)
+	err = store.Set(UserIDSessionKey, 999)
 	require.Nil(t, err)
 
 	session := &session{}
 	ctx := session.Generate(req)
+	assert.Nil(t, ctx)
+	err = store.Set(UserIDSessionKey, user.UserID)
+	require.Nil(t, err)
+	ctx = session.Generate(req)
 	assert.NotNil(t, ctx)
+
 }