Same site cookie fix #35
Conversation
Implemented SameSiteCookieManager as suggested https://docs.microsoft.com/en-us/aspnet/samesite/owin-samesite
TiagoBrenck
requested review from
kalyankrishna1,
bgavrilMS,
jmprieur and
pmaytak
There was a problem hiding this comment.
LGTM, @TiagoBrenck
I did not test it yet (but was worried about the major change)
| @@ -65,20 +65,20 @@ | |||
| <Reference Include="Microsoft.IdentityModel.Tokens, Version=5.4.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL"> | |||
| <HintPath>..\packages\Microsoft.IdentityModel.Tokens.5.4.0\lib\net461\Microsoft.IdentityModel.Tokens.dll</HintPath> | |||
| </Reference> | |||
| <Reference Include="Microsoft.Owin, Version=4.0.1.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL"> | |||
| <HintPath>..\packages\Microsoft.Owin.4.0.1\lib\net45\Microsoft.Owin.dll</HintPath> | |||
| <Reference Include="Microsoft.Owin, Version=4.1.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL"> | |||
There was a problem hiding this comment.
Did you test with the major Owin change?
There was a problem hiding this comment.
Yes, I tested. We actually need to update to 4.1.0 according to the docs, to handle the issue on net framework 4.7.2
There was a problem hiding this comment.
Oh yes. Thanks for the update. This makes sense. I had forgotten
|
Tried to replicate the same steps as mentioned in the commit. i still get IDX21323 OpenIdConnectProtocolValidationContext.Nonce was null, OpenIdConnectProtocolValidatedIdToken.Paylocad.Nonce was not null steps done: 1.upgraded .net framework to 4.72 |
|
@TiagoBrenck FYI |
|
@remojan23 could you share your |
|
@TiagoBrenck
|
|
@remojan23 I am still not able to reproduce it here. I noticed that many of your packages is targeting net45 or 46. I wonder if that could be the problem. When I updated the framework in this sample, I have also run the follow command, to re-target all the packages to the right framework: |
|
When the user first navigates to the app, the app detects that they are not authenticated and redirects them to Azure AD. During this step, OWIN sets a cookie named OpenIdConnect.nonce. This cookie is somehow not seen in the list in beta 80 due to which that |
|
@remojan23 were you able to reproduce this error using the sample as well? |
|
@remojan23 did it work? |
Update to DotNet Framework 4.7.2
Implemented SameSiteCookieManager as suggested ms docs
Tested using Chromium 60 before and after implementation to validate the fix.