Skip to content

Commit 1abc175

Browse files
authored
[OIDC] Add IFederatedCredentialValidator for additional token validation (#10306)
1 parent 7d8689d commit 1abc175

10 files changed

Lines changed: 364 additions & 56 deletions

File tree

Directory.Packages.props

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
<PackageVersion Include="Microsoft.CodeDom.Providers.DotNetCompilerPlatform" Version="3.6.0" />
6464
<PackageVersion Include="Microsoft.Data.Services.Client" Version="5.8.4" />
6565
<PackageVersion Include="Microsoft.Data.Services" Version="5.8.4" />
66+
<PackageVersion Include="Microsoft.Extensions.Caching.Memory" Version="8.0.1" />
6667
<PackageVersion Include="Microsoft.Extensions.CommandLineUtils" Version="1.1.1" />
6768
<PackageVersion Include="Microsoft.Extensions.Configuration.Abstractions" Version="8.0.0" />
6869
<PackageVersion Include="Microsoft.Extensions.Configuration.Binder" Version="8.0.2" />
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
#nullable enable
5+
6+
namespace NuGetGallery.Services.Authentication
7+
{
8+
public enum FederatedCredentialValidationType
9+
{
10+
/// <summary>
11+
/// The federated credential validation resulted in the request being unauthorized. A user visible error may be provided.
12+
/// This result will take precedence over other results. This may be due to a bad credential format or another failure reason.
13+
/// </summary>
14+
Unauthorized = 1,
15+
16+
/// <summary>
17+
/// The validator is not applicable to the given input. If no other validator is available, the token should be considered invalid.
18+
/// </summary>
19+
NotApplicable = 2,
20+
21+
/// <summary>
22+
/// The token is valid. This will take precedence over other <see cref="NotApplicable"/> results.
23+
/// </summary>
24+
Valid = 3,
25+
}
26+
27+
public class FederatedCredentialValidation
28+
{
29+
private static readonly FederatedCredentialValidation ValidInstance = new(FederatedCredentialValidationType.Valid, userError: null);
30+
private static readonly FederatedCredentialValidation NotApplicableInstance = new(FederatedCredentialValidationType.NotApplicable, userError: null);
31+
32+
private FederatedCredentialValidation(FederatedCredentialValidationType type, string? userError)
33+
{
34+
Type = type;
35+
UserError = userError;
36+
}
37+
38+
public FederatedCredentialValidationType Type { get; }
39+
public string? UserError { get; }
40+
41+
public static FederatedCredentialValidation Unauthorized(string? userError) => new(FederatedCredentialValidationType.Unauthorized, userError);
42+
public static FederatedCredentialValidation NotApplicable() => NotApplicableInstance;
43+
public static FederatedCredentialValidation Valid() => ValidInstance;
44+
}
45+
}
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
#nullable enable
5+
6+
using System.Collections.Generic;
7+
using System.Collections.Specialized;
8+
using System.Security.Claims;
9+
using System.Threading.Tasks;
10+
11+
namespace NuGetGallery.Services.Authentication
12+
{
13+
/// <summary>
14+
/// A validator for federated credentials, in addition to any built-in validations.
15+
/// </summary>
16+
public interface IFederatedCredentialValidator
17+
{
18+
/// <summary>
19+
/// Validate the request headers of a given federated credential type.
20+
/// </summary>
21+
/// <param name="requestHeaders">The request headers containing the federated credential.</param>
22+
/// <param name="issuer">The detected issuer type.</param>
23+
/// <param name="unvalidatedClaims">
24+
/// The claims provided by the federated credential.
25+
/// It is the responsiblity of this method to validate claims before using them.</param>
26+
Task<FederatedCredentialValidation> ValidateAsync(
27+
NameValueCollection requestHeaders,
28+
FederatedCredentialIssuerType issuer,
29+
IEnumerable<Claim>? unvalidatedClaims);
30+
}
31+
}

src/NuGetGallery.Core/NuGetGallery.Core.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
<PackageReference Include="Azure.Identity" />
4545
<PackageReference Include="Azure.Storage.Blobs" />
4646
<PackageReference Include="EntityFramework" />
47+
<PackageReference Include="Microsoft.Extensions.Caching.Memory" />
4748
<PackageReference Include="NuGet.Packaging" />
4849
<PackageReference Include="System.Formats.Asn1" />
4950
</ItemGroup>

src/NuGetGallery.Services/Authentication/Federated/FederatedCredentialPolicyEvaluator.cs

Lines changed: 64 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System;
55
using System.Collections.Generic;
6+
using System.Collections.Specialized;
67
using System.Linq;
78
using System.Text.Json;
89
using System.Threading.Tasks;
@@ -19,40 +20,53 @@ namespace NuGetGallery.Services.Authentication
1920
{
2021
public interface IFederatedCredentialPolicyEvaluator
2122
{
22-
Task<EvaluatedFederatedCredentialPolicies> GetMatchingPolicyAsync(IReadOnlyCollection<FederatedCredentialPolicy> policies, string bearerToken);
23+
Task<EvaluatedFederatedCredentialPolicies> GetMatchingPolicyAsync(
24+
IReadOnlyCollection<FederatedCredentialPolicy> policies,
25+
string bearerToken,
26+
NameValueCollection requestHeaders);
2327
}
2428

2529
public class FederatedCredentialPolicyEvaluator : IFederatedCredentialPolicyEvaluator
2630
{
2731
private readonly IEntraIdTokenValidator _entraIdTokenValidator;
32+
private readonly IReadOnlyList<IFederatedCredentialValidator> _additionalValidators;
2833
private readonly IAuditingService _auditingService;
2934
private readonly IDateTimeProvider _dateTimeProvider;
3035
private readonly ILogger<FederatedCredentialPolicyEvaluator> _logger;
3136

3237
public FederatedCredentialPolicyEvaluator(
3338
IEntraIdTokenValidator entraIdTokenValidator,
39+
IReadOnlyList<IFederatedCredentialValidator> additionalValidators,
3440
IAuditingService auditingService,
3541
IDateTimeProvider dateTimeProvider,
3642
ILogger<FederatedCredentialPolicyEvaluator> logger)
3743
{
3844
_entraIdTokenValidator = entraIdTokenValidator ?? throw new ArgumentNullException(nameof(entraIdTokenValidator));
45+
_additionalValidators = additionalValidators ?? throw new ArgumentNullException(nameof(additionalValidators));
3946
_auditingService = auditingService ?? throw new ArgumentNullException(nameof(auditingService));
4047
_dateTimeProvider = dateTimeProvider ?? throw new ArgumentNullException(nameof(dateTimeProvider));
4148
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
4249
}
4350

44-
public async Task<EvaluatedFederatedCredentialPolicies> GetMatchingPolicyAsync(IReadOnlyCollection<FederatedCredentialPolicy> policies, string bearerToken)
51+
public async Task<EvaluatedFederatedCredentialPolicies> GetMatchingPolicyAsync(
52+
IReadOnlyCollection<FederatedCredentialPolicy> policies,
53+
string bearerToken,
54+
NameValueCollection requestHeaders)
4555
{
4656
// perform basic validations not specific to any federated credential policy
4757
// the error message is user-facing and should not leak sensitive information
4858
var (userError, jwtInfo) = await ValidateJwtByIssuer(bearerToken);
4959

60+
// Whether or not we have detected a problem already, pass the information to all additional validators.
61+
// This allows custom logic to execute but will not override any initial failed validation result.
62+
userError = await ExecuteAdditionalValidatorsAsync(requestHeaders, userError, jwtInfo);
63+
5064
var externalCredentialAudit = jwtInfo.CreateAuditRecord();
5165
await AuditExternalCredentialAsync(externalCredentialAudit);
5266

5367
if (userError is not null)
5468
{
55-
_logger.LogInformation("The bearer token could not be validated. Reason: {UserError}", userError);
69+
_logger.LogInformation("The bearer token failed validation. Reason: {UserError}", userError);
5670
return EvaluatedFederatedCredentialPolicies.BadToken(userError);
5771
}
5872

@@ -81,6 +95,53 @@ public async Task<EvaluatedFederatedCredentialPolicies> GetMatchingPolicyAsync(I
8195
return EvaluatedFederatedCredentialPolicies.NoMatchingPolicy(results);
8296
}
8397

98+
private async Task<string?> ExecuteAdditionalValidatorsAsync(NameValueCollection requestHeaders, string? userError, JwtInfo jwtInfo)
99+
{
100+
bool hasUnauthorized = false;
101+
foreach (var validator in _additionalValidators)
102+
{
103+
var result = await validator.ValidateAsync(requestHeaders, jwtInfo.IssuerType, jwtInfo.Jwt?.Claims);
104+
switch (result.Type)
105+
{
106+
case FederatedCredentialValidationType.Unauthorized:
107+
// prefer the first user error, which will be the built-in user error if the bearer token is already rejected
108+
userError ??= result.UserError;
109+
hasUnauthorized = true;
110+
if (jwtInfo.IsValid)
111+
{
112+
_logger.LogWarning(
113+
"With issuer type {IssuerType}, the additional validator {Type} rejected the request, but the base validation accepted the bearer token. " +
114+
"Additional validator user message: {UserError}",
115+
jwtInfo.IssuerType,
116+
validator.GetType().FullName,
117+
result.UserError ?? "(none)");
118+
}
119+
break;
120+
case FederatedCredentialValidationType.Valid:
121+
if (!jwtInfo.IsValid)
122+
{
123+
_logger.LogWarning(
124+
"With issuer type {IssuerType}, the additional validator {Type} accepted the request, but the base validation rejected the bearer token.",
125+
jwtInfo.IssuerType,
126+
validator.GetType().FullName);
127+
}
128+
break;
129+
case FederatedCredentialValidationType.NotApplicable:
130+
break;
131+
default:
132+
throw new NotImplementedException("Unsupported validation type:" + result.Type);
133+
}
134+
}
135+
136+
if (hasUnauthorized)
137+
{
138+
userError ??= "The request could not be authenticated.";
139+
jwtInfo.IsValid = false;
140+
}
141+
142+
return userError;
143+
}
144+
84145
private async Task AuditPolicyComparisonAsync(ExternalSecurityTokenAuditRecord externalCredentialAudit, FederatedCredentialPolicy policy, bool success)
85146
{
86147
await _auditingService.SaveAuditRecordAsync(FederatedCredentialPolicyAuditRecord.Compare(

src/NuGetGallery.Services/Authentication/Federated/FederatedCredentialService.cs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
5+
using System.Collections.Specialized;
56
using System.Data;
67
using System.Text.Json;
78
using System.Threading.Tasks;
@@ -22,8 +23,9 @@ public interface IFederatedCredentialService
2223
/// </summary>
2324
/// <param name="username">The username of the user account that owns the federated credential policy.</param>
2425
/// <param name="bearerToken">The bearer token to use for federated credential evaluation.</param>
26+
/// <param name="requestHeaders">The HTTP headers used for the request. This provides full context needed for additional request validation.</param>
2527
/// <returns>The result, successful if <see cref="GenerateApiKeyResult.Type"/> is <see cref="GenerateApiKeyResultType.Created"/>.</returns>
26-
Task<GenerateApiKeyResult> GenerateApiKeyAsync(string username, string bearerToken);
28+
Task<GenerateApiKeyResult> GenerateApiKeyAsync(string username, string bearerToken, NameValueCollection requestHeaders);
2729

2830
/// <summary>
2931
/// Adds a new federated credential policy for an Entra ID service principal. The policy will be owned by the user account
@@ -138,7 +140,7 @@ public async Task DeletePolicyAsync(FederatedCredentialPolicy policy)
138140
await _auditingService.SaveAuditRecordAsync(auditRecord);
139141
}
140142

141-
public async Task<GenerateApiKeyResult> GenerateApiKeyAsync(string username, string bearerToken)
143+
public async Task<GenerateApiKeyResult> GenerateApiKeyAsync(string username, string bearerToken, NameValueCollection requestHeaders)
142144
{
143145
var currentUser = _userService.FindByUsername(username, includeDeleted: false);
144146
if (currentUser is null)
@@ -147,7 +149,7 @@ public async Task<GenerateApiKeyResult> GenerateApiKeyAsync(string username, str
147149
}
148150

149151
var policies = _repository.GetPoliciesCreatedByUser(currentUser.Key);
150-
var policyEvaluation = await _evaluator.GetMatchingPolicyAsync(policies, bearerToken);
152+
var policyEvaluation = await _evaluator.GetMatchingPolicyAsync(policies, bearerToken, requestHeaders);
151153
switch (policyEvaluation.Type)
152154
{
153155
case EvaluatedFederatedCredentialPoliciesType.BadToken:

src/NuGetGallery/App_Start/DefaultDependenciesModule.cs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,18 @@ private static void ConfigureFederatedCredentials(ContainerBuilder builder, Conf
586586
.As<IEntraIdTokenValidator>()
587587
.InstancePerLifetimeScope();
588588

589+
builder
590+
.Register(c =>
591+
{
592+
var configurationFactory = c.Resolve<IConfigurationFactory>();
593+
return GetAddInServices<IFederatedCredentialValidator>(sp =>
594+
{
595+
sp.ComposeExportedValue(configurationFactory);
596+
}).ToList();
597+
})
598+
.As<IReadOnlyList<IFederatedCredentialValidator>>() // a singleton, materialized list
599+
.SingleInstance();
600+
589601
builder
590602
.RegisterType<FederatedCredentialPolicyEvaluator>()
591603
.As<IFederatedCredentialPolicyEvaluator>()

src/NuGetGallery/Web.config

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -532,6 +532,14 @@
532532
</system.diagnostics>
533533
<runtime>
534534
<assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
535+
<dependentAssembly>
536+
<assemblyIdentity name="Microsoft.Extensions.Caching.Memory" publicKeyToken="ADB9793829DDAE60" culture="neutral"/>
537+
<bindingRedirect oldVersion="0.0.0.0-8.0.0.1" newVersion="8.0.0.1"/>
538+
</dependentAssembly>
539+
<dependentAssembly>
540+
<assemblyIdentity name="Microsoft.Extensions.Caching.Abstractions" publicKeyToken="ADB9793829DDAE60" culture="neutral"/>
541+
<bindingRedirect oldVersion="0.0.0.0-8.0.0.0" newVersion="8.0.0.0"/>
542+
</dependentAssembly>
535543
<dependentAssembly>
536544
<assemblyIdentity name="WebGrease" publicKeyToken="31BF3856AD364E35" culture="neutral"/>
537545
<bindingRedirect oldVersion="0.0.0.0-1.6.5135.21930" newVersion="1.6.5135.21930"/>

0 commit comments

Comments
 (0)