Что означает «пользователь не должен решать, является ли он администратором или нет. Привилегии или Система безопасности должны ».

55

В примере, использованном в этом вопросе, передается минимальное количество данных для функции, которая помогает определить, является ли пользователь администратором или нет. Один общий ответ был:

user.isAdmin()

Это побудило комментарий, который был повторен несколько раз и проголосовал много раз:

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

Я ответил,

Пользователь ничего не решает. Пользовательский объект / таблица хранит данные о каждом пользователе. Реальные пользователи не могут ничего изменить в себе.

Но это не было продуктивным. Ясно, что есть принципиальное различие во взглядах, которое затрудняет общение Может кто-нибудь объяснить мне, почему user.isAdmin () плохой, и нарисовать краткий набросок того, как это выглядит «правильно»?

На самом деле, я не вижу преимущества отделения безопасности от системы, которую она защищает. В любом тексте по безопасности будет сказано, что безопасность должна быть спроектирована в систему с самого начала и учитываться на каждом этапе разработки, развертывания, обслуживания и даже окончания срока службы. Это не то, что можно прикрутить сбоку. Но 17 голосов за этот комментарий говорят, что я упускаю что-то важное.

GlenPeterson
источник
8
Я не думаю, что это плохо, особенно если это только флаг в пользовательской таблице БД. Если будут изменения в будущем, измените это в будущем. Вы также можете вызвать систему безопасности из объекта пользователя. Однажды я прочитал о шаблоне проектирования, что каждый доступ к базе данных / ресурсам должен проходить через объект текущего пользователя, чтобы убедиться, что вы никогда не попытаетесь сделать то, что не разрешено. Может быть, немного, но пользователь может предоставить объект «привилегий», который охватывает все, что связано с безопасностью.
Головоногий
Я думаю, что было недоразумение. Для вас «Пользователь» означает человека, который использует программное обеспечение. Для них «Пользователь» означает код, который использует вашу функциональность.
Филипп
2
Не то чтобы вы спросили, но в этом случае я бы больше всего позаботился о методе IsAdmin (), где бы вы его ни указывали. Я обычно рекомендовал бы, чтобы «Admin» не был жестко запрограммирован, а просто имел набор возможных разрешений, а «Admin» просто имел полный набор. Таким образом, когда вам нужно разделить роль администратора позже, все гораздо проще. И это сокращает логику особого случая.
PSR
1
@ Филипп Я так не думаю ... Оба сайта явно говорят об Userобъекте с помощью isAdmin()метода (что бы он ни делал за кадром)
Izkata
Слишком поздно для редактирования; Я имел в виду "Обе стороны", а не "Обе сайты" ... = P
Izkata

Ответы:

12

Думайте о Пользователе как о ключе, а о разрешениях как о значении.

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

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

Уилберт
источник
40

Этот комментарий был плохо сформулирован.

Дело не в том, «решает» ли пользовательский объект, является ли он администратором, пробным пользователем, суперпользователем или чем- то необычным. Очевидно, что это не решает ничего из того, что делает программная система в целом, как вы ее внедрили. Дело в том, является ли «пользователь» класс должен представлять роль основных , что ее объекты представляют собой, или это ответственность другого класса.

Килиан Фот
источник
6
Ой (это был мой комментарий), но вы выразились намного лучше, чем я.
Марьян Венема
Вы говорите, чтобы моделировать роли в отдельном классе Role, а не в классе User? Вы не рекомендуете отдельную систему, такую ​​как LDAP? Что если каждое отдельное решение, которое принимает класс Role, требует запроса соответствующей пользовательской записи и не требует никакой другой информации? Должно ли оно быть отдельно от Пользователя и если да, то почему?
ГленПетерсон
2
@GlenPeterson: не обязательно, просто таблицы классов <>, и обычно их гораздо больше, чем таблиц. Ориентация на данные действительно приводит к определению подсписков в классах, хотя много раз эти списки должны быть определены в другой «бухгалтерской книге» (заказах) или некотором таком классе, передаваемом пользователю (или заказу), для которого нужно извлечь ... Это скорее вопрос ответственности, чем глаголов и существительных.
Марьян Венема
9
Думайте о Пользователе как о ключе, а о разрешениях как о значении. Различные контексты могут иметь разные разрешения для каждого пользователя. Поскольку разрешения зависят от контекста, вам придется либо изменить пользователя для каждого контекста. Так что лучше поместить эту логику в другое место (например, в класс контекста).
Уилберт
2
@ Уилберт: Контекст! Это особенно поучительно! Да, если у вас есть более одного контекста для этих разрешений, тогда все имеет смысл для меня. Обычно я вижу эти разрешения в одном контексте, и в этом случае самое простое решение - прикрепить их к пользователю. Ясно, что они не принадлежат там, если они применяются по-другому во втором контексте. Если вы напишите это как ответ, я, скорее всего, приму это. Спасибо.
ГленПетерсон
30

