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

Integrate HttpClientFactory further and remove HPKP #186

merged 50 commits into from Jun 3, 2018


2 participants
Copy link

martincostello commented Jun 3, 2018

What started as a small PR kind of snowballed into a much larger thing, but overall for the better I hope.

  • Integrate HttpClientFactory with the external authentication providers.
  • Add browser integration tests for external authentication.
  • Remove HPKP to resolve #185.
  • Improve HTTP request's User-Agent to include the Git SHA.
  • Set default HttpClient timeout to 20 seconds.
  • Add basic wait-and-retry support using Polly to HTTP GET requests through HttpClientFactory.
  • Prefer display names that aren't the user Id or email address.
  • Add Source Link to the build to embed the Git SHA into the assembly.
  • Fix incorrect launch configuration for Visual Studio Code.
  • Rename correlation cookies when negotiating with external authentication providers.
  • Remove Autofac.
  • Remove obsolete Serilog literate sink.
  • Update Refit to the latest NuGet package version.
  • Update Application Insights and Azure Storage NuGet packages.
  • Fix analyzer NuGet packages not being marked as private assets.
  • Always mark cookies as secure, even during development.
  • Fix intermittent JavaScript error in puppeteer tests.
  • Fix NullReferenceException when parsing Application Insights telemetry.
  • Fix typo in XML documentation.
  • Lots of refactoring to move code around.

martincostello added some commits Jun 2, 2018

Remove Autofac
Remove Autofac as its capabilities aren't really used.
Remove obsolete Serilog sink
Remove the deprecated Serilog.Sinks.Literate package and replace it with Serilog.Sinks.Console.
Update Refit
Update Refit to 4.5.6.
Upgrade Application Insights and Azure Storage
Upgrade the Application Insights Serilog sink and the Azure Storage NuGet packages.
Add Source Link
Add Source Link to embed the Git SHA into the assembly informational version.
Fix NuGet packages without PrivateAssets set to All.
Refactor HttpClient configuration
Configure default HttpClient instance.
Fix incorrect type being used for User Agent version.
Refactor User Agent to include the Git SHA.
Move HttpClient extension methods to their own class.
Set default HttpClient timeout to 20s instead of 100s.
Fix VS Code configuration
Fix the launch configuration for VS Code broken by #178.
Refactor HttpClientFactory integration
Refactor the HttpClientFactory integration to separate the service collection from the HttpClient builders.
Support automatic decompression on HttpClient instances.
Add basic Polly retries
Add a basic Polly wait-and-retry backoff policy to HTTP GET requests.
Extend HTTP timeout if debugging
Extend HTTP timeout if debugging (e.g. stepping through policies and/or message handlers).
Use policy registry
Use policy registry for HTTP requests rather than create per-request.
Move Swagger extensions to own class
Move extension method to add Swagger to its own class.
Use HttpClientFactory with auth providers
Use HttpClientFactory to configure HttpClient instances for use with external authentication providers (Google etc.).
Move identity extensions to own class
Move the extension methods for adding identity to the service collection to their own class.
Define cookie names in a specific class
Define all cookie names in a specific class, rather than literal strings here and there.
Refactor authentication setup
Refactor authentication setup from Startup to extension method.
Configure all cookies to be secure always.
Refactor authentication provider setup
Refactor the setup of external authentication providers.
Refactor integration test fixtures
Refactor the integration test fixtures to support browser tests with a self-hosted server.
Use default Kestrel port
Use the default Kestrel port for the browser tests instead of whatever port happens to be free.
Fix occasional JS error in puppeteer tests
Fix occasional JS error in puppeteer tests by checking that SwaggerUIBundle exists in the window before using it onload.
Add very simple browser test
Add a very simple browser test using Chrome.
Restore free port usage
Restore usage of the first free port for the browser tests to prevent binding errors when the application is being debugged at the same time as running tests.
Fix NullReferenceException
Fix NullReferenceException that seems to happen when a Selenium request for a session is made that Application Insights tries to collect telemetry for.
Enable all auth providers in tests
Enable all of the external authentication providers in the tests.
Add data attributes for tests
Add data attributes to some HTML elements to use for tests.
Prefer names that aren't emails or Ids
Ignore Name claims that have the same value as the user's email address or user Id.
Workaround issue with ApplicationInsights
Work around ApplicationInsights throwing NullReferenceExceptions during test execution with HTTP requests if HttpServerFixture and TestServerFixture are used at the same time.
Add hooks for authentication integration tests
Add event hooks for integration tests for authentication that allow the tests to redirect requests to Google etc. as part of OAuth flow back into the application.
Add builders for HTTP requests
Add builders to use for intercepting HTTP requests in integration tests.
Configure HTTP request interception
Configure the use of HTTP request interception for integration tests.

