Enable Easy Auth Local Dev #789

glennamanns · 2018-10-10T17:24:13Z

Main goal: Enable local development with Easy Auth / Middleware (rebranding in process).

This is based on Easy Auth Linux, and cannot be incorporated into this repo until the EA Linux + Windows merge is complete. However, I'd like to get feedback on it now.

One new command, used two different ways:

func auth create-aad --aad-name {AADAppName}
This one is exclusively for local testing. It creates a new AAD application with localhost:7071 in the reply URLs, a homepage of localhost:7071, and a URI with localhost in the name. The client ID, client secret, and other settings that Easy Auth needs to run locally are saved to a new file local.middleware.json. This file can later be consumed by the Easy Auth webhost.
func auth create-aad --aad-name {AADAppName} --app-name {ExistingAzureAppName}
This is designed to link an existing Function/Site with a new AAD application. It creates a new AAD application, but there is nothing related to localhost in its settings. The settings are the same as if you were to create a new app from the Portal's Easy Auth flow. This command also does not create a new .json file (to avoid writing secrets to disk). The existing Function/Site's authsettings (WEBSITE_AUTH_ENABLED, etc) are updated via an ARM call.

Still to do:

Easy Auth Linux + Windows merge. Need to inject dependency upon EasyAuth/Middleware Nuget.

…cally

ahmelsayed · 2018-10-10T17:29:28Z

src/Azure.Functions.Cli/Actions/HostActions/StartHostAction.cs

+            }
+            else
+            {
+                throw new FileNotFoundException("Must have dotnet installed in order to use local authentication.");


I haven't looked at the full PR yet, but this will be a problem. We only have one code path that depends on dotnet right now (other than .NET development scenarios of course) and we are tracking removing that dependency here #367. Ideally we wouldn't add more dependencies on dotnet for other non-.NET specific scenarios. Any plans to have self-contained builds of that console app?

We can certainly make it a priority to have a self-contained build if removing the dotnet dependency is important. I'll look into it.

Indeed. The dotnet dependency is something we're actively working on moving away from, so we definitely don't want to deepen that dependency if possible. Worse case, we'd need to position Easy Auth as an optional feature that depends on it, but we'd need to make the message here clearer, perhaps also pointing to a fwlink with details.

Looked over the PR and it does seem like a lot of things are still pending/in flux, would you mind adding a comment when you feel this is ready for review?

ahmelsayed

I see you have a couple of TODOs in the code, I also added few more comments. Let me know if you have any questions and ping us again once you're done with your TODOs and ready for a final review. Over all looks like a great addition to the core-tools that people have been asking for for a while now!

ahmelsayed · 2018-10-11T17:01:58Z

src/Azure.Functions.Cli/Actions/AuthActions/BaseAuthAction.cs

+
+namespace Azure.Functions.Cli.Actions.AuthActions
+{
+    abstract class BaseAuthAction : BaseAzureAction


You don't really need this class since it's empty, right? you can just drive from BaseAzureAction if you need the access token, or were you planning on adding some common code here?

src/Azure.Functions.Cli/Actions/AzureActions/CreateAADApplication.cs

ahmelsayed · 2018-10-11T17:13:37Z

src/Azure.Functions.Cli/Actions/HostActions/StartHostAction.cs

+                var process = Process.Start("CMD.exe", $"/C dotnet {dllPath} {query}");
+                var cancellationToken = new CancellationTokenSource();
+                cancellationToken.Token.Register(() => process.Kill());
+                await process.WaitForExitAsync();


I'm a bit unsure about the use of the cancellationToken here. It doesn't look like you really need it. You can also use the Executable class I have defined to help with this. The usage is fairly straight forward. Check here or here for samples

ahmelsayed · 2018-10-11T17:16:21Z

src/Azure.Functions.Cli/Actions/HostActions/StartHostAction.cs

+
+            (var listenUri, var baseUri, var certificate, string certPath, string certPassword) = await Setup();
+
+            int originalPort = Port;


maybe call it hostPort for clarity.

ahmelsayed · 2018-10-11T17:17:48Z

src/Azure.Functions.Cli/Actions/HostActions/StartHostAction.cs

+
+            // Regardless of whether or not auth is enabled, clients should send requests here
+            var originalListenUri = listenUri.SetPort(originalPort);
+            var originalBaseUri = baseUri.SetPort(originalPort);


same thing here, I think it'll be clearer if you called them hostListenUri and authListenUri

ahmelsayed · 2018-10-11T17:24:57Z

src/Azure.Functions.Cli/Common/AuthManager.cs

+            authSettingsToPublish.Add("issuer", "https://sts.windows.net/" + tenant + "/");
+            authSettingsToPublish.Add("runtimeVersion", "1.0.0");
+            authSettingsToPublish.Add("tokenStoreEnabled", "True");
+            authSettingsToPublish.Add("unauthenticatedClientAction", "1"); // Corresponds to AllowAnonymous


You could use the dictionary initialization syntax, it makes it a bit more readable, i.e:

var authSettingsToPublish = new Dictionary<string, string> { { "allowedAudiences", serializedArray }, { "isAadAutoProvisioned", "True" }, ... }

ahmelsayed · 2018-10-11T17:27:40Z

src/Azure.Functions.Cli/Common/AuthManager.cs

+                    .Error
+                    .WriteLine(ErrorColor("Error updating app settings:"))
+                    .WriteLine(ErrorColor(result.ErrorResult));
+                return false;


same thing here regarding the error. If this is an error that you would like to communicate as a cli error, then consider either throwing here, or where you call this like I mentioned in the other comment above

I based this on PublishAppSettings / UpdateFunctionAppAppSettings, which modifies /config/AppSettings (mine modifies /config/authsettings). Is there a reason those do not throw?

ahmelsayed · 2018-10-11T17:29:34Z

src/Azure.Functions.Cli/Common/AuthManager.cs

+            return true;
+        }
+
+        static string ComputeSha256Hash(string rawData)


You can add this method to StringExtensions.cs

ahmelsayed · 2018-10-11T17:30:39Z

src/Azure.Functions.Cli/Common/AuthManager.cs

+            }
+        }
+
+        public static string GeneratePassword(int length)


