diff --git a/Application/EdFi.Admin.DataAccess/Models/AccountModels.cs b/Application/EdFi.Admin.DataAccess/Models/AccountModels.cs index 12e820c7ec..5d5d8e373c 100644 --- a/Application/EdFi.Admin.DataAccess/Models/AccountModels.cs +++ b/Application/EdFi.Admin.DataAccess/Models/AccountModels.cs @@ -36,13 +36,14 @@ public User() /// Empty, Minimal, Populated /// optional parameter, value is created randomly if it is null or empty. Both Key and Secret are required if providing either. /// optional parameter, value is created randomly if it is null or empty. Both Key and Secret are required if providing either. + /// optional parameter, is this client allowed to authenticate. Default is true. /// ApiClient information about the created sandbox - 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 index 0089e6d3df..96d463f925 100644 --- 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; } /// - /// Lock-out the application if not approved (Auto-approve in sandbox) + /// Lock-out the application if not approved /// public bool IsApproved { get; set; } diff --git a/Application/EdFi.Admin.DataAccess/Repositories/ClientAppRepo.cs b/Application/EdFi.Admin.DataAccess/Repositories/ClientAppRepo.cs index 258b2f3149..f07a8c1831 100644 --- 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 index 5b3540b2e6..67b1a559ed 100644 --- 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 edOrgIds, int applicationId); diff --git a/Application/EdFi.Ods.Api/EdFi.Ods.Api.csproj b/Application/EdFi.Ods.Api/EdFi.Ods.Api.csproj index fbbfee15d7..1cc6d07b9f 100644 --- a/Application/EdFi.Ods.Api/EdFi.Ods.Api.csproj +++ b/Application/EdFi.Ods.Api/EdFi.Ods.Api.csproj @@ -26,7 +26,7 @@ - + diff --git a/Application/EdFi.Ods.Api/Providers/ClientCredentialsTokenRequestProvider.cs b/Application/EdFi.Ods.Api/Providers/ClientCredentialsTokenRequestProvider.cs index 53ab9d7a5a..2ea3322336 100644 --- a/Application/EdFi.Ods.Api/Providers/ClientCredentialsTokenRequestProvider.cs +++ b/Application/EdFi.Ods.Api/Providers/ClientCredentialsTokenRequestProvider.cs @@ -82,6 +82,11 @@ public async Task 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 index c10e1f47a3..04670a3d4e 100644 --- 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 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 index 0a7d1d1d04..9d41137f36 100644 --- a/Application/EdFi.Ods.Sandbox/EdFi.Ods.Sandbox.csproj +++ b/Application/EdFi.Ods.Sandbox/EdFi.Ods.Sandbox.csproj @@ -20,7 +20,7 @@ - + diff --git a/Application/EdFi.Ods.Tests/EdFi.Ods.Tests.csproj b/Application/EdFi.Ods.Tests/EdFi.Ods.Tests.csproj index ad438d4ed4..4e2c8e977b 100644 --- a/Application/EdFi.Ods.Tests/EdFi.Ods.Tests.csproj +++ b/Application/EdFi.Ods.Tests/EdFi.Ods.Tests.csproj @@ -27,7 +27,7 @@ - + diff --git a/Artifacts/MsSql/Structure/Admin/0176-Update-CreateClientAccessToken.sql b/Artifacts/MsSql/Structure/Admin/0176-Update-CreateClientAccessToken.sql new file mode 100644 index 0000000000..bdfe0bb929 --- /dev/null +++ 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 new file mode 100644 index 0000000000..df1fa810cd --- /dev/null +++ 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 index f082086f82..7711d29f11 100644 --- 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 index db8a9b1deb..e2b6b3b7b1 100644 --- 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 @@ - + diff --git a/tests/EdFi.Ods.WebApi.IntegrationTests/EdFi.Ods.WebApi.IntegrationTests.csproj b/tests/EdFi.Ods.WebApi.IntegrationTests/EdFi.Ods.WebApi.IntegrationTests.csproj index 35b6b41ff7..883806ab43 100644 --- 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 @@ - +