martincostello added some commits Jun 3, 2018

Fix incorrect XML documentation
Add missing content to see tag.
Add in-memory document store
Add an in-memory document store to use for integration tests as it is not possible to intercept HTTP requests made by the Cosmos SDK.
Use in-memory document store for integration tests
Use the in-memory document store for the integration tests so that the authentication tests can be run without needing the Cosmos DB emulator to be running.
Refactor to use page objects
Refactor the UI tests to use page objects to make the tests easier to follow and compose.
Remove HPKP
Remove all references to HTTP Public Key Pins to resolve #185.
Simplify testsettings.json and remove redundant content.
Install Chrome in Travis CI
Install Chrome for Travis CI builds.
Run browser tests in CI
Remove restriction on running browser tests in CI builds.
Bundle development certificate
Bundle the development certificate with the tests so that the self-hosting works in AppVeyor and Travis CI.

@martincostello martincostello force-pushed the Hook-In-HttpClientFactory-More branch from eb20e48 to eec4560 Jun 3, 2018

martincostello added some commits Jun 3, 2018

Install latest version of Chrome
Install the latest version of Chrome using apt, which hopefully will install a newer version that the chrome add-on.
Install Chrome in AppVeyor
Use Chocolatey to install latest Chrome in AppVeyor.
Enable full web driver logging
Enable full Selenium driver logging to try and diagnose issues.
Fix duplicate apt package lists
Fix there being two apt package lists for Travis CI.
Add non-standard source for the apt Google Chrome package.

This comment has been minimized.

Copy link

codecov-io commented Jun 3, 2018

Codecov Report

Merging #186 into master will increase coverage by 6.6%.
The diff coverage is 92.28%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #186     +/-   ##
+ Coverage    58.9%   65.51%   +6.6%     
  Files          68       74      +6     
  Lines        3105     3204     +99     
  Branches      403      403             
+ Hits         1829     2099    +270     
+ Misses       1088      907    -181     
- Partials      188      198     +10
Impacted Files Coverage Δ
...el.Site/Extensions/IServiceCollectionExtensions.cs 70% <ø> (-8.05%) ⬇️
...vel.Site/Middleware/CustomHttpHeadersMiddleware.cs 85.18% <ø> (+6.23%) ⬆️
...vel.Site/Telemetry/ApplicationInsightsUrlFilter.cs 79.16% <0%> (-2.09%) ⬇️
src/LondonTravel.Site/Program.cs 52.63% <100%> (-2.37%) ⬇️
...ite/Extensions/PollyServiceCollectionExtensions.cs 100% <100%> (ø)
...Site/Extensions/HttpServiceCollectionExtensions.cs 100% <100%> (ø)
src/LondonTravel.Site/ApplicationCookie.cs 100% <100%> (ø)
...ravel.Site/Extensions/ILoggingBuilderExtensions.cs 76.19% <100%> (ø) ⬆️
...c/LondonTravel.Site/Identity/OAuthEventsHandler.cs 71.42% <50%> (+3.65%) ⬆️
...ravel.Site/Extensions/ClaimsPrincipalExtensions.cs 89.83% <57.14%> (+1.15%) ⬆️
... and 33 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b599761...56af239. Read the comment docs.

martincostello added some commits Jun 3, 2018

Output web driver logs
Output web driver logs to the test output.
By default only emit browser logs
Unless debugging, only emit browser logs from web driver.
Remove redundant namespace
Remove redundant namespace declaration.
Collect any screenshots in AppVeyor
Collect any screenshots from failing tests as artifacts when running in AppVeyor.
Set ChromeDriver timeout
Set the ChromeDriver remote timeout to 10 seconds.

@martincostello martincostello merged commit 5425bf1 into master Jun 3, 2018

4 checks passed

codecov/patch 92.28% of diff hit (target 58.9%)
codecov/project 65.51% (+6.6%) compared to b599761
continuous-integration/appveyor/pr AppVeyor build succeeded
continuous-integration/travis-ci/pr The Travis CI build passed

@martincostello martincostello deleted the Hook-In-HttpClientFactory-More branch Jun 3, 2018

martincostello added a commit that referenced this pull request Jul 1, 2018

Enable test for invalid tokens
Enable the test for invalid API access tokens as that test has actually been runnable since #186.

martincostello added a commit that referenced this pull request Jul 1, 2018

Enable skipped test
Enable skipped test that has actually been usable since #186 was merged.
Remove obsolete code to skip tests when a real CosmosDB instance isn't configured.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment