Skip to content
This repository has been archived by the owner on Dec 20, 2018. It is now read-only.

Expose internals of UserManager and UserStore functions and properties to help implementors #596

Closed
slaneyrw opened this issue Oct 20, 2015 · 8 comments

Comments

@slaneyrw
Copy link

As I extended aspects of the UserManager and UserStore object I keep having to re-implement private functions. Can you please make all private ( and maybe internal ) functions and properties available to protected scope as well.

i.e.

UserManager<TUser>.CancellationToken { get; }
UserManager<TUser>.GetClaimStore()
UserManager<TUser>.UpdateSecurityStampInternal(TUser)
UserManager<TUser>.ThrowIfDisposed()

UserStore<TUser, TRole, TContext, TKey>.SaveChanges(CancellationToken)
UserStore<TUser, TRole, TContext, TKey>.ThrowIfDisposed()

I can't really see any reason to hide this stuff from sub classes and just makes life easier.

The private static NewSecurityStamp() function is ONLY accessed from an instance function, as as it is static it cannot be replaced ( ie with a cryptographically security implementation - it is used to create tokens after all ). Please make it protected virtual.

@slaneyrw slaneyrw changed the title Expose internals of UserManager and UserStore function to help implementors Expose internals of UserManager and UserStore functions and properties to help implementors Oct 20, 2015
@slaneyrw
Copy link
Author

Happy to submit a pull request if you want me to do the leg work

@slaneyrw
Copy link
Author

Further more this will allow per-user PasswordHashing, as at the moment it's a one size fits all.

Say I want to increase the number of iterations for password hashing over time ( as hardware power increases ). I would need to store the number of iterations last used, read that off the user record and verify the password with that number of iterations. If the number of iterations is lower than currently configured, rehash the password and store the updated information

@rustd
Copy link

rustd commented Oct 21, 2015

Sure. Please send a PR so we can look at all places where you need this.
Also consider replacing services as a way to extend Identity.

@HaoK HaoK added this to the Backlog milestone Oct 26, 2015
@slaneyrw
Copy link
Author

Had a first cut, but I'm getting errors doing a package restore in the IdentitySample.MVC project, and also getting a exception from DNX when discovering tests. I can't verify any of my changes apart from a compile. Any ideas on what is wrong ?

I have Installed Beta8 tooling and runtime

DNX Core 5.0 References

NotFound https://api.nuget.org/v3-flatcontainer/microsoft.extensions.optionsmodel/index.json 994ms
NotFound https://api.nuget.org/v3-flatcontainer/microsoft.extensions.configuration.json/index.json 1008ms
NotFound https://api.nuget.org/v3-flatcontainer/microsoft.extensions.configuration.environmentvariables/index.json 1037ms
NotFound https://api.nuget.org/v3-flatcontainer/microsoft.extensions.logging.nlog/index.json 963ms
NotFound https://api.nuget.org/v3-flatcontainer/microsoft.extensions.logging.console/index.json 1021ms
NotFound https://api.nuget.org/v3-flatcontainer/entityframework.microsoftsqlserver/index.json 1599ms

------ Test started: Project: Microsoft.AspNet.Identity.EntityFramework.InMemory.Test ------
Starting Microsoft.Dnx.TestHost [C:\Users\slaneyr.dnx\runtimes\dnx-clr-win-x86.1.0.0-beta8\bin\dnx.exe --appbase "C:\Users\slaneyr\Documents\GitHub\ASP.NET\Identity\test\Microsoft.AspNet.Identity.EntityFramework.InMemory.Test" Microsoft.Dnx.ApplicationHost --port 3014 Microsoft.Dnx.TestHost --port 3071 --parentProcessId 218492]

System.NullReferenceException: Object reference not set to an instance of an object.
at Microsoft.Dnx.TestHost.Program.Main(String[] args)

@slaneyrw
Copy link
Author

I have the follow changeset in my fork you can look at

slaneyrw@f2722c9

@bprentice
Copy link

I am also caught up here in looking at implementing a custom UserManager in order to extend
public virtual Task<TUser> FindByIdAsync(int companyCode, string userId) {...}
and
public virtual Task<TUser> FindByNameAsync(int companyCode, string userName) {...}
(Reason for this is to enable me to manage a custom multitenant implementation).
But I am also constantly running into issues with the private members of UserManager:
private CancellationToken CancellationToken => _context?.RequestAborted ?? CancellationToken.None;

and the fact that
ThrowIfDisposed() Method is set to private.

If these methods were made protected, then extending UserManager would be much easier to create functions that follow common patterns that exist in UserManager Code

@luisgizirian
Copy link

@bprentice, I'm on a similar situation where I have a Tenant discovered at Middleware stage (with its int key I'm able to inject) on which I'd like to base at least some User activities (creating, registering, finding -by name or email-, login, to say the least).

One choice, seems to be extending and injecting custom UserManager, UserStore, SignInManager. But as it is (lots of privates), seems really anti-DRY.

Did you continue digging on a solution for this? Found any?

@HaoK
Copy link
Member

HaoK commented Nov 17, 2016

Done as part of #798

@HaoK HaoK closed this as completed Nov 17, 2016
@HaoK HaoK modified the milestones: 1.1.0, Backlog Nov 17, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants