fix: keystone idp join project roles check (#25653)

Co-authored-by: Qiu Jian <qiujian@yunionyun.com>

pkg/keystone/driver/utils/validate.go

@@ -49,8 +49,7 @@ func ValidateConfig(ctx context.Context, conf api.SIdpAttributeOptions, userCred
 		}
 		conf.DefaultRoleId = obj.GetId()
 	}
-	if len(conf.DefaultProjectId) > 0 && len(conf.DefaultRoleId) > 0 {
-		// validate policy
+	if len(conf.DefaultRoleId) > 0 {
 		err := models.ValidateJoinProjectRoles(userCred, conf.DefaultProjectId, []string{conf.DefaultRoleId})
 		if err != nil {
 			return conf, errors.Wrap(err, "ValidateJoinProjectRoles")

pkg/keystone/models/identity_provider.go

@@ -1303,6 +1303,7 @@ func (idp *SIdentityProvider) TryUserJoinProject(attrConf api.SIdpAttributeOptio
 	}
 
 	var targetProject *SProject
+	projectFromAttr := false
 	log.Debugf("userTryJoinProject resp %s proj %s", attrs, attrConf.ProjectAttribute)
 	if !consts.GetNonDefaultDomainProjects() {
 		// if non-default-domain-project is disabled, place new project in default domain
@@ -1325,6 +1326,9 @@ func (idp *SIdentityProvider) TryUserJoinProject(attrConf api.SIdpAttributeOptio
 					}
 				}
 			}
+			if targetProject != nil {
+				projectFromAttr = true
+			}
 		}
 	}
 	if targetProject == nil && len(attrConf.DefaultProjectId) > 0 {
@@ -1343,6 +1347,8 @@ func (idp *SIdentityProvider) TryUserJoinProject(attrConf api.SIdpAttributeOptio
 					targetRole, err := RoleManager.FetchRole("", roleName, domainId, "")
 					if err != nil {
 						log.Errorf("fetch role %s fail %s", roleName, err)
+					} else if err := validateIdpJoinRole(targetProject, targetRole, idpJoinAllowsSystemRole(projectFromAttr, true)); err != nil {
+						log.Errorf("skip role %s for idp %s: %s", roleName, idp.Name, err)
 					} else {
 						targetRoles = append(targetRoles, targetRole)
 					}
@@ -1353,6 +1359,8 @@ func (idp *SIdentityProvider) TryUserJoinProject(attrConf api.SIdpAttributeOptio
 			targetRole, err := RoleManager.FetchRoleById(attrConf.DefaultRoleId)
 			if err != nil {
 				log.Errorf("fetch default role %s fail %s", attrConf.DefaultRoleId, err)
+			} else if err := validateIdpJoinRole(targetProject, targetRole, idpJoinAllowsSystemRole(projectFromAttr, false)); err != nil {
+				log.Errorf("skip default role %s for idp %s: %s", targetRole.Name, idp.Name, err)
 			} else {
 				targetRoles = append(targetRoles, targetRole)
 			}

pkg/keystone/models/idp_join.go

@@ -0,0 +1,52 @@
+// Copyright 2019 Yunion
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package models
+
+import (
+	"time"
+
+	"yunion.io/x/pkg/errors"
+	"yunion.io/x/pkg/util/rbacscope"
+
+	"yunion.io/x/onecloud/pkg/httperrors"
+	"yunion.io/x/onecloud/pkg/keystone/options"
+	"yunion.io/x/onecloud/pkg/util/rbacutils"
+)
+
+// idpJoinAllowsSystemRole is true only when both project and role come from IdP configuration defaults.
+func idpJoinAllowsSystemRole(projectFromAttr, roleFromAttr bool) bool {
+	return !projectFromAttr && !roleFromAttr
+}
+
+func validateIdpJoinPolicies(assignPolicies rbacutils.TPolicyGroup, allowSystem bool) error {
+	if !allowSystem && assignPolicies.HighestScope() == rbacscope.ScopeSystem {
+		return errors.Wrap(httperrors.ErrNotSufficientPrivilege, "assigning roles requires higher privilege scope")
+	}
+	return nil
+}
+
+func validateIdpJoinRole(project *SProject, role *SRole, allowSystem bool) error {
+	_, assignPolicies, err := RolePolicyManager.GetMatchPolicyGroup2(false, []string{role.Id}, project.Id, "", time.Time{}, false)
+	if err != nil {
+		return errors.Wrap(err, "GetMatchPolicyGroup2")
+	}
+	if err := validateIdpJoinPolicies(assignPolicies, allowSystem); err != nil {
+		return err
+	}
+	if options.Options.ThreeAdminRoleSystem {
+		return threeMemberSystemValidatePolicies(GetDefaultAdminCred(), project.Id, assignPolicies)
+	}
+	return nil
+}

pkg/keystone/models/idp_join_test.go

@@ -0,0 +1,60 @@
+// Copyright 2019 Yunion
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package models
+
+import (
+	"testing"
+
+	"yunion.io/x/pkg/util/rbacscope"
+
+	"yunion.io/x/onecloud/pkg/util/rbacutils"
+)
+
+func TestIdpJoinAllowsSystemRole(t *testing.T) {
+	cases := []struct {
+		projectFromAttr bool
+		roleFromAttr    bool
+		want            bool
+	}{
+		{false, false, true},
+		{true, false, false},
+		{false, true, false},
+		{true, true, false},
+	}
+	for _, c := range cases {
+		got := idpJoinAllowsSystemRole(c.projectFromAttr, c.roleFromAttr)
+		if got != c.want {
+			t.Fatalf("projectFromAttr=%v roleFromAttr=%v got %v want %v", c.projectFromAttr, c.roleFromAttr, got, c.want)
+		}
+	}
+}
+
+func TestValidateIdpJoinPolicies(t *testing.T) {
+	sys := rbacutils.TPolicyGroup{rbacscope.ScopeSystem: {}}
+	if err := validateIdpJoinPolicies(sys, false); err == nil {
+		t.Fatal("expected error for system-scope role from attributes")
+	}
+	if err := validateIdpJoinPolicies(sys, true); err != nil {
+		t.Fatalf("configured default may assign system-scope role: %v", err)
+	}
+	proj := rbacutils.TPolicyGroup{rbacscope.ScopeProject: {}}
+	if err := validateIdpJoinPolicies(proj, false); err != nil {
+		t.Fatalf("project-scope role should be allowed: %v", err)
+	}
+	domain := rbacutils.TPolicyGroup{rbacscope.ScopeDomain: {}}
+	if err := validateIdpJoinPolicies(domain, false); err != nil {
+		t.Fatalf("domain-scope role should be allowed: %v", err)
+	}
+}