Я бы не стал называть исходный комментарий так, как он был сформулирован, но он идентифицирует потенциально законную проблему.

В частности, проблемы, которые требуют разделения, - это аутентификация и авторизация .

Аутентификация относится к процессу входа в систему и получения личности. Это то, как системы узнают, кто вы , и используются для таких вещей, как персонализация, владение объектами и т. Д.

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

Эти два могут меняться ортогонально друг к другу. Например, вы можете изменить модель аутентификации, добавив поставщиков OpenID / OpenAuth. И вы можете изменить политику безопасности, добавив новую роль или сменив RBAC на ABAC.

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

Я работал с системами, где аутентификация и авторизация были слишком тесно связаны. В одной системе было две параллельные пользовательские базы данных, каждая для одного типа «роли». Человек или команда, которые его разработали, очевидно, никогда не считали, что один физический пользователь может быть в обеих ролях, или что могут быть определенные действия, которые являются общими для нескольких ролей, или что могут быть проблемы с конфликтами идентификаторов пользователей. Это, по общему признанию, крайний пример, но работать с ним было невероятно больно.

Microsoft и Sun / Oracle (Java) называют совокупность информации об аутентификации и авторизации как принципала безопасности . Это не идеально, но работает достаточно хорошо. В .NET, к примеру, у вас есть IPrincipal, что инкапсулируетIIdentity - бывшую ЯВЛЯЮЩУЮСЯ политика (авторизация) объект , а последняя является идентичностью (аутентификация). Можно обоснованно ставить под сомнение решение поставить один внутри другого, но главное в том , что большая часть кода вы пишете будет только для одного из абстракций , который означает , что это легко проверить и реорганизовать.

Нет ничего плохого в User.IsAdminполе ... если только нет User.Nameполя. Это может указывать на то, что концепция «Пользователь» не определена должным образом, и, к сожалению, это очень распространенная ошибка среди разработчиков, которые немного не понимают, когда речь заходит о безопасности. Как правило, единственной вещью, которая должна совместно использоваться идентификаторами и политиками, является идентификатор пользователя, который, не случайно, является именно тем, как он реализован в моделях безопасности Windows и * nix.

Вполне допустимо создавать объекты-оболочки, которые инкапсулируют как идентичность, так и политику. Например, это облегчит создание экрана панели мониторинга, на котором вам необходимо отобразить «привет» сообщение в дополнение к различным виджетам или ссылкам, к которым у текущего пользователя есть доступ. Пока эта обертка просто оборачивает идентификационную информацию и информацию о политике и не претендует на владение ею. Другими словами, до тех пор, пока он не будет представлен как совокупный корень .

Упрощенная модель безопасности всегда кажется хорошей идеей, когда вы впервые разрабатываете новое приложение, из-за YAGNI и тому подобного, но она почти всегда заканчивается тем, что позже вас кусает, потому что, к удивлению, добавляются новые функции!

Итак, если вы знаете, что лучше для вас, вы будете хранить информацию об аутентификации и авторизации отдельно. Даже если «авторизация» прямо сейчас так же проста, как флаг «IsAdmin», вам все равно будет лучше, если он не входит в тот же класс или таблицу, что и информация аутентификации, так что если и когда ваша политика безопасности должна изменить, вам не нужно делать реконструктивные операции в ваших системах аутентификации, которая уже работает нормально.

