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.

Sign up for GitHub

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

Jump to bottom

Enable Easy Auth Local Dev #789

Closed
wants to merge 6 commits into from
Closed

Enable Easy Auth Local Dev #789

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a44ff28
create testing/prod aad, publish easy auth settings, run easy auth lo…
glennamanns Oct 10, 2018
c30f947
Remove dotnet dependency by injecting Easy Auth middleware into reque…
glennamanns Oct 11, 2018
c0b2b08
Cleanup based on PR comments
glennamanns Oct 12, 2018
67e1bd4
Enable local testing of Token binding
glennamanns Oct 16, 2018
c3813e2
Update based on Chris' CR feedback
glennamanns Oct 29, 2018
c802d4b
Allow for fsharp projects, remove allowed audiences
glennamanns Oct 30, 2018
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
43 changes: 43 additions & 0 deletions src/Azure.Functions.Cli/Actions/AzureActions/CreateAADApplication.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
using System;
using System.Threading.Tasks;
using Azure.Functions.Cli.Common;
using Azure.Functions.Cli.Interfaces;
using Fclp;
namespace Azure.Functions.Cli.Actions.AzureActions
glennamanns marked this conversation as resolved.
Show resolved Hide resolved
{
// Invoke via `func azure auth create-aad -aad-name {displayNameOfAAD} --app-name {displayNameOfApp}`
[Action(Name = "create-aad", Context = Context.Azure, SubContext = Context.Auth, HelpText = "Creates a production Azure Active Directory application with given name. Can be linked to specified Azure Application")]
class CreateAADApplication : BaseAzureAction
glennamanns marked this conversation as resolved.
Show resolved Hide resolved
{
private readonly IAuthManager _authManager;

public string AADName { get; set; }
Copy link
Contributor

@cgillum cgillum Oct 12, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naming nit: "AAD" is not really a thing, so AADName sounds unnatural. I'm also not sure what the difference is between an "AADName" and an "AppName". Should they be the same? Or should "AADName" be renamed to "AppRegistrationName"? #Closed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment for the parameter syntax below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are your thoughts on AADRegistrationName for both the variable and what goes on the command line? And for Azure App Service apps/functions, appName?

Copy link
Contributor

@cgillum cgillum Oct 24, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe AADAppRegistrationName instead of AADRegistrationName? appName is fine for the function app name. #Closed


public string AppName { get; set; }

public CreateAADApplication(IAuthManager authManager)
{
_authManager = authManager;
}

public override async Task RunAsync()
{
await _authManager.CreateAADApplication(AccessToken, AADName, AppName);
}

public override ICommandLineParserResult ParseArgs(string[] args)
{
Parser
.Setup<string>("aad-name")
.WithDescription("Name of AD application to create")
Copy link
Contributor

@cgillum cgillum Oct 12, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Product naming nit: "Name of the Azure Active Directory app registration to create." #Closed

.Callback(f => AADName = f);

Parser
.Setup<string>("app-name")
.WithDescription("Name of Azure Websites Application/Function to link AAD application to")
Copy link
Contributor

@cgillum cgillum Oct 12, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grammar and product naming nits: "Name of the Azure App Service or Azure Functions app which corresponds to the Azure AD app registration." #Closed

Copy link
Contributor

@cgillum cgillum Oct 12, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get nit-picky about this since these strings are public-facing and therefore need to be considered somewhat official. #Closed

.Callback(f => AppName = f);

return base.ParseArgs(args);
}
}
}
4 changes: 1 addition & 3 deletions src/Azure.Functions.Cli/Actions/AzureActions/PublishFunctionAppAction.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@
using System.Net.Http.Headers;
using System.Threading;
using System.Threading.Tasks;
using Azure.Functions.Cli.Actions.LocalActions;
using Azure.Functions.Cli.Arm;
using Azure.Functions.Cli.Arm.Models;
using Azure.Functions.Cli.Common;
using Azure.Functions.Cli.Extensions;
Expand Down Expand Up @@ -430,7 +428,7 @@ private IDictionary<string, string> MergeAppSettings(IDictionary<string, string>
foreach (var pair in local)
{
if (result.ContainsKeyCaseInsensitive(pair.Key) &&
!result.GetValueCaseInsensitive(pair.Key).Equals(pair.Value, StringComparison.OrdinalIgnoreCase))
!string.Equals(result.GetValueCaseInsensitive(pair.Key), pair.Value, StringComparison.OrdinalIgnoreCase))
{
ColoredConsole.WriteLine($"App setting {pair.Key} is different between azure and {SecretsManager.AppSettingsFileName}");
if (OverwriteSettings)
Expand Down
75 changes: 62 additions & 13 deletions src/Azure.Functions.Cli/Actions/HostActions/StartHostAction.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
using System;
using System.Collections;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Net;
using System.Net.Http;
using System.Security.Cryptography.X509Certificates;
using System.Threading;
using System.Threading.Tasks;
using Azure.Functions.Cli.Actions.HostActions.WebHost.Security;
using Azure.Functions.Cli.Common;
Expand All @@ -17,6 +19,7 @@
using Fclp;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting;
using Microsoft.Azure.AppService.Middleware;
using Microsoft.Azure.WebJobs.Extensions.Http;
using Microsoft.Azure.WebJobs.Script;
using Microsoft.Azure.WebJobs.Script.WebHost;
Expand All @@ -39,7 +42,7 @@ namespace Azure.Functions.Cli.Actions.HostActions
[Action(Name = "start", HelpText = "Launches the functions runtime host")]
internal class StartHostAction : BaseAction
{
private const int DefaultPort = 7071;
public const int DefaultPort = 7071;
private const int DefaultTimeout = 20;
private readonly ISecretsManager _secretsManager;

Expand Down Expand Up @@ -124,7 +127,7 @@ public override ICommandLineParserResult ParseArgs(string[] args)
return Parser.Parse(args);
}

private async Task<IWebHost> BuildWebHost(ScriptApplicationHostOptions hostOptions, WorkerRuntime workerRuntime, Uri baseAddress, X509Certificate2 certificate)
private async Task<IWebHost> BuildWebHost(ScriptApplicationHostOptions hostOptions, WorkerRuntime workerRuntime, Uri baseAddress, X509Certificate2 certificate, bool authenticationEnabled, Uri baseUri)
{
IDictionary<string, string> settings = await GetConfigurationSettings(hostOptions.ScriptPath, baseAddress);
settings.AddRange(LanguageWorkerHelper.GetWorkerConfiguration(workerRuntime, LanguageWorkerSetting));
Expand All @@ -137,13 +140,18 @@ private async Task<IWebHost> BuildWebHost(ScriptApplicationHostOptions hostOptio
defaultBuilder
.UseKestrel(options =>
{
options.Listen(IPAddress.Loopback, baseAddress.Port, listenOptins =>
options.Listen(IPAddress.Loopback, baseAddress.Port, listenOptions =>
{
listenOptins.UseHttps(certificate);
listenOptions.UseHttps(certificate);
});
});
}

if (authenticationEnabled)
{
SetupMiddlewareConfig(baseUri);
}

return defaultBuilder
.UseSetting(WebHostDefaults.ApplicationKey, typeof(Startup).Assembly.GetName().Name)
.UseUrls(baseAddress.ToString())
Expand All @@ -157,7 +165,7 @@ private async Task<IWebHost> BuildWebHost(ScriptApplicationHostOptions hostOptio
loggingBuilder.AddDefaultWebJobsFilters();
loggingBuilder.AddProvider(new ColoredConsoleLoggerProvider((cat, level) => level >= LogLevel.Information));
})
.ConfigureServices((context, services) => services.AddSingleton<IStartup>(new Startup(context, hostOptions, CorsOrigins)))
.ConfigureServices((context, services) => services.AddSingleton<IStartup>(new Startup(context, hostOptions, CorsOrigins, authenticationEnabled)))
.Build();
}

Expand Down Expand Up @@ -217,9 +225,14 @@ public override async Task RunAsync()

var settings = SelfHostWebHostSettingsFactory.Create(Environment.CurrentDirectory);

(var listenUri, var baseUri, var certificate) = await Setup();
// Determine if middleware (Easy Auth) is enabled
var middlewareAuthSettings = _secretsManager.GetMiddlewareAuthSettings();
bool authenticationEnabled = middlewareAuthSettings.ContainsKeyCaseInsensitive(Constants.MiddlewareAuthEnabledSetting) &&
middlewareAuthSettings[Constants.MiddlewareAuthEnabledSetting].ToLower().Equals("true");
Copy link
Contributor

@cgillum cgillum Oct 12, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be a bit cleaner:

bool authenticationEnabled = 
    middlewareAuthSettings.TryGetValue(Constants.MiddlewareAuthEnabledSetting, out string enabledValue) &&
    bool.TryParse(enabledValue, out bool isEnabled) &&
    isEnabled;
``` #Closed


IWebHost host = await BuildWebHost(settings, workerRuntime, listenUri, certificate);
(var listenUri, var baseUri, var certificate, string certPath, string certPassword) = await Setup();

IWebHost host = await BuildWebHost(settings, workerRuntime, listenUri, certificate, authenticationEnabled, baseUri);
var runTask = host.RunAsync();

var hostService = host.Services.GetRequiredService<WebJobsScriptHostService>();
Expand Down Expand Up @@ -395,25 +408,53 @@ internal static async Task CheckNonOptionalSettings(IEnumerable<KeyValuePair<str
}
}

private async Task<(Uri listenUri, Uri baseUri, X509Certificate2 cert)> Setup()
/// <summary>
/// Add additional settings to be consumed by the EasyAuth Middleware
/// </summary>
/// <param name="baseUri"></param>
private static void SetupMiddlewareConfig(Uri baseUri)
{
// These are not necessary when Easy Auth is used in conjunction with the Functions CLI
// We add them here for compatibility with existing Easy Auth / Middleware codebase
var args = new string[] {
string.Format("{0}={1}", Constants.MiddlewareListenUrlSetting, baseUri.ToString()),
string.Format("{0}={1}", Constants.MiddlewareHostUrlSetting, baseUri.ToString()),
};

// Add above arguments, as well as the settings from local.middleware.json
IConfigurationRoot config = new ConfigurationBuilder()
.SetBasePath(Directory.GetCurrentDirectory())
.AddJsonFile(SecretsManager.MiddlewareAuthSettingsFilePath)
.AddCommandLine(args)
.Build();

ModuleConfig.HostConfig = config;
}

private async Task<(Uri listenUri, Uri baseUri, X509Certificate2 cert, string path, string password)> Setup()
{
var protocol = UseHttps ? "https" : "http";
X509Certificate2 cert = UseHttps
? await SecurityHelpers.GetOrCreateCertificate(CertPath, CertPassword)
: null;
return (new Uri($"{protocol}://0.0.0.0:{Port}"), new Uri($"{protocol}://localhost:{Port}"), cert);
if (UseHttps)
{
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

todo: revert this to the original logic

(X509Certificate2 cert, string certPath, string certPassword) = await SecurityHelpers.GetOrCreateCertificate(CertPath, CertPassword);
Copy link
Contributor

@cgillum cgillum Oct 12, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels unnatural to fetch the certificate and both its path and password. Is this because the Easy Auth code you're calling expects these? Could the Easy Auth code be refactored to take an X509Certificate2 for embedded scenarios like this? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing all of this altogether. Easy Auth, when truly middleware (which it is now), doesn't need its own reference to the certificate.

return (new Uri($"{protocol}://0.0.0.0:{Port}"), new Uri($"{protocol}://localhost:{Port}"), cert, certPath, certPassword);
}

return (new Uri($"{protocol}://0.0.0.0:{Port}"), new Uri($"{protocol}://localhost:{Port}"), null, null, null);
}

public class Startup : IStartup
{
private readonly WebHostBuilderContext _builderContext;
private readonly ScriptApplicationHostOptions _hostOptions;
private readonly string[] _corsOrigins;
private readonly bool _authenticationEnabled;

public Startup(WebHostBuilderContext builderContext, ScriptApplicationHostOptions hostOptions, string corsOrigins)
public Startup(WebHostBuilderContext builderContext, ScriptApplicationHostOptions hostOptions, string corsOrigins, bool authEnabled)
{
_builderContext = builderContext;
_hostOptions = hostOptions;
_authenticationEnabled = authEnabled;

if (!string.IsNullOrEmpty(corsOrigins))
{
Expand Down Expand Up @@ -469,6 +510,14 @@ public void Configure(IApplicationBuilder app)
IApplicationLifetime applicationLifetime = app.ApplicationServices
.GetRequiredService<IApplicationLifetime>();

if (_authenticationEnabled)
{
ILoggerProvider loggerProvider = app.ApplicationServices
.GetRequiredService<ILoggerProvider>();

app.UseAppServiceMiddleware(loggerProvider);
}

app.UseWebJobsScriptHost(applicationLifetime);
}
}
Expand Down
33 changes: 33 additions & 0 deletions src/Azure.Functions.Cli/Actions/HostActions/WebHost/Security/AuthMiddleware.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
using System.Threading.Tasks;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Logging;

namespace Azure.Functions.Cli.Actions.HostActions.WebHost.Security
{
static class AuthMiddlewareExtensions
{
public static IApplicationBuilder UseAppServiceMiddleware(this IApplicationBuilder builder, ILoggerProvider loggerProvider)
{
return builder.UseMiddleware<AuthMiddleware>(builder, loggerProvider);
}
}

class AuthMiddleware : Microsoft.Azure.AppService.MiddlewareShim.Startup
{
// The middleware delegate to call after this one finishes processing
private readonly RequestDelegate _next;

public AuthMiddleware(RequestDelegate next, IApplicationBuilder app, ILoggerProvider loggerProvider)
{
_next = next;
this.Configure(app: app, loggerFactory: null, loggerProvider: loggerProvider);
}

public Task Invoke(HttpContext httpContext)
{
// OnRequest handles calling Invoke on _next and sending the response
return this.OnRequest(httpContext, _next);
}
}
}
2 changes: 2 additions & 0 deletions src/Azure.Functions.Cli/Arm/Models/Site.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ public class Site

public IDictionary<string, string> AzureAppSettings { get; set; }

public IDictionary<string, string> AzureAuthSettings { get; set; }

public IDictionary<string, AppServiceConnectionString> ConnectionStrings { get; set; }

public bool IsLinux
Expand Down
10 changes: 10 additions & 0 deletions src/Azure.Functions.Cli/Azure.Functions.Cli.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,16 @@
<PackageReference Include="Microsoft.Azure.Functions.NodeJsWorker" Version="1.0.0-beta4-10082" />
<PackageReference Include="Microsoft.Azure.Functions.PowerShellWorker" Version="0.1.20-alpha" />
<PackageReference Include="Microsoft.Azure.WebJobs.Script.WebHost" Version="2.0.0-12134" />
<PackageReference Include="Microsoft.Extensions.Logging.AzureAppServices" Version="2.1.1" />
<PackageReference Include="Newtonsoft.Json" Version="11.0.2" />
</ItemGroup>
<ItemGroup>
<!-- ! CANNOT BE MERGED. WILL NOT BE NECESSARY WHEN AVAILABLE VIA NUGET !-->
<Reference Include="Microsoft.Azure.AppService.Middleware">
<HintPath>C:\wagit\AAPT\Antares\EasyAuth\src\EasyAuth\MiddlewareLinux\bin\Debug\netcoreapp2.0\Microsoft.Azure.AppService.Middleware.dll</HintPath>
</Reference>
<Reference Include="MiddlewareLinux">
<HintPath>C:\wagit\AAPT\Antares\EasyAuth\src\EasyAuth\MiddlewareLinux\bin\Debug\netcoreapp2.0\MiddlewareLinux.dll</HintPath>
</Reference>
</ItemGroup>
</Project>
Loading