Add test WebListener module and tests for Web Cmdlet Certificate Authentication #4622

markekraus · 2017-08-20T10:45:24Z

Closes #4609 and replaces tests for #4546.

This test uses an ASP.NET Core 2.0 app to run a simple service to echo back the client certificate details if one was provided. If a client cert is not provided, the details are empty and the status reports FAILED. The app runs in a Job.

msftclas · 2017-08-20T10:45:29Z

@markekraus,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by Microsoft. We will now review your pull request.
Thanks,
Microsoft Pull Request Bot

iSazonov · 2017-08-20T11:24:52Z

@markekraus See as we prepare TestExe.csproj

PowerShell/build.psm1

Line 780 in b3e7fd3

@{Path="${PSScriptRoot}/test/tools/TestExe";Output="testexe"}

markekraus · 2017-08-20T11:58:22Z

@iSazonov Thanks, I'll see how I can fit this one in into Publish-PSTestTools

on another note, the mdspell is falsely calling out ASP.NET in my README.md. Is there place to add that to be ignored? Or is there another way to handle it?

edit: I assume i can modify .spelling to address that.

iSazonov · 2017-08-20T14:25:39Z

Yes, it is .spelling - add Asp.Net (case-sensitive).

iSazonov · 2017-08-20T14:39:55Z

@markekraus Can we replace our ListenerHttp script with the application (not in the PR)?

markekraus · 2017-08-20T14:52:38Z

@iSazonov In theory we could move all of the web tests to this (with some slight modifications, such as adding a Non-HTTPS listener to it), but, it would mean rewriting the existing features of HttpListener into ASP.NET. Or doing something that allowed PowerShell to manage the business logic.

I was already considering switching the Validate Invoke-WebRequest -SkipCertificateCheck tests over to this new listener since the ServerCert fails as a self-signed cert.

iSazonov · 2017-08-20T16:27:45Z

@markekraus Please keep the PR as small as possibly - it is better to make cleanups later.

If we can move to the new application from HttpListener we should ask the test area experts to this /cc @adityapatwardhan - if it will be approved we'll review the code with the point in mind.

markekraus · 2017-08-20T16:38:48Z

@iSazonov Should I roll back the baddssl changes? Aside from those, the rest of the changes on this PR are interdependent.

Can you restart the failed Travis CI test? it failed an unrelated test due to httpbin not being unavailable.

iSazonov · 2017-08-20T16:49:22Z

@markekraus Main rule is to keep PR as small as possible and don't add unneeded changes.
So I believe, yes, we should rollback - only keep the commit for follow PR. Also you can wait conclusion from @adityapatwardhan I asked to lead this change as the test area expert.
Main question now to continue the review is - do we want move our web tests from HttpListener in future or not.

iSazonov · 2017-08-20T17:11:27Z

test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1

