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

Add coding standard #90

Merged
merged 9 commits into from
Aug 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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