Merge pull request #13053 from reasonerjt/fix-ldap-admingroup

Revert "Store User ID in session instead of the whole user model (#12…

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(sec.UserIDSessionKey, u.UserID)
+	b.SetSession(userSessionKey, u)
 }
 
 // Init related objects/configurations for the API controllers

src/server/middleware/security/auth_proxy_test.go

@@ -17,23 +17,25 @@ 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()
@@ -47,7 +49,7 @@ func TestAuthProxy(t *testing.T) {
 		common.HTTPAuthProxyTokenReviewEndpoint: server.URL,
 		common.AUTHMode:                         common.HTTPAuth,
 	}
-	config.InitWithSettings(c)
+	config.Upload(c)
 	v, e := config.HTTPAuthProxySetting()
 	require.Nil(t, e)
 	assert.Equal(t, *v, models.HTTPAuthProxy{

src/server/middleware/security/basic_auth_test.go

@@ -15,39 +15,18 @@
 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("tester", "Harbor12345")
+	req.SetBasicAuth("admin", "Harbor12345")
 	ctx := basicAuth.Generate(req)
 	assert.NotNil(t, ctx)
 }

src/server/middleware/security/security_test.go

@@ -15,21 +15,13 @@
 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,7 +19,6 @@ 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"
@@ -27,9 +26,6 @@ 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 {
@@ -39,24 +35,15 @@ 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(UserIDSessionKey)
+	userInterface := store.Get("user")
 	if userInterface == nil {
 		return nil
 	}
-	uid, ok := userInterface.(int)
+	user, ok := userInterface.(models.User)
 	if !ok {
-		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)
+		log.Warning("can not convert the user in session to user model")
 		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,15 +15,11 @@
 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"
@@ -45,31 +41,20 @@ func TestSession(t *testing.T) {
 	beego.GlobalSessions, err = beegosession.NewManager("memory", conf)
 	require.Nil(t, err)
 
-	user := &models.User{
-		Username:     "session_test",
-		Email:        "session_test@example.com",
+	user := models.User{
+		Username:     "admin",
+		UserID:       1,
+		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(UserIDSessionKey, 999)
+	err = store.Set("user", user)
 	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)
-
 }