diff --git a/src/common/dao/project.go b/src/common/dao/project.go index 5e3165ff2..75e2fc3ed 100644 --- a/src/common/dao/project.go +++ b/src/common/dao/project.go @@ -292,8 +292,10 @@ func GetRolesByLDAPGroup(projectID int64, groupDNCondition string) ([]int, error return roles, nil } o := GetOrmer() + //Because an LDAP user can be memberof multiple groups, + //the role is in descent order (1-admin, 2-developer, 3-guest), use min to select the max privilege role. sql := fmt.Sprintf( - `select pm.role from project_member pm + `select min(pm.role) from project_member pm left join user_group ug on pm.entity_type = 'g' and pm.entity_id = ug.id where ug.ldap_group_dn in ( %s ) and pm.project_id = ? `, groupDNCondition) @@ -302,5 +304,9 @@ func GetRolesByLDAPGroup(projectID int64, groupDNCondition string) ([]int, error log.Warningf("Error in GetRolesByLDAPGroup, error: %v", err) return nil, err } + //If there is no row selected, the min returns an empty row, to avoid return 0 as role + if len(roles) == 1 && roles[0] == 0 { + return []int{}, nil + } return roles, nil } diff --git a/src/common/utils/ldap/ldap.go b/src/common/utils/ldap/ldap.go index 505b3f277..5ca2fbacd 100644 --- a/src/common/utils/ldap/ldap.go +++ b/src/common/utils/ldap/ldap.go @@ -202,7 +202,10 @@ func (session *Session) SearchUser(username string) ([]models.LdapUser, error) { case "email": u.Email = val case "memberof": - groupDNList = append(groupDNList, val) + for _, dnItem := range attr.Values { + groupDNList = append(groupDNList, strings.TrimSpace(dnItem)) + log.Debugf("Found memberof %v", dnItem) + } } u.GroupDNList = groupDNList } diff --git a/src/ui/auth/ldap/ldap.go b/src/ui/auth/ldap/ldap.go index 26a28fdcb..f530935c0 100644 --- a/src/ui/auth/ldap/ldap.go +++ b/src/ui/auth/ldap/ldap.go @@ -59,6 +59,8 @@ func (l *Auth) Authenticate(m models.AuthModel) (*models.User, error) { ldapUsers, err := ldapSession.SearchUser(p) + log.Debugf("Found ldap user %+v", ldapUsers[0]) + if err != nil { log.Warningf("ldap search fail: %v", err) return nil, err diff --git a/src/ui/controllers/base.go b/src/ui/controllers/base.go index 79d1b3bf1..8cdad8463 100644 --- a/src/ui/controllers/base.go +++ b/src/ui/controllers/base.go @@ -69,10 +69,7 @@ func (cc *CommonController) Login() { if user == nil { cc.CustomAbort(http.StatusUnauthorized, "") } - - cc.SetSession("userId", user.UserID) - cc.SetSession("username", user.Username) - cc.SetSession("isSysAdmin", user.HasAdminRole) + cc.SetSession("user", *user) } // LogOut Habor UI diff --git a/src/ui/filter/security.go b/src/ui/filter/security.go index de6c3d733..afa6fc851 100644 --- a/src/ui/filter/security.go +++ b/src/ui/filter/security.go @@ -215,7 +215,6 @@ func (b *basicAuthReqCtxModifier) Modify(ctx *beegoctx.Context) bool { pm := config.GlobalProjectMgr log.Debug("creating local database security context...") securCtx := local.NewSecurityContext(user, pm) - setSecurCtxAndPM(ctx.Request, securCtx, pm) return true } @@ -223,24 +222,25 @@ func (b *basicAuthReqCtxModifier) Modify(ctx *beegoctx.Context) bool { type sessionReqCtxModifier struct{} func (s *sessionReqCtxModifier) Modify(ctx *beegoctx.Context) bool { - username := ctx.Input.Session("username") - if username == nil { + var user models.User + userInterface := ctx.Input.Session("user") + + if userInterface == nil { + log.Debug("can not get user information from session") return false } log.Debug("got user information from session") - user := &models.User{ - Username: username.(string), + user, ok := userInterface.(models.User) + if !ok { + log.Info("can not get user information from session") + return false } - isSysAdmin := ctx.Input.Session("isSysAdmin") - if isSysAdmin != nil && isSysAdmin.(bool) { - user.HasAdminRole = true - } - + log.Debug("Getting user %+v", user) log.Debug("using local database project manager") pm := config.GlobalProjectMgr log.Debug("creating local database security context...") - securCtx := local.NewSecurityContext(user, pm) + securCtx := local.NewSecurityContext(&user, pm) setSecurCtxAndPM(ctx.Request, securCtx, pm) diff --git a/src/ui/filter/security_test.go b/src/ui/filter/security_test.go index e581f7ea1..d1e7818d1 100644 --- a/src/ui/filter/security_test.go +++ b/src/ui/filter/security_test.go @@ -30,6 +30,7 @@ import ( "github.com/astaxie/beego/session" "github.com/stretchr/testify/assert" "github.com/vmware/harbor/src/common/dao" + "github.com/vmware/harbor/src/common/models" commonsecret "github.com/vmware/harbor/src/common/secret" "github.com/vmware/harbor/src/common/security" "github.com/vmware/harbor/src/common/security/local" @@ -146,6 +147,12 @@ func TestBasicAuthReqCtxModifier(t *testing.T) { } func TestSessionReqCtxModifier(t *testing.T) { + user := models.User{ + Username: "admin", + UserID: 1, + Email: "admin@example.com", + HasAdminRole: true, + } req, err := http.NewRequest(http.MethodGet, "http://127.0.0.1/api/projects/", nil) if err != nil { @@ -155,10 +162,7 @@ func TestSessionReqCtxModifier(t *testing.T) { if err != nil { t.Fatalf("failed to create session store: %v", err) } - if err = store.Set("username", "admin"); err != nil { - t.Fatalf("failed to set session: %v", err) - } - if err = store.Set("isSysAdmin", true); err != nil { + if err = store.Set("user", user); err != nil { t.Fatalf("failed to set session: %v", err) } @@ -184,6 +188,41 @@ func TestSessionReqCtxModifier(t *testing.T) { assert.Equal(t, "admin", s.GetUsername()) assert.True(t, s.IsSysAdmin()) assert.NotNil(t, projectManager(ctx)) + +} + +func TestSessionReqCtxModifierFailed(t *testing.T) { + user := "admin" + req, err := http.NewRequest(http.MethodGet, + "http://127.0.0.1/api/projects/", nil) + if err != nil { + t.Fatalf("failed to create request: %v", req) + } + store, err := beego.GlobalSessions.SessionStart(httptest.NewRecorder(), req) + if err != nil { + t.Fatalf("failed to create session store: %v", err) + } + if err = store.Set("user", user); err != nil { + t.Fatalf("failed to set session: %v", err) + } + + req, err = http.NewRequest(http.MethodGet, + "http://127.0.0.1/api/projects/", nil) + if err != nil { + t.Fatalf("failed to create request: %v", req) + } + addSessionIDToCookie(req, store.SessionID()) + + ctx, err := newContext(req) + if err != nil { + t.Fatalf("failed to crate context: %v", err) + } + + modifier := &sessionReqCtxModifier{} + modified := modifier.Modify(ctx) + + assert.False(t, modified) + } // TODO add test case