Skip to content

Commit

Permalink
[OIDC] Add allow list for Entra ID tenants acceptable for federation (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
joelverhagen authored Dec 5, 2024
1 parent 9279e51 commit af2aca9
Show file tree
Hide file tree
Showing 6 changed files with 117 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using Microsoft.IdentityModel.Protocols;
using Microsoft.IdentityModel.Tokens;
using Microsoft.IdentityModel.Validators;
using System.Linq;

#nullable enable

Expand All @@ -18,6 +19,11 @@ namespace NuGetGallery.Services.Authentication
/// </summary>
public interface IEntraIdTokenValidator
{
/// <summary>
/// Determines if a given Entra tenant ID GUID is in the allow list.
/// </summary>
bool IsTenantAllowed(Guid tenantId);

/// <summary>
/// Perform minimal validation of the token to ensure it was issued by Entra ID. Validations:
/// - Expected issuer (Entra ID)
Expand Down Expand Up @@ -49,6 +55,23 @@ public EntraIdTokenValidator(
_configuration = configuration ?? throw new ArgumentNullException(nameof(configuration));
}

public bool IsTenantAllowed(Guid tenantId)
{
if (_configuration.AllowedEntraIdTenants.Length == 0)
{
return false;
}

if (_configuration.AllowedEntraIdTenants.Length == 1
&& _configuration.AllowedEntraIdTenants[0] == "all")
{
return true;
}

var tenantIdString = tenantId.ToString();
return _configuration.AllowedEntraIdTenants.Contains(tenantIdString, StringComparer.OrdinalIgnoreCase);
}

public async Task<TokenValidationResult> ValidateAsync(JsonWebToken token)
{
if (string.IsNullOrWhiteSpace(_configuration.EntraIdAudience))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
#nullable enable

using System;
using System.ComponentModel;
using NuGet.Services.Configuration;
using NuGet.Services.Entities;

namespace NuGetGallery.Services.Authentication
{
Expand All @@ -19,12 +22,24 @@ public interface IFederatedCredentialConfiguration
/// How long the short lived API keys should last.
/// </summary>
TimeSpan ShortLivedApiKeyDuration { get; }

/// <summary>
/// The list of all Entra ID tenant GUIDs that are allowed for <see cref="FederatedCredentialType.EntraIdServicePrincipal"/>
/// federated credential policies. If this list is empty, no tenants are allowed. If this list has a single
/// item "all", then all Entra ID tenants are allowed.
///
/// Values are separated by a semicolon when provided in the configuration file.
/// </summary>
string[] AllowedEntraIdTenants { get; }
}

public class FederatedCredentialConfiguration : IFederatedCredentialConfiguration
{
public string? EntraIdAudience { get; set; }

public TimeSpan ShortLivedApiKeyDuration { get; set; } = TimeSpan.FromMinutes(15);

[TypeConverter(typeof(StringArrayConverter))]
public string[] AllowedEntraIdTenants { get; set; } = [];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,11 @@ public ValidatedJwt(JsonWebToken jwt, string identifier, RecognizedIssuer recogn
return $"The JSON web token must have a {ClaimConstants.Tid} claim that matches the policy.";
}

if (!_entraIdTokenValidator.IsTenantAllowed(parsedTid))
{
return "The tenant ID in the JSON web token is not in allow list.";
}

if (string.IsNullOrWhiteSpace(oid) || !Guid.TryParse(oid, out var parsedOid) || parsedOid != criteria.ObjectId)
{
return $"The JSON web token must have a {ClaimConstants.Oid} claim that matches the policy.";
Expand Down
1 change: 1 addition & 0 deletions src/NuGetGallery/Web.config
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@
<add key="Gallery.AllowLicenselessPackages" value="true"/>
<add key="FederatedCredential.ShortLivedApiKeyDuration" value="00:20:00"/>
<add key="FederatedCredential.EntraIdAudience" value=""/>
<add key="FederatedCredential.AllowedEntraIdTenants" value="all"/>
</appSettings>
<connectionStrings>
<add name="Gallery.SqlServer" connectionString="Data Source=(localdb)\mssqllocaldb; Initial Catalog=NuGetGallery; Integrated Security=True; MultipleActiveResultSets=True" providerName="System.Data.SqlClient"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,55 @@ namespace NuGetGallery.Services.Authentication
{
public class EntraIdTokenValidatorFacts
{
public class TheIsTenantAllowedMethod : EntraIdTokenValidatorFacts
{
[Fact]
public void AllowsTenantIdWhenInAllowList()
{
// Act
var allowed = Target.IsTenantAllowed(new Guid(AllowedTenantIds[0]));

// Assert
Assert.True(allowed);
}

[Fact]
public void RejectsTenantIdWhenInAllowList()
{
// Act
var allowed = Target.IsTenantAllowed(new Guid("b3ad8ee4-f667-4a19-9091-206ef363beb1"));

// Assert
Assert.False(allowed);
}

[Fact]
public void AllowsTenantIdWhenAllAreAllowed()
{
// Arrange
AllowedTenantIds[0] = "all";

// Act
var allowed = Target.IsTenantAllowed(new Guid("b3ad8ee4-f667-4a19-9091-206ef363beb1"));

// Assert
Assert.True(allowed);
}

[Fact]
public void AllTenantIdsAreNotAllowedWhenAllIsNotOnlyArrayItem()
{
// Arrange
AllowedTenantIds = ["all", "c311b905-19a2-483e-a014-41d0fcdc99cf"];

// Act
var allowed = Target.IsTenantAllowed(new Guid("b3ad8ee4-f667-4a19-9091-206ef363beb1"));

// Assert
Assert.False(allowed);
}
}

public class TheValidateAsyncMethod : EntraIdTokenValidatorFacts
{
[Fact]
Expand Down Expand Up @@ -136,10 +185,13 @@ public EntraIdTokenValidatorFacts()
ConfigurationRetriever.Object);
JsonWebTokenHandler = new Mock<JsonWebTokenHandler>();
Configuration = new Mock<IFederatedCredentialConfiguration>();
Configuration.Setup(x => x.EntraIdAudience).Returns("nuget-audience");

TenantId = "c311b905-19a2-483e-a014-41d0fcdc99cf";
Issuer = $"https://login.microsoftonline.com/{TenantId}/v2.0";
AllowedTenantIds = ["c311b905-19a2-483e-a014-41d0fcdc99cf"];

Configuration.Setup(x => x.EntraIdAudience).Returns("nuget-audience");
Configuration.Setup(x => x.AllowedEntraIdTenants).Returns(() => AllowedTenantIds);

Target = new EntraIdTokenValidator(
OidcConfigManager.Object,
Expand All @@ -154,6 +206,7 @@ public EntraIdTokenValidatorFacts()
public Mock<IFederatedCredentialConfiguration> Configuration { get; }
public string TenantId { get; }
public string Issuer { get; set; }
public string[] AllowedTenantIds { get; set; }

public JsonWebToken Token
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,22 @@ public async Task RejectsWrongTenantId()
Assert.Equal(FederatedCredentialPolicyResultType.Unauthorized, Assert.Single(evaluation.Results).Type);
}

[Fact]
public async Task RejectsNotAllowedTenantId()
{
// Arrange
EntraIdTokenValidator
.Setup(x => x.IsTenantAllowed(TenantId))
.Returns(() => false);

// Act
var evaluation = await Target.GetMatchingPolicyAsync(Policies, BearerToken);

// Assert
Assert.Equal(EvaluatedFederatedCredentialPoliciesType.NoMatchingPolicy, evaluation.Type);
Assert.Equal(FederatedCredentialPolicyResultType.Unauthorized, Assert.Single(evaluation.Results).Type);
}

[Fact]
public async Task RejectsWrongObjectId()
{
Expand Down Expand Up @@ -402,6 +418,9 @@ public FederatedCredentialEvaluatorFacts()
UtcNow = new DateTimeOffset(2024, 10, 10, 13, 35, 0, TimeSpan.Zero);
Expires = new DateTimeOffset(2024, 10, 11, 0, 0, 0, TimeSpan.Zero);

EntraIdTokenValidator
.Setup(x => x.IsTenantAllowed(TenantId))
.Returns(() => true);
EntraIdTokenValidator
.Setup(x => x.ValidateAsync(It.IsAny<JsonWebToken>()))
.ReturnsAsync(() => EntraIdTokenResult);
Expand Down

0 comments on commit af2aca9

Please sign in to comment.