Safi Mustafa
Safi Mustafa

Reputation: 517

Marker Interface To avoid Generic Controllers & Constructor with generic paramaters

I have overridden default IdentityUser and UserStore provided by Microsoft Identity.

    public class ApplicationUser<TIdentityKey, TClientKey> : IdentityUser<TIdentityKey>, IApplicationUser<TIdentityKey, TClientKey>
    where TIdentityKey : IEquatable<TIdentityKey>
    where TClientKey : IEquatable<TClientKey>
    {
        public TClientKey TenantId { get; set; }
    }

    public class ApplicationUserStore<TUser, TRole, TIdentityKey, TClientKey> : UserStore<TUser, TRole, IdentityServerDbContext<TIdentityKey, TClientKey>, TIdentityKey>
    where TUser : ApplicationUser<TIdentityKey, TClientKey>
    where TRole : ApplicationRole<TIdentityKey>
    where TIdentityKey : IEquatable<TIdentityKey>
    where TClientKey : IEquatable<TClientKey>
    {
        private readonly IdentityServerDbContext<TIdentityKey, TClientKey> _context;
        private readonly ITenantService<TIdentityKey, TClientKey> _tenantService;
        public ApplicationUserStore(IdentityServerDbContext<TIdentityKey, TClientKey> context, ITenantService<TIdentityKey, TClientKey> tenantService) : base(context)
        {
            _context = context;
            _tenantService = tenantService;
        }
        public async override Task<IdentityResult> CreateAsync(TUser user, CancellationToken cancellationToken = default)
        {
            user.TenantId = await GetTenantId();
            bool combinationExists = await _context.Users
            .AnyAsync(x => x.UserName == user.UserName
                        && x.Email == user.Email
                        && x.TenantId.Equals(user.TenantId));
    
            if (combinationExists)
            {
                var IdentityError = new IdentityError { Description = "The specified username and email are already registered" };
                return IdentityResult.Failed(IdentityError);
            }
    
            return await base.CreateAsync(user);
        }
        
        private async Task<TClientKey> GetTenantId()
        {
            var tenant = await _tenantService.GetCurrentTenant();
            if (tenant == null)
                return default(TClientKey);
            else
                return tenant.Id;
        }
    }

I have made these inside a class library and imported into different projects. So that I can provide different Keys for user such as Guid, int, string based on project needs. The problem I am facing is that when I try to use these in Identity Pages such as ConfirmPassword Page I need to specify Generic in model so that I can control it using dependency injection.

    public class ConfirmEmailModel<TIdentityKey,TClientKey> : PageModel
    where TIdentityKey:IEqutable<TIdentityKey>
    where TClientKey:IEqutable<TClientKey>
    {
        private readonly UserManager<ApplicationUser<TIdentityKey,TClientKey>> _userManager;

        public ConfirmEmailModel (UserManager<ApplicationUser<TIdentityKey,TClientKey>> userManager)
        {
            _userManager = userManager;
        }

        [TempData]
        public virtual string StatusMessage { get; set; }

        public virtual async Task<IActionResult> OnGetAsync(string userId, string code)
        {
            if (userId == null || code == null)
            {
                return RedirectToPage("/Index");
            }

            var user = await _userManager.FindByIdAsync(userId);
            if (user == null)
            {
                return NotFound($"Unable to load user with ID '{userId}'.");
            }

            code = Encoding.UTF8.GetString(WebEncoders.Base64UrlDecode(code));
            var result = await _userManager.ConfirmEmailAsync(user, code);
            StatusMessage = result.Succeeded ? "Thank you for confirming your email." : "Error confirming your email.";
            return Page();
        }
    }

When I specify Generic Type like this. I cant use it inside razor pages as razor pages does not support generic types.

@page
@model ConfirmEmailModel<T>// SYNTAX ERROR
@{
    ViewData["Title"] = "Confirm email";
}

<h1>@ViewData["Title"]</h1>

Other problem is when I try to use SignInManager or UserStore inside controller. I again cant use dependency injection to inject generics in places

