Merge pull request #3797 from reasonerjt/uaa-restriction

Disable user management features when auth mode is UAA.
This commit is contained in:
Daniel Jiang 2017-12-18 22:47:08 +08:00 committed by GitHub
commit 62cebbdb5d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 182 additions and 64 deletions

View File

@ -305,6 +305,7 @@ var currentUser *models.User
func TestGetUser(t *testing.T) { func TestGetUser(t *testing.T) {
queryUser := models.User{ queryUser := models.User{
Username: username, Username: username,
Email: "tester01@vmware.com",
} }
var err error var err error
currentUser, err = GetUser(queryUser) currentUser, err = GetUser(queryUser)

View File

@ -52,6 +52,11 @@ func GetUser(query models.User) (*models.User, error) {
queryParam = append(queryParam, query.ResetUUID) queryParam = append(queryParam, query.ResetUUID)
} }
if query.Email != "" {
sql += ` and email = ? `
queryParam = append(queryParam, query.Email)
}
var u []models.User var u []models.User
n, err := o.Raw(sql, queryParam).QueryRows(&u) n, err := o.Raw(sql, queryParam).QueryRows(&u)

View File

@ -21,6 +21,7 @@ import (
"strconv" "strconv"
"strings" "strings"
"github.com/vmware/harbor/src/common"
"github.com/vmware/harbor/src/common/dao" "github.com/vmware/harbor/src/common/dao"
"github.com/vmware/harbor/src/common/models" "github.com/vmware/harbor/src/common/models"
"github.com/vmware/harbor/src/common/utils/log" "github.com/vmware/harbor/src/common/utils/log"
@ -160,16 +161,9 @@ func (ua *UserAPI) List() {
// Put ... // Put ...
func (ua *UserAPI) Put() { func (ua *UserAPI) Put() {
ldapAdminUser := (ua.AuthMode == "ldap_auth" && ua.userID == 1 && ua.userID == ua.currentUserID) if !ua.modifiable() {
ua.RenderError(http.StatusForbidden, fmt.Sprintf("User with ID %d cannot be modified", ua.userID))
if !(ua.AuthMode == "db_auth" || ldapAdminUser) { return
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.")
}
} }
user := models.User{UserID: ua.userID} user := models.User{UserID: ua.userID}
ua.DecodeJSONReq(&user) ua.DecodeJSONReq(&user)
@ -210,7 +204,7 @@ func (ua *UserAPI) Put() {
// Post ... // Post ...
func (ua *UserAPI) Post() { func (ua *UserAPI) Post() {
if !(ua.AuthMode == "db_auth") { if !(ua.AuthMode == common.DBAuth) {
ua.CustomAbort(http.StatusForbidden, "") ua.CustomAbort(http.StatusForbidden, "")
} }
@ -258,22 +252,8 @@ func (ua *UserAPI) Post() {
// Delete ... // Delete ...
func (ua *UserAPI) Delete() { func (ua *UserAPI) Delete() {
if !ua.IsAdmin { if !ua.IsAdmin || ua.AuthMode != common.DBAuth || ua.userID == 1 || ua.currentUserID == ua.userID {
log.Warningf("current user, id: %d does not have admin role, can not remove user", ua.currentUserID) 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))
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())
return return
} }
@ -288,17 +268,9 @@ func (ua *UserAPI) Delete() {
// ChangePassword handles PUT to /api/users/{}/password // ChangePassword handles PUT to /api/users/{}/password
func (ua *UserAPI) ChangePassword() { func (ua *UserAPI) ChangePassword() {
ldapAdminUser := (ua.AuthMode == "ldap_auth" && ua.userID == 1 && ua.userID == ua.currentUserID) if !ua.modifiable() {
ua.RenderError(http.StatusForbidden, fmt.Sprintf("User with ID: %d is not modifiable", ua.userID))
if !(ua.AuthMode == "db_auth" || ldapAdminUser) { return
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.")
}
} }
var req passwordReq 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 // validate only validate when user register
func validate(user models.User) error { func validate(user models.User) error {

View File

@ -16,8 +16,11 @@ package api
import ( import (
"fmt" "fmt"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/vmware/harbor/src/common/api"
"github.com/vmware/harbor/tests/apitests/apilib" "github.com/vmware/harbor/tests/apitests/apilib"
"testing" "testing"
"github.com/astaxie/beego"
) )
var testUser0002ID, testUser0003ID int var testUser0002ID, testUser0003ID int
@ -430,3 +433,51 @@ func TestUsersDelete(t *testing.T) {
assert.Equal(200, code, "Delete test user status should be 200") 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())
}

View File

@ -26,15 +26,6 @@ import (
// 1.5 seconds // 1.5 seconds
const frozenTime time.Duration = 1500 * time.Millisecond 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) var lock = NewUserLock(frozenTime)
// AuthenticateHelper provides interface for user management in different auth modes. // AuthenticateHelper provides interface for user management in different auth modes.

View File

@ -61,12 +61,17 @@ var (
func Init() error { func Init() error {
//init key provider //init key provider
initKeyProvider() initKeyProvider()
adminServerURL := os.Getenv("ADMINSERVER_URL") adminServerURL := os.Getenv("ADMINSERVER_URL")
if len(adminServerURL) == 0 { if len(adminServerURL) == 0 {
adminServerURL = "http://adminserver" 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) log.Infof("initializing client for adminserver %s ...", adminServerURL)
authorizer := auth.NewSecretAuthorizer(secretCookieName, UISecret()) authorizer := auth.NewSecretAuthorizer(secretCookieName, UISecret())
AdminserverClient = client.NewClient(adminServerURL, authorizer) AdminserverClient = client.NewClient(adminServerURL, authorizer)

View File

@ -25,6 +25,7 @@ import (
"github.com/astaxie/beego" "github.com/astaxie/beego"
"github.com/beego/i18n" "github.com/beego/i18n"
"github.com/vmware/harbor/src/common"
"github.com/vmware/harbor/src/common/dao" "github.com/vmware/harbor/src/common/dao"
"github.com/vmware/harbor/src/common/models" "github.com/vmware/harbor/src/common/models"
"github.com/vmware/harbor/src/common/utils" "github.com/vmware/harbor/src/common/utils"
@ -101,8 +102,8 @@ func (cc *CommonController) UserExists() {
cc.ServeJSON() cc.ServeJSON()
} }
// SendEmail verifies the Email address and contact SMTP server to send reset password Email. // SendResetEmail verifies the Email address and contact SMTP server to send reset password Email.
func (cc *CommonController) SendEmail() { func (cc *CommonController) SendResetEmail() {
email := cc.GetString("email") email := cc.GetString("email")
@ -117,16 +118,21 @@ func (cc *CommonController) SendEmail() {
} }
queryUser := models.User{Email: email} queryUser := models.User{Email: email}
exist, err := dao.UserExists(queryUser, "email") u, err := dao.GetUser(queryUser)
if err != nil { 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.") cc.CustomAbort(http.StatusInternalServerError, "Internal error.")
} }
if !exist { if u == nil {
log.Debugf("email %s not found", email) log.Debugf("email %s not found", email)
cc.CustomAbort(http.StatusNotFound, "email_does_not_exist") 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() uuid := utils.GenerateRandomString()
user := models.User{ResetUUID: uuid, Email: email} user := models.User{ResetUUID: uuid, Email: email}
if err = dao.UpdateUserResetUUID(user); err != nil { if err = dao.UpdateUserResetUUID(user); err != nil {
@ -192,6 +198,7 @@ func (cc *CommonController) ResetPassword() {
queryUser := models.User{ResetUUID: resetUUID} queryUser := models.User{ResetUUID: resetUUID}
user, err := dao.GetUser(queryUser) user, err := dao.GetUser(queryUser)
if err != nil { if err != nil {
log.Errorf("Error occurred in GetUser: %v", err) log.Errorf("Error occurred in GetUser: %v", err)
cc.CustomAbort(http.StatusInternalServerError, "Internal error.") cc.CustomAbort(http.StatusInternalServerError, "Internal error.")
@ -201,6 +208,11 @@ func (cc *CommonController) ResetPassword() {
cc.CustomAbort(http.StatusBadRequest, "User does not exist") 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") password := cc.GetString("password")
if 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() { func init() {
//conf/app.conf -> os.Getenv("config_path") //conf/app.conf -> os.Getenv("config_path")
configPath := os.Getenv("CONFIG_PATH") configPath := os.Getenv("CONFIG_PATH")

View File

@ -22,11 +22,14 @@ import (
"testing" "testing"
"fmt" "fmt"
"os"
"strings" "strings"
"github.com/astaxie/beego" "github.com/astaxie/beego"
"github.com/stretchr/testify/assert" "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/config"
"github.com/vmware/harbor/src/ui/proxy" "github.com/vmware/harbor/src/ui/proxy"
) )
@ -44,14 +47,6 @@ import (
//var admin *usrInfo //var admin *usrInfo
func init() { 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) _, file, _, _ := runtime.Caller(1)
apppath, _ := filepath.Abs(filepath.Dir(filepath.Join(file, ".."+string(filepath.Separator)))) apppath, _ := filepath.Abs(filepath.Dir(filepath.Join(file, ".."+string(filepath.Separator))))
beego.BConfig.WebConfig.Session.SessionOn = true beego.BConfig.WebConfig.Session.SessionOn = true
@ -64,15 +59,74 @@ func init() {
beego.Router("/log_out", &CommonController{}, "get:LogOut") beego.Router("/log_out", &CommonController{}, "get:LogOut")
beego.Router("/reset", &CommonController{}, "post:ResetPassword") beego.Router("/reset", &CommonController{}, "post:ResetPassword")
beego.Router("/userExists", &CommonController{}, "post:UserExists") beego.Router("/userExists", &CommonController{}, "post:UserExists")
beego.Router("/sendEmail", &CommonController{}, "get:SendEmail") beego.Router("/sendEmail", &CommonController{}, "get:SendResetEmail")
beego.Router("/registryproxy/*", &RegistryProxy{}, "*:Handle") beego.Router("/registryproxy/*", &RegistryProxy{}, "*:Handle")
}
func TestMain(m *testing.M) {
rc := m.Run()
if rc != 0 {
os.Exit(rc)
}
//Init user Info //Init user Info
//admin = &usrInfo{adminName, adminPwd} //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 // TestMain is a sample to run an endpoint test
func TestAll(t *testing.T) { 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) assert := assert.New(t)
// v := url.Values{} // v := url.Values{}

View File

@ -65,7 +65,7 @@ func initRouters() {
beego.Router("/log_out", &controllers.CommonController{}, "get:LogOut") beego.Router("/log_out", &controllers.CommonController{}, "get:LogOut")
beego.Router("/reset", &controllers.CommonController{}, "post:ResetPassword") beego.Router("/reset", &controllers.CommonController{}, "post:ResetPassword")
beego.Router("/userExists", &controllers.CommonController{}, "post:UserExists") beego.Router("/userExists", &controllers.CommonController{}, "post:UserExists")
beego.Router("/sendEmail", &controllers.CommonController{}, "get:SendEmail") beego.Router("/sendEmail", &controllers.CommonController{}, "get:SendResetEmail")
//API: //API:
beego.Router("/api/projects/:pid([0-9]+)/members/?:mid", &api.ProjectMemberAPI{}) beego.Router("/api/projects/:pid([0-9]+)/members/?:mid", &api.ProjectMemberAPI{})