В примере, использованном в этом вопросе, передается минимальное количество данных для функции, которая помогает определить, является ли пользователь администратором или нет. Один общий ответ был:
user.isAdmin()
Это побудило комментарий, который был повторен несколько раз и проголосовал много раз:
Пользователь не должен решать, является ли он администратором или нет. Привилегии или Система безопасности должны. То, что что-то тесно связано с классом, не означает, что было бы хорошей идеей сделать его частью этого класса.
Я ответил,
Пользователь ничего не решает. Пользовательский объект / таблица хранит данные о каждом пользователе. Реальные пользователи не могут ничего изменить в себе.
Но это не было продуктивным. Ясно, что есть принципиальное различие во взглядах, которое затрудняет общение Может кто-нибудь объяснить мне, почему user.isAdmin () плохой, и нарисовать краткий набросок того, как это выглядит «правильно»?
На самом деле, я не вижу преимущества отделения безопасности от системы, которую она защищает. В любом тексте по безопасности будет сказано, что безопасность должна быть спроектирована в систему с самого начала и учитываться на каждом этапе разработки, развертывания, обслуживания и даже окончания срока службы. Это не то, что можно прикрутить сбоку. Но 17 голосов за этот комментарий говорят, что я упускаю что-то важное.
User
объекте с помощьюisAdmin()
метода (что бы он ни делал за кадром)Ответы:
Думайте о Пользователе как о ключе, а о разрешениях как о значении.
Различные контексты могут иметь разные разрешения для каждого пользователя. Поскольку разрешения зависят от контекста, вам придется менять пользователя для каждого контекста. Поэтому лучше поместить эту логику в другое место (например, в класс контекста) и просто посмотреть разрешения, где это необходимо.
Побочным эффектом является то, что это может сделать вашу систему более безопасной, потому что вы не можете забыть очистить флаг администратора для мест, где это не имеет значения.
источник
Этот комментарий был плохо сформулирован.
Дело не в том, «решает» ли пользовательский объект, является ли он администратором, пробным пользователем, суперпользователем или чем- то необычным. Очевидно, что это не решает ничего из того, что делает программная система в целом, как вы ее внедрили. Дело в том, является ли «пользователь» класс должен представлять роль основных , что ее объекты представляют собой, или это ответственность другого класса.
источник
Я бы не стал называть исходный комментарий так, как он был сформулирован, но он идентифицирует потенциально законную проблему.
В частности, проблемы, которые требуют разделения, - это аутентификация и авторизация .
Аутентификация относится к процессу входа в систему и получения личности. Это то, как системы узнают, кто вы , и используются для таких вещей, как персонализация, владение объектами и т. Д.
Авторизация относится к тому, что вам разрешено делать , и это (как правило) не определяется тем, кто вы есть . Вместо этого это определяется некоторой политикой безопасности, например ролями или разрешениями, которые не заботятся о таких вещах, как ваше имя или адрес электронной почты.
Эти два могут меняться ортогонально друг к другу. Например, вы можете изменить модель аутентификации, добавив поставщиков OpenID / OpenAuth. И вы можете изменить политику безопасности, добавив новую роль или сменив RBAC на ABAC.
Если все это входит в один класс или абстракцию, то , по иронии судьбы, ваш код безопасности, который является одним из ваших самых важных инструментов для снижения риска , становится, по иронии, высоким.
Я работал с системами, где аутентификация и авторизация были слишком тесно связаны. В одной системе было две параллельные пользовательские базы данных, каждая для одного типа «роли». Человек или команда, которые его разработали, очевидно, никогда не считали, что один физический пользователь может быть в обеих ролях, или что могут быть определенные действия, которые являются общими для нескольких ролей, или что могут быть проблемы с конфликтами идентификаторов пользователей. Это, по общему признанию, крайний пример, но работать с ним было невероятно больно.
Microsoft и Sun / Oracle (Java) называют совокупность информации об аутентификации и авторизации как принципала безопасности . Это не идеально, но работает достаточно хорошо. В .NET, к примеру, у вас есть
IPrincipal
, что инкапсулируетIIdentity
- бывшую ЯВЛЯЮЩУЮСЯ политика (авторизация) объект , а последняя является идентичностью (аутентификация). Можно обоснованно ставить под сомнение решение поставить один внутри другого, но главное в том , что большая часть кода вы пишете будет только для одного из абстракций , который означает , что это легко проверить и реорганизовать.Нет ничего плохого в
User.IsAdmin
поле ... если только нетUser.Name
поля. Это может указывать на то, что концепция «Пользователь» не определена должным образом, и, к сожалению, это очень распространенная ошибка среди разработчиков, которые немного не понимают, когда речь заходит о безопасности. Как правило, единственной вещью, которая должна совместно использоваться идентификаторами и политиками, является идентификатор пользователя, который, не случайно, является именно тем, как он реализован в моделях безопасности Windows и * nix.Вполне допустимо создавать объекты-оболочки, которые инкапсулируют как идентичность, так и политику. Например, это облегчит создание экрана панели мониторинга, на котором вам необходимо отобразить «привет» сообщение в дополнение к различным виджетам или ссылкам, к которым у текущего пользователя есть доступ. Пока эта обертка просто оборачивает идентификационную информацию и информацию о политике и не претендует на владение ею. Другими словами, до тех пор, пока он не будет представлен как совокупный корень .
Упрощенная модель безопасности всегда кажется хорошей идеей, когда вы впервые разрабатываете новое приложение, из-за YAGNI и тому подобного, но она почти всегда заканчивается тем, что позже вас кусает, потому что, к удивлению, добавляются новые функции!
Итак, если вы знаете, что лучше для вас, вы будете хранить информацию об аутентификации и авторизации отдельно. Даже если «авторизация» прямо сейчас так же проста, как флаг «IsAdmin», вам все равно будет лучше, если он не входит в тот же класс или таблицу, что и информация аутентификации, так что если и когда ваша политика безопасности должна изменить, вам не нужно делать реконструктивные операции в ваших системах аутентификации, которая уже работает нормально.
источник
isAdmin
такое метод? Он может просто делегировать объекту, в обязанности которого входит отслеживание разрешений. То, что является наилучшей практикой при разработке реляционной модели (какие поля есть у сущности), не обязательно переводится на то, что является наилучшей практикой в модели ОО (на какие сообщения может отвечать объект).User.IsAdmin
вполне можете быть однострочником, который ничего не делает, кроме вызоваPermissionManager.IsAdmin(this.Id)
, но если на него ссылаются 30 других классов, вы не можете изменить или удалить его. Вот почему SRP существует.IsAdmin
поле . Это та область, которая имеет тенденцию сохраняться долго после того, как система превратилась в RBAC или что-то более сложное, и постепенно теряет синхронизацию с «реальными» разрешениями, и люди забывают, что они больше не должны ее использовать, и ... ну, просто скажи нет. Даже если вы думаете, что у вас есть только две роли, «администратор» и «не-администратор», не храните его как логическое / битовое поле, нормализуйте его правильно и получите таблицу разрешений.Ну, по крайней мере, это нарушает принцип единой ответственности . Если бы произошли изменения (несколько уровней администратора, еще больше разных ролей?), Такой дизайн был бы довольно грязным и ужасным в обслуживании.
Рассмотрим преимущество отделения системы безопасности от пользовательского класса, так что оба могут иметь одну четко определенную роль. В итоге вы получите более чистый и удобный дизайн.
источник
class User
является ключевым компонентом в триплете безопасности Идентификация, Аутентификация, Авторизация . Они тесно связаны на уровне разработки, поэтому код не обязательно должен отражать эту взаимозависимость.Should there be changes
- ну, мы не знаем, что будут перемены. ЯГНИ и все. Есть ли те изменения, когда это становится необходимым, верно?Я думаю, что проблема, возможно, плохо сформулированная, состоит в том, что это придает слишком большой вес
User
классу. Нет, проблема не связана с наличием методаUser.isAdmin()
, как было предложено в этих комментариях. В конце концов, если кто-то достаточно глубоко в вашем коде вводит вредоносный код для одного из ваших основных классов, у вас есть серьезные проблемы. С другой стороны, не имеет смыслаUser
решать, является ли он администратором для каждого контекста. Представьте, что вы добавляете разные роли: модераторы для вашего форума, редакторы, которые могут публиковать посты на главной странице, авторы, которые могут писать контент для публикации в редакторах, и так далее. С подходом размещения его наUser
объекте вы получите слишком много методов и слишком много логики, живущей на том,User
что не имеет прямого отношения к классу.Вместо этого вы можете использовать перечисление различных типов разрешений и
PermissionsManager
класс. Возможно, у вас может быть такой метод, какPermissionsManager.userHasPermission(User, Permission)
возвращение логического значения. На практике это может выглядеть так (в Java):По сути, это эквивалентно методу
before_filter
метода Rails , который предоставляет пример такой проверки прав доступа в реальном мире.источник
user.hasPermission(Permissions.EDITOR)
, кроме того, что оно более многословно и процедурно вместо ОО?user.hasPermissions
накладывает слишком много обязанностей на пользовательский класс. Это требует, чтобы пользователь знал о разрешениях, в то время как пользователь (класс) должен иметь возможность существовать без таких знаний. Вы не хотите, чтобы пользовательский класс знал о том, как печатать или отображать (создавать форму) сам по себе, вы оставляете это для рендерера / построителя принтера или отображаете класс рендерера / построителя.user.hasPermission(P)
это правильный API, но это только интерфейс. Конечно, реальное решение должно быть принято в подсистеме безопасности. Чтобы ответить на комментарий Сириона о тестировании, во время тестирования вы можете заменить эту подсистему. Тем не менее, выгодаuser.hasPermission(P)
заключается в том, что вы не загрязняете весь код только для того, чтобы его можно было протестироватьclass User
.Object.isX () используется для представления четкой связи между объектом и X, которая может быть выражена как логический результат, например:
Water.isBoiling ()
Circuit.isOpen ()
User.isAdmin () предполагает единственное значение «Администратор» в каждом контексте системы , что пользователь в каждой части системы является администратором.
Хотя это звучит достаточно просто, реальная программа почти никогда не будет соответствовать этой модели, но наверняка для пользователя будет требоваться администрирование ресурса [X], а не [Y], или для администраторов более различных типов (проект [проект] ] администратор против [системного] администратора).
Эта ситуация обычно требует изучения требований. Редко, если когда-либо, клиент захочет отношения, которые фактически представляет User.isAdmin (), поэтому я не решался бы реализовать любое такое решение без разъяснения.
источник
Плакат просто имел в виду другой и более сложный дизайн. На мой взгляд,
User.isAdmin()
все в порядке . Даже если вы позже представите некоторую сложную модель разрешений, я не вижу особых причин для этогоUser.isAdmin()
. Позже вы можете разделить класс User на объект, представляющий вошедшего в систему пользователя, и статический объект, представляющий данные о пользователе. Или вы не можете. Сохранить завтра на завтра.источник
Save tomorrow for tomorrow
, Да уж. Когда вы обнаружите, что тесно связанный код, который вы только что написали, запутали вас, и вы должны переписать его, потому что вы не потратили лишних 20 минут, чтобы отделить его ...User.isAdmin
метод, связанный с произвольным механизмом разрешений, не связывая класс User с этим конкретным механизмом.Save tomorrow for tomorrow
это ужасный подход к проектированию, потому что он делает проблемы, которые были бы тривиальными, если система была правильно реализована, намного хуже. Фактически, именно такой менталитет приводит к чрезмерно сложным API, так как слой за слоем добавляется для исправления первоначального плохого дизайна. Я думаю , что моя позицияmeasure twice, cut once
.Не действительно проблема ...
Я не вижу проблемы с
User.isAdmin()
. Я, конечно, предпочитаю что-то вроде этогоPermissionManager.userHasPermission(user, permissions.ADMIN)
, что во святое имя SRP делает код менее понятным и не добавляет ничего ценного.Я думаю, что SRP интерпретируется немного буквально некоторыми. Я думаю, что это хорошо, даже предпочтительнее для класса иметь богатый интерфейс. SRP просто означает, что объект должен делегировать сотрудникам все, что не попадает под его единственную ответственность. Если роль администратора пользователя является чем-то более сложным, чем логическое поле, вполне может иметь смысл делегировать объекту пользователя объект PermissionsManager. Но это не значит, что для пользователя не стоит также придерживаться своего
isAdmin
метода. Фактически это означает, что когда ваше приложение переходит от простого к сложному, вам нужно изменить код, который использует объект User. Итак, вашему клиенту не нужно знать, что на самом деле требуется, чтобы ответить на вопрос, является ли пользователь администратором.... но почему вы действительно хотите знать?
Тем не менее, мне кажется, что вам редко нужно знать, является ли пользователь администратором, кроме как для того, чтобы ответить на какой-то другой вопрос, например, разрешено ли пользователю выполнять какое-то конкретное действие, например, обновить виджет. Если это так, я бы предпочел иметь метод на Widget, например
isUpdatableBy(User user)
:Таким образом, виджет отвечает за знание того, какие критерии должны быть выполнены, чтобы пользователь обновлял его, а пользователь отвечает за знание, является ли он администратором. Этот дизайн четко сообщает о своих намерениях и облегчает переход к более сложной бизнес-логике, если и когда придет такое время.
[Редактировать]
Проблема не имея
User.isAdmin()
и используяPermissionManager.userHasPermission(...)
Я решил добавить свой ответ, чтобы объяснить, почему я предпочитаю вызывать метод объекта User, а не метод метода PermissionManager, если я хочу узнать, является ли пользователь администратором (или имеет роль администратора).
Я думаю, будет справедливо предположить, что вы всегда будете зависеть от класса User, где бы вам ни хотелось задать вопрос, является ли этот пользователь администратором? Это зависимость, от которой вы не можете избавиться. Но если вам нужно передать пользователя другому объекту, чтобы задать вопрос об этом, это создаст новую зависимость от этого объекта поверх того, который у вас уже есть у пользователя. Если вопрос часто задают, это много мест, где вы создаете дополнительную зависимость, и есть много мест, которые вам, возможно, придется изменить, если того требует любая из зависимостей.
Сравните это с перемещением зависимости в класс User. Теперь, внезапно, у вас есть система, в которой клиентский код (код, который должен задать вопрос - это пользователь-администратор ) не связан с реализацией того, как на этот вопрос получают ответ. Вы можете полностью изменить систему разрешений, и вам нужно всего лишь обновить один метод в одном классе, чтобы сделать это. Весь код клиента остается прежним.
Настаивать на том, чтобы
isAdmin
у пользователя не было метода из-за боязни создать зависимость в классе User от подсистемы разрешений, на мой взгляд, все равно, что тратить доллар на копейки. Конечно, вы избегаете одной зависимости в классе User, но за счет ее создания в каждом месте, где вам нужно задать вопрос. Не выгодная сделка.источник
Эта дискуссия напомнила мне пост в блоге Эрика Липперта, на который мне обратили внимание в аналогичной дискуссии о дизайне класса, безопасности и корректности.
Почему так много базовых классов запечатаны?
В частности, Эрик поднимает вопрос:
Это может показаться касательным, но я думаю, что это соответствует точке зрения других авторов о принципе единоличной ответственности SOLID . Рассмотрите следующий вредоносный класс как причину, чтобы не реализовывать метод или свойство.
User.IsAdmin
Конечно, это немного натянуто: еще никто не предлагал создать
IsAmdmin
виртуальный метод, но это может произойти, если ваша архитектура пользователя / роли окажется странно сложной. (Или, возможно, нет. Учтитеpublic class Admin : User { ... }
: в таком классе может быть точно такой же код, что и выше.) Установка вредоносного двоичного файла не является распространенным вектором атаки и имеет гораздо больший потенциал для хаоса, чем неясная пользовательская библиотека - опять же, это Это может быть ошибка повышения привилегий, которая открывает дверь настоящим махинациям. Наконец, если вредоносный бинарный или объектный экземпляр попал во время выполнения, не так уж сложно представить, что «Система привилегий или безопасности» будет заменена подобным образом.Но если принять к сведению Эрика, если вы поместите свой пользовательский код в уязвимую систему определенного типа, возможно, вы просто проиграли игру.
Да, и если быть точным для записи, я согласен с постулатом в вопросе: «Пользователь не должен решать, является ли он администратором или нет. Система привилегий или безопасности должна ».
User.IsAdmin
Метод - плохая идея, если вы выполняете код в системе, вы не контролируете код на 100% - вы должны вместо этого делать этоPermissions.IsAdmin(User user)
.источник
System.String
было наследуемым, потому что все виды критического фреймворкового кода делают очень конкретные предположения о том, как это работает. На практике большая часть «пользовательского кода» (включая безопасность) наследуется, чтобы допускать двойные тесты.IsAdmin
способа переопределить / кооптировать, потенциальной дыры в безопасности нет. Обзор методов этих системных интерфейсов,IPrincipal
иIIdentity
, очевидно , что дизайнеры не согласны со мной, и поэтому я признаю точку.Проблема здесь в следующем:
Либо вы правильно настроили свою защиту, и в этом случае привилегированный метод должен проверить, обладает ли вызывающий объект достаточными полномочиями - в этом случае проверка является излишней. Или вы не доверяете непривилегированному классу принимать собственные решения в отношении безопасности.
источник
User.IsAdmin
особенно проблематичным.