O que se entende por: “Um usuário não deve decidir se é um administrador ou não. Os privilégios ou o sistema de segurança deveriam. ”

55

O exemplo usado na pergunta passa dados mínimos nus para uma função que aborda a melhor maneira de determinar se o usuário é administrador ou não. Uma resposta comum foi:

user.isAdmin()

Isso gerou um comentário repetido várias vezes e votado várias vezes:

Um usuário não deve decidir se é um administrador ou não. Os privilégios ou sistema de segurança devem. Algo que está sendo fortemente associado a uma classe não significa que é uma boa ideia fazer parte dela.

Eu respondi,

O usuário não está decidindo nada. O objeto / tabela Usuário armazena dados sobre cada usuário. Usuários reais não conseguem mudar tudo sobre si mesmos.

Mas isso não foi produtivo. Claramente, há uma diferença subjacente de perspectiva que está dificultando a comunicação. Alguém pode me explicar por que user.isAdmin () é ruim e fazer um breve esboço do que parece ser feito "certo"?

Realmente, não vejo a vantagem de separar a segurança do sistema que ela protege. Qualquer texto de segurança dirá que a segurança precisa ser projetada em um sistema desde o início e considerada em todos os estágios de desenvolvimento, implantação, manutenção e até final da vida útil. Não é algo que possa ser parafusado na lateral. Mas 17 votos positivos até agora neste comentário dizem que estou perdendo algo importante.

GlenPeterson
fonte
8
Eu não acho que seja ruim, especialmente se for apenas uma bandeira na tabela de banco de dados do usuário. Se houver mudanças no futuro, mude no futuro. Você também pode chamar o sistema de segurança a partir do objeto de usuário. Uma vez eu li sobre o padrão de design que todo acesso a db / recursos deve passar pelo objeto de usuário atual, para garantir que você nunca tente fazer algo que não é permitido. Pode ser um pouco demais, mas o usuário pode fornecer um objeto de "privilégios" que cubra todos os itens relacionados à segurança.
Cefalópode #
Eu acho que houve um mal-entendido. Para você, "Usuário" significa a pessoa que usa o software. Para eles, "Usuário" significa o código que usa sua funcionalidade.
Philipp
2
Não que você tenha perguntado, mas o que eu mais teria cuidado nesse caso é um método IsAdmin (), onde quer que você o coloque. Normalmente, eu recomendaria que um "Admin" não fosse codificado, mas apenas tenho um conjunto de permissões possíveis e um "Admin" possui todo o conjunto. Dessa forma, quando você precisar dividir a função de administrador posteriormente, as coisas serão muito mais fáceis. E reduz a lógica de caso especial.
Psr #
11
@Philipp Eu não penso assim ... Ambos os sites são explicitamente falando de um Userobjeto com um isAdmin()método (o que ele faz nos bastidores)
Izkata
Tarde demais para editar; Eu quis dizer "Ambos os lados", não "Ambos os sites" ... = P
Izkata

Respostas:

12

Pense no usuário como uma chave e nas permissões como um valor.

Contextos diferentes podem ter permissões diferentes para cada usuário. Como as permissões são relevantes para o contexto, você precisaria alterar o usuário para cada contexto. Portanto, é melhor você colocar essa lógica em outro lugar (por exemplo, a classe de contexto) e apenas procurar as permissões quando necessário.

Um efeito colateral é que isso pode tornar seu sistema mais seguro, porque você não pode esquecer de limpar o sinalizador de administrador para locais onde não é relevante.

Wilbert
fonte
40

Esse comentário foi mal formulado.

A questão não é se o objeto de usuário "decide" se é um administrador, um usuário de avaliação, um superusuário ou qualquer coisa dentro ou fora do comum. Obviamente, ele não decide nada disso, o sistema de software em geral, conforme implementado por você, decide. A questão é se a classe "usuário" deve representar as funções do principal que seus objetos representam ou se é responsabilidade de outra classe.

