Access control: add further role checks and update tests

Author: webprofusion-chrisc

src/Certify.Core/Management/Access/AccessControl.cs

@@ -453,9 +453,9 @@ public string HashPassword(string password, string saltString = null)
             return hashed;
         }
 
-        public async Task<bool> AddRole(string contextUserId, Role r)
+        public async Task<bool> AddRole(string contextUserId, Role r, bool bypassIntegrityCheck = false)
         {
-            if (!await IsPrincipleInRole(contextUserId, contextUserId, StandardRoles.Administrator.Id))
+            if (!bypassIntegrityCheck && !await IsPrincipleInRole(contextUserId, contextUserId, StandardRoles.Administrator.Id))
             {
                 await AuditWarning("User {contextUserId} attempted to add an role action without being in required role.", contextUserId);
                 return false;
@@ -465,9 +465,9 @@ public async Task<bool> AddRole(string contextUserId, Role r)
             return true;
         }
 
-        public async Task<bool> AddAssignedRole(string contextUserId, AssignedRole r)
+        public async Task<bool> AddAssignedRole(string contextUserId, AssignedRole r, bool bypassIntegrityCheck = false)
         {
-            if (!await IsPrincipleInRole(contextUserId, contextUserId, StandardRoles.Administrator.Id))
+            if (!bypassIntegrityCheck && !await IsPrincipleInRole(contextUserId, contextUserId, StandardRoles.Administrator.Id))
             {
                 await AuditWarning("User {contextUserId} attempted to add an assigned role without being in required role.", contextUserId);
                 return false;
@@ -477,9 +477,9 @@ public async Task<bool> AddAssignedRole(string contextUserId, AssignedRole r)
             return true;
         }
 
-        public async Task<bool> AddResourceAction(string contextUserId, ResourceAction action)
+        public async Task<bool> AddResourceAction(string contextUserId, ResourceAction action, bool bypassIntegrityCheck = false)
         {
-            if (!await IsPrincipleInRole(contextUserId, contextUserId, StandardRoles.Administrator.Id))
+            if (!bypassIntegrityCheck && !await IsPrincipleInRole(contextUserId, contextUserId, StandardRoles.Administrator.Id))
             {
                 await AuditWarning("User {contextUserId} attempted to add a resource action without being in required role.", contextUserId);
                 return false;
@@ -494,7 +494,7 @@ public async Task<List<AssignedRole>> GetAssignedRoles(string contextUserId, str
             if (id != contextUserId && !await IsPrincipleInRole(contextUserId, contextUserId, StandardRoles.Administrator.Id))
             {
                 await AuditWarning("User {contextUserId} attempted to read assigned role for [{id}] without being in required role.", contextUserId, id);
-                return new List<AssignedRole>();
+                return null;
             }
 
             var assignedRoles = await _store.GetItems<AssignedRole>(nameof(AssignedRole));
@@ -507,6 +507,7 @@ public async Task<RoleStatus> GetSecurityPrincipleRoleStatus(string contextUserI
             if (id != contextUserId && !await IsPrincipleInRole(contextUserId, contextUserId, StandardRoles.Administrator.Id))
             {
                 await AuditWarning("User {contextUserId} attempted to read role status role for [{id}] without being in required role.", contextUserId, id);
+                return null;
             }
 
             var allAssignedRoles = await _store.GetItems<AssignedRole>(nameof(AssignedRole));
@@ -590,7 +591,7 @@ public async Task<List<AssignedAccessToken>> GetAssignedAccessTokens(string cont
             if (!await IsPrincipleInRole(contextUserId, contextUserId, StandardRoles.Administrator.Id))
             {
                 await AuditWarning("User {contextUserId} attempted to list assigned access tokens without being in required role.", contextUserId);
-                return [];
+                return null;
             }
 
             return await _store.GetItems<AssignedAccessToken>(nameof(AssignedAccessToken));

src/Certify.Core/Management/Access/IAccessControl.cs

@@ -27,9 +27,9 @@ public interface IAccessControl
         Task<bool> UpdateSecurityPrinciplePassword(string contextUserId, SecurityPrinciplePasswordUpdate passwordUpdate);
         Task<SecurityPrincipleCheckResponse> CheckSecurityPrinciplePassword(string contextUserId, SecurityPrinciplePasswordCheck passwordCheck);
 
-        Task<bool> AddRole(string contextUserId, Role role);
-        Task<bool> AddAssignedRole(string contextUserId, AssignedRole assignedRole);
-        Task<bool> AddResourceAction(string contextUserId, ResourceAction action);
+        Task<bool> AddRole(string contextUserId, Role role, bool bypassIntegrityCheck = false);
+        Task<bool> AddAssignedRole(string contextUserId, AssignedRole assignedRole, bool bypassIntegrityCheck = false);
+        Task<bool> AddResourceAction(string contextUserId, ResourceAction action, bool bypassIntegrityCheck = false);
 
         Task<List<AssignedAccessToken>> GetAssignedAccessTokens(string contextUserId);
         Task<bool> AddAssignedAccessToken(string contextUserId, AssignedAccessToken token);

src/Certify.Models/Hub/AccessControl.cs

@@ -1,4 +1,4 @@
-using System;
+using System;
 using System.Collections.Generic;
 
 namespace Certify.Models.Hub
@@ -104,6 +104,11 @@ public class AccessTokenCheck
         public AccessToken Token { get; set; }
         public AccessCheck Check { get; set; }
     }
+
+    public class AccessTokenTypes
+    {
+        public const string Simple = "simple";
+    }
     public class AccessToken : ConfigurationStoreItem
     {
         public string TokenType { get; set; } = default!;

src/Certify.Tests/Certify.Core.Tests.Unit/Tests/AccessControlTests.cs

@@ -1,4 +1,4 @@
-using System;
+using System;
 using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Linq;
@@ -229,15 +229,19 @@ public async Task TestAddGetAssignedRoles()
 
             // Setup policy with actions and add policy to store
             var policy = Policies.GetStandardPolicies().Find(p => p.Id == StandardPolicies.AccessAdmin);
-            _ = await access.AddResourcePolicy(contextUserId, policy, bypassIntegrityCheck: true);
+            var addPolicy = await access.AddResourcePolicy(contextUserId, policy, bypassIntegrityCheck: true);
+
+            Assert.IsTrue(addPolicy, "Expected to add role");
 
             // Setup and add roles and policy assignments to store
             var role = Policies.GetStandardRoles().Find(r => r.Id == StandardRoles.Administrator.Id);
-            await access.AddRole(contextUserId, role);
+            var addedRole = await access.AddRole(contextUserId, role, bypassIntegrityCheck: true);
+
+            Assert.IsTrue(addedRole, "Expected to add role");
 
             // Assign security principles to roles and add roles and policy assignments to store
             var assignedRoles = new List<AssignedRole> { TestAssignedRoles.Admin, TestAssignedRoles.TestAdmin };
-            assignedRoles.ForEach(async r => await access.AddAssignedRole(contextUserId, r));
+            assignedRoles.ForEach(async r => await access.AddAssignedRole(contextUserId, r, bypassIntegrityCheck: true));
 
             // Validate AssignedRole list returned by AccessControl.GetAssignedRoles()
             foreach (var assignedRole in assignedRoles)
@@ -256,6 +260,9 @@ public async Task TestGetAssignedRolesNoRoles()
             var adminSecurityPrinciples = new List<SecurityPrinciple> { TestSecurityPrinciples.Admin, TestSecurityPrinciples.TestAdmin };
             adminSecurityPrinciples.ForEach(async p => await access.AddSecurityPrinciple(contextUserId, p, bypassIntegrityCheck: true));
 
+            // assigned admin role to TestAdmin (also the contextUserId) so they can check roles for the other admin user
+            await access.AddAssignedRole(TestSecurityPrinciples.TestAdmin.Id, TestAssignedRoles.TestAdmin, bypassIntegrityCheck: true);
+
             // Validate AssignedRole list returned by AccessControl.GetAssignedRoles()
             var adminAssignedRoles = await access.GetAssignedRoles(contextUserId, adminSecurityPrinciples[0].Id);
             Assert.IsNotNull(adminAssignedRoles, "Expected list returned by AccessControl.GetAssignedRoles() to not be null");
@@ -303,7 +310,7 @@ public async Task TestUpdateSecurityPrinciple()
 
             // Assign security principles to roles and add roles and policy assignments to store
             var assignedRoles = new List<AssignedRole> { TestAssignedRoles.Admin, TestAssignedRoles.TestAdmin };
-            assignedRoles.ForEach(async r => await access.AddAssignedRole(contextUserId, r));
+            assignedRoles.ForEach(async r => await access.AddAssignedRole(contextUserId, r, bypassIntegrityCheck: true));
 
             // Validate email of SecurityPrinciple object returned by AccessControl.GetSecurityPrinciple() before update
             var storedSecurityPrinciple = await access.GetSecurityPrinciple(contextUserId, adminSecurityPrinciples[0].Id);
@@ -363,11 +370,11 @@ public async Task TestUpdateSecurityPrincipleBadUpdate()
 
             // Setup and add roles and policy assignments to store
             var role = Policies.GetStandardRoles().Find(r => r.Id == StandardRoles.Administrator.Id);
-            await access.AddRole(contextUserId, role);
+            await access.AddRole(contextUserId, role, bypassIntegrityCheck: true);
 
             // Assign security principles to roles and add roles and policy assignments to store
             var assignedRoles = new List<AssignedRole> { TestAssignedRoles.Admin, TestAssignedRoles.TestAdmin };
-            assignedRoles.ForEach(async r => await access.AddAssignedRole(contextUserId, r));
+            assignedRoles.ForEach(async r => await access.AddAssignedRole(contextUserId, r, bypassIntegrityCheck: true));
 
             // Validate email of SecurityPrinciple object returned by AccessControl.GetSecurityPrinciple() before update
             var storedSecurityPrinciple = await access.GetSecurityPrinciple(contextUserId, adminSecurityPrinciples[0].Id);
@@ -400,11 +407,11 @@ public async Task TestUpdateSecurityPrinciplePassword()
 
             // Setup and add roles and policy assignments to store
             var role = Policies.GetStandardRoles().Find(r => r.Id == StandardRoles.Administrator.Id);
-            await access.AddRole(contextUserId, role);
+            await access.AddRole(contextUserId, role, bypassIntegrityCheck: true);
 
             // Assign security principles to roles and add roles and policy assignments to store
             var assignedRoles = new List<AssignedRole> { TestAssignedRoles.Admin, TestAssignedRoles.TestAdmin };
-            assignedRoles.ForEach(async r => await access.AddAssignedRole(contextUserId, r));
+            assignedRoles.ForEach(async r => await access.AddAssignedRole(contextUserId, r, bypassIntegrityCheck: true));
 
             // Validate password of SecurityPrinciple object returned by AccessControl.GetSecurityPrinciple() before update
             var storedSecurityPrinciple = await access.GetSecurityPrinciple(contextUserId, adminSecurityPrinciples[0].Id);
@@ -496,11 +503,11 @@ public async Task TestDeleteSecurityPrinciple()
 
             // Setup and add roles and policy assignments to store
             var role = Policies.GetStandardRoles().Find(r => r.Id == StandardRoles.Administrator.Id);
-            await access.AddRole(contextUserId, role);
+            await access.AddRole(contextUserId, role, bypassIntegrityCheck: true);
 
             // Assign security principles to roles and add roles and policy assignments to store
             var assignedRoles = new List<AssignedRole> { TestAssignedRoles.Admin, TestAssignedRoles.TestAdmin };
-            assignedRoles.ForEach(async r => await access.AddAssignedRole(contextUserId, r));
+            assignedRoles.ForEach(async r => await access.AddAssignedRole(contextUserId, r, bypassIntegrityCheck: true));
 
             // Validate SecurityPrinciple object returned by AccessControl.GetSecurityPrinciple() before delete is not null
             var storedSecurityPrinciple = await access.GetSecurityPrinciple(contextUserId, adminSecurityPrinciples[0].Id);
@@ -620,19 +627,19 @@ public async Task TestIsPrincipleInRole()
 
             // Setup security principle actions
             var actions = Policies.GetStandardResourceActions().FindAll(a => a.ResourceType == ResourceTypes.System);
-            actions.ForEach(async a => await access.AddResourceAction(contextUserId, a));
+            actions.ForEach(async a => await access.AddResourceAction(contextUserId, a, bypassIntegrityCheck: true));
 
             // Setup policy with actions and add policy to store
             var policy = Policies.GetStandardPolicies().Find(p => p.Id == StandardPolicies.AccessAdmin);
             _ = await access.AddResourcePolicy(contextUserId, policy, bypassIntegrityCheck: true);
 
             // Setup and add roles and policy assignments to store
             var role = Policies.GetStandardRoles().Find(r => r.Id == StandardRoles.Administrator.Id);
-            await access.AddRole(contextUserId, role);
+            await access.AddRole(contextUserId, role, bypassIntegrityCheck: true);
 
             // Assign security principles to roles and add roles and policy assignments to store
             var assignedRoles = new List<AssignedRole> { TestAssignedRoles.Admin, TestAssignedRoles.TestAdmin };
-            assignedRoles.ForEach(async r => await access.AddAssignedRole(contextUserId, r));
+            assignedRoles.ForEach(async r => await access.AddAssignedRole(contextUserId, r, bypassIntegrityCheck: true));
 
             // Validate specified admin user is a principle role
             bool hasAccess;
@@ -662,10 +669,10 @@ public async Task TestDomainAuth()
 
             // Setup and add roles and policy assignments to store
             var role = Policies.GetStandardRoles().Find(r => r.Id == StandardRoles.CertificateConsumer.Id);
-            await access.AddRole(contextUserId, role);
+            await access.AddRole(contextUserId, role, bypassIntegrityCheck: true);
 
             // Assign security principles to roles and add roles and policy assignments to store
-            await access.AddAssignedRole(contextUserId, TestAssignedRoles.DevopsUserDomainConsumer); // devops user in consumer role for a specific domain
+            await access.AddAssignedRole(contextUserId, TestAssignedRoles.DevopsUserDomainConsumer, true); // devops user in consumer role for a specific domain
 
             // Validate user can consume a cert for a given domain 
             var isAuthorised = await access.IsSecurityPrincipleAuthorised(contextUserId, new AccessCheck(TestSecurityPrinciples.DevopsAppDomainConsumer.Id, ResourceTypes.Domain, StandardResourceActions.CertificateDownload, identifier: "www.example.com"));
@@ -691,10 +698,10 @@ public async Task TestWildcardDomainAuth()
 
             // Setup and add roles and policy assignments to store
             var role = Policies.GetStandardRoles().Find(r => r.Id == StandardRoles.CertificateConsumer.Id);
-            await access.AddRole(contextUserId, role);
+            await access.AddRole(contextUserId, role, bypassIntegrityCheck: true);
 
             // Assign security principles to roles and add roles and policy assignments to store
-            await access.AddAssignedRole(contextUserId, TestAssignedRoles.DevopsUserWildcardDomainConsumer); // devops user in consumer role for a wildcard domain
+            await access.AddAssignedRole(contextUserId, TestAssignedRoles.DevopsUserWildcardDomainConsumer, bypassIntegrityCheck: true); // devops user in consumer role for a wildcard domain
 
             // Validate user can consume any subdomain via a granted wildcard
             var isAuthorised = await access.IsSecurityPrincipleAuthorised(contextUserId, new AccessCheck(TestSecurityPrinciples.DevopsUser.Id, ResourceTypes.Domain, StandardResourceActions.CertificateDownload, identifier: "random.microsoft.com"));
@@ -761,7 +768,7 @@ public async Task TestUserAPIToken()
 
             // allow test admin to perform access checks
             var assignedRoles = new List<AssignedRole> { TestAssignedRoles.TestAdmin };
-            assignedRoles.ForEach(async r => await access.AddAssignedRole(contextUserId, r));
+            assignedRoles.ForEach(async r => await access.AddAssignedRole(contextUserId, r, bypassIntegrityCheck: true));
 
             // Add test devops user security principle
             _ = await access.AddSecurityPrinciple(contextUserId, TestSecurityPrinciples.DevopsUser, bypassIntegrityCheck: true);
@@ -783,10 +790,10 @@ public async Task TestUserAPIToken()
             var assignedRolesForDevopsUser = await access.GetAssignedRoles(contextUserId, TestSecurityPrinciples.DevopsUser.Id);
 
             // create and assign a new API token
-            var apiToken = new AccessToken { ClientId = TestSecurityPrinciples.DevopsUser.Id, Secret = Guid.NewGuid().ToString(), TokenType = "Simple", Description = "An example API token" };
-            var apiExpiredToken = new AccessToken { ClientId = TestSecurityPrinciples.DevopsUser.Id, Secret = Guid.NewGuid().ToString(), TokenType = "Simple", Description = "An example expired API token", DateExpiry = DateTimeOffset.UtcNow.AddDays(-1) };
-            var apiRevokedToken = new AccessToken { ClientId = TestSecurityPrinciples.DevopsUser.Id, Secret = Guid.NewGuid().ToString(), TokenType = "Simple", Description = "An example revoked API token", DateRevoked = DateTimeOffset.UtcNow.AddDays(-1) };
-            var apiTokenBad = new AccessToken { ClientId = TestSecurityPrinciples.DomainOwner.Id, Secret = Guid.NewGuid().ToString(), TokenType = "Simple", Description = "An example bad API token (invalid client id)" };
+            var apiToken = new AccessToken { ClientId = TestSecurityPrinciples.DevopsUser.Id, Secret = Guid.NewGuid().ToString(), TokenType = AccessTokenTypes.Simple, Description = "An example API token" };
+            var apiExpiredToken = new AccessToken { ClientId = TestSecurityPrinciples.DevopsUser.Id, Secret = Guid.NewGuid().ToString(), TokenType = AccessTokenTypes.Simple, Description = "An example expired API token", DateExpiry = DateTimeOffset.UtcNow.AddDays(-1) };
+            var apiRevokedToken = new AccessToken { ClientId = TestSecurityPrinciples.DevopsUser.Id, Secret = Guid.NewGuid().ToString(), TokenType = AccessTokenTypes.Simple, Description = "An example revoked API token", DateRevoked = DateTimeOffset.UtcNow.AddDays(-1) };
+            var apiTokenBad = new AccessToken { ClientId = TestSecurityPrinciples.DomainOwner.Id, Secret = Guid.NewGuid().ToString(), TokenType = AccessTokenTypes.Simple, Description = "An example bad API token (invalid client id)" };
             var assignedToken = new AssignedAccessToken
             {
                 AccessTokens = [apiToken, apiExpiredToken, apiRevokedToken],