From e16364bf24f4f58f0cb81aba0215f4d05cb4276f Mon Sep 17 00:00:00 2001 From: Jarupat Jisarojito Date: Tue, 19 Jul 2022 16:34:46 -0700 Subject: [PATCH 01/20] update test' --- .../REST/RestAuthorizationHandlerUnitTests.cs | 33 ++++++++++++++++--- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/DataGateway.Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs b/DataGateway.Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs index 501e2855e2..f4694a1703 100644 --- a/DataGateway.Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs +++ b/DataGateway.Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs @@ -1,6 +1,7 @@ using System.Collections; using System.Collections.Generic; using System.Security.Claims; +using System.Text.Json; using System.Threading.Tasks; using Azure.DataGateway.Auth; using Azure.DataGateway.Config; @@ -350,10 +351,34 @@ IEnumerable columnsRequested /// private static AuthorizationResolver SetupAuthResolverWithWildcardActions() { - RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig( - entityName: AuthorizationHelpers.TEST_ENTITY, - roleName: "admin", - actionName: "*" + string roleName = "admin"; + string entityName = AuthorizationHelpers.TEST_ENTITY; + + PermissionSetting permissionForEntity = new( + role: roleName, + actions: new object[] { JsonSerializer.SerializeToElement("*") }); + + Entity sampleEntity = new( + Source: AuthorizationHelpers.TEST_ENTITY, + Rest: null, + GraphQL: null, + Permissions: new PermissionSetting[] { permissionForEntity }, + Relationships: null, + Mappings: null + ); + + Dictionary entityMap = new(); + entityMap.Add(entityName, sampleEntity); + + RuntimeConfig runtimeConfig = new( + Schema: "UnitTestSchema", + MsSql: null, + CosmosDb: null, + PostgreSql: null, + MySql: null, + DataSource: new DataSource(DatabaseType: DatabaseType.mssql), + RuntimeSettings: new Dictionary(), + Entities: entityMap ); return AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); From 59519d0eb1a0a6f0eba49714886c6eefdd296b1f Mon Sep 17 00:00:00 2001 From: Jarupat Jisarojito Date: Tue, 19 Jul 2022 16:41:39 -0700 Subject: [PATCH 02/20] Update * string to constant variable. --- .../Authorization/AuthorizationResolverUnitTests.cs | 12 ++++++------ .../REST/RestAuthorizationHandlerUnitTests.cs | 13 ++++++------- .../Authorization/AuthorizationResolver.cs | 2 +- .../Configurations/RuntimeConfigValidator.cs | 12 ++++++------ 4 files changed, 19 insertions(+), 20 deletions(-) diff --git a/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs b/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs index ba5408d039..09d42e2acc 100644 --- a/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs +++ b/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs @@ -165,7 +165,7 @@ public void WildcardIncludeColDefinedForAction_ColsForActionTest() AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, - includedCols: new HashSet { "*" } + includedCols: new HashSet { AuthorizationResolver.WILDCARD } ); AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); @@ -183,7 +183,7 @@ public void WildcardIncludeColsSomeExcludeDefinedForActionSuccess_ColsForActionT AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, - includedCols: new HashSet { "*" }, + includedCols: new HashSet { AuthorizationResolver.WILDCARD }, excludedCols: new HashSet { "col1", "col2" } ); AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); @@ -278,7 +278,7 @@ public void WildcardExcludeColsDefinedForAction_ColsForActionTest() AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, - excludedCols: new HashSet { "*" } + excludedCols: new HashSet { AuthorizationResolver.WILDCARD } ); AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); @@ -296,7 +296,7 @@ public void WildcardIncludeColsSomeExcludeDefinedForAction_ColsForActionTest() AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, - includedCols: new HashSet { "*" }, + includedCols: new HashSet { AuthorizationResolver.WILDCARD }, excludedCols: new HashSet { "col1", "col2" } ); AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); @@ -316,7 +316,7 @@ public void WildcardExcludeColsSomeIncludeDefinedForAction_ColsForActionTest() AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, includedCols: new HashSet { "col1", "col2" }, - excludedCols: new HashSet { "*" } + excludedCols: new HashSet { AuthorizationResolver.WILDCARD } ); AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); @@ -335,7 +335,7 @@ public void WildcardExcludeColsSomeIncludeDefinedForActionSuccess_ColsForActionT AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, includedCols: new HashSet { "col1", "col2" }, - excludedCols: new HashSet { "*" } + excludedCols: new HashSet { AuthorizationResolver.WILDCARD } ); AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); diff --git a/DataGateway.Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs b/DataGateway.Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs index f4694a1703..84cd22d506 100644 --- a/DataGateway.Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs +++ b/DataGateway.Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs @@ -347,16 +347,15 @@ IEnumerable columnsRequested } /// - /// Sets up an authorization resolver with a config that specifies the wildcard ("*") as the test entity's actions + /// Sets up an authorization resolver with a config that specifies the wildcard ("*") as the test entity's actions. + /// Explicitly use this instead of AuthorizationHelpers.InitRuntimeConfig() because we want to create actions as + /// array of string instead of array of object. /// private static AuthorizationResolver SetupAuthResolverWithWildcardActions() { - string roleName = "admin"; - string entityName = AuthorizationHelpers.TEST_ENTITY; - PermissionSetting permissionForEntity = new( - role: roleName, - actions: new object[] { JsonSerializer.SerializeToElement("*") }); + role: "admin", + actions: new object[] { JsonSerializer.SerializeToElement(AuthorizationResolver.WILDCARD) }); Entity sampleEntity = new( Source: AuthorizationHelpers.TEST_ENTITY, @@ -368,7 +367,7 @@ private static AuthorizationResolver SetupAuthResolverWithWildcardActions() ); Dictionary entityMap = new(); - entityMap.Add(entityName, sampleEntity); + entityMap.Add(AuthorizationHelpers.TEST_ENTITY, sampleEntity); RuntimeConfig runtimeConfig = new( Schema: "UnitTestSchema", diff --git a/DataGateway.Service/Authorization/AuthorizationResolver.cs b/DataGateway.Service/Authorization/AuthorizationResolver.cs index 67699d383b..763d84051e 100644 --- a/DataGateway.Service/Authorization/AuthorizationResolver.cs +++ b/DataGateway.Service/Authorization/AuthorizationResolver.cs @@ -25,7 +25,7 @@ namespace Azure.DataGateway.Service.Authorization public class AuthorizationResolver : IAuthorizationResolver { private ISqlMetadataProvider _metadataProvider; - private const string WILDCARD = "*"; + public const string WILDCARD = "*"; public const string CLAIM_PREFIX = "@claims."; public const string FIELD_PREFIX = "@item."; public const string CLIENT_ROLE_HEADER = "X-MS-API-ROLE"; diff --git a/DataGateway.Service/Configurations/RuntimeConfigValidator.cs b/DataGateway.Service/Configurations/RuntimeConfigValidator.cs index 97a6c791a9..220dd3c6dd 100644 --- a/DataGateway.Service/Configurations/RuntimeConfigValidator.cs +++ b/DataGateway.Service/Configurations/RuntimeConfigValidator.cs @@ -162,10 +162,10 @@ public void ValidatePermissionsInConfig(RuntimeConfig runtimeConfig) // Check if the IncludeSet/ExcludeSet contain wildcard. If they contain wildcard, we make sure that they // don't contain any other field. If they do, we throw an appropriate exception. - if (configAction.Fields!.Include.Contains("*") && configAction.Fields.Include.Count > 1 || - configAction.Fields.Exclude.Contains("*") && configAction.Fields.Exclude.Count > 1) + if (configAction.Fields!.Include.Contains(AuthorizationResolver.WILDCARD) && configAction.Fields.Include.Count > 1 || + configAction.Fields.Exclude.Contains(AuthorizationResolver.WILDCARD) && configAction.Fields.Exclude.Count > 1) { - string incExc = configAction.Fields.Include.Contains("*") && configAction.Fields.Include.Count > 1 ? "included" : "excluded"; + string incExc = configAction.Fields.Include.Contains(AuthorizationResolver.WILDCARD) && configAction.Fields.Include.Count > 1 ? "included" : "excluded"; throw new DataGatewayException( message: $"No other field can be present with wildcard in the {incExc} set for: entity:{entityName}," + $" role:{permissionSetting.Role}, action:{actionName}", @@ -389,8 +389,8 @@ private static void AreFieldsAccessible(string policy, HashSet includedF private static bool IsFieldAccessible(Match columnNameMatch, HashSet includedFields, HashSet excludedFields) { string columnName = columnNameMatch.Value.Substring(AuthorizationResolver.FIELD_PREFIX.Length); - if (excludedFields.Contains(columnName!) || excludedFields.Contains("*") || - !(includedFields.Contains("*") || includedFields.Contains(columnName))) + if (excludedFields.Contains(columnName!) || excludedFields.Contains(AuthorizationResolver.WILDCARD) || + !(includedFields.Contains(AuthorizationResolver.WILDCARD) || includedFields.Contains(columnName))) { // If column is present in excluded OR excluded='*' // If column is absent from included and included!=* @@ -472,7 +472,7 @@ private static void ValidateActionName(string actionName, string entityName, str /// Boolean value indicating whether the actionName is valid or not. public static bool IsValidActionName(string actionName) { - return actionName.Equals("*") || _validActions.Contains(actionName); + return actionName.Equals(AuthorizationResolver.WILDCARD) || _validActions.Contains(actionName); } } } From a37b8fa6cd2ce8833b627a70ab2eed75e627772f Mon Sep 17 00:00:00 2001 From: Jarupat Jisarojito Date: Tue, 19 Jul 2022 17:47:16 -0700 Subject: [PATCH 03/20] Update code formatting --- DataGateway.Auth/IAuthorizationResolver.cs | 9 +++++++-- .../Authorization/AuthorizationResolver.cs | 5 +++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/DataGateway.Auth/IAuthorizationResolver.cs b/DataGateway.Auth/IAuthorizationResolver.cs index 934a84de7e..f418a2ea10 100644 --- a/DataGateway.Auth/IAuthorizationResolver.cs +++ b/DataGateway.Auth/IAuthorizationResolver.cs @@ -91,14 +91,19 @@ public interface IAuthorizationResolver /// Entity to lookup permissions /// Action to lookup applicable roles /// Collection of roles. Empty list if entityPermissionsMap is null. - public static IEnumerable GetRolesForAction(string entityName, string actionName, Dictionary? entityPermissionsMap) + public static IEnumerable GetRolesForAction( + string entityName, + string actionName, + Dictionary? entityPermissionsMap) { if (entityName is null) { throw new ArgumentNullException(paramName: "entityName"); } - if (entityPermissionsMap is not null && entityPermissionsMap[entityName].ActionToRolesMap.TryGetValue(actionName, out List? roleList) && roleList is not null) + if (entityPermissionsMap is not null && + entityPermissionsMap[entityName].ActionToRolesMap.TryGetValue(actionName, out List? roleList) && + roleList is not null) { return roleList; } diff --git a/DataGateway.Service/Authorization/AuthorizationResolver.cs b/DataGateway.Service/Authorization/AuthorizationResolver.cs index 763d84051e..31a508103c 100644 --- a/DataGateway.Service/Authorization/AuthorizationResolver.cs +++ b/DataGateway.Service/Authorization/AuthorizationResolver.cs @@ -141,7 +141,7 @@ public bool AreColumnsAllowedForAction(string entityName, string roleName, strin { // backingColumn will not be null when TryGetBackingColumn() is true. if (actionToColumnMap.Excluded.Contains(backingColumn!) || actionToColumnMap.Excluded.Contains(WILDCARD) || - !(actionToColumnMap.Included.Contains(WILDCARD) || actionToColumnMap.Included.Contains(backingColumn!))) + !(actionToColumnMap.Included.Contains(WILDCARD) || actionToColumnMap.Included.Contains(backingColumn!))) { // If column is present in excluded OR excluded='*' // If column is absent from included and included!=* @@ -289,7 +289,8 @@ public void SetEntityPermissionMap(RuntimeConfig? runtimeConfig) // Try to add the actionName to the map if not present. // Builds up mapping: i.e. ActionType.CREATE permitted in {Role1, Role2, ..., RoleN} - if (!string.IsNullOrWhiteSpace(actionName) && !entityToRoleMap.ActionToRolesMap.TryAdd(actionName, new List(new string[] { role }))) + if (!string.IsNullOrWhiteSpace(actionName) && + !entityToRoleMap.ActionToRolesMap.TryAdd(actionName, new List(new string[] { role }))) { entityToRoleMap.ActionToRolesMap[actionName].Add(role); } From 32982ea95b5956e16984c5f0b40800ddc5294e05 Mon Sep 17 00:00:00 2001 From: Jarupat Jisarojito Date: Tue, 19 Jul 2022 18:14:12 -0700 Subject: [PATCH 04/20] Expand wildcard actions --- .../Authorization/AuthorizationResolver.cs | 54 ++++++++++++++++--- 1 file changed, 47 insertions(+), 7 deletions(-) diff --git a/DataGateway.Service/Authorization/AuthorizationResolver.cs b/DataGateway.Service/Authorization/AuthorizationResolver.cs index 31a508103c..c2603eedd0 100644 --- a/DataGateway.Service/Authorization/AuthorizationResolver.cs +++ b/DataGateway.Service/Authorization/AuthorizationResolver.cs @@ -289,18 +289,24 @@ public void SetEntityPermissionMap(RuntimeConfig? runtimeConfig) // Try to add the actionName to the map if not present. // Builds up mapping: i.e. ActionType.CREATE permitted in {Role1, Role2, ..., RoleN} - if (!string.IsNullOrWhiteSpace(actionName) && - !entityToRoleMap.ActionToRolesMap.TryAdd(actionName, new List(new string[] { role }))) + // Expand wildcard action to explicit action type. + // + if (actionName.Equals(AuthorizationResolver.WILDCARD)) { - entityToRoleMap.ActionToRolesMap[actionName].Add(role); + AddRoleToAction(entityToRoleMap.ActionToRolesMap, ActionType.READ, role); + AddRoleToAction(entityToRoleMap.ActionToRolesMap, ActionType.CREATE, role); + AddRoleToAction(entityToRoleMap.ActionToRolesMap, ActionType.DELETE, role); + AddRoleToAction(entityToRoleMap.ActionToRolesMap, ActionType.UPDATE, role); } - - foreach (string allowedColumn in actionToColumn.Allowed) + // Otherwise, we know explicit action name. Just add that. + // + else if (!string.IsNullOrWhiteSpace(actionName)) { - entityToRoleMap.FieldToRolesMap.TryAdd(key: allowedColumn, CreateActionToRoleMap()); - entityToRoleMap.FieldToRolesMap[allowedColumn][actionName].Add(role); + AddRoleToAction(entityToRoleMap.ActionToRolesMap, actionName, role); } + PopuldateFieldToRoleMap(entityToRoleMap.FieldToRolesMap, role, actionName, actionToColumn); + roleToAction.ActionToColumnMap[actionName] = actionToColumn; } @@ -311,6 +317,40 @@ public void SetEntityPermissionMap(RuntimeConfig? runtimeConfig) } } + private static void PopuldateFieldToRoleMap( + Dictionary>> fieldToRolesMap, + string role, + string actionName, + ActionMetadata actionToColumn) + { + foreach (string allowedColumn in actionToColumn.Allowed) + { + fieldToRolesMap.TryAdd(key: allowedColumn, CreateActionToRoleMap()); + + // Expand wildcard action to explicit action type. + // + if (actionName.Equals(AuthorizationResolver.WILDCARD)) + { + fieldToRolesMap[allowedColumn][ActionType.READ].Add(role); + fieldToRolesMap[allowedColumn][ActionType.CREATE].Add(role); + fieldToRolesMap[allowedColumn][ActionType.DELETE].Add(role); + fieldToRolesMap[allowedColumn][ActionType.UPDATE].Add(role); + } + else + { + fieldToRolesMap[allowedColumn][actionName].Add(role); + } + } + } + + private static void AddRoleToAction(Dictionary> actionToRolesMap, string actionName, string role) + { + if (!actionToRolesMap.TryAdd(actionName, new List(new string[] { role }))) + { + actionToRolesMap[actionName].Add(role); + } + } + /// public IEnumerable GetAllowedColumns(string entityName, string roleName, string action) { From 4fc4b221e98d47417cfad9ce295aefc6f7703593 Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Wed, 20 Jul 2022 09:38:20 +0530 Subject: [PATCH 05/20] Wildcard action gets resolved to CRUD actions --- .../Authorization/AuthorizationResolver.cs | 74 +++++++++---------- .../Configurations/RuntimeConfigValidator.cs | 2 +- 2 files changed, 34 insertions(+), 42 deletions(-) diff --git a/DataGateway.Service/Authorization/AuthorizationResolver.cs b/DataGateway.Service/Authorization/AuthorizationResolver.cs index 67699d383b..bcdd97851f 100644 --- a/DataGateway.Service/Authorization/AuthorizationResolver.cs +++ b/DataGateway.Service/Authorization/AuthorizationResolver.cs @@ -102,8 +102,7 @@ public bool AreRoleAndActionDefinedForEntity(string entityName, string roleName, { if (valueOfEntityToRole.RoleToActionMap.TryGetValue(roleName, out RoleMetadata? valueOfRoleToAction)) { - if (valueOfRoleToAction!.ActionToColumnMap.ContainsKey(WILDCARD) || - valueOfRoleToAction!.ActionToColumnMap.ContainsKey(action)) + if (valueOfRoleToAction!.ActionToColumnMap.ContainsKey(action)) { return true; } @@ -119,17 +118,7 @@ public bool AreColumnsAllowedForAction(string entityName, string roleName, strin // Columns.Count() will never be zero because this method is called after a check ensures Count() > 0 Assert.IsFalse(columns.Count() == 0, message: "columns.Count() should be greater than 0."); - ActionMetadata actionToColumnMap; - RoleMetadata roleInEntity = EntityPermissionsMap[entityName].RoleToActionMap[roleName]; - - try - { - actionToColumnMap = roleInEntity.ActionToColumnMap[actionName]; - } - catch (KeyNotFoundException) - { - actionToColumnMap = roleInEntity.ActionToColumnMap[WILDCARD]; - } + ActionMetadata actionToColumnMap = EntityPermissionsMap[entityName].RoleToActionMap[roleName].ActionToColumnMap[actionName]; // Each column present in the request is an "exposedColumn". // Authorization permissions reference "backingColumns" @@ -189,20 +178,8 @@ private string GetDBPolicyForRequest(string entityName, string roleName, string RoleMetadata roleMetadata = EntityPermissionsMap[entityName].RoleToActionMap[roleName]; roleMetadata.ActionToColumnMap.TryGetValue(action, out ActionMetadata? actionMetadata); - // If action exists in map (explicitly specified in config), use its policy - // action should only be absent in roleMetadata if WILDCARD is in the map instead of specific actions, - // as authorization happens before policy parsing (would have already returned forbidden) - string? dbPolicy; - if (actionMetadata is not null) - { - dbPolicy = actionMetadata.DatabasePolicy; - - } // else check if wildcard exists in action map, if so use its policy, else null - else - { - roleMetadata.ActionToColumnMap.TryGetValue(WILDCARD, out ActionMetadata? wildcardMetadata); - dbPolicy = wildcardMetadata is not null ? wildcardMetadata.DatabasePolicy : null; - } + // Get the database policy for the specified action. + string? dbPolicy = actionMetadata!.DatabasePolicy; return dbPolicy is not null ? dbPolicy : string.Empty; } @@ -227,7 +204,7 @@ public void SetEntityPermissionMap(RuntimeConfig? runtimeConfig) object[] Actions = permission.Actions; foreach (JsonElement actionElement in Actions) { - string actionName = string.Empty; + string action = string.Empty; ActionMetadata actionToColumn = new(); IEnumerable allTableColumns = ResolveTableDefinitionColumns(entityName); @@ -235,7 +212,7 @@ public void SetEntityPermissionMap(RuntimeConfig? runtimeConfig) // Since no granular field permissions exist for this action within the current role. if (actionElement.ValueKind is JsonValueKind.String) { - actionName = actionElement.ToString(); + action = actionElement.ToString(); actionToColumn.Included.UnionWith(allTableColumns); actionToColumn.Allowed.UnionWith(allTableColumns); } @@ -246,7 +223,7 @@ public void SetEntityPermissionMap(RuntimeConfig? runtimeConfig) if (RuntimeConfig.TryGetDeserializedConfig(actionElement.ToString(), out Action? actionObj) && actionObj is not null) { - actionName = actionObj.Name; + action = actionObj.Name; if (actionObj.Fields!.Include is not null) { // When a wildcard (*) is defined for Included columns, all of the table's @@ -287,20 +264,24 @@ public void SetEntityPermissionMap(RuntimeConfig? runtimeConfig) } } - // Try to add the actionName to the map if not present. - // Builds up mapping: i.e. ActionType.CREATE permitted in {Role1, Role2, ..., RoleN} - if (!string.IsNullOrWhiteSpace(actionName) && !entityToRoleMap.ActionToRolesMap.TryAdd(actionName, new List(new string[] { role }))) + HashSet actionNames = GetAllActions(action); + foreach (string actionName in actionNames) { - entityToRoleMap.ActionToRolesMap[actionName].Add(role); - } + // Try to add the actionName to the map if not present. + // Builds up mapping: i.e. ActionType.CREATE permitted in {Role1, Role2, ..., RoleN} + if (!string.IsNullOrWhiteSpace(actionName) && !entityToRoleMap.ActionToRolesMap.TryAdd(actionName, new List(new string[] { role }))) + { + entityToRoleMap.ActionToRolesMap[actionName].Add(role); + } - foreach (string allowedColumn in actionToColumn.Allowed) - { - entityToRoleMap.FieldToRolesMap.TryAdd(key: allowedColumn, CreateActionToRoleMap()); - entityToRoleMap.FieldToRolesMap[allowedColumn][actionName].Add(role); - } + foreach (string allowedColumn in actionToColumn.Allowed) + { + entityToRoleMap.FieldToRolesMap.TryAdd(key: allowedColumn, CreateActionToRoleMap()); + entityToRoleMap.FieldToRolesMap[allowedColumn][actionName].Add(role); + } - roleToAction.ActionToColumnMap[actionName] = actionToColumn; + roleToAction.ActionToColumnMap[actionName] = actionToColumn; + } } entityToRoleMap.RoleToActionMap[role] = roleToAction; @@ -310,6 +291,17 @@ public void SetEntityPermissionMap(RuntimeConfig? runtimeConfig) } } + /// + /// Helper method to resolve action into the corresponding actionNames. + /// In case the action is a wildcard, it gets resolved to a set of CRUD operations. + /// + /// + /// + private static HashSet GetAllActions(string action) + { + return "*".Equals(action) ? RuntimeConfigValidator._validActions: new HashSet { action }; + } + /// public IEnumerable GetAllowedColumns(string entityName, string roleName, string action) { diff --git a/DataGateway.Service/Configurations/RuntimeConfigValidator.cs b/DataGateway.Service/Configurations/RuntimeConfigValidator.cs index 97a6c791a9..829b3712aa 100644 --- a/DataGateway.Service/Configurations/RuntimeConfigValidator.cs +++ b/DataGateway.Service/Configurations/RuntimeConfigValidator.cs @@ -35,7 +35,7 @@ public class RuntimeConfigValidator : IConfigValidator private static readonly string _claimChars = @"@claims\.[^\s\)]*"; // Set of allowed actions for a request. - private static readonly HashSet _validActions = new() { ActionType.CREATE, ActionType.READ, ActionType.UPDATE, ActionType.DELETE }; + public static readonly HashSet _validActions = new() { ActionType.CREATE, ActionType.READ, ActionType.UPDATE, ActionType.DELETE }; public RuntimeConfigValidator( RuntimeConfigProvider runtimeConfigProvider, From c024972060c79765a5a37f899b63306ea2db0e03 Mon Sep 17 00:00:00 2001 From: Jarupat Jisarojito Date: Tue, 19 Jul 2022 21:13:57 -0700 Subject: [PATCH 06/20] Remove wildcard special case from authorization checks. --- .../Authorization/AuthorizationResolver.cs | 67 ++++++------------- DataGateway.Service/Services/RestService.cs | 8 +-- 2 files changed, 24 insertions(+), 51 deletions(-) diff --git a/DataGateway.Service/Authorization/AuthorizationResolver.cs b/DataGateway.Service/Authorization/AuthorizationResolver.cs index c2603eedd0..a2c7922f2f 100644 --- a/DataGateway.Service/Authorization/AuthorizationResolver.cs +++ b/DataGateway.Service/Authorization/AuthorizationResolver.cs @@ -102,8 +102,7 @@ public bool AreRoleAndActionDefinedForEntity(string entityName, string roleName, { if (valueOfEntityToRole.RoleToActionMap.TryGetValue(roleName, out RoleMetadata? valueOfRoleToAction)) { - if (valueOfRoleToAction!.ActionToColumnMap.ContainsKey(WILDCARD) || - valueOfRoleToAction!.ActionToColumnMap.ContainsKey(action)) + if (valueOfRoleToAction!.ActionToColumnMap.ContainsKey(action)) { return true; } @@ -122,14 +121,7 @@ public bool AreColumnsAllowedForAction(string entityName, string roleName, strin ActionMetadata actionToColumnMap; RoleMetadata roleInEntity = EntityPermissionsMap[entityName].RoleToActionMap[roleName]; - try - { - actionToColumnMap = roleInEntity.ActionToColumnMap[actionName]; - } - catch (KeyNotFoundException) - { - actionToColumnMap = roleInEntity.ActionToColumnMap[WILDCARD]; - } + actionToColumnMap = roleInEntity.ActionToColumnMap[actionName]; // Each column present in the request is an "exposedColumn". // Authorization permissions reference "backingColumns" @@ -140,8 +132,8 @@ public bool AreColumnsAllowedForAction(string entityName, string roleName, strin if (_metadataProvider.TryGetBackingColumn(entityName, field: exposedColumn, out string? backingColumn)) { // backingColumn will not be null when TryGetBackingColumn() is true. - if (actionToColumnMap.Excluded.Contains(backingColumn!) || actionToColumnMap.Excluded.Contains(WILDCARD) || - !(actionToColumnMap.Included.Contains(WILDCARD) || actionToColumnMap.Included.Contains(backingColumn!))) + if (actionToColumnMap.Excluded.Contains(backingColumn!) || + !actionToColumnMap.Included.Contains(backingColumn!)) { // If column is present in excluded OR excluded='*' // If column is absent from included and included!=* @@ -189,20 +181,7 @@ private string GetDBPolicyForRequest(string entityName, string roleName, string RoleMetadata roleMetadata = EntityPermissionsMap[entityName].RoleToActionMap[roleName]; roleMetadata.ActionToColumnMap.TryGetValue(action, out ActionMetadata? actionMetadata); - // If action exists in map (explicitly specified in config), use its policy - // action should only be absent in roleMetadata if WILDCARD is in the map instead of specific actions, - // as authorization happens before policy parsing (would have already returned forbidden) - string? dbPolicy; - if (actionMetadata is not null) - { - dbPolicy = actionMetadata.DatabasePolicy; - - } // else check if wildcard exists in action map, if so use its policy, else null - else - { - roleMetadata.ActionToColumnMap.TryGetValue(WILDCARD, out ActionMetadata? wildcardMetadata); - dbPolicy = wildcardMetadata is not null ? wildcardMetadata.DatabasePolicy : null; - } + string? dbPolicy = actionMetadata!.DatabasePolicy; return dbPolicy is not null ? dbPolicy : string.Empty; } @@ -291,23 +270,29 @@ public void SetEntityPermissionMap(RuntimeConfig? runtimeConfig) // Builds up mapping: i.e. ActionType.CREATE permitted in {Role1, Role2, ..., RoleN} // Expand wildcard action to explicit action type. // - if (actionName.Equals(AuthorizationResolver.WILDCARD)) + if (actionName.Equals(WILDCARD)) { - AddRoleToAction(entityToRoleMap.ActionToRolesMap, ActionType.READ, role); - AddRoleToAction(entityToRoleMap.ActionToRolesMap, ActionType.CREATE, role); - AddRoleToAction(entityToRoleMap.ActionToRolesMap, ActionType.DELETE, role); - AddRoleToAction(entityToRoleMap.ActionToRolesMap, ActionType.UPDATE, role); + string[] allAvailableActions = { ActionType.READ, ActionType.CREATE, ActionType.DELETE, ActionType.UPDATE }; + + foreach (string action in allAvailableActions) + { + AddRoleToAction(entityToRoleMap.ActionToRolesMap, action, role); + + PopuldateFieldToRoleMap(entityToRoleMap.FieldToRolesMap, role, action, actionToColumn); + + roleToAction.ActionToColumnMap[action] = actionToColumn; + } } // Otherwise, we know explicit action name. Just add that. // else if (!string.IsNullOrWhiteSpace(actionName)) { AddRoleToAction(entityToRoleMap.ActionToRolesMap, actionName, role); - } - PopuldateFieldToRoleMap(entityToRoleMap.FieldToRolesMap, role, actionName, actionToColumn); + PopuldateFieldToRoleMap(entityToRoleMap.FieldToRolesMap, role, actionName, actionToColumn); - roleToAction.ActionToColumnMap[actionName] = actionToColumn; + roleToAction.ActionToColumnMap[actionName] = actionToColumn; + } } entityToRoleMap.RoleToActionMap[role] = roleToAction; @@ -327,19 +312,7 @@ private static void PopuldateFieldToRoleMap( { fieldToRolesMap.TryAdd(key: allowedColumn, CreateActionToRoleMap()); - // Expand wildcard action to explicit action type. - // - if (actionName.Equals(AuthorizationResolver.WILDCARD)) - { - fieldToRolesMap[allowedColumn][ActionType.READ].Add(role); - fieldToRolesMap[allowedColumn][ActionType.CREATE].Add(role); - fieldToRolesMap[allowedColumn][ActionType.DELETE].Add(role); - fieldToRolesMap[allowedColumn][ActionType.UPDATE].Add(role); - } - else - { - fieldToRolesMap[allowedColumn][actionName].Add(role); - } + fieldToRolesMap[allowedColumn][actionName].Add(role); } } diff --git a/DataGateway.Service/Services/RestService.cs b/DataGateway.Service/Services/RestService.cs index d9490039e8..3757356843 100644 --- a/DataGateway.Service/Services/RestService.cs +++ b/DataGateway.Service/Services/RestService.cs @@ -263,16 +263,16 @@ public static string HttpVerbToActions(string httpVerbName) switch (httpVerbName) { case "POST": - return "create"; + return ActionType.CREATE; case "PUT": case "PATCH": // Please refer to the use of this method, which is to look out for policy based on crud operation type. // Since create doesn't have filter predicates, PUT/PATCH would resolve to update operation. - return "update"; + return ActionType.UPDATE;; case "DELETE": - return "delete"; + return ActionType.DELETE; case "GET": - return "read"; + return ActionType.READ; default: throw new DataGatewayException( message: "Unsupported operation type.", From 0aba9313b51eb5807cd95f537b390fcf7b85efa3 Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Wed, 20 Jul 2022 11:27:46 +0530 Subject: [PATCH 07/20] Adding test cases for wildcard action --- .../AuthorizationResolverUnitTests.cs | 96 ++++++++++++++++++- .../Authorization/AuthorizationResolver.cs | 2 +- 2 files changed, 96 insertions(+), 2 deletions(-) diff --git a/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs b/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs index ba5408d039..cf22442aa3 100644 --- a/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs +++ b/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs @@ -87,7 +87,6 @@ public void NoRoleHeader_RoleContextTest() Assert.AreEqual(authZResolver.IsValidRoleContext(context.Object), expected); } #endregion - #region Role and Action on Entity Tests /// /// Tests the AreRoleAndActionDefinedForEntity stage of authorization. /// Request Action is defined for role -> VALID @@ -95,6 +94,7 @@ public void NoRoleHeader_RoleContextTest() /// Ensures method short ciruits in circumstances role is not defined -> INVALID /// Request Action does not match an action defined for role (role has >=1 defined action) -> INVALID /// + #region Role and Action on Entity Tests [DataTestMethod] [DataRow("Writer", ActionType.CREATE, "Writer", ActionType.CREATE, true)] [DataRow("Reader", ActionType.CREATE, "Reader", "", false)] @@ -112,6 +112,48 @@ public void AreRoleAndActionDefinedForEntityTest( // Mock Request Values Assert.AreEqual(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, roleName, actionName), expected); } + + /// + /// Test to validate that for wildcard action, the authorization stage for role/action check + /// would pass if the action is one among create, read, update, delete. + /// + [TestMethod] + public void AreRoleAndActionDefinedForEntityViaWildcardAction() + { + RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig( + AuthorizationHelpers.TEST_ENTITY, + AuthorizationHelpers.TEST_ROLE, + "*", + includedCols: new HashSet { "*" } + ); + AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); + + bool expected = true; + Assert.AreEqual(expected, authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, TEST_ROLE, ActionType.CREATE)); + Assert.AreEqual(expected, authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, TEST_ROLE, ActionType.READ)); + Assert.AreEqual(expected, authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, TEST_ROLE, ActionType.UPDATE)); + Assert.AreEqual(expected, authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, TEST_ROLE, ActionType.DELETE)); + } + + /// + /// Test to validate that for wildcard action, the authorization stage for role/action check + /// would fail if the action is not one among create, read, update, delete. + /// + [TestMethod] + public void UndefinedRoleOrActionDefinedForEntityViaWildcardAction() + { + RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig( + AuthorizationHelpers.TEST_ENTITY, + AuthorizationHelpers.TEST_ROLE, + "*", + includedCols: new HashSet { "*" } + ); + AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); + + bool expected = false; + Assert.AreEqual(expected, authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, TEST_ROLE, "patch")); + Assert.AreEqual(expected, authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, TEST_ROLE, "fetch")); + } #endregion /// @@ -194,6 +236,32 @@ public void WildcardIncludeColsSomeExcludeDefinedForActionSuccess_ColsForActionT Assert.AreEqual(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, columns), expected); } + + /// + /// Test to validate that for wildcard action, the authorization stage for column check + /// would pass if the action is one among create, read, update, delete and the columns are accessible. + /// + [TestMethod] + public void AreColumnsAllowedForActionViaWildcardAction_ColsForActionTest() + { + RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig( + AuthorizationHelpers.TEST_ENTITY, + AuthorizationHelpers.TEST_ROLE, + "*", + includedCols: new HashSet { "col1", "col2" }, + excludedCols: new HashSet { "col3" } + ); + + // Mock Request Values - Query a configured entity/role/action with column allowed. + List columns = new(new string[] { "col1", "col2" }); + AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); + + bool expected = true; + Assert.AreEqual(expected, authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, columns)); + Assert.AreEqual(expected, authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.READ, columns)); + Assert.AreEqual(expected, authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.UPDATE, columns)); + Assert.AreEqual(expected, authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.DELETE, columns)); + } #endregion #region Negative Column Tests [TestMethod("Columns NOT allowed for action on role")] @@ -345,6 +413,32 @@ public void WildcardExcludeColsSomeIncludeDefinedForActionSuccess_ColsForActionT Assert.AreEqual(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, columns), expected); } + + /// + /// Test to validate that for wildcard action, the authorization stage for column check + /// would fail even if the action is one among create, read, update, delete and the columns are inaccessible. + /// + [TestMethod] + public void InaccessibleColumnsForActionViaWildcardAction_ColsForActionTest() + { + RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig( + AuthorizationHelpers.TEST_ENTITY, + AuthorizationHelpers.TEST_ROLE, + "*", + includedCols: new HashSet { "col1", "col2" }, + excludedCols: new HashSet { "col3" } + ); + + // Mock Request Values - Query a configured entity/role/action with column not allowed. + List columns = new(new string[] { "col1", "col3" }); + AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); + + bool expected = false; + Assert.AreEqual(expected, authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, columns)); + Assert.AreEqual(expected, authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.READ, columns)); + Assert.AreEqual(expected, authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.UPDATE, columns)); + Assert.AreEqual(expected, authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.DELETE, columns)); + } #endregion #region Tests to validate Database policy parsing diff --git a/DataGateway.Service/Authorization/AuthorizationResolver.cs b/DataGateway.Service/Authorization/AuthorizationResolver.cs index bcdd97851f..2766dd0cc5 100644 --- a/DataGateway.Service/Authorization/AuthorizationResolver.cs +++ b/DataGateway.Service/Authorization/AuthorizationResolver.cs @@ -293,7 +293,7 @@ public void SetEntityPermissionMap(RuntimeConfig? runtimeConfig) /// /// Helper method to resolve action into the corresponding actionNames. - /// In case the action is a wildcard, it gets resolved to a set of CRUD operations. + /// In case the action is a wildcard(*), it gets resolved to a set of CRUD operations. /// /// /// From 882fbf5af9ceb6c6114a5c7136fbe6631969aae9 Mon Sep 17 00:00:00 2001 From: Jarupat Jisarojito Date: Tue, 19 Jul 2022 23:54:48 -0700 Subject: [PATCH 08/20] Add unittests for wildcard handling --- .../AuthorizationResolverUnitTests.cs | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs b/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs index 09d42e2acc..d40c782101 100644 --- a/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs +++ b/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs @@ -110,7 +110,31 @@ public void AreRoleAndActionDefinedForEntityTest( AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); // Mock Request Values - Assert.AreEqual(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, roleName, actionName), expected); + Assert.AreEqual(expected, authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, roleName, actionName)); + } + + /// + /// Test that wildcard actions are expanded to explicit actions. + /// + [TestMethod] + public void TestWildcardAction() + { + string roleName = "myRole"; + RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig(AuthorizationHelpers.TEST_ENTITY, roleName, "*"); + runtimeConfig.Entities[AuthorizationHelpers.TEST_ENTITY].Permissions[0].Actions = new object[] { JsonSerializer.SerializeToElement(AuthorizationResolver.WILDCARD) }; + AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); + + // There should not be a wildcard action in AuthorizationResolver.EntityPermissionsMap + // + Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, roleName, "*")); + + // All the wildcard action should be expand to explicit actions. + // + string[] allAvailableActions = { ActionType.READ, ActionType.CREATE, ActionType.DELETE, ActionType.UPDATE }; + foreach (string action in allAvailableActions) + { + Assert.IsTrue(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, roleName, action)); + } } #endregion From cfdcf040e6fc7962836d72b1ac00d05a54d75efb Mon Sep 17 00:00:00 2001 From: Jarupat Jisarojito Date: Wed, 20 Jul 2022 00:33:44 -0700 Subject: [PATCH 09/20] Add tests that verify internal data structure when wildcard is used. --- .../AuthorizationMetadataHelpers.cs | 11 ++++--- .../AuthorizationResolverUnitTests.cs | 21 ++++++++++-- .../REST/RestAuthorizationHandlerUnitTests.cs | 32 ++++--------------- .../Authorization/AuthorizationResolver.cs | 12 +++---- 4 files changed, 39 insertions(+), 37 deletions(-) diff --git a/DataGateway.Auth/AuthorizationMetadataHelpers.cs b/DataGateway.Auth/AuthorizationMetadataHelpers.cs index b7873bd70d..ef44cd77d0 100644 --- a/DataGateway.Auth/AuthorizationMetadataHelpers.cs +++ b/DataGateway.Auth/AuthorizationMetadataHelpers.cs @@ -14,10 +14,13 @@ public class EntityMetadata public Dictionary RoleToActionMap { get; set; } = new(); /// - /// Given the key (actionName) returns a key/value collection of fieldName to Roles - /// i.e. READ action - /// Key(field): id -> Value(collection): permitted in {Role1, Role2, ..., RoleN} - /// Key(field): title -> Value(collection): permitted in {Role1} + /// Field to action to role mapping. + /// Given the key (Field aka. column name) returns a key/value collection of action to Roles + /// i.e. ID column + /// Key(field): id -> Dictionary(actions) + /// each entry in the dictionary contains action to role map. + /// create: permitted in {Role1, Role2, ..., RoleN} + /// delete: permitted in {Role1, RoleN} /// public Dictionary>> FieldToRolesMap { get; set; } = new(); diff --git a/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs b/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs index d40c782101..933e2eab2e 100644 --- a/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs +++ b/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs @@ -1,5 +1,6 @@ #nullable enable using System.Collections.Generic; +using System.Linq; using System.Net; using System.Security.Claims; using System.Text.Json; @@ -87,7 +88,9 @@ public void NoRoleHeader_RoleContextTest() Assert.AreEqual(authZResolver.IsValidRoleContext(context.Object), expected); } #endregion + #region Role and Action on Entity Tests + /// /// Tests the AreRoleAndActionDefinedForEntity stage of authorization. /// Request Action is defined for role -> VALID @@ -115,18 +118,24 @@ public void AreRoleAndActionDefinedForEntityTest( /// /// Test that wildcard actions are expanded to explicit actions. + /// Verifies that internal data structure are created correctly. /// [TestMethod] public void TestWildcardAction() { string roleName = "myRole"; - RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig(AuthorizationHelpers.TEST_ENTITY, roleName, "*"); + List expectedRoles = new() { roleName }; + + RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig(AuthorizationHelpers.TEST_ENTITY, roleName, AuthorizationResolver.WILDCARD); + + // Override the action to be a list of string for wildcard instead of a list of object created by InitRuntimeConfig() + // runtimeConfig.Entities[AuthorizationHelpers.TEST_ENTITY].Permissions[0].Actions = new object[] { JsonSerializer.SerializeToElement(AuthorizationResolver.WILDCARD) }; AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); // There should not be a wildcard action in AuthorizationResolver.EntityPermissionsMap // - Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, roleName, "*")); + Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, roleName, AuthorizationResolver.WILDCARD)); // All the wildcard action should be expand to explicit actions. // @@ -134,8 +143,16 @@ public void TestWildcardAction() foreach (string action in allAvailableActions) { Assert.IsTrue(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, roleName, action)); + + IEnumerable actualRolesForCol1 = authZResolver.GetRolesForField(AuthorizationHelpers.TEST_ENTITY, "col1", action); + + CollectionAssert.AreEquivalent(expectedRoles, actualRolesForCol1.ToList()); } + + IEnumerable actualRolesForAction = authZResolver.GetRolesForAction(AuthorizationHelpers.TEST_ENTITY, ActionType.CREATE); + CollectionAssert.AreEquivalent(expectedRoles, actualRolesForAction.ToList()); } + #endregion /// diff --git a/DataGateway.Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs b/DataGateway.Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs index 84cd22d506..d4943d9da1 100644 --- a/DataGateway.Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs +++ b/DataGateway.Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs @@ -353,32 +353,14 @@ IEnumerable columnsRequested /// private static AuthorizationResolver SetupAuthResolverWithWildcardActions() { - PermissionSetting permissionForEntity = new( - role: "admin", - actions: new object[] { JsonSerializer.SerializeToElement(AuthorizationResolver.WILDCARD) }); - - Entity sampleEntity = new( - Source: AuthorizationHelpers.TEST_ENTITY, - Rest: null, - GraphQL: null, - Permissions: new PermissionSetting[] { permissionForEntity }, - Relationships: null, - Mappings: null - ); + RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig( + entityName: AuthorizationHelpers.TEST_ENTITY, + roleName: "admin", + actionName: "*"); - Dictionary entityMap = new(); - entityMap.Add(AuthorizationHelpers.TEST_ENTITY, sampleEntity); - - RuntimeConfig runtimeConfig = new( - Schema: "UnitTestSchema", - MsSql: null, - CosmosDb: null, - PostgreSql: null, - MySql: null, - DataSource: new DataSource(DatabaseType: DatabaseType.mssql), - RuntimeSettings: new Dictionary(), - Entities: entityMap - ); + // Override the action to be a list of string for wildcard instead of a list of object created by InitRuntimeConfig() + // + runtimeConfig.Entities[AuthorizationHelpers.TEST_ENTITY].Permissions[0].Actions = new object[] { JsonSerializer.SerializeToElement(AuthorizationResolver.WILDCARD) }; return AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); } diff --git a/DataGateway.Service/Authorization/AuthorizationResolver.cs b/DataGateway.Service/Authorization/AuthorizationResolver.cs index a2c7922f2f..29e17f576f 100644 --- a/DataGateway.Service/Authorization/AuthorizationResolver.cs +++ b/DataGateway.Service/Authorization/AuthorizationResolver.cs @@ -276,9 +276,9 @@ public void SetEntityPermissionMap(RuntimeConfig? runtimeConfig) foreach (string action in allAvailableActions) { - AddRoleToAction(entityToRoleMap.ActionToRolesMap, action, role); + PopulateActionToRoleMap(entityToRoleMap.ActionToRolesMap, action, role); - PopuldateFieldToRoleMap(entityToRoleMap.FieldToRolesMap, role, action, actionToColumn); + PopulateFieldToRoleMap(entityToRoleMap.FieldToRolesMap, role, action, actionToColumn); roleToAction.ActionToColumnMap[action] = actionToColumn; } @@ -287,9 +287,9 @@ public void SetEntityPermissionMap(RuntimeConfig? runtimeConfig) // else if (!string.IsNullOrWhiteSpace(actionName)) { - AddRoleToAction(entityToRoleMap.ActionToRolesMap, actionName, role); + PopulateActionToRoleMap(entityToRoleMap.ActionToRolesMap, actionName, role); - PopuldateFieldToRoleMap(entityToRoleMap.FieldToRolesMap, role, actionName, actionToColumn); + PopulateFieldToRoleMap(entityToRoleMap.FieldToRolesMap, role, actionName, actionToColumn); roleToAction.ActionToColumnMap[actionName] = actionToColumn; } @@ -302,7 +302,7 @@ public void SetEntityPermissionMap(RuntimeConfig? runtimeConfig) } } - private static void PopuldateFieldToRoleMap( + private static void PopulateFieldToRoleMap( Dictionary>> fieldToRolesMap, string role, string actionName, @@ -316,7 +316,7 @@ private static void PopuldateFieldToRoleMap( } } - private static void AddRoleToAction(Dictionary> actionToRolesMap, string actionName, string role) + private static void PopulateActionToRoleMap(Dictionary> actionToRolesMap, string actionName, string role) { if (!actionToRolesMap.TryAdd(actionName, new List(new string[] { role }))) { From d60e75cafcb6af39ee58da02781630dfdfbe63af Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Wed, 20 Jul 2022 13:51:54 +0530 Subject: [PATCH 10/20] Using defined constants for wildcard and actions --- .../AuthorizationResolverUnitTests.cs | 24 +++++++++---------- .../Authorization/AuthorizationResolver.cs | 4 ++-- .../Configurations/RuntimeConfigValidator.cs | 12 +++++----- DataGateway.Service/Services/RestService.cs | 8 +++---- 4 files changed, 24 insertions(+), 24 deletions(-) diff --git a/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs b/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs index cf22442aa3..55f0c984ce 100644 --- a/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs +++ b/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs @@ -123,8 +123,8 @@ public void AreRoleAndActionDefinedForEntityViaWildcardAction() RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig( AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, - "*", - includedCols: new HashSet { "*" } + AuthorizationResolver.WILDCARD, + includedCols: new HashSet { AuthorizationResolver.WILDCARD } ); AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); @@ -145,8 +145,8 @@ public void UndefinedRoleOrActionDefinedForEntityViaWildcardAction() RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig( AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, - "*", - includedCols: new HashSet { "*" } + AuthorizationResolver.WILDCARD, + includedCols: new HashSet { AuthorizationResolver.WILDCARD } ); AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); @@ -207,7 +207,7 @@ public void WildcardIncludeColDefinedForAction_ColsForActionTest() AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, - includedCols: new HashSet { "*" } + includedCols: new HashSet { AuthorizationResolver.WILDCARD } ); AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); @@ -225,7 +225,7 @@ public void WildcardIncludeColsSomeExcludeDefinedForActionSuccess_ColsForActionT AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, - includedCols: new HashSet { "*" }, + includedCols: new HashSet { AuthorizationResolver.WILDCARD }, excludedCols: new HashSet { "col1", "col2" } ); AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); @@ -247,7 +247,7 @@ public void AreColumnsAllowedForActionViaWildcardAction_ColsForActionTest() RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig( AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, - "*", + AuthorizationResolver.WILDCARD, includedCols: new HashSet { "col1", "col2" }, excludedCols: new HashSet { "col3" } ); @@ -346,7 +346,7 @@ public void WildcardExcludeColsDefinedForAction_ColsForActionTest() AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, - excludedCols: new HashSet { "*" } + excludedCols: new HashSet { AuthorizationResolver.WILDCARD } ); AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); @@ -364,7 +364,7 @@ public void WildcardIncludeColsSomeExcludeDefinedForAction_ColsForActionTest() AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, - includedCols: new HashSet { "*" }, + includedCols: new HashSet { AuthorizationResolver.WILDCARD }, excludedCols: new HashSet { "col1", "col2" } ); AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); @@ -384,7 +384,7 @@ public void WildcardExcludeColsSomeIncludeDefinedForAction_ColsForActionTest() AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, includedCols: new HashSet { "col1", "col2" }, - excludedCols: new HashSet { "*" } + excludedCols: new HashSet { AuthorizationResolver.WILDCARD } ); AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); @@ -403,7 +403,7 @@ public void WildcardExcludeColsSomeIncludeDefinedForActionSuccess_ColsForActionT AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, includedCols: new HashSet { "col1", "col2" }, - excludedCols: new HashSet { "*" } + excludedCols: new HashSet { AuthorizationResolver.WILDCARD } ); AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); @@ -424,7 +424,7 @@ public void InaccessibleColumnsForActionViaWildcardAction_ColsForActionTest() RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig( AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, - "*", + AuthorizationResolver.WILDCARD, includedCols: new HashSet { "col1", "col2" }, excludedCols: new HashSet { "col3" } ); diff --git a/DataGateway.Service/Authorization/AuthorizationResolver.cs b/DataGateway.Service/Authorization/AuthorizationResolver.cs index 2766dd0cc5..0810bf37a0 100644 --- a/DataGateway.Service/Authorization/AuthorizationResolver.cs +++ b/DataGateway.Service/Authorization/AuthorizationResolver.cs @@ -25,7 +25,7 @@ namespace Azure.DataGateway.Service.Authorization public class AuthorizationResolver : IAuthorizationResolver { private ISqlMetadataProvider _metadataProvider; - private const string WILDCARD = "*"; + public const string WILDCARD = "*"; public const string CLAIM_PREFIX = "@claims."; public const string FIELD_PREFIX = "@item."; public const string CLIENT_ROLE_HEADER = "X-MS-API-ROLE"; @@ -299,7 +299,7 @@ public void SetEntityPermissionMap(RuntimeConfig? runtimeConfig) /// private static HashSet GetAllActions(string action) { - return "*".Equals(action) ? RuntimeConfigValidator._validActions: new HashSet { action }; + return "*".Equals(action) ? RuntimeConfigValidator._validActions : new HashSet { action }; } /// diff --git a/DataGateway.Service/Configurations/RuntimeConfigValidator.cs b/DataGateway.Service/Configurations/RuntimeConfigValidator.cs index 829b3712aa..f1a4b7be5d 100644 --- a/DataGateway.Service/Configurations/RuntimeConfigValidator.cs +++ b/DataGateway.Service/Configurations/RuntimeConfigValidator.cs @@ -162,10 +162,10 @@ public void ValidatePermissionsInConfig(RuntimeConfig runtimeConfig) // Check if the IncludeSet/ExcludeSet contain wildcard. If they contain wildcard, we make sure that they // don't contain any other field. If they do, we throw an appropriate exception. - if (configAction.Fields!.Include.Contains("*") && configAction.Fields.Include.Count > 1 || - configAction.Fields.Exclude.Contains("*") && configAction.Fields.Exclude.Count > 1) + if (configAction.Fields!.Include.Contains(AuthorizationResolver.WILDCARD) && configAction.Fields.Include.Count > 1 || + configAction.Fields.Exclude.Contains(AuthorizationResolver.WILDCARD) && configAction.Fields.Exclude.Count > 1) { - string incExc = configAction.Fields.Include.Contains("*") && configAction.Fields.Include.Count > 1 ? "included" : "excluded"; + string incExc = configAction.Fields.Include.Contains(AuthorizationResolver.WILDCARD) && configAction.Fields.Include.Count > 1 ? "included" : "excluded"; throw new DataGatewayException( message: $"No other field can be present with wildcard in the {incExc} set for: entity:{entityName}," + $" role:{permissionSetting.Role}, action:{actionName}", @@ -389,8 +389,8 @@ private static void AreFieldsAccessible(string policy, HashSet includedF private static bool IsFieldAccessible(Match columnNameMatch, HashSet includedFields, HashSet excludedFields) { string columnName = columnNameMatch.Value.Substring(AuthorizationResolver.FIELD_PREFIX.Length); - if (excludedFields.Contains(columnName!) || excludedFields.Contains("*") || - !(includedFields.Contains("*") || includedFields.Contains(columnName))) + if (excludedFields.Contains(columnName!) || excludedFields.Contains(AuthorizationResolver.WILDCARD) || + !(includedFields.Contains(AuthorizationResolver.WILDCARD) || includedFields.Contains(columnName))) { // If column is present in excluded OR excluded='*' // If column is absent from included and included!=* @@ -472,7 +472,7 @@ private static void ValidateActionName(string actionName, string entityName, str /// Boolean value indicating whether the actionName is valid or not. public static bool IsValidActionName(string actionName) { - return actionName.Equals("*") || _validActions.Contains(actionName); + return actionName.Equals(AuthorizationResolver.WILDCARD) || _validActions.Contains(actionName); } } } diff --git a/DataGateway.Service/Services/RestService.cs b/DataGateway.Service/Services/RestService.cs index d9490039e8..ea38592dbe 100644 --- a/DataGateway.Service/Services/RestService.cs +++ b/DataGateway.Service/Services/RestService.cs @@ -263,16 +263,16 @@ public static string HttpVerbToActions(string httpVerbName) switch (httpVerbName) { case "POST": - return "create"; + return ActionType.CREATE; case "PUT": case "PATCH": // Please refer to the use of this method, which is to look out for policy based on crud operation type. // Since create doesn't have filter predicates, PUT/PATCH would resolve to update operation. - return "update"; + return ActionType.UPDATE; case "DELETE": - return "delete"; + return ActionType.DELETE; case "GET": - return "read"; + return ActionType.READ; default: throw new DataGatewayException( message: "Unsupported operation type.", From a5079399bd3f6cdb01507efd77d2545244f98d18 Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Wed, 20 Jul 2022 14:18:51 +0530 Subject: [PATCH 11/20] Using Assert true/false --- .../AuthorizationResolverUnitTests.cs | 38 ++++++++++--------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs b/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs index 55f0c984ce..1547b030bd 100644 --- a/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs +++ b/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs @@ -5,6 +5,7 @@ using System.Text.Json; using Azure.DataGateway.Config; using Azure.DataGateway.Service.Authorization; +using Azure.DataGateway.Service.Configurations; using Azure.DataGateway.Service.Exceptions; using Azure.DataGateway.Service.Models; using Microsoft.AspNetCore.Http; @@ -128,11 +129,11 @@ public void AreRoleAndActionDefinedForEntityViaWildcardAction() ); AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); - bool expected = true; - Assert.AreEqual(expected, authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, TEST_ROLE, ActionType.CREATE)); - Assert.AreEqual(expected, authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, TEST_ROLE, ActionType.READ)); - Assert.AreEqual(expected, authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, TEST_ROLE, ActionType.UPDATE)); - Assert.AreEqual(expected, authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, TEST_ROLE, ActionType.DELETE)); + foreach (string actionName in RuntimeConfigValidator._validActions) + { + // Validate that the authorization check passes for valid CRUD actions. + Assert.IsTrue(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, TEST_ROLE, actionName)); + } } /// @@ -150,9 +151,9 @@ public void UndefinedRoleOrActionDefinedForEntityViaWildcardAction() ); AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); - bool expected = false; - Assert.AreEqual(expected, authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, TEST_ROLE, "patch")); - Assert.AreEqual(expected, authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, TEST_ROLE, "fetch")); + // Validate that the authorization check fails because the actions are invalid. + Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, TEST_ROLE, "patch")); + Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, TEST_ROLE, "fetch")); } #endregion @@ -256,11 +257,12 @@ public void AreColumnsAllowedForActionViaWildcardAction_ColsForActionTest() List columns = new(new string[] { "col1", "col2" }); AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); - bool expected = true; - Assert.AreEqual(expected, authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, columns)); - Assert.AreEqual(expected, authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.READ, columns)); - Assert.AreEqual(expected, authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.UPDATE, columns)); - Assert.AreEqual(expected, authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.DELETE, columns)); + foreach (string actionName in RuntimeConfigValidator._validActions) + { + // Validate that the authorization check passes for valid CRUD actions + // because columns are accessbile. + Assert.IsTrue(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, actionName, columns)); + } } #endregion #region Negative Column Tests @@ -433,11 +435,11 @@ public void InaccessibleColumnsForActionViaWildcardAction_ColsForActionTest() List columns = new(new string[] { "col1", "col3" }); AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); - bool expected = false; - Assert.AreEqual(expected, authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, columns)); - Assert.AreEqual(expected, authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.READ, columns)); - Assert.AreEqual(expected, authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.UPDATE, columns)); - Assert.AreEqual(expected, authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.DELETE, columns)); + foreach (string actionName in RuntimeConfigValidator._validActions) + { + // Validate that the authorization check fails as the some columns are inaccessible. + Assert.IsFalse(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, actionName, columns)); + } } #endregion From 838631fc0c9ad0988bd411a543d4bb6f80c87fb3 Mon Sep 17 00:00:00 2001 From: Jarupat Jisarojito Date: Wed, 20 Jul 2022 10:05:56 -0700 Subject: [PATCH 12/20] Add more test. --- .../AuthorizationResolverUnitTests.cs | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs b/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs index 933e2eab2e..11f03c107a 100644 --- a/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs +++ b/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs @@ -153,6 +153,93 @@ public void TestWildcardAction() CollectionAssert.AreEquivalent(expectedRoles, actualRolesForAction.ToList()); } + /// + /// Verify that the internal data structure is created correctly when we have + /// Two roles for the same entity with different permission. + /// readOnlyRole - Read permission only for col1 and no policy. + /// readAndUpdateRole - read and update permission for col1 and no policy. + /// + [TestMethod] + public void TestRoleAndActionCombination() + { + string readOnlyRole = "readOnlyRole"; + string readAndUpdateRole = "readAndUpdateRole"; + + Field fieldsForRole = new( + include: new HashSet { "col1" }, + exclude: null); + + Action readAction = new( + Name: ActionType.READ, + Fields: fieldsForRole, + Policy: null); + + Action updateAction = new( + Name: ActionType.UPDATE, + Fields: fieldsForRole, + Policy: null); + + PermissionSetting readOnlyPermission = new( + role: readOnlyRole, + actions: new object[] { JsonSerializer.SerializeToElement(readAction) }); + + PermissionSetting readAndUpdatePermission = new( + role: readAndUpdateRole, + actions: new object[] { JsonSerializer.SerializeToElement(readAction), JsonSerializer.SerializeToElement(updateAction) }); + + Entity sampleEntity = new( + Source: TEST_ENTITY, + Rest: null, + GraphQL: null, + Permissions: new PermissionSetting[] { readOnlyPermission, readAndUpdatePermission }, + Relationships: null, + Mappings: null + ); + + Dictionary entityMap = new(); + entityMap.Add(AuthorizationHelpers.TEST_ENTITY, sampleEntity); + + RuntimeConfig runtimeConfig = new( + Schema: "UnitTestSchema", + MsSql: null, + CosmosDb: null, + PostgreSql: null, + MySql: null, + DataSource: new DataSource(DatabaseType: DatabaseType.mssql), + RuntimeSettings: new Dictionary(), + Entities: entityMap + ); + + AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); + + // Verify that read only role has permission for read and nothing else. + // + Assert.IsTrue(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, readOnlyRole, ActionType.READ)); + Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, readOnlyRole, ActionType.UPDATE)); + Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, readOnlyRole, ActionType.CREATE)); + Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, readOnlyRole, ActionType.DELETE)); + + // Verify that read only role has permission for read and nothing else. + // + Assert.IsTrue(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, readAndUpdateRole, ActionType.READ)); + Assert.IsTrue(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, readAndUpdateRole, ActionType.UPDATE)); + Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, readAndUpdateRole, ActionType.CREATE)); + Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, readAndUpdateRole, ActionType.DELETE)); + + List expectedRolesForRead = new() { readOnlyRole, readAndUpdateRole }; + List expectedRolesForUpdate = new() { readAndUpdateRole }; + + IEnumerable actualReadRolesForCol1 = authZResolver.GetRolesForField(AuthorizationHelpers.TEST_ENTITY, "col1", ActionType.READ); + CollectionAssert.AreEquivalent(expectedRolesForRead, actualReadRolesForCol1.ToList()); + IEnumerable actualUpdateRolesForCol1 = authZResolver.GetRolesForField(AuthorizationHelpers.TEST_ENTITY, "col1", ActionType.UPDATE); + CollectionAssert.AreEquivalent(expectedRolesForUpdate, actualUpdateRolesForCol1.ToList()); + + IEnumerable actualRolesForRead = authZResolver.GetRolesForAction(AuthorizationHelpers.TEST_ENTITY, ActionType.READ); + CollectionAssert.AreEquivalent(expectedRolesForRead, actualRolesForRead.ToList()); + IEnumerable actualRolesForUpdate = authZResolver.GetRolesForAction(AuthorizationHelpers.TEST_ENTITY, ActionType.UPDATE); + CollectionAssert.AreEquivalent(expectedRolesForUpdate, actualRolesForUpdate.ToList()); + } + #endregion /// From 1161320708050806835f41593572cdba9ff0bb2b Mon Sep 17 00:00:00 2001 From: Jarupat Jisarojito Date: Wed, 20 Jul 2022 10:33:53 -0700 Subject: [PATCH 13/20] Add more comprehensive tests for AuthorizationResolver --- .../AuthorizationResolverUnitTests.cs | 77 ++++++------------- .../Authorization/AuthorizationResolver.cs | 31 ++------ 2 files changed, 27 insertions(+), 81 deletions(-) diff --git a/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs b/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs index 7e12745858..187ab82f99 100644 --- a/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs +++ b/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs @@ -90,6 +90,7 @@ public void NoRoleHeader_RoleContextTest() } #endregion + #region Role and Action on Entity Tests /// /// Tests the AreRoleAndActionDefinedForEntity stage of authorization. @@ -98,7 +99,6 @@ public void NoRoleHeader_RoleContextTest() /// Ensures method short ciruits in circumstances role is not defined -> INVALID /// Request Action does not match an action defined for role (role has >=1 defined action) -> INVALID /// - #region Role and Action on Entity Tests [DataTestMethod] [DataRow("Writer", ActionType.CREATE, "Writer", ActionType.CREATE, true)] [DataRow("Reader", ActionType.CREATE, "Reader", "", false)] @@ -117,59 +117,19 @@ public void AreRoleAndActionDefinedForEntityTest( Assert.AreEqual(expected, authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, roleName, actionName)); } - /// - /// Test to validate that for wildcard action, the authorization stage for role/action check - /// would pass if the action is one among create, read, update, delete. - /// - [TestMethod] - public void AreRoleAndActionDefinedForEntityViaWildcardAction() - { - RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig( - AuthorizationHelpers.TEST_ENTITY, - AuthorizationHelpers.TEST_ROLE, - AuthorizationResolver.WILDCARD, - includedCols: new HashSet { AuthorizationResolver.WILDCARD } - ); - AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); - - foreach (string actionName in RuntimeConfigValidator._validActions) - { - // Validate that the authorization check passes for valid CRUD actions. - Assert.IsTrue(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, TEST_ROLE, actionName)); - } - } - - /// - /// Test to validate that for wildcard action, the authorization stage for role/action check - /// would fail if the action is not one among create, read, update, delete. - /// - [TestMethod] - public void UndefinedRoleOrActionDefinedForEntityViaWildcardAction() - { - RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig( - AuthorizationHelpers.TEST_ENTITY, - AuthorizationHelpers.TEST_ROLE, - AuthorizationResolver.WILDCARD, - includedCols: new HashSet { AuthorizationResolver.WILDCARD } - ); - AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); - - // Validate that the authorization check fails because the actions are invalid. - Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, TEST_ROLE, "patch")); - Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, TEST_ROLE, "fetch")); - } - /// /// Test that wildcard actions are expanded to explicit actions. /// Verifies that internal data structure are created correctly. /// - [TestMethod] + [TestMethod("Wildcard actions are expand to all valid actions")] public void TestWildcardAction() { - string roleName = "myRole"; - List expectedRoles = new() { roleName }; + List expectedRoles = new() { AuthorizationHelpers.TEST_ROLE }; - RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig(AuthorizationHelpers.TEST_ENTITY, roleName, AuthorizationResolver.WILDCARD); + RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig( + AuthorizationHelpers.TEST_ENTITY, + AuthorizationHelpers.TEST_ROLE, + AuthorizationResolver.WILDCARD); // Override the action to be a list of string for wildcard instead of a list of object created by InitRuntimeConfig() // @@ -178,22 +138,26 @@ public void TestWildcardAction() // There should not be a wildcard action in AuthorizationResolver.EntityPermissionsMap // - Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, roleName, AuthorizationResolver.WILDCARD)); + Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, AuthorizationResolver.WILDCARD)); // All the wildcard action should be expand to explicit actions. // - string[] allAvailableActions = { ActionType.READ, ActionType.CREATE, ActionType.DELETE, ActionType.UPDATE }; - foreach (string action in allAvailableActions) + foreach (string actionName in RuntimeConfigValidator._validActions) { - Assert.IsTrue(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, roleName, action)); + Assert.IsTrue(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, actionName)); - IEnumerable actualRolesForCol1 = authZResolver.GetRolesForField(AuthorizationHelpers.TEST_ENTITY, "col1", action); + IEnumerable actualRolesForCol1 = authZResolver.GetRolesForField(AuthorizationHelpers.TEST_ENTITY, "col1", actionName); CollectionAssert.AreEquivalent(expectedRoles, actualRolesForCol1.ToList()); } IEnumerable actualRolesForAction = authZResolver.GetRolesForAction(AuthorizationHelpers.TEST_ENTITY, ActionType.CREATE); CollectionAssert.AreEquivalent(expectedRoles, actualRolesForAction.ToList()); + + // Validate that the authorization check fails because the actions are invalid. + // + Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, TEST_ROLE, "patch")); + Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, TEST_ROLE, "fetch")); } /// @@ -205,8 +169,8 @@ public void TestWildcardAction() [TestMethod] public void TestRoleAndActionCombination() { - string readOnlyRole = "readOnlyRole"; - string readAndUpdateRole = "readAndUpdateRole"; + const string readOnlyRole = "readOnlyRole"; + const string readAndUpdateRole = "readAndUpdateRole"; Field fieldsForRole = new( include: new HashSet { "col1" }, @@ -285,6 +249,8 @@ public void TestRoleAndActionCombination() #endregion + #region Positive Column Tests + /// /// Tests the authorization stage: Columns defined for Action /// Columns are allowed for role @@ -292,7 +258,6 @@ public void TestRoleAndActionCombination() /// Wildcard included and/or excluded columns handling /// and assumes request validation has already occurred /// - #region Positive Column Tests [TestMethod("Column allowed for action on role")] public void ColsDefinedForAction_ColsForActionTest() { @@ -393,6 +358,7 @@ public void AreColumnsAllowedForActionViaWildcardAction_ColsForActionTest() } } #endregion + #region Negative Column Tests [TestMethod("Columns NOT allowed for action on role")] public void ColsNotDefinedForAction_ColsForActionTest() @@ -714,6 +680,7 @@ public void ParsePolicyWithDuplicateUserClaims(bool exceptionExpected, params st } } #endregion + #region Helpers public static RuntimeConfig InitRuntimeConfig( string entityName = "SampleEntity", diff --git a/DataGateway.Service/Authorization/AuthorizationResolver.cs b/DataGateway.Service/Authorization/AuthorizationResolver.cs index 30b9d99bd2..5e6f33e3fc 100644 --- a/DataGateway.Service/Authorization/AuthorizationResolver.cs +++ b/DataGateway.Service/Authorization/AuthorizationResolver.cs @@ -264,12 +264,13 @@ public void SetEntityPermissionMap(RuntimeConfig? runtimeConfig) } } - HashSet actionNames = GetAllActions(action); + IEnumerable actionNames = GetAllActions(action); foreach (string actionName in actionNames) { // Try to add the actionName to the map if not present. // Builds up mapping: i.e. ActionType.CREATE permitted in {Role1, Role2, ..., RoleN} - if (!string.IsNullOrWhiteSpace(actionName) && !entityToRoleMap.ActionToRolesMap.TryAdd(actionName, new List(new string[] { role }))) + if (!string.IsNullOrWhiteSpace(actionName) && + !entityToRoleMap.ActionToRolesMap.TryAdd(actionName, new List(new string[] { role }))) { entityToRoleMap.ActionToRolesMap[actionName].Add(role); } @@ -291,37 +292,15 @@ public void SetEntityPermissionMap(RuntimeConfig? runtimeConfig) } } - private static void PopulateFieldToRoleMap( - Dictionary>> fieldToRolesMap, - string role, - string actionName, - ActionMetadata actionToColumn) - { - foreach (string allowedColumn in actionToColumn.Allowed) - { - fieldToRolesMap.TryAdd(key: allowedColumn, CreateActionToRoleMap()); - - fieldToRolesMap[allowedColumn][actionName].Add(role); - } - } - - private static void PopulateActionToRoleMap(Dictionary> actionToRolesMap, string actionName, string role) - { - if (!actionToRolesMap.TryAdd(actionName, new List(new string[] { role }))) - { - actionToRolesMap[actionName].Add(role); - } - } - /// /// Helper method to resolve action into the corresponding actionNames. /// In case the action is a wildcard(*), it gets resolved to a set of CRUD operations. /// /// /// - private static HashSet GetAllActions(string action) + private static IEnumerable GetAllActions(string action) { - return "*".Equals(action) ? RuntimeConfigValidator._validActions : new HashSet { action }; + return WILDCARD.Equals(action) ? RuntimeConfigValidator._validActions : new List { action }; } /// From 668c90157727ef3128669cf400ffb9da0b38e535 Mon Sep 17 00:00:00 2001 From: Jarupat Jisarojito Date: Wed, 20 Jul 2022 11:41:55 -0700 Subject: [PATCH 14/20] Clean up include exclude column test. Remove unnessary duplicate tests and combine the verifications in the same test. --- .../AuthorizationResolverUnitTests.cs | 254 +++++------------- .../Authorization/AuthorizationResolver.cs | 4 +- 2 files changed, 72 insertions(+), 186 deletions(-) diff --git a/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs b/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs index 187ab82f99..4f5e50af71 100644 --- a/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs +++ b/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs @@ -249,7 +249,7 @@ public void TestRoleAndActionCombination() #endregion - #region Positive Column Tests + #region Column Tests /// /// Tests the authorization stage: Columns defined for Action @@ -258,283 +258,169 @@ public void TestRoleAndActionCombination() /// Wildcard included and/or excluded columns handling /// and assumes request validation has already occurred /// - [TestMethod("Column allowed for action on role")] - public void ColsDefinedForAction_ColsForActionTest() + [TestMethod("Explicit include columns with no exclusion")] + public void ExplicitIncludeColumn() { + HashSet includedColumns = new() { "col1", "col2", "col3" }; RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig( AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, - includedCols: new HashSet { "col1", "col2", "col3" } + includedCols: includedColumns ); AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); - // Mock Request Values - Query a configured entity/role/action with column allowed. - List columns = new(new string[] { "col1" }); - bool expected = true; + Assert.IsTrue(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, includedColumns)); - Assert.AreEqual(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, columns), expected); - } - - [TestMethod("All Columns allowed for action on role")] - public void ColDefinedForAction_ColsForActionTest() - { - RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig( - AuthorizationHelpers.TEST_ENTITY, - AuthorizationHelpers.TEST_ROLE, - ActionType.CREATE, - includedCols: new HashSet { "col1", "col2", "col3" } - ); - AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); - - // Mock Request Values - Query a configured entity/role/action with columns allowed. - List columns = new(new string[] { "col1", "col2", "col3" }); - bool expected = true; - - Assert.AreEqual(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, columns), expected); - } - - [TestMethod("Wildcard included columns allowed for action on role")] - public void WildcardIncludeColDefinedForAction_ColsForActionTest() - { - RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig( - AuthorizationHelpers.TEST_ENTITY, - AuthorizationHelpers.TEST_ROLE, - ActionType.CREATE, - includedCols: new HashSet { AuthorizationResolver.WILDCARD } - ); - AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); - - // Mock Request Values - Query a configured entity/role/action with columns allowed. - List columns = new(new string[] { "col1", "col2", "col3" }); - bool expected = true; - - Assert.AreEqual(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, columns), expected); - } - - [TestMethod("Wildcard excluded columns with some included for action on role success")] - public void WildcardIncludeColsSomeExcludeDefinedForActionSuccess_ColsForActionTest() - { - RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig( - AuthorizationHelpers.TEST_ENTITY, - AuthorizationHelpers.TEST_ROLE, - ActionType.CREATE, - includedCols: new HashSet { AuthorizationResolver.WILDCARD }, - excludedCols: new HashSet { "col1", "col2" } - ); - AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); + // Not allow column. + // + Assert.IsFalse(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, new List { "col4" })); - // Mock Request Values - Query a configured entity/role/action with column allowed. - List columns = new(new string[] { "col3", "col4" }); - bool expected = true; + // Mix of allow and not allow. Should result in not allow. + Assert.IsFalse(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, new List { "col3", "col4" })); - Assert.AreEqual(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, columns), expected); + // Column does not exist + Assert.IsFalse(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, new List { "col5", "col6" })); } /// /// Test to validate that for wildcard action, the authorization stage for column check /// would pass if the action is one among create, read, update, delete and the columns are accessible. + /// Similarly if the column is in accessible, then we should not have access. /// - [TestMethod] - public void AreColumnsAllowedForActionViaWildcardAction_ColsForActionTest() + [TestMethod("Explicit include and exclude columns")] + public void ExplicitIncludeAndExcludeColumns() { - RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig( - AuthorizationHelpers.TEST_ENTITY, - AuthorizationHelpers.TEST_ROLE, - AuthorizationResolver.WILDCARD, - includedCols: new HashSet { "col1", "col2" }, - excludedCols: new HashSet { "col3" } - ); - - // Mock Request Values - Query a configured entity/role/action with column allowed. - List columns = new(new string[] { "col1", "col2" }); - AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); + HashSet includeColumns = new() { "col1", "col2" }; + HashSet excludeColumns = new() { "col3" }; - foreach (string actionName in RuntimeConfigValidator._validActions) - { - // Validate that the authorization check passes for valid CRUD actions - // because columns are accessbile. - Assert.IsTrue(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, actionName, columns)); - } - } - #endregion - - #region Negative Column Tests - [TestMethod("Columns NOT allowed for action on role")] - public void ColsNotDefinedForAction_ColsForActionTest() - { RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig( AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, - includedCols: new HashSet { "col1", "col2", "col3" } + includedCols: includeColumns, + excludedCols: excludeColumns ); - AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); - // Mock Request Values - Query a configured entity/role/action with column NOT allowed. - List columns = new(new string[] { "col4" }); - bool expected = false; - - Assert.AreEqual(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, columns), expected); - } - - [TestMethod("Columns NOT allowed for action on role - with some valid cols")] - public void ColsNotDefinedForAction2_ColsForActionTest() - { - RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig( - AuthorizationHelpers.TEST_ENTITY, - AuthorizationHelpers.TEST_ROLE, - ActionType.CREATE, - includedCols: new HashSet { "col1", "col2", "col3" } - ); AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); - // Mock Request Values - Query a configured entity/role/action - // to match all allowed columns, with one NOT allowed column. - List columns = new(new string[] { "col1", "col2", "col3", "col4" }); - bool expected = false; - - Assert.AreEqual(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, columns), expected); - } + Assert.IsTrue(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, includeColumns)); - [TestMethod("Columns NOT allowed for action on role - definition has inc/excl - req has only excluded cols")] - public void ColsNotDefinedForAction3_ColsForActionTest() - { - RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig( - AuthorizationHelpers.TEST_ENTITY, - AuthorizationHelpers.TEST_ROLE, - ActionType.CREATE, - includedCols: new HashSet { "col1", "col2", "col3" }, - excludedCols: new HashSet { "col4", "col5", "col6" } - ); - AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); + Assert.IsFalse(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, excludeColumns)); - // Mock Request Values - Query a configured entity/role/action with multiple columns NOT allowed. - List columns = new(new string[] { "col4", "col5" }); - bool expected = false; + // Not exist column in the inclusion or exclusion list + Assert.IsFalse(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, new List { "col4" })); - Assert.AreEqual(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, columns), expected); + // Mix of allow and not allow. Should result in not allow. + Assert.IsFalse(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, new List { "col1", "col3" })); } - [TestMethod("Columns NOT allowed for action on role - Mixed allowed/disallowed in req.")] - public void ColsNotDefinedForAction4Mixed_ColsForActionTest() + [TestMethod("Wildcard included columns")] + public void WildcardColumnInclusion() { RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig( AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, - includedCols: new HashSet { "col1", "col2", "col3" }, - excludedCols: new HashSet { "col4", "col5", "col6" } + includedCols: new HashSet { AuthorizationResolver.WILDCARD } ); AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); - // Mock Request Values - Query a configured entity/role/action with 1 allowed/ 1 disallwed column(s). - List columns = new(new string[] { "col2", "col5" }); - bool expected = false; + // Mock Request Values - Query a configured entity/role/action with columns allowed. + List includedColumns = new() { "col1", "col2", "col3", "col4"}; - Assert.AreEqual(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, columns), expected); + Assert.IsTrue(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, includedColumns)); } - [TestMethod("Wildcard excluded for action on role")] - public void WildcardExcludeColsDefinedForAction_ColsForActionTest() + [TestMethod("Wildcard include columns with some column exclusion")] + public void WildcardColumnInclusionWithExplictExclusion() { - RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig( - AuthorizationHelpers.TEST_ENTITY, - AuthorizationHelpers.TEST_ROLE, - ActionType.CREATE, - excludedCols: new HashSet { AuthorizationResolver.WILDCARD } - ); - AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); + List includedColumns = new() { "col1", "col2" }; + HashSet excludedColumns = new() { "col3", "col4" }; - // Mock Request Values - Query a configured entity/role/action with columns not allowed. - List columns = new(new string[] { "col1", "col2", "col3" }); - bool expected = false; - - Assert.AreEqual(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, columns), expected); - } - - [TestMethod("Wildcard include all except some columns for action on role")] - public void WildcardIncludeColsSomeExcludeDefinedForAction_ColsForActionTest() - { RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig( AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, includedCols: new HashSet { AuthorizationResolver.WILDCARD }, - excludedCols: new HashSet { "col1", "col2" } + excludedCols: excludedColumns ); AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); - // Mock Request Values - Query a configured entity/role/action with column allowed and column not allowed. - List columns = new(new string[] { "col3", "col1" }); - bool expected = false; - - Assert.AreEqual(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, columns), expected); + Assert.IsTrue(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, includedColumns)); + Assert.IsFalse(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, excludedColumns)); } - [TestMethod("Wildcard exclude all except for some columns for action on role - Request with excluded column")] - public void WildcardExcludeColsSomeIncludeDefinedForAction_ColsForActionTest() + [TestMethod("Wildcard column exclusion")] + public void WildcardColumnExclusion() { + HashSet excludedColumns = new() { "col1", "col2", "col3", "col4"}; + RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig( AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, - includedCols: new HashSet { "col1", "col2" }, excludedCols: new HashSet { AuthorizationResolver.WILDCARD } ); AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); - // Mock Request Values - Query a configured entity/role/action with two columns allowed, one not. - List columns = new(new string[] { "col1", "col2", "col3" }); - bool expected = false; - - Assert.AreEqual(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, columns), expected); + Assert.IsFalse(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, excludedColumns)); } - [TestMethod("Wildcard exclude all except some columns for action on role - Request with all included columns")] - public void WildcardExcludeColsSomeIncludeDefinedForActionSuccess_ColsForActionTest() + [Ignore] + [TestMethod("Wildcard column exclusion with some explicit columns inclusion")] + public void WildcardColumnExclusionWithExplicitColumnInclusion() { + HashSet includedColumns = new() { "col1", "col2"}; + HashSet excludedColumns = new() { "col3", "col4" }; + RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig( AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, - includedCols: new HashSet { "col1", "col2" }, + includedCols: includedColumns, excludedCols: new HashSet { AuthorizationResolver.WILDCARD } ); AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); - // Mock Request Values - Query a configured entity/role/action with columns allowed. - List columns = new(new string[] { "col1", "col2" }); - bool expected = false; + Assert.IsTrue(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, includedColumns)); + Assert.IsFalse(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, excludedColumns)); - Assert.AreEqual(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, columns), expected); + // Mix of include and exclud columns should result in failure. + // + Assert.IsFalse(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, new List { "col1", "col4" })); } /// /// Test to validate that for wildcard action, the authorization stage for column check - /// would fail even if the action is one among create, read, update, delete and the columns are inaccessible. + /// would pass if the action is one among create, read, update, delete and the columns are accessible. + /// Similarly if the column is in accessible, then we should not have access. /// - [TestMethod] - public void InaccessibleColumnsForActionViaWildcardAction_ColsForActionTest() + [TestMethod("Explicit include and exclude columns with wildcard actions")] + public void CheckIncludeAndExcludeColumnForWildcardAction() { + HashSet includeColumns = new() { "col1", "col2" }; + HashSet excludeColumns = new() { "col3" }; + RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig( AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, AuthorizationResolver.WILDCARD, - includedCols: new HashSet { "col1", "col2" }, - excludedCols: new HashSet { "col3" } + includedCols: includeColumns, + excludedCols: excludeColumns ); - // Mock Request Values - Query a configured entity/role/action with column not allowed. - List columns = new(new string[] { "col1", "col3" }); AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); foreach (string actionName in RuntimeConfigValidator._validActions) { - // Validate that the authorization check fails as the some columns are inaccessible. - Assert.IsFalse(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, actionName, columns)); + // Validate that the authorization check passes for valid CRUD actions + // because columns are accessbile or inaccessible. + Assert.IsTrue(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, actionName, includeColumns)); + Assert.IsFalse(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, actionName, excludeColumns)); } } + #endregion #region Tests to validate Database policy parsing diff --git a/DataGateway.Service/Authorization/AuthorizationResolver.cs b/DataGateway.Service/Authorization/AuthorizationResolver.cs index 5e6f33e3fc..a2552957bd 100644 --- a/DataGateway.Service/Authorization/AuthorizationResolver.cs +++ b/DataGateway.Service/Authorization/AuthorizationResolver.cs @@ -296,8 +296,8 @@ public void SetEntityPermissionMap(RuntimeConfig? runtimeConfig) /// Helper method to resolve action into the corresponding actionNames. /// In case the action is a wildcard(*), it gets resolved to a set of CRUD operations. /// - /// - /// + /// Action name. + /// IEnumerable of all available action name private static IEnumerable GetAllActions(string action) { return WILDCARD.Equals(action) ? RuntimeConfigValidator._validActions : new List { action }; From 7b5e3b1c965534ede131cc77dfebb6dced293953 Mon Sep 17 00:00:00 2001 From: Jarupat Jisarojito Date: Wed, 20 Jul 2022 11:53:03 -0700 Subject: [PATCH 15/20] Add test comments and fix expected behavior of WildcardColumnExclusionWithExplicitColumnInclusion --- .../AuthorizationResolverUnitTests.cs | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs b/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs index 4f5e50af71..94c8606909 100644 --- a/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs +++ b/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs @@ -315,6 +315,9 @@ public void ExplicitIncludeAndExcludeColumns() Assert.IsFalse(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, new List { "col1", "col3" })); } + /// + /// Test that wildcard inclusion will include all the columns in the table. + /// [TestMethod("Wildcard included columns")] public void WildcardColumnInclusion() { @@ -326,12 +329,15 @@ public void WildcardColumnInclusion() ); AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); - // Mock Request Values - Query a configured entity/role/action with columns allowed. List includedColumns = new() { "col1", "col2", "col3", "col4"}; Assert.IsTrue(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, includedColumns)); } + /// + /// Test that wildcard inclusion will include all column except column specify in exclusion. + /// Exclusion has priority over inclusion. + /// [TestMethod("Wildcard include columns with some column exclusion")] public void WildcardColumnInclusionWithExplictExclusion() { @@ -351,6 +357,9 @@ public void WildcardColumnInclusionWithExplictExclusion() Assert.IsFalse(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, excludedColumns)); } + /// + /// Test that all columns should be excluded if the exclusion contains wildcard character. + /// [TestMethod("Wildcard column exclusion")] public void WildcardColumnExclusion() { @@ -367,7 +376,10 @@ public void WildcardColumnExclusion() Assert.IsFalse(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, excludedColumns)); } - [Ignore] + /// + /// For this test, exclusion has precedence over inclusion. So for this test case, + /// all columns will be excluded. + /// [TestMethod("Wildcard column exclusion with some explicit columns inclusion")] public void WildcardColumnExclusionWithExplicitColumnInclusion() { @@ -383,12 +395,8 @@ public void WildcardColumnExclusionWithExplicitColumnInclusion() ); AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); - Assert.IsTrue(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, includedColumns)); + Assert.IsFalse(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, includedColumns)); Assert.IsFalse(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, excludedColumns)); - - // Mix of include and exclud columns should result in failure. - // - Assert.IsFalse(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, new List { "col1", "col4" })); } /// From 872f196776304024666c2c78828bff8c0f96dafa Mon Sep 17 00:00:00 2001 From: Jarupat Jisarojito Date: Wed, 20 Jul 2022 13:28:22 -0700 Subject: [PATCH 16/20] Run dotnet format --- .../Authorization/AuthorizationResolverUnitTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs b/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs index 94c8606909..337bff033a 100644 --- a/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs +++ b/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs @@ -329,7 +329,7 @@ public void WildcardColumnInclusion() ); AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); - List includedColumns = new() { "col1", "col2", "col3", "col4"}; + List includedColumns = new() { "col1", "col2", "col3", "col4" }; Assert.IsTrue(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, includedColumns)); } @@ -363,7 +363,7 @@ public void WildcardColumnInclusionWithExplictExclusion() [TestMethod("Wildcard column exclusion")] public void WildcardColumnExclusion() { - HashSet excludedColumns = new() { "col1", "col2", "col3", "col4"}; + HashSet excludedColumns = new() { "col1", "col2", "col3", "col4" }; RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig( AuthorizationHelpers.TEST_ENTITY, @@ -383,7 +383,7 @@ public void WildcardColumnExclusion() [TestMethod("Wildcard column exclusion with some explicit columns inclusion")] public void WildcardColumnExclusionWithExplicitColumnInclusion() { - HashSet includedColumns = new() { "col1", "col2"}; + HashSet includedColumns = new() { "col1", "col2" }; HashSet excludedColumns = new() { "col3", "col4" }; RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig( From ce0e2631251fad2f1e354054d6e78350cd89cd41 Mon Sep 17 00:00:00 2001 From: Jarupat Jisarojito Date: Wed, 20 Jul 2022 13:32:06 -0700 Subject: [PATCH 17/20] Apply suggestions from code review Co-authored-by: Aniruddh Munde Co-authored-by: aaronburtle <93220300+aaronburtle@users.noreply.github.com> --- DataGateway.Service/Authorization/AuthorizationResolver.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/DataGateway.Service/Authorization/AuthorizationResolver.cs b/DataGateway.Service/Authorization/AuthorizationResolver.cs index a2552957bd..83647f1e9f 100644 --- a/DataGateway.Service/Authorization/AuthorizationResolver.cs +++ b/DataGateway.Service/Authorization/AuthorizationResolver.cs @@ -293,10 +293,10 @@ public void SetEntityPermissionMap(RuntimeConfig? runtimeConfig) } /// - /// Helper method to resolve action into the corresponding actionNames. + /// Helper method to create a list consisting of the given action name. /// In case the action is a wildcard(*), it gets resolved to a set of CRUD operations. /// - /// Action name. + /// Action name. /// IEnumerable of all available action name private static IEnumerable GetAllActions(string action) { From fe2f82856b58124f0c3a106701e6bc9246650cdb Mon Sep 17 00:00:00 2001 From: Jarupat Jisarojito Date: Wed, 20 Jul 2022 13:38:38 -0700 Subject: [PATCH 18/20] Address comments. --- .../AuthorizationResolverUnitTests.cs | 38 +++++++++---------- .../Authorization/AuthorizationResolver.cs | 2 +- .../Configurations/RuntimeConfigValidator.cs | 4 +- 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs b/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs index 337bff033a..50af96ea44 100644 --- a/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs +++ b/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs @@ -142,17 +142,17 @@ public void TestWildcardAction() // All the wildcard action should be expand to explicit actions. // - foreach (string actionName in RuntimeConfigValidator._validActions) + foreach (string actionName in RuntimeConfigValidator.ValidActions) { Assert.IsTrue(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, actionName)); IEnumerable actualRolesForCol1 = authZResolver.GetRolesForField(AuthorizationHelpers.TEST_ENTITY, "col1", actionName); CollectionAssert.AreEquivalent(expectedRoles, actualRolesForCol1.ToList()); - } - IEnumerable actualRolesForAction = authZResolver.GetRolesForAction(AuthorizationHelpers.TEST_ENTITY, ActionType.CREATE); - CollectionAssert.AreEquivalent(expectedRoles, actualRolesForAction.ToList()); + IEnumerable actualRolesForAction = authZResolver.GetRolesForAction(AuthorizationHelpers.TEST_ENTITY, actionName); + CollectionAssert.AreEquivalent(expectedRoles, actualRolesForAction.ToList()); + } // Validate that the authorization check fails because the actions are invalid. // @@ -169,8 +169,8 @@ public void TestWildcardAction() [TestMethod] public void TestRoleAndActionCombination() { - const string readOnlyRole = "readOnlyRole"; - const string readAndUpdateRole = "readAndUpdateRole"; + const string READ_ONLY_ROLE = "readOnlyRole"; + const string READ_AND_UPDATE_ROLE = "readAndUpdateRole"; Field fieldsForRole = new( include: new HashSet { "col1" }, @@ -187,11 +187,11 @@ public void TestRoleAndActionCombination() Policy: null); PermissionSetting readOnlyPermission = new( - role: readOnlyRole, + role: READ_ONLY_ROLE , actions: new object[] { JsonSerializer.SerializeToElement(readAction) }); PermissionSetting readAndUpdatePermission = new( - role: readAndUpdateRole, + role: READ_AND_UPDATE_ROLE, actions: new object[] { JsonSerializer.SerializeToElement(readAction), JsonSerializer.SerializeToElement(updateAction) }); Entity sampleEntity = new( @@ -221,20 +221,20 @@ public void TestRoleAndActionCombination() // Verify that read only role has permission for read and nothing else. // - Assert.IsTrue(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, readOnlyRole, ActionType.READ)); - Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, readOnlyRole, ActionType.UPDATE)); - Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, readOnlyRole, ActionType.CREATE)); - Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, readOnlyRole, ActionType.DELETE)); + Assert.IsTrue(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, READ_ONLY_ROLE , ActionType.READ)); + Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, READ_ONLY_ROLE , ActionType.UPDATE)); + Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, READ_ONLY_ROLE , ActionType.CREATE)); + Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, READ_ONLY_ROLE , ActionType.DELETE)); // Verify that read only role has permission for read and nothing else. // - Assert.IsTrue(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, readAndUpdateRole, ActionType.READ)); - Assert.IsTrue(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, readAndUpdateRole, ActionType.UPDATE)); - Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, readAndUpdateRole, ActionType.CREATE)); - Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, readAndUpdateRole, ActionType.DELETE)); + Assert.IsTrue(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, READ_AND_UPDATE_ROLE, ActionType.READ)); + Assert.IsTrue(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, READ_AND_UPDATE_ROLE, ActionType.UPDATE)); + Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, READ_AND_UPDATE_ROLE, ActionType.CREATE)); + Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, READ_AND_UPDATE_ROLE, ActionType.DELETE)); - List expectedRolesForRead = new() { readOnlyRole, readAndUpdateRole }; - List expectedRolesForUpdate = new() { readAndUpdateRole }; + List expectedRolesForRead = new() { READ_ONLY_ROLE , READ_AND_UPDATE_ROLE }; + List expectedRolesForUpdate = new() { READ_AND_UPDATE_ROLE }; IEnumerable actualReadRolesForCol1 = authZResolver.GetRolesForField(AuthorizationHelpers.TEST_ENTITY, "col1", ActionType.READ); CollectionAssert.AreEquivalent(expectedRolesForRead, actualReadRolesForCol1.ToList()); @@ -420,7 +420,7 @@ public void CheckIncludeAndExcludeColumnForWildcardAction() AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); - foreach (string actionName in RuntimeConfigValidator._validActions) + foreach (string actionName in RuntimeConfigValidator.ValidActions) { // Validate that the authorization check passes for valid CRUD actions // because columns are accessbile or inaccessible. diff --git a/DataGateway.Service/Authorization/AuthorizationResolver.cs b/DataGateway.Service/Authorization/AuthorizationResolver.cs index 83647f1e9f..a1c77bc3e7 100644 --- a/DataGateway.Service/Authorization/AuthorizationResolver.cs +++ b/DataGateway.Service/Authorization/AuthorizationResolver.cs @@ -300,7 +300,7 @@ public void SetEntityPermissionMap(RuntimeConfig? runtimeConfig) /// IEnumerable of all available action name private static IEnumerable GetAllActions(string action) { - return WILDCARD.Equals(action) ? RuntimeConfigValidator._validActions : new List { action }; + return WILDCARD.Equals(action) ? RuntimeConfigValidator.ValidActions : new List { action }; } /// diff --git a/DataGateway.Service/Configurations/RuntimeConfigValidator.cs b/DataGateway.Service/Configurations/RuntimeConfigValidator.cs index f1a4b7be5d..37abfa37a7 100644 --- a/DataGateway.Service/Configurations/RuntimeConfigValidator.cs +++ b/DataGateway.Service/Configurations/RuntimeConfigValidator.cs @@ -35,7 +35,7 @@ public class RuntimeConfigValidator : IConfigValidator private static readonly string _claimChars = @"@claims\.[^\s\)]*"; // Set of allowed actions for a request. - public static readonly HashSet _validActions = new() { ActionType.CREATE, ActionType.READ, ActionType.UPDATE, ActionType.DELETE }; + public static readonly HashSet ValidActions = new() { ActionType.CREATE, ActionType.READ, ActionType.UPDATE, ActionType.DELETE }; public RuntimeConfigValidator( RuntimeConfigProvider runtimeConfigProvider, @@ -472,7 +472,7 @@ private static void ValidateActionName(string actionName, string entityName, str /// Boolean value indicating whether the actionName is valid or not. public static bool IsValidActionName(string actionName) { - return actionName.Equals(AuthorizationResolver.WILDCARD) || _validActions.Contains(actionName); + return actionName.Equals(AuthorizationResolver.WILDCARD) || ValidActions.Contains(actionName); } } } From f787e6b983026802edcc3d95effed9452d02dc19 Mon Sep 17 00:00:00 2001 From: Jarupat Jisarojito Date: Wed, 20 Jul 2022 14:34:33 -0700 Subject: [PATCH 19/20] Add test more test case to check exclusion precedence. --- .../AuthorizationResolverUnitTests.cs | 34 +++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs b/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs index 50af96ea44..51829d26ad 100644 --- a/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs +++ b/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs @@ -315,6 +315,36 @@ public void ExplicitIncludeAndExcludeColumns() Assert.IsFalse(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, new List { "col1", "col3" })); } + /// + /// Exclusion has precedence over inclusion. So for this test case, + /// col1 will be excluded even if it is in the inclusion list. + /// + [TestMethod("Same column in exclusion and inclusion list")] + public void ColumnExclusionWithSameColumnInclusion() + { + HashSet includedColumns = new() { "col1", "col2" }; + HashSet excludedColumns = new() { "col1", "col4" }; + + RuntimeConfig runtimeConfig = AuthorizationHelpers.InitRuntimeConfig( + AuthorizationHelpers.TEST_ENTITY, + AuthorizationHelpers.TEST_ROLE, + ActionType.CREATE, + includedCols: includedColumns, + excludedCols: excludedColumns + ); + AuthorizationResolver authZResolver = AuthorizationHelpers.InitAuthorizationResolver(runtimeConfig); + + // Col2 should be included. + // + Assert.IsTrue(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, new List { "col2" })); + + // Col1 should NOT to included since it is in exclusion list. + // + Assert.IsFalse(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, new List { "col1" })); + + Assert.IsFalse(authZResolver.AreColumnsAllowedForAction(AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, ActionType.CREATE, excludedColumns)); + } + /// /// Test that wildcard inclusion will include all the columns in the table. /// @@ -377,8 +407,8 @@ public void WildcardColumnExclusion() } /// - /// For this test, exclusion has precedence over inclusion. So for this test case, - /// all columns will be excluded. + /// For this test, exclusion has precedence over inclusion. So all columns will be excluded + /// because wildcard is specified in the exclusion list. /// [TestMethod("Wildcard column exclusion with some explicit columns inclusion")] public void WildcardColumnExclusionWithExplicitColumnInclusion() From f283d6d3041052c3d4e7133f0ea8347148502fdb Mon Sep 17 00:00:00 2001 From: Jarupat Jisarojito Date: Wed, 20 Jul 2022 14:36:42 -0700 Subject: [PATCH 20/20] Update format using dotnet format --- .../AuthorizationResolverUnitTests.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs b/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs index 51829d26ad..f68b5d1994 100644 --- a/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs +++ b/DataGateway.Service.Tests/Authorization/AuthorizationResolverUnitTests.cs @@ -169,7 +169,7 @@ public void TestWildcardAction() [TestMethod] public void TestRoleAndActionCombination() { - const string READ_ONLY_ROLE = "readOnlyRole"; + const string READ_ONLY_ROLE = "readOnlyRole"; const string READ_AND_UPDATE_ROLE = "readAndUpdateRole"; Field fieldsForRole = new( @@ -187,7 +187,7 @@ public void TestRoleAndActionCombination() Policy: null); PermissionSetting readOnlyPermission = new( - role: READ_ONLY_ROLE , + role: READ_ONLY_ROLE, actions: new object[] { JsonSerializer.SerializeToElement(readAction) }); PermissionSetting readAndUpdatePermission = new( @@ -221,10 +221,10 @@ public void TestRoleAndActionCombination() // Verify that read only role has permission for read and nothing else. // - Assert.IsTrue(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, READ_ONLY_ROLE , ActionType.READ)); - Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, READ_ONLY_ROLE , ActionType.UPDATE)); - Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, READ_ONLY_ROLE , ActionType.CREATE)); - Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, READ_ONLY_ROLE , ActionType.DELETE)); + Assert.IsTrue(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, READ_ONLY_ROLE, ActionType.READ)); + Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, READ_ONLY_ROLE, ActionType.UPDATE)); + Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, READ_ONLY_ROLE, ActionType.CREATE)); + Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, READ_ONLY_ROLE, ActionType.DELETE)); // Verify that read only role has permission for read and nothing else. // @@ -233,7 +233,7 @@ public void TestRoleAndActionCombination() Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, READ_AND_UPDATE_ROLE, ActionType.CREATE)); Assert.IsFalse(authZResolver.AreRoleAndActionDefinedForEntity(AuthorizationHelpers.TEST_ENTITY, READ_AND_UPDATE_ROLE, ActionType.DELETE)); - List expectedRolesForRead = new() { READ_ONLY_ROLE , READ_AND_UPDATE_ROLE }; + List expectedRolesForRead = new() { READ_ONLY_ROLE, READ_AND_UPDATE_ROLE }; List expectedRolesForUpdate = new() { READ_AND_UPDATE_ROLE }; IEnumerable actualReadRolesForCol1 = authZResolver.GetRolesForField(AuthorizationHelpers.TEST_ENTITY, "col1", ActionType.READ);