Skip to content

Commit

Permalink
Add coding standard (#90)
Browse files Browse the repository at this point in the history
  • Loading branch information
meziantou authored Aug 5, 2024
1 parent 7a45556 commit 11ce971
Show file tree
Hide file tree
Showing 21 changed files with 77 additions and 90 deletions.
5 changes: 5 additions & 0 deletions Build.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ Process {
Push-Location $workingDir
Remove-Item $outputDir -Force -Recurse -ErrorAction SilentlyContinue

# Duende.IdentityServer fails signature validation
# https://github.com/DuendeSoftware/Support/issues/1352
# Note that the new certificate should be included in the next version of the .NET SDK (release on 2024/08/13)
$env:DOTNET_NUGET_SIGNATURE_VERIFICATION="false"

Exec { & dotnet clean -c Release }
Exec { & dotnet build -c Release }
Exec { & dotnet test -c Release --no-build --results-directory "$outputDir" --no-restore -l "trx" -l "console;verbosity=detailed" }
Expand Down
19 changes: 2 additions & 17 deletions src/Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -3,39 +3,24 @@
<Copyright>Copyright © Workleap $([System.DateTime]::UtcNow.ToString(yyyy))</Copyright>
<Authors>Workleap</Authors>
<Owners>Workleap</Owners>
<PackageProjectUrl>https://github.com/gsoft-inc/wl-authentication-clientcredentialsgrant</PackageProjectUrl>
<PackageLicenseExpression>Apache-2.0</PackageLicenseExpression>
<PublishRepositoryUrl>true</PublishRepositoryUrl>
<LangVersion>latest</LangVersion>
<Nullable>enable</Nullable>
<AnalysisLevel>latest-All</AnalysisLevel>
<ImplicitUsings>enable</ImplicitUsings>
</PropertyGroup>

<PropertyGroup Condition=" '$(Configuration)' == 'Release' ">
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="StyleCop.Analyzers" Version="1.2.0-beta.556">
<PackageReference Include="Workleap.DotNet.CodingStandards" Version="0.6.1-preview.3">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
</PackageReference>
<PackageReference Include="GitVersion.MsBuild" Version="5.12.0" Condition=" '$(Configuration)' == 'Release' ">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
</PackageReference>
</ItemGroup>

<!-- https://learn.microsoft.com/en-us/dotnet/standard/library-guidance/sourcelink -->
<PropertyGroup Condition=" '$(GITHUB_ACTIONS)' == 'true' Or '$(TF_BUILD)' == 'true' ">
<ContinuousIntegrationBuild>true</ContinuousIntegrationBuild>
<EmbedUntrackedSources>true</EmbedUntrackedSources>
</PropertyGroup>
<ItemGroup Condition=" '$(GITHUB_ACTIONS)' == 'true' ">
<PackageReference Include="Microsoft.SourceLink.GitHub" Version="8.0.0" PrivateAssets="All" />
</ItemGroup>

<ItemGroup Condition="$(MSBuildProjectName.Contains('Tests'))">
<Using Include="Xunit" />
<Using Include="Xunit.Abstractions" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public static AuthenticationBuilder AddClientCredentials(this AuthenticationBuil
{
options.OperationFilter<SecurityRequirementOperationFilter>();
}

if (options.DocumentFilterDescriptors.All(x => x.Type != typeof(SecurityDefinitionDocumentFilter)))
{
options.DocumentFilter<SecurityDefinitionDocumentFilter>();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using Workleap.AspNetCore.Authentication.ClientCredentialsGrant;
using Workleap.AspNetCore.Authentication.ClientCredentialsGrant;
using Microsoft.AspNetCore.Authentication.JwtBearer;
using Microsoft.AspNetCore.Authorization;
using Microsoft.Extensions.DependencyInjection.Extensions;
Expand Down Expand Up @@ -49,15 +49,15 @@ public static IServiceCollection AddClientCredentialsAuthorization(this IService
.AddAuthenticationSchemes(ClientCredentialsDefaults.AuthenticationScheme)
.RequireAuthenticatedUser()
.RequireClaim("scope", $"{jwtOptions.Audience}:{ScopeClaimMapping[ClientCredentialsScope.Admin]}"));

authorizationOptions.AddPolicy(
ClientCredentialsDefaults.RequireClientCredentialsPolicyName,
policy => policy
.AddAuthenticationSchemes(ClientCredentialsDefaults.AuthenticationScheme)
.RequireAuthenticatedUser()
.AddRequirements(new RequireClientCredentialsRequirement()));
});

services.TryAddEnumerable(ServiceDescriptor.Singleton<IAuthorizationHandler, RequireClientCredentialsRequirementHandler>());

return services;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ public static class ClientCredentialsDefaults
public const string AuthenticationScheme = "ClientCredentials";

internal const string AuthenticationType = "ClientCredentials";

internal const string RequireClientCredentialsPolicyName = "ClientCredentialsPolicy";

internal const string AuthorizationReadPolicy = "ClientCredentialsRead";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using Microsoft.AspNetCore.Authentication.JwtBearer;
using Microsoft.AspNetCore.Authentication.JwtBearer;
using Microsoft.Extensions.Options;
using Microsoft.OpenApi.Models;
using Swashbuckle.AspNetCore.SwaggerGen;
Expand All @@ -15,7 +15,7 @@ internal sealed class SecurityDefinitionDocumentFilter(IOptionsMonitor<JwtBearer
public void Apply(OpenApiDocument swaggerDoc, DocumentFilterContext context)
{
var apiPermissions = context.ApiDescriptions.SelectMany(SwaggerUtils.GetRequiredPermissions).ToHashSet(StringComparer.Ordinal);

swaggerDoc.Components.SecuritySchemes.Add(
ClientCredentialsDefaults.OpenApiSecurityDefinitionId,
new OpenApiSecurityScheme
Expand All @@ -31,19 +31,19 @@ public void Apply(OpenApiDocument swaggerDoc, DocumentFilterContext context)
},
});
}

private Uri GetTokenUrl()
{
// Authority has already been validated as an absolute URL
var authority = this._jwtOptions.Authority!.TrimEnd('/');
return new Uri($"{authority}/oauth2/token", UriKind.Absolute);
}

private Dictionary<string, string> ExtractScopes(IEnumerable<string> permissions)
{
// Audience has already been validated as non-empty
var audience = this._jwtOptions.Audience!;

var scopes = new Dictionary<string, string>
{
[SwaggerUtils.GetScopeForAnyPermission(audience)] = "Request all permissions for specified client ID",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Globalization;
using System.Globalization;
using System.Text;
using Microsoft.AspNetCore.Authentication.JwtBearer;
using Microsoft.AspNetCore.Http;
Expand Down Expand Up @@ -29,21 +29,21 @@ public void Apply(OpenApiOperation operation, OperationFilterContext context)
this.AddOperationSecurityReference(operation, attributes);
AppendScopeToOperationSummary(operation, attributes);
}

private static void AddAuthenticationAndAuthorizationErrorResponse(OpenApiOperation operation)
{
operation.Responses.TryAdd(StatusCodes.Status401Unauthorized.ToString(CultureInfo.InvariantCulture), new OpenApiResponse { Description = ReasonPhrases.GetReasonPhrase(StatusCodes.Status401Unauthorized) });
operation.Responses.TryAdd(StatusCodes.Status403Forbidden.ToString(CultureInfo.InvariantCulture), new OpenApiResponse { Description = ReasonPhrases.GetReasonPhrase(StatusCodes.Status403Forbidden) });
}

private void AddOperationSecurityReference(OpenApiOperation operation, HashSet<string> permissions)
{
var isAlreadyReferencingSecurityDefinition = operation.Security.Any(requirement => requirement.Keys.Any(key => key.Reference?.Id == ClientCredentialsDefaults.OpenApiSecurityDefinitionId));
if (isAlreadyReferencingSecurityDefinition)
{
return;
}

var securityScheme = new OpenApiSecurityScheme
{
Reference = new OpenApiReference
Expand All @@ -65,24 +65,24 @@ private static void AppendScopeToOperationSummary(OpenApiOperation operation, Ha
requireScopeSummary.Append(scopes.Count == 1 ? "Required permission: " : "Required permissions: ");
requireScopeSummary.Append(string.Join(", ", scopes));
requireScopeSummary.Append('.');

var isRequireScopeSummaryPresent = operation.Summary?.Contains(requireScopeSummary.ToString()) ?? false;
if (isRequireScopeSummaryPresent)
{
return;
}

var summary = new StringBuilder(operation.Summary?.TrimEnd('.'));
if (summary.Length > 0)
{
summary.Append(". ");
}

summary.Append(requireScopeSummary);

operation.Summary = summary.ToString();
}

private IEnumerable<string> ExtractScopes(HashSet<string> permissions)
{
foreach (var permission in permissions)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Reflection;
using System.Reflection;
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Mvc.ApiExplorer;
using Swashbuckle.AspNetCore.SwaggerGen;
Expand All @@ -18,7 +18,7 @@ public static IEnumerable<string> GetRequiredPermissions(ApiDescription apiDescr
{
return [];
}

// Controllers - Attributes on the action method (empty for minimal APIs)
attributes.AddRange(methodInfo.GetCustomAttributes<RequireClientCredentialsAttribute>(inherit: true));
}
Expand All @@ -34,7 +34,7 @@ public static string FormatScopeForSpecificPermission(string audience, string pe
{
return $"target-entity:{audience}:{permission}";
}

public static string GetScopeForAnyPermission(string audience)
{
return $"target-entity:{audience}";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Diagnostics.CodeAnalysis;
using System.Diagnostics.CodeAnalysis;
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Builder;

Expand Down Expand Up @@ -32,7 +32,7 @@ public RequireClientCredentialsAttribute(ClientCredentialsScope scope)
: this(EnumScopeNameMapping.GetValueOrDefault(scope) ?? throw new ArgumentOutOfRangeException(nameof(scope), scope, $"'{scope}' is not an valid scope value"))
{
}

/// <summary>
/// Verifies that the endpoint is called with the right granular permissions. <br/>
/// - It will check in those claims type: scope, scp or http://schemas.microsoft.com/identity/claims/scope <br/>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Diagnostics.CodeAnalysis;
using System.Diagnostics.CodeAnalysis;
using System.Security.Claims;
using Microsoft.AspNetCore.Authentication.JwtBearer;
using Microsoft.AspNetCore.Authorization;
Expand Down Expand Up @@ -29,14 +29,14 @@ public RequireClientCredentialsRequirementHandler(IOptionsMonitor<JwtBearerOptio
{
this._jwtOptions = jwtOptionsMonitor.Get(ClientCredentialsDefaults.AuthenticationScheme);
}

protected override Task HandleRequirementAsync(AuthorizationHandlerContext context, RequireClientCredentialsRequirement requirement)
{
if (!this.TryGetRequiredScopes(context, out var requiredScopes))
{
return Task.CompletedTask;
return Task.CompletedTask;
}

var hasRequiredScope = HasRequiredScope(context.User, requiredScopes);
if (hasRequiredScope)
{
Expand All @@ -49,7 +49,7 @@ protected override Task HandleRequirementAsync(AuthorizationHandlerContext conte
private bool TryGetRequiredScopes(AuthorizationHandlerContext context, [NotNullWhen(true)] out string[]? requiredScopes)
{
requiredScopes = null;

var endpoint = context.Resource switch
{
HttpContext httpContext => httpContext.GetEndpoint(),
Expand All @@ -71,7 +71,7 @@ private string[] FormatScopes(string requiredPermission)
{
return [requiredPermission, $"{this._jwtOptions.Audience}:{requiredPermission}"];
}

private static bool HasRequiredScope(ClaimsPrincipal claimsPrincipal, string[] requiredScopes)
{
return claimsPrincipal.Claims
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>net6.0;net7.0;net8.0</TargetFrameworks>
<TargetFrameworks>net6.0;net8.0</TargetFrameworks>
<IsPackable>true</IsPackable>
<IncludeSymbols>true</IncludeSymbols>
<SymbolPackageFormat>snupkg</SymbolPackageFormat>
Expand All @@ -16,7 +16,6 @@

<ItemGroup>
<PackageReference Include="Microsoft.AspNetCore.Authentication.JwtBearer" Version="6.0.0" Condition=" '$(TargetFramework)' == 'net6.0' " />
<PackageReference Include="Microsoft.AspNetCore.Authentication.JwtBearer" Version="7.0.0" Condition=" '$(TargetFramework)' == 'net7.0' " />
<PackageReference Include="Microsoft.AspNetCore.Authentication.JwtBearer" Version="8.0.0" Condition=" '$(TargetFramework)' == 'net8.0' " />
<PackageReference Include="Swashbuckle.AspNetCore" Version="6.6.2" />
<PackageReference Include="Microsoft.CodeAnalysis.PublicApiAnalyzers" Version="3.3.4">
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using Workleap.AspNetCore.Authentication.ClientCredentialsGrant;
using Workleap.AspNetCore.Authentication.ClientCredentialsGrant;
using Microsoft.AspNetCore.Authentication.JwtBearer;
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Authorization.Infrastructure;
Expand Down Expand Up @@ -50,11 +50,11 @@ public async Task GivenIServiceCollection_WhenAddClientCredentialsAuthorization_

var adminPolicy = authorizationValues.GetPolicy(ClientCredentialsDefaults.AuthorizationAdminPolicy);
ValidateClassicPolicy(adminPolicy, ClientCredentialsScope.Admin);

var requireClientCredentialsPolicy = authorizationValues.GetPolicy(ClientCredentialsDefaults.RequireClientCredentialsPolicyName);
ValidateRequireClientCredentialsPolicy(requireClientCredentialsPolicy);
}

[Fact]
public async Task GivenIServiceCollection_WhenAddClientCredentialsAuthorization_ThenRequirementHandlerRegistered()
{
Expand Down Expand Up @@ -91,7 +91,7 @@ private static void ValidateClassicPolicy(AuthorizationPolicy? policy, ClientCre
Assert.Equal($"{DefaultAudience}:{AuthorizationExtensions.ScopeClaimMapping[scope]}", allowedScope);
});
}

private static void ValidateRequireClientCredentialsPolicy(AuthorizationPolicy? policy)
{
Assert.NotNull(policy);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Collections.Concurrent;
using System.Collections.Concurrent;
using System.Net;
using Duende.IdentityServer.Models;
using Duende.IdentityServer.Stores;
Expand Down Expand Up @@ -152,7 +152,7 @@ public async Task Real_Client_Server_Communication()
// Using the classic policy, reading invoices should be successful because we're authenticated with a JWT that has the "invoices" audience and "invoices.read" scope
var readInvoicesResponse = await invoicesReadHttpClient.GetStringAsync("https://invoice-app.local/read-invoices", cts.Token);
Assert.Equal("This protected endpoint is for reading invoices", readInvoicesResponse);

// Using the granular policy, reading invoices should be successful because we're authenticated with a JWT that has the "invoices" audience and "invoices.read" scope
var readInvoicesGranularResponse = await invoicesReadHttpClient.GetStringAsync("https://invoice-app.local/read-invoices-granular", cts.Token);
Assert.Equal("This protected endpoint is for reading invoices", readInvoicesGranularResponse);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using CliWrap;
using CliWrap;
using Meziantou.Framework;

namespace Workleap.Authentication.ClientCredentialsGrant.Tests.OpenAPI;
Expand All @@ -9,7 +9,7 @@ public class OpenApiSecurityDescriptionTests
public async Task Given_API_With_Client_Credentials_Attribute_When_Generating_OpenAPI_Then_Equal_Expected_Document()
{
var solutionPath = GetSolutionPath();

var testsFolder = Path.Combine(solutionPath, "tests");
var projectFolder = Path.Combine(testsFolder, "WebApi.OpenAPI.SystemTest");
var generatedFilePath = Path.Combine(projectFolder, "openapi-v1.yaml");
Expand All @@ -31,12 +31,12 @@ public async Task Given_API_With_Client_Credentials_Attribute_When_Generating_Op

Assert.Equal(expectedFileContent, generatedFileContent, ignoreLineEndingDifferences: true);
}

private static string GetSolutionPath()
{
return GetGitRoot() / "src";
}

private static FullPath GetGitRoot()
{
if (FullPath.CurrentDirectory().TryFindFirstAncestorOrSelf(current => Directory.Exists(current / ".git"), out var root))
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Collections;
using System.Collections;
using Microsoft.AspNetCore.Authorization;
using Workleap.AspNetCore.Authentication.ClientCredentialsGrant;

Expand All @@ -20,7 +20,7 @@ public void GivenInvalidClassicScope_WhenCreate_ThenThrowArgumentException()
var scope = (ClientCredentialsScope)999;
Assert.Throws<ArgumentOutOfRangeException>(() => new RequireClientCredentialsAttribute(scope));
}

[Fact]
public void GivenSinglePermission_WhenCreate_ThenSamePermission()
{
Expand Down
Loading

0 comments on commit 11ce971

Please sign in to comment.