Как мне создать многоцелевую авторизацию для почтовых запросов? - PullRequest
0 голосов
/ 13 мая 2018

В системе, которую я создаю, есть сложная и постоянно меняющаяся авторизация на основе ресурсов.Всего на данный момент шесть ролей.

Система обрабатывает участников, где все участники могут редактировать основную информацию в своем профиле, другой человек в другой роли может редактировать еще больше информации в своем профиле и так далее.

Я не могу понять, какой лучший способ создать это с помощью конечных точек / действий для сообщений, таких как действие редактирования члена.То, что я в итоге сделал, но мне не понравилось, это то, что у каждой роли есть одно действие контроллера, представление и модель представления.Основная причина сделать это вместо того, чтобы иметь одну модель представления, состоит в том, что я чувствовал, что не имеет смысла иметь все свойства, которые кто-то даже не может редактировать, это чрезмерная публикация, верно?

Я не совсем доволенрезультат.6 представлений моделей, 6 представлений, 6 безумно похожих действий контроллера, 6 валидаторов и т. Д.

Теперь моя идея состоит в том, что у меня будет только одно действие редактирования, а затем будет набор операторов if при сопоставлении с объектом домена, в представлении и на классах валидатора.Передозировка все еще там, но справилась с заявлениями if.Я тоже так думаю - что если система станет API?api/members/1/edit/ имеет больше смысла, чем api/members/1/editAsTreasurer?

Что вы думаете?У кого-нибудь есть другое решение, о котором я не задумывался?

Некоторые части кода, например дублированный код, конечно, есть больше в классах валидаторов, представлениях и сопоставлениях, не уверен, сколько их включить:

[HttpPost]
public IActionResult EditAsSecretary(EditMemberAsSecretaryViewModel viewModel)
{
    if (!ModelState.IsValid)
    {
        viewModel.Init(_basicDataProvider, _authorizationProvider.GetAuthorizedLogesForManageMember());
        return View("EditAsSecretary", viewModel);
    }

    var member = _unitOfWork.Members.GetByMemberNumber(viewModel.MemberNumber, true);
    if (member == null) return NotFound();

    // Authorize
    if (!_authorizationProvider.Authorize(viewModel.MemberInfo.LogeId, AdminType.Sekreterare))
        return Forbid();

    var user = _unitOfWork.Members.GetByUserName(User.Identity.Name);

    var finallyEmail = viewModel.MemberContactInfo.Email != null && member.Email == null &&
                       !member.HasBeenSentResetPasswordMail && member.MemberNumber != user.MemberNumber;

    _domainLogger.UpdateLog(viewModel, member, user);
    UpdateMember(viewModel, member, user.Id);
    _unitOfWork.Complete();

    if (finallyEmail) SendUserResetPasswordMail(member).Wait();

    TempData["Message"] = "Member has been updated.";

    return RedirectToAction("Details", "Members", new { memberNumber = member.MemberNumber });
}


[HttpPost]
public IActionResult EditAsManager(EditMemberAsManagerViewModel viewModel)
{
    if (!ModelState.IsValid)
    {
        viewModel.Init(_basicDataProvider, _authorizationProvider.GetAuthorizedLogesForManageMember());
        return View("EditAsManager", viewModel);
    }

    var member = _unitOfWork.Members.GetByMemberNumber(viewModel.MemberNumber, true);
    if (member == null) return NotFound();

    // Authorize
    if (!_authorizationProvider.Authorize(member.LogeId, AdminType.Manager))
        return Forbid();

    var user = _unitOfWork.Members.GetByUserName(User.Identity.Name);

    var finallyEmail = viewModel.MemberContactInfo.Email != null && member.Email == null &&
                       !member.HasBeenSentResetPasswordMail && member.MemberNumber != user.MemberNumber;

    _domainLogger.UpdateLog(viewModel, member, user);
    UpdateMember(viewModel, member, user.Id);
    _unitOfWork.Complete();

    if (finallyEmail) SendUserResetPasswordMail(member).Wait();

    TempData["Message"] = "Member has been updated.";

    return RedirectToAction("Details", "Members", new { memberNumber = member.MemberNumber });
}


private void UpdateMember(EditMemberAsSecretaryViewModel viewModel, Member member, string userId)
{
    _mapper.Map(viewModel, member);
    MapGodfathers(viewModel.MemberInfo, member);
    AfterUpdateMember(member, userId);
    _userManager.UpdateNormalizedEmailAsync(member).Wait();
}

private void UpdateMember(EditMemberAsManagerViewModel viewModel, Member member, string userId)
{
    _mapper.Map(viewModel, member);
    MapGodfathers(viewModel.MemberInfo, member);
    AfterUpdateMember(member, userId);
    _userManager.UpdateNormalizedEmailAsync(member).Wait();
}

1 Ответ

0 голосов
/ 13 мая 2018

Моя идея сейчас заключается в том, что у меня будет только одно действие редактирования, а затем будет набор операторов if при сопоставлении с объектом домена, в представлении и в классах валидатора. Передозировка по-прежнему существует, но с помощью операторов if

Не.

Помимо того, что код становится намного менее читаемым, он также создает угрозу безопасности. Каждое действие должно принимать столько параметров, сколько необходимо. Вам не нужно платить больше за действия, так что нет никаких причин для этого.

Есть некоторые проблемы с вашим кодом, которые помогают в этом дублировании:

  1. Похоже, вы проводите проверки безопасности в отношении того, что вы получаете от пользователя, вместо того, чтобы использовать аутентифицированного пользователя. Это большая проблема, поскольку вы доверяете данным, полученным от пользователя.
    Вместо этого создайте собственную Политику авторизации (проверок), которая проверяет тип пользователя, используя вашу бизнес-логику. Затем их можно добавить во встроенный контейнер и использовать:

    [Authorize(Policy = "EnsureManager")]
    public IActionResult EditAsManager(...)
    

    Это позволит вам удалить весь этот дублированный код и быть ближе к SRP.

  2. Ваш дубликат UpdateMember выглядит так, как будто ваши модели не связаны. В таком случае было бы гораздо лучше иметь базовую модель, а затем детей с необходимыми свойствами:

    public abstract class EditMemberBaseViewModel
    {
        [Required]
        public Something Something { get; set; }
    }
    
    public class EditMemberAsSecretaryViewModel : EditMemberBaseViewModel
    {
        [Required]
        public AnotherThing AnotherThing { get; set; }
    }
    

    Это позволит вам иметь один UpdateMember, поскольку логика основана на EditMemberBaseViewModel, а не на их дочерних элементах, насколько вы показали, что:

    private void UpdateMember(EditMemberAsManagerViewModel viewModel, Member member, string userId)
    {
        _mapper.Map(viewModel, member);
        MapGodfathers(viewModel.MemberInfo, member);
        AfterUpdateMember(member, userId);
        _userManager.UpdateNormalizedEmailAsync(member).Wait();
    }
    

Последний и, вероятно, самый важный момент, есть проблема с этим кодом:

_userManager.UpdateNormalizedEmailAsync(member).Wait();

Это действительно плохо. Вы заставляете ASP.NET Core зависать весь поток, ожидая завершения этого действия. Это синхронно, код 2000-х годов.
Вам необходимо научиться использовать асинхронный код для каждой связанной с вводом-выводом операции (например, вызовов базы данных) в вашем приложении, в противном случае производительность пострадает lot . Как пример:

public async Task<IActionResult> EditAsManager(...)
{
    .....
    await UpdateMemberAsync(...);
}

public async Task UpdateMemberAsync(...)
{
    await _userManager.UpdateNormalizedEmailAsync(member);
}
...