From ab15a751e44a03ce0841d43165408e891a67e1df Mon Sep 17 00:00:00 2001 From: Qu Xuan Date: Thu, 14 Jan 2021 17:30:29 +0800 Subject: [PATCH] fix(cloudid): optimized saml sso --- pkg/apis/cloudid/cloudrole.go | 1 + pkg/cloudid/models/cloudaccount.go | 102 ++++++++++++++------ pkg/cloudid/models/cloudrole.go | 16 ++- pkg/cloudid/saml/providers/aliyun/driver.go | 4 +- pkg/cloudid/saml/providers/aws/driver.go | 4 +- pkg/cloudid/saml/providers/awscn/driver.go | 4 +- pkg/cloudid/saml/providers/qcloud/driver.go | 18 ++-- pkg/multicloud/aliyun/provider/provider.go | 8 ++ 8 files changed, 110 insertions(+), 47 deletions(-) diff --git a/pkg/apis/cloudid/cloudrole.go b/pkg/apis/cloudid/cloudrole.go index 6620d78430..a1040039a3 100644 --- a/pkg/apis/cloudid/cloudrole.go +++ b/pkg/apis/cloudid/cloudrole.go @@ -27,6 +27,7 @@ type CloudroleListInput struct { apis.StatusInfrasResourceBaseListInput CloudaccountResourceListInput + CloudgroupResourceListInput } type CloudroleDetails struct { diff --git a/pkg/cloudid/models/cloudaccount.go b/pkg/cloudid/models/cloudaccount.go index 5f30b88d54..768619ca6a 100644 --- a/pkg/cloudid/models/cloudaccount.go +++ b/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") diff --git a/pkg/cloudid/models/cloudrole.go b/pkg/cloudid/models/cloudrole.go index b0164a9e54..74f717b7a9 100644 --- a/pkg/cloudid/models/cloudrole.go +++ b/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]) diff --git a/pkg/cloudid/saml/providers/aliyun/driver.go b/pkg/cloudid/saml/providers/aliyun/driver.go index 4a16cd453f..a80a62c298 100644 --- a/pkg/cloudid/saml/providers/aliyun/driver.go +++ b/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", } { diff --git a/pkg/cloudid/saml/providers/aws/driver.go b/pkg/cloudid/saml/providers/aws/driver.go index 338673527e..8355bc1c66 100644 --- a/pkg/cloudid/saml/providers/aws/driver.go +++ b/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", diff --git a/pkg/cloudid/saml/providers/awscn/driver.go b/pkg/cloudid/saml/providers/awscn/driver.go index f96131a8bf..af4846d380 100644 --- a/pkg/cloudid/saml/providers/awscn/driver.go +++ b/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", diff --git a/pkg/cloudid/saml/providers/qcloud/driver.go b/pkg/cloudid/saml/providers/qcloud/driver.go index 06883b455f..2db35bec0c 100644 --- a/pkg/cloudid/saml/providers/qcloud/driver.go +++ b/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{ diff --git a/pkg/multicloud/aliyun/provider/provider.go b/pkg/multicloud/aliyun/provider/provider.go index 16835a46e0..9df4fa2892 100644 --- a/pkg/multicloud/aliyun/provider/provider.go +++ b/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() }