Reputation: 1514

C# Net Core 2.0 refactoring

Writing code for controllers could lead to repeat myself again and again.

How can reuse the code below and apply DRY principle on C# Net Core 2.0. MVC controllers?

See the below example.

The coding for getting a full list of departments using EF and web API is as follows..

[HttpGet]
public async Task<IActionResult> Department()
{

    using (var client = await _apiHttpClient.GetHttpClientAsync())
    {
        var response = await client.GetAsync("api/Department");

        if (response.IsSuccessStatusCode)
        {
            var content = await response.Content.ReadAsStringAsync();
            var dptos = JsonConvert.DeserializeObject<Department[]>(content);

            return View(dptos);
        }

        if (response.StatusCode == HttpStatusCode.Unauthorized || response.StatusCode == HttpStatusCode.Forbidden)
            return RedirectToAction("AccessDenied", "Authorization");

        throw new Exception($"A problem happened while calling the API: {response.ReasonPhrase}");
    }
}

Is indeed almost identical to get a single department..

[HttpGet]
public async Task<IActionResult> DeparmentEdit(string id)
{
    ViewData["id"] = id;

    using (var client = await _apiHttpClient.GetHttpClientAsync())
    {
        var response = await client.GetAsync($"api/Department/{id}");

        if (response.IsSuccessStatusCode)
        {
            var content = await response.Content.ReadAsStringAsync();
            var dpto = JsonConvert.DeserializeObject<Department>(content);

            return View(dpto);
        }

        if (response.StatusCode == HttpStatusCode.Unauthorized || response.StatusCode == HttpStatusCode.Forbidden)
            return RedirectToAction("AccessDenied", "Authorization");

        throw new Exception($"A problem happened while calling the API: {response.ReasonPhrase}");
    }
}

The _apiHttpClient field holds a custom implementation of an HttpClient for tokens and refreshing tokens to access the web API.

I think that IS NOT relevant here to apply refactoring and DRY but anyway I will copy his implementation here below.

BR and thanks in advance for your reply.

public class ApiHttpClient : IApiHttpClient
{
    private HttpClient _httpClient;
    private HttpClient HttpClient => _httpClient ?? (_httpClient = new HttpClient());

    private readonly IHttpContextAccessor _httpContextAccessor;

    public ApiHttpClient(IHttpContextAccessor httpContextAccessor)
    {
        _httpContextAccessor = httpContextAccessor;
    }

    public async Task<HttpClient> GetHttpClientAsync()
    {
        string accessToken;

        var context = _httpContextAccessor.HttpContext;

        var expiresAt = await context.GetTokenAsync(Constants.Tokens.ExpiresAt);                 // Get expires_at value

        if (string.IsNullOrWhiteSpace(expiresAt)                                                 // Should we renew access & refresh tokens?
            || (DateTime.Parse(expiresAt).AddSeconds(-60)).ToUniversalTime() < DateTime.UtcNow)  // Make sure to use the exact UTC date formats for comparison 
        {
            accessToken = await RefreshTokensAsync(_httpContextAccessor.HttpContext);            // Get the current HttpContext to access the tokens
        }
        else
        {
            accessToken = await context.GetTokenAsync(OpenIdConnectParameterNames.AccessToken);  // Get access token
        }

        HttpClient.BaseAddress = new Uri(Constants.Urls.ApiHost);

        if (!string.IsNullOrWhiteSpace(accessToken))
            HttpClient.SetBearerToken(accessToken);

        return HttpClient;
    }

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    protected void Dispose(bool disposing)
    {
        if (disposing)
        {
            if (_httpClient != null)
            {
                _httpClient.Dispose();
                _httpClient = null;
            }
        }
    }

