Валидация generic параметров в Spring контроллерах

  • Tutorial
image
Все мы часто пишем простые методы в контроллерах работающие через числовые идентификаторы.

    @RequestMapping(value = {"/entityName/{entityId}/get"}, method = RequestMethod.GET)
    @ResponseBody
    public Entity get(@PathVariable(value = "entityId") Integer entityId) {
        //возврат значения сущности по ID
    }

Пришедший ID надо валидировать.Например, не у всех пользователей есть доступ ко всем ID.

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

Валидацию можно сделать по простому:

    @RequestMapping(value = {"/entityName/{entityId}/get"}, method = RequestMethod.GET)
    @ResponseBody
    public Entity get(@PathVariable(value = "entityId") Integer entityId) {
        if(!@dao.validate(entityId))
             return some_error;

        //возврат значения сущности по ID
    }

Плюс в таком решении только один. Просто и быстро.
Все остальное плохо. Дублирование кода, валидация не совместима с валидацией объектов, нужна отдельная обработка в каждом методе.

Хочется сделать так:

    @RequestMapping(value = {"/entityName/{entityId}/get"}, method = RequestMethod.GET)
    @ResponseBody
    public Entity get(@Validated @PathVariable(value = "entityId") Integer entityId) {
        //возврат значения сущности по ID
    }

Этот простой и логичный вариант не работает. Валидатор просто не вызывается. Валидация PathVariable Спрингом не поддерживается.

Для работоспособности этого варианта надо превратить PathVariable в ModelAttribute:


    @ModelAttribute
    private Integer integerAsModelAttribute(@PathVariable("entityId") Integer id) {
        return id;
}

И получить ошибку при обращении к контроллеру. У врапперов генерик типов нет дефолтного конструктора без параметров и нет сеттера. Обходится это с помощью использования Optional. У него есть и дефолтный конструктор и сеттер принимающий обычные инты.

Превращаем Integer в Optional:


    @ModelAttribute
    private Integer integerAsModelAttribute(@PathVariable("entityId") Optional<Integer> id) {
        return id.orElse(null);
    }

И соответственно сам метод контроллера и объявление валидатора:


    @InitBinder({"entityId"})
    protected void initCommissionIdBinder(WebDataBinder binder) {
        binder.setValidator(validateEntityIdValidator);
        binder.setBindEmptyMultipartFiles(false);
    }

    @RequestMapping(value = {"/entityName/{entityId}/get"}, method = RequestMethod.GET)
    @ResponseBody
    public Entity get(@Validated @ModelAttribute(value = "entityId") Integer entityId) {
       //Можно смело работать с ID. Оно уже валидировано.
    }

Класс валидатора абсолютно обычный:


public class ValidateIntegerValidator implements Validator {
    @Override
    public boolean supports(Class<?> aClass) {
        return Integer.class.equals(aClass);
    }

    @Override
    public void validate(Object o, Errors errors) {

        if(o instanceof Integer) {
            Integer integer = (Integer) o;

            if(!dao.checkId(integer)) {
                errors.reject("-1", "ERROR");
            }
        } else {
            errors.reject("-2","WRONG TYPE");
        }

    }
}

Полный рабочий пример можно взять тут.
Ads
AdBlock has stolen the banner, but banners are not teeth — they will be back

More