Kilian Foth
fonte
6
Ai (foi o meu comentário), mas você coloca muito melhor do que eu.
Marjan Venema #
Você está dizendo para modelar funções em uma classe Role separada, não na classe User? Você não está recomendando um sistema separado como o LDAP? E se todas as decisões tomadas pela classe Role exigirem a consulta do registro do usuário apropriado e não exigirem outras informações? Ainda deve ser separado do usuário e, em caso afirmativo, por quê?
GlenPeterson
2
@GlenPeterson: não necessariamente, apenas as tabelas de classes <> e geralmente há muito mais classes a serem discernidas do que as tabelas. Ser orientado a dados leva à definição de sublistas nas classes, no entanto, quando muitas vezes essas listas devem ser definidas em outro "razão" (pedidos) ou em alguma classe que passa no usuário (ou no pedido) para o qual recuperar o ... É mais uma questão de responsabilidades do que de verbos e substantivos.
Marjan Venema #
9
Pense no usuário como uma chave e nas permissões como um valor. Contextos diferentes podem ter permissões diferentes para cada usuário. Como as permissões são relevantes para o contexto, você precisaria alterar o usuário para cada contexto. Portanto, é melhor você colocar essa lógica em outro lugar (por exemplo, a classe de contexto).
Wilbert #
2
@ Wilbert: Contexto! Isso é particularmente esclarecedor! Sim, se você tiver mais de um contexto para essas permissões, tudo fará sentido para mim. Normalmente, vejo essas permissões em um único contexto e colocá-las no usuário é a solução mais simples nesse caso. Claramente, eles não pertencem a ele se forem aplicados de maneira diferente em um segundo contexto. Se você escrever isso como resposta, é provável que aceite. Obrigado.
precisa saber é o seguinte
30

Eu não teria escolhido redigir o comentário original da maneira como foi redigido, mas identifica um problema potencialmente legítimo.

Especificamente, as preocupações que justificam a separação são autenticação versus autorização .

Autenticação refere-se ao processo de logon e obtenção de uma identidade. É assim que os sistemas sabem quem você é e usados ​​para coisas como personalização, propriedade de objetos etc.

Autorização refere-se ao que você tem permissão para fazer , e isso (geralmente) não é determinado por quem você é . Em vez disso, é determinado por algumas políticas de segurança, como funções ou permissões, que não se importam com coisas como seu nome ou endereço de email.

Esses dois podem mudar ortogonalmente um para o outro. Por exemplo, você pode alterar o modelo de autenticação adicionando provedores OpenID / OpenAuth. E você pode alterar a política de segurança adicionando uma nova função ou alterando de RBAC para ABAC.

Se tudo isso ocorrer em uma classe ou abstração, seu código de segurança, que é uma das ferramentas mais importantes para mitigação de riscos , se torna ironicamente de alto risco.

Eu trabalhei com sistemas em que autenticação e autorização eram muito fortemente acopladas. Em um sistema, havia dois bancos de dados de usuários paralelos, cada um para um tipo de "função". A pessoa ou equipe que o projetou aparentemente nunca considerou que um único usuário físico pode estar em ambas as funções, ou que pode haver certas ações comuns a várias funções ou que podem haver problemas com colisões de ID do usuário. Este é um exemplo reconhecidamente extremo, mas foi / é incrivelmente doloroso de se trabalhar.

Microsoft e Sun / Oracle (Java) se referem ao agregado de informações de autenticação e autorização como Principal de Segurança . Não é perfeito, mas funciona razoavelmente bem. No .NET, por exemplo, você possui o IPrincipalque encapsula o primeiro IIdentity- sendo um objeto de política (autorização) enquanto o último é uma identidade (autenticação). Você pode questionar razoavelmente a decisão de colocar um dentro do outro, mas o importante é que a maioria dos códigos que você escreve será para apenas uma das abstrações, o que significa que é fácil testar e refatorar.

Não há nada errado com um User.IsAdmincampo ... a menos que haja também um User.Namecampo. Isso indicaria que o conceito "Usuário" não está definido corretamente e, infelizmente, esse é um erro muito comum entre os desenvolvedores que estão um pouco atrasados ​​no que diz respeito à segurança. Normalmente, a única coisa que deve ser compartilhada por identidade e política é o ID do usuário, que, por coincidência, é exatamente como ele é implementado nos modelos de segurança Windows e * nix.

É completamente aceitável criar objetos de invólucro que encapsulem a identidade e a política. Por exemplo, isso facilitaria a criação de uma tela do painel em que você precisa exibir uma mensagem de "olá", além de vários widgets ou links que o usuário atual tem permissão para acessar. Contanto que este invólucro apenas agrupe as informações de identidade e política e não pretenda possuí-las. Em outras palavras, desde que não seja apresentado como uma raiz agregada .

