Merge pull request #9888 from ioito/hotfix/qx-optimized-saml-auth

fix(cloudid): optimized saml sso

pkg/apis/cloudid/cloudrole.go

@@ -27,6 +27,7 @@ type CloudroleListInput struct {
 	apis.StatusInfrasResourceBaseListInput
 
 	CloudaccountResourceListInput
+	CloudgroupResourceListInput
 }
 
 type CloudroleDetails struct {

pkg/cloudid/models/cloudaccount.go

@@ -383,6 +383,10 @@ func (manager *SCloudaccountManager) SyncCloudaccounts(ctx context.Context, user
 }
 
 func (self *SCloudaccount) StartSyncSamlProvidersTask(ctx context.Context, userCred mcclient.TokenCredential, parentTaskId string) error {
+	if self.SAMLAuth.IsFalse() {
+		log.Debugf("cloudaccount %s(%s) not enable saml auth, skip sycing saml provider", self.Name, self.Provider)
+		return nil
+	}
 	params := jsonutils.NewDict()
 	task, err := taskman.TaskManager.NewTask(ctx, "SyncSAMLProvidersTask", self, userCred, params, parentTaskId, "", nil)
 	if err != nil {
@@ -1635,9 +1639,14 @@ func (self *SCloudaccount) SyncSystemCloudpoliciesForCloud(ctx context.Context,
 	return nil
 }
 
-func (self *SCloudaccount) GetLocalUserCloudroles(userId, spId string) ([]SCloudrole, error) {
+func (self *SCloudaccount) GetLocalCloudroles(userId, groupId string, spId string, grouped bool) ([]SCloudrole, error) {
 	roles := []SCloudrole{}
-	q := CloudroleManager.Query().Equals("cloudaccount_id", self.Id).Equals("owner_id", userId).Equals("saml_provider_id", spId)
+	q := CloudroleManager.Query().Equals("cloudaccount_id", self.Id).Equals("saml_provider_id", spId)
+	if grouped {
+		q = q.Equals("cloudgroup_id", groupId)
+	} else {
+		q = q.Equals("owner_id", userId)
+	}
 	err := db.FetchModelObjects(CloudroleManager, q, &roles)
 	if err != nil {
 		return nil, errors.Wrapf(err, "db.FetchModelObjects")
@@ -1645,55 +1654,90 @@ func (self *SCloudaccount) GetLocalUserCloudroles(userId, spId string) ([]SCloud
 	return roles, nil
 }
 
-func (self *SCloudaccount) RegisterCloudrole(userId, spId string) (*SCloudrole, error) {
-	roles, err := self.GetLocalUserCloudroles(userId, spId)
+func (self *SCloudaccount) RegisterCloudroles(userId string, grouped bool, spId string) ([]SCloudrole, error) {
+	samlUsers, err := self.GetSamlusers()
 	if err != nil {
-		return nil, errors.Wrapf(err, "GetLocalUserCloudroles")
+		return nil, errors.Wrapf(err, "GetSamlusers")
 	}
-	if len(roles) > 0 {
-		return &roles[0], nil
+	ret := []SCloudrole{}
+	roleIds := []string{}
+	for i := range samlUsers {
+		if samlUsers[i].OwnerId == userId {
+			roles, err := self.GetLocalCloudroles(userId, samlUsers[i].CloudgroupId, spId, grouped)
+			if err != nil {
+				return nil, errors.Wrapf(err, "GetLocalUserCloudroles")
+			}
+			for i := range roles {
+				if !utils.IsInStringArray(roles[i].Id, roleIds) {
+					ret = append(ret, roles[i])
+					break
+				}
+			}
+			if len(roles) == 0 {
+				role := SCloudrole{}
+				role.SetModelManager(CloudroleManager, &role)
+				role.CloudaccountId = self.Id
+				role.SAMLProviderId = spId
+				if grouped {
+					group, err := CloudgroupManager.FetchById(samlUsers[i].CloudgroupId)
+					if err != nil {
+						return nil, errors.Wrapf(err, "CloudgroupManager.FetchById(%s)", samlUsers[i].CloudgroupId)
+					}
+					role.Name = stringutils2.GenerateRoleName(group.GetName())
+					role.CloudgroupId = group.GetId()
+				} else {
+					user, err := db.UserCacheManager.FetchById(userId)
+					if err != nil {
+						return nil, errors.Wrapf(err, "UserCacheManager.FetchById(%s)", userId)
+					}
+					role.Name = stringutils2.GenerateRoleName(user.GetName())
+					role.OwnerId = userId
+				}
+				role.Status = api.CLOUD_ROLE_STATUS_CREATING
+				role.DomainId = self.DomainId
+				err = CloudroleManager.TableSpec().Insert(context.TODO(), &role)
+				if err != nil {
+					return nil, errors.Wrapf(err, "Insert role")
+				}
+				ret = append(ret, role)
+			}
+		}
 	}
-	user, err := db.UserCacheManager.FetchById(userId)
-	if err != nil {
-		return nil, errors.Wrapf(err, "UserCacheManager.FetchById(%s)", userId)
-	}
-	role := &SCloudrole{}
-	role.SetModelManager(CloudroleManager, role)
-	role.CloudaccountId = self.Id
-	role.OwnerId = userId
-	role.SAMLProviderId = spId
-	role.Name = stringutils2.GenerateRoleName(user.GetName())
-	role.Status = api.CLOUD_ROLE_STATUS_CREATING
-	role.DomainId = self.DomainId
-	return role, CloudroleManager.TableSpec().Insert(context.TODO(), role)
+	return ret, nil
 }
 
-func (self *SCloudaccount) GetCloudrole(userId string) (*SCloudrole, error) {
+func (self *SCloudaccount) getCloudrolesForSync(userId string, grouped bool) ([]SCloudrole, error) {
 	sp, valid := self.IsSAMLProviderValid()
 	if !valid {
 		return nil, fmt.Errorf("SAMLProvider for account %s not ready", self.Id)
 	}
 
-	return self.RegisterCloudrole(userId, sp.Id)
+	return self.RegisterCloudroles(userId, grouped, sp.Id)
 }
 
-func (self *SCloudaccount) SyncRole(userId string) (*SCloudrole, error) {
-	role, err := self.GetCloudrole(userId)
+func (self *SCloudaccount) SyncRoles(userId string, grouped bool) ([]SCloudrole, error) {
+	roles, err := self.getCloudrolesForSync(userId, grouped)
 	if err != nil {
 		return nil, errors.Wrapf(err, "GetCloudrole")
 	}
 
-	err = role.SyncRoles()
-	if err != nil {
-		return nil, errors.Wrapf(err, "SyncRoles")
+	for i := range roles {
+		err = roles[i].SyncRoles()
+		if err != nil {
+			return nil, errors.Wrapf(err, "SyncRoles")
+		}
 	}
 
-	return role, nil
+	if len(roles) == 0 {
+		return nil, fmt.Errorf("not found any available roles")
+	}
+
+	return roles, nil
 }
 
 func (self *SCloudaccount) GetCloudroles() ([]SCloudrole, error) {
 	roles := []SCloudrole{}
-	q := CloudroleManager.Query()
+	q := CloudroleManager.Query().Equals("cloudaccount_id", self.Id)
 	err := db.FetchModelObjects(CloudroleManager, q, &roles)
 	if err != nil {
 		return nil, errors.Wrapf(err, "db.FetchModelObjects")

pkg/cloudid/models/cloudrole.go

@@ -41,6 +41,7 @@ type SCloudroleManager struct {
 	db.SExternalizedResourceBaseManager
 	SCloudaccountResourceBaseManager
 	SAMLProviderResourceBaseManager
+	SCloudgroupResourceBaseManager
 }
 
 var CloudroleManager *SCloudroleManager
@@ -62,6 +63,7 @@ type SCloudrole struct {
 	db.SExternalizedResourceBase
 	SCloudaccountResourceBase
 	SAMLProviderResourceBase
+	SCloudgroupResourceBase
 
 	Document *jsonutils.JSONDict `length:"long" charset:"ascii" list:"domain" update:"domain" create:"domain_required"`
 	OwnerId  string              `width:"128" charset:"ascii" index:"true" list:"user" nullable:"false" create:"optional"`
@@ -80,6 +82,11 @@ func (manager *SCloudroleManager) ListItemFilter(ctx context.Context, q *sqlchem
 		return nil, err
 	}
 
+	q, err = manager.SCloudgroupResourceBaseManager.ListItemFilter(ctx, q, userCred, query.CloudgroupResourceListInput)
+	if err != nil {
+		return nil, err
+	}
+
 	return q, nil
 }
 
@@ -161,10 +168,13 @@ func (self *SCloudrole) GetICloudrole() (cloudprovider.ICloudrole, error) {
 	if err != nil {
 		return nil, errors.Wrapf(err, "GetSAMLProvider")
 	}
-	for i := 0; i < 10; i++ {
+	for {
 		_, err := provider.GetICloudroleByName(self.Name)
-		if err != nil && errors.Cause(err) == cloudprovider.ErrNotFound {
-			break
+		if err != nil {
+			if errors.Cause(err) == cloudprovider.ErrNotFound {
+				break
+			}
+			return nil, errors.Wrapf(err, "GetICloudroleByName(%s)", self.Name)
 		}
 		info := strings.Split(self.Name, "-")
 		num, err := strconv.Atoi(info[len(info)-1])

pkg/cloudid/saml/providers/aliyun/driver.go

@@ -52,7 +52,7 @@ func (d *SAliyunSAMLDriver) GetIdpInitiatedLoginData(ctx context.Context, userCr
 		return data, httperrors.NewResourceNotReadyError("SAMLProvider for account %s not ready", account.Id)
 	}
 
-	role, err := account.SyncRole(userCred.GetUserId())
+	roles, err := account.SyncRoles(userCred.GetUserId(), true)
 	if err != nil {
 		return data, httperrors.NewGeneralError(errors.Wrapf(err, "SyncRole"))
 	}
@@ -61,7 +61,7 @@ func (d *SAliyunSAMLDriver) GetIdpInitiatedLoginData(ctx context.Context, userCr
 	data.NameIdFormat = samlutils.NAME_ID_FORMAT_PERSISTENT
 	data.AudienceRestriction = sp.GetEntityId()
 	for k, v := range map[string]string{
-		"https://www.aliyun.com/SAML-Role/Attributes/Role":            fmt.Sprintf("%s,%s", role.ExternalId, SAMLProvider.ExternalId),
+		"https://www.aliyun.com/SAML-Role/Attributes/Role":            fmt.Sprintf("%s,%s", roles[0].ExternalId, SAMLProvider.ExternalId),
 		"https://www.aliyun.com/SAML-Role/Attributes/RoleSessionName": userCred.GetUserId(),
 		"https://www.aliyun.com/SAML-Role/Attributes/SessionDuration": "1800",
 	} {

pkg/cloudid/saml/providers/aws/driver.go

@@ -52,7 +52,7 @@ func (d *SAWSSAMLDriver) GetIdpInitiatedLoginData(ctx context.Context, userCred
 		return data, httperrors.NewResourceNotReadyError("SAMLProvider for account %s not ready", account.Id)
 	}
 
-	role, err := account.SyncRole(userCred.GetUserId())
+	roles, err := account.SyncRoles(userCred.GetUserId(), true)
 	if err != nil {
 		return data, httperrors.NewGeneralError(errors.Wrapf(err, "SyncRole"))
 	}
@@ -68,7 +68,7 @@ func (d *SAWSSAMLDriver) GetIdpInitiatedLoginData(ctx context.Context, userCred
 		{
 			name:         "https://aws.amazon.com/SAML/Attributes/Role",
 			friendlyName: "RoleEntitlement",
-			value:        fmt.Sprintf("%s,%s", role.ExternalId, SAMLProvider.ExternalId),
+			value:        fmt.Sprintf("%s,%s", roles[0].ExternalId, SAMLProvider.ExternalId),
 		},
 		{
 			name:         "https://aws.amazon.com/SAML/Attributes/RoleSessionName",

pkg/cloudid/saml/providers/awscn/driver.go

@@ -51,7 +51,7 @@ func (d *SAWSCNSAMLDriver) GetIdpInitiatedLoginData(ctx context.Context, userCre
 		return data, httperrors.NewResourceNotReadyError("SAMLProvider for account %s not ready", account.Id)
 	}
 
-	role, err := account.SyncRole(userCred.GetUserId())
+	roles, err := account.SyncRoles(userCred.GetUserId(), true)
 	if err != nil {
 		return data, httperrors.NewGeneralError(errors.Wrapf(err, "SyncRole"))
 	}
@@ -67,7 +67,7 @@ func (d *SAWSCNSAMLDriver) GetIdpInitiatedLoginData(ctx context.Context, userCre
 		{
 			name:         "https://aws.amazon.com/SAML/Attributes/Role",
 			friendlyName: "RoleEntitlement",
-			value:        fmt.Sprintf("%s,%s", role.ExternalId, SAMLProvider.ExternalId),
+			value:        fmt.Sprintf("%s,%s", roles[0].ExternalId, SAMLProvider.ExternalId),
 		},
 		{
 			name:         "https://aws.amazon.com/SAML/Attributes/RoleSessionName",

pkg/cloudid/saml/providers/qcloud/driver.go

@@ -52,14 +52,14 @@ func (d *SQcloudSAMLDriver) GetIdpInitiatedLoginData(ctx context.Context, userCr
 		return data, httperrors.NewResourceNotReadyError("SAMLProvider for account %s not ready", account.Id)
 	}
 
-	role, err := account.SyncRole(userCred.GetUserId())
+	roles, err := account.SyncRoles(userCred.GetUserId(), true)
 	if err != nil {
 		return data, httperrors.NewGeneralError(errors.Wrapf(err, "SyncRole"))
 	}
 
-	roleStr := fmt.Sprintf("qcs::cam::uin/%s:roleName/%s,qcs::cam::uin/%s:saml-provider/%s", account.AccountId, role.ExternalId, account.AccountId, SAMLProvider.ExternalId)
+	roleStr := fmt.Sprintf("qcs::cam::uin/%s:roleName/%s,qcs::cam::uin/%s:saml-provider/%s", account.AccountId, roles[0].ExternalId, account.AccountId, SAMLProvider.ExternalId)
 
-	data.NameId = role.Name
+	data.NameId = roles[0].Name
 	data.NameIdFormat = samlutils.NAME_ID_FORMAT_TRANSIENT
 	data.AudienceRestriction = "https://cloud.tencent.com"
 	for _, v := range []struct {
@@ -75,7 +75,7 @@ func (d *SQcloudSAMLDriver) GetIdpInitiatedLoginData(ctx context.Context, userCr
 		{
 			name:         "https://cloud.tencent.com/SAML/Attributes/RoleSessionName",
 			friendlyName: "RoleSessionName",
-			value:        role.Name,
+			value:        roles[0].Name,
 		},
 	} {
 		data.Attributes = append(data.Attributes, samlutils.SSAMLResponseAttribute{
@@ -94,7 +94,7 @@ func (d *SQcloudSAMLDriver) GetSpInitiatedLoginData(ctx context.Context, userCre
 	_account, err := models.CloudaccountManager.FetchById(cloudAccountId)
 	if err != nil {
 		if errors.Cause(err) == sql.ErrNoRows {
-			return data, httperrors.NewResourceNotFoundError("cloudaccount", cloudAccountId)
+			return data, httperrors.NewResourceNotFoundError2("cloudaccount", cloudAccountId)
 		}
 		return data, httperrors.NewGeneralError(err)
 	}
@@ -111,14 +111,14 @@ func (d *SQcloudSAMLDriver) GetSpInitiatedLoginData(ctx context.Context, userCre
 		return data, httperrors.NewResourceNotReadyError("SAMLProvider for account %s not ready", account.Id)
 	}
 
-	role, err := account.SyncRole(userCred.GetUserId())
+	roles, err := account.SyncRoles(userCred.GetUserId(), true)
 	if err != nil {
 		return data, httperrors.NewGeneralError(errors.Wrapf(err, "SyncRole"))
 	}
 
-	roleStr := fmt.Sprintf("qcs::cam::uin/%s:roleName/%s,qcs::cam::uin/%s:saml-provider/%s", account.AccountId, role.ExternalId, account.AccountId, SAMLProvider.ExternalId)
+	roleStr := fmt.Sprintf("qcs::cam::uin/%s:roleName/%s,qcs::cam::uin/%s:saml-provider/%s", account.AccountId, roles[0].ExternalId, account.AccountId, SAMLProvider.ExternalId)
 
-	data.NameId = role.Name
+	data.NameId = roles[0].Name
 	data.NameIdFormat = samlutils.NAME_ID_FORMAT_TRANSIENT
 	data.AudienceRestriction = "https://cloud.tencent.com"
 	for _, v := range []struct {
@@ -134,7 +134,7 @@ func (d *SQcloudSAMLDriver) GetSpInitiatedLoginData(ctx context.Context, userCre
 		{
 			name:         "https://cloud.tencent.com/SAML/Attributes/RoleSessionName",
 			friendlyName: "RoleSessionName",
-			value:        role.Name,
+			value:        roles[0].Name,
 		},
 	} {
 		data.Attributes = append(data.Attributes, samlutils.SSAMLResponseAttribute{

pkg/multicloud/aliyun/provider/provider.go

@@ -478,6 +478,14 @@ func (self *SAliyunProvider) CreateICloudrole(opts *cloudprovider.SRoleCreateOpt
 	return role, nil
 }
 
+func (self *SAliyunProvider) GetICloudroleByName(name string) (cloudprovider.ICloudrole, error) {
+	role, err := self.client.GetRole(name)
+	if err != nil {
+		return nil, errors.Wrapf(err, "GetRole(%s)", name)
+	}
+	return role, nil
+}
+
 func (self *SAliyunProvider) GetICloudSAMLProviders() ([]cloudprovider.ICloudSAMLProvider, error) {
 	return self.client.GetICloudSAMLProviders()
 }