+{
+    param([int]$Port)
+    $initTimeoutSeconds  = 15
+    $projectRootPath     = Resolve-Path (Join-Path $PSScriptRoot "\..\..\..\..\")


function Publish-PSTestTools already added the path into PATH environment variable - so we don't need build the full path.

@iSazonov That is used to pass to the Job scope where the path changes made by the Publish-PSTestTools are not available.

not available? I wonder. Do you check? Usually env variables is inherited in sub process.

@iSazonov it turns out you were right. I was able to adjust this and pull the parent path from the command.

iSazonov · 2017-08-20T17:11:54Z

test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1

@@ -296,6 +296,46 @@ function ExecuteWebRequest
    return $result
 }

+# Starts the ClientCertificateCheck listener
+function StartClientCertificateCheck 


Maybe Start-ClientCertificateCheck

@iSazonov I tried to match the other private functions in the test already. I originally wanted to use Start-ClientCertificateCheck, but thought I would get a note for not matching the others. I can't win :D

My thoughts was about HttpListener style. Maybe somebody else comment this too.

If we move the function in HelpersCommon.psm1 it is ok rename it to Start-ClientCertificateCheck.

iSazonov · 2017-08-20T17:14:50Z

test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1

+    $initCompleteMessage = 'Now listening on'
+
+    $timeOut = (get-date).AddSeconds($initTimeoutSeconds)
+    $Job = Start-Job {


If we'll use the application in multiple test files we should save time - run it once and check if it already is running.

@iSazonov I'm open to suggestions on how to implement that. As it stands now, it starts in less than a second. I was just following the current paradigm for how Start-HttpListener is used in the tests. Currently, StartClientCertificateCheck is called once at the beginning of the Invoke-WebReqeust and once at the begining of the Invoke-RestMethod tests. it is stopped at the end of the tests for those describe blocks.

CI VMs is very-very slow. If locally you see no start delay then on CI it is possible the delay will be large. That's why I think it's best to use one tool (remove HttpListener) and run it only once.

how to implement that

Call Invoke-WebRequest and analyze the response.

@iSazonov As for the less than 1 second start time, I was referring to the start time within the CI VMs. The majority of the overhead is in the build stage, which has now been offloaded to Publih-PSTestTools. I just have the wait in there to prevent a race condition between the job delay and the first test that requires the resource.

I think the current method for ensuring the process is running is fine. It makes sense to use Invoke-WebRequest for Start-HttpListener as there is no other means for ensuring the server is live. But with this I have access to the console output from the server itself saying it is live.

What I was asking about was how to implement it to be running for multiple tests. What mechanism would start it? Start-PSPester?

We want run tests without Start-PSPester as .\file.test.ps1.
I think we should move the code (1-function to start the app and 2-function check that it is running) in HelpersCommon - in first file we detect that the app is not running and start it, in second file we detect that the app is already running and use it.

@iSazonov Ah. I see. Since it may be decided to move all tests to this, would it make sense to create a module of its own, similar to HttpLitsener? If we want to have logic detecting if it is running or not and with possibility of running multiple of these on different ports, it might warrant a class definition to contain the port and job for a specific listener and a module variable to track started listeners. It might also warrant a rename of the app to something more generic than ClientCertificateCheck

For this PR, to keep it small, Moving the start and stop functions to HelpersCommon and renaming them proper Verb-Noun would be a start.

We could convert the app to binary module (and rename it) and get benefits from autoloading. It may require more effort, but it seems worth.

I'm not sure how that would work. This is an executable binary. I've never seen that kind of thing included in a binary module. I'm not saying it's impossible, just that it is beyond me how to do that.

Scripts can still autoload. I think the optimization of converting it to an assembly module can be filed as an issue and let @markekraus prove the concept. Unless someone has time to work with him now on writing an assembly based module.

@TravisEz13 and @iSazonov Ok, since it appears we may head in the direction of using this for more tests than just the certificate authentication, I have renamed the app HttpsListener and added an HttpsListener module to house the Start-, Stop, and Get- commands. There is a module script variable to track the running listeners so if it is already listening on the requested port the running listener is returned. To keep it always running, I have removed the stop commands form the AfterAll blocks. I think that incorporates everything discussed here except making it a binary module.

iSazonov · 2017-08-20T17:16:35Z

test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1

+}
+
+# Stops the ClientCertificateCheck listener
+function StopClientCertificateCheck 


Maybe Stop-ClientCertificateCheck.
If we'll use the application in multiple test files we should it once and shouldn't stop it.

@iSazonov same as with Start-ClientCertificateCheck

iSazonov · 2017-08-20T17:23:54Z

test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1

@@ -1183,6 +1200,26 @@ Describe "Invoke-WebRequest tests" -Tags "Feature" {

    #endregion charset encoding tests

+    #region Client Certificate Authentication


It is better use Context {}. I suggest don't mix HTTP and HTTPS tests and put HTTPS test in separate Context {} while we use different tools for them.

@iSazonov I think we should maybe do that as a separate PR to clean up all of the current HTTPS tests too. Or, I could build the contexts now and move the other HTTPS into them as we update the tests (i.e. tests referencing badssl.com).

You have to edit all HTTPS tests so I believe it is ok to add Context in the PR.
But if you want I agree to follow the file's pattern now and make cleanups later.

I'll do a separate followup PR to this one under the guise of HTTPS test adjustments. moving all the tests (including those not touched in this PR) that depend on HTTPS to a single context in each cmdlet section and switching the URL to the new app... assuming this PR get's approved and that is the way we want to go with this.

iSazonov · 2017-08-20T17:41:20Z

test/tools/ClientCertificateCheck/Pages/Index.cshtml

+Issuer: @Model.CertIssuer
+Issuer Name: @Model.CertIssuerName
+Not After: @Model.CertNotAfter
+Not Before: @Model.CertNotBefore


GitHub mark absent new line at EOF as error and suggest GitHub community to fix it. We already fixed most of our files in the repo. So please add new line at EOF in all files you change until it is really need empty file.

@iSazonov will fix

iSazonov · 2017-08-20T17:41:38Z

test/tools/ClientCertificateCheck/Pages/_Layout.cshtml

@@ -0,0 +1 @@
+@RenderBody()


@iSazonov will fix

iSazonov · 2017-08-20T17:41:52Z

test/tools/ClientCertificateCheck/Pages/_ValidationScriptsPartial.cshtml

@@ -0,0 +1 @@
+


@iSazonov it lookslike I don't need this file. i will remove it.

iSazonov · 2017-08-20T17:43:02Z

test/tools/ClientCertificateCheck/README.md

+
+The `ClientCertificateCheck.dll` takes 3 arguments: the path to the Server Certificate, the Server Certificate Password, and the TCP Port to bind on.
+
+# Run As a Docker Container


Is it actual?

I'm not sure what you asking here.

Sorry. If it is not supported on Mac it make sense to remove.

@iSazonov removed

iSazonov · 2017-08-20T17:44:19Z

test/tools/ClientCertificateCheck/dockerfile

@@ -0,0 +1,18 @@
+# Stage 1: Build the Application


Do we still need the file?

@iSazonov If we don't want to try this as a docker container, then no. I can remove the docker parts.

If it is not supported on Mac it make sense to remove.

@iSazonov I will remove it and .dockerignore as well as the docker info in the readme.

markekraus · 2017-08-20T21:34:40Z

Hmm... The macOS Build is "passing" but in actuality it is hitting an unhandled exception and exiting without property saying it has failed.

https://travis-ci.org/PowerShell/PowerShell/jobs/266586350#L8280

Edit:

It looks like macOS has a service that binds to port 8443. I will try switching to port 8083.

The bigger mystery to me is how this managed to terminate all tests without pester complaining about it and how this managed to bubble up from a Job process into the parent process and then up to pester.

Edit 2: switching the port to 8083 fixed the issue in the macOS VM.

iSazonov · 2017-08-21T04:27:35Z

test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1

-        throw 'Container did not start before the timeout was reached.'
+        $Job | Stop-Job -PassThru | Receive-Job
+        $Job | Remove-Job
+        throw 'Job did not start before the timeout was reached.'


We should make the message more clear - maybe "ClientCertificateCheck is not started."

Since we're still throw maybe move the throw into do-while and make the cycle unlimited:

do Start-Sleep -Milliseconds 100 $containerStatus = $Job.ChildJobs[0].Output | Out-String if ($containerStatus -match $initCompleteMessage) { # Application started. break; } if (get-date) -gt $timeOut) { throw "ClientCertificateCheck is not started." } while ($true)

@iSazonov I will update the throw message.

On the other suggestion, I'm not sure it makes sense to introduce flow logic inside a do block when the while is intended to manage the do's exit. I have never been a fan of while ($true) Is there something wrong with the current logic?

No, only my sample looks more clear for me 😄 because we replace two code blocks with single logical block.

😆 Ok so it's just a difference of style. I think of it as 2 separate actions, 1) waiting for initialization with timeout and 2) cleanup and error reporting.

iSazonov · 2017-08-21T04:29:09Z

test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1

    }
    # Wait until the app is running or until the initTimeoutSeconds have been reached
    do
    {
-        Start-Sleep -Seconds 1
+        Start-Sleep -Milliseconds 100
        $containerStatus = $Job.ChildJobs[0].Output | Out-String


We don't use container - please rename the variable.

@iSazonov I will change it to $initStatus

iSazonov · 2017-08-21T04:32:29Z

test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1

@@ -296,6 +296,46 @@ function ExecuteWebRequest
    return $result
 }

+# Starts the ClientCertificateCheck listener
+function StartClientCertificateCheck 


If we move the function in HelpersCommon.psm1 it is ok rename it to Start-ClientCertificateCheck.

GavinEke · 2017-08-21T06:57:41Z

test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1

    }

-    #endregion Certificate Authentication Tests
+    #edregion Client Certificate Authentication


@markekraus slight typo on endregion

@GavinEke Thanks. I will correct that.

markekraus · 2017-08-21T09:47:54Z

It looks like the AppVeyor build beta.5-4786 failed due to temporary issue with powershell.myget.org. if someone could restart the build, that would be helpful.

iSazonov · 2017-08-21T10:36:42Z

@markekraus It seems I haven't permissions. So you can "rename" last commit and push - it will be restart CI jobs.

markekraus · 2017-08-21T12:28:01Z

@iSazonov Now It looks like https://dotnetcli.azureedge.net/dotnet/Sdk/2.0.0/dotnet-dev-osx-x64.2.0.0.tar.gz has disappeared.

edit: rerunning the CI was successful, but Travis CI has not updated the status in the PR due to some issues.

SteveL-MSFT · 2017-08-21T16:00:23Z

@iSazonov I'm going to assign this to you as you've been involved while @adityapatwardhan is taking time off

iSazonov · 2017-08-21T16:49:00Z

@SteveL-MSFT We need get approve from test area experts because of significant changes in the PR - please ping anybody if @adityapatwardhan is busy.

iSazonov · 2017-08-24T15:44:31Z

test/tools/Modules/WebListener/WebListener.psd1

+    FunctionsToExport = @(
+        'Start-WebListener'
+        'Stop-WebListener'
+        'Get-WebListener'


'G' after 'S' ? 😄

They are "S" class cmdlets... higher priority 😆. I will fix it.

iSazonov · 2017-08-24T15:56:55Z

test/tools/Modules/WebListener/WebListener.psm1

+    [OutputType([System.Security.Cryptography.X509Certificates.X509Certificate2])]
+    param ()
+    process {
+        $parentPath = Split-Path -parent (get-command WebListener).Path


I meant we can $MyInvocation.MyCommand.Module.ModuleBase

ClientCert.pfx is in the WebListener published directory. If I switch to $MyInvocation.MyCommand.Module.ModuleBase I would need to move it to the WebListener module folder. Is that what you would suggest?

Looks good to me.

@iSazonov for clarity, are you asking me to move it and update the code or leave it as is?

I agree to move ClientCert.pfx to module folder.

* Enum was not used * GetStatus() was not accessing the correct property chain * Added -Test param to make URL generation smoother in test code and to fix double / issues

* Https when it matters. * Expand property... not exclude.. * Remove superfluous and outdated ToString() override

* Move the cert * Adjust Get-WebListenerClientCertificate * Remove cert from csproj * ActionResult -> JsonResult (was mistakenly left as ActionResult during testing)..

iSazonov · 2017-08-25T12:14:00Z

test/tools/Modules/WebListener/WebListener.psm1

+    {
+        return $This.Job.JobStateInfo.State
+    }
+


Extra line.

iSazonov · 2017-08-25T12:16:52Z

test/tools/WebListener/Controllers/CertController.cs

+            Hashtable output = new Hashtable 
+            {
+                {"Status", "FAILED"}
+            };


I still see ';'.

iSazonov · 2017-08-25T12:16:58Z

test/tools/WebListener/Controllers/CertController.cs

+                    {"IssuerName"  , HttpContext.Connection.ClientCertificate.IssuerName.Name},
+                    {"NotAfter"    , HttpContext.Connection.ClientCertificate.NotAfter},
+                    {"NotBefore"   , HttpContext.Connection.ClientCertificate.NotBefore}
+                };


I still see ';'.

The Semicolon has to be there. When I tried to remove it I get compile errors saying it is missing.

Sorry. Closed.

iSazonov · 2017-08-25T12:22:05Z

test/tools/WebListener/WebListener.csproj

+  </ItemGroup>
+
+  <ItemGroup>
+     <None Include="ServerCert.pfx" CopyToPublishDirectory="PreserveNewest" />


Maybe put it near client certificate and remove the group?

iSazonov · 2017-08-25T13:26:19Z

@markekraus Thank you for your patience and your speedy work.

LGTM.

markekraus · 2017-08-25T13:43:03Z

@iSazonov Thanks for being patient with me too and for all your suggestions!

* Move cert * Upate csproj * Update module * Add/Update README.md's CI Retest.

SteveL-MSFT · 2017-08-25T16:28:07Z

@iSazonov can you formally approve?

iSazonov · 2017-08-25T16:47:52Z

It will be good to get a review from test area experts.

iSazonov · 2017-08-25T17:37:30Z

Thanks! I'll merge on next week.

iSazonov · 2017-08-31T10:02:53Z

@markekraus Many thanks for great work!

The PR was merged and way is opened for follow work:

- Cleanup HTTPS tests, replace #region with "Context".
- Cleanup HTTP test to use WebListener.

Please move the points to a tracking Issue.

markekraus · 2017-08-31T10:10:57Z

@iSazonov I have opened #4179 to track the HTTPS tests.

Should I continue the work for the second item under the already open #2504 or should I create a new issue?

iSazonov · 2017-08-31T14:57:19Z

@markekraus #2504 is good. Please leave a message about WebListener there for cross reference. And if you want work on this you should ask Travis there.

markekraus requested review from adityapatwardhan, daxian-dbw and TravisEz13 as code owners August 20, 2017 10:45

msftclas added the cla-already-signed label Aug 20, 2017

iSazonov assigned adityapatwardhan Aug 20, 2017

markekraus force-pushed the ClientCertAuthTest branch from 39d21a4 to aea9177 Compare August 20, 2017 16:57

iSazonov reviewed Aug 20, 2017

View reviewed changes

markekraus force-pushed the ClientCertAuthTest branch from ce4296a to 9796ce2 Compare August 20, 2017 19:32

iSazonov reviewed Aug 21, 2017

View reviewed changes

GavinEke reviewed Aug 21, 2017

View reviewed changes

markekraus mentioned this pull request Aug 21, 2017

Create Native Test Solution for Certificate Authentication #4609

Closed

markekraus force-pushed the ClientCertAuthTest branch from 1d09ecc to 1cbab33 Compare August 21, 2017 11:13

markekraus force-pushed the ClientCertAuthTest branch from 1cbab33 to 1dc95f3 Compare August 21, 2017 12:34

SteveL-MSFT assigned iSazonov and unassigned adityapatwardhan Aug 21, 2017

iSazonov reviewed Aug 24, 2017

View reviewed changes

[feature] Address PR Feedback

1a2fd9b

iSazonov reviewed Aug 24, 2017

View reviewed changes

[feature] Cleanup and minor fix

d9770ba

* Enum was not used * GetStatus() was not accessing the correct property chain * Added -Test param to make URL generation smoother in test code and to fix double / issues

markekraus force-pushed the ClientCertAuthTest branch 2 times, most recently from e807b5b to 89cabf5 Compare August 24, 2017 19:35

[feature] More minor fixes

2c3cbd3

* Https when it matters. * Expand property... not exclude.. * Remove superfluous and outdated ToString() override

markekraus force-pushed the ClientCertAuthTest branch from 89cabf5 to 2c3cbd3 Compare August 24, 2017 20:41

[Feature] Move ClientCeret.pfx to WebListener Module

ced727c

* Move the cert * Adjust Get-WebListenerClientCertificate * Remove cert from csproj * ActionResult -> JsonResult (was mistakenly left as ActionResult during testing)..

markekraus force-pushed the ClientCertAuthTest branch from 5a0ff37 to ced727c Compare August 25, 2017 11:59

iSazonov reviewed Aug 25, 2017

View reviewed changes

[Feature] Move ServerCert.pfx to Module

b93674e

* Move cert * Upate csproj * Update module * Add/Update README.md's CI Retest.

markekraus force-pushed the ClientCertAuthTest branch from 4664b1c to b93674e Compare August 25, 2017 15:06

iSazonov approved these changes Aug 25, 2017

View reviewed changes

TravisEz13 approved these changes Aug 25, 2017

View reviewed changes

iSazonov changed the title ~~[Feature] Add Tests for Web Cmdlet Certificate Authentication~~ Add test WebListener module and tests for Web Cmdlet Certificate Authentication Aug 31, 2017

iSazonov merged commit 3b2d169 into PowerShell:master Aug 31, 2017

markekraus mentioned this pull request Aug 31, 2017

Move WebCmdlet Tests relying on HTTPS to WebListener Module #4719

Closed

daxian-dbw mentioned this pull request Aug 31, 2017

Add explicit ContentType detection to Invoke-RestMethod #4692

Merged

markekraus deleted the ClientCertAuthTest branch September 22, 2017 20:36

		@@ -1183,6 +1200,26 @@ Describe "Invoke-WebRequest tests" -Tags "Feature" {

		#endregion charset encoding tests

		#region Client Certificate Authentication


		The `ClientCertificateCheck.dll` takes 3 arguments: the path to the Server Certificate, the Server Certificate Password, and the TCP Port to bind on.

		# Run As a Docker Container

		@@ -0,0 +1 @@
		@RenderBody()

Add test WebListener module and tests for Web Cmdlet Certificate Authentication #4622

Add test WebListener module and tests for Web Cmdlet Certificate Authentication #4622

Conversation

markekraus commented Aug 20, 2017 • edited

msftclas commented Aug 20, 2017

iSazonov commented Aug 20, 2017

markekraus commented Aug 20, 2017 • edited

iSazonov commented Aug 20, 2017

iSazonov commented Aug 20, 2017

markekraus commented Aug 20, 2017 • edited

iSazonov commented Aug 20, 2017

markekraus commented Aug 20, 2017

iSazonov commented Aug 20, 2017 • edited

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

markekraus Aug 20, 2017 • edited

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

markekraus Aug 20, 2017 • edited

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markekraus commented Aug 20, 2017 • edited

Choose a reason for hiding this comment

markekraus Aug 21, 2017 • edited

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

markekraus Aug 24, 2017 • edited

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

markekraus commented Aug 21, 2017

iSazonov commented Aug 21, 2017

markekraus commented Aug 21, 2017 • edited

SteveL-MSFT commented Aug 21, 2017

iSazonov commented Aug 21, 2017

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

markekraus commented Aug 20, 2017 •

edited

markekraus commented Aug 20, 2017 •

edited

markekraus commented Aug 20, 2017 •

edited

iSazonov commented Aug 20, 2017 •

edited

markekraus Aug 20, 2017 •

edited

markekraus Aug 20, 2017 •

edited

markekraus commented Aug 20, 2017 •

edited

markekraus Aug 21, 2017 •

edited

markekraus Aug 24, 2017 •

edited

markekraus commented Aug 21, 2017 •

edited