can you explain a bit what this password is for and what its format is like?

This generates an AAD application client secret. An example of what it generates is: "77S2ca0Gwf8mTr7s85ZEqe7dcxKnF4DdyMAF5gNFaTBY6gBEDtcPwQWlTP3KWNElvld9eFyXZdvFfmEnqmrHQA7ki0$LRQ8TL3uLrj6S$F0PRtRYnZ8Dp9xReKNQK9z"

ahmelsayed · 2018-10-11T17:31:15Z

src/Azure.Functions.Cli/Common/AuthManager.cs

+        }
+    }
+
+    static class AADConstants


move this class to the Constants.cs class as a nested class there.

…st pipeline

fabiocav · 2018-10-12T17:29:19Z

@glennamanns just saw the last commit land. Would you remove the WIP/let us know when this is ready for review? Just want to make sure we don't ignore your changes once this is ready to go :)

cgillum

Some initial feedback. I'm not yet done reviewing, so I'll add more later.

src/Azure.Functions.Cli/Actions/AzureActions/CreateAADApplication.cs

cgillum · 2018-10-12T18:12:27Z

src/Azure.Functions.Cli/Actions/AzureActions/CreateAADApplication.cs

+    {
+        private readonly IAuthManager _authManager;
+
+        public string AADName { get; set; }


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

Same comment for the parameter syntax below.

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?

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

cgillum · 2018-10-12T18:13:32Z

src/Azure.Functions.Cli/Actions/AzureActions/CreateAADApplication.cs

+        {
+            Parser
+                .Setup<string>("aad-name")
+                .WithDescription("Name of AD application to create")


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

cgillum · 2018-10-12T18:15:31Z

src/Azure.Functions.Cli/Actions/AzureActions/CreateAADApplication.cs

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


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

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

cgillum · 2018-10-12T18:20:25Z

src/Azure.Functions.Cli/Common/AuthSettingsFile.cs

+    class AuthSettingsFile
+    {
+        public bool IsEncrypted { get; set; }
+        public Dictionary<string, string> Values { get; set; } = new Dictionary<string, string>();


Make this a case-insensitive dictionary: new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase). That way you don't have to remember to use case-sensitive lookups every time you reference this. #Closed

cgillum · 2018-10-12T18:24:48Z

src/Azure.Functions.Cli/Actions/HostActions/StartHostAction.cs

+            // Determine if middleware (Easy Auth) is enabled
+            var middlewareAuthSettings = _secretsManager.GetMiddlewareAuthSettings();
+            bool authenticationEnabled = middlewareAuthSettings.ContainsKeyCaseInsensitive(Constants.MiddlewareAuthEnabledSetting) &&
+                middlewareAuthSettings[Constants.MiddlewareAuthEnabledSetting].ToLower().Equals("true");


This would be a bit cleaner:

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

cgillum · 2018-10-12T18:28:10Z

src/Azure.Functions.Cli/Actions/HostActions/StartHostAction.cs

-            return (new Uri($"{protocol}://0.0.0.0:{Port}"), new Uri($"{protocol}://localhost:{Port}"), cert);
+            if (UseHttps)
+            {
+                (X509Certificate2 cert, string certPath, string certPassword) = await SecurityHelpers.GetOrCreateCertificate(CertPath, CertPassword);


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

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

get AAD permissions from bindings, cleanup Cleanup and bug fixes cont.

src/Azure.Functions.Cli/Common/AuthSettingsFile.cs

cgillum · 2018-10-24T19:54:33Z

src/Azure.Functions.Cli/Common/AuthSettingsFile.cs

+            }
+            else
+            {
+                return Values.ToDictionary(k => k.Key, v => (string)v.Value);


Is it necessary to cast v.Value to string explicitly? Isn't it already a string? #Closed

cgillum · 2018-10-24T19:55:44Z

src/Azure.Functions.Cli/Common/SecretsManager.cs

+            get
+            {
+                var authFile = "local.middleware.json";
+                var rootPath = ScriptHostHelpers.GetFunctionAppRootDirectory(Environment.CurrentDirectory, new List<string>


Suggested change

var rootPath = ScriptHostHelpers.GetFunctionAppRootDirectory(Environment.CurrentDirectory, new List<string>

var rootPath = ScriptHostHelpers.GetFunctionAppRootDirectory(Environment.CurrentDirectory, new []

``` #Closed

Yes, and they are more immutable (you cannot add or remove elements, making them easier to reason about). Generally speaking you should try to use the most restrictive data types. #Closed

cgillum · 2018-10-24T19:57:34Z

src/Azure.Functions.Cli/Helpers/AzureHelper.cs

@@ -250,6 +250,26 @@ private static async Task ArmHttpAsync(HttpMethod method, Uri uri, string access
            }
        }

+        public static async Task<HttpResult<Dictionary<string, string>, string>> UpdateFunctionAppAuthSettings(Site site, string accessToken)
+        {
+            var url = new Uri($"{ArmUriTemplates.ArmUrl}{site.SiteId}/config/authsettings?api-version={_storageApiVersion}");


Is _storageApiVersion correct? This is an ARM call, not a storage call, so this seems not quite right.

GetStorageAccount, an existing method+ARM call, uses it. Authsettings requires a later API version than appsettings, because I got the following error with the older version: "The current api-version doesn't support RBAC users. Please use a newer version instead"

I think I'll add a second string called _authSettingsApiVersion and hardcode it to 2018-02-01, which is what the storage api version is. That way, it's clear the two are used for separate endpoints and can be changed independently of one another.

cgillum · 2018-10-24T19:59:25Z

src/Azure.Functions.Cli/Helpers/ExtensionsHelper.cs

@@ -52,6 +52,23 @@ private static IEnumerable<string> GetBindings()
            return bindings;
        }

+        public static IEnumerable<(string, BindingDirection)> GetBindingsWithDirection()


Who is using this new API?

The create-aad command. In order to know which permissions to add, I need to know the bindings and their direction

cgillum · 2018-10-24T20:00:22Z

src/Azure.Functions.Cli/Extensions/StringExtensions.cs

@@ -41,5 +43,22 @@ public static string SanitizeImageName(this string imageName)

            return cleanImageName.ToLowerInvariant().Substring(0, Math.Min(cleanImageName.Length, 128)).Trim();
        }
+
+        public static string ComputeSha256Hash(string rawData)


Who uses this new API?

create-aad to generate an encryption and signing key

cgillum · 2018-10-24T21:35:38Z

src/Azure.Functions.Cli/Common/AuthManager.cs

+            {
+                _workerRuntime = workerRuntime;
+
+                List<string> replyUrls;


Do you need this to be a List<string> or can you instead use string[]. string[] array is preferred since it is less overhead and it is a fixed size, making it easier to reason about how it is used throughout the code (i.e. nobody is going to add to it or remove from it).

It's a List in order to get the serialized array (string serializedArray = JsonConvert.SerializeObject(replyUrls, Formatting.Indented)) that we use to update /authsettings in the proper format. The other parts of the code could use string[], but not this one. I can either change all of them to string[] and then convert to List in the publish to /authsettings scenario, or eat the overhead elsewhere.

From a serialization perspective, List<string> and string[] create exactly the same JSON and can be used interchangeably.

cgillum · 2018-10-24T21:36:37Z

src/Azure.Functions.Cli/Common/AuthManager.cs

+        /// <param name="AADName">Name of the new AAD application</param>
+        /// <param name="appName">Name of an existing Azure Application to link to this AAD application</param>
+        /// <returns></returns>
+        private string CreateQuery(string AADName, string appName, out string tempFile, out string clientSecret, out List<string> replyUrls, out string hostName)


Should this be named GetCommandLineArguments? #Closed

src/Azure.Functions.Cli/Common/AuthManager.cs

cgillum · 2018-10-24T21:42:57Z

src/Azure.Functions.Cli/Common/AuthManager.cs

+        private Dictionary<string, string> CreateAuthSettingsToPublish(string clientId, string clientSecret, string tenant, List<string> replyUrls)
+        {
+            // The 'allowedAudiences' setting of /config/authsettings is of the form ["{replyURL1}", "{replyURL2}"]
+            string serializedArray = JsonConvert.SerializeObject(replyUrls, Formatting.Indented);


It's important that variable names give us an idea of what the data actually is and/or what it will be used for. In this case, replyUrlArray or something similar seems more appropriate.

cgillum · 2018-10-24T21:43:56Z

src/Azure.Functions.Cli/Common/AuthManager.cs

+
+            var authSettingsToPublish = new Dictionary<string, string>
+            {
+                { "allowedAudiences", serializedArray },


allowedAudiences should not be the same as the reply URLs. It should just be the base URL - e.g. "https://myapp.azurewebsites.net" or "http://myapp.localhost".

I could be misunderstanding, but I'm not sure that's true. I looked at the allowed_audiences of several function apps set up with Easy Auth (via the portal, not this new command) and they have the /.auth/login/aad/callback in their allowed audiences.

Examples: glennahttps, graphbinding-experiment, graphextension-beta1, FunctionsEasyAuth

cgillum

A few more feedback items.

cgillum · 2018-10-29T21:22:06Z

src/Azure.Functions.Cli/Common/AuthManager.cs

@@ -149,8 +153,8 @@ private string CreateQuery(string AADName, string appName, out string tempFile,

            string serializedReplyUrls = string.Join(" ", replyUrls.ToArray<string>());


nit: I don't think you need the .ToArray<string>() here.

cgillum · 2018-10-29T21:23:09Z

src/Azure.Functions.Cli/Common/AuthManager.cs

+            {
+                _workerRuntime = workerRuntime;
+
+                List<string> replyUrls;


From a serialization perspective, List<string> and string[] create exactly the same JSON and can be used interchangeably.

cgillum · 2018-10-29T22:42:54Z

src/Azure.Functions.Cli/Actions/AzureActions/CreateAADApplication.cs

+namespace Azure.Functions.Cli.Actions.AzureActions
+{
+    // Invoke via `func azure auth create-aad --AADAppRegistrationName {yourAppRegistrationName} --appName {yourAppName}`
+    [Action(Name = "create-aad", Context = Context.Azure, SubContext = Context.Auth, HelpText = "Creates an Azure Active Directory app registration. Can be linked to an Azure App Service or Function app.")]


I suggest "create-aad-app" as the action name since "aad" is not really a thing you can create.

cgillum · 2018-10-29T22:52:17Z

src/Azure.Functions.Cli/Common/AuthManager.cs

+                {
+                    // Connect this AAD application to the Site whose name was supplied (set site's /config/authsettings)
+                    // Tell customer what we're doing, since finding and updating the site can take a number of seconds
+                    ColoredConsole.WriteLine($"\nUpdating auth settings of application {appName}..");


Use Environment.NewLine instead of \n.

cgillum · 2018-10-29T22:53:16Z

src/Azure.Functions.Cli/Common/AuthManager.cs

+            }
+            else
+            {
+                throw new FileNotFoundException("Cannot find az cli. `auth create-aad` requires the Azure CLI.");


If we rename create-aad, don't forget to update it here as well.

cgillum · 2018-10-29T23:02:18Z

src/Azure.Functions.Cli/Common/AuthManager.cs

+
+                if (csProjFile == null)
+                {
+                    throw new CliException($"Auth settings file {SecretsManager.MiddlewareAuthSettingsFileName} could not be added to a .csproj and will not be present in the the bin or output directories.");


C# scripts (.csx) are .NET, but they don't have a .csproj file. Will this exception exclude those users? I wonder if it's safer to just skip that step and write an informational trace if we can't find the .csproj file.

cgillum · 2018-10-29T23:04:45Z

src/Azure.Functions.Cli/Common/AuthManager.cs

+            {
+                // If this is a dotnet function, we also need to add this file to the .csproj 
+                // so that it copies to \bin\debug\netstandard2.x when the Function builds
+                string csProjFile = GetCSProjFilePath();


What about .fsproj for F# developers?

cgillum · 2018-10-29T23:11:26Z

src/Azure.Functions.Cli/Common/AuthManager.cs

+            // Create a local auth .json file that will be used by the middleware
+            var middlewareAuthSettingsFile = SecretsManager.MiddlewareAuthSettingsFileName;
+            MiddlewareAuthSettings = new AuthSettingsFile(middlewareAuthSettingsFile);
+            MiddlewareAuthSettings.SetAuthSetting("WEBSITE_AUTH_ALLOWED_AUDIENCES", serializedReplyUrls);


GitHub isn't letting me reply to our existing comment thread on this, but the portal behavior you observed is wrong, and a bug which can cause CRIs. The allowed_audiences should NOT contain /.auth/login/aad/callback.

cgillum · 2018-10-29T23:12:48Z

src/Azure.Functions.Cli/Common/AuthManager.cs

+
+            var authSettingsToPublish = new Dictionary<string, string>
+            {
+                { "allowedAudiences", serializedReplyUrls },


(Duplicating my comment from elsewhere)

GitHub isn't letting me reply to our existing comment thread on this, but the portal behavior you observed is wrong, and a bug which can cause CRIs. The allowed_audiences should NOT contain /.auth/login/aad/callback.

cgillum · 2018-10-29T23:16:03Z

src/Azure.Functions.Cli/Extensions/StringExtensions.cs

@@ -41,5 +43,22 @@ public static string SanitizeImageName(this string imageName)

            return cleanImageName.ToLowerInvariant().Substring(0, Math.Min(cleanImageName.Length, 128)).Trim();
        }
+
+        public static string ComputeSha256Hash(string rawData)


nit: Since this is a class intended to create extension methods, it's better to be consistent and make this method also an extension method:

Suggested change

public static string ComputeSha256Hash(string rawData)

public static string ComputeSha256Hash(this string rawData)

glennamanns · 2019-01-23T18:04:36Z

src/Azure.Functions.Cli/Actions/HostActions/StartHostAction.cs

-                : null;
-            return (new Uri($"{protocol}://0.0.0.0:{Port}"), new Uri($"{protocol}://localhost:{Port}"), cert);
+            if (UseHttps)
+            {


todo: revert this to the original logic

jabbera · 2019-09-05T11:19:29Z

Any chance of this making it any time soon? This breaks so many things. (Such as not being able to debug authenticated serverless Azure Functions locally at all)

ahmelsayed · 2019-10-15T23:24:51Z

Closing old PR. Please reopen if this still needs to be looked at

lock · 2019-12-18T05:32:43Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

create testing/prod aad, publish easy auth settings, run easy auth lo…

a44ff28

…cally

glennamanns added the feature label Oct 10, 2018

glennamanns requested a review from ahmelsayed October 10, 2018 17:24

ahmelsayed reviewed Oct 10, 2018

View reviewed changes

glennamanns mentioned this pull request Oct 10, 2018

Add Easy Auth local settings file to git ignore, project template Azure/azure-functions-templates#770

Closed

glennamanns requested review from ahmelsayed and cgillum October 10, 2018 21:08

ahmelsayed reviewed Oct 11, 2018

View reviewed changes

glennamanns added 2 commits October 11, 2018 15:30

Remove dotnet dependency by injecting Easy Auth middleware into reque…

c30f947

…st pipeline

Cleanup based on PR comments

c0b2b08

cgillum requested changes Oct 12, 2018

View reviewed changes

Enable local testing of Token binding

67e1bd4

get AAD permissions from bindings, cleanup Cleanup and bug fixes cont.

cgillum reviewed Oct 24, 2018

View reviewed changes

src/Azure.Functions.Cli/Common/AuthSettingsFile.cs Show resolved Hide resolved

cgillum reviewed Oct 24, 2018

View reviewed changes

Update based on Chris' CR feedback

c3813e2

cgillum reviewed Oct 29, 2018

View reviewed changes

Allow for fsharp projects, remove allowed audiences

c802d4b

glennamanns changed the title ~~WIP: Enable Easy Auth Local Dev~~ Enable Easy Auth Local Dev Nov 13, 2018

ConnorMcMahon mentioned this pull request Jan 9, 2019

Support EasyAuth Azure/azure-functions-host#33

Closed

ahmelsayed changed the base branch from master to dev January 23, 2019 01:15

glennamanns commented Jan 23, 2019

View reviewed changes

ahmelsayed closed this Oct 15, 2019

lock bot locked as resolved and limited conversation to collaborators Dec 18, 2019


		(var listenUri, var baseUri, var certificate, string certPath, string certPassword) = await Setup();

		int originalPort = Port;

	var rootPath = ScriptHostHelpers.GetFunctionAppRootDirectory(Environment.CurrentDirectory, new List<string>
	var rootPath = ScriptHostHelpers.GetFunctionAppRootDirectory(Environment.CurrentDirectory, new []
	``` #Closed

		@@ -149,8 +153,8 @@ private string CreateQuery(string AADName, string appName, out string tempFile,

		string serializedReplyUrls = string.Join(" ", replyUrls.ToArray<string>());

	public static string ComputeSha256Hash(string rawData)
	public static string ComputeSha256Hash(this string rawData)

Enable Easy Auth Local Dev #789

Enable Easy Auth Local Dev #789

Conversation

glennamanns commented Oct 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ahmelsayed left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabiocav commented Oct 12, 2018

cgillum left a comment

Choose a reason for hiding this comment

cgillum Oct 12, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cgillum Oct 24, 2018 • edited Loading

Choose a reason for hiding this comment

cgillum Oct 12, 2018 • edited Loading

Choose a reason for hiding this comment

cgillum Oct 12, 2018 • edited Loading

Choose a reason for hiding this comment

cgillum Oct 12, 2018 • edited Loading

Choose a reason for hiding this comment

cgillum Oct 12, 2018 • edited Loading

Choose a reason for hiding this comment

cgillum Oct 12, 2018 • edited Loading

Choose a reason for hiding this comment

cgillum Oct 12, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cgillum Oct 24, 2018 • edited Loading

Choose a reason for hiding this comment

cgillum Oct 24, 2018 • edited Loading

Choose a reason for hiding this comment

cgillum Oct 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glennamanns Oct 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glennamanns Oct 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cgillum Oct 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cgillum left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jabbera commented Sep 5, 2019

ahmelsayed commented Oct 15, 2019

lock bot commented Dec 18, 2019

glennamanns commented Oct 10, 2018 •

edited

Loading

cgillum Oct 12, 2018 •

edited

Loading

cgillum Oct 24, 2018 •

edited

Loading

cgillum Oct 12, 2018 •

edited

Loading

cgillum Oct 12, 2018 •

edited

Loading

cgillum Oct 12, 2018 •

edited

Loading

cgillum Oct 12, 2018 •

edited

Loading

cgillum Oct 12, 2018 •

edited

Loading

cgillum Oct 12, 2018 •

edited

Loading

cgillum Oct 24, 2018 •

edited

Loading

cgillum Oct 24, 2018 •

edited

Loading

cgillum Oct 24, 2018 •

edited

Loading

glennamanns Oct 24, 2018 •

edited

Loading

glennamanns Oct 24, 2018 •

edited

Loading

cgillum Oct 24, 2018 •

edited

Loading