Don't lock the subscription owner of his tenants when TenantManager authorizations have been removed. (need to be checked further)

fdonnet · fdonnet · commit a751c4ed0316 · 2025-02-16T12:25:41.000+01:00
diff --git a/README.md b/README.md
@@ -34,9 +34,6 @@ User accounts for login in Security UI:
 | adminuser | admin | can manage system roles and authorizations |
 | user1| user | manage subscription and tenant |
 
-Don't remove the "TenantManger" role on the user1, or detatch him from the subscription, or your will be locked out of your tenant.
-(implements your own better rules).
-
 3. Play and deep dive in the code...
 
 *If you want to run the integration test project, pls comment this line: 
diff --git a/UbikLink.IntegrationTests/AspireFixture.cs b/UbikLink.IntegrationTests/AspireFixture.cs
@@ -140,8 +140,6 @@ private void ModifyResourcesForTesting()
                 //    port.Port = 8081;
                 //}
 
-
-
                 var containerLifetimeAnnotation = keycloak.Annotations
                 .OfType<ContainerLifetimeAnnotation>()
                 .FirstOrDefault();
diff --git a/UbikLink.Proxy/Authorizations/UserTenantRolesOrAuthorizationsRequirement.cs b/UbikLink.Proxy/Authorizations/UserTenantRolesOrAuthorizationsRequirement.cs
@@ -10,10 +10,11 @@ public enum PermissionMode
         Authorization
     }
 
-    public class UserTenantRolesOrAuthorizationsRequirement(string[] values, PermissionMode mode) : IAuthorizationRequirement
+    public class UserTenantRolesOrAuthorizationsRequirement(string[] values, PermissionMode mode, bool isSubscriptionOwnerAllowed) : IAuthorizationRequirement
     {
         public string[] Values { get; set; } = values;
         public PermissionMode Mode { get; set; } = mode;
+        public bool IsSubscriptionOwnerAllowed { get; set; } = isSubscriptionOwnerAllowed;
     }
 
     public class UserRolesAuthorizationOkHandler(UserService userService)
@@ -44,14 +45,12 @@ protected override async Task HandleRequirementAsync(AuthorizationHandlerContext
                 return;
             }
 
-            //TODO: it's a security hole (need to check)
-            // Certain tenant authorizations don't need to be deleted or you will be locked out of a tenant...
-            //if (requirement.IsSubscriptionOwnerAllowed
-            //               && userInfo.OwnerOfSubscriptionsIds.Any())
-            //{
-            //    context.Succeed(requirement);
-            //    return;
-            //}
+             //TODO: check that
+            if (requirement.IsSubscriptionOwnerAllowed && userInfo.IsSubOwnerOfTheSelectetdTenant)
+            {
+                context.Succeed(requirement);
+                return;
+            }
 
             switch (requirement.Mode)
             {
diff --git a/UbikLink.Proxy/Program.cs b/UbikLink.Proxy/Program.cs
@@ -99,13 +99,13 @@
     .AddPolicy("IsSubOwner", policy =>
         policy.Requirements.Add(new UserInfoOnlyRequirement(RoleRequirement.SubscriptionOwner)))
     .AddPolicy("CanReadTenant", policy =>
-        policy.Requirements.Add(new UserTenantRolesOrAuthorizationsRequirement(["tenant:read"], PermissionMode.Authorization)))
+        policy.Requirements.Add(new UserTenantRolesOrAuthorizationsRequirement(["tenant:read"], PermissionMode.Authorization, true)))
     .AddPolicy("CanReadTenantAndReadUser", policy =>
-        policy.Requirements.Add(new UserTenantRolesOrAuthorizationsRequirement(["tenant:read", "user:read"], PermissionMode.Authorization)))
+        policy.Requirements.Add(new UserTenantRolesOrAuthorizationsRequirement(["tenant:read", "user:read"], PermissionMode.Authorization, true)))
     .AddPolicy("CanReadTenantAndWriteUserRole", policy =>
-        policy.Requirements.Add(new UserTenantRolesOrAuthorizationsRequirement(["tenant:read", "user:read", "tenant-user-role:write"], PermissionMode.Authorization)))
+        policy.Requirements.Add(new UserTenantRolesOrAuthorizationsRequirement(["tenant:read", "user:read", "tenant-user-role:write"], PermissionMode.Authorization, true)))
     .AddPolicy("CanReadTenantAndReadTenantRoles", policy =>
-        policy.Requirements.Add(new UserTenantRolesOrAuthorizationsRequirement(["tenant:read", "tenant-role:read"], PermissionMode.Authorization)));
+        policy.Requirements.Add(new UserTenantRolesOrAuthorizationsRequirement(["tenant:read", "tenant-role:read"], PermissionMode.Authorization, true)));
 
 
 //Proxy
