Comments 24
Пришедший ID надо валидировать.
Зачем?
Ваш пример, на мой взгляд, неудачный хотя бы потому что вы по сути пихаете бизнес логику в не предназначенное для нее место.
А другого примера мне придумать не удалось.
В смысле зачем? Сценарий с доступом пользователя не ко всем сущностям одного типа очень стандартный.
От документооборотов (не все документы доступны) до форумов (не все разделы доступны) везде встречается.
Разрулить это спринговыми ролями невозможно. Доступ к методу у пользователя должен быть.
Бизнес логика это действия чтобы собрать и отдать этот объект. Неправильно в ней писать проверку прав пользователя.
От документооборотов (не все документы доступны) до форумов (не все разделы доступны) везде встречается.
Разрулить это спринговыми ролями невозможно. Доступ к методу у пользователя должен быть.
Бизнес логика это действия чтобы собрать и отдать этот объект. Неправильно в ней писать проверку прав пользователя.
Я не про сценарий, а про использование валидатора не по назначению.
Например доступ к документу зависит от его статуса. Что бы узнать статус вы должны получить этот документ из хранилища и если все хорошо, то вызывается метод бизнес логики который опять запрашивает документ. Т.е. уже дважды придется делать одну и ту же работу.
Бизнес логика это действия чтобы собрать и отдать этот объект. Неправильно в ней писать проверку прав пользователя.
Например доступ к документу зависит от его статуса. Что бы узнать статус вы должны получить этот документ из хранилища и если все хорошо, то вызывается метод бизнес логики который опять запрашивает документ. Т.е. уже дважды придется делать одну и ту же работу.
Кеширование в помощь.
Второе обращение к документу не будет стоить практически ничего, по сравнению с первым.
Второе обращение к документу не будет стоить практически ничего, по сравнению с первым.
А код?
Вы в двух местах напишите одинаковый код, а потом когда правила поменяются будете его править в двух местах.
А придет новый человек и поправит только в одном месте.
Вы в двух местах напишите одинаковый код, а потом когда правила поменяются будете его править в двух местах.
А придет новый человек и поправит только в одном месте.
Получения объекта из хранилища это настолько типовая и дублирующаяся в куче мест операция что о ней можно не беспокоиться. Она должна работать, проверяться тестами и кешироваться. Правки затрагивающие ее не должны ломать существующий код.
Все остальное только в одном месте. Отдельно проверка прав доступа. Отдельно работа с объектом.
Новый человек должен быть в курсе таких вещей.
Все остальное только в одном месте. Отдельно проверка прав доступа. Отдельно работа с объектом.
Новый человек должен быть в курсе таких вещей.
так а что проверяет dao метод checkId? ровно тоже самое, что и для всех остальных пользователей.
традиционно делается поиск типо такого repository.findByUsernameAndId(username, id).orElseThrow { new ItemNotAvailableException }
построение связности доступа к этому объекту можно решить многий ко многим и убрать непонятный валидатор с непонятным dao.
традиционно делается поиск типо такого repository.findByUsernameAndId(username, id).orElseThrow { new ItemNotAvailableException }
построение связности доступа к этому объекту можно решить многий ко многим и убрать непонятный валидатор с непонятным dao.
С методами findByUsernameAndId(username, id) проверяюшими права возникает проблема когда надо делать доступ без пользователей. К примеру, автоматические выгрузки и тому подобные сервисные операции.
Приходится или разрешать username==null и давать доступ ко всему, что черевато дырами при ошибках в контроллерах.
Или заводить технические учетные записи, что вызывает проблемы управления и поддержки.
Или писать рядом такие же методы, не проверяющие права. Плохо само по себе.
Вариант findById(id) не проверяющее никаких прав и проверка прав на уровне контроллеров выглядит гораздо лучше.
Приходится или разрешать username==null и давать доступ ко всему, что черевато дырами при ошибках в контроллерах.
Или заводить технические учетные записи, что вызывает проблемы управления и поддержки.
Или писать рядом такие же методы, не проверяющие права. Плохо само по себе.
Вариант findById(id) не проверяющее никаких прав и проверка прав на уровне контроллеров выглядит гораздо лучше.
Я б тоже на @Validated отдавал бы только простейшие влидации, который там сейчас поддерживаются javax.validation аннотациями. А более сложные, с участием базы данных, пусть делает сервис и кидает исключение в случае чего.
Optional * exception mapper Самый правильный вариант, если хочется для однотипных вызовов иметь предсказуемое и "стандартное" в рамках проекта поведение, но это больше похоже на gist в гите, а не на статью для хабра IMO
Вот при всем моем уважении — вариант с явным вызовом валидатора обладает одним преимуществом, которое на длинной дистанции перевешивает все его минусы. Даже для человека, который только вчера начал программировать и увидел проект первый раз в жизни — совершенно очевидно, что в этой точке производится валидация параметра, и ясно — какой класс (объект) за это отвечает. И если что-то пошло не так, то можно поставить точку останова и посмотреть — что, где, и почему… А в случае с аннотациями — вызов валидатора утонет в сгенерированных проксях и магии байткода. И когда это как-то сломается (а ломается рано или поздно все!) — разборки с магией могут длиться часами и днями.
Мой опыт в программировании для промышленного производства показывает, что код системы следует писать так, чтобы он был очевидным для разработчика квалификации ниже средней. Не потому что разработчики плохие — а потому что если разработчика в два ночи поднять срочным звонком, работоспособность и острота мышления (оказывается!) сильно снижается. А проблему надо решать, и притом прямо сейчас, и с первого раза. Когда код изначально был написан так, чтобы «дураку понятно было» — это очень помогает. Пусть даже ценой некоторого дублирования и общей некрасивости.
Как это обычно бывает — не догма, и your mileage may vary! :-)
Мой опыт в программировании для промышленного производства показывает, что код системы следует писать так, чтобы он был очевидным для разработчика квалификации ниже средней. Не потому что разработчики плохие — а потому что если разработчика в два ночи поднять срочным звонком, работоспособность и острота мышления (оказывается!) сильно снижается. А проблему надо решать, и притом прямо сейчас, и с первого раза. Когда код изначально был написан так, чтобы «дураку понятно было» — это очень помогает. Пусть даже ценой некоторого дублирования и общей некрасивости.
Как это обычно бывает — не догма, и your mileage may vary! :-)
Вы мало проектов на Спринге видели. @Validated это понятнее некуда. Самое простое и элегантное решение. Проверка в каждом методе и создание сообщений об ошибках во вью тоже в каждом методе, выходит в конечном итоге сложнее и потенциально содержит больше ошибок.
И обвязка это самое простое из того что я нашел в залежах «нестандартного Спринга». Все остальное там гораздо сложнее. Проблемы вида совместить windows domain sso и spring form authorization и заставить все это работать работать на виндовом кластере решаются гораздо сложнее. Там такие дебри…
И обвязка это самое простое из того что я нашел в залежах «нестандартного Спринга». Все остальное там гораздо сложнее. Проблемы вида совместить windows domain sso и spring form authorization и заставить все это работать работать на виндовом кластере решаются гораздо сложнее. Там такие дебри…
Ну вот, я же говорю — тут каждый балансирует как может. Мы в свое время наелись магии и чертовщины в Java EE, и теперь очень критично выбираем фичи из фреймворков. Из того же спринга, пользуемся только Transactional, Autowired, RequestMapping и еще парой-тройкой сопутствующих аннотаций. Может быть везет что задачи специфичные — но пока хватает. А может быть возраст и опыт прививают аллергию на сложные решения… В общем, я остаюсь по отношению к внешним компонентам на позиции: пусть или оно незаметно сразу правильно работает «из коробки», или пишем свой функционал и явным образом где нужно вызываем. А сложная магия — ну ее: и порог вхождения для специалиста выше, и при отказе — поди-пойми куда бежать…
Java EE вообще оказалась неудачной концепцией. JSR гораздо удобнее.
Ну тоже вариант. Мне ближе вариант все что можно повесить на аннотации и по максимуму задействовать функционал уже подключенных библиотек. Иногда это требует доработки напильником. Порог входа повышается, зато мест где можно сделать трудноуловимые ошибки становится гораздо меньше. Хотя и ловить их временами сложнее.
Ну тоже вариант. Мне ближе вариант все что можно повесить на аннотации и по максимуму задействовать функционал уже подключенных библиотек. Иногда это требует доработки напильником. Порог входа повышается, зато мест где можно сделать трудноуловимые ошибки становится гораздо меньше. Хотя и ловить их временами сложнее.
Смешались кони, люди… Зачем смешивать авторизацию и контроль входных данных?
Кстати спрнг для вашего решения имеет стандарт: @PreAuthorize, @PostAuthorize и даже фильтрации данных @PreFilter и @PostFilter. Для них тоже прекрасно пишутся свои функции контроля. И соответственно в коде отделяется бизнес логика от секюрити логики.
А нет впечатления, что вот этот подход спринга к безопасности очень похож на ситуацию, когда мы сначала нарисовали приложение — а потом (вдруг!) решили навесить сверху разграничение доступа. То есть для простых приложений — это где-то и удобно: сделали MVP, потом улучшили, потом набросили какую-то безопасность, и оно живет. Но в более-менее сложном и критичном приложении безопасность полагается интегрировать. И чтобы оно при нарушении этой подсистемы, желательно, вообще работать не могло…
Нет. Так как бизнес логика и секюрити это две разные подсистемы и одна о другой должны знать как можно меньше.
В простых системах, пожалуй да. А если нужно от пользователя через запрос тащить какой-нибудь seciruty-token через всю бизнес-логику, чтобы где-то в конце упихать его в подсистему аудита (а то и в лог БД)? А если с требованием, чтобы если аудит не записался, то и операцию не исполнять? Как тут провести различие между бизнес-логикой и безопасностью. По сути, они в ТЗ делают требования безопасности частью логики бизнес-процессов.
Абсолютно согласен. Валидатор должен проверять корректность запроса (или его тела), а не доступ к сущности (И вернуть, к примеру HTTP 400). Для проверки секьюрити можно просто воспользоваться @PreAuthorize:
Далее написать секьюрити сервис, который проверит права доступа. В данном случае, при отсутствии доступа вернётся HTTP 403 Forbidden.
@PreAuthorize("@AccessService.checkAccess(#entityId)")
@RequestMapping(value = {"/entityName/{entityId}/get"}, method = RequestMethod.GET)
@ResponseBody
public Entity get(@PathVariable(value = "entityId") Integer entityId) {
//возврат значения сущности по ID
}
Далее написать секьюрити сервис, который проверит права доступа. В данном случае, при отсутствии доступа вернётся HTTP 403 Forbidden.
Проверка прав доступа это просто пример. Самый очевидный, но пример.
Никто не мешает так валидировать обычные параметры. Когда по каким-то соображениям передается не объект, а пачка переменных.
Там какой-нибудь: /edit/setExternalLinkTo/anotherEntityId
Возвращать 400 нехорошо, а проверить существование сущности перед началом обновления объектов хочется.
Никто не мешает так валидировать обычные параметры. Когда по каким-то соображениям передается не объект, а пачка переменных.
Там какой-нибудь: /edit/setExternalLinkTo/anotherEntityId
Возвращать 400 нехорошо, а проверить существование сущности перед началом обновления объектов хочется.
А вот ещё вопрос: если хочется валидировать параметры, почему бы не воспользоваться всей мощью АОР, да и написать реальный обработчик таких ситуаций. А функцию валидации можно передать через класс в @Validated.
Sign up to leave a comment.
Валидация generic параметров в Spring контроллерах