Aaronaught
источник
11
О, если бы у меня было время написать это. Опять же, я бы, наверное, не смог бы выразить это так же хорошо, как вы, не подумав с моей стороны. Часто мой внутренний инстинкт подсказывает мне идти тем или иным путем, но объяснение, почему - для меня или кого-то еще - требует гораздо больше мыслей и усилий. Спасибо за этот пост. Многократно плюс, но SE не позволит мне ...
Марьян Венема
Это звучит поразительно похоже на проект, над которым я работаю, и ваш ответ дал мне другую перспективу. Спасибо вам большое!
Брэндон
«Нет ничего плохого в поле User.IsAdmin ... если только нет поля User.Name», - пишите вы. Но что isAdminтакое метод? Он может просто делегировать объекту, в обязанности которого входит отслеживание разрешений. То, что является наилучшей практикой при разработке реляционной модели (какие поля есть у сущности), не обязательно переводится на то, что является наилучшей практикой в ​​модели ОО (на какие сообщения может отвечать объект).
KaptajnKold
@KaptajnKold: я продолжаю читать это мнение в различных ветках вопросов и ответов на этом форуме. Если метод существует, не имеет значения, выполняет ли он операции напрямую или делегирует их, он все равно считается ответственностью, поскольку другие классы могут полагаться на него . Вы User.IsAdminвполне можете быть однострочником, который ничего не делает, кроме вызова PermissionManager.IsAdmin(this.Id), но если на него ссылаются 30 других классов, вы не можете изменить или удалить его. Вот почему SRP существует.
Ааронаут
И @KaptajnKold, что касается реляционных моделей, я не уверен, к чему вы клоните; еще хуже иметь IsAdmin поле . Это та область, которая имеет тенденцию сохраняться долго после того, как система превратилась в RBAC или что-то более сложное, и постепенно теряет синхронизацию с «реальными» разрешениями, и люди забывают, что они больше не должны ее использовать, и ... ну, просто скажи нет. Даже если вы думаете, что у вас есть только две роли, «администратор» и «не-администратор», не храните его как логическое / битовое поле, нормализуйте его правильно и получите таблицу разрешений.
Ааронаут
28

Ну, по крайней мере, это нарушает принцип единой ответственности . Если бы произошли изменения (несколько уровней администратора, еще больше разных ролей?), Такой дизайн был бы довольно грязным и ужасным в обслуживании.

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

Zavior
источник
2
@GlenPeterson: Нет, пользователи могут очень хорошо существовать без безопасности. Как насчет моего имени, моего отображаемого имени, моего адреса, моего адреса электронной почты и т. Д. Где бы вы их разместили? Идентификация (аутентификация) и авторизация - это разные звери.
Марьян Венема
2
class Userявляется ключевым компонентом в триплете безопасности Идентификация, Аутентификация, Авторизация . Они тесно связаны на уровне разработки, поэтому код не обязательно должен отражать эту взаимозависимость.
MSalters
1
Вопрос в том, должен ли класс User содержать всю логику для обработки триплета!
Завиор,
3
@MSalters: если это API для такой логики, он знает об этом, даже если он делегирует в другом месте точную логику. Смысл в том, что Авторизация (разрешения) - это комбинация пользователя с чем-то другим и, следовательно, не должна быть частью ни пользователя, ни чего-то еще, а частью систем разрешений, которые по праву осведомлены как о пользователях, так и о чем угодно. дается разрешение на.
Марьян Венема
2
Should there be changes- ну, мы не знаем, что будут перемены. ЯГНИ и все. Есть ли те изменения, когда это становится необходимым, верно?
Иската
10

Я думаю, что проблема, возможно, плохо сформулированная, состоит в том, что это придает слишком большой вес Userклассу. Нет, проблема не связана с наличием метода User.isAdmin(), как было предложено в этих комментариях. В конце концов, если кто-то достаточно глубоко в вашем коде вводит вредоносный код для одного из ваших основных классов, у вас есть серьезные проблемы. С другой стороны, не имеет смысла Userрешать, является ли он администратором для каждого контекста. Представьте, что вы добавляете разные роли: модераторы для вашего форума, редакторы, которые могут публиковать посты на главной странице, авторы, которые могут писать контент для публикации в редакторах, и так далее. С подходом размещения его на Userобъекте вы получите слишком много методов и слишком много логики, живущей на том, Userчто не имеет прямого отношения к классу.

