Separate logic from the controller and use a class library instead

Question

I am trying to implement OWIN into my application but i hate having login in controllers, so I decided to try to and move the login into my provider.

My controller method looks like this:

[OverrideAuthentication]
[HostAuthentication(DefaultAuthenticationTypes.ExternalCookie)]
[AllowAnonymous]
[Route("externallogin", Name = "ExternalLogin")]
public async Task<IHttpActionResult> GetExternalLoginAsync(string provider, string error = null) => await _userProvider.GetExternalLoginAsync(this, User.Identity, provider, error);

and in my UserProvider I have changed the code to this:

public async Task<IHttpActionResult> GetExternalLoginAsync(ApiController controller, IIdentity identity, string provider, string error = null)
{

    // If we have an error, return it
    if (error != null) throw new Exception(Uri.EscapeDataString(error));

    // If the current user is not authenticated
    if (!identity.IsAuthenticated) return new ChallengeResult(provider, controller);

    // Validate the client and the redirct url
    var redirectUri = await ValidateClientAndRedirectUriAsync(controller.Request);

    // If we have no validated
    if (!string.IsNullOrWhiteSpace(redirectUri)) throw new Exception(redirectUri);

    // Get the current users external login
    var externalLogin = GetExternalLoginDataFromIdentity(identity as ClaimsIdentity);

    // If the current user has no external logins, throw an error
    if (externalLogin == null) throw new ObjectNotFoundException();

    // if the login provider doesn't match the one specificed
    if (externalLogin.LoginProvider != provider)
    {

        // Sign the user out
        AuthenticationManager(controller.Request).SignOut(DefaultAuthenticationTypes.ExternalCookie);

        // Challenge the result
        return new ChallengeResult(provider, controller);
    }

    // Get the user
    var user = await FindAsync(new UserLoginInfo(externalLogin.LoginProvider, externalLogin.ProviderKey));

    // Have they registered
    bool hasRegistered = user != null;

    // Update our url
    redirectUri += $"{redirectUri}#external_access_token={externalLogin.ExternalAccessToken}&provider={externalLogin.LoginProvider}&haslocalaccount={hasRegistered.ToString()}&external_user_name={externalLogin.UserName}";

    // Return our url
    return new RedirectResult(redirectUri);
}

This wasn't too much of a problem for this particular method, but I would like to be able to use Ok(), BadRequest() and InternalServerError() methods of the controller. Also, I would like to use ModelState.IsValid. The latter can be used by passing the controller into the method and I can simply do:

controller.ModelState.IsValid

But the other methods are internal methods, so I would have to inherit the ApiController in my Provider which I can't do or find another way of doing this.

I found this on stackoverflow which made me think I could do something similar as I was already passing the controller to the method, so I created a static Extension class like this:

public static class ResponseExtensions
{
    public static HttpResponseMessage Ok(T content) => new HttpResponseMessage(HttpStatusCode.Accepted, content);
}

In my method I was hoping to do something like this:

return controller.ResponseMessage(ResponseExtensions.Ok());

But the same issue arises in that ResponseMessage is also a protected internal method.

Has anyone come across this before and can give me some direction in how to solve this?

score 2 · Answer 1 · answered Apr 17 '17 at 14:16

First and foremost, you don't have "login" in your controllers. The actual authentication and authorization is already sufficiently abstracted. All you have in your controller is the calls to make that stuff happen which is a perfectly valid thing for controllers to have. The only reason to abstract further would be if you needed to do something like sign in in multiple different controllers. Otherwise, you're just wasting your time.

Second, you don't need a controller instance to return various result types. The methods on Controller like Ok, BadRequest, etc. are just convenience methods. They each individually return ActionResult subclasses, i.e. OkResult, BadRequestResult, etc. If your "provider" needs to return action results, just return new instances of these classes.

score 0 · Answer 2 · answered Apr 17 '17 at 13:46

I think the way to do this is to use (argh) method hiding. I created a BaseApiController:

public class BaseApiController : ApiController
{
    public IHttpActionResult Ok<T>(T content) => Ok<T>(content);
    public IHttpActionResult Ok() => Ok();
    public IHttpActionResult InternalServerError() => InternalServerError();
    public IHttpActionResult InternalServerError(Exception exception) => InternalServerError(exception);
    public IHttpActionResult BadRequest() => BadRequest();
    public IHttpActionResult BadRequest(ModelStateDictionary modelState) => BadRequest(modelState);
    public IHttpActionResult BadRequest(string message) => BadRequest(message);
    public IHttpActionResult Redirect(Uri location) => Redirect(location);
    public IHttpActionResult Redirect(string location) => Redirect(location);
}

Then my UserController simply inherits this base controller:

public class UsersController : BaseApiController

and then I can change my Provider method to this:

public async Task<IHttpActionResult> GetExternalLoginAsync(BaseApiController controller, IIdentity identity, string provider, string error = null)
{

    // If we have an error, return it
    if (error != null) return controller.BadRequest(Uri.EscapeDataString(error));

    // If the current user is not authenticated
    if (!identity.IsAuthenticated) return new ChallengeResult(provider, controller);

    // Validate the client and the redirct url
    var redirectUri = await ValidateClientAndRedirectUriAsync(controller.Request);

    // If we have no validated
    if (!string.IsNullOrWhiteSpace(redirectUri)) return controller.BadRequest(redirectUri);

    // Get the current users external login
    var externalLogin = GetExternalLoginDataFromIdentity(identity as ClaimsIdentity);

    // If the current user has no external logins, throw an error
    if (externalLogin == null) return controller.InternalServerError();

    // if the login provider doesn't match the one specificed
    if (externalLogin.LoginProvider != provider)
    {

        // Sign the user out
        AuthenticationManager(controller.Request).SignOut(DefaultAuthenticationTypes.ExternalCookie);

        // Challenge the result
        return new ChallengeResult(provider, controller);
    }

    // Get the user
    var user = await FindAsync(new UserLoginInfo(externalLogin.LoginProvider, externalLogin.ProviderKey));

    // Have they registered
    bool hasRegistered = user != null;

    // Update our url
    redirectUri += $"{redirectUri}#external_access_token={externalLogin.ExternalAccessToken}&provider={externalLogin.LoginProvider}&haslocalaccount={hasRegistered.ToString()}&external_user_name={externalLogin.UserName}";

    // Return our url
    return controller.Redirect(redirectUri);
}

this seems to work. If there are any better solutions I would be interested as I don't really want to send the entire controller to my method.

Andrii Litvinov · Answer 3 · 2017-04-17T14:32:43.120

Alternatively, if you know that you need to use Redirect method you can pass redirect lambda to your method. You can also pass all the data from controller you would like to use. Or create an object that will contain those lambdas as properties:

class Context
{
    Func<string, IHttpActionResult> Redirect { get; set; }
    ...
}

Action method:

var context = new Context {Redirect = this.Redirect};
return UserProvider.GetExternalLoginAsync(..., context)

In Provider:

return context.Redirect(redirectUri);

It can be a workaround, but I would rather reconsider your services and their responsibilities.

Separate logic from the controller and use a class library instead

3 Answers3