-
Notifications
You must be signed in to change notification settings - Fork 496
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
Auth for dashboard web app and resource service client #3033
Conversation
806b7ba
to
fa49451
Compare
$"{ResourceServiceClientCertificatePasswordVariableName}. Review the dashboard's configuration."); | ||
} | ||
|
||
httpHandler.SslOptions = new SslClientAuthenticationOptions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may wish to have a RemoteCertificateValidationCallback
here that validates the server certificate's thumbprint (and corresponding config to pass that cert in).
We currently don't have a resource service implementation that works with certificates. To test this, I created certs with:
Then the following changes to diff --git a/src/Aspire.Hosting/Aspire.Hosting.csproj b/src/Aspire.Hosting/Aspire.Hosting.csproj
index 0707a57c..bc25bdd0 100644
--- a/src/Aspire.Hosting/Aspire.Hosting.csproj
+++ b/src/Aspire.Hosting/Aspire.Hosting.csproj
@@ -26,6 +26,7 @@
<ItemGroup>
<PackageReference Include="Grpc.AspNetCore" />
<PackageReference Include="KubernetesClient" />
+ <PackageReference Include="Microsoft.AspNetCore.Authentication.Certificate" />
<PackageReference Include="Microsoft.Extensions.Hosting" />
</ItemGroup> diff --git a/src/Aspire.Hosting/Dashboard/DashboardService.cs b/src/Aspire.Hosting/Dashboard/DashboardService.cs
index 16cec459a..4c2b1da2d 100644
--- a/src/Aspire.Hosting/Dashboard/DashboardService.cs
+++ b/src/Aspire.Hosting/Dashboard/DashboardService.cs
@@ -4,6 +4,7 @@
using System.Text.RegularExpressions;
using Aspire.V1;
using Grpc.Core;
+using Microsoft.AspNetCore.Authorization;
using Microsoft.Extensions.Hosting;
namespace Aspire.Hosting.Dashboard;
@@ -15,6 +16,7 @@ namespace Aspire.Hosting.Dashboard;
/// An instance of this type is created for every gRPC service call, so it may not hold onto any state
/// required beyond a single request. Longer-scoped data is stored in <see cref="DashboardServiceData"/>.
/// </remarks>
+[Authorize]
internal sealed partial class DashboardService(DashboardServiceData serviceData, IHostEnvironment hostEnvironment, IHostApplicationLifetime hostApplicationLifetime)
: V1.DashboardService.DashboardServiceBase
{ diff --git a/src/Aspire.Hosting/Dashboard/DashboardServiceHost.cs b/src/Aspire.Hosting/Dashboard/DashboardServiceHost.cs
index 20ee29d0..71d2df3a 100644
--- a/src/Aspire.Hosting/Dashboard/DashboardServiceHost.cs
+++ b/src/Aspire.Hosting/Dashboard/DashboardServiceHost.cs
@@ -3,13 +3,16 @@
using System.Diagnostics;
using System.Net;
+using System.Security.Cryptography.X509Certificates;
using Aspire.Hosting.ApplicationModel;
using Aspire.Hosting.Dcp;
+using Microsoft.AspNetCore.Authentication.Certificate;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Hosting.Server;
using Microsoft.AspNetCore.Hosting.Server.Features;
using Microsoft.AspNetCore.Server.Kestrel.Core;
+using Microsoft.AspNetCore.Server.Kestrel.Https;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
@@ -72,6 +75,53 @@ internal sealed class DashboardServiceHost : IHostedService
// Turn on HTTPS
builder.WebHost.UseKestrelHttpsConfiguration();
+ #region TEMPORARY TEST CODE
+
+ // Auth
+ builder.Services
+ .AddAuthentication(CertificateAuthenticationDefaults.AuthenticationScheme)
+ .AddCertificate(options =>
+ {
+ // Disallow self-signed certs.
+ options.AllowedCertificateTypes = CertificateTypes.Chained;
+
+ // Revocation checks require an online CA, which we don't have during testing.
+ options.RevocationMode = X509RevocationMode.NoCheck;
+
+ options.Events = new CertificateAuthenticationEvents()
+ {
+ OnAuthenticationFailed = context =>
+ {
+ _logger.LogError(context.Exception, "Failed authentication.");
+
+ return Task.CompletedTask;
+ },
+ OnCertificateValidated = context =>
+ {
+ // TODO validate certificate is as expected
+
+ _logger.LogInformation("Authentication complete.");
+
+ return Task.CompletedTask;
+ }
+ };
+ });
+
+ builder.Services.AddAuthorization();
+
+ builder.Services.Configure<KestrelServerOptions>(options =>
+ {
+ options.ConfigureHttpsDefaults(options =>
+ {
+ options.ServerCertificate = new X509Certificate2(@"d:\repos\tmp\certs\server.pfx", "server", X509KeyStorageFlags.DefaultKeySet);
+ options.ClientCertificateMode = ClientCertificateMode.RequireCertificate;
+ });
+ });
+
+ #endregion
+
// Configuration
builder.Services.AddSingleton(configuration);
@@ -80,7 +130,7 @@ internal sealed class DashboardServiceHost : IHostedService
builder.Services.AddSingleton(loggerOptions);
builder.Services.Add(ServiceDescriptor.Singleton(typeof(ILogger<>), typeof(Logger<>)));
- builder.Services.AddGrpc();
+ builder.Services.AddGrpc(options => options.EnableDetailedErrors = true);
builder.Services.AddSingleton(applicationModel);
builder.Services.AddSingleton(kubernetesService);
builder.Services.AddSingleton<DashboardServiceData>();
@@ -91,6 +141,13 @@ internal sealed class DashboardServiceHost : IHostedService
_app = builder.Build();
+ #region TEMPORARY TEST CODE
+
+ _app.UseAuthentication();
+ _app.UseAuthorization();
+
+ #endregion
+
_app.MapGrpcService<DashboardService>();
}
catch (Exception ex) The config must be updated to ensure |
This adds the ability to configured the gRPC connection from the dashboard to a resource service to use certificates for authentication. Such auth is not used in the local dev scenario, so the app host is changed to suppress auth via the `DOTNET_RESOURCE_SERVICE_DISABLE_AUTH` environment variable. The dashboard will now require certificates unless this variable is set to `1`. This discourages self-hosting the dashboard in an insecure manner.
fa49451
to
d1baf4c
Compare
Integration tests for this would be valuable. You can create a very simple ASP.NET Core app with a dummy OTLP endpoint and cert auth, then call it from the dashboard. I added a number of integration tests for various OTLP scenarios. You can look to them as examples for how to do integration tests with ASP.NET Core apps. Launching the dashboard to call the OTLP endpoint would be the most realistic, but I don't know a could way to then assert results. Could just test DashboardClient in isolation. |
This is on my radar. I won't have time to add them before the p5 snap. |
@kvenkatrajan @JamesNK @davidfowl this is ready for review. I'll investigate the test failure tomorrow. EDIT test failures addressed |
It would be great to have an example using this with keycloak. Could be a playground project (and sample). Not required to get this in but we should do it (especially since it'll serve as a way of testing the functionality). |
@javiercn @halter73 @SteveSandersonMS can I get you to take a look at how we're doing auth in Blazor server? One thing I think we may want to do is show the user information somewhere. @kvenkatrajan @leslierichardson95 do we want more ux around logging in? The current mode will just challenges for auth immediately without a user gesture. |
@davidfowl I've taken a look and it looks good. The user should be automatically available in Blazor during SSR render and in interactive Server. |
Should we be using the top level attribute or the authorize view on the router so we can show login screen options? |
Do you mean the AuthorizeView with the |
This was missed in dotnet#3033.
This was missed in dotnet#3033.
This was missed in dotnet#3033.
* Enable basic certificate auth for resource service This adds the ability to configured the gRPC connection from the dashboard to a resource service to use certificates for authentication. Such auth is not used in the local dev scenario, so the app host is changed to suppress auth via the `DOTNET_RESOURCE_SERVICE_DISABLE_AUTH` environment variable. The dashboard will now require certificates unless this variable is set to `1`. This discourages self-hosting the dashboard in an insecure manner. * Support loading client cert from keystore * Use enum for resource service auth mode * Dashboard web app supports OpenID Connect * Use a single authentication call for both the web app and OTLP Use explicit policies and configure those policies with the authentication schemes required. Also rejig some config settings for consistency. * Remove redundant configuration key * Rename "web app" to "frontend" * Reorder readme sections and adjust headings and their levels * Seal class * Simplify frontend auth implementation * Add docs * More thorough enum validation * Remove unused config key * Change exception type
This was missed in dotnet#3033.
Adds auth to both:
Several new configuration options exist. These are documented in the README changes in the PR.
Here's a recording of sign in with Google:
Follow up items:
Microsoft Reviewers: Open in CodeFlow