diff --git a/UbikLink.Security.Api/Features/Users/Services/Poco/UserWithSubscriptionInfo.cs b/UbikLink.Security.Api/Features/Users/Services/Poco/UserWithSubscriptionInfo.cs
@@ -14,6 +14,7 @@ public class UserWithSubscriptionInfo
         public required string Email { get; set; }
         public bool IsMegaAdmin { get; set; } = false;
         public Guid? SelectedTenantId { get; set; }
+        public bool IsSubOwnerOfTheSelectetdTenant { get; init; } = false;
         public bool IsActiveInSelectedSubscription { get; set; }
         public List<Guid> OwnerOfSubscriptionsIds { get; set; } = [];
         public List<AuthorizationModel> SelectedTenantAuthorizations { get; set; } = [];
diff --git a/UbikLink.Security.Api/Features/Users/Services/UserQueryService.cs b/UbikLink.Security.Api/Features/Users/Services/UserQueryService.cs
@@ -30,7 +30,7 @@ public async Task<Either<IFeatureError, UserWithSubscriptionInfo>>
         {
             var con = _ctx.Database.GetDbConnection();
             var sql = $"""
-                       SELECT su.is_activated 
+                       SELECT su.is_activated, su.is_owner 
                        FROM subscriptions_users su
                        INNER JOIN users u ON su.user_id = u.id
                        INNER JOIN tenants t ON t.id = u.selected_tenant_id
@@ -39,7 +39,7 @@ FROM subscriptions_users su
                        AND su.subscription_id = t.subscription_id
                        """;
 
-            var active = await con.QuerySingleOrDefaultAsync<bool?>(sql, new { userId = user.Id, user.SelectedTenantId }) ?? false;
+            var activeAndOwner = await con.QuerySingleOrDefaultAsync<(bool? Active,bool? Owner)>(sql, new { userId = user.Id, user.SelectedTenantId });
 
             if (con.State == System.Data.ConnectionState.Open)
                 await con.CloseAsync();
@@ -48,7 +48,8 @@ FROM subscriptions_users su
             {
                 AuthId = user.AuthId,
                 Id = user.Id,
-                IsActiveInSelectedSubscription = active,
+                IsActiveInSelectedSubscription = activeAndOwner.Active == null ? false : (bool)activeAndOwner.Active,
+                IsSubOwnerOfTheSelectetdTenant = activeAndOwner.Owner == null ? false : (bool)activeAndOwner.Owner,
                 SelectedTenantId = user.SelectedTenantId,
                 Email = user.Email,
                 Firstname = user.Firstname,
diff --git a/UbikLink.Security.Api/Mappers/UserMappers.cs b/UbikLink.Security.Api/Mappers/UserMappers.cs
@@ -19,6 +19,7 @@ public static UserProxyResult MapToUserProxyResult(this UserWithSubscriptionInfo
                 Firstname = user.Firstname,
                 Lastname = user.Lastname,
                 Email = user.Email,
+                IsSubOwnerOfTheSelectetdTenant = user.IsSubOwnerOfTheSelectetdTenant,
                 IsActivatedInSelectedSubscription = user.IsActiveInSelectedSubscription,
                 IsMegaAdmin = user.IsMegaAdmin,
                 SelectedTenantId = user.SelectedTenantId,
@@ -38,6 +39,7 @@ public static UserMeResult MapToUserMeResult(this UserWithSubscriptionInfo user)
                 Firstname = user.Firstname,
                 Lastname = user.Lastname,
                 Email = user.Email,
+                IsSubOwnerOfTheSelectetdTenant = user.IsSubOwnerOfTheSelectetdTenant,
                 IsActivatedInSelectedSubscription = user.IsActiveInSelectedSubscription,
                 IsMegaAdmin = user.IsMegaAdmin,
                 SelectedTenantId = user.SelectedTenantId,
diff --git a/UbikLink.Security.Contracts/Users/Results/UserMeResult.cs b/UbikLink.Security.Contracts/Users/Results/UserMeResult.cs
@@ -19,6 +19,7 @@ public record UserMeResult
         public bool IsMegaAdmin { get; init; } = false;
         public IEnumerable<Guid> OwnerOfSubscriptionsIds { get; init; } = default!;
         public Guid? SelectedTenantId { get; init; }
+        public bool IsSubOwnerOfTheSelectetdTenant { get; init; } = false;
         public List<AuthorizationLightResult> SelectedTenantAuthorizations { get; init; } = [];
         public List<RoleLightResult> SelectedTenantRoles { get; init; } = [];
         public Guid Version { get; init; }
diff --git a/UbikLink.Security.Contracts/Users/Results/UserProxyResult.cs b/UbikLink.Security.Contracts/Users/Results/UserProxyResult.cs
@@ -14,6 +14,7 @@ public record UserProxyResult
         public bool IsMegaAdmin { get; init; } = false;
         public IEnumerable<Guid> OwnerOfSubscriptionsIds { get; init; } = default!;
         public Guid? SelectedTenantId { get; init; }
+        public bool IsSubOwnerOfTheSelectetdTenant { get; init; } = false;
         public List<AuthorizationLightResult> SelectedTenantAuthorizations { get; init; } = [];
         public  List<RoleLightResult> SelectedTenantRoles { get; init; } = [];
         public Guid Version { get; init; }
diff --git a/UbikLink.Security.UI/Components/Tenant/TenantPage.razor b/UbikLink.Security.UI/Components/Tenant/TenantPage.razor
@@ -12,7 +12,7 @@
 <UbikContentContainer>
     <UbikFluentBreadcrumb Items="BreadcrumbItems" WithClose="false" />
 
-    @if (_isTenantManager)
+    @if (_isTenantManager || _isSubscriptionOwner)
     {
         <FluentStack Orientation="Orientation.Vertical" Class="tw-mb-2 tw-mt-3">
             <div>
diff --git a/UbikLink.Security.UI/Components/Tenant/TenantPage.razor.cs b/UbikLink.Security.UI/Components/Tenant/TenantPage.razor.cs
@@ -18,6 +18,8 @@ protected override List<BreadcrumbListItem> BreadcrumbItems
             }
         }
 
+        private bool _isSubscriptionOwner = false;
+
         private static readonly List<BreadcrumbListItem> _breadcrumbItems = [
             new BreadcrumbListItem()
             {
@@ -30,6 +32,7 @@ protected override async Task OnInitializedAsync()
         {
             await base.OnInitializedAsync();
             _isTenantManager = CanAccessIfHasAuthorizations(["tenant:read", "user:read", "tenant-user-role:write", "tenant-role:read"]);
+            _isSubscriptionOwner = User?.IsSubOwnerOfTheSelectetdTenant ?? false;
         }
     }
 }
diff --git a/UbikLink.Security.UI/Components/Tenant/TenantUpdateUserRolesPage.razor b/UbikLink.Security.UI/Components/Tenant/TenantUpdateUserRolesPage.razor
@@ -12,7 +12,7 @@
 <UbikContentContainer>
     <UbikFluentBreadcrumb Items="BreadcrumbItems" WithClose="true" CloseUrl="tenant" />
 
-    @if (_isTenantManager)
+    @if (_isTenantManager || _isSubscriptionOwner)
     {
         <FluentStack Orientation="Orientation.Vertical" Class="tw-mb-2 tw-mt-3">
             <div>
diff --git a/UbikLink.Security.UI/Components/Tenant/TenantUpdateUserRolesPage.razor.cs b/UbikLink.Security.UI/Components/Tenant/TenantUpdateUserRolesPage.razor.cs
@@ -21,6 +21,8 @@ protected override List<BreadcrumbListItem> BreadcrumbItems
             }
         }
 
+        private bool _isSubscriptionOwner = false;
+
         private static readonly List<BreadcrumbListItem> _breadcrumbItems = [
        new BreadcrumbListItem()
             {
@@ -39,6 +41,7 @@ protected override async Task OnInitializedAsync()
         {
             await base.OnInitializedAsync();
             _isTenantManager = CanAccessIfHasAuthorizations(["tenant:read", "user:read", "tenant-user-role:write", "tenant-role:read"]);
+            _isSubscriptionOwner = User?.IsSubOwnerOfTheSelectetdTenant ?? false;
         }
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -10,10 +10,11 @@ public enum PermissionMode`
`10`	`10`	`Authorization`
`11`	`11`	`}`
`12`	`12`
`13`		`- public class UserTenantRolesOrAuthorizationsRequirement(string[] values, PermissionMode mode) : IAuthorizationRequirement`
	`13`	`+ public class UserTenantRolesOrAuthorizationsRequirement(string[] values, PermissionMode mode, bool isSubscriptionOwnerAllowed) : IAuthorizationRequirement`
`14`	`14`	`{`
`15`	`15`	`public string[] Values { get; set; } = values;`
`16`	`16`	`public PermissionMode Mode { get; set; } = mode;`
	`17`	`+ public bool IsSubscriptionOwnerAllowed { get; set; } = isSubscriptionOwnerAllowed;`
`17`	`18`	`}`
`18`	`19`
`19`	`20`	`public class UserRolesAuthorizationOkHandler(UserService userService)`
`@@ -44,14 +45,12 @@ protected override async Task HandleRequirementAsync(AuthorizationHandlerContext`
`44`	`45`	`return;`
`45`	`46`	`}`
`46`	`47`
`47`		`- //TODO: it's a security hole (need to check)`
`48`		`- // Certain tenant authorizations don't need to be deleted or you will be locked out of a tenant...`
`49`		`- //if (requirement.IsSubscriptionOwnerAllowed`
`50`		`- // && userInfo.OwnerOfSubscriptionsIds.Any())`
`51`		`- //{`
`52`		`- // context.Succeed(requirement);`
`53`		`- // return;`
`54`		`- //}`
	`48`	`+ //TODO: check that`
	`49`	`+ if (requirement.IsSubscriptionOwnerAllowed && userInfo.IsSubOwnerOfTheSelectetdTenant)`
	`50`	`+ {`
	`51`	`+ context.Succeed(requirement);`
	`52`	`+ return;`
	`53`	`+ }`
`55`	`54`
`56`	`55`	`switch (requirement.Mode)`
`57`	`56`	`{`
Original file line number	Diff line number	Diff line change
`@@ -12,7 +12,7 @@`
`12`	`12`	`<UbikContentContainer>`
`13`	`13`	`<UbikFluentBreadcrumb Items="BreadcrumbItems" WithClose="false" />`
`14`	`14`
`15`		`- @if (_isTenantManager)`
	`15`	`+ @if (_isTenantManager \|\| _isSubscriptionOwner)`
`16`	`16`	`{`
`17`	`17`	`<FluentStack Orientation="Orientation.Vertical" Class="tw-mb-2 tw-mt-3">`
`18`	`18`	`<div>`