more scope duplicate detection #2194

src/IdentityServer4/Extensions/IResourceStoreExtensions.cs

@@ -26,18 +26,46 @@ public static async Task<Resources> FindResourcesByScopeAsync(this IResourceStor
             var identity = await store.FindIdentityResourcesByScopeAsync(scopeNames);
             var apiResources = await store.FindApiResourcesByScopeAsync(scopeNames);
 
+            Validate(identity, apiResources);
+
+            var apis = new List<ApiResource>();
+            foreach (var apiResource in apiResources)
+            {
+                apis.Add(apiResource.CloneWithScopes(apiResource.Scopes.Where(x => scopeNames.Contains(x.Name))));
+            }
+
+            var resources = new Resources(identity, apis)
+            {
+                OfflineAccess = scopeNames.Contains(IdentityServerConstants.StandardScopes.OfflineAccess)
+            };
+            return resources;
+        }
+
+        private static void Validate(IEnumerable<IdentityResource> identity, IEnumerable<ApiResource> apiResources)
+        {
             // attempt to detect invalid configuration. this is about the only place
             // we can do this, since it's hard to get the values in the store.
             var identityScopeNames = identity.Select(x => x.Name).ToArray();
             var apiScopeNames = (from api in apiResources
                                  from scope in api.Scopes
                                  select scope.Name).ToArray();
+            CheckForDuplicates(identityScopeNames, apiScopeNames);
+
+            var overlap = identityScopeNames.Intersect(apiScopeNames).ToArray();
+            if (overlap.Any())
+            {
+                var names = overlap.Aggregate((x, y) => x + ", " + y);
+                throw new Exception(String.Format("Found identity scopes and API scopes that use the same names. This is an invalid configuration. Use different names for identity scopes and API scopes. Scopes found: {0}", names));
+            }
+        }
 
+        private static void CheckForDuplicates(string[] identityScopeNames, string[] apiScopeNames)
+        {
             var identityDuplicates = identityScopeNames
-                .GroupBy(x => x)
-                .Where(g => g.Count() > 1)
-                .Select(y => y.Key)
-                .ToArray();
+                            .GroupBy(x => x)
+                            .Where(g => g.Count() > 1)
+                            .Select(y => y.Key)
+                            .ToArray();
             if (identityDuplicates.Any())
             {
                 var names = identityDuplicates.Aggregate((x, y) => x + ", " + y);
@@ -54,25 +82,6 @@ from scope in api.Scopes
                 var names = apiDuplicates.Aggregate((x, y) => x + ", " + y);
                 throw new Exception(String.Format("Duplicate API scopes found. This is an invalid configuration. Use different names for API scopes. Scopes found: {0}", names));
             }
-
-            var overlap = identityScopeNames.Intersect(apiScopeNames).ToArray();
-            if (overlap.Any())
-            {
-                var names = overlap.Aggregate((x, y) => x + ", " + y);
-                throw new Exception(String.Format("Found identity scopes and API scopes that use the same names. This is an invalid configuration. Use different names for identity scopes and API scopes. Scopes found: {0}", names));
-            }
-
-            var apis = new List<ApiResource>();
-            foreach (var apiResource in apiResources)
-            {
-                apis.Add(apiResource.CloneWithScopes(apiResource.Scopes.Where(x => scopeNames.Contains(x.Name))));
-            }
-
-            var resources = new Resources(identity, apis)
-            {
-                OfflineAccess = scopeNames.Contains(IdentityServerConstants.StandardScopes.OfflineAccess)
-            };
-            return resources;
         }
 
         /// <summary>
@@ -94,6 +103,8 @@ public static async Task<Resources> FindEnabledResourcesByScopeAsync(this IResou
         public static async Task<Resources> GetAllEnabledResourcesAsync(this IResourceStore store)
         {
             var resources = await store.GetAllResourcesAsync();
+            Validate(resources.IdentityResources, resources.ApiResources);
+
             return resources.FilterEnabled();
         }
 

test/IdentityServer.UnitTests/Extensions/IResourceStoreExtensionsTests.cs

@@ -15,6 +15,56 @@ namespace IdentityServer.UnitTests.Extensions
 {
     public class IResourceStoreExtensionsTests
     {
+        [Fact]
+        public void GetAllEnabledResourcesAsync_on_duplicate_identity_scopes_should_fail()
+        {
+            var store = new MockResourceStore()
+            {
+                IdentityResources = {
+                    new IdentityResource { Name = "A" },
+                    new IdentityResource { Name = "A" } }
+            };
+
+            Func<Task> a = () => store.GetAllEnabledResourcesAsync();
+            a.Should().Throw<Exception>().And.Message.ToLowerInvariant().Should().Contain("duplicate").And.Contain("identity scopes");
+        }
+
+        [Fact]
+        public async Task GetAllEnabledResourcesAsync_without_duplicate_identity_scopes_should_succeed()
+        {
+            var store = new MockResourceStore()
+            {
+                IdentityResources = {
+                    new IdentityResource { Name = "A" },
+                    new IdentityResource { Name = "B" } }
+            };
+
+            await store.GetAllEnabledResourcesAsync();
+        }
+
+        [Fact]
+        public void GetAllEnabledResourcesAsync_on_duplicate_api_scopes_should_fail()
+        {
+            var store = new MockResourceStore()
+            {
+                ApiResources = { new ApiResource("A"), new ApiResource("A") }
+            };
+
+            Func<Task> a = () => store.GetAllEnabledResourcesAsync();
+            a.Should().Throw<Exception>().And.Message.ToLowerInvariant().Should().Contain("duplicate").And.Contain("api scopes");
+        }
+
+        [Fact]
+        public async Task GetAllEnabledResourcesAsync_without_duplicate_api_scopes_should_succeed()
+        {
+            var store = new MockResourceStore()
+            {
+                ApiResources = { new ApiResource("A"), new ApiResource("B") }
+            };
+
+            await store.GetAllEnabledResourcesAsync();
+        }
+
         [Fact]
         public void FindResourcesByScopeAsync_on_duplicate_identity_scopes_should_fail()
         {