    public static async Task<string> RefreshTokensAsync(HttpContext context)
    {
        var discoveryResponse = await DiscoveryClient.GetAsync(Constants.Authority);                                            // Retrive metadata information about our IDP

        var tokenClient = new TokenClient(discoveryResponse.TokenEndpoint, Constants.ClientMvc.Id, Constants.ClientMvc.Secret); // Get token client using the token end point. We will use this client to request new tokens later on

        var refreshToken = await context.GetTokenAsync(OpenIdConnectParameterNames.RefreshToken);                               // Get the current refresh token

        var tokenResponse = await tokenClient.RequestRefreshTokenAsync(refreshToken);                                           // We request a new pair of access and refresh tokens using the current refresh token

        if (tokenResponse.IsError)
            return null;            // Let's the unauthorized page bubbles up
          // throw new Exception("Problem encountered while refreshing tokens", tokenResponse.Exception);

        var expiresAt = (DateTime.UtcNow
                         + TimeSpan.FromSeconds(tokenResponse.ExpiresIn)).ToString("O", CultureInfo.InvariantCulture);         // New expires_at token ISO 860

        var authenticateResult = await context.AuthenticateAsync(CookieAuthenticationDefaults.AuthenticationScheme);           // HttpContext.Authentication.GetAuthenticateInfoAsync() deprecated

        authenticateResult.Properties.UpdateTokenValue(OpenIdConnectParameterNames.AccessToken, tokenResponse.AccessToken);    // New access_token
        authenticateResult.Properties.UpdateTokenValue(OpenIdConnectParameterNames.RefreshToken, tokenResponse.RefreshToken);  // New refresh_token 
        authenticateResult.Properties.UpdateTokenValue(Constants.Tokens.ExpiresAt, expiresAt);                                 // New expires_at token ISO 8601 WHY _at TODO

        await context.SignInAsync(CookieAuthenticationDefaults.AuthenticationScheme, authenticateResult.Principal, authenticateResult.Properties); // Signing in again with the new values, doing such a user relogin, ensuring that we change the cookies on client side. Doig so the user that has logged in has the refreshed tokens

        return tokenResponse.AccessToken;
    }

    public static async Task RevokeTokensAsync(HttpContext context)
    {
        var discoveryResponse = await DiscoveryClient.GetAsync(Constants.Authority);                                                                // Retrive metadata information about our IDP

        var revocationClient = new TokenRevocationClient(discoveryResponse.RevocationEndpoint, Constants.ClientMvc.Id, Constants.ClientMvc.Secret); // Get token revocation client using the token revocation endpoint. We will use this client to revoke tokens later on

        var accessToken = await context.GetTokenAsync(OpenIdConnectParameterNames.AccessToken);                                                     // Get the access token token to revoke

        if (!string.IsNullOrWhiteSpace(accessToken))
        {
            var revokeAccessTokenTokenResponse = await revocationClient.RevokeAccessTokenAsync(accessToken);

            if (revokeAccessTokenTokenResponse.IsError)
                throw new Exception("Problem encountered while revoking the access token.", revokeAccessTokenTokenResponse.Exception);
        }

        var refreshToken = await context.GetTokenAsync(OpenIdConnectParameterNames.RefreshToken);                                                   // Get the refresh token to revoke

        if (!string.IsNullOrWhiteSpace(refreshToken))
        {
            var revokeRefreshTokenResponse = await revocationClient.RevokeRefreshTokenAsync(refreshToken);

            if (revokeRefreshTokenResponse.IsError)
                throw new Exception("Problem encountered while revoking the refresh token.", revokeRefreshTokenResponse.Exception);
        }
    }

}

Upvotes: 0

Answers (2)

alhpe

Reputation: 1514

I had refactored the code as follows having in mind the following workflow.

We will need: a) an API service class, b) a HttpContextAccessor and c) a HttpClient.

1) DI principle!. We register them in our dependency injection container at ConfigureServices

    services
        .AddTransient<IGameApiService, GameApiService>()
        .AddSingleton<IHttpContextAccessor, HttpContextAccessor>()
        .AddSingleton(c => new HttpClient { BaseAddress = new Uri(Constants.Urls.ApiHost) });

2) The big job!. The new GameApiService will do the "heavy job" of calling our API methods. We will call the API using a "composed" request string. The API service will use our HttpClient, passing our request string and returning the response code and A STRING! (instead of using generics or other object) with the content. (I would need help on moving to generic since I fear that the registration on the dependency container will be "hard" to do with generics). (the HttpContextAccessor is used for some token methods)

