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

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ê teria que alterar o usuário para cada contexto. Então é melhor você colocar essa lógica em outro lugar (por exemplo, a classe de contexto) e apenas procurar as permissões onde 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 em que não é relevante.

Esse comentário foi mal formulado.

O ponto não é se o objeto de usuário "decide" se é um administrador, um usuário de teste, um superusuário ou qualquer coisa dentro ou fora do comum. Obviamente, ele não decide nenhuma dessas coisas, o sistema de software em geral, como implementado por você, faz. O ponto é se a classe "usuário" deve representar as funções do principal que seus objetos representam, ou se isso é uma responsabilidade de outra classe.

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

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

Autenticação refere-se ao processo de login e obtenção de uma identidade. É como os sistemas conhecem quem você é e são usados para coisas como personalização, propriedade de objetos, etc.

Autorização refere-se a o que você pode fazer , e isso (geralmente) não é determinado por quem você é . Em vez disso, ele é determinado por alguma política de segurança, como funções ou permissões, que não se importam com coisas como seu nome ou endereço de e-mail.

Estes dois podem mudar ortogonalmente entre si. Por exemplo, você pode alterar o modelo de autenticação adicionando fornecedores OpenID / OpenAuth. E você pode alterar a política de segurança adicionando uma nova função ou mudando de RBAC para ABAC.

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

Trabalhei com sistemas em que a autenticação e a autorização estavam muito juntas. 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 projetou isso aparentemente nunca considerou que um único usuário físico pode estar em ambas as funções, ou que pode haver determinadas ações que eram comuns a várias funções, ou que poderia haver problemas com colisões de User ID. Este é um exemplo reconhecidamente extremo, mas foi incrivelmente doloroso de trabalhar.

Microsoft e Sun / Oracle (Java) referem-se 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ê tem IPrincipal , que encapsula o IIdentity - o primeiro sendo um objeto política (autorização) enquanto o último é um > identidade (autenticação). Você poderia razoavelmente questionar a decisão de colocar um dentro do outro, mas o importante é que a maioria do código que você escreve será para apenas uma das abstrações , o que significa que é fácil de testar e refatorar.

Não há nada errado com um campo User.IsAdmin ... a menos que também haja um campo User.Name . Isso indicaria que o conceito "Usuário" não está adequadamente definido e isso é, infelizmente, um erro muito comum entre os desenvolvedores que estão um pouco molhados atrás das orelhas quando se trata de segurança. Normalmente, a única coisa que deve ser compartilhada por identidade e política é a ID do usuário, que, não coincidentemente, é exatamente como ela é implementada no Windows e no * nix modelos de segurança.

É completamente aceitável criar objetos wrapper que encapsulem identidade e política. Por exemplo, isso facilitaria a criação de uma tela de painel em que você precisa exibir uma mensagem "hello" além de vários widgets ou links que o usuário atual tem permissão para acessar. Contanto que esse wrapper apenas contenha as informações de identidade e política, e não reivindique a propriedade dele. Em outras palavras, desde que não esteja sendo apresentado como uma raiz agregada .

Um modelo de segurança simplista sempre parece uma boa idéia quando você cria um novo aplicativo, 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" agora 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, para que, se e quando sua política de segurança precisar mudar, você não precisa fazer cirurgia reconstrutiva em seus sistemas de autenticação que já funciona bem.

Bem, pelo menos viola o princípio de responsabilidade única . Se houver mudanças (vários níveis de administração, papéis ainda mais diferentes?), Esse tipo de design seria bastante confuso e horrível de se manter.

Considere a vantagem de separar o sistema de segurança da classe de usuário, para que ambos tenham um papel único e bem definido. Você acaba com um design mais limpo e fácil de manter.

Eu acho que a questão, talvez formulada de forma inadequada, é que ela coloca muito peso na classe User . Não, não há nenhum problema de segurança em ter um método User.isAdmin() , como foi sugerido nesses comentários. Afinal, se alguém tiver profundidade suficiente em seu código para injetar código malicioso em uma de suas principais classes, você terá sérios problemas. Por outro lado, não faz sentido para o User decidir se é um administrador para cada contexto. Imagine que você inclua diferentes funções: moderadores para seu fórum, editores que podem publicar postagens na primeira página, escritores que podem escrever conteúdo para os editores publicarem, e assim por diante. Com a abordagem de colocá-lo no objeto User , você acabará com muitos métodos e muita lógica vivendo no User que não está diretamente relacionado à classe.