Вместо этого вы можете использовать перечисление различных типов разрешений и PermissionsManagerкласс. Возможно, у вас может быть такой метод, как PermissionsManager.userHasPermission(User, Permission)возвращение логического значения. На практике это может выглядеть так (в Java):

if (!PermissionsManager.userHasPermission(user, Permissions.EDITOR)) {
  Site.renderSecurityRejectionPage();
}

По сути, это эквивалентно методу before_filterметода Rails , который предоставляет пример такой проверки прав доступа в реальном мире.

asthasr
источник
6
Чем это отличается user.hasPermission(Permissions.EDITOR), кроме того, что оно более многословно и процедурно вместо ОО?
Головоногий
9
@Arian: потому что user.hasPermissionsнакладывает слишком много обязанностей на пользовательский класс. Это требует, чтобы пользователь знал о разрешениях, в то время как пользователь (класс) должен иметь возможность существовать без таких знаний. Вы не хотите, чтобы пользовательский класс знал о том, как печатать или отображать (создавать форму) сам по себе, вы оставляете это для рендерера / построителя принтера или отображаете класс рендерера / построителя.
Марьян Венема
4
user.hasPermission(P)это правильный API, но это только интерфейс. Конечно, реальное решение должно быть принято в подсистеме безопасности. Чтобы ответить на комментарий Сириона о тестировании, во время тестирования вы можете заменить эту подсистему. Тем не менее, выгода user.hasPermission(P)заключается в том, что вы не загрязняете весь код только для того, чтобы его можно было протестировать class User.
MSalters
3
@MarjanVenema По этой логике, пользователь останется объектом данных без каких-либо обязанностей. Я не говорю, что все управление разрешениями должно быть реализовано в пользовательском классе, если есть необходимость в такой сложной системе. Вы говорите, что пользователю не нужно знать о разрешениях, но я не понимаю, почему каждый другой класс должен знать, как разрешать разрешения для пользователя. Это делает издевательство и тестирование намного сложнее.
Головоногий
6
@Arian: хотя анемичные классы являются запахом, некоторые классы просто не имеют никакого поведения ... Пользователь, imho, является одним из таких классов, который лучше использовать в качестве параметра для целого ряда других вещей (которые хотят что-то делать / принимать решения, основываясь на том, кто их запрашивает), а не использовать в качестве контейнера для всех видов поведения только потому, что кажется, что его так удобно кодировать.
Марьян Венема
9

Object.isX () используется для представления четкой связи между объектом и X, которая может быть выражена как логический результат, например:

Water.isBoiling ()

Circuit.isOpen ()

User.isAdmin () предполагает единственное значение «Администратор» в каждом контексте системы , что пользователь в каждой части системы является администратором.

Хотя это звучит достаточно просто, реальная программа почти никогда не будет соответствовать этой модели, но наверняка для пользователя будет требоваться администрирование ресурса [X], а не [Y], или для администраторов более различных типов (проект [проект] ] администратор против [системного] администратора).

Эта ситуация обычно требует изучения требований. Редко, если когда-либо, клиент захочет отношения, которые фактически представляет User.isAdmin (), поэтому я не решался бы реализовать любое такое решение без разъяснения.

FMJaguar
источник
6

Плакат просто имел в виду другой и более сложный дизайн. На мой взгляд, User.isAdmin()все в порядке . Даже если вы позже представите некоторую сложную модель разрешений, я не вижу особых причин для этого User.isAdmin(). Позже вы можете разделить класс User на объект, представляющий вошедшего в систему пользователя, и статический объект, представляющий данные о пользователе. Или вы не можете. Сохранить завтра на завтра.