public class GameApiService : IGameApiService
    {
        private readonly HttpClient _httpClient;
        private readonly HttpContext _httpContext;

        public GameApiService(HttpClient httpClient, IHttpContextAccessor httpContextAccessor)
        {
            _httpClient = httpClient;
            _httpContext = httpContextAccessor.HttpContext;

            _httpClient.AddBearerToken(_httpContext); // Add current access token to the authorization header
        }

        public async Task<(HttpResponseMessage response, string content)> GetDepartments()
        {
            return await GetAsync(Constants.EndPoints.GameApi.Department); // "api/Department"
        }

        public async Task<(HttpResponseMessage response, string content)> GetDepartmenById(string id)
        {
            return await GetAsync($"{Constants.EndPoints.GameApi.Department}/{id}"); // "api/Department/id"
        }

        private async Task<(HttpResponseMessage response, string content)> GetAsync(string request)
        {
            string content = null;

            var expiresAt = await _httpContext.GetTokenAsync(Constants.Tokens.ExpiresAt);             // Get expires_at value

            if (string.IsNullOrWhiteSpace(expiresAt)                                                  // Should we renew access & refresh tokens?
                || (DateTime.Parse(expiresAt).AddSeconds(-60)).ToUniversalTime() < DateTime.UtcNow)   // Make sure to use the exact UTC date formats for comparison 
            {
                var accessToken = await _httpClient.RefreshTokensAsync(_httpContext);                 // Try to ge a new access token

                if (!string.IsNullOrWhiteSpace(accessToken))                                          // If succeded set add the new access token to the authorization header
                    _httpClient.AddBearerToken(_httpContext);
            }

            var response = await _httpClient.GetAsync(request);

            if (response.IsSuccessStatusCode)
            {
                content = await response.Content.ReadAsStringAsync();
            }
            else if (response.StatusCode != HttpStatusCode.Unauthorized && response.StatusCode != HttpStatusCode.Forbidden)
            {
                throw new Exception($"A problem happened while calling the API: {response.ReasonPhrase}");
            }

            return (response, content);
        }

    }

public interface IGameApiService
{
    Task<(HttpResponseMessage response, string content)> GetDepartments();
    Task<(HttpResponseMessage response, string content)> GetDepartmenById(string id);
}

3) Great DRY! Our MVC controller will use this new API service as follows.. (we really don't have very much code there and THIS IS THE GOAL.. ;-) GREAT!!. We still keep the responsibility of de-serialize the content string on the controller action on which the service API method was invoked. The code for the service API looks like...

[Route("[controller]/[action]")] public class DepartmentController : Controller { private readonly IGameApiService _apiService;

public DepartmentController(IGameApiService apiService)
{
    _apiService = apiService;
}

[HttpGet]
public async Task<IActionResult> Department()
{
    ViewData["Name"] = User.Claims.FirstOrDefault(c => c.Type == JwtClaimTypes.Name)?.Value;

    var (response, content) = await _apiService.GetDepartments();

    if (!response.IsSuccessStatusCode) return Forbid();

    return View(JsonConvert.DeserializeObject<Department[]>(content));
}


[HttpGet]
public async Task<IActionResult> DepartmentEdit(string id)
{
    ViewData["id"] = id;

    var (response, content) = await _apiService.GetDepartmenById(id);

    if (!response.IsSuccessStatusCode) return Forbid();

    return View(JsonConvert.DeserializeObject<Department>(content));
}

}

4) Last trick!. To redirect to a custom page when we are not authorized or the permission has been denied we have issued if (!response.IsSuccessStatusCode) return Forbid(); yes Forbid(). But we still need to configure the default denied page on the cookie middleware. Thus on ConfigureServices we do it with services.AddAuthentication().AddCookie(AddCookie) methods, configuring the relevant options, mainly the AccessDeniedPath option as follows.

private static void AddCookie(CookieAuthenticationOptions options)
{
    options.Cookie.Name = "mgame";
    options.AccessDeniedPath = "/Authorization/AccessDenied";                           // Redirect to custom access denied page when user get access is denied

    options.Cookie.HttpOnly = true;                                                     // Prevent cookies from being accessed by malicius javascript code
    options.Cookie.SecurePolicy = CookieSecurePolicy.Always;                            // Cookie only will be sent over https
    options.ExpireTimeSpan = TimeSpan.FromMinutes(Constants.CookieTokenExpireTimeSpan); // Cookie will expire automaticaly after being created and the client will redirect back to Identity Server
}

5) A word about the HTTP Client!. It will be instantiated using a factory on the dependency injection. A new instance is created per GameApiService instance. The helper code to set the bearer token on the header and refresh the access token has been moved to a convenient extension method helper class as follows..

