From bee28cdbf8941091e140bd8ce71a26b90c11b2de Mon Sep 17 00:00:00 2001 From: Geoffrey McElhanon Date: Sat, 14 Oct 2023 00:55:19 -0500 Subject: [PATCH] [ODS-6045] Role-named fields in EdOrg Only and People Only authorization strategies do not apply appropriate filters --- .../EducationOrganizationIdNamesProvider.cs | 9 +++- .../IEducationOrganizationIdNamesProvider.cs | 4 +- .../PersonEntitySpecification.cs | 45 ++++++++++--------- .../Specifications/UniqueIdSpecification.cs | 4 +- 4 files changed, 37 insertions(+), 25 deletions(-) diff --git a/Application/EdFi.Ods.Api/Security/Authorization/EducationOrganizationIdNamesProvider.cs b/Application/EdFi.Ods.Api/Security/Authorization/EducationOrganizationIdNamesProvider.cs index de401a69c4..85d2475c34 100644 --- a/Application/EdFi.Ods.Api/Security/Authorization/EducationOrganizationIdNamesProvider.cs +++ b/Application/EdFi.Ods.Api/Security/Authorization/EducationOrganizationIdNamesProvider.cs @@ -57,7 +57,14 @@ public string[] GetConcreteNames() /// public bool IsEducationOrganizationIdName(string candidateName) { - return _sortedEducationOrganizationIdNames.Value.BinarySearch(candidateName) >= 0; + // First look for an exact match (no role name) + if (_sortedEducationOrganizationIdNames.Value.BinarySearch(candidateName) >= 0) + { + return true; + } + + // Now check iteratively through known names as a matching suffix on the property name (allowing for a role name) + return _sortedEducationOrganizationIdNames.Value.Any(candidateName.EndsWith); } } } diff --git a/Application/EdFi.Ods.Api/Security/Authorization/IEducationOrganizationIdNamesProvider.cs b/Application/EdFi.Ods.Api/Security/Authorization/IEducationOrganizationIdNamesProvider.cs index 6bcb22eb34..c606dad8e5 100644 --- a/Application/EdFi.Ods.Api/Security/Authorization/IEducationOrganizationIdNamesProvider.cs +++ b/Application/EdFi.Ods.Api/Security/Authorization/IEducationOrganizationIdNamesProvider.cs @@ -23,10 +23,10 @@ public interface IEducationOrganizationIdNamesProvider string[] GetConcreteNames(); /// - /// Indicates whether the supplied name is a known education organization id. + /// Indicates whether the supplied name is a known education organization id, allowing for the presence of role names. /// /// The name to be evaluated. - /// true if the name matches a known education organization id; otherwise false. + /// true if the name matches a known education organization id as a suffix; otherwise false. bool IsEducationOrganizationIdName(string candidateName); } } diff --git a/Application/EdFi.Ods.Common/Specifications/PersonEntitySpecification.cs b/Application/EdFi.Ods.Common/Specifications/PersonEntitySpecification.cs index 4c5af50867..17fc6b1c39 100644 --- a/Application/EdFi.Ods.Common/Specifications/PersonEntitySpecification.cs +++ b/Application/EdFi.Ods.Common/Specifications/PersonEntitySpecification.cs @@ -21,6 +21,8 @@ public static class PersonEntitySpecification Staff, Student, Parent }; + private static readonly string[] _suffixes = { UniqueIdSpecification.USI, UniqueIdSpecification.UniqueId }; + /// /// Indicates whether the specified represents a type of person. /// @@ -41,40 +43,43 @@ public static bool IsPersonEntity(string typeName) return ValidPersonTypes.Contains(typeName, StringComparer.CurrentCultureIgnoreCase); } - /// - /// Indicates whether the specified property name is an identifier for a person. - /// - /// The name of the property to be evaluated. - /// true if the property is an identifier for a type of person; otherwise false. - public static bool IsPersonIdentifier(string propertyName) - { - return IsPersonIdentifier(propertyName, null); - } - /// /// Indicates whether the specified property name is an identifier for the specified person type. /// /// The name of the property to be evaluated. - /// A specific type of person. + /// A specific type of person; or null for any type. /// true if the property is an identifier for the specified type of person; otherwise false. - public static bool IsPersonIdentifier(string propertyName, string personType) + public static bool IsPersonIdentifier(string propertyName, string personType = null) { if (personType != null && !ValidPersonTypes.Any(pt => pt.EqualsIgnoreCase(personType))) { throw new ArgumentException("'{0}' is not a supported person type."); } - string entityName; - // TODO: Embedded convention (Person identifiers can end with "USI" or "UniqueId") - if (propertyName.TryTrimSuffix("UniqueId", out entityName) - || propertyName.TryTrimSuffix("USI", out entityName)) + return _suffixes.Any( + s => propertyName.EndsWith(s) + && ValidPersonTypes.Any( + pt => + // Person type was either not specified, or current person type matches what was specified + (personType == null || personType == pt) + && PersonTypeAppearsImmediatelyBeforeSuffix(pt, s))); + + bool PersonTypeAppearsImmediatelyBeforeSuffix(string pt, string suffix) { - return IsPersonEntity(entityName) - && (personType == null || entityName.EqualsIgnoreCase(personType)); - } + int lastIndexOfPersonType = propertyName.LastIndexOf(pt, StringComparison.Ordinal); + + if (lastIndexOfPersonType < 0) + { + // Person type is not in the property name + return false; + } + + // Identify expected location of the person type in the property name + int expectedLastIndexOfPersonType = propertyName.Length - (pt.Length + suffix.Length); - return false; + return lastIndexOfPersonType == expectedLastIndexOfPersonType; + } } } } diff --git a/Application/EdFi.Ods.Common/Specifications/UniqueIdSpecification.cs b/Application/EdFi.Ods.Common/Specifications/UniqueIdSpecification.cs index 293b8c22d6..2c9b6e0010 100644 --- a/Application/EdFi.Ods.Common/Specifications/UniqueIdSpecification.cs +++ b/Application/EdFi.Ods.Common/Specifications/UniqueIdSpecification.cs @@ -11,8 +11,8 @@ namespace EdFi.Ods.Common.Specifications { public static class UniqueIdSpecification { - private const string USI = "USI"; - private const string UniqueId = "UniqueId"; + public const string USI = "USI"; + public const string UniqueId = "UniqueId"; public static string GetUniqueIdPersonType(string propertyName) {