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.

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

External authentication failing on iOS 12 #2595

Closed
ryanmendoza opened this Issue Sep 7, 2018 · 39 comments

Comments

Projects
None yet
@ryanmendoza
Copy link

ryanmendoza commented Sep 7, 2018

Please only use the issue tracker for bug reports and/or feature requests. For general security questions, or free or commercial support options do not use the issue tracker and instead see here for more details.

For bug reports, include the relevant log files related to your issue. See here how to enable logging. Delete this line once you have.

Finally, please keep the issue concise and to the point. If you paste in more code than the text for the issue you are reporting then we will most likely not read it.

Issue / Steps to reproduce the problem

External authentication failing in iOS 12 Safari.

I am seeing failures across all iOS 12 (beta) devices when authenticating using an external provider. There is no issue with iOS <11 or mobile Firefox, Chrome (nor desktop/Android browsers).

var result = await HttpContext.AuthenticateAsync(IdentityServer4.IdentityServerConstants.ExternalCookieAuthenticationScheme); is failing for devices running Safari in iOS 12. Tracing the request, it looks like idsrv.external cookie is empty when redirecting from signin-oidc endpoint to the externalcallback endpoint, but is populated if you simply refresh the browser on the externalcallback page.

If you refresh the errored /externalcallback page, it will continue on and work fine.

I understand that iOS 12 is a beta OS, but it is very close to release and it would be nice to get this figured out.

Update: I was able to replicate this in Safari 12.0 for macOS Mojave as well. My guess is this may be more related to Microsoft authentication libraries...

Relevant parts of the log file

2018-09-07 01:58:39.674 +00:00 [Error] Microsoft.AspNetCore.Server.Kestrel: Connection id "0HLGKCKUC9350", Request id "0HLGKCKUC9350:0000000C": An unhandled exception was thrown by the application.
System.Exception: External authentication error
   at IdentityServer.UI.AccountController.ExternalLoginCallback() in C:\Users\Ryan\Projects\Identity Server\src\IdentityServer\UI\Account\AccountController.cs:line 192
   at Microsoft.AspNetCore.Mvc.Internal.ActionMethodExecutor.TaskOfIActionResultExecutor.Execute(IActionResultTypeMapper mapper, ObjectMethodExecutor executor, Object controller, Object[] arguments)
   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker.InvokeActionMethodAsync()
   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker.InvokeNextActionFilterAsync()
   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker.Rethrow(ActionExecutedContext context)
   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker.InvokeInnerFilterAsync()
   at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeNextResourceFilter()
   at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.Rethrow(ResourceExecutedContext context)
   at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeFilterPipelineAsync()
   at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeAsync()
   at Microsoft.AspNetCore.Builder.RouterMiddleware.Invoke(HttpContext httpContext)
   at Microsoft.AspNetCore.StaticFiles.StaticFileMiddleware.Invoke(HttpContext context)
   at IdentityServer4.Hosting.IdentityServerMiddleware.Invoke(HttpContext context, IEndpointRouter router, IUserSession session, IEventService events)
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Cors.Infrastructure.CorsMiddleware.Invoke(HttpContext context)
   at IdentityServer4.Hosting.BaseUrlMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
   at IdentityServer.Startup.<>c.<<Configure>b__6_0>d.MoveNext() in C:\Users\Ryan\Projects\Identity Server\src\IdentityServer\Startup.cs:line 213
