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. ”

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.

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.

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.

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.

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.

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.

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.

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ã.

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.

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).

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.