Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 13 commits into from
Oct 28, 2024
5 changes: 3 additions & 2 deletions Application/EdFi.Admin.DataAccess/Models/AccountModels.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
Expand Down
2 changes: 1 addition & 1 deletion Application/EdFi.Admin.DataAccess/Models/ApiClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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; }

Expand Down
16 changes: 8 additions & 8 deletions Application/EdFi.Admin.DataAccess/Repositories/ClientAppRepo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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);

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion Application/EdFi.Ods.Api/EdFi.Ods.Api.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
<PackageReference Include="EdFi.Suite3.Common" Version="7.3.162" />
</ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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,
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion Application/EdFi.Ods.Sandbox/EdFi.Ods.Sandbox.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion Application/EdFi.Ods.Tests/EdFi.Ods.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
Expand Up @@ -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": []
}
]
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Loading