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

Fix using user credentials during purge background operation #18

Merged
merged 17 commits into from
Mar 22, 2024
Merged
Show file tree
Hide file tree
Changes from 15 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
3 changes: 3 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ jobs:
- name: Build project
run: dotnet build -c Release

- name: Test project
run: dotnet test --no-build

- name: Lint C# code
run: dotnet format --verify-no-changes --verbosity detailed --no-restore

Expand Down
64 changes: 64 additions & 0 deletions Kerbee.Tests/Graph/GraphServiceTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
using AutoFixture.Xunit2;

using Kerbee.Graph;

using Microsoft.Graph;
using Microsoft.Kiota.Abstractions;

using Moq;
using Moq.AutoMock;

namespace Kerbee.Tests.Graph;

public class GraphServiceTests
{
private static readonly AutoMocker _mocker = new();
private static readonly Mock<IRequestAdapter> _requestAdapter = new();

private static GraphService Sut => _mocker.CreateInstance<GraphService>();


[Theory, AutoData]
public async Task GetApplicationAsync_ShouldUseManagedIdentityClient(string objectId)
{
// Arrange
SetupManagedIdentityProvider();

// Act
await Sut.GetApplicationAsync(objectId);

// Assert
_mocker.VerifyAll();
}

[Theory, AutoData]
public async Task RemoveCertificateAsync_ShouldUseManagedIdentityClient(string objectId, string keyId)
{
// Arrange
SetupManagedIdentityProvider();

// Act
await Sut.RemoveCertificateAsync(objectId, keyId);

// Assert
_mocker.VerifyAll();
}

[Theory, AutoData]
public async Task RemoveSecretAsync_ShouldUseManagedIdentityClient(string objectId)
{
// Arrange
SetupManagedIdentityProvider();

// Act
await Sut.GetApplicationAsync(objectId);

// Assert
_mocker.VerifyAll();
}

private static void SetupManagedIdentityProvider()
{
_mocker.Use<IManagedIdentityProvider>(x => x.GetClient() == new GraphServiceClient(_requestAdapter.Object, null));
}
}
29 changes: 29 additions & 0 deletions Kerbee.Tests/Kerbee.Tests.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>net8.0</TargetFramework>
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>

<IsPackable>false</IsPackable>
<IsTestProject>true</IsTestProject>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="AutoFixture.Xunit2" Version="4.18.1" />
<PackageReference Include="coverlet.collector" Version="6.0.0" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.8.0" />
<PackageReference Include="Moq.AutoMock" Version="3.5.0" />
<PackageReference Include="xunit" Version="2.5.3" />
<PackageReference Include="xunit.runner.visualstudio" Version="2.5.3" />
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\Kerbee\Kerbee.csproj" />
</ItemGroup>

<ItemGroup>
<Using Include="Xunit" />
</ItemGroup>

</Project>
6 changes: 6 additions & 0 deletions Kerbee.sln
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ VisualStudioVersion = 17.0.31912.275
MinimumVisualStudioVersion = 10.0.40219.1
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Kerbee", "Kerbee\Kerbee.csproj", "{81F62D09-D16D-4B0C-9DAE-C075580F5021}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Kerbee.Tests", "Kerbee.Tests\Kerbee.Tests.csproj", "{E880CE14-61C4-4EBA-A073-793F415DD2F0}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Expand All @@ -15,6 +17,10 @@ Global
{81F62D09-D16D-4B0C-9DAE-C075580F5021}.Debug|Any CPU.Build.0 = Debug|Any CPU
{81F62D09-D16D-4B0C-9DAE-C075580F5021}.Release|Any CPU.ActiveCfg = Release|Any CPU
{81F62D09-D16D-4B0C-9DAE-C075580F5021}.Release|Any CPU.Build.0 = Release|Any CPU
{E880CE14-61C4-4EBA-A073-793F415DD2F0}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{E880CE14-61C4-4EBA-A073-793F415DD2F0}.Debug|Any CPU.Build.0 = Debug|Any CPU
{E880CE14-61C4-4EBA-A073-793F415DD2F0}.Release|Any CPU.ActiveCfg = Release|Any CPU
{E880CE14-61C4-4EBA-A073-793F415DD2F0}.Release|Any CPU.Build.0 = Release|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
Expand Down
14 changes: 0 additions & 14 deletions Kerbee/Functions/PurgeExpiredCertificatesAndSecrets.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using Kerbee.Options;

