Closed wagich closed 3 years ago
@wagich Thanks for this! What is the backwards compatibility story here - if I apply this change, will it break existing databases?
@JudahGabriel This should be fully backwards compatible IMHO. It's just enforcing the (previously implicit) constraint that Roles
always is a List<string>
.
I think this worked in previous releases of Newtonsoft.Json because it always chose to use a List<T>
when deserializing into IReadOnlyList<T>
and somewhere along the way Newtonsoft.Json began using a more applicable type for this interface which broke the assumption.
I've deployed your fix to RavenDB.Identity 8.0.7. 👍
When adding a user to roles with
UserManager.AddToRolesAsync
, I get an exception (see stacktrace below). I tracked it down to theIdentityUser
implementation which exposesIReadOnlyList<string> Roles
but assumes the underlying implementation to be assignable toList<string>
. When deserializing the type from RavenDB, the serializer seems to create aReadOnlyCollection<string>
(to best match the interface contract) which violates this assumption.This PR makes sure that
IdentityUser.Roles
is always backed by aList<string>
.