Um modelo de segurança simplista sempre parece uma boa idéia quando você cria um novo aplicativo pela primeira vez, por causa do YAGNI e tudo mais, mas quase sempre acaba voltando para te morder mais tarde, porque, surpresa surpresa, novos recursos são adicionados!

Portanto, se você souber o que é melhor para você, manterá as informações de autenticação e autorização separadas. Mesmo que a "autorização" no momento seja tão simples quanto um sinalizador "IsAdmin", você ainda estará melhor se não fizer parte da mesma classe ou tabela que as informações de autenticação, de modo que se e quando sua política de segurança precisar mudança, você não precisa fazer cirurgia reconstrutiva em seus sistemas de autenticação, que já funciona bem.

Aaronaught
fonte
11
Oh, eu gostaria de ter tido tempo para escrever isso. Por outro lado, eu provavelmente não teria sido capaz de dizer o mesmo que você fez sem muito mais pensamento da minha parte. Muitas vezes meu instinto me diz para seguir uma maneira ou de outra, mas explicar por que - para mim ou para outra pessoa - requer muito mais esforço e reflexão. Obrigado por este post. Teria de múltipla plussed, mas SE não vai me deixar ...
Marjan Venema
Isso soa chocantemente semelhante a um projeto no qual estou trabalhando e sua resposta me deu outra perspectiva. Muito obrigado!
Brandon
"Não há nada de errado com um campo User.IsAdmin ... a menos que haja também um campo User.Name" que você escreve. Mas o que isAdminé um método? Ele pode apenas delegar para um objeto cuja responsabilidade é acompanhar as permissões. O que é uma prática recomendada no design de um modelo relacional (quais campos uma entidade possui) não é necessariamente traduzível para o que é uma prática recomendada em um modelo OO (a quais mensagens um objeto pode responder).
KaptajnKold
@KaptajnKold: Eu continuo lendo esse sentimento em vários tópicos de perguntas e respostas neste fórum. Se um método existe, não importa se ele executa diretamente as operações ou as delega , ele ainda conta como uma responsabilidade, porque outras classes podem confiar nele . Você User.IsAdminpode muito bem ser uma linha que não faz nada além de chamar PermissionManager.IsAdmin(this.Id), mas se for referenciada por outras 30 classes, você não poderá alterá-lo ou removê-lo. É por isso que o SRP existe.
Aaronaught
@KaptajnKold, no que diz respeito aos modelos relacionais, não sei ao certo o que você está dizendo; é ainda pior ter um IsAdmin campo . É o tipo de campo que tende a permanecer por muito tempo depois que o sistema evoluiu para RBAC ou algo mais complexo, e gradualmente sai de sincronia com as permissões "reais", e as pessoas esquecem que não deveriam mais usá-lo, e ... bem, apenas diga não. Mesmo se você acha que possui apenas duas funções, "admin" e "non-admin", não o armazene como um campo booleano / bit, normalize adequadamente e tenha uma tabela de permissão.
Aaronaught #
28

Bem, no mínimo, viola o princípio da responsabilidade única . Deve haver mudanças (vários níveis de administrador, funções ainda mais diferentes?). Esse tipo de design seria bastante confuso e horrível de manter.

Considere a vantagem de separar o sistema de segurança da classe de usuário, para que ambos possam ter uma função única e bem definida. Você acaba com um design mais limpo e fácil de manter.

Zavior
fonte
2
@GlenPeterson: Não, os usuários podem muito bem existir sem segurança. E o meu nome, meu nome de exibição, meu endereço, meu endereço de e-mail etc. Onde você os colocaria? Identificação (autenticação) e autorização são completamente diferentes.
Marjan Venema
2
class Useré um componente essencial do trio de segurança de identificação, autenticação e autorização . Eles estão intimamente ligados ao nível do design, portanto, não é razoável que o código reflita essa interdependência.
precisa saber é o seguinte
11
A questão é: a classe User deve conter toda a lógica para lidar com o trigêmeo!
Zavior # 04/13
3
@ MSalters: se é uma API para essa lógica, ela sabe disso, mesmo que delegue em outro lugar para a lógica exata. O argumento é que Autorização (permissões) é sobre uma combinação de um usuário com outra coisa e, portanto, não deve fazer parte do usuário ou de outra coisa, mas parte dos sistemas de permissões que estão cientes dos usuários e de qualquer outra coisa. está recebendo permissão para.
Marjan Venema
2
Should there be changes- bem, não sabemos que haverá mudanças. YAGNI e tudo. Faça essas mudanças quando necessário, certo?
Izkata
10