Em vez disso, você pode usar um enum de diferentes tipos de permissões e uma PermissionsManager de classe. Talvez você possa ter um método como PermissionsManager.userHasPermission(User, Permission) , retornando um valor booleano. Na prática, pode parecer com (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.

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

Water.isBoiling ()

Circuit.isOpen ()

User.isAdmin () assume um único significado de 'Administrador' dentro de todo contexto do sistema , que um usuário é, em cada parte do sistema, um administrador.

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

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

O cartaz meramente tinha um design diferente e mais complicado em mente. Na minha opinião, User.isAdmin() é bom . Mesmo se você introduzir mais tarde algum modelo de permissões complicado, vejo poucos motivos para mover User.isAdmin() . Posteriormente, convém dividir a classe Usuário em um objeto que representa um usuário conectado e um objeto estático representando os dados sobre o usuário. Ou você não pode. Salve amanhã para amanhã.

Não é realmente um problema ...

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

Eu acho que o SRP está sendo interpretado um pouco literalmente por alguns. Eu acho que é bom, até mesmo 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 encaixe em sua única responsabilidade. Se a função administrativa de um usuário é algo mais envolvido do que um campo booleano, pode muito bem fazer sentido para o objeto de usuário delegar a um PermissionsManager. Mas isso não significa que também não seja uma boa idéia para um usuário manter o método isAdmin . Na verdade, isso significa que, quando seu aplicativo graduar do simples para o 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 sobre se um usuário é ou não um administrador.

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

Dito isso, parece que você raramente precisa saber se um usuário é um administrador, exceto para poder responder a alguma outra pergunta, como se um usuário pode ou não executar alguma ação específica, por exemplo, atualizar um Ferramenta. 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 transformação em lógica de negócios mais complexa, se e quando chegar a hora.

[Editar]

O problema com não ter User.isAdmin() e usar PermissionManager.userHasPermission(...)

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

Acho justo presumir que você sempre dependerá da classe User sempre que precisar fazer a pergunta esse 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 daquele que você já tem no usuário. Se a pergunta for muito perguntada, são muitos lugares onde você cria uma dependência extra, e há muitos lugares que você pode ter que mudar se alguma das dependências exigir isso.

Compare isso com a transferência da dependência para a classe User. Agora, de repente você tem um sistema onde o código do cliente (código que precisa fazer a pergunta é este usuário um administrador ) não está acoplado à implementação de como essa questão é respondida. Você é livre para alterar completamente o sistema de permissões, e você 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 método isAdmin no usuário por medo de criar uma dependência na classe Usuário no subsistema de permissões é, na minha opinião, como gastar um dólar para ganhar um centavo. Claro, você evita uma dependência na classe User, mas ao custo de criar uma em cada lugar onde precisa fazer a pergunta. Não é um bom negócio.

Esta discussão me lembrou de uma postagem no 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 classes de frameworks são seladas?

Em particular, Eric levanta o ponto:

4) Secure. the whole point of polymorphism is that you can pass around objects that look like Animals but are in fact Giraffes. There are potential security issues here.

Every time you implement a method which takes an instance of an unsealed type, you MUST write that method to be robust in the face of potentially hostile instances of that type. You cannot rely upon any invariants which you know to be true of YOUR implementations, because some hostile web page might subclass your implementation, override the virtual methods to do stuff that messes up your logic, and passes it in. Every time I seal a class, I can write methods that use that class with the confidence that I know what that class does.

Isso pode parecer uma tangente, mas acho que se encaixa com o ponto que outros cartazes levantaram sobre o SOLID princípio de responsabilidade única . Considere a seguinte classe maliciosa como uma razão 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?

}

Concedido, é um pouco exagerado: ninguém ainda sugeriu fazer IsAmdmin um 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 o caos do que uma biblioteca de usuários obscura - então Novamente, esse pode ser o bug Privilege Escalation que abre as portas para as verdadeiras travessuras. Finalmente, se um binário mal-intencionado ou uma instância de objeto encontrar seu caminho no tempo de execução, não é muito mais difícil imaginar o "Sistema de Privilégios ou Segurança" sendo substituído de maneira semelhante.

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

Ah, e só para ser preciso para o registro, eu concordo com o postulado na pergunta: “Um usuário não deve decidir se é um administrador ou não. O sistema Privileges or Security deveria. ”Um método User.IsAdmin é uma má ideia se você estiver executando o código no sistema que não permanece com 100% de controle do código - você deve fazer Permissions.IsAdmin(User user) .

O problema aqui é:

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

Você configurou sua segurança corretamente, caso em que 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.