1
\$\begingroup\$

I'm working with a ASP.NET Core (v2.2) Web Api project and implemented some APIs. However, I'm facing performance issues, hence trying to optimizing the code. Here is the login API code.

[ValidateModel]
[AllowAnonymous]
[HttpPost("login")]
public async Task<IActionResult> Login([FromBody]UserLoginDto loginDto)
{
    var model = loginDto.ToModel();
    model.Password = ApiSecurityHelper.Decrypt(model.Password, securitySecret);

    var user = await _userService.GetUser(model);
    if (user == null)
    {
        return BadRequest(T("Account.InvalidLogin"));
    }

    var ldapUserInfo = await CheckLdapUserByEmail(model.Email);
    if (ldapUserInfo.IsLdapUser)
    {
        model.LdapUserName = ldapUserInfo.LdapUserName;
        model.LdapDomainName = ldapUserInfo.LdapDomainName;

        await _userService.LdapUserLogin(model, user);
        model.IsLdapUser = user.IsLdapLogin.Equals(LdapCodes.LOGIN_SUCCESS);        
    }

    if (user.EmailConfirmed == null || user.EmailConfirmed == false)
    {
        if (string.IsNullOrEmpty(user.PasswordHash))
        {
            return BadRequest(T("Account.VerifyUser"));
        }
        return BadRequest(T("Constants.KindlyActivateYourAccountToLogin"));
    }

    if (!user.IsLdapLogin.Equals(LdapCodes.LOGIN_SUCCESS))
    {
        if (user.IsLdapLogin.Equals(LdapCodes.ERROR_INVALID_CREDENTIALS) || user.IsLdapLogin.Equals(LdapCodes.ERROR_PASSWORD_EXPIRED_1) || user.IsLdapLogin.Equals(LdapCodes.ERROR_PASSWORD_EXPIRED_2))
        {
            await _activityLogService.AddActivity<object>(null, user.UserId, user.FullName,
                              ActionType.InvalidCredentials, ModuleType.Account, null, null, null);

            return BadRequest(T("Account.InvalidLogin"));
        }
        if (user.IsLdapLogin.Equals(LdapCodes.NOT_USER))
        {
            return BadRequest(T("Account.UserNotExist"));
        }
    }

    var userDto = user.ToDto();

    //generate access token and refresh token and save refresh token to the db
    userDto.Token = ApiTokenHelper.GenerateAccessToken(user, _tokenSecret, _tokenExpiryMinutes);
    userDto.RefreshToken = ApiTokenHelper.GenerateRefreshToken();
    await _userService.SaveApiRefreshToken(userDto.UserId, userDto.RefreshToken);

    //add user activity in CRM
    await CreateUpdateCrmUser(user, user.CountryId);

    return Success(userDto);
}

We have requirement that user within the organization should able to login using their system password, for the rest, they should able to login using the password they have set.

T is the base controller method, it is fetching resource string from the cache.

Is there a better, cleaner way to handle this?

EDIT: Added requested coded

protected IActionResult BadRequest(string errorMessage)
{
    return Error(HttpStatusCode.BadRequest, errorMessage);
}

protected IActionResult Error(HttpStatusCode statusCode, string errorMessage = "")
{
    var errorsRootObject = new ErrorsRootObject()
    {
        Error = errorMessage
    };

    var errorsJson = _jsonFieldsSerializer.Serialize(errorsRootObject, null);
    return new ErrorActionResult(errorsJson, statusCode);
}

public class ErrorActionResult : IActionResult
{
    private readonly string _jsonString;
    private readonly HttpStatusCode _statusCode;
    public ErrorActionResult(string jsonString, HttpStatusCode statusCode)
    {
        _jsonString = jsonString;
        _statusCode = statusCode;
    }

    public Task ExecuteResultAsync(ActionContext context)
    {        
        var response = context.HttpContext.Response;

        response.StatusCode = (int)_statusCode;
        response.ContentType = Constants.applicationslashjson;

        using (TextWriter writer = new HttpResponseStreamWriter(response.Body, Encoding.UTF8))
        {
            writer.Write(_jsonString);
        }

        return Task.CompletedTask;
    }
}

[JsonObject(Title = "UserLoginDto")]
public class UserLoginDto
{
    [JsonProperty("email")]
    public string Email { get; set; }

    [JsonProperty("password")]
    public string Password { get; set; }
}

public class LoginViewModel
{
    public string Email { get; set; }

    [DataType(DataType.Password)]
    public string Password { get; set; }        
    public bool RememberMe { get; set; }
    public string PhoneNumber { get; set; }
    public string Otp { get; set; }
    public string ReturnUrl { get; set; }
    //Add Lat-Long parameters & UserAgent
    public decimal UserLatitude { get; set; }
    public decimal UserLongitude { get; set; }
    public string UserAgent { get; set; }
    public string UserTimeZone { get; set; }
    public string Token { get; set; }
    public bool TwoFactorEnabled { get; set; }
    public string MobileNumberBlockSpanMessage { get; set; }
    public string ErrorMessage { get; set; }
    public LdapCodes IsLdapLogin { get; set; }
    public bool IsLdapUser { get; set; }
    public string LdapDomainName { get; set; }
    public string LdapUserName { get; set; }

    [HiddenInput]
    public string ExternalAppName { get; set; }

    [HiddenInput]
    public string ExternalAppToken { get; set; }
}
\$\endgroup\$
  • 2
    \$\begingroup\$ Welcome to code review, it would be better if the whole class was included but we need to definitely see what BadRequest() and model are. \$\endgroup\$ – pacmaninbw Feb 28 at 14:18
  • \$\begingroup\$ Thanks @pacmaninbw, the whole controller contain a lot of code though, will try to add required information. \$\endgroup\$ – Div Feb 29 at 17:23
  • 1
    \$\begingroup\$ @pacmaninbw Added required information \$\endgroup\$ – Div Mar 2 at 6:46
  • \$\begingroup\$ If you have a performance issues, we have to know what _userService.GetUser(model), CheckLdapUserByEmail(model.Email) and _userService.LdapUserLogin(model, user) do. Maybe put some logging into your code to narrow longest running method. \$\endgroup\$ – Peska Mar 3 at 14:06

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.