Public class BaseUserInfoController<TIdentityKey,TClientKey> : Controller
where TIdentityKey:IEqutable<TIdentityKey>
where TClientKey:IEqutable<TClientKey>

    {
        private readonly UserManager<ApplicationUser<TIdentityKey,TClientKey>> _userManager;

        public BaseUserInfoController(UserManager<ApplicationUser<TIdentityKey,TClientKey>> userManager)
            => _userManager = userManager;

        //
        // GET: /api/userinfo
        [Authorize(AuthenticationSchemes = OpenIddictServerAspNetCoreDefaults.AuthenticationScheme)]
        [HttpGet("~/connect/userinfo"), HttpPost("~/connect/userinfo"), Produces("application/json")]
        public virtual async Task<IActionResult> Userinfo()
        {
            var user = await _userManager.GetUserAsync(User);
            if (user == null)
            {
                return Challenge(
                    authenticationSchemes: OpenIddictServerAspNetCoreDefaults.AuthenticationScheme,
                    properties: new AuthenticationProperties(new Dictionary<string, string>
                    {
                        [OpenIddictServerAspNetCoreConstants.Properties.Error] = Errors.InvalidToken,
                        [OpenIddictServerAspNetCoreConstants.Properties.ErrorDescription] =
                            "The specified access token is bound to an account that no longer exists."
                    }));
            }

            var claims = new Dictionary<string, object>(StringComparer.Ordinal)
            {
                // Note: the "sub" claim is a mandatory claim and must be included in the JSON response.
                [Claims.Subject] = await _userManager.GetUserIdAsync(user)
            };

            if (User.HasScope(Scopes.Email))
            {
                claims[Claims.Email] = await _userManager.GetEmailAsync(user);
                claims[Claims.EmailVerified] = await _userManager.IsEmailConfirmedAsync(user);
            }

            if (User.HasScope(Scopes.Phone))
            {
                claims[Claims.PhoneNumber] = await _userManager.GetPhoneNumberAsync(user);
                claims[Claims.PhoneNumberVerified] = await _userManager.IsPhoneNumberConfirmedAsync(user);
            }

            if (User.HasScope(Scopes.Roles))
            {
                //claims[Claims.Role] = await _userManager.GetRolesAsync(user);
                List<string> roles = new List<string> { "dataEventRecords", "dataEventRecords.admin", "admin", "dataEventRecords.user" };
            }

            // Note: the complete list of standard claims supported by the OpenID Connect specification
            // can be found here: http://openid.net/specs/openid-connect-core-1_0.html#StandardClaims

            return Ok(claims);
        }

    }

For another service I have written an IUnitOfWork. To use this IUnitOfWork inside a controller. I again need to specify all the keys inside a controller.

public interface IUnitOfWork<TRoleKey, TUserKey, TClientKey> : IDisposable
        where TRoleKey : IEquatable<TRoleKey>
        where TUserKey : IEquatable<TUserKey>
        where TClientKey : IEquatable<TClientKey>
    {
        IUserService<TRoleKey, TUserKey, TClientKey> UserService { get; }
        IRoleService<TRoleKey, TUserKey, TClientKey> RoleService { get; }
        IUserRoleService<TRoleKey, TUserKey, TClientKey> UserRoleService { get; }
        IRolePermissionService<TRoleKey, TUserKey, TClientKey> RolePermissionService { get; }

        Task<bool> Commit();
    }

To solve all of these issues. I was thinking to using MarkerInterfaces for all these different services. Like for example for using ApplicationUser.

public interface IMarkerApplicationUser{}



public class ApplicationUser<TIdentityKey, TClientKey> : IMarkerApplicationUser,IdentityUser<TIdentityKey>, IApplicationUser<TIdentityKey, TClientKey>
    where TIdentityKey : IEquatable<TIdentityKey>
    where TClientKey : IEquatable<TClientKey>
    {
        
        public TClientKey TenantId { get; set; }
        
    }

