Авторизуем ресурсы в REST-сервисе

[Smerig]

В DocumentController нет проверки полномочий на удаление документа. Ещё непонятно, почему бы пользователей сразу не наследовать от Accessor? Соответсвенно Accessor от IAccessor.

[AndreyRodin]

Да, вы правы, кое-что потерялось при адаптации реального проекта, поправил.
Переименовал класс Accessor в UserBase, так, пожалуй, будет лучше.
Спасибо за внимательное чтение и замечания!

[Smerig]

Ещё замечание: правильно было бы возвращать 403, а не 404, если у пользователя нет прав на ресурс. Все-таки API пилим…

[AndreyRodin]

А вот это уже вопрос.
Есть мнение, что с точки зрения безопасности наилучшим выбором будет возврат NotFound, поскольку это не дает потенциальному взломщику знания о том, какие ресурсы реально существуют, а какие – нет.
Это старая дискуссия, и каждая компания выбирает свой стандарт.

[mexahuk61]

Если не хотите, чтобы потенциальный взломщик узнал о том, какие ресурсы реально существуют, не используйте числовое представление в идентификаторе сущности. А подменять HTTP стандарт чем то своим не стоит. Пользователи API будут сильно негодовать, когда по какой то причине конечная точка будет отсутствовать, а в документации она описана.

[AndreyRodin]

Ответ 404 может использоваться вместо 403, если требуется тщательно скрыть от посторонних глаз определённые ресурсы

ru.wikipedia.org/wiki/%D0%A1%D0%BF%D0%B8%D1%81%D0%BE%D0%BA_%D0%BA%D0%BE%D0%B4%D0%BE%D0%B2_%D1%81%D0%BE%D1%81%D1%82%D0%BE%D1%8F%D0%BD%D0%B8%D1%8F_HTTP#404

[unsafePtr]

Почему бы не использовать стандартные Claims? Можно создать claim c типом permission, а значением будет Read, Write, Delete операции.

[AndreyRodin]

Да, вполне можно. Класс Factory приведён лишь для иллюстрации, а реализация IPrincipal может быть самой различной, в том числе, с использованием Claims. Надо только иметь в виду, что для данного подхода необходимо наличие Id пользователя в Thread.CurrentPrincipal

[RomanPokrovskij]

Спасибо за матерьял. «Как делают другие» узнавать важно. Однако мое мнение будет критическим: итоговый код не выразителен (или я чего-то не уловил).

if (!Factory.CreateAccessor(Thread.CurrentPrincipal, _db)
.HasPermission(id, Permission.Write))
return NotFound();

Когда простое и достижимое выглядело бы лучше:

if (!_db.PrincipalHasWritePermission(id, Thread.CurrentPrincipal)
return NotFound();

А так еще лучше:

return _db.Batch(id,
(doc)=> {
if (!doc.CheckWritePermission(Thread.CurrentPrincipal))
return NotFound();
// ...
doc.DoMyBusinessMethodThatRequiresWritePermissions(...);
return Ok();
});

тут уже явно отделена логика репозитория от бизнес логики (бизнес логика перенесена в методы Entity, но можно ее разместить и в сервисах).

[AndreyRodin]

Не думаю, что перенос бизнес-логики в DB модель – это хорошая идея. Зона ответственности класса DbContext состоит в обеспечении модели сущностей, и ничего больше.
Но вы навели меня на важную мысль. Вызовы в контроллерах, действительно, смотрятся несколько неуклюже и могут выглядеть как нарушение закона Деметры.
Слегка подправил код, спасибо.

[RomanPokrovskij]

«Зона ответственности класса DbContext состоит в обеспечении модели сущностей, и ничего больше. „

Мне не понятно какой смысл противостовлять модель и бизнес логику.
А вот модель и DbContext это разные уровни: классы и ORM реализация. Или я не правильно использую термин Модель. В прочем я его и не использую а говорю о Entity классах, которы могут быть определены в самом базовом assembly, где нет никакого DbContex (в том же assembly могут быть interface cервисов которые бытут реализованы уже в другом assembly где есть EF и определен DbContext). И в этом самом базовом assembly (где нет никакого DbContext) я могу реализовать бизнес логику (посредством манипуляций с полями и коллекциями Enitity, и вызовом служб чьи interface определены здесь же). _db.Batch(id, func) заботится о том чтобы подать мне Entity и службы, а под конец вызовет Save на DbContext (и закроит транзакцию).
Это всегда возможно, но не всегда удобно.

Сомневаюсь что написал понятно и что комментарий интересный, но надеюсь это узнать из вашего отзыва.

[AndreyRodin]

Класс DbContext предназначен для обеспечения чтения и записи в базу данных. Проще говоря, он должен содержать только свойства DbSet — и ничего более. Попытка добавить в этот класс бизнес-логику выглядит как смешение зон ответственности. Обычно этот класс используется многими контроллерами совместно. Что будет, если ради каждого контроллера мы начнём добавлять свою бизнес-логику в наш DbContext?
Классы Entity тем более должны содержать исключительно свойства, соответствующие полям в базе. Сущности неразрывно связаны с нашим классом DbContext.
Бизнес-логика – это отдельный слой более высокого порядка. В данном примере логика авторизации сосредоточена в реализациях интерфейса IAccessor.

[RomanPokrovskij]

Классы Entity тем более должны содержать исключительно свойства, соответствующие полям в базе

Почему только поля? Почему тем более?
Это ваше мнение, или есть соглашение «entity не должны содержать методы», или действительно известны ситуации когда методы вылазят боком?

Пока остаюсь при мнении что это просто условности передачи параметров. И по большому счету нет никакой разницы между вызовами:

document.Sign(key, encodingRealization);
и
var service = new Service(encodingRealization);
service.Sign(document, key);

IModel кстати сам по себе ничего такого в код не приносит. Везде где на него ссылаются можно просто IQueryable передавать. Заметьте интерфейс не из методов а из геттеров — звоночек, я б напрягся.

[LeonThundeR]

Спасибо за статью! Когда почти дочитал очень хотелось написать в коменте: Почему не сделать возможность задания прав как для конкретных пользователей так и для ролей. Но Вы все же это в принципе учли в самом конце статьи ))