Кевин Клайн
источник
4
Save tomorrow for tomorrow, Да уж. Когда вы обнаружите, что тесно связанный код, который вы только что написали, запутали вас, и вы должны переписать его, потому что вы не потратили лишних 20 минут, чтобы отделить его ...
MirroredFate
5
@MirroredFate: вполне возможно иметь User.isAdminметод, связанный с произвольным механизмом разрешений, не связывая класс User с этим конкретным механизмом.
Кевин Клайн
3
Для того, чтобы User.isAdmin был связан с механизмом произвольных разрешений, даже без привязки к конкретному механизму, требуется ... хорошо ... соединение. Минимум из которых будет интерфейс. И этого интерфейса просто не должно быть. Он дает пользовательскому классу (и интерфейсу) то, за что он просто не должен отвечать. Ааронот объясняет это намного лучше, чем я (как и комбинация всех других ответов на этот вопрос).
Марьян Венема
8
@MirroredFate: Мне все равно, если мне придется переписать простую реализацию, чтобы удовлетворить более сложные требования. Но у меня был действительно плохой опыт работы со слишком сложными API, разработанными для удовлетворения воображаемых будущих требований, которые никогда не возникали.
Кевин Клайн
3
@kevincline Хотя чрезмерно сложные API-интерфейсы (по определению) являются плохой вещью, я не предлагал писать слишком сложные API-интерфейсы. Я утверждал, что Save tomorrow for tomorrowэто ужасный подход к проектированию, потому что он делает проблемы, которые были бы тривиальными, если система была правильно реализована, намного хуже. Фактически, именно такой менталитет приводит к чрезмерно сложным API, так как слой за слоем добавляется для исправления первоначального плохого дизайна. Я думаю , что моя позиция measure twice, cut once.
MirroredFate
5

Не действительно проблема ...

Я не вижу проблемы с User.isAdmin(). Я, конечно, предпочитаю что-то вроде этого PermissionManager.userHasPermission(user, permissions.ADMIN), что во святое имя SRP делает код менее понятным и не добавляет ничего ценного.

Я думаю, что SRP интерпретируется немного буквально некоторыми. Я думаю, что это хорошо, даже предпочтительнее для класса иметь богатый интерфейс. SRP просто означает, что объект должен делегировать сотрудникам все, что не попадает под его единственную ответственность. Если роль администратора пользователя является чем-то более сложным, чем логическое поле, вполне может иметь смысл делегировать объекту пользователя объект PermissionsManager. Но это не значит, что для пользователя не стоит также придерживаться своего isAdminметода. Фактически это означает, что когда ваше приложение переходит от простого к сложному, вам нужно изменить код, который использует объект User. Итак, вашему клиенту не нужно знать, что на самом деле требуется, чтобы ответить на вопрос, является ли пользователь администратором.

... но почему вы действительно хотите знать?

Тем не менее, мне кажется, что вам редко нужно знать, является ли пользователь администратором, кроме как для того, чтобы ответить на какой-то другой вопрос, например, разрешено ли пользователю выполнять какое-то конкретное действие, например, обновить виджет. Если это так, я бы предпочел иметь метод на Widget, например isUpdatableBy(User user):

boolean isUpdatableBy(User user) {
    return user.isAdmin();
}

Таким образом, виджет отвечает за знание того, какие критерии должны быть выполнены, чтобы пользователь обновлял его, а пользователь отвечает за знание, является ли он администратором. Этот дизайн четко сообщает о своих намерениях и облегчает переход к более сложной бизнес-логике, если и когда придет такое время.

[Редактировать]

Проблема не имея User.isAdmin()и используяPermissionManager.userHasPermission(...)

Я решил добавить свой ответ, чтобы объяснить, почему я предпочитаю вызывать метод объекта User, а не метод метода PermissionManager, если я хочу узнать, является ли пользователь администратором (или имеет роль администратора).

Я думаю, будет справедливо предположить, что вы всегда будете зависеть от класса User, где бы вам ни хотелось задать вопрос, является ли этот пользователь администратором? Это зависимость, от которой вы не можете избавиться. Но если вам нужно передать пользователя другому объекту, чтобы задать вопрос об этом, это создаст новую зависимость от этого объекта поверх того, который у вас уже есть у пользователя. Если вопрос часто задают, это много мест, где вы создаете дополнительную зависимость, и есть много мест, которые вам, возможно, придется изменить, если того требует любая из зависимостей.

