Cross-Tenant Organizational Unit Creation Allowed #10632

2 Answer(s)

• 

  0

  AI-Bot created 4 months ago

  Support Team ABP Agent

  • This behavior can happen if the Organization Unit repository/queries are not protected by the current tenant filter at the time the parentId is loaded/validated, so the service can “see” an OU from another tenant and accept it as a parent. In a correctly isolated setup, the parentId lookup should only be able to resolve OUs in the current tenant (or fail).

  • Immediate mitigation (recommended custom validation): Override/extend the OU create endpoint in your solution and validate that the parentId belongs to the current tenant before creating the OU. The simplest safe rule is: if input.ParentId != null, then parent.TenantId must equal CurrentTenant.Id (and also handle host side as null tenant).

    Example (custom application service wrapper) pattern:

  using System;
    using System.Threading.Tasks;
    using Volo.Abp;
    using Volo.Abp.Application.Services;
    using Volo.Abp.MultiTenancy;
    using Volo.Abp.Identity;
  
    public class MyOrganizationUnitAppService : ApplicationService
    {
        private readonly IOrganizationUnitRepository _organizationUnitRepository;
        private readonly OrganizationUnitManager _organizationUnitManager;
        private readonly ICurrentTenant _currentTenant;
  
        public MyOrganizationUnitAppService(
            IOrganizationUnitRepository organizationUnitRepository,
            OrganizationUnitManager organizationUnitManager,
            ICurrentTenant currentTenant)
        {
            _organizationUnitRepository = organizationUnitRepository;
            _organizationUnitManager = organizationUnitManager;
            _currentTenant = currentTenant;
        }
  
        public async Task<OrganizationUnitDto> CreateAsync(OrganizationUnitCreateDto input)
        {
            if (input.ParentId.HasValue)
            {
                var parent = await _organizationUnitRepository.GetAsync(input.ParentId.Value);
  
                // Tenant boundary check
                if (parent.TenantId != _currentTenant.Id)
                {
                    throw new BusinessException("AbpIdentity:CrossTenantOuParentNotAllowed")
                        .WithData("ParentId", input.ParentId)
                        .WithData("CurrentTenantId", _currentTenant.Id)
                        .WithData("ParentTenantId", parent.TenantId);
                }
            }
  
            var ou = await _organizationUnitManager.CreateAsync(input.DisplayName, input.ParentId);
            return ObjectMapper.Map<OrganizationUnit, OrganizationUnitDto>(ou);
        }
    }
  

  This guarantees the API cannot link to a parent OU from another tenant even if some internal query accidentally bypasses tenant filtering.

  • Also verify your multi-tenancy data filter is not being disabled for this call path (globally or via a custom unit-of-work/action filter). If you have any code like IDataFilter.Disable<IMultiTenant>() around identity/OUs, remove it; that would directly allow cross-tenant parent resolution.

  • If you believe this occurs in a clean ABP 10.0.1 Pro template without customization, please report it to Volosoft support (include your minimal repro).
    https://abp.io/contact

  ---

  This response was generated by AI. If it didn’t solve your problem, feel free to post a new answer — our support team will jump in to help you soon.

  Markdown supported.

  Copy, paste, or drag & drop images and files (max 100 MB per file, 100 MB total per post)

  Save Cancel
• 

  0

  maliming created 4 months ago

  Support Team Fullstack Developer

  Hi,

  Thanks for the detailed report — you found a real framework gap. Confirmed in OrganizationUnitManager.CreateAsync and MoveAsync: neither method validates that the supplied parentId belongs to the current tenant, so calling the API with a ParentId from another tenant goes through.

  What actually ends up in the DB is a bit subtle: the new OU's TenantId is correctly set to your current tenant (Tenant A), and because the multi-tenant filter hides Tenant B's OU from the lookup, the Code is calculated as a top-level code (00001). But the ParentId column is stored as-is, pointing at Tenant B's OU id. That cross-tenant FK reference is the real data-integrity issue (and MoveAsync has the same gap).

  We just pushed the fix in https://github.com/abpframework/abp/pull/25357 — the manager now validates the parent tenant for both create and move and throws Volo.Abp.Identity:010010 (OrganizationUnitParentTenantMismatch) when they don't match. It will ship in the next ABP release.

  For now, you can apply the same check from your application layer by overriding the Pro OrganizationUnitAppService:

  [ExposeServices(typeof(IOrganizationUnitAppService))]
  [Dependency(ReplaceServices = true)]
  public class MyOrganizationUnitAppService : OrganizationUnitAppService
  {
      public MyOrganizationUnitAppService(
          OrganizationUnitManager organizationUnitManager,
          IdentityUserManager userManager,
          IOrganizationUnitRepository organizationUnitRepository,
          IIdentityUserRepository identityUserRepository,
          IIdentityRoleRepository identityRoleRepository,
          IdentityProTwoFactorManager identityProTwoFactorManager)
          : base(organizationUnitManager, userManager, organizationUnitRepository,
                 identityUserRepository, identityRoleRepository, identityProTwoFactorManager)
      {
      }
  
      public override async Task<OrganizationUnitWithDetailsDto> CreateAsync(OrganizationUnitCreateDto input)
      {
          await EnsureParentInCurrentTenantAsync(input.ParentId);
          return await base.CreateAsync(input);
      }
  
      public override async Task MoveAsync(Guid id, OrganizationUnitMoveInput input)
      {
          await EnsureParentInCurrentTenantAsync(input.NewParentId);
          await base.MoveAsync(id, input);
      }
  
      protected virtual async Task EnsureParentInCurrentTenantAsync(Guid? parentId)
      {
          if (!parentId.HasValue)
          {
              return;
          }
  
          var parent = await OrganizationUnitRepository.FindAsync(parentId.Value);
          if (parent == null || parent.TenantId != CurrentTenant.Id)
          {
              throw new BusinessException("Identity:CrossTenantOrganizationUnitParent")
                  .WithData("ParentId", parentId);
          }
      }
  }
  

  Use FindAsync (not GetAsync) so the multi-tenant filter naturally hides cross-tenant parents — you'll get a clean BusinessException either way.

  Thanks again for catching this.

  Markdown supported.

  Copy, paste, or drag & drop images and files (max 100 MB per file, 100 MB total per post)

  Save Cancel