From 388b793490a4513c2163ed54a9b4c4551c1e7253 Mon Sep 17 00:00:00 2001 From: Qiu Jian Date: Thu, 8 Nov 2018 01:36:19 +0800 Subject: [PATCH 1/2] =?UTF-8?q?=E4=BF=AE=E6=AD=A3=EF=BC=9A=E5=A2=9E?= =?UTF-8?q?=E5=8A=A0owner=E7=BA=A7=E5=88=AB=E6=9D=83=E9=99=90=EF=BC=8C?= =?UTF-8?q?=E6=94=BE=E7=BD=AE=E6=99=AE=E9=80=9A=E7=94=A8=E6=88=B7=E5=8F=AF?= =?UTF-8?q?=E4=BB=A5=E8=AE=BF=E9=97=AE=E7=B3=BB=E7=BB=9F=E8=B5=84=E6=BA=90?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- pkg/cloudcommon/db/db_dispatcher.go | 51 +++++---- pkg/cloudcommon/db/db_joint_dispatcher.go | 9 +- pkg/cloudcommon/db/jointbase.go | 6 +- pkg/cloudcommon/db/quotas/handler.go | 21 ++-- pkg/cloudcommon/db/rbac.go | 129 +++++++++++++++------- pkg/cloudcommon/db/resourcebase.go | 2 +- pkg/cloudcommon/policy/policy.go | 29 +++-- pkg/compute/models/cloudproviders.go | 8 ++ pkg/compute/usages/handler.go | 7 +- pkg/util/rbacutils/rabc.go | 44 ++++++-- pkg/util/rbacutils/rabc_test.go | 28 ++++- 11 files changed, 222 insertions(+), 112 deletions(-) diff --git a/pkg/cloudcommon/db/db_dispatcher.go b/pkg/cloudcommon/db/db_dispatcher.go index 9ea9967088..69a9813c35 100644 --- a/pkg/cloudcommon/db/db_dispatcher.go +++ b/pkg/cloudcommon/db/db_dispatcher.go @@ -24,6 +24,7 @@ import ( "yunion.io/x/onecloud/pkg/mcclient/modules" "yunion.io/x/onecloud/pkg/util/httputils" "yunion.io/x/onecloud/pkg/util/logclient" + "yunion.io/x/onecloud/pkg/util/rbacutils" ) type DBModelDispatcher struct { @@ -265,7 +266,10 @@ func listItemQueryFilters(manager IModelManager, ctx context.Context, q *sqlchem userCred mcclient.TokenCredential, query jsonutils.JSONObject) (*sqlchemy.SQuery, error) { if !jsonutils.QueryBoolean(query, "admin", false) { - q = manager.FilterByOwner(q, manager.GetOwnerId(userCred)) + ownerId := manager.GetOwnerId(userCred) + if len(ownerId) > 0 { + q = manager.FilterByOwner(q, ownerId) + } } q, err := manager.ListItemFilter(ctx, q, userCred, query) if err != nil { @@ -500,8 +504,13 @@ func (dispatcher *DBModelDispatcher) List(ctx context.Context, query jsonutils.J var isAllow bool if consts.IsRbacEnabled() { isAdmin := jsonutils.QueryBoolean(query, "admin", false) - isAllow = policy.PolicyManager.Allow(isAdmin, userCred, consts.GetServiceType(), - dispatcher.modelManager.KeywordPlural(), policy.PolicyActionList) + manager := dispatcher.modelManager + jointManager, ok := manager.(IJointModelManager) + if ok { + isAllow = isJointListRbacAllowed(jointManager, userCred, isAdmin) + } else { + isAllow = isListRbacAllowed(manager, userCred, isAdmin) + } } else { isAllow = dispatcher.modelManager.AllowListItems(ctx, userCred, query) } @@ -611,7 +620,7 @@ func (dispatcher *DBModelDispatcher) Get(ctx context.Context, idStr string, quer // log.Debugf("Get found %s", model) var isAllow bool if consts.IsRbacEnabled() { - isAllow = isRbacAllowed(dispatcher.modelManager, model, userCred, policy.PolicyActionGet) + isAllow = isObjectRbacAllowed(dispatcher.modelManager, model, userCred, policy.PolicyActionGet) } else { isAllow = model.AllowGetDetails(ctx, userCred, query) } @@ -642,7 +651,7 @@ func (dispatcher *DBModelDispatcher) GetSpecific(ctx context.Context, idStr stri var isAllow bool if consts.IsRbacEnabled() { - isAllow = isRbacAllowed(dispatcher.modelManager, model, userCred, policy.PolicyActionGet, spec) + isAllow = isObjectRbacAllowed(dispatcher.modelManager, model, userCred, policy.PolicyActionGet, spec) } else { funcName := fmt.Sprintf("AllowGetDetails%s", specCamel) @@ -686,21 +695,21 @@ func (dispatcher *DBModelDispatcher) GetSpecific(ctx context.Context, idStr stri } } -func fetchOwnerProjectId(ctx context.Context, userCred mcclient.TokenCredential, data jsonutils.JSONObject) (string, error) { +func fetchOwnerProjectId(ctx context.Context, manager IModelManager, userCred mcclient.TokenCredential, data jsonutils.JSONObject) (string, error) { var projId string if data != nil { - projId, _ = data.GetString("project") - if len(projId) == 0 { - projId, _ = data.GetString("tenant") - } + projId = jsonutils.GetAnyString(data, []string{"project", "tenant", "project_id", "tenant_id"}) } if len(projId) == 0 { - return userCred.GetProjectId(), nil + return manager.GetOwnerId(userCred), nil } var isAllow bool if consts.IsRbacEnabled() { - isAllow = policy.PolicyManager.Allow(true, userCred, + result := policy.PolicyManager.Allow(true, userCred, consts.GetServiceType(), policy.PolicyDelegation, "") + if result == rbacutils.Allow { + isAllow = true + } } else { isAllow = userCred.IsSystemAdmin() } @@ -798,7 +807,7 @@ func doCreateItem(manager IModelManager, ctx context.Context, userCred mcclient. func (dispatcher *DBModelDispatcher) Create(ctx context.Context, query jsonutils.JSONObject, data jsonutils.JSONObject, ctxId string) (jsonutils.JSONObject, error) { userCred := fetchUserCredential(ctx) - ownerProjId, err := fetchOwnerProjectId(ctx, userCred, data) + ownerProjId, err := fetchOwnerProjectId(ctx, dispatcher.modelManager, userCred, data) if err != nil { return nil, httperrors.NewGeneralError(err) } @@ -821,7 +830,7 @@ func (dispatcher *DBModelDispatcher) Create(ctx context.Context, query jsonutils var isAllow bool if consts.IsRbacEnabled() { - isAllow = isRbacAllowed(dispatcher.modelManager, nil, userCred, policy.PolicyActionCreate) + isAllow = isClassActionRbacAllowed(dispatcher.modelManager, userCred, ownerProjId, policy.PolicyActionCreate) } else { isAllow = dispatcher.modelManager.AllowCreateItem(ctx, userCred, query, data) } @@ -864,7 +873,7 @@ func expandMultiCreateParams(data jsonutils.JSONObject, count int) ([]jsonutils. func (dispatcher *DBModelDispatcher) BatchCreate(ctx context.Context, query jsonutils.JSONObject, data jsonutils.JSONObject, count int, ctxId string) ([]modules.SubmitResult, error) { userCred := fetchUserCredential(ctx) - ownerProjId, err := fetchOwnerProjectId(ctx, userCred, data) + ownerProjId, err := fetchOwnerProjectId(ctx, dispatcher.modelManager, userCred, data) if err != nil { return nil, httperrors.NewGeneralError(err) } @@ -885,7 +894,7 @@ func (dispatcher *DBModelDispatcher) BatchCreate(ctx context.Context, query json var isAllow bool if consts.IsRbacEnabled() { - isAllow = isRbacAllowed(dispatcher.modelManager, nil, userCred, policy.PolicyActionCreate) + isAllow = isClassActionRbacAllowed(dispatcher.modelManager, userCred, ownerProjId, policy.PolicyActionCreate) } else { isAllow = dispatcher.modelManager.AllowCreateItem(ctx, userCred, query, data) } @@ -933,7 +942,7 @@ func (dispatcher *DBModelDispatcher) BatchCreate(ctx context.Context, query json func (dispatcher *DBModelDispatcher) PerformClassAction(ctx context.Context, action string, query jsonutils.JSONObject, data jsonutils.JSONObject) (jsonutils.JSONObject, error) { userCred := fetchUserCredential(ctx) - ownerProjId, err := fetchOwnerProjectId(ctx, userCred, data) + ownerProjId, err := fetchOwnerProjectId(ctx, dispatcher.modelManager, userCred, data) if err != nil { return nil, httperrors.NewGeneralError(err) } @@ -952,7 +961,7 @@ func (dispatcher *DBModelDispatcher) PerformClassAction(ctx context.Context, act var isAllow bool if consts.IsRbacEnabled() { - isAllow = isRbacAllowed(manager, nil, userCred, policy.PolicyActionPerform, action) + isAllow = isClassActionRbacAllowed(manager, userCred, ownerProjId, policy.PolicyActionPerform, action) } else { isAllow = manager.AllowPerformCheckCreateData(ctx, userCred, query, data) } @@ -1030,7 +1039,7 @@ func objectPerformAction(dispatcher *DBModelDispatcher, model IModel, modelValue var isAllow bool if consts.IsRbacEnabled() { - isAllow = isRbacAllowed(dispatcher.modelManager, model, userCred, policy.PolicyActionPerform, action) + isAllow = isObjectRbacAllowed(dispatcher.modelManager, model, userCred, policy.PolicyActionPerform, action) } else { allowFuncName := "Allow" + funcName allowFuncValue := modelValue.MethodByName(allowFuncName) @@ -1142,7 +1151,7 @@ func (dispatcher *DBModelDispatcher) Update(ctx context.Context, idStr string, q var isAllow bool if consts.IsRbacEnabled() { - isAllow = isRbacAllowed(dispatcher.modelManager, model, userCred, policy.PolicyActionUpdate) + isAllow = isObjectRbacAllowed(dispatcher.modelManager, model, userCred, policy.PolicyActionUpdate) } else { isAllow = model.AllowUpdateItem(ctx, userCred) } @@ -1178,7 +1187,7 @@ func deleteItem(manager IModelManager, model IModel, ctx context.Context, userCr var isAllow bool if consts.IsRbacEnabled() { - isAllow = isRbacAllowed(manager, model, userCred, policy.PolicyActionDelete) + isAllow = isObjectRbacAllowed(manager, model, userCred, policy.PolicyActionDelete) } else { isAllow = model.AllowDeleteItem(ctx, userCred, query, data) } diff --git a/pkg/cloudcommon/db/db_joint_dispatcher.go b/pkg/cloudcommon/db/db_joint_dispatcher.go index 638e810e65..aa2080a40f 100644 --- a/pkg/cloudcommon/db/db_joint_dispatcher.go +++ b/pkg/cloudcommon/db/db_joint_dispatcher.go @@ -103,8 +103,7 @@ func (dispatcher *DBJointModelDispatcher) _listJoint(ctx context.Context, userCr var isAllow bool if consts.IsRbacEnabled() { isAdmin := jsonutils.QueryBoolean(queryDict, "admin", false) - isAllow = policy.PolicyManager.Allow(isAdmin, userCred, consts.GetServiceType(), - dispatcher.JointModelManager().KeywordPlural(), policy.PolicyActionList) + isAllow = isJointListRbacAllowed(dispatcher.JointModelManager(), userCred, isAdmin) } else { isAllow = dispatcher.JointModelManager().AllowListDescendent(ctx, userCred, ctxModel, queryDict) } @@ -147,7 +146,7 @@ func (dispatcher *DBJointModelDispatcher) Get(ctx context.Context, id1 string, i } var isAllow bool if consts.IsRbacEnabled() { - isAllow = isJointRbacAllowed(dispatcher.JointModelManager(), item, userCred, policy.PolicyActionGet) + isAllow = isJointObjectRbacAllowed(dispatcher.JointModelManager(), item, userCred, policy.PolicyActionGet) } else { isAllow = item.AllowGetJointDetails(ctx, userCred, query, item) } @@ -161,7 +160,7 @@ func attachItems(dispatcher *DBJointModelDispatcher, master IStandaloneModel, sl if !dispatcher.JointModelManager().AllowAttach(ctx, userCred, master, slave) { return nil, httperrors.NewForbiddenError("Not allow to attach") } - ownerProjId, err := fetchOwnerProjectId(ctx, userCred, data) + ownerProjId, err := fetchOwnerProjectId(ctx, dispatcher.JointModelManager(), userCred, data) dataDict, ok := data.(*jsonutils.JSONDict) if !ok { return nil, fmt.Errorf("body not a json dict") @@ -218,7 +217,7 @@ func (dispatcher *DBJointModelDispatcher) Update(ctx context.Context, id1 string var isAllow bool if consts.IsRbacEnabled() { - isAllow = isJointRbacAllowed(dispatcher.JointModelManager(), item, userCred, policy.PolicyActionUpdate) + isAllow = isJointObjectRbacAllowed(dispatcher.JointModelManager(), item, userCred, policy.PolicyActionUpdate) } else { isAllow = item.AllowUpdateJointItem(ctx, userCred, item) } diff --git a/pkg/cloudcommon/db/jointbase.go b/pkg/cloudcommon/db/jointbase.go index 06dfefce28..6dcf2e3c97 100644 --- a/pkg/cloudcommon/db/jointbase.go +++ b/pkg/cloudcommon/db/jointbase.go @@ -38,7 +38,11 @@ func NewJointResourceBaseManager(dt interface{}, tableName string, keyword strin log.Errorf(msg) panic(msg) } - return SJointResourceBaseManager{SResourceBaseManager: NewResourceBaseManager(dt, tableName, keyword, keywordPlural), _master: master, _slave: slave} + return SJointResourceBaseManager{ + SResourceBaseManager: NewResourceBaseManager(dt, tableName, keyword, keywordPlural), + _master: master, + _slave: slave, + } } func (manager *SJointResourceBaseManager) GetMasterManager() IStandaloneModelManager { diff --git a/pkg/cloudcommon/db/quotas/handler.go b/pkg/cloudcommon/db/quotas/handler.go index 3544e780a1..6d16c8c708 100644 --- a/pkg/cloudcommon/db/quotas/handler.go +++ b/pkg/cloudcommon/db/quotas/handler.go @@ -16,6 +16,7 @@ import ( "yunion.io/x/onecloud/pkg/cloudcommon/policy" "yunion.io/x/onecloud/pkg/httperrors" "yunion.io/x/onecloud/pkg/mcclient/auth" + "yunion.io/x/onecloud/pkg/util/rbacutils" ) var _manager *SQuotaManager @@ -78,8 +79,9 @@ func getQuotaHanlder(ctx context.Context, w http.ResponseWriter, r *http.Request if len(projectId) == 0 { projectId = userCred.GetProjectId() if consts.IsRbacEnabled() { - if !policy.PolicyManager.Allow(false, userCred, consts.GetServiceType(), - "quotas", policy.PolicyActionGet) { + result := policy.PolicyManager.Allow(false, userCred, consts.GetServiceType(), + "quotas", policy.PolicyActionGet) + if result == rbacutils.Deny { httperrors.ForbiddenError(w, "not allow to get quota") return } @@ -87,8 +89,9 @@ func getQuotaHanlder(ctx context.Context, w http.ResponseWriter, r *http.Request } else { isAllow := false if consts.IsRbacEnabled() { - isAllow = policy.PolicyManager.Allow(true, userCred, consts.GetServiceType(), + result := policy.PolicyManager.Allow(true, userCred, consts.GetServiceType(), policy.PolicyDelegation, policy.PolicyActionGet) + isAllow = result == rbacutils.Allow } else { isAllow = userCred.IsSystemAdmin() } @@ -97,8 +100,8 @@ func getQuotaHanlder(ctx context.Context, w http.ResponseWriter, r *http.Request return } if consts.IsRbacEnabled() { - if !policy.PolicyManager.Allow(true, userCred, consts.GetServiceType(), - "quotas", policy.PolicyActionGet) { + if policy.PolicyManager.Allow(true, userCred, consts.GetServiceType(), + "quotas", policy.PolicyActionGet) != rbacutils.Allow { httperrors.ForbiddenError(w, "not allow to query quota") return } @@ -135,7 +138,7 @@ func setQuotaHanlder(ctx context.Context, w http.ResponseWriter, r *http.Request var isAllow bool if consts.IsRbacEnabled() { isAllow = policy.PolicyManager.Allow(true, userCred, consts.GetServiceType(), - "quotas", policy.PolicyActionUpdate) + "quotas", policy.PolicyActionUpdate) == rbacutils.Allow } else { isAllow = userCred.IsSystemAdmin() } @@ -198,7 +201,7 @@ func checkQuotaHanlder(ctx context.Context, w http.ResponseWriter, r *http.Reque isAllow := false if consts.IsRbacEnabled() { isAllow = policy.PolicyManager.Allow(true, userCred, consts.GetServiceType(), - policy.PolicyDelegation, policy.PolicyActionGet) + policy.PolicyDelegation, policy.PolicyActionGet) == rbacutils.Allow } else { isAllow = userCred.IsSystemAdmin() } @@ -207,8 +210,8 @@ func checkQuotaHanlder(ctx context.Context, w http.ResponseWriter, r *http.Reque return } if consts.IsRbacEnabled() { - if !policy.PolicyManager.Allow(true, userCred, consts.GetServiceType(), - "quotas", policy.PolicyActionGet) { + if policy.PolicyManager.Allow(true, userCred, consts.GetServiceType(), + "quotas", policy.PolicyActionGet) != rbacutils.Allow { httperrors.ForbiddenError(w, "not allow to query quota") return } diff --git a/pkg/cloudcommon/db/rbac.go b/pkg/cloudcommon/db/rbac.go index f3b91e3617..fb112db42d 100644 --- a/pkg/cloudcommon/db/rbac.go +++ b/pkg/cloudcommon/db/rbac.go @@ -1,57 +1,102 @@ package db import ( + "yunion.io/x/log" + "yunion.io/x/onecloud/pkg/cloudcommon/consts" "yunion.io/x/onecloud/pkg/cloudcommon/policy" "yunion.io/x/onecloud/pkg/mcclient" + "yunion.io/x/onecloud/pkg/util/rbacutils" ) -func isRbacAllowed(manager IModelManager, model IModel, userCred mcclient.TokenCredential, action string, extra ...string) bool { - var isAllow bool - var isAdmin bool - if model == nil { - _, ok := manager.(IVirtualModelManager) - if ok { - isAdmin = false - } - } else { - virtModel, ok := model.(IVirtualModel) - if ok { - if virtModel.IsOwner(userCred) { - isAdmin = false - } else { - isAdmin = true - } - } else { - isAdmin = true - } - } - if !isAdmin { - isAllow = policy.PolicyManager.Allow(false, userCred, consts.GetServiceType(), - manager.KeywordPlural(), action, extra...) - } - if !isAllow { - isAllow = policy.PolicyManager.Allow(true, userCred, consts.GetServiceType(), - manager.KeywordPlural(), action, extra...) - } - return isAllow +func isListRbacAllowed(manager IModelManager, userCred mcclient.TokenCredential, isAdminMode bool) bool { + return isListRbacAllowedInternal(manager, manager.KeywordPlural(), userCred, isAdminMode) } -func isJointRbacAllowed(manager IJointModelManager, item IJointModel, userCred mcclient.TokenCredential, action string, extra ...string) bool { - isAllow := false - isAdmin := true - master := item.Master() - virtualMaster, ok := master.(IVirtualModel) - if ok && virtualMaster.IsOwner(userCred) { - isAdmin = false +func isListRbacAllowedInternal(manager IModelManager, resource string, userCred mcclient.TokenCredential, isAdminMode bool) bool { + log.Debugf("%s %s", manager.KeywordPlural(), resource) + var requireAdmin bool + ownerId := manager.GetOwnerId(userCred) + if len(ownerId) > 0 { + if isAdminMode { + requireAdmin = true + } else { + requireAdmin = false + } + } else { + requireAdmin = true } - if !isAdmin { - isAllow = policy.PolicyManager.Allow(false, userCred, consts.GetServiceType(), - manager.KeywordPlural(), action, extra...) + result := policy.PolicyManager.Allow(false, userCred, consts.GetServiceType(), + resource, policy.PolicyActionList) + log.Debugf("allow list for non-admin %s %v ownerId: %s", result, requireAdmin, ownerId) + if (result == rbacutils.OwnerAllow && !requireAdmin) || (result == rbacutils.Allow && requireAdmin) { + return true } - if !isAllow { - isAllow = policy.PolicyManager.Allow(true, userCred, consts.GetServiceType(), + result = policy.PolicyManager.Allow(true, userCred, consts.GetServiceType(), + resource, policy.PolicyActionList) + log.Debugf("allow list for admin %s %v ownerId: %s", result, requireAdmin, ownerId) + return result == rbacutils.Allow +} + +func isJointListRbacAllowed(manager IJointModelManager, userCred mcclient.TokenCredential, isAdminMode bool) bool { + return isListRbacAllowedInternal(manager.GetMasterManager(), manager.KeywordPlural(), userCred, isAdminMode) +} + +func isClassActionRbacAllowed(manager IModelManager, userCred mcclient.TokenCredential, ownerProjId string, action string, extra...string) bool { + var requireAdmin bool + ownerId := manager.GetOwnerId(userCred) + if len(ownerId) > 0 { + if ownerProjId == ownerId { + requireAdmin = false + } else { + requireAdmin = true + } + } else { + requireAdmin = true + } + if !requireAdmin { + result := policy.PolicyManager.Allow(false, userCred, consts.GetServiceType(), + manager.KeywordPlural(), action, extra...) + if result == rbacutils.Allow || result == rbacutils.OwnerAllow { + return true + } + } + result := policy.PolicyManager.Allow(false, userCred, consts.GetServiceType(), + manager.KeywordPlural(), action, extra...) + return result == rbacutils.Allow +} + +func isObjectRbacAllowed(manager IModelManager, model IModel, userCred mcclient.TokenCredential, action string, extra ...string) bool { + var requireAdmin bool + var isOwner bool + + ownerId := model.GetModelManager().GetOwnerId(userCred) + + if len(ownerId) > 0 { + objOwnerId := model.GetOwnerProjectId() + if ownerId == objOwnerId { + isOwner = true + requireAdmin = false + } else { + isOwner = false + requireAdmin = true + } + } else { + requireAdmin = true + } + + if !requireAdmin { + result := policy.PolicyManager.Allow(false, userCred, consts.GetServiceType(), manager.KeywordPlural(), action, extra...) + if result == rbacutils.Allow || (result == rbacutils.OwnerAllow && isOwner) { + return true + } } - return isAllow + result := policy.PolicyManager.Allow(true, userCred, consts.GetServiceType(), + manager.KeywordPlural(), action, extra...) + return result == rbacutils.Allow +} + +func isJointObjectRbacAllowed(manager IJointModelManager, item IJointModel, userCred mcclient.TokenCredential, action string, extra ...string) bool { + return isObjectRbacAllowed(manager, item.Master(), userCred, action, extra...) } diff --git a/pkg/cloudcommon/db/resourcebase.go b/pkg/cloudcommon/db/resourcebase.go index 7bf0f32cb7..74451aa69a 100644 --- a/pkg/cloudcommon/db/resourcebase.go +++ b/pkg/cloudcommon/db/resourcebase.go @@ -37,7 +37,7 @@ func (manager *SResourceBaseManager) RawQuery(fields ...string) *sqlchemy.SQuery } func (manager *SResourceBaseManager) DoCreate(ctx context.Context, userCred mcclient.TokenCredential, kwargs jsonutils.JSONObject, data jsonutils.JSONObject, realManager IModelManager) (IModel, error) { - ownerProjId, err := fetchOwnerProjectId(ctx, userCred, kwargs) + ownerProjId, err := fetchOwnerProjectId(ctx, manager, userCred, kwargs) if err != nil { return nil, err } diff --git a/pkg/cloudcommon/policy/policy.go b/pkg/cloudcommon/policy/policy.go index 745e1b0bbc..8505a7555f 100644 --- a/pkg/cloudcommon/policy/policy.go +++ b/pkg/cloudcommon/policy/policy.go @@ -135,7 +135,7 @@ func (manager *SPolicyManager) sync() { time.AfterFunc(PolicyRefreshInterval, manager.sync) } -func (manager *SPolicyManager) Allow(isAdmin bool, userCred mcclient.TokenCredential, service string, resource string, action string, extra ...string) bool { +func (manager *SPolicyManager) Allow(isAdmin bool, userCred mcclient.TokenCredential, service string, resource string, action string, extra ...string) rbacutils.TRbacResult { var policies map[string]rbacutils.SRbacPolicy if isAdmin { policies = manager.adminPolicies @@ -144,29 +144,30 @@ func (manager *SPolicyManager) Allow(isAdmin bool, userCred mcclient.TokenCreden } if policies == nil { log.Warningf("no policies fetched") - return false + return rbacutils.Deny } userCredJson := userCred.ToJson() - // log.Debugf("%s", userCredJson) + currentPriv := rbacutils.Deny for _, p := range policies { - if p.Allow(userCredJson, service, resource, action, extra...) { - return true + result := p.Allow(userCredJson, service, resource, action, extra...) + if result.IsHigherPrivilege(currentPriv) { + currentPriv = result } } - return false + return currentPriv } -func (manager *SPolicyManager) explainPolicy(userCred mcclient.TokenCredential, policyReq jsonutils.JSONObject) (bool, error) { +func (manager *SPolicyManager) explainPolicy(userCred mcclient.TokenCredential, policyReq jsonutils.JSONObject) (rbacutils.TRbacResult, error) { policySeq, err := policyReq.GetArray() if err != nil { - return false, httperrors.NewInputParameterError("invalid format") + return rbacutils.Deny, httperrors.NewInputParameterError("invalid format") } isAdmin, _ := policySeq[0].Bool() if !consts.IsRbacEnabled() { if !isAdmin || (isAdmin && userCred.IsSystemAdmin()) { - return true, nil + return rbacutils.Allow, nil } else { - return false, httperrors.NewForbiddenError("operation not allowed") + return rbacutils.Deny, httperrors.NewForbiddenError("operation not allowed") } } service := rbacutils.WILD_MATCH @@ -198,15 +199,11 @@ func (manager *SPolicyManager) ExplainRpc(userCred mcclient.TokenCredential, par } ret := jsonutils.NewDict() for key, policyReq := range paramDict { - allow, err := manager.explainPolicy(userCred, policyReq) + result, err := manager.explainPolicy(userCred, policyReq) if err != nil { return nil, err } - if allow { - ret.Add(jsonutils.JSONTrue, key) - } else { - ret.Add(jsonutils.JSONFalse, key) - } + ret.Add(jsonutils.NewString(string(result)), key) } return ret, nil } diff --git a/pkg/compute/models/cloudproviders.go b/pkg/compute/models/cloudproviders.go index 27241408b9..d8d1977f5e 100644 --- a/pkg/compute/models/cloudproviders.go +++ b/pkg/compute/models/cloudproviders.go @@ -75,6 +75,14 @@ type SCloudprovider struct { Provider string `width:"64" charset:"ascii" list:"admin" create:"admin_required"` } +func (manager *SCloudproviderManager) GetOwnerId(userCred mcclient.IIdentityProvider) string { + return userCred.GetProjectId() +} + +func (self *SCloudprovider) GetOwnerProjectId() string { + return self.ProjectId +} + func (self *SCloudprovider) ValidateDeleteCondition(ctx context.Context) error { if self.Enabled { return httperrors.NewInvalidStatusError("provider is enabled") diff --git a/pkg/compute/usages/handler.go b/pkg/compute/usages/handler.go index 0726931503..ae74134055 100644 --- a/pkg/compute/usages/handler.go +++ b/pkg/compute/usages/handler.go @@ -20,6 +20,7 @@ import ( "yunion.io/x/onecloud/pkg/httperrors" "yunion.io/x/onecloud/pkg/mcclient" "yunion.io/x/onecloud/pkg/mcclient/auth" + "yunion.io/x/onecloud/pkg/util/rbacutils" ) type Usage map[string]interface{} @@ -271,7 +272,7 @@ func ReportGeneralUsage(userCred mcclient.TokenCredential, rangeObj db.IStandalo if consts.IsRbacEnabled() { if policy.PolicyManager.Allow(true, userCred, consts.GetServiceType(), - "usages", policy.PolicyActionGet) { + "usages", policy.PolicyActionGet) == rbacutils.Allow { isAdmin = true } } else { @@ -286,8 +287,8 @@ func ReportGeneralUsage(userCred mcclient.TokenCredential, rangeObj db.IStandalo } if consts.IsRbacEnabled() { - if !policy.PolicyManager.Allow(false, userCred, consts.GetServiceType(), - "usages", policy.PolicyActionGet) { + if policy.PolicyManager.Allow(false, userCred, consts.GetServiceType(), + "usages", policy.PolicyActionGet) == rbacutils.Deny { err = httperrors.NewForbiddenError("not allow to get usages") return } diff --git a/pkg/util/rbacutils/rabc.go b/pkg/util/rbacutils/rabc.go index a586a42dcf..ebf1fb911c 100644 --- a/pkg/util/rbacutils/rabc.go +++ b/pkg/util/rbacutils/rabc.go @@ -12,10 +12,32 @@ type TRbacResult string const ( WILD_MATCH = "*" + Allow = TRbacResult("allow") + OwnerAllow = TRbacResult("owner") Deny = TRbacResult("deny") ) +func (result TRbacResult) IsHigherPrivilege(r2 TRbacResult) bool { + switch result { + case Allow: + if r2 == Allow { + return false + } else { + return true + } + case OwnerAllow: + if r2 == Deny { + return true + } else { + return false + } + case Deny: + return false + } + return false +} + type SRbacPolicy struct { Condition string IsAdmin bool @@ -185,11 +207,14 @@ func decode(rules jsonutils.JSONObject, decodeRule SRbacRule, level int) ([]SRba case *jsonutils.JSONString: ruleJsonStr := rules.(*jsonutils.JSONString) ruleStr, _ := ruleJsonStr.GetString() - if ruleStr == string(Allow) { + switch ruleStr { + case string(Allow): decodeRule.Result = Allow - } else if ruleStr == string(Deny) { + case string(OwnerAllow): + decodeRule.Result = OwnerAllow + case string(Deny): decodeRule.Result = Deny - } else { + default: return nil, fmt.Errorf("unsupported rule string %s", ruleStr) } return []SRbacRule{decodeRule}, nil @@ -318,23 +343,20 @@ func (policy *SRbacPolicy) Explain(request [][]string) [][]string { return output } -func (policy *SRbacPolicy) Allow(userCred jsonutils.JSONObject, service, resource, action string, extra ...string) bool { +func (policy *SRbacPolicy) Allow(userCred jsonutils.JSONObject, service, resource, action string, extra ...string) TRbacResult { if len(policy.Condition) > 0 { match, err := conditionparser.Eval(policy.Condition, userCred) if err != nil { log.Errorf("eval condition %s fail %s", policy.Condition, err) - return false + return Deny } if !match { - return false + return Deny } } rule := policy.GetMatchRule(service, resource, action, extra...) if rule == nil { - return false + return Deny } - if rule.Result == Deny { - return false - } - return true + return rule.Result } diff --git a/pkg/util/rbacutils/rabc_test.go b/pkg/util/rbacutils/rabc_test.go index 0edcb3ff7b..765127dc74 100644 --- a/pkg/util/rbacutils/rabc_test.go +++ b/pkg/util/rbacutils/rabc_test.go @@ -191,7 +191,7 @@ func TestSRbacPolicy_Allow(t *testing.T) { cases := []struct { policy string ops []string - want bool + want TRbacResult }{ { `{ @@ -202,12 +202,12 @@ func TestSRbacPolicy_Allow(t *testing.T) { } }`, []string{"compute", "servers", "list"}, - true, + Allow, }, { `{"is_admin":"false","policy":{"*":{"*":{"*":"allow","delete":"deny"}}}}`, []string{"compute", "servers", "delete"}, - false, + Deny, }, } @@ -280,3 +280,25 @@ func TestSRabcPolicy_Explain(t *testing.T) { t.Logf("%#v", output) } + +func TestTRbacResult_IsHigherPrivilege(t *testing.T) { + cases := []struct { + t1 TRbacResult + t2 TRbacResult + want bool + } { + {Allow, Allow, false}, + {Allow, OwnerAllow, true}, + {Deny, Deny, false}, + {Deny, OwnerAllow, false}, + {Deny, Allow, false}, + {OwnerAllow, Allow, false}, + } + + for _, c := range cases { + got := c.t1.IsHigherPrivilege(c.t2) + if got != c.want { + t.Errorf("%s IsHigherPrivilege %s want %v got %v", c.t1, c.t2, c.want, got) + } + } +} \ No newline at end of file From 1d0ad17e360de5bf7ac772d4e721985453be9708 Mon Sep 17 00:00:00 2001 From: Qiu Jian Date: Thu, 8 Nov 2018 11:39:48 +0800 Subject: [PATCH 2/2] fix format --- pkg/cloudcommon/db/rbac.go | 2 +- pkg/util/rbacutils/rabc_test.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/cloudcommon/db/rbac.go b/pkg/cloudcommon/db/rbac.go index fb112db42d..f19ce67698 100644 --- a/pkg/cloudcommon/db/rbac.go +++ b/pkg/cloudcommon/db/rbac.go @@ -42,7 +42,7 @@ func isJointListRbacAllowed(manager IJointModelManager, userCred mcclient.TokenC return isListRbacAllowedInternal(manager.GetMasterManager(), manager.KeywordPlural(), userCred, isAdminMode) } -func isClassActionRbacAllowed(manager IModelManager, userCred mcclient.TokenCredential, ownerProjId string, action string, extra...string) bool { +func isClassActionRbacAllowed(manager IModelManager, userCred mcclient.TokenCredential, ownerProjId string, action string, extra ...string) bool { var requireAdmin bool ownerId := manager.GetOwnerId(userCred) if len(ownerId) > 0 { diff --git a/pkg/util/rbacutils/rabc_test.go b/pkg/util/rbacutils/rabc_test.go index 765127dc74..56aeca8736 100644 --- a/pkg/util/rbacutils/rabc_test.go +++ b/pkg/util/rbacutils/rabc_test.go @@ -283,10 +283,10 @@ func TestSRabcPolicy_Explain(t *testing.T) { func TestTRbacResult_IsHigherPrivilege(t *testing.T) { cases := []struct { - t1 TRbacResult - t2 TRbacResult + t1 TRbacResult + t2 TRbacResult want bool - } { + }{ {Allow, Allow, false}, {Allow, OwnerAllow, true}, {Deny, Deny, false}, @@ -301,4 +301,4 @@ func TestTRbacResult_IsHigherPrivilege(t *testing.T) { t.Errorf("%s IsHigherPrivilege %s want %v got %v", c.t1, c.t2, c.want, got) } } -} \ No newline at end of file +}