diff --git a/src/common/dao/dao_test.go b/src/common/dao/dao_test.go index 26c7e1965..03252787e 100644 --- a/src/common/dao/dao_test.go +++ b/src/common/dao/dao_test.go @@ -305,6 +305,7 @@ var currentUser *models.User func TestGetUser(t *testing.T) { queryUser := models.User{ Username: username, + Email: "tester01@vmware.com", } var err error currentUser, err = GetUser(queryUser) diff --git a/src/common/dao/user.go b/src/common/dao/user.go index 0f11706b3..17ed5e37f 100644 --- a/src/common/dao/user.go +++ b/src/common/dao/user.go @@ -52,6 +52,11 @@ func GetUser(query models.User) (*models.User, error) { queryParam = append(queryParam, query.ResetUUID) } + if query.Email != "" { + sql += ` and email = ? ` + queryParam = append(queryParam, query.Email) + } + var u []models.User n, err := o.Raw(sql, queryParam).QueryRows(&u) diff --git a/src/ui/api/user.go b/src/ui/api/user.go index d96b02dbd..b43de5a4e 100644 --- a/src/ui/api/user.go +++ b/src/ui/api/user.go @@ -21,6 +21,7 @@ import ( "strconv" "strings" + "github.com/vmware/harbor/src/common" "github.com/vmware/harbor/src/common/dao" "github.com/vmware/harbor/src/common/models" "github.com/vmware/harbor/src/common/utils/log" @@ -160,16 +161,9 @@ func (ua *UserAPI) List() { // Put ... func (ua *UserAPI) Put() { - ldapAdminUser := (ua.AuthMode == "ldap_auth" && ua.userID == 1 && ua.userID == ua.currentUserID) - - if !(ua.AuthMode == "db_auth" || ldapAdminUser) { - ua.CustomAbort(http.StatusForbidden, "") - } - if !ua.IsAdmin { - if ua.userID != ua.currentUserID { - log.Warning("Guests can only change their own account.") - ua.CustomAbort(http.StatusForbidden, "Guests can only change their own account.") - } + if !ua.modifiable() { + ua.RenderError(http.StatusForbidden, fmt.Sprintf("User with ID %d cannot be modified", ua.userID)) + return } user := models.User{UserID: ua.userID} ua.DecodeJSONReq(&user) @@ -210,7 +204,7 @@ func (ua *UserAPI) Put() { // Post ... func (ua *UserAPI) Post() { - if !(ua.AuthMode == "db_auth") { + if !(ua.AuthMode == common.DBAuth) { ua.CustomAbort(http.StatusForbidden, "") } @@ -258,22 +252,8 @@ func (ua *UserAPI) Post() { // Delete ... func (ua *UserAPI) Delete() { - if !ua.IsAdmin { - log.Warningf("current user, id: %d does not have admin role, can not remove user", ua.currentUserID) - ua.RenderError(http.StatusForbidden, "User does not have admin role") - return - } - - if ua.AuthMode == "ldap_auth" { - ua.CustomAbort(http.StatusForbidden, "user can not be deleted in LDAP authentication mode") - } - - if ua.currentUserID == ua.userID { - ua.CustomAbort(http.StatusForbidden, "can not delete yourself") - } - - if ua.userID == 1 { - ua.HandleForbidden(ua.SecurityCtx.GetUsername()) + if !ua.IsAdmin || ua.AuthMode != common.DBAuth || ua.userID == 1 || ua.currentUserID == ua.userID { + ua.RenderError(http.StatusForbidden, fmt.Sprintf("User with ID: %d cannot be removed, auth mode: %s, current user ID: %d", ua.userID, ua.AuthMode, ua.currentUserID)) return } @@ -288,17 +268,9 @@ func (ua *UserAPI) Delete() { // ChangePassword handles PUT to /api/users/{}/password func (ua *UserAPI) ChangePassword() { - ldapAdminUser := (ua.AuthMode == "ldap_auth" && ua.userID == 1 && ua.userID == ua.currentUserID) - - if !(ua.AuthMode == "db_auth" || ldapAdminUser) { - ua.CustomAbort(http.StatusForbidden, "") - } - - if !ua.IsAdmin { - if ua.userID != ua.currentUserID { - log.Error("Guests can only change their own account.") - ua.CustomAbort(http.StatusForbidden, "Guests can only change their own account.") - } + if !ua.modifiable() { + ua.RenderError(http.StatusForbidden, fmt.Sprintf("User with ID: %d is not modifiable", ua.userID)) + return } var req passwordReq @@ -345,6 +317,18 @@ func (ua *UserAPI) ToggleUserAdminRole() { } } +// modifiable returns whether the modify is allowed based on current auth mode and context +func (ua *UserAPI) modifiable() bool { + if ua.AuthMode == common.DBAuth { + //When the auth mode is local DB, admin can modify anyone, non-admin can modify himself. + return ua.IsAdmin || ua.userID == ua.currentUserID + } + //When the auth mode is external IDM backend, only the super user can modify himself, + //because he's the only one whose information is stored in local DB. + return ua.userID == 1 && ua.userID == ua.currentUserID + +} + // validate only validate when user register func validate(user models.User) error { diff --git a/src/ui/api/user_test.go b/src/ui/api/user_test.go index 2c74a177f..089e786b2 100644 --- a/src/ui/api/user_test.go +++ b/src/ui/api/user_test.go @@ -16,8 +16,11 @@ package api import ( "fmt" "github.com/stretchr/testify/assert" + "github.com/vmware/harbor/src/common/api" "github.com/vmware/harbor/tests/apitests/apilib" "testing" + + "github.com/astaxie/beego" ) var testUser0002ID, testUser0003ID int @@ -430,3 +433,51 @@ func TestUsersDelete(t *testing.T) { assert.Equal(200, code, "Delete test user status should be 200") } } + +func TestModifiable(t *testing.T) { + assert := assert.New(t) + base := BaseController{ + api.BaseAPI{ + beego.Controller{}, + }, + nil, + nil, + } + + ua1 := &UserAPI{ + base, + 3, + 4, + false, + false, + "db_auth", + } + assert.False(ua1.modifiable()) + ua2 := &UserAPI{ + base, + 3, + 4, + false, + true, + "db_auth", + } + assert.True(ua2.modifiable()) + ua3 := &UserAPI{ + base, + 3, + 4, + false, + true, + "ldap_auth", + } + assert.False(ua3.modifiable()) + ua4 := &UserAPI{ + base, + 1, + 1, + false, + true, + "ldap_auth", + } + assert.True(ua4.modifiable()) +} diff --git a/src/ui/auth/authenticator.go b/src/ui/auth/authenticator.go index 6124e3aa1..d58fa4ac7 100644 --- a/src/ui/auth/authenticator.go +++ b/src/ui/auth/authenticator.go @@ -26,15 +26,6 @@ import ( // 1.5 seconds const frozenTime time.Duration = 1500 * time.Millisecond -const ( - // DBAuth is the database auth mode. - DBAuth = "db_auth" - // LDAPAuth is the ldap auth mode. - LDAPAuth = "ldap_auth" - // UAAAuth is the uaa auth mode. - UAAAuth = "uaa_auth" -) - var lock = NewUserLock(frozenTime) // AuthenticateHelper provides interface for user management in different auth modes. diff --git a/src/ui/config/config.go b/src/ui/config/config.go index 1609e82d3..1fdb34c13 100644 --- a/src/ui/config/config.go +++ b/src/ui/config/config.go @@ -61,12 +61,17 @@ var ( func Init() error { //init key provider initKeyProvider() - adminServerURL := os.Getenv("ADMINSERVER_URL") if len(adminServerURL) == 0 { adminServerURL = "http://adminserver" } + return InitByURL(adminServerURL) + +} + +// InitByURL Init configurations with given url +func InitByURL(adminServerURL string) error { log.Infof("initializing client for adminserver %s ...", adminServerURL) authorizer := auth.NewSecretAuthorizer(secretCookieName, UISecret()) AdminserverClient = client.NewClient(adminServerURL, authorizer) diff --git a/src/ui/controllers/base.go b/src/ui/controllers/base.go index bac64b5a4..9e67d24af 100644 --- a/src/ui/controllers/base.go +++ b/src/ui/controllers/base.go @@ -25,6 +25,7 @@ import ( "github.com/astaxie/beego" "github.com/beego/i18n" + "github.com/vmware/harbor/src/common" "github.com/vmware/harbor/src/common/dao" "github.com/vmware/harbor/src/common/models" "github.com/vmware/harbor/src/common/utils" @@ -101,8 +102,8 @@ func (cc *CommonController) UserExists() { cc.ServeJSON() } -// SendEmail verifies the Email address and contact SMTP server to send reset password Email. -func (cc *CommonController) SendEmail() { +// SendResetEmail verifies the Email address and contact SMTP server to send reset password Email. +func (cc *CommonController) SendResetEmail() { email := cc.GetString("email") @@ -117,16 +118,21 @@ func (cc *CommonController) SendEmail() { } queryUser := models.User{Email: email} - exist, err := dao.UserExists(queryUser, "email") + u, err := dao.GetUser(queryUser) if err != nil { - log.Errorf("Error occurred in UserExists: %v", err) + log.Errorf("Error occurred in GetUser: %v", err) cc.CustomAbort(http.StatusInternalServerError, "Internal error.") } - if !exist { + if u == nil { log.Debugf("email %s not found", email) cc.CustomAbort(http.StatusNotFound, "email_does_not_exist") } + if !isUserResetable(u) { + log.Errorf("Resetting password for user with ID: %d is not allowed", u.UserID) + cc.CustomAbort(http.StatusForbidden, http.StatusText(http.StatusForbidden)) + } + uuid := utils.GenerateRandomString() user := models.User{ResetUUID: uuid, Email: email} if err = dao.UpdateUserResetUUID(user); err != nil { @@ -192,6 +198,7 @@ func (cc *CommonController) ResetPassword() { queryUser := models.User{ResetUUID: resetUUID} user, err := dao.GetUser(queryUser) + if err != nil { log.Errorf("Error occurred in GetUser: %v", err) cc.CustomAbort(http.StatusInternalServerError, "Internal error.") @@ -201,6 +208,11 @@ func (cc *CommonController) ResetPassword() { cc.CustomAbort(http.StatusBadRequest, "User does not exist") } + if !isUserResetable(user) { + log.Errorf("Resetting password for user with ID: %d is not allowed", user.UserID) + cc.CustomAbort(http.StatusForbidden, http.StatusText(http.StatusForbidden)) + } + password := cc.GetString("password") if password != "" { @@ -215,6 +227,21 @@ func (cc *CommonController) ResetPassword() { } } +func isUserResetable(u *models.User) bool { + if u == nil { + return false + } + mode, err := config.AuthMode() + if err != nil { + log.Errorf("Failed to get the auth mode, error: %v", err) + return false + } + if mode == common.DBAuth { + return true + } + return u.UserID == 1 +} + func init() { //conf/app.conf -> os.Getenv("config_path") configPath := os.Getenv("CONFIG_PATH") diff --git a/src/ui/controllers/controllers_test.go b/src/ui/controllers/controllers_test.go index c5231560f..f8bb7ec7f 100644 --- a/src/ui/controllers/controllers_test.go +++ b/src/ui/controllers/controllers_test.go @@ -22,11 +22,14 @@ import ( "testing" "fmt" + "os" "strings" "github.com/astaxie/beego" "github.com/stretchr/testify/assert" - "github.com/vmware/harbor/src/common/utils/log" + "github.com/vmware/harbor/src/common" + "github.com/vmware/harbor/src/common/models" + "github.com/vmware/harbor/src/common/utils/test" "github.com/vmware/harbor/src/ui/config" "github.com/vmware/harbor/src/ui/proxy" ) @@ -44,14 +47,6 @@ import ( //var admin *usrInfo func init() { - if err := config.Init(); err != nil { - log.Fatalf("failed to initialize configurations: %v", err) - } - - if err := proxy.Init(); err != nil { - log.Fatalf("Failed to initialize the proxy: %v", err) - } - _, file, _, _ := runtime.Caller(1) apppath, _ := filepath.Abs(filepath.Dir(filepath.Join(file, ".."+string(filepath.Separator)))) beego.BConfig.WebConfig.Session.SessionOn = true @@ -64,15 +59,74 @@ func init() { beego.Router("/log_out", &CommonController{}, "get:LogOut") beego.Router("/reset", &CommonController{}, "post:ResetPassword") beego.Router("/userExists", &CommonController{}, "post:UserExists") - beego.Router("/sendEmail", &CommonController{}, "get:SendEmail") + beego.Router("/sendEmail", &CommonController{}, "get:SendResetEmail") beego.Router("/registryproxy/*", &RegistryProxy{}, "*:Handle") +} +func TestMain(m *testing.M) { + + rc := m.Run() + if rc != 0 { + os.Exit(rc) + } //Init user Info //admin = &usrInfo{adminName, adminPwd} } +// TestUserResettable +func TestUserResettable(t *testing.T) { + assert := assert.New(t) + DBAuthConfig := map[string]interface{}{ + common.AUTHMode: common.DBAuth, + common.CfgExpiration: 5, + common.TokenExpiration: 30, + } + + LDAPAuthConfig := map[string]interface{}{ + common.AUTHMode: common.LDAPAuth, + common.CfgExpiration: 5, + common.TokenExpiration: 30, + } + DBAuthAdminsvr, err := test.NewAdminserver(DBAuthConfig) + if err != nil { + panic(err) + } + LDAPAuthAdminsvr, err := test.NewAdminserver(LDAPAuthConfig) + if err != nil { + panic(err) + } + defer DBAuthAdminsvr.Close() + defer LDAPAuthAdminsvr.Close() + if err := config.InitByURL(LDAPAuthAdminsvr.URL); err != nil { + panic(err) + } + u1 := &models.User{ + UserID: 3, + Username: "daniel", + Email: "daniel@test.com", + } + u2 := &models.User{ + UserID: 1, + Username: "jack", + Email: "jack@test.com", + } + assert.False(isUserResetable(u1)) + assert.True(isUserResetable(u2)) + if err := config.InitByURL(DBAuthAdminsvr.URL); err != nil { + panic(err) + } + assert.True(isUserResetable(u1)) +} + // TestMain is a sample to run an endpoint test func TestAll(t *testing.T) { + if err := config.Init(); err != nil { + panic(err) + } + if err := proxy.Init(); err != nil { + panic(err) + } + assert := assert.New(t) // v := url.Values{} diff --git a/src/ui/router.go b/src/ui/router.go index 2c2078e9d..105226f65 100644 --- a/src/ui/router.go +++ b/src/ui/router.go @@ -65,7 +65,7 @@ func initRouters() { beego.Router("/log_out", &controllers.CommonController{}, "get:LogOut") beego.Router("/reset", &controllers.CommonController{}, "post:ResetPassword") beego.Router("/userExists", &controllers.CommonController{}, "post:UserExists") - beego.Router("/sendEmail", &controllers.CommonController{}, "get:SendEmail") + beego.Router("/sendEmail", &controllers.CommonController{}, "get:SendResetEmail") //API: beego.Router("/api/projects/:pid([0-9]+)/members/?:mid", &api.ProjectMemberAPI{})