brockallen / BrockAllen.MembershipReboot

MembershipReboot is a user identity management and authentication library.
Other
742 stars 238 forks source link

Database First pattern #94

Closed rducom closed 10 years ago

rducom commented 11 years ago

Hi,

I have difficulty implementing a "database-first" scenario with the way UserAccount is designed : the internal constructor make me feels it's pretty impossible...

As I have understood your code, the extensibility point is the IRepository interface. But since the UserAccount constructor is internal, I can't create items in a separate project implementing IRepository. On another side, it seems impossible to make a CustomUserAccount class which inherits UserAccount.

Is this choice (the internal constructor) a security concern ? How can I implement a for example a XML repository with this constraint ?

Thanks in advance, Bests

brockallen commented 11 years ago

Yep, this is a very valid point. Are you not using EF? I'm happy to work with you to get a solution -- can you describe your scenario a little more?

One approach would be to derive from UserAccount, but I don't know if there will be more issues beyond this one.

rducom commented 11 years ago

If I inherits from UserAccount, the problem would be the same : If I create a CustomAccountClass, I can't implement a parameter-less CustomAccountClass constructor...

My scenario is a little specific : I work in a enterprise context, developing some enterprise back-offices, deploying apps on customers Active's Directory, and I intend to make a cross application authentication system, lettings users to log in against their active directory, and in the same time, lettings some external users to log in the same apps but with few rights, so I need to have a link between active directory accounts and Google/Facebook account. The goal is to build a system letting user log from anywhere, and let users go from on application to any other with login again (a form of SSO, cross authentication platform).

brockallen commented 11 years ago

Ok, so I spent some time thinking about this.

My intent with locking them all down was to try to ensure that they weren't corrupted in anyway, but since I'm trying to let someone plug in a custom repository there's no guarantee that the values wouldn't get corrupted (consciously or not) in the database.

I suspect opening up the ctor to public and marking all the properties as protected would help your scenario, yes?

christolo commented 11 years ago

I suspect that a public constructor would help. How I got around this while messing around for RavenDb was to create a proxy at runtime for T.

    #region Create Proxies

    private static T CreateObject<T>()
    {
        var obj = CreateConstructor<T>() as Func<T>;
        return obj != null ? obj() : default(T);
    }

    private static ConstructorInfo GetConstructorForType(Type type)
    {
        ConstructorInfo constructor = type.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.CreateInstance, null, Type.EmptyTypes, null);
        if (null == constructor)
        {
            var message = string.Format("No constructor available for type '{0}'.", type);
            throw new Exception(message);
        }
        return constructor;
    }

    private static Delegate CreateConstructor<T>()
    {
        ConstructorInfo constructorForType = GetConstructorForType(typeof (T));
        DynamicMethod dynamicMethod = CreateDynamicMethod(constructorForType.DeclaringType.Name, typeof(T), Type.EmptyTypes);
        ILGenerator iLGenerator = dynamicMethod.GetILGenerator();
        iLGenerator.Emit(OpCodes.Newobj, constructorForType);
        iLGenerator.Emit(OpCodes.Ret);
        return dynamicMethod.CreateDelegate(typeof(Func<T>));
    }

    private static DynamicMethod CreateDynamicMethod(string name, Type returnType, Type[] parameterTypes)
    {
        return new DynamicMethod(name, returnType, parameterTypes, true);
    }

    #endregion Create Proxies
brockallen commented 11 years ago

@christolo yea, i saw that. i guess opening up the ctor will work.

@raphsharp -- any other thoughts?

brockallen commented 11 years ago

Ok, I opened up the ctor: https://github.com/brockallen/BrockAllen.MembershipReboot/commit/6f5044e7d76384d5c955bff5ff0b32764e5a899b

HTH

rducom commented 11 years ago

Yes, one other thought one the same subject : Maybe it should be easier to implement extensibility over the "core" structure by replacing the IUserAccountRepository and IGroupRepository by theses generic's versions :

public interface IGroupRepository < T > : IRepository< T > where T : Group { }

public interface IUserAccountRepository< T > : IRepository< T > where T : UserAccount { }

brockallen commented 11 years ago

@raphsharp Isn't that what the code is already doing?

public interface IUserAccountRepository : IRepository { }

public interface IRepository : IDisposable where T : class { IQueryable GetAll(); T Get(params object[] keys); T Create(); void Add(T item); void Remove(T item); void Update(T item); }

rducom commented 11 years ago

For example the UserAccountService class exposes virtual methods, so the goal is to inherits this class. Actually this is the class declaration :

public class UserAccountService : IDisposable { public virtual void Update(UserAccount account) { } }

If I want to use my CustomUserAccount class with a CustomUserAccountService , it should be easier to inherits from : public class UserAccountService< T > : IDisposable where T : AccountAccount { public virtual void Update(T account) { } }

by this way, I can write : public class CustomUserAccountService ; UserAccountService< CustomUserAccount > { public override void Update(CustomUserAccount account) { base.Update(account); ............. } }

and a simple public class UserAccountService : UserAccountService< UserAccount > { } let a all the code be compatible with previous versions.

brockallen commented 11 years ago

Ah, so not the repository, but the service itself you want to customize. Or rather, both.

Do you have some specific use-cases that can help me think about this?

rducom commented 11 years ago

Yes, the services themselves : the User's and the Group's ones.

I actually work on a "solution" witch includes a lot of components (apps) : some back and front offices in MVC, Silvelight, or WPF/WCF. Here are the constraints and the goals I have :

brockallen commented 11 years ago

@raphsharp just to let you know, i've been thinking about this issue. Also coincidentally another person has emailed me with some thoughts somewhat related to some of these ideas. So I've not forgotten about this. My main constraint is finding a day to prototype some of it.

rducom commented 11 years ago

No PB. I also miss free time to work on this project, but be sure I'll contact you as soon as I'll start to build it. Bests,

brockallen commented 10 years ago

Ok, time for feedback -- I spent the entire day working on making MR have better support for custom user account classes and for custom properties on the user account class. While I wasn't able to pull off a pure interface based approach, I did end up with an ok approach where it's easy to add custom properties to your own user account table and the user account service is a generic class based upon your custom user account type.

This is prototyped on a branch: https://github.com/brockallen/BrockAllen.MembershipReboot/tree/poco

The customization sample is the best example of it in use: https://github.com/brockallen/BrockAllen.MembershipReboot/tree/poco/samples/CustomizationsSample

Please look over this and provide feedback and let me know if you think this is a move in the right direction and if it satisfies your requirements. Thanks!

brockallen commented 10 years ago

Ok, closing this now given that the rework is mostly done.