Afterwards I can just take these as constructor parameter and use dependency injection to specify generic instead of GenericType functions and classes.

services.AddScoped<IMarkerApplicationUser, ApplicationUser<Guid,Guid>>();

Is this a good approach? I have read every where that using marker interfaces is a bad practice.

The main purpose of doing all this is to create generic microservices for my common projects. Like UserManagement, RoleManagement, Audit Management, Exception Management and then pass the type of keys from the main project. I don't want to use GUID everywhere as the primary keys because, some of the systems doesn't have the requirement to use Guid and have space constraints.

Upvotes: 6

Views: 454

Answers (1)

Matt Thomas
Matt Thomas

Reputation: 5744

ConfirmEmailModel does not need to be generic

These are the only things I can see that are affected by the generic type parameters in ConfirmEmailModel:

  • The return type of _userManager.FindByIdAsync(...)
  • The type of the user variable in OnGetAsync(...)
  • The type of the user parameter in _userManager.ConfirmEmailAsync(user, ...)

(Also, user has to be a nullable reference type for your null check)

You don't need generic type parameters to make these pieces fit together. What if your ConfirmEmailModel instead looked like the following?

public class ConfirmEmailModel : PageModel
{
    readonly IUserManager _userManager;

    public ConfirmEmailModel(IUserManager userManager)
    {
        _userManager = userManager;
    }

    [TempData]
    public virtual string StatusMessage { get; set; }

    public virtual async Task<IActionResult> OnGetAsync(string userId, string code)
    {
        if (userId == null || code == null)
        {
            return RedirectToPage("/Index");
        }

        var user = await _userManager.FindByIdAsync(userId);
        if (user == null)
        {
            return NotFound($"Unable to load user with ID '{userId}'.");
        }

        code = Encoding.UTF8.GetString(WebEncoders.Base64UrlDecode(code));
        var result = await _userManager.ConfirmEmailAsync(user, code);
        StatusMessage = result.Succeeded
            ? "Thank you for confirming your email."
            : "Error confirming your email.";
        return Page();
    }
}

public interface IUserManager
{
    Task<Result> ConfirmEmailAsync(object user, string code);

    Task<object?> FindByIdAsync(string userId);
}

sealed class UserManagerAdapter : IUserManager
{
    readonly UserManager<ApplicationUser<TIdentityKey,TClientKey>> _userManager;

    public UserManagerAdapter(UserManager<ApplicationUser<TIdentityKey,TClientKey>> userManager)
    {
        _userManager = userManager;
    }

    public async Task<Result> ConfirmEmailAsync(object user, string code)
    {
        if (user is not ApplicationUser<TIdentityKey,TClientKey> applicationUser)
            return Fail();
        return await _userManager.ConfirmEmailAsync(applicationUser, code);
    }

    public async Task<object?> FindByIdAsync(string userId)
    {
        return await _userManager.FindByIdAsync(userId);
    }
}

Then you could wire up your IoC registrations in such a way that the UserManagerAdapter is the registered implementation of IUserManager.

BaseUserInfoController also does not need to be generic

You can apply a similar way of thinking to BaseUserInfoController.

What are the generic type parameters actually used for?

To me it seems that the only thing that really cares what type the user variable is, is the very same _userManager that gave it to you in the first place. That raises a little warning flag in my head that says "implementation detail". From the perspective of your BaseUserInfoController it doesn't matter what that type is, so what's the point of the generic type parameters? If _userManager just returned opaque objects then you could lose the generic type parameters and your BaseUserInfoController would be no worse off.

Leaky abstractions

I think your abstractions are kind of leaky (Google the phrase "leaky abstraction"). Generic type parameters in your case are an implementation detail—not even your model and controller care what they are—and yet everything that uses your model or controller has to deal with those details.

What I'm proposing instead is that you hide those implementation details behind interfaces that are tailored to the consumers of those interfaces.

I hope this helps!

The above code was written off the cuff and might not compile

Upvotes: 0

Related Questions