using Microsoft.Azure.Functions.Worker;
using Microsoft.Azure.Functions.Worker.Http;
using Microsoft.DurableTask;
using Microsoft.DurableTask.Client;
using Microsoft.Extensions.Logging;
Expand Down Expand Up @@ -45,19 +44,6 @@ public async Task Timer([TimerTrigger("0 0 0 * * *")] TimerInfo timer, [DurableC
// Function input comes from the request content.
var instanceId = await starter.ScheduleNewOrchestrationInstanceAsync($"{nameof(PurgeExpiredCertificatesAndSecrets)}_{nameof(Orchestrator)}");

_logger.LogInformation($"Started orchestration with ID = '{instanceId}'.");
}

[Function($"{nameof(PurgeExpiredCertificatesAndSecrets)}_{nameof(HttpStart)}")]
public async Task<HttpResponseData> HttpStart(
[HttpTrigger(AuthorizationLevel.Anonymous, "get", Route = "api/purge")] HttpRequestData req,
[DurableClient] DurableTaskClient starter)
{
// Function input comes from the request content.
var instanceId = await starter.ScheduleNewOrchestrationInstanceAsync($"{nameof(PurgeExpiredCertificatesAndSecrets)}_{nameof(Orchestrator)}");

_logger.LogInformation("Started orchestration with {OrchestrationId}", instanceId);

return starter.CreateCheckStatusResponse(req, instanceId);
}
}
2 changes: 1 addition & 1 deletion Kerbee/Functions/PurgeExpiredKeysActivity.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public async Task<object> RunAsync([ActivityTrigger] Application application)
}
catch (Exception ex)
{
_logger.LogError(ex, "Error purging keys for application {ApplicationId}", application.Id);
_logger.LogError(ex, "Error purging keys for application {ApplicationId}", application.AppId);
throw;
}
}
Expand Down
2 changes: 1 addition & 1 deletion Kerbee/Functions/RenewKeyActivity.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public async Task<object> RunAsync([ActivityTrigger] Application application)
}
catch (Exception ex)
{
_logger.LogError(ex, "Error renewing key for application {ApplicationId}", application.Id);
_logger.LogError(ex, "Error renewing key for application {ApplicationId}", application.AppId);
throw;
}
}
Expand Down
17 changes: 14 additions & 3 deletions Kerbee/Graph/ApplicationService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,17 @@ public async Task UpdateApplications()
applications.Remove(application);
}

// Update the names of the applications in the table
foreach (var application in applications)
{
var graphApplication = graphApplications.FirstOrDefault(x => x.Id == application.Id.ToString());
if (graphApplication?.DisplayName is not null && graphApplication.DisplayName != application.DisplayName)
{
application.DisplayName = graphApplication.DisplayName;
await _tableClient.UpdateEntityAsync(application.ToEntity(), ETag.All);
}
}

// Add applications that are owned by kerbee in the graph but not in the table
var applicationsPendingManagement = graphApplications
.Where(x => applications.None(application => application.Id.ToString() == x.Id))
Expand All @@ -132,7 +143,7 @@ public async Task UpdateApplications()