--- End of stack trace from previous location where exception was thrown ---
   at Microsoft.AspNetCore.Server.IISIntegration.IISMiddleware.Invoke(HttpContext httpContext)
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequests[TContext](IHttpApplication`1 application)

        /// <summary>
        /// Post processing of external authentication
        /// </summary>
        [HttpGet]
        public async Task<IActionResult> ExternalLoginCallback()
        {
            // read external identity from the temporary cookie
            var result = await HttpContext.AuthenticateAsync(IdentityServer4.IdentityServerConstants.ExternalCookieAuthenticationScheme);

            if (result.Succeeded != true)
            {
                throw new Exception("External authentication error");
            }
            ...
         }
@pierslawson

This comment has been minimized.

Copy link

pierslawson commented Sep 7, 2018

I have the same issue, but can provide more information.

I don't believe this is a problem with the Identity Server package itself, rather with your application that is hosting the Identity Server package (not your client application). I guess like me you built your Identity Server application using one of the quick start samples. These all use a class called SecurityHeadersAttribute.cs which adds a Content Security Policy (CSP).

I think the upcoming version of Safari either has a bug in it that means it is interpreting this CSP incorrectly and no longer sending cookies with a cross domain POST... or they have implemented a tighter level of security than previously (and tighter than other browsers) which results in the same thing... the correlation and nonce cookies that are set by your client application during authentication are not being sent back to your client application as the user's browser POSTs back from the Identity Server application.

I suspect there are many more Identity Server based applications out there (that are derived from the samples) who will fall foul of this in two weeks time... unless Apple know there is an issue and are about to fix it or the implementer of the Identity Server based application weakens/removes the CSP built into it.

@ryanmendoza

This comment has been minimized.

Copy link
Author

ryanmendoza commented Sep 7, 2018

@pierslawson thanks for the insight! I removed my CSP policies from AccountController and ExternalController, but I'm still seeing the issue. I noticed that this problem does not exist with the IdentityServer4 Demo site.

@Tratcher

This comment has been minimized.

Copy link

Tratcher commented Sep 7, 2018

Sounds like an issue with the SameSite cookie attribute. Support for this was just added in Safari 12. https://caniuse.com/#feat=same-site-cookie-attribute. Do you have a Fiddler trace? That would show the SameSite attribute on Set-Cookie headers. We've had to disable SameSite on most OAuth related cookies.

@ryanmendoza

This comment has been minimized.

Copy link
Author

ryanmendoza commented Sep 7, 2018

@Tratcher interesting. Any idea how to disable same site on the ExternalCookieAuthenticationScheme? Am I best off just creating a custom scheme?

@ryanmendoza

This comment has been minimized.

Copy link
Author

ryanmendoza commented Sep 7, 2018

Looks like bullet dodged on potential login failures starting next week.

This did it for me:

.AddCookie(Constants.PingExternalAuthenticationScheme, options =>
{
     options.Cookie.Name = Constants.PingExternalAuthenticationScheme;
      options.Cookie.SameSite = Microsoft.AspNetCore.Http.SameSiteMode.None;
})
@Tratcher

This comment has been minimized.

Copy link

Tratcher commented Sep 7, 2018

Ah, services.ConfigureExternalCookie(...); for folks using ASP.NET Identity.

@ryanmendoza

This comment has been minimized.

Copy link
Author

ryanmendoza commented Sep 10, 2018

@Tratcher would this be considered a bug in Safari 12's implementation of SameSite cookies? I used their Feedback app to report the discrepancy in behavior; not sure if there is a better route.

@Tratcher

This comment has been minimized.

Copy link

Tratcher commented Sep 10, 2018

None of the browsers have quite decided how to handle OAuth scenarios with SameSite. When we tested them a few months ago they all broke and we had to disable SameSite. I hear a few of them now work the the Lax setting but apparently not IOS.

@ryanmendoza

This comment has been minimized.

Copy link
Author

ryanmendoza commented Sep 10, 2018

Got it... so disabling SameSite completely is the best option for now. Thanks for your background on the issue!

@cdwaddell

This comment has been minimized.

Copy link

cdwaddell commented Sep 21, 2018

@brockallen , it looks like the idsrv.external cookie options in AddIdentityServer cannot be overridden, or I can't figure out how. This cookie is being assigned the samesite=lax setting. This setting breaks in iOS 12. Any direction on how to change this?

I need to add options.Cookie.SameSite = SameSiteMode.None;
to ConfigureInternalCookieOptions Line 41

if (name == IdentityServerConstants.ExternalCookieAuthenticationScheme)
{
    ///need options.Cookie.SameSite = SameSiteMode.None; here
    options.Cookie.Name = IdentityServerConstants.ExternalCookieAuthenticationScheme;
}

I have already modified all other cookies, but can't seem to figure out how to modify this one. This is what I have done.

            service.ConfigureApplicationCookie(options =>
            {
                options.Cookie.SameSite = SameSiteMode.None;
            });

            service.ConfigureExternalCookie(options =>
            {
                options.Cookie.SameSite = SameSiteMode.None;
            });
...
            app.UseCookiePolicy(new CookiePolicyOptions {MinimumSameSitePolicy = SameSiteMode.None});
...
           //for each of my external login providers
           options.CorrelationCookie.SameSite = SameSiteMode.None;
@brockallen

This comment has been minimized.

Copy link
Member

brockallen commented Sep 21, 2018

You don't have to use our external cookie. As such, configure your own and make your UI use it (instead of ours). Having said that, I will fix ours in the next release.

@brockallen brockallen self-assigned this Sep 21, 2018

@cdwaddell

This comment has been minimized.

Copy link

cdwaddell commented Sep 21, 2018

Thanks again for the great tools @brockallen and for supporting the community.

@NickyM

This comment has been minimized.

Copy link

NickyM commented Sep 25, 2018

When is this fix set to release @brockallen ?

@brockallen

This comment has been minimized.

Copy link
Member

brockallen commented Sep 25, 2018

We will be working on the next preview late next week.

@brockallen brockallen added this to the 2.3 milestone Sep 25, 2018

@NickyM

This comment has been minimized.

Copy link

NickyM commented Sep 25, 2018

Okay, thanks for answering. :)

I can comfirm creating our own cookie scheme, without SameSite solved all iOS 12 related problems here as well.

@dpotoole

This comment has been minimized.

Copy link

dpotoole commented Sep 26, 2018

Running into this problem as well using Azure AD for authentication.

@NickyM can you please post some code on what you had to do to get your own cookie scheme working? Thanks!

@NickyM

This comment has been minimized.

Copy link

NickyM commented Sep 26, 2018

@dpotoole, well basically I just renamed the cookie, changed SameSite to SameSiteMode.None, and then signed in using the renamed cookie.

I have two instances of IdentityServer, where one is using AspNetIdentity and one doesnt.

IdentityServer with MS and Google as IDPs, and AspNetIdentity for handling local users

Startup.cs:

services.AddAuthentication(".YourCookieScheme.Name")
	.AddCookie(".YourCookieScheme.Name", options =>
	{
		options.Cookie.Name = ".YourCookieScheme.Name";
		options.Cookie.SameSite = SameSiteMode.None;
	})
	.AddMicrosoftAccount(options =>
	{
		options.CorrelationCookie.SameSite = SameSiteMode.None;
		options.SignInScheme = ".YourCookieScheme.Name";
		//Rest of settings here
	})
	.AddGoogle(options =>
	{
		options.CorrelationCookie.SameSite = SameSiteMode.None;
		options.SignInScheme = ".YourCookieScheme.Name";
		//Rest of settings here
	});

And in /Account/ExternalLoginCallback:

public async Task<IActionResult> ExternalLoginCallback(string returnUrl = null, string remoteError = null)
{
	var info = await GetExternalLoginInfoAsync();

	//( ... irellevant business logic ... )
	
	var result = await _signInManager.ExternalLoginSignInAsync(info.LoginProvider, info.ProviderKey, isPersistent: false);

	//( ... irellevant business logic ... )
	
	await _httpContextAccessor.HttpContext.SignOutAsync(".YourCookieScheme.Cookies");


	//( ... irellevant business logic ... )
}

Where GetExternalLoginInfoAsync() is a modified copy of _sigInManager.GetExternalLoginInfo (https://github.com/aspnet/Identity/blob/master/src/Identity/SignInManager.cs)

private async Task<ExternalLoginInfo> GetExternalLoginInfoAsync(string expectedXsrf = null)
{
	var auth = await _httpContextAccessor.HttpContext.AuthenticateAsync(".YourCookieScheme.Name");
	//rest of method
}

private async Task<IEnumerable<AuthenticationScheme>> GetExternalAuthenticationSchemesAsync()
{
	var schemes = await _schemes.GetAllSchemesAsync();
	return schemes.Where(s => !string.IsNullOrEmpty(s.DisplayName));
}

IdentityServer, that uses another IdentityServer as IDP.

Startup.cs:

services.AddAuthentication(".AnotherCookieScheme.Name")
	.AddCookie(".AnotherCookieScheme.Name", options =>
	{
		options.Cookie.Name = ".AnotherCookieScheme.Name";
		options.Cookie.SameSite = SameSiteMode.None;
	})
	.AddOpenIdConnect(options =>
	{
		options.CorrelationCookie.SameSite = SameSiteMode.None;
		options.SignInScheme = ".AnotherCookieScheme.Name";
		//Rest of settings here
	})

Account/ExternalLoginCallback:

public async Task<IActionResult> ExternalCallback(string returnUrl)
{
	//Read external identity from the tempoary cookie
	var result = await HttpContext.AuthenticateAsync(".AnotherCookieScheme.Cookies");

	//(.... extracting claims from result and signing in ... )
   

	// Delete tempoary cookie used during external authentication
	await HttpContext.SignOutAsync(".AnotherCookieScheme.Cookies");
}

Hope that it helps. I have no clue, if this is the best and most optimal solution, but it works and I'm happy.

@cdwaddell

This comment has been minimized.

Copy link

cdwaddell commented Sep 27, 2018

After looking more closely at what IdentityServer was doing, I figure out a workaround. This has been confirmed on our system. My workaround just targets this cookie:

    public class FixIos12Cookies : IConfigureNamedOptions<CookieAuthenticationOptions>
    {
        public void Configure(CookieAuthenticationOptions options)
        {
        }

        public void Configure(string name, CookieAuthenticationOptions options)
        {

            if (name == IdentityServerConstants.ExternalCookieAuthenticationScheme)
            {
                options.Cookie.SameSite = SameSiteMode.None;
            }
        }
    }

Then after you setup IdentityServer, you add this to DI

var idServerBuilder = services.AddIdentityServer();
...
services.AddSingleton<IConfigureOptions<CookieAuthenticationOptions>, FixIos12Cookies>();

This should be able to be removed when the patch goes through.

@Devvox93

This comment has been minimized.

Copy link

Devvox93 commented Sep 27, 2018

I tried the workaround above and it didn't work. But keep reading if it didn't work for you either. I then tried the solution described here: aspnet/Identity#1970 (comment) which did work.

Then I thought that this option may work after all, but the singleton needs to be registered AFTER services.AddAuthentication().

Please correct me if I'm wrong @cdwaddell :)

Deeper information below
I've kept reading on as I wanted to know what this SameSite cookie option was and why it was broken in iOS 12, which led me to the following bug reported to Webkit: https://bugs.webkit.org/show_bug.cgi?id=188165
And that led me to https://datatracker.ietf.org/doc/draft-ietf-httpbis-rfc6265bis/ which is an Expired Internet Draft.

As far as I can see, Webkit implemented an expired draft, which is probably expired for a reason (seeing it doesn't work with oAuth).

@cdwaddell

This comment has been minimized.

Copy link

cdwaddell commented Sep 27, 2018

@Devvox93 , you do have to combine it with a couple of other settings changes that I had posted earlier. The most important is changing the MinimumSameSitePolicy to allow the SameSiteMode.None. Otherwise the same site setting may not get applied because it is too low. Here they all are in one place.

            service.ConfigureApplicationCookie(options =>
            {
                options.Cookie.SameSite = SameSiteMode.None;
            });

            service.ConfigureExternalCookie(options =>
            {
                options.Cookie.SameSite = SameSiteMode.None;
            });
...
            app.UseCookiePolicy(new CookiePolicyOptions {MinimumSameSitePolicy = SameSiteMode.None});
...
           //for each of your external login providers
           options.CorrelationCookie.SameSite = SameSiteMode.None;

If it still doesn't work for you, then you have another cookie with the bad policy. The way I tracked them all down was inspecting the cookies in Chrome's Developer Tools. I had to inspect each response for new cookies being added. Each cookie needed it's own samesite policy change. After the changes above, the external cookie itself still had the issue. That was what caused the need for the workaround:

    public class FixIos12Cookies : IConfigureNamedOptions<CookieAuthenticationOptions>
    {
        public void Configure(CookieAuthenticationOptions options)
        {
        }

        public void Configure(string name, CookieAuthenticationOptions options)
        {

            if (name == IdentityServerConstants.ExternalCookieAuthenticationScheme)
            {
                options.Cookie.SameSite = SameSiteMode.None;
            }
        }
    }

Then add it to DI. The only requirement (I think) is for it to be after AddIdentityServer:

var idServerBuilder = services.AddIdentityServer();
...
services.AddSingleton<IConfigureOptions<CookieAuthenticationOptions>, FixIos12Cookies>();
@Devvox93

This comment has been minimized.

Copy link

Devvox93 commented Sep 27, 2018

In my case, this (and only this) was what I did to fix the issue:

services.AddAuthentication()
    .Services.ConfigureExternalCookie(options =>
    {
        options.Cookie = new CookieBuilder
        {
            SameSite = SameSiteMode.None
        };
    });

EDIT: This was done as the final step in ConfigureServices.

@brockallen

This comment has been minimized.

Copy link
Member

brockallen commented Oct 3, 2018

I see @NickyM also setting the None setting on the correlation cookies for the external providers. Can anyone confirm if that is also necessary for this iOS 12 browser issue? I did not see that as necessary from the Microsoft announcement.

@NickyM

This comment has been minimized.

Copy link

NickyM commented Oct 3, 2018

@brockallen: I'll double check tomorrow morning. It's 10:15pm here, and I'll have to get to my workplace to run and iOS device agains my development machine.

Better have as little disabled as possible. Haven't tried the ConfigureExternalCookie. ConfigureApplicationCookie alone isn't enough, and I did unfortunately not spot that one at the time of patching.

brockallen added a commit that referenced this issue Oct 3, 2018

@brockallen

This comment has been minimized.

Copy link
Member

brockallen commented Oct 3, 2018

I've made the change for the external cookie (also done in the ASP.NET Identity integration library as well).

I'll wait to hear back from @NickyM on the others (but I don't expect we will need to, as those cookies are lax and the redirect to the external providers are typically in the top browser window and often use GET). We might need them for the OIDC/WS-Fed providers though, come to think of it, since they send results back via POST (and thus the same issue would arise).

@brockallen brockallen added the ready label Oct 3, 2018

@Tratcher

This comment has been minimized.

Copy link

Tratcher commented Oct 3, 2018

@brockallen @NickyM The correlation cookies already default to none due to issues we observed with earlier implementations on other browsers.

@brockallen

This comment has been minimized.

Copy link
Member

brockallen commented Oct 3, 2018

Ok, good to hear. Thanks for the info @Tratcher.

@brockallen brockallen removed the ready label Oct 3, 2018

@brockallen

This comment has been minimized.

Copy link
Member

brockallen commented Oct 3, 2018

Given that info, I can close this. Thanks all.

@brockallen brockallen closed this Oct 3, 2018

@brockallen brockallen added bug and removed bug report labels Oct 3, 2018

@brockallen brockallen removed this from the 2.3 milestone Oct 3, 2018

brockallen added a commit that referenced this issue Oct 25, 2018

Review1 changes (#2694)
* Add built-in support for Confirmation (cnf) (#2440)

* add confirmation object to pipeline

* Add test validator

* fixing NRE

* Switching to string for cnf

* move test validator to test project

* revert client config change

* add try/catch in JSON logic

* added notes that CNF string must be a JSON object

* added test

* Move default payload creation to extension method - closes #2299

* Update README.md

* Scrub id_token_hint from authorize logs

* use constant instead of string

* add refresh_token to scrub list in token request logger

* move is4.csproj to top-level src folder, move host

* fix XML comment

* updating for july

* update ignore

* rework to use IdentityServerUser

* rework folder names

* rework using new storage abstractions

* remove cors service

* make EndSession public #2469

* add null check when unprotecting data #2504

* use GetIdentityServerBasePath instead of Request.PathBase #2446

* reorg default impls and interfaces for consistency

* nuget updates in test projects

* Documentation: Added claimsaction to map website claim (#1) (#2377)

* Make AddScriptCspHeaders and AddStyleCspHeaders public #2513

* Add more strict cache control headers when softer headers are already added by HttpContext.SignInAsync #2514

* add better/more error descriptions to authorize response validator #2218 (#2515)

* add invalid uri scheme validation (#2506)

* add invalid uri scheme validation

* move uri redirect uri prefix validation to client configuration validator

* add option to explicitly configure the cookie auth scheme for interactive users #2489 (#2516)

* Add parameters to IntrospectionRequestValidationResult - #2388 (#2512)

* Update refresh_tokens.rst (#2316)

Adapt text to indicate refresh tokens still expire according to the sliding refresh token timeline.

* "update"

* fix validation bug on config; better config logs for authN schemes

* Remove unused ctor (#2524)

* enable default client validator by default (#2525)

* Fixes 404 (#2527)

* CorsService doesn't handle null for origin #2523

* DistributedCacheStateDataFormatter should handle failed Unprotect workflows #2533

* 2.3.0-preview1

* resolve login/logout url, et al from named options (#2540)

* resolve login/logout url, et al from named options #2532

* log effective login, et al. paths

* preview1-update1

* bug in consent when user denies

* add Securing Angular Apps with OpenID and OAuth2

* Migrate tests to new IdentityModel style (1)

* Migrate tests to new IdentityModel style (2)

* Migrate tests to new IdentityModel style (3)

* Migrate tests to new IdentityModel style (4)

* remove unused handler

* Migrate tests to new IdentityModel style (5)

* Migrate tests to new IdentityModel style (6)

* Finished integration clients with new idm style

* added SO CC-BY-SA info and links

* Renamed Client -> BackChannelClient

* Update client authentication tests

* Migrated PKCE tests

* Migrated introspection tests

* Migrated revocation tests

* Found missing introspection test

* Migrated DiscoveryEndpointTests

* Merge fixes

* Matched PR to new IdentityServer project structure

* Switched to new device flow store

* Moved in-memory device flow store to singleton

* 6_aspnet_identity.rst (#2570)

Incorrectly states "which replaces the call to UseIdentity" instead of "which replaces the call to UseAuthentication".

* Added DeviceFlowCodeService to handle hashing codes and handle generation

* update preview version

* add new dotnet tool based build script

* Add alternative dotnet tool based build file for bash

* update bash

* update ignore file

* switch to new cake (#2593)

* august sponsor update

* Add strong name (#2597)

* add strong name

* update references to strongly named packages

* updated ignore

* Create jwk document when signing with JsonWebKey (#2604)

* Update introspection.rst (#2606)

Was referring to scope secrets. Reused sentence from https://github.com/IdentityServer/IdentityServer4/blob/release/docs/topics/reference_tokens.rst

* Update secrets.rst (#2611)

* add issue templates

* Update issue templates

* Update Feature_request.md

* Delete feature_request.md

* Delete bug_report.md

* Update Bug_report.md

* add NoBuild to build file

* fix build - again

* Create SECURITY.MD

* update to new build/versioning

* update bash script

* update bash script

* Switched validator to use code service instead of store

* recursion ftw

* Initial working device flow consent

* Changing cake file to skip versioning on non-Windows (#2637)

Changing cake file to skip versioning on non-Windows

* update bash script

* remove hard-coded versions

* disable source link support because of problems with msbuild task

* Update to new IdM docs

* update endpoint docs to use new IdentityModel style

* fix links

* change color coding style

* update from september

* update to IdentityModel 3.10

* add source link back

* Make some internal types public to facilitate custom service implementations (#2545)

* Make TokenCreationRequest.Validate() public so it can be invoked by custom impl of ITokenService

* Make ClientExtensions public so they can be reused by custom IClientSecretValidator impl

* move AccessTokenAudience to public constants for reuse in custom ITokenService impl

* Change: Made DefaultUserSession.AuthenticateAsync overrideable so that (#2607)

it will be easier to support user impersonation.

* Corrected value for parsed secret type (#2658)

* update csproj

* update csproj

* disable same-site for external cookie #2595

* remove redundant call  #2582

* make EndSessionRequestValidator public #2560

* set cookies to IsEssential #2554

* nuget update

* code comments

* support idp:local as idp hint #2641

* add logic to enfore client's user sso lifetime #2609

* Fixed access denied logic. Made use of new IdentityModel constants

* Reviewed TODOs

* Moved user code generator to correct folder

* Basic retry policy for response generator. Updated some comments and class name

* Added retry limit handling

* Update unpredictable test

* More IdentityModel constants

* Redacted device code from logging

* Updated IdentityServer4.Storage

* Thread safety for InMemoryDeviceFlowStore

* Ctrl+Shift+D

brockallen added a commit that referenced this issue Oct 30, 2018

Device flow (take 2) (#2760)
* Add constants for device flow grant type

* add constants, endpoint options and discovery

* Add device flow authorize request validator (#2306)

* Added models and interfaces for device authorization request validator

* Added device authorization request validator

* Validator tests (#2391)

* Switched to underscores...

* Reworked Device Authoirzation Request Validator to be more inline with Authorization Request Validator

* Added tests for device authorization request validator

* Added client secret to device flow test client

* Response generator & tests (#2395)

* Added user code generation

* Added device flow options

* Initial implementation of device authorization response generator & stores

* Started response generator unit tests

* Completed device authorization response generator tests

* Store tests

* Device Flow Authorization Endpoint (#2403)

* Started device authorization endpoint

* Made scope parameter optional

* Device authorization endpoint tests. Added events. Added default interval

* Device Flow Response Validator (#2422)

* Initial device code validation

* Device code token request tests

* Initial throttling service. TODO: tests & IDistributedCache dependency

* Added dependency for IDistributedCache. Improved tests

* Throttling service tests and fixes

* Added device code grant to TokenResponseGenerator

* Initial working device flow end to end (#2449)

* Review1 changes (#2694)

* Add built-in support for Confirmation (cnf) (#2440)

* add confirmation object to pipeline

* Add test validator

* fixing NRE

* Switching to string for cnf

* move test validator to test project

* revert client config change

* add try/catch in JSON logic

* added notes that CNF string must be a JSON object

* added test

* Move default payload creation to extension method - closes #2299

* Update README.md

* Scrub id_token_hint from authorize logs

* use constant instead of string

* add refresh_token to scrub list in token request logger

* move is4.csproj to top-level src folder, move host

* fix XML comment

* updating for july

* update ignore

* rework to use IdentityServerUser

* rework folder names

* rework using new storage abstractions

* remove cors service

* make EndSession public #2469

* add null check when unprotecting data #2504

* use GetIdentityServerBasePath instead of Request.PathBase #2446

* reorg default impls and interfaces for consistency

* nuget updates in test projects

* Documentation: Added claimsaction to map website claim (#1) (#2377)

* Make AddScriptCspHeaders and AddStyleCspHeaders public #2513

* Add more strict cache control headers when softer headers are already added by HttpContext.SignInAsync #2514

* add better/more error descriptions to authorize response validator #2218 (#2515)

* add invalid uri scheme validation (#2506)

* add invalid uri scheme validation

* move uri redirect uri prefix validation to client configuration validator

* add option to explicitly configure the cookie auth scheme for interactive users #2489 (#2516)

* Add parameters to IntrospectionRequestValidationResult - #2388 (#2512)

* Update refresh_tokens.rst (#2316)

Adapt text to indicate refresh tokens still expire according to the sliding refresh token timeline.

* "update"

* fix validation bug on config; better config logs for authN schemes

* Remove unused ctor (#2524)

* enable default client validator by default (#2525)

* Fixes 404 (#2527)

* CorsService doesn't handle null for origin #2523

* DistributedCacheStateDataFormatter should handle failed Unprotect workflows #2533

* 2.3.0-preview1

* resolve login/logout url, et al from named options (#2540)

* resolve login/logout url, et al from named options #2532

* log effective login, et al. paths

* preview1-update1

* bug in consent when user denies

* add Securing Angular Apps with OpenID and OAuth2

* Migrate tests to new IdentityModel style (1)

* Migrate tests to new IdentityModel style (2)

* Migrate tests to new IdentityModel style (3)

* Migrate tests to new IdentityModel style (4)

* remove unused handler

* Migrate tests to new IdentityModel style (5)

* Migrate tests to new IdentityModel style (6)

* Finished integration clients with new idm style

* added SO CC-BY-SA info and links

* Renamed Client -> BackChannelClient

* Update client authentication tests

* Migrated PKCE tests

* Migrated introspection tests

* Migrated revocation tests

* Found missing introspection test

* Migrated DiscoveryEndpointTests

* Merge fixes

* Matched PR to new IdentityServer project structure

* Switched to new device flow store

* Moved in-memory device flow store to singleton

* 6_aspnet_identity.rst (#2570)

Incorrectly states "which replaces the call to UseIdentity" instead of "which replaces the call to UseAuthentication".

* Added DeviceFlowCodeService to handle hashing codes and handle generation

* update preview version

* add new dotnet tool based build script

* Add alternative dotnet tool based build file for bash

* update bash

* update ignore file

* switch to new cake (#2593)

* august sponsor update

* Add strong name (#2597)

* add strong name

* update references to strongly named packages

* updated ignore

* Create jwk document when signing with JsonWebKey (#2604)

* Update introspection.rst (#2606)

Was referring to scope secrets. Reused sentence from https://github.com/IdentityServer/IdentityServer4/blob/release/docs/topics/reference_tokens.rst

* Update secrets.rst (#2611)

* add issue templates

* Update issue templates

* Update Feature_request.md

* Delete feature_request.md

* Delete bug_report.md

* Update Bug_report.md

* add NoBuild to build file

* fix build - again

* Create SECURITY.MD

* update to new build/versioning

* update bash script

* update bash script

* Switched validator to use code service instead of store

* recursion ftw

* Initial working device flow consent

* Changing cake file to skip versioning on non-Windows (#2637)

Changing cake file to skip versioning on non-Windows

* update bash script

* remove hard-coded versions

* disable source link support because of problems with msbuild task

* Update to new IdM docs

* update endpoint docs to use new IdentityModel style

* fix links

* change color coding style

* update from september

* update to IdentityModel 3.10

* add source link back

* Make some internal types public to facilitate custom service implementations (#2545)

* Make TokenCreationRequest.Validate() public so it can be invoked by custom impl of ITokenService

* Make ClientExtensions public so they can be reused by custom IClientSecretValidator impl

* move AccessTokenAudience to public constants for reuse in custom ITokenService impl

* Change: Made DefaultUserSession.AuthenticateAsync overrideable so that (#2607)

it will be easier to support user impersonation.

* Corrected value for parsed secret type (#2658)

* update csproj

* update csproj

* disable same-site for external cookie #2595

* remove redundant call  #2582

* make EndSessionRequestValidator public #2560

* set cookies to IsEssential #2554

* nuget update

* code comments

* support idp:local as idp hint #2641

* add logic to enfore client's user sso lifetime #2609

* Fixed access denied logic. Made use of new IdentityModel constants

* Reviewed TODOs

* Moved user code generator to correct folder

* Basic retry policy for response generator. Updated some comments and class name

* Added retry limit handling

* Update unpredictable test

* More IdentityModel constants

* Redacted device code from logging

* Updated IdentityServer4.Storage

* Thread safety for InMemoryDeviceFlowStore

* Ctrl+Shift+D

* Merge fixes

* Cake merge fixes
@rushfrisby

This comment has been minimized.

Copy link

rushfrisby commented Oct 31, 2018

@brockallen any idea when this will be released? I can implement one of these workarounds but if it's going to be pushed out this week I'll just wait. Thanks!

@brockallen

This comment has been minimized.

Copy link
Member

brockallen commented Oct 31, 2018

preview2 will be in the next week. rtm end of next month (november).

@rushfrisby

This comment has been minimized.

Copy link

rushfrisby commented Oct 31, 2018

I will give preview 2 a try then. thanks!

@ckrandhir

This comment has been minimized.

Copy link

ckrandhir commented Nov 7, 2018

Version 2.2.0-preview3-35497 did not solve the problem still signInManager.GetExternalLoginInfoAsync() returning null but it is working fine on windows an earlier version ios

@jonasnystrom

This comment has been minimized.

Copy link

jonasnystrom commented Nov 28, 2018

Hi @leastprivilege , have the same issue (I think) in Safari for windows. Have upgraded to 2.3.0 and can see that the external cookies now gone from LAX to None. Any ideas?

@prflores

This comment has been minimized.

Copy link

prflores commented Dec 26, 2018

does this mean that updating my identityserver to 2.3.0 would not need these workarounds? should there be any codes changes in the web client app as well? Thanks!

@rushfrisby

This comment has been minimized.

Copy link

rushfrisby commented Dec 26, 2018

@prflores updating to 2.3 fixed this issue for me

@prflores

This comment has been minimized.

Copy link

prflores commented Dec 27, 2018

Thanks for the info @rushfrisby. I am not sure if this is related to my issue but I'm experiencing an infinite redirection between my identityserver and client site from iOS 12 users only (android, windows desktop and old version of iOS is fine) when signing-in (identity server with asp.net identity). I upgraded to 2.3.0 but still same result. I compared the logs got from iOS users and desktop users and surprised it is almost the same. I'm not sure where to start checking. Any help would do. Thank you.

@brockallen

This comment has been minimized.

Copy link
Member

brockallen commented Jan 11, 2019

FYI, here's a different approach to solving this issue, while being able to use same-site cookies.

https://brockallen.com/2019/01/11/same-site-cookies-asp-net-core-and-external-authentication-providers/

Maybe someone can test on Safari and iOS for me?

@cdwaddell

This comment has been minimized.

Copy link

cdwaddell commented Jan 12, 2019

@prflores

This comment has been minimized.

Copy link

prflores commented Jan 18, 2019

Thanks @brockallen! this approach fixed my issue.

@mrlund

This comment has been minimized.

Copy link

mrlund commented Jan 24, 2019

The latest approach from @brockallen worked for me too - as a point of interest I only started experiencing this issue after adding
services.AddOidcStateDataFormatterCache("oidc");
in order to fix an intermittent problem I was seeing with "Correlation failed" errors that I believe was related to the state sometimes being too large. Before that iOS/Safari login was working fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.