Comments 24

    0
    Пришедший ID надо валидировать.

    Зачем?
    Ваш пример, на мой взгляд, неудачный хотя бы потому что вы по сути пихаете бизнес логику в не предназначенное для нее место.
    А другого примера мне придумать не удалось.
      0
      В смысле зачем? Сценарий с доступом пользователя не ко всем сущностям одного типа очень стандартный.
      От документооборотов (не все документы доступны) до форумов (не все разделы доступны) везде встречается.

      Разрулить это спринговыми ролями невозможно. Доступ к методу у пользователя должен быть.

      Бизнес логика это действия чтобы собрать и отдать этот объект. Неправильно в ней писать проверку прав пользователя.
        0
        Я не про сценарий, а про использование валидатора не по назначению.
        Бизнес логика это действия чтобы собрать и отдать этот объект. Неправильно в ней писать проверку прав пользователя.

        Например доступ к документу зависит от его статуса. Что бы узнать статус вы должны получить этот документ из хранилища и если все хорошо, то вызывается метод бизнес логики который опять запрашивает документ. Т.е. уже дважды придется делать одну и ту же работу.
          0
          Кеширование в помощь.
          Второе обращение к документу не будет стоить практически ничего, по сравнению с первым.
            0
            А код?
            Вы в двух местах напишите одинаковый код, а потом когда правила поменяются будете его править в двух местах.
            А придет новый человек и поправит только в одном месте.
              0
              Получения объекта из хранилища это настолько типовая и дублирующаяся в куче мест операция что о ней можно не беспокоиться. Она должна работать, проверяться тестами и кешироваться. Правки затрагивающие ее не должны ломать существующий код.

              Все остальное только в одном месте. Отдельно проверка прав доступа. Отдельно работа с объектом.

              Новый человек должен быть в курсе таких вещей.
          • UFO just landed and posted this here
            0
            так а что проверяет dao метод checkId? ровно тоже самое, что и для всех остальных пользователей.
            традиционно делается поиск типо такого repository.findByUsernameAndId(username, id).orElseThrow { new ItemNotAvailableException }
            построение связности доступа к этому объекту можно решить многий ко многим и убрать непонятный валидатор с непонятным dao.
              0
              С методами findByUsernameAndId(username, id) проверяюшими права возникает проблема когда надо делать доступ без пользователей. К примеру, автоматические выгрузки и тому подобные сервисные операции.

              Приходится или разрешать username==null и давать доступ ко всему, что черевато дырами при ошибках в контроллерах.
              Или заводить технические учетные записи, что вызывает проблемы управления и поддержки.
              Или писать рядом такие же методы, не проверяющие права. Плохо само по себе.

              Вариант findById(id) не проверяющее никаких прав и проверка прав на уровне контроллеров выглядит гораздо лучше.
              0
              Я б тоже на @Validated отдавал бы только простейшие влидации, который там сейчас поддерживаются javax.validation аннотациями. А более сложные, с участием базы данных, пусть делает сервис и кидает исключение в случае чего.
              0

              Optional * exception mapper Самый правильный вариант, если хочется для однотипных вызовов иметь предсказуемое и "стандартное" в рамках проекта поведение, но это больше похоже на gist в гите, а не на статью для хабра IMO

                0
                Меня и так в сложности решения чуть ниже упрекают. Хотя я и упрощал как мог.
                Захотелось на русском написать. А кроме Хабра некуда.
                0
                Вот при всем моем уважении — вариант с явным вызовом валидатора обладает одним преимуществом, которое на длинной дистанции перевешивает все его минусы. Даже для человека, который только вчера начал программировать и увидел проект первый раз в жизни — совершенно очевидно, что в этой точке производится валидация параметра, и ясно — какой класс (объект) за это отвечает. И если что-то пошло не так, то можно поставить точку останова и посмотреть — что, где, и почему… А в случае с аннотациями — вызов валидатора утонет в сгенерированных проксях и магии байткода. И когда это как-то сломается (а ломается рано или поздно все!) — разборки с магией могут длиться часами и днями.

                Мой опыт в программировании для промышленного производства показывает, что код системы следует писать так, чтобы он был очевидным для разработчика квалификации ниже средней. Не потому что разработчики плохие — а потому что если разработчика в два ночи поднять срочным звонком, работоспособность и острота мышления (оказывается!) сильно снижается. А проблему надо решать, и притом прямо сейчас, и с первого раза. Когда код изначально был написан так, чтобы «дураку понятно было» — это очень помогает. Пусть даже ценой некоторого дублирования и общей некрасивости.

                Как это обычно бывает — не догма, и your mileage may vary! :-)
                  +1
                  Вы мало проектов на Спринге видели. @Validated это понятнее некуда. Самое простое и элегантное решение. Проверка в каждом методе и создание сообщений об ошибках во вью тоже в каждом методе, выходит в конечном итоге сложнее и потенциально содержит больше ошибок.

                  И обвязка это самое простое из того что я нашел в залежах «нестандартного Спринга». Все остальное там гораздо сложнее. Проблемы вида совместить windows domain sso и spring form authorization и заставить все это работать работать на виндовом кластере решаются гораздо сложнее. Там такие дебри…
                    0
                    Ну вот, я же говорю — тут каждый балансирует как может. Мы в свое время наелись магии и чертовщины в Java EE, и теперь очень критично выбираем фичи из фреймворков. Из того же спринга, пользуемся только Transactional, Autowired, RequestMapping и еще парой-тройкой сопутствующих аннотаций. Может быть везет что задачи специфичные — но пока хватает. А может быть возраст и опыт прививают аллергию на сложные решения… В общем, я остаюсь по отношению к внешним компонентам на позиции: пусть или оно незаметно сразу правильно работает «из коробки», или пишем свой функционал и явным образом где нужно вызываем. А сложная магия — ну ее: и порог вхождения для специалиста выше, и при отказе — поди-пойми куда бежать…
                      +1
                      Java EE вообще оказалась неудачной концепцией. JSR гораздо удобнее.

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

                  Смешались кони, люди… Зачем смешивать авторизацию и контроль входных данных?
                  Кстати спрнг для вашего решения имеет стандарт: @PreAuthorize, @PostAuthorize и даже фильтрации данных @PreFilter и @PostFilter. Для них тоже прекрасно пишутся свои функции контроля. И соответственно в коде отделяется бизнес логика от секюрити логики.

                    0
                    А нет впечатления, что вот этот подход спринга к безопасности очень похож на ситуацию, когда мы сначала нарисовали приложение — а потом (вдруг!) решили навесить сверху разграничение доступа. То есть для простых приложений — это где-то и удобно: сделали MVP, потом улучшили, потом набросили какую-то безопасность, и оно живет. Но в более-менее сложном и критичном приложении безопасность полагается интегрировать. И чтобы оно при нарушении этой подсистемы, желательно, вообще работать не могло…
                      0
                      Нет. Так как бизнес логика и секюрити это две разные подсистемы и одна о другой должны знать как можно меньше.
                        0
                        В простых системах, пожалуй да. А если нужно от пользователя через запрос тащить какой-нибудь seciruty-token через всю бизнес-логику, чтобы где-то в конце упихать его в подсистему аудита (а то и в лог БД)? А если с требованием, чтобы если аудит не записался, то и операцию не исполнять? Как тут провести различие между бизнес-логикой и безопасностью. По сути, они в ТЗ делают требования безопасности частью логики бизнес-процессов.
                          0
                          Такое даже в банк клиентах не используется, насколько я в курсе.
                          2fa и хватит.

                          Не поделитесь где такое видели в реальной жизни?
                      0
                      Абсолютно согласен. Валидатор должен проверять корректность запроса (или его тела), а не доступ к сущности (И вернуть, к примеру HTTP 400). Для проверки секьюрити можно просто воспользоваться @PreAuthorize:

                      @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.
                        0
                        Проверка прав доступа это просто пример. Самый очевидный, но пример.
                        Никто не мешает так валидировать обычные параметры. Когда по каким-то соображениям передается не объект, а пачка переменных.

                        Там какой-нибудь: /edit/setExternalLinkTo/anotherEntityId
                        Возвращать 400 нехорошо, а проверить существование сущности перед началом обновления объектов хочется.
                      0
                      А вот ещё вопрос: если хочется валидировать параметры, почему бы не воспользоваться всей мощью АОР, да и написать реальный обработчик таких ситуаций. А функцию валидации можно передать через класс в @Validated.

                      Only users with full accounts can post comments. Log in, please.