public async Task RenewCertificate(Application application, bool replaceCurrent)
{
_logger.LogInformation("Renewing certificate for application {applicationId}", application.Id);
_logger.LogInformation("Renewing certificate for application {applicationId}", application.AppId);

// Generate certificate in Azure Key Vault
var policy = CertificatePolicy.Default;
Expand Down Expand Up @@ -187,7 +198,7 @@ private async Task RestoreDeletedCertificate(string? keyName)

public async Task RenewSecret(Application application, bool replaceCurrent)
{
_logger.LogInformation("Renewing secret for application {applicationId}", application.Id);
_logger.LogInformation("Renewing secret for application {applicationId}", application.AppId);

// Delete the current secret if requested
if (replaceCurrent && application.KeyId is not null)
Expand Down Expand Up @@ -283,7 +294,7 @@ public async Task RemoveKeyAsync(Application application)
{
if (application.KeyId is null)
{
_logger.LogWarning("Application {applicationId} does not have a key", application.Id);
_logger.LogWarning("Application {applicationId} does not have a key", application.AppId);
return;
}

Expand Down
13 changes: 6 additions & 7 deletions Kerbee/Graph/GraphService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
using Microsoft.Graph.Applications.Item.AddPassword;
using Microsoft.Graph.Applications.Item.RemovePassword;
using Microsoft.Graph.Models;
using Microsoft.Identity.Client.AppConfig;
using Microsoft.Kiota.Abstractions;
using Microsoft.Kiota.Http.HttpClientLibrary.Middleware.Options;

Expand All @@ -24,12 +23,12 @@ namespace Kerbee.Graph;
public class GraphService : IGraphService
{
private readonly ILogger _logger;
private readonly ManagedIdentityProvider _managedIdentityProvider;
private readonly IManagedIdentityProvider _managedIdentityProvider;
private readonly IClaimsPrincipalAccessor _claimsPrincipalAccessor;
private readonly IOptions<WebsiteOptions> _websiteOptions;

public GraphService(
ManagedIdentityProvider managedIdentityProvider,
IManagedIdentityProvider managedIdentityProvider,
IClaimsPrincipalAccessor claimsPrincipalAccessor,
IOptions<WebsiteOptions> websiteOptions,
ILoggerFactory loggerFactory)
Expand Down Expand Up @@ -165,7 +164,7 @@ await client.Applications[applicationObjectId]

public async Task RemoveCertificateAsync(string applicationObjectId, string keyId)
{
var client = GetClientForUser();
var client = _managedIdentityProvider.GetClient();

var application = await client.Applications[applicationObjectId].GetAsync();
var key = application?.KeyCredentials?
Expand All @@ -181,7 +180,8 @@ public async Task RemoveCertificateAsync(string applicationObjectId, string keyI

public async Task RemoveSecretAsync(string applicationObjectId, Guid keyId)
{
var client = GetClientForUser();
var client = _managedIdentityProvider.GetClient();

await client.Applications[applicationObjectId]
.RemovePassword
.PostAsync(new RemovePasswordPostRequestBody()
Expand Down Expand Up @@ -284,8 +284,7 @@ public async Task<PasswordCredential> GenerateSecretAsync(string applicationObje

public async Task<Application?> GetApplicationAsync(string applicationObjectId)
{
var client = GetClientForUser();
return await client.Applications[applicationObjectId].GetAsync();
return await _managedIdentityProvider.GetClient().Applications[applicationObjectId].GetAsync();
}

private async Task<IEnumerable<Application>> GetApplicationsInternalAsync()
Expand Down
11 changes: 11 additions & 0 deletions Kerbee/Graph/IManagedIdentityProvider.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
using System.Threading.Tasks;

using Microsoft.Graph;
using Microsoft.Graph.Models;

namespace Kerbee.Graph;
public interface IManagedIdentityProvider
{
Task<ServicePrincipal> GetAsync();
GraphServiceClient GetClient();
}
2 changes: 1 addition & 1 deletion Kerbee/Graph/ManagedIdentityProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

namespace Kerbee.Graph;

public class ManagedIdentityProvider
public class ManagedIdentityProvider : IManagedIdentityProvider
{
private readonly GraphServiceClient _managedIdentityClient;
private readonly Task<ServicePrincipal> _loadManagedIdentity;
Expand Down
2 changes: 1 addition & 1 deletion Kerbee/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@

services.AddSingleton<WebhookInvoker>();
services.AddSingleton<ILifecycleNotificationHelper, WebhookLifeCycleNotification>();
services.AddSingleton<ManagedIdentityProvider>();
services.AddSingleton<IManagedIdentityProvider, ManagedIdentityProvider>();

services.AddScoped<IGraphService, GraphService>();

Expand Down
12 changes: 11 additions & 1 deletion Kerbee/wwwroot/dashboard/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,17 @@ <h2 class="title is-4">
</div>
<div class="field-body">
<div class="content">
{{ details.application.displayName }}
<a :href="`https://portal.azure.com/#view/Microsoft_AAD_RegisteredApps/ApplicationMenuBlade/~/Overview/appId/${details.application.appId}/isMSAApp~/false`">{{ details.application.displayName }}</a>
</div>
</div>
</div>
<div class="field is-horizontal">
<div class="field-label">
<label class="label">Application ID</label>
</div>
<div class="field-body">
<div class="content">
{{ details.application.appId }}
</div>
</div>
</div>
Expand Down
Loading