Сравните это с перемещением зависимости в класс User. Теперь, внезапно, у вас есть система, в которой клиентский код (код, который должен задать вопрос - это пользователь-администратор ) не связан с реализацией того, как на этот вопрос получают ответ. Вы можете полностью изменить систему разрешений, и вам нужно всего лишь обновить один метод в одном классе, чтобы сделать это. Весь код клиента остается прежним.

Настаивать на том, чтобы isAdminу пользователя не было метода из-за боязни создать зависимость в классе User от подсистемы разрешений, на мой взгляд, все равно, что тратить доллар на копейки. Конечно, вы избегаете одной зависимости в классе User, но за счет ее создания в каждом месте, где вам нужно задать вопрос. Не выгодная сделка.

KaptajnKold
источник
2
«SRP просто означает, что объект должен делегировать сотрудникам все, что не входит в его единственную ответственность» - ложь . Определение SRP включает в себя все, что объект делает напрямую, и все, что он делегирует. Класс, который выполняет только одну задачу напрямую, но выполняет 50 других задач косвенно через делегирование, все еще (вероятно) нарушает SRP.
Aaronaught
KaptajnKold: я +1, потому что я согласен с тобой и чувствую себя немного виноватым по этому поводу. Ваш пост добавляет к обсуждению, но не отвечает на вопрос.
ГленПетерсон
@GlenPeterson Ну, я попытался ответить на часть вопроса, которая была «как это выглядит, сделано« правильно »» Также: Вы не должны чувствовать себя виноватыми, соглашаясь со мной :)
KaptajnKold
1
@JamesSnell Возможно. Если. Но это деталь реализации, которую я бы все-таки ограничил методом isAdmin для User. Таким образом, ваш клиентский код не должен меняться, когда «администратор» пользователя превращается из логического поля в нечто более продвинутое. У вас может быть много мест, где вам нужно знать, является ли пользователь администратором, и вам не нужно менять их каждый раз, когда меняется ваша система авторизации.
KaptajnKold
1
@JamesSnell Может быть, мы не понимаем друг друга? Вопрос о том, является ли пользователь администратором , не совпадает с вопросом о том, разрешено ли пользователю выполнять определенное действие . Ответ на первый вопрос всегда не зависит от контекста. Ответ на второй вопрос очень сильно зависит от контекста. Это то, к чему я пытался обратиться во второй половине моего первоначального ответа.
KaptajnKold
1

Эта дискуссия напомнила мне пост в блоге Эрика Липперта, на который мне обратили внимание в аналогичной дискуссии о дизайне класса, безопасности и корректности.

Почему так много базовых классов запечатаны?

В частности, Эрик поднимает вопрос:

4) Безопасный. весь смысл полиморфизма в том, что вы можете передавать объекты, похожие на животных, но на самом деле это жирафы. Здесь есть потенциальные проблемы с безопасностью.

Каждый раз, когда вы реализуете метод, который принимает экземпляр незапечатанного типа, вы ДОЛЖНЫ написать этот метод, который будет устойчивым перед лицом потенциально враждебных экземпляров этого типа. Вы не можете полагаться на любые инварианты, которые, как вы знаете, верны для ВАШИХ реализаций, потому что некоторые враждебные веб-страницы могут создавать подклассы для вашей реализации, переопределять виртуальные методы, чтобы делать вещи, которые портят вашу логику и передают ее. Каждый раз, когда я опечатываю класс Я могу написать методы, которые используют этот класс с уверенностью, что я знаю, что делает этот класс.

Это может показаться касательным, но я думаю, что это соответствует точке зрения других авторов о принципе единоличной ответственности SOLID . Рассмотрите следующий вредоносный класс как причину, чтобы не реализовывать метод или свойство.User.IsAdmin

public class MyUser : User
{

    public new boolean IsAdmin()
    {
        // You know it!
        return true;
    }

    // Anything else we can break today?

}