public static class HttpClientExtensions
    {
        public static async void AddBearerToken(this HttpClient client, HttpContext context)
        {
            var accessToken = await context.GetTokenAsync(OpenIdConnectParameterNames.AccessToken);

            if (!string.IsNullOrWhiteSpace(accessToken))
                client.SetBearerToken(accessToken);
        }

        public static async Task<string> RefreshTokensAsync(this HttpClient client, HttpContext context)
        {
            var discoveryResponse = await DiscoveryClient.GetAsync(Constants.Authority);                                                  // Retrive metadata information about our IDP

            var tokenClient = new TokenClient(discoveryResponse.TokenEndpoint, Constants.ClientMvc.Id, Constants.ClientMvc.Secret);       // Get token client using the token end point. We will use this client to request new tokens later on

            var refreshToken = await context.GetTokenAsync(OpenIdConnectParameterNames.RefreshToken);                                     // Get the current refresh token

            var tokenResponse = await tokenClient.RequestRefreshTokenAsync(refreshToken);                                                 // We request a new pair of access and refresh tokens using the current refresh token

            if (tokenResponse.IsError)                                                                                                    // Let's the unauthorized page bubbles up instead doing throw new Exception("Problem encountered while refreshing tokens", tokenResponse.Exception)
                return null;

            var expiresAt = (DateTime.UtcNow + TimeSpan.FromSeconds(tokenResponse.ExpiresIn)).ToString("O", CultureInfo.InvariantCulture); // New expires_at token ISO 860

            var authenticateResult = await context.AuthenticateAsync(CookieAuthenticationDefaults.AuthenticationScheme);                  // HttpContext.Authentication.GetAuthenticateInfoAsync() deprecated

            authenticateResult.Properties.UpdateTokenValue(OpenIdConnectParameterNames.AccessToken, tokenResponse.AccessToken);           // New access_token
            authenticateResult.Properties.UpdateTokenValue(OpenIdConnectParameterNames.RefreshToken, tokenResponse.RefreshToken);         // New refresh_token 
            authenticateResult.Properties.UpdateTokenValue(Constants.Tokens.ExpiresAt, expiresAt);                                        // New expires_at token ISO 8601

            await context.SignInAsync(CookieAuthenticationDefaults.AuthenticationScheme, authenticateResult.Principal, authenticateResult.Properties); // Signing in again with the new values, doing such a user relogin, ensuring that we change the cookies on client side. Doig so the user that has logged in has the refreshed tokens

            return tokenResponse.AccessToken;
        }


        public static async Task RevokeTokensAsync(this HttpClient client, HttpContext context)
        {
            var discoveryResponse = await DiscoveryClient.GetAsync(Constants.Authority);                                                                // Retrive metadata information about our IDP

            var revocationClient = new TokenRevocationClient(discoveryResponse.RevocationEndpoint, Constants.ClientMvc.Id, Constants.ClientMvc.Secret); // Get token revocation client using the token revocation endpoint. We will use this client to revoke tokens later on

            var accessToken = await context.GetTokenAsync(OpenIdConnectParameterNames.AccessToken);                                                     // Get the access token token to revoke

            if (!string.IsNullOrWhiteSpace(accessToken))
            {
                var revokeAccessTokenTokenResponse = await revocationClient.RevokeAccessTokenAsync(accessToken);

                if (revokeAccessTokenTokenResponse.IsError)
                    throw new Exception("Problem encountered while revoking the access token.", revokeAccessTokenTokenResponse.Exception);
            }

            var refreshToken = await context.GetTokenAsync(OpenIdConnectParameterNames.RefreshToken);                                                   // Get the refresh token to revoke

            if (!string.IsNullOrWhiteSpace(refreshToken))
            {
                var revokeRefreshTokenResponse = await revocationClient.RevokeRefreshTokenAsync(refreshToken);

                if (revokeRefreshTokenResponse.IsError)
                    throw new Exception("Problem encountered while revoking the refresh token.", revokeRefreshTokenResponse.Exception);
            }
        }

    }

Now the code after refactoring it looks more pretty and clean.. ;-)

Upvotes: 1

bri

Reputation: 3030

You could just split it up using generics. I haven't debugged this code (obviously), but I think it gets you where you need to go.

using System.Security.Authentication;

[HttpGet]
public async Task<IActionResult> Department() {
    try {
        var myObject = await GetSafeData<Department[]>("api/Department");
        return view(myObj);
    } catch(AuthenticationException ex) {
        return RedirectToAction("AccessDenied", "Authorization");
    }
}

internal T GetSafeData<T>(string url) {

    using (var client = await _apiHttpClient.GetHttpClientAsync()) {
        var response = await client.GetAsync(url);
        if (response.IsSuccessStatusCode) {
            var content = await response.Content.ReadAsStringAsync();
            return JsonConvert.DeserializeObject<T>(content);
        }
        if (response.StatusCode == HttpStatusCode.Unauthorized || response.StatusCode == HttpStatusCode.Forbidden)
            Throw New AuthenticationException("");

        throw new Exception($"A problem happened while calling the API: {response.ReasonPhrase}");
    }

}

You can sorta see how you might pass response to that same method, so you could do your AccessDenied redirect within that method as well and reduce your repetitive code everywhere.

It's a generic method, so you can use it for ANY call to that api. That should be enough to get you started. Hope it helps!

Upvotes: 0

C# Net Core 2.0 refactoring

Answers (2)

Related Questions