[mexahuk61]

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

[AndreyRodin]

Я описывал не приложение, а подход к реализации механизма авторизации в микросервисе.
Фильтрация сущностей на основе прав доступа входит в задачу авторизации. Операция GET для коллекции должна возвращать список объектов, к которым текущий пользователь имеет доступ.

[mexahuk61]

Я долго думал, отвечать ли на это сообщение, потому что прислушиваться вы явно не хотите. Я говорю об абстрактном приложении, которое будет использовать данный подход. И в микросервисной архитектуре вы не будете иметь доступ одновременно к пользователям, сущности Permission и документам, потому что это 3 разных микросервиса (и 3 разных базы данных соответственно).

Вы когда по улице идете, здания тоже неожиданным образом пропадают, в которые вам нельзя входить? :) В действительности у вас должен быть endpoint вида /documents?user={userId} (или в вашем случае просто /?user={userId}) и на основе параметров запроса и маркера безопасности производится выдача ответа или отклонение запроса.

[AndreyRodin]

Пользователи – это, действительно, отдельный микросервис. У меня нигде не используется прямой доступ к базе пользователей. Маркер доступа позволяет получать данные текущего пользователя. Текущий пользователь – это пользователь, который был аутентифицирован приложением, и используется во всех межсервисных обменах, поэтому параметр userId в запросах не имеет смысла.

Разделение ресурсов и прав доступа к ним на два разных микросервиса теоретически возможно, равно как и совмещение их в одном микросервисе.

Аналогия с пропадающими зданиями не опровергает необходимость возвращения текущему пользователю по запросу только тех ресурсов, к которым он имеет доступ.

Всё, что описано в данной статье, реализовано в реальных проектах, и работает без нареканий.

[mexahuk61]

Моя аналогия, конечно, и не должна опровергать, а лишь посеять нотку сомнения. И более того, я не опровергал утверждение, что пользователю необходимо возвращать только те ресурсы, к которому он имеет доступ. Я говорил, что не процесс авторизации этим занимается. Процесс авторизации только проверяет допустимые ли данные были запрошены и не более.

Тут и спорить не о чем, достаточно разобраться в значении слова Авторизация
Или вот что пишет сама Майкрософт: Authorization is deciding whether a user is allowed to perform an action.

И я не сомневаюсь, что данный код работает. Слишком много кода для такой простой задачи. Строить DATA модель на базе процесса авторизации — ну очень сомнительное решение. А если мы захотим убрать авторизацию в приложении совсем? Предоставить все документы в открытый доступ (т.е. без наличия токена)… Переписывать весь проект? Жуть какая, ведь это делается стиранием одной строчки кода.

Посмотрите стандартные подходы к процессу авторизации. Они существуют уже с десяток лет (и разработаны не самой майкрософт, а включены в фреймворк лишь в последних версиях, как лучшее решение) и сама реализация, конечно, может отличаться, но подход всегда оставался один.

[RomanPokrovskij]

Спасибо за ссылку. Так вот про «стандартные подходы» и интерфейс IAuthorizationService получаемый из контейнера (фактически с единственным методом «AuthorizeAsync(ClaimsPrincipal, Object, IEnumerable)» ). Я не очень понимаю в чем выигрыш по сравнению со своим кастомезированным методом? Доп сложность видна: Claimsы пользователя лежали в дб как entity, теперь их оборачивай в ClaimsPrincipal, документ приводи к object, а в реализации AuthorizeAsync назад, запрос оборачивай вообще в IEnumerable.

Как кода станет меньше?

Еще один отзыв попал не к вам, но хотелось бы услышать ответ. Мне не хватает альтернативных точек зрений.

[RomanPokrovskij]

«и 3 разных базы данных соответственно» — это не так, или вы не должны очереди сообщений имеющие persistent level называть «базами данных».
Нет причин множить RDBMS, хватает разделить ответсвенность: вот эти таблички досутпны, через этот сервис, другие таблички через другой. Доступными одновременно они от этого не станут, но тем не менее ваша формулировка нуждается в коррекции.