Конечно, это немного натянуто: еще никто не предлагал создать IsAmdminвиртуальный метод, но это может произойти, если ваша архитектура пользователя / роли окажется странно сложной. (Или, возможно, нет. Учтите public class Admin : User { ... }: в таком классе может быть точно такой же код, что и выше.) Установка вредоносного двоичного файла не является распространенным вектором атаки и имеет гораздо больший потенциал для хаоса, чем неясная пользовательская библиотека - опять же, это Это может быть ошибка повышения привилегий, которая открывает дверь настоящим махинациям. Наконец, если вредоносный бинарный или объектный экземпляр попал во время выполнения, не так уж сложно представить, что «Система привилегий или безопасности» будет заменена подобным образом.

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

Да, и если быть точным для записи, я согласен с постулатом в вопросе: «Пользователь не должен решать, является ли он администратором или нет. Система привилегий или безопасности должна ». User.IsAdminМетод - плохая идея, если вы выполняете код в системе, вы не контролируете код на 100% - вы должны вместо этого делать это Permissions.IsAdmin(User user).

Патрик М
источник
А? Как это уместно? Независимо от того, где вы разместите политику безопасности, вы всегда можете подклассить ее чем-то небезопасным. Неважно, что это - пользователь, принципал, политика, PermissionSet или что-то еще. Это совершенно не связанная проблема.
Aaronaught
Это аргумент безопасности в пользу единой ответственности. Чтобы прямо ответить на ваш вопрос, модуль безопасности и разрешений, черт возьми, будет запечатанным классом, но в некоторых разработках класс User может быть виртуальным и наследуемым. Это последний пункт в статье блога (а, возможно, наименее вероятный / важный?), И, возможно, я немного объединяю вопросы, но, учитывая источник, ясно, что полиморфизм может представлять угрозу безопасности и, следовательно, ограничивать обязанности данный класс / объект более безопасен.
Патрик М
Я не уверен, почему ты так говоришь. Множество структур безопасности и разрешений легко расширяются. Большинство основных типов .NET, связанных с безопасностью (IPrincipal, IIdentity, ClaimSet и т. Д.), Не только не запечатаны, но на самом деле являются интерфейсами или абстрактными классами, так что вы можете подключить все, что захотите. Эрик говорит о том, что вы не хотели бы, чтобы что-то подобное System.Stringбыло наследуемым, потому что все виды критического фреймворкового кода делают очень конкретные предположения о том, как это работает. На практике большая часть «пользовательского кода» (включая безопасность) наследуется, чтобы допускать двойные тесты.
Aaronaught
1
В любом случае, полиморфизм не упоминается нигде в этом вопросе, и если вы перейдете по ссылке автора на место, где был сделан исходный комментарий, то станет ясно, что он не имел в виду наследование или даже инварианты класса в целом - речь шла об обязанностях.
Aaronaught
Я знаю, что это не упоминалось, но принцип классовой ответственности напрямую связан с полиморфизмом. Если нет IsAdminспособа переопределить / кооптировать, потенциальной дыры в безопасности нет. Обзор методов этих системных интерфейсов, IPrincipalи IIdentity, очевидно , что дизайнеры не согласны со мной, и поэтому я признаю точку.
Патрик М
0

Проблема здесь в следующем:

if (user.isAdmin()) {
   doPriviledgedOp();
} else {
   // maybe we can anyway!
   doPriviledgedOp();
}

Либо вы правильно настроили свою защиту, и в этом случае привилегированный метод должен проверить, обладает ли вызывающий объект достаточными полномочиями - в этом случае проверка является излишней. Или вы не доверяете непривилегированному классу принимать собственные решения в отношении безопасности.

Джеймс Андерсон
источник
4
Что такое непривилегированный класс?
Брэндон
1
На самом деле вы ничего не можете сделать для предотвращения написания такого кода. Независимо от того, как вы разрабатываете свое приложение, где-то будет такая явная проверка, и кто-то может написать ее, чтобы она была небезопасной. Даже если ваша система разрешений так же рационализирована, как и украшение метода с помощью роли или атрибута разрешений - кто-то может неправильно выдать «публичные» или «все» разрешения на это. Так что я не вижу, как это делает метод User.IsAdmin особенно проблематичным.
Aaronaught