Penso que a questão, talvez fracassada, é que isso coloca muito peso na Userclasse. Não, não há problema de segurança em ter um método User.isAdmin(), conforme sugerido nesses comentários. Afinal, se alguém for suficientemente profundo em seu código para injetar código malicioso em uma de suas classes principais, você terá sérios problemas. Por outro lado, não faz sentido Userdecidir se é um administrador para todos os contextos. Imagine que você adicione funções diferentes: moderadores para o fórum, editores que podem publicar postagens na primeira página, escritores que podem escrever conteúdo para os editores publicar, e assim por diante. Com a abordagem de colocá-lo no Userobjeto, você acabará com muitos métodos e muita lógica vivendo sobre o Userque não está diretamente relacionado à classe.

Em vez disso, você pode usar uma enumeração de diferentes tipos de permissões e uma PermissionsManagerclasse. Talvez você possa ter um método como PermissionsManager.userHasPermission(User, Permission), retornando um valor booleano. Na prática, pode parecer (em java):

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

É essencialmente equivalente ao método Rails before_filter, que fornece um exemplo desse tipo de verificação de permissões no mundo real.

asthasr
fonte
6
Como isso é diferente user.hasPermission(Permissions.EDITOR), exceto que é mais detalhado e processual em vez de OO?
Cefalópode #
9
@Arian: porque user.hasPermissionscoloca muitas responsabilidades na classe de usuário. É necessário que o usuário tenha conhecimento das permissões, enquanto um usuário (classe) deve existir sem esse conhecimento. Você também não deseja que uma classe de usuário saiba como imprimir ou exibir (criar um formulário), deixe isso para um renderizador / construtor de impressoras ou uma classe renderizador / construtor de impressoras.
Marjan Venema
4
user.hasPermission(P)é a API correta, mas essa é apenas a interface. Naturalmente, a decisão real deve ser tomada em um subsistema de segurança. Para abordar o comentário de syrion sobre o teste, durante o teste, você pode trocar esse subsistema. No entanto, o benefício do direto user.hasPermission(P)é que você não polui todo o código apenas para poder testar class User.
precisa saber é o seguinte
3
@MarjanVenema Por essa lógica, o Usuário permaneceria um objeto de dados sem nenhuma responsabilidade. Não estou dizendo que todo o gerenciamento de permissões deve ser implementado na classe de usuário, se houver necessidade de um sistema tão complexo. Você diz que um usuário não precisa saber sobre permissões, mas não vejo por que todas as outras classes precisam saber como resolver permissões para um usuário. Isso torna a zombaria e o teste muito mais difíceis.
Cefalópode #
6
@Arian: enquanto classes anêmicas são um cheiro, algumas classes simplesmente não têm nenhum comportamento ... O usuário, imho, é uma dessas classes que é melhor usada como parâmetro para um monte de outras coisas (que querem fazer coisas / tomar decisões com base em quem as pede), do que ser usado como um contêiner para todos os tipos de comportamento apenas porque parece conveniente codificá-lo dessa maneira.
Marjan Venema #
9

Object.isX () é usado para representar um relacionamento claro entre o objeto e X, que pode ser expresso como um resultado booleano, por exemplo:

Water.isBoiling ()

Circuit.isOpen ()

User.isAdmin () assume um único significado de 'Administrador' em todos os contextos do sistema , que um usuário é, em todas as partes do sistema, um administrador.

Embora pareça bastante simples, um programa do mundo real quase nunca se encaixa nesse modelo, certamente haverá um requisito para um usuário administrar [X] recursos, mas não [Y], ou para tipos mais distintos de administradores (um [projeto ] administrador versus administrador do [sistema]).

Essa situação geralmente requer uma investigação dos requisitos. Raramente, se alguma vez, um cliente desejará o relacionamento que User.isAdmin () realmente representa, então hesitaria em implementar qualquer solução desse tipo sem esclarecimentos.

FMJaguar
fonte
6

O pôster tinha apenas um design diferente e mais complicado em mente. Na minha opinião, User.isAdmin()está bem . Mesmo que mais tarde você introduza um modelo de permissões complicado, vejo poucas razões para mudar User.isAdmin(). Posteriormente, você pode dividir a classe User em um objeto que representa um usuário conectado e um objeto estático que representa os dados sobre o usuário. Ou você não pode. Economize amanhã para amanhã.

Kevin Cline
fonte
4
Save tomorrow for tomorrow. Sim. Quando você descobrir que o código fortemente acoplados você acabou de escrever tem rotulado você e você tem que reescrevê-lo porque você não tomar as extra de 20 minutos para dissociar-lo ...
MirroredFate
5
@MirroredFate: É perfeitamente possível ter um User.isAdminmétodo conectado a um mecanismo de permissão arbitrário sem acoplar a classe User a esse mecanismo específico.
kevin Cline
3
Para que o User.isAdmin seja acoplado a um mecanismo de permissão arbitrário, mesmo sem acoplar a um mecanismo específico, é necessário ... bem ... acoplamento. O mínimo seria para uma interface. E essa interface simplesmente não deveria estar lá. Ele está dando à classe de usuário (e interface) algo pelo qual ela simplesmente não deve ser responsável. Aaronaught explica muito melhor do que eu (assim como a combinação de todas as outras respostas a esta pergunta).
Marjan Venema
8
@MirroredFate: não me importo se preciso reescrever uma implementação simples para atender a requisitos mais complexos. Mas tive experiências muito ruins com APIs excessivamente complexas, projetadas para atender a requisitos futuros imaginados que nunca surgiram.
Kevin cline #
3
@kevincline Embora APIs excessivamente complexas sejam (por definição) uma coisa ruim, eu não estava sugerindo que alguém escrevesse APIs excessivamente complexas. Eu estava afirmando que essa Save tomorrow for tomorrowé uma péssima abordagem de design, pois cria problemas que seriam triviais se o sistema fosse implementado corretamente muito pior. De fato, essa mentalidade é o que resulta em APIs excessivamente complexas, à medida que camada após camada é adicionada para corrigir o design inicial ruim. Eu acho que minha posição é measure twice, cut once.
precisa
5

Não é realmente um problema ...

Não vejo problema com User.isAdmin(). Eu certamente prefiro isso a algo como PermissionManager.userHasPermission(user, permissions.ADMIN), que no santo nome do SRP torna o código menos claro e não agrega nada de valor.

Eu acho que o SRP está sendo interpretado um pouco para literalmente por alguns. Eu acho que é bom, até preferível que uma classe tenha uma interface rica. SRP significa apenas que um objeto deve delegar aos colaboradores qualquer coisa que não se enquadre em sua única responsabilidade. Se a função de administrador de um usuário for algo mais envolvido do que um campo booleano, pode fazer muito sentido que o objeto de usuário delegue a um PermissionsManager. Mas isso não significa que também não é uma boa ideia para um usuário manter seu isAdminmétodo. De fato, isso significa que, quando seu aplicativo passar de simples para complexo, você precisará alterar o código que usa o objeto Usuário. IOW, seu cliente não precisa saber o que é realmente necessário para responder à pergunta se um usuário é ou não um administrador.

... mas por que você realmente quer saber?

Dito isso, parece-me que você raramente precisa saber se um usuário é um administrador, exceto para poder responder a outra pergunta, como se um usuário pode ou não executar alguma ação específica, por exemplo, como atualizar um widget. Se for esse o caso, eu preferiria ter um método no Widget, como isUpdatableBy(User user):

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

Dessa forma, um Widget é responsável por saber quais critérios devem ser satisfeitos para que ele seja atualizado por um usuário, e um usuário é responsável por saber se é um administrador. Esse design comunica claramente suas intenções e facilita a transição para uma lógica de negócios mais complexa, se e quando chegar a hora.

[Editar]

O problema de não ter User.isAdmin()e usarPermissionManager.userHasPermission(...)

Pensei em adicionar minha resposta para explicar por que prefiro chamar um método no objeto Usuário a chamar um método em um objeto PermissionManager se eu quiser saber se um usuário é um administrador (ou tem uma função de administrador).

Eu acho que é justo supor que você sempre dependerá da classe User sempre que precisar fazer a pergunta : este usuário é um administrador? Essa é uma dependência da qual você não pode se livrar. Mas se você precisar passar o usuário para um objeto diferente para fazer uma pergunta sobre ele, isso cria uma nova dependência desse objeto em cima do objeto que você já possui no Usuário. Se a pergunta é muito solicitada, há muitos lugares onde você cria uma dependência extra, e muitos lugares você pode precisar mudar se alguma dependência exigir.

Compare isso com mover a dependência para a classe User. Agora, de repente, você tem um sistema em que o código do cliente (o código que precisa fazer a pergunta é este usuário e administrador ) não está associado à implementação de como essa pergunta é respondida. Você é livre para alterar completamente o sistema de permissões e só precisa atualizar um método em uma classe para fazê-lo. Todo o código do cliente permanece o mesmo.

Insistir em não ter um isAdminmétodo no Usuário por medo de criar uma dependência na classe Usuário no subsistema de permissões é, na minha opinião, semelhante a gastar um dólar para ganhar um centavo. Claro, você evita uma dependência na classe Usuário, mas com o custo de criar uma em todos os lugares em que precisa fazer a pergunta. Não é uma boa pechincha.

KaptajnKold
fonte
2
"SRP significa apenas que um objeto deve delegar aos colaboradores qualquer coisa que não caia na sua responsabilidade" - falso . A definição do SRP abrange tudo o que um objeto faz diretamente e tudo o que delega. Uma classe que executa apenas uma tarefa diretamente, mas executa 50 outras tarefas indiretamente via delegação, ainda está (provavelmente) violando o SRP.
Aaronaught 4/11
KaptajnKold: Eu marquei +1 porque concordo com você e me sinto um pouco culpado por isso. Sua postagem é adicionada à discussão, mas não responde à pergunta.
GlenPeterson
@GlenPeterson Bem, tentei abordar a parte da pergunta que foi 'o que parece feito 'direito'' Também: Você absolutamente não deve se sentir culpado por concordando comigo :)
KaptajnKold
11
@JamesSnell Talvez. E se. Mas esse é um detalhe de implementação que eu ainda limitaria dentro do método isAdmin no usuário. Dessa forma, o código do seu cliente não precisa ser alterado quando a "administração" de um Usuário evolui de um campo booleano para algo mais avançado. Você pode ter muitos lugares onde precisa saber se um Usuário é um administrador e não deseja alterá-lo sempre que o sistema de autorização for alterado.
KaptajnKold
11
@JamesSnell Talvez nos entendamos mal? A questão de saber se um usuário é ou não um administrador não é a mesma questão de saber se um usuário tem ou não permissão para executar uma ação específica . A resposta para a primeira pergunta é sempre independente do contexto. A resposta para o segundo depende muito do contexto. Foi isso que tentei abordar na segunda metade da minha resposta original.
precisa saber é o seguinte
1

Essa discussão me lembrou uma postagem de blog de Eric Lippert, que foi trazida à minha atenção em uma discussão semelhante sobre design de classe, segurança e correção.

Por que tantas das classes de estrutura são seladas?

Em particular, Eric levanta a questão:

4) Seguro. o ponto principal do polimorfismo é que você pode passar objetos que se parecem com animais, mas na verdade são girafas. Existem possíveis problemas de segurança aqui.

Toda vez que você implementa um método que leva uma instância de um tipo não lacrado, você DEVE escrever esse método para ser robusto diante de instâncias potencialmente hostis desse tipo. Você não pode confiar em nenhum invariável que você saiba ser verdade em SUAS implementações, porque alguma página hostil da Web pode subclassificar sua implementação, substituir os métodos virtuais para fazer coisas que atrapalham sua lógica e a passar. Toda vez que selo uma classe , Posso escrever métodos que usam essa classe com a confiança de que sei o que essa classe faz.

Isso pode parecer uma tangente, mas acho que se encaixa no ponto que outros pôsteres levantaram sobre o princípio de responsabilidade única do SOLID . Considere a seguinte classe maliciosa como um motivo para não implementar um método ou propriedade.User.IsAdmin

public class MyUser : User
{

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

    // Anything else we can break today?

}

É um pouco complicado: ninguém ainda sugeriu a criação de IsAmdminum método virtual, mas isso poderia acontecer se sua arquitetura de usuário / função acabasse sendo estranhamente complexa. (Ou talvez não. Considere public class Admin : User { ... }: essa classe pode ter exatamente esse código acima.) Colocar um binário malicioso no lugar não é um vetor de ataque comum e tem muito mais potencial para caos do que uma biblioteca de usuário obscura - então, novamente, isso pode ser o bug de escalonamento de privilégios que abre as portas para as verdadeiras travessuras. Por fim, se uma instância binária ou de objeto mal-intencionado chegou ao tempo de execução, não é muito difícil imaginar o "Sistema de Privilégios ou Segurança" sendo substituído de maneira semelhante.

Mas levando em consideração o ponto de Eric, se você colocar seu código de usuário em um tipo específico de sistema vulnerável, talvez você tenha acabado de perder o jogo.

Ah, e para ser mais preciso, eu concordo com o postulado da pergunta: “Um usuário não deve decidir se é um administrador ou não. Os privilégios ou o sistema de segurança devem. ”Um User.IsAdminmétodo é uma má idéia, se você estiver executando o código no sistema, não permanecerá 100% no controle do código - deve fazê-lo Permissions.IsAdmin(User user).

Patrick M
fonte
Hã? Como isso é pertinente? Não importa onde você coloque a política de segurança, você sempre poderá subclassificá-la com algo inseguro. Não importa se algo é um Usuário, Principal, Política, PermissionSet ou qualquer outra coisa. É uma questão totalmente não relacionada.
precisa saber é o seguinte
É um argumento de segurança em favor da responsabilidade única. Para responder diretamente ao seu argumento, o módulo de segurança e permissões será uma classe selada, mas certos designs podem desejar que a classe User seja virtual e herdada. É o último ponto do artigo do blog (portanto, talvez o menos provável / importante?), E talvez eu esteja conflitando um pouco com os problemas, mas, dada a fonte, é claro que o polimorfismo pode ser uma ameaça à segurança e, portanto, limitar as responsabilidades de uma determinada classe / objeto é mais segura.
Patrick M
Não sei por que você diz isso. Muitas estruturas de segurança e permissões são altamente extensíveis. A maioria dos principais tipos de .NET relacionados à segurança (IPrincipal, IIdentity, ClaimSet etc.) não são apenas não lacrados, mas também são interfaces ou classes abstratas, para que você possa conectar o que quiser. O que Eric está falando é que você não gostaria que algo parecesse System.Stringherdável porque todos os tipos de código de estrutura crítica fazem suposições muito específicas sobre como ele funciona. Na prática, a maioria dos "códigos de usuário" (incluindo segurança) é herdável para permitir duplas de teste.
Aaronaught #
11
De qualquer forma, o polimorfismo não é mencionado em nenhum lugar da pergunta e, se você seguir o link do autor para onde o comentário original foi feito, fica claro que não se referia a herança ou mesmo a invariantes de classe em geral - era sobre responsabilidades.
Aaronaught
Sei que não foi mencionado, mas o princípio das responsabilidades de classe está diretamente relacionado ao polimorfismo. Se não houver um IsAdminmétodo para substituir / cooptar, não haverá uma falha de segurança em potencial. Revendo os métodos dessas interfaces de sistema IPrincipale IIdentity, é claro que os designers discordam de mim, e por isso eu concordo.
Patrick M
0

O problema aqui é:

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

Você configurou sua segurança corretamente; nesse caso, o método privilegiado deve verificar se o chamador tem autoridade suficiente - nesse caso, a verificação é supérflua. Ou você não tem e está confiando em uma classe não privilegiada para tomar suas próprias decisões sobre segurança.

James Anderson
fonte
4
O que é uma classe sem privilégios?
quer
11
Realmente não há nada que você possa fazer para impedir que esse tipo de código seja gravado. Não importa como você cria seu aplicativo, em algum lugar haverá uma verificação explícita como esta, e alguém poderá escrevê-lo como inseguro. Mesmo que o seu sistema de permissões seja tão simplificado quanto decorar um método com um atributo de função ou permissão - alguém pode lançar incorretamente uma permissão "pública" ou "todos". Então, eu realmente não vejo como isso torna um método User.IsAdmin especificamente um problema.
Aaronaught