[ODS-6448] Disable a key/secret without deleting it (#1170)

Ed-Fi-Alliance-OSS · Oct 28, 2024 · e0bce1a · e0bce1a
1 parent 945d3f5
commit e0bce1a
Show file tree

Hide file tree

Showing 14 changed files with 217 additions and 17 deletions.
diff --git a/Application/EdFi.Admin.DataAccess/Models/AccountModels.cs b/Application/EdFi.Admin.DataAccess/Models/AccountModels.cs
@@ -36,13 +36,14 @@ public User()
         /// <param name="sandboxType">Empty, Minimal, Populated</param>
         /// <param name="key">optional parameter, value is created randomly if it is null or empty. Both Key and Secret are required if providing either.</param>
         /// <param name="secret">optional parameter, value is created randomly if it is null or empty. Both Key and Secret are required if providing either.</param>
+        /// <param name="isApproved">optional parameter, is this client allowed to authenticate. Default is <c>true</c>.</param>
         /// <returns>ApiClient information about the created sandbox</returns>
-        public ApiClient AddSandboxClient(string name, SandboxType sandboxType, string key, string secret)
+        public ApiClient AddSandboxClient(string name, SandboxType sandboxType, string key, string secret, bool isApproved = true)
         {
             var client = new ApiClient(true)
             {
                 Name = name,
-                IsApproved = true,
+                IsApproved = isApproved,
                 UseSandbox = true,
                 SandboxType = sandboxType
             };

diff --git a/Application/EdFi.Admin.DataAccess/Models/ApiClient.cs b/Application/EdFi.Admin.DataAccess/Models/ApiClient.cs
@@ -75,7 +75,7 @@ public ApiClient(bool generateKey = false)
         public string Name { get; set; }
 
         /// <summary>
-        /// Lock-out the application if not approved (Auto-approve in sandbox)
+        /// Lock-out the application if not approved
         /// </summary>
         public bool IsApproved { get; set; }
 

diff --git a/Application/EdFi.Admin.DataAccess/Repositories/ClientAppRepo.cs b/Application/EdFi.Admin.DataAccess/Repositories/ClientAppRepo.cs
@@ -439,11 +439,11 @@ public void AddApiClientToUserWithVendorApplication(int userId, ApiClient client
             }
         }
 
-        public ApiClient CreateApiClient(int userId, string name, string key, string secret)
+        public ApiClient CreateApiClient(int userId, string name, string key, string secret, bool isApproved)
         {
             using (var context = _contextFactory.CreateContext())
             {
-                var client = CreateApiClient(context, userId, name, SandboxType.Sample, key, secret);
+                var client = CreateApiClient(context, userId, name, SandboxType.Sample, key, secret, isApproved);
 
                 context.SaveChanges();
 
@@ -461,7 +461,7 @@ public void SetupKeySecret(
         {
             using (var context = _contextFactory.CreateContext())
             {
-                var client = CreateApiClient(context, userId, name, sandboxType, key, secret);
+                var client = CreateApiClient(context, userId, name, sandboxType, key, secret, true);
 
                 AddApplicationEducationOrganizations(context, applicationId, client);
 
@@ -509,7 +509,7 @@ public ApiClient SetupDefaultSandboxClient(
             using (var context = _contextFactory.CreateContext())
             {
                 _logger.Debug($"Creating API Client");
-                var client = GetClient(key, secret) ?? CreateApiClient(context, userId, name, sandboxType, key, secret);
+                var client = GetClient(key, secret) ?? CreateApiClient(context, userId, name, sandboxType, key, secret, true);
 
                 _logger.Debug($"Adding Education Organization to client");
                 AddApplicationEducationOrganizations(context, applicationId, client);
@@ -723,17 +723,17 @@ private OwnershipToken GetOrCreateOwnershipToken(string ownershipToken)
             }
         }
 
-        private ApiClient CreateApiClient(
-            IUsersContext context,
+        private ApiClient CreateApiClient(IUsersContext context,
             int userId,
             string name,
             SandboxType sandboxType,
             string key,
-            string secret)
+            string secret,
+            bool isApproved)
         {
             var attachedUser = context.Users.Find(userId);
 
-            return attachedUser.AddSandboxClient(name, sandboxType, key, secret);
+            return attachedUser.AddSandboxClient(name, sandboxType, key, secret, isApproved);
         }
 
         private void AddApplicationEducationOrganizations(IUsersContext context, int applicationId, ApiClient client)

diff --git a/Application/EdFi.Admin.DataAccess/Repositories/IClientAppRepo.cs b/Application/EdFi.Admin.DataAccess/Repositories/IClientAppRepo.cs
@@ -68,7 +68,7 @@ void SetupKeySecret(string name, SandboxType sandboxType, string key, string sec
 
         Application CreateApplicationForVendor(int vendorId, string applicationName, string claimSetName);
 
-        ApiClient CreateApiClient(int userId, string name, string key, string secret);
+        ApiClient CreateApiClient(int userId, string name, string key, string secret, bool isApproved = true);
 
         void AddEdOrgIdsToApiClient(int userId, int apiClientId, IList<long> edOrgIds, int applicationId);
 

diff --git a/Application/EdFi.Ods.Api/EdFi.Ods.Api.csproj b/Application/EdFi.Ods.Api/EdFi.Ods.Api.csproj
@@ -26,7 +26,7 @@
     </When>
     <Otherwise>
       <ItemGroup>
-        <PackageReference Include="EdFi.Suite3.Admin.DataAccess" Version="7.3.168" />
+        <PackageReference Include="EdFi.Suite3.Admin.DataAccess" Version="7.3.367" />
         <PackageReference Include="EdFi.Suite3.Security.DataAccess" Version="7.3.201" />
       </ItemGroup>
     </Otherwise>

diff --git a/Application/EdFi.Ods.Api/Providers/ClientCredentialsTokenRequestProvider.cs b/Application/EdFi.Ods.Api/Providers/ClientCredentialsTokenRequestProvider.cs
@@ -82,6 +82,11 @@ public async Task<AuthenticationResponse> HandleAsync(TokenRequest tokenRequest)
 
             // create a new token
             var token = await _accessTokenFactory.CreateAccessTokenAsync(authenticationResult.ApiClientDetails.ApiClientId, tokenRequestScope);
+
+            if (token == null)
+            {
+                return new AuthenticationResponse {TokenError = new TokenError(TokenErrorType.InvalidClient)};
+            }
 
             var tokenResponse = new TokenResponse();
             tokenResponse.SetToken(token.Id, (int) token.Duration.TotalSeconds, token.Scope);

diff --git a/Application/EdFi.Ods.Api/Security/Authentication/EdFiAdminAccessTokenFactory.cs b/Application/EdFi.Ods.Api/Security/Authentication/EdFiAdminAccessTokenFactory.cs
@@ -9,7 +9,9 @@
 using System.Threading.Tasks;
 using Dapper;
 using EdFi.Admin.DataAccess.Providers;
+using EdFi.Ods.Api.Middleware;
 using EdFi.Ods.Common.Exceptions;
+using Microsoft.AspNetCore.Authentication;
 
 namespace EdFi.Ods.Api.Security.Authentication;
 
@@ -22,6 +24,7 @@ public class EdFiAdminAccessTokenFactory : IAccessTokenFactory
 
     private const string AddTokenProcedureName = "dbo.CreateClientAccessToken";
     private const string TokenLimitReachedDbMessage = "Token limit reached";
+    private const string ClientNotApprovedDbMessage = "Client is not approved";
 
     public EdFiAdminAccessTokenFactory(
         DbProviderFactory dbProviderFactory,
@@ -60,6 +63,10 @@ public async Task<AccessToken> CreateAccessTokenAsync(int apiClientId, string sc
         {
             await connection.ExecuteAsync(AddTokenProcedureName, @params, commandType: CommandType.StoredProcedure);
         }
+        catch (DbException ex) when (ex.Message.Contains(ClientNotApprovedDbMessage))
+        {
+            return null;
+        }
         catch (DbException ex) when (ex.Message.Contains(TokenLimitReachedDbMessage))
         {
             throw new TooManyTokensException(_tokenPerClientLimit);

diff --git a/Application/EdFi.Ods.Sandbox/EdFi.Ods.Sandbox.csproj b/Application/EdFi.Ods.Sandbox/EdFi.Ods.Sandbox.csproj
@@ -20,7 +20,7 @@
     </When>
     <Otherwise>
       <ItemGroup>
-        <PackageReference Include="EdFi.Suite3.Admin.DataAccess" Version="7.3.168" />
+        <PackageReference Include="EdFi.Suite3.Admin.DataAccess" Version="7.3.367" />
         <PackageReference Include="EdFi.Suite3.Common" Version="7.3.162" />
       </ItemGroup>
     </Otherwise>

diff --git a/Application/EdFi.Ods.Tests/EdFi.Ods.Tests.csproj b/Application/EdFi.Ods.Tests/EdFi.Ods.Tests.csproj
@@ -27,7 +27,7 @@
     </When>
     <Otherwise>
       <ItemGroup>
-        <PackageReference Include="EdFi.Suite3.Admin.DataAccess" Version="7.3.168" />
+        <PackageReference Include="EdFi.Suite3.Admin.DataAccess" Version="7.3.367" />
         <PackageReference Include="EdFi.Suite3.Common" Version="7.3.162" />
       </ItemGroup>
     </Otherwise>

diff --git a/Artifacts/MsSql/Structure/Admin/0176-Update-CreateClientAccessToken.sql b/Artifacts/MsSql/Structure/Admin/0176-Update-CreateClientAccessToken.sql
@@ -0,0 +1,48 @@
+-- SPDX-License-Identifier: Apache-2.0
+-- Licensed to the Ed-Fi Alliance under one or more agreements.
+-- The Ed-Fi Alliance licenses this file to you under the Apache License, Version 2.0.
+-- See the LICENSE and NOTICES files in the project root for more information.
+
+CREATE OR ALTER PROCEDURE dbo.CreateClientAccessToken(
+    @Id UNIQUEIDENTIFIER = NULL,
+    @Expiration DATETIME = NULL,
+    @Scope NVARCHAR(max) = NULL,
+    @ApiClientId INT = NULL,
+    @MaxTokenCount INT = NULL
+)
+AS
+BEGIN
+  SET NOCOUNT ON
+
+  DECLARE @ActiveTokenCount INT
+  DECLARE @ClientIsApproved INT
+
+  SET @ClientIsApproved = (SELECT COUNT(1)
+                                 FROM dbo.ApiClients ac
+                                 WHERE ac.ApiClientId = @ApiClientId
+                                   AND ac.IsApproved = 1)
+
+  IF (@ClientIsApproved = 0)
+  BEGIN
+    THROW 50000, 'Client is not approved', 1;
+  END
+
+  IF @MaxTokenCount < 1
+    SET @ActiveTokenCount = 0
+  ELSE
+  BEGIN
+    SET @ActiveTokenCount = (SELECT COUNT(1)
+                                   FROM dbo.ClientAccessTokens actoken
+                                   WHERE ApiClient_ApiClientId = @ApiClientId
+                                     AND actoken.Expiration > GETUTCDATE())
+  END
+
+  IF (@MaxTokenCount < 1) OR (@ActiveTokenCount < @MaxTokenCount)
+  BEGIN
+    INSERT INTO dbo.ClientAccessTokens(Id, Expiration, Scope, ApiClient_ApiClientId)
+    VALUES (@Id, @Expiration, @Scope, @ApiClientId)
+  END
+  ELSE
+    THROW 50000, 'Token limit reached', 1;
+END
+GO
diff --git a/Artifacts/PgSql/Structure/Admin/0176-Update-CreateClientAccessToken.sql b/Artifacts/PgSql/Structure/Admin/0176-Update-CreateClientAccessToken.sql
@@ -0,0 +1,48 @@
+-- SPDX-License-Identifier: Apache-2.0
+-- Licensed to the Ed-Fi Alliance under one or more agreements.
+-- The Ed-Fi Alliance licenses this file to you under the Apache License, Version 2.0.
+-- See the LICENSE and NOTICES files in the project root for more information.
+
+CREATE OR REPLACE PROCEDURE dbo.CreateClientAccessToken(
+    id uuid,
+    expiration timestamp without time zone,
+    scope text,
+    apiclientid integer,
+    maxtokencount integer)
+AS
+$BODY$
+DECLARE
+    active_token_count integer;
+    client_is_approved integer;
+BEGIN
+
+  	SELECT count(1)
+  	INTO client_is_approved
+    FROM dbo.apiclients ac
+    WHERE ac.apiclientid = createclientaccesstoken.ApiClientId
+	    AND ac.isapproved = true;
+
+  	IF (client_is_approved = 0) THEN
+  		RAISE EXCEPTION USING MESSAGE = 'Client is not approved';
+  	END IF;
+
+    IF maxtokencount < 1 THEN
+        active_token_count := 0;
+    ELSE
+        active_token_count := (SELECT COUNT(1)
+                               FROM dbo.clientaccesstokens actoken
+                               WHERE apiclient_apiclientid = ApiClientId
+                                 AND actoken.expiration > current_timestamp at time zone 'utc');
+    END IF;
+
+    IF (maxtokencount < 1) OR (active_token_count < maxtokencount) THEN
+        INSERT INTO dbo.ClientAccessTokens(id, expiration, scope, apiclient_apiclientid)
+        VALUES (CreateClientAccessToken.id, CreateClientAccessToken.expiration, CreateClientAccessToken.scope,
+                apiclientid);
+    ELSE
+        RAISE EXCEPTION USING MESSAGE = 'Token limit reached';
+    END IF;
+
+END
+$BODY$
+    LANGUAGE plpgsql;
diff --git a/Postman Test Suite/Ed-Fi ODS-API Integration Test Suite.postman_collection.json b/Postman Test Suite/Ed-Fi ODS-API Integration Test Suite.postman_collection.json
@@ -1090,6 +1090,97 @@
 						}
 					},
 					"response": []
+				},
+				{
+					"name": "Unapproved Client With Correct Key/Secret Request",
+					"event": [
+						{
+							"listen": "prerequest",
+							"script": {
+								"exec": [
+									"const clientSuffix = \"Unapproved_Client\";\r",
+									"\r",
+									"const apiKeyName = `ApiKey_${clientSuffix}`;\r",
+									"const apiSecretName = `ApiSecret_${clientSuffix}`;\r",
+									"\r",
+									"let clientId = pm.environment.get(apiKeyName) ?? clientSuffix;\r",
+									"let clientSecret = pm.environment.get(apiSecretName) ?? clientSuffix;\r",
+									"\r",
+									"pm.environment.set(`known:${apiKeyName}`, clientId);\r",
+									"pm.environment.set(`known:${apiSecretName}`, clientSecret);\r",
+									""
+								],
+								"type": "text/javascript",
+								"packages": {}
+							}
+						},
+						{
+							"listen": "test",
+							"script": {
+								"exec": [
+									"pm.test(\"Status code is 401\", () => {\r",
+									"  pm.expect(pm.response.code).to.equal(401);\r",
+									"});\r",
+									"\r",
+									"pm.test(\"Should return a invalid_client\", () => {\r",
+									"  const responseItem = pm.response.json();\r",
+									"  pm.expect(responseItem.error).to.equal(\"invalid_client\");\r",
+									"});\r",
+									""
+								],
+								"type": "text/javascript",
+								"packages": {}
+							}
+						}
+					],
+					"protocolProfileBehavior": {
+						"disabledSystemHeaders": {}
+					},
+					"request": {
+						"auth": {
+							"type": "basic",
+							"basic": [
+								{
+									"key": "password",
+									"value": "{{known:ApiSecret_Unapproved_Client}}",
+									"type": "string"
+								},
+								{
+									"key": "username",
+									"value": "{{known:ApiKey_Unapproved_Client}}",
+									"type": "string"
+								}
+							]
+						},
+						"method": "POST",
+						"header": [],
+						"body": {
+							"mode": "urlencoded",
+							"urlencoded": [
+								{
+									"key": "mode",
+									"value": "urlencoded",
+									"type": "default"
+								},
+								{
+									"key": "grant_type",
+									"value": "client_credentials",
+									"type": "default"
+								}
+							]
+						},
+						"url": {
+							"raw": "{{ApiBaseUrl}}/oauth/token",
+							"host": [
+								"{{ApiBaseUrl}}"
+							],
+							"path": [
+								"oauth",
+								"token"
+							]
+						}
+					},
+					"response": []
 				}
 			]
 		},

diff --git a/tests/EdFi.Ods.WebApi.CompositeSpecFlowTests/EdFi.Ods.WebApi.CompositeSpecFlowTests.csproj b/tests/EdFi.Ods.WebApi.CompositeSpecFlowTests/EdFi.Ods.WebApi.CompositeSpecFlowTests.csproj
@@ -32,7 +32,7 @@
     </When>
     <Otherwise>
       <ItemGroup>
-        <PackageReference Include="EdFi.Suite3.Admin.DataAccess" Version="7.3.168" />
+        <PackageReference Include="EdFi.Suite3.Admin.DataAccess" Version="7.3.367" />
         <PackageReference Include="EdFi.Suite3.Common" Version="7.3.162" />
       </ItemGroup>
     </Otherwise>

diff --git a/tests/EdFi.Ods.WebApi.IntegrationTests/EdFi.Ods.WebApi.IntegrationTests.csproj b/tests/EdFi.Ods.WebApi.IntegrationTests/EdFi.Ods.WebApi.IntegrationTests.csproj
@@ -21,7 +21,7 @@
     </When>
     <Otherwise>
       <ItemGroup>
-        <PackageReference Include="EdFi.Suite3.Admin.DataAccess" Version="7.3.168" />
+        <PackageReference Include="EdFi.Suite3.Admin.DataAccess" Version="7.3.367" />
         <PackageReference Include="EdFi.Suite3.Common" Version="7.3.162" />
       </ItemGroup>
     </Otherwise>