Skip to content
This repository has been archived by the owner on Sep 18, 2021. It is now read-only.

Login by Username or Email #1108

Closed
dzolnjan opened this issue Mar 23, 2015 · 8 comments
Closed

Login by Username or Email #1108

dzolnjan opened this issue Mar 23, 2015 · 8 comments

Comments

@dzolnjan
Copy link

Default setup supports login by Username. Is combined input (username or email) possible?

@leastprivilege
Copy link
Member

Sure - your user service can interpret the username field in any way you like.

@dzolnjan
Copy link
Author

Yes that makes sense, thanks for clarification.

In case/sample of MembershipReboot sample would making this simple override in CustomUserAccountService be enough to enable combined login?

https://github.com/IdentityServer/IdentityServer3.Samples/blob/master/source/MembershipReboot/SelfHost/MR/CustomUser.cs

    // override to enable auth with email
    public override bool Authenticate(string tenant, string username, string password, out CustomUser account)
    {
        var withEmail = AuthenticateWithEmail(tenant, username, password, out account);

        if (withEmail)
            return true;

        return base.Authenticate(tenant, username, password, out account);
    }

@brockallen
Copy link
Member

Yep - that's the point of that virtual.

@brockallen
Copy link
Member

Also I think there's already an API on MR called AuthenticateWithUsernameOrEmail IIRC.

@dzolnjan
Copy link
Author

I found the AuthenticateWithUsernameOrEmail and override works with that directly.

    public override bool Authenticate(string tenant, string username, string password, out CustomUser account)
    {
        return AuthenticateWithUsernameOrEmail(tenant, username, password, out account);
    }

Note that if in config EmailIsUsername = true
then AuthenticateWithUsernameOrEmail causes stackOverFlow exception
so my initial override might be safer in general..

@brockallen
Copy link
Member

Do you mind opening an issue in the MR repo for that? If there's a bug, I'd like to fix it. Thx.

@dzolnjan
Copy link
Author

It is only an issue if you try to make the override like this

    public override bool Authenticate(string tenant, string username, string password, out CustomUser account)
    {
        return AuthenticateWithUsernameOrEmail(tenant, username, password, out account);
    }

combined with

Config.EmailIsUsername = true;

simply because AuthenticateWithUsernameOrEmail calls Authenticate internally causing the call loop.

So I'm not sure if you want this filed as issue in MR.


If you really want this override possible in MR
https://github.com/brockallen/BrockAllen.MembershipReboot/blob/master/src/BrockAllen.MembershipReboot/AccountService/UserAccountService.cs

simply replace this method definition:

    public virtual bool AuthenticateWithUsernameOrEmail(string tenant, string userNameOrEmail, string password, out TAccount account)
    {
        account = null;

        if (Configuration.EmailIsUnique == false)
        {
            throw new InvalidOperationException("AuthenticateWithUsernameOrEmail can't be used when EmailIsUnique is false");
        }

        if (!Configuration.MultiTenant)
        {
            Tracing.Verbose("[UserAccountService.AuthenticateWithUsernameOrEmail] applying default tenant");
            tenant = Configuration.DefaultTenant;
        }

        Tracing.Information("[UserAccountService.AuthenticateWithUsernameOrEmail] called {0}, {1}", tenant, userNameOrEmail);

        if (String.IsNullOrWhiteSpace(tenant)) return false;
        if (String.IsNullOrWhiteSpace(userNameOrEmail)) return false;
        if (String.IsNullOrWhiteSpace(password))
        {
            Tracing.Error("[UserAccountService.AuthenticateWithUsernameOrEmail] failed -- empty password");
            return false;
        }

        if (!Configuration.EmailIsUsername && userNameOrEmail.Contains("@"))
        {
            Tracing.Verbose("[UserAccountService.AuthenticateWithUsernameOrEmail] email detected");
            return AuthenticateWithEmail(tenant, userNameOrEmail, password, out account);
        }
        else
        {
            Tracing.Verbose("[UserAccountService.AuthenticateWithUsernameOrEmail] username detected");
            return Authenticate(tenant, userNameOrEmail, password, out account);
        }
    }

with

    public virtual bool AuthenticateWithUsernameOrEmail(string tenant, string userNameOrEmail, string password, out TAccount account)
    {
        account = null;

        if (Configuration.EmailIsUnique == false)
        {
            throw new InvalidOperationException("AuthenticateWithUsernameOrEmail can't be used when EmailIsUnique is false");
        }

        if (!Configuration.MultiTenant)
        {
            Tracing.Verbose("[UserAccountService.AuthenticateWithUsernameOrEmail] applying default tenant");
            tenant = Configuration.DefaultTenant;
        }

        Tracing.Information("[UserAccountService.AuthenticateWithUsernameOrEmail] called {0}, {1}", tenant, userNameOrEmail);

        if (String.IsNullOrWhiteSpace(tenant)) return false;
        if (String.IsNullOrWhiteSpace(userNameOrEmail)) return false;
        if (String.IsNullOrWhiteSpace(password))
        {
            Tracing.Error("[UserAccountService.AuthenticateWithUsernameOrEmail] failed -- empty password");
            return false;
        }

        return AuthenticateWithEmail(tenant, userNameOrEmail, password, out account) || Authenticate(tenant, userNameOrEmail, password, out account);
    }

Only bottom part is modified (ignoring EmailIsUsername and '@' check). All tests run green in MR with the change.

@leastprivilege
Copy link
Member

@brock will open an issue in MR for that.

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

3 participants