Closed marcuslindblom closed 4 years ago
Thanks for the bug report. I'll look into this today and see what can be done.
I'm not sure how it should work but it seems strange that the AuthenticationTokens
and AuthenticationProperties
never gets updated. Maybe they should not be stored on the user document at all or maybe it should be in sync with IdentityUserAuthTokens
and IdentityUserLogins
documents. Not sure what is the right thing to do.
Marcus, looking into this a bit, where is AuthenticationTokens
and AuthenticationProperties
?
It's not on Raven.Identity.IdentityUser, nor on UserLogins nor IdentityUserClaims. Am I missing something?
@JudahGabriel They are properties on ExternalLoginInfo
. Maybe it has something to do with this issue. I use the implementation you linked to here. This is how the serialization looks like for me.
"Logins": [
{
"$type": "Microsoft.AspNetCore.Identity.ExternalLoginInfo, Microsoft.AspNetCore.Identity",
"Principal": {
"Identities": [
{
"AuthenticationType": "OAuth2-Github",
"IsAuthenticated": true,
"Actor": null,
"BootstrapContext": null,
"Claims": [
{
"Type": "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/nameidentifier",
"Value": "319720",
"ValueType": "http://www.w3.org/2001/XMLSchema#string",
"Issuer": "OAuth2-Github",
"OriginalIssuer": "OAuth2-Github"
},
{
"Type": "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/name",
"Value": "marcuslindblom",
"ValueType": "http://www.w3.org/2001/XMLSchema#string",
"Issuer": "OAuth2-Github",
"OriginalIssuer": "OAuth2-Github"
},
{
"Type": "urn:github:name",
"Value": "Marcus Lindblom",
"ValueType": "http://www.w3.org/2001/XMLSchema#string",
"Issuer": "OAuth2-Github",
"OriginalIssuer": "OAuth2-Github"
},
{
"Type": "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress",
"Value": "m@...",
"ValueType": "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress",
"Issuer": "OAuth2-Github",
"OriginalIssuer": "OAuth2-Github"
},
{
"Type": "urn:github:url",
"Value": "https://api.github.com/users/marcuslindblom",
"ValueType": "http://www.w3.org/2001/XMLSchema#string",
"Issuer": "OAuth2-Github",
"OriginalIssuer": "OAuth2-Github"
}
],
"Label": null,
"Name": "marcuslindblom",
"NameClaimType": "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/name",
"RoleClaimType": "http://schemas.microsoft.com/ws/2008/06/identity/claims/role"
}
]
},
"AuthenticationTokens": [
{
"Name": "access_token",
"Value": "xxx"
},
{
"Name": "token_type",
"Value": "bearer"
}
],
"AuthenticationProperties": {
"Items": {
".AuthScheme": "GitHub",
"LoginProvider": "GitHub",
".Token.access_token": "xxx",
".Token.token_type": "bearer",
".TokenNames": "access_token;token_type",
".issued": "Wed, 18 Dec 2019 19:58:11 GMT",
".expires": "Wed, 18 Dec 2019 20:03:11 GMT"
},
"Parameters": {},
"IsPersistent": false,
"RedirectUri": null,
"IssuedUtc": "2019-12-18T19:58:11.0000000+00:00",
"ExpiresUtc": "2019-12-18T20:03:11.0000000+00:00",
"AllowRefresh": null
},
"LoginProvider": "GitHub",
"ProviderKey": "319720",
"ProviderDisplayName": "GitHub"
}
],
After the first login I also get two IdentityUserAuthTokens
documents with this content.
The IdentityUserAuthTokens
documents seems to get updated with a new access_token when I run _signInManager.UpdateExternalAuthenticationTokensAsync
.
Thanks. I agree that the issue seems to be due to our fix in the other thread.
I think the best best is to rewrite this external login / token auth code to reflect what we see in other identity providers. Another option is to modify our JSON serializers. Let me explore these options more; I'll post an update here later.
I've updated the code such that user.Logins
are always updated when a new login occurs.
Also, we no longer store IdentityUserAutyhToken
as separate documents, but rather as a user.Tokens
. You may delete any IdentityUserAuthTokens
; they are not needed any longer.
I've published the update as RavenDB.Identity 7.0.1 on NuGet. Can you try it and let me know if it works for you?
It seems to work but the AuthenticationProperties
properties does not seem to be updated. This means that these properties does not match and the fact that the .expires
property is incorrect I get some strange behaviour I think.
"AuthenticationProperties": {
"Items": {
".AuthScheme": "GitHub",
"LoginProvider": "GitHub",
".Token.access_token": "xxx",
".Token.token_type": "bearer",
".TokenNames": "access_token;token_type",
".issued": "Fri, 20 Dec 2019 06:55:20 GMT",
".expires": "Fri, 20 Dec 2019 07:00:20 GMT"
},
"Parameters": {},
"IsPersistent": false,
"RedirectUri": null,
"IssuedUtc": "2019-12-20T06:55:20.0000000+00:00",
"ExpiresUtc": "2019-12-20T07:00:20.0000000+00:00",
"AllowRefresh": null
},
Besides that it seems to work.
Hmm - at our level, we handle adding a claim and setting a token. AuthenticationProperties get set during adding of a claim, IIRC. I'm not sure what more can be done. @JoshClose any idea?
I can probably take a look this weekend. For now, I would suggest looking at what EF Core is doing with it.
Hm, ok I see. Maybe it's correct then? It is strange that the expires value does not seem to be used as the real expire? Or I'm I missing something?
Marcus, I agree is super strange that AuthenticationProperties doesn't update. Looking at the code, I'd expect you to get a new item in the user.Logins list each time you login. Is that happening?
user.Tokens
is updated but not user.Logins
. The code I use to login looks like this. Am I missing something maybe? info
contains new tokens and a new expire value so I guess that await _signInManager.UpdateExternalAuthenticationTokensAsync(info)
is doing what it should?
// Sign in the user with this external login provider if the user already has a login.
var signInResult = await _signInManager.ExternalLoginSignInAsync(info.LoginProvider, info.ProviderKey, isPersistent: true);
if (signInResult.Succeeded)
{
// Clear the existing external cookie to ensure a clean login process
await _signInManager.SignOutAsync();
await _signInManager.SignInWithClaimsAsync(user, info.AuthenticationProperties, user.Claims.Select(c => new Claim(c.ClaimType, c.ClaimValue)));
// Update any authentication tokens as well
await _signInManager.UpdateExternalAuthenticationTokensAsync(info);
// We're responsible for calling .SaveChangesAsync when we're done making changes.
await _session.SaveChangesAsync();
return Redirect(returnUrl);
}
It seems like I have misunderstood what UpdateExternalAuthenticationTokensAsync
is doing. It just seems to update the cookie with updated tokens and not user.Logins. https://github.com/aspnet/AspNetCore/blob/4303bbe78650abf0bc6941d81956b02e4357feb5/src/Identity/Core/src/SignInManager.cs#L677
Maybe I need to run some additional code to update the login or maybe I don't need to update it at all. I feel that I don't really understand the hole concept here.
Does this help at all?
I think for now I'm going to close this issue, as I'm fairly confident RavenDB.Identity is doing the right thing in response to claim creation and token storage. If it turns out there's an issue here in our code, feel free to reopen this.
I use Github authentication with
SaveTokens=true
. When for the first time I get a new login on my user document. The login has bothAuthenticationTokens
andAuthenticationProperties
.In addition to that I also get
IdentityUserAuthTokens
documents andIdentityUserLogins
document.That seems fine and all but when I login a second time with the external login I run
UpdateExternalAuthenticationTokensAsync
and then it seems like it's only theIdentityUserAuthTokens
documents that gets updated and not the user propertiesAuthenticationTokens
andAuthenticationProperties
.AuthenticationProperties
contains expires and so on so I guess that the user document also should get an update onUpdateExternalAuthenticationTokensAsync
?