Lo que significa "un usuario no debe decidir si es un administrador o no". Los privilegios o el sistema de seguridad deberían ".

Piense en el usuario como clave y los permisos como valor.

Los diferentes contextos pueden tener diferentes permisos para cada usuario. Como los permisos son relevantes para el contexto, tendría que cambiar el usuario para cada contexto. Por lo tanto, es mejor que coloques esa lógica en otro lugar (por ejemplo, la clase de contexto) y simplemente busques los permisos donde sea necesario.

Un efecto secundario es que puede hacer que su sistema sea más seguro, porque no puede olvidar borrar el indicador de administración para los lugares donde no es relevante.

Ese comentario estaba mal redactado.

El punto no es si el objeto de usuario "decide" si es un administrador, un usuario de prueba, un superusuario o cualquier cosa dentro o fuera de lo común. Obviamente, no decide ninguna de esas cosas, el sistema de software en general, según lo implementado por usted, sí lo hace. El punto es si el "usuario" clase debería representar los roles del principal que representan sus objetos, o si esto es responsabilidad de otra clase.

No hubiera elegido redactar el comentario original de la forma en que estaba redactado, pero identifica un problema legítimo potencialmente .

Específicamente, las preocupaciones que justifican la separación son autenticación frente a autorización .

Autenticación se refiere al proceso de inicio de sesión y obtención de una identidad. Es así como los sistemas saben quién eres y se usan para cosas como personalización, propiedad de objetos, etc.

Autorización se refiere a lo que se te permite hacer , y esto (generalmente) no está ni determinado por quién eres . En su lugar, está determinada por alguna política de seguridad como roles o permisos, que no se preocupa por cosas como su nombre o dirección de correo electrónico.

Estos dos pueden cambiar ortogonalmente entre sí. Por ejemplo, puede cambiar el modelo de autenticación agregando proveedores de OpenID / OpenAuth. Y puede cambiar la política de seguridad agregando una nueva función o cambiando de RBAC a ABAC.

Si todo esto entra en una clase o abstracción, entonces su código de seguridad, que es una de sus herramientas más importantes para mitigar el riesgo , se convierte, irónicamente, en alto riesgo.

He trabajado con sistemas en los que la autenticación y la autorización estaban demasiado vinculadas. En un sistema, había dos bases de datos de usuarios paralelas, cada una para un tipo de "función". La persona o el equipo que lo diseñó aparentemente nunca consideró que un solo usuario físico pudiera estar en ambos roles, o que podría haber ciertas acciones que fueran comunes a múltiples roles, o que podría haber problemas con las colisiones de ID de usuario. Este es un ejemplo ciertamente extremo, pero fue / es increíblemente doloroso trabajar con él.

Microsoft y Sun / Oracle (Java) se refieren al agregado de la información de autenticación y autorización como Director de seguridad . No es perfecto, pero funciona razonablemente bien. En .NET, por ejemplo, tiene IPrincipal , que encapsula el IIdentity - el primero es un objeto policy (autorización) mientras que el último es un identidad (autenticación). Podría cuestionar razonablemente la decisión de poner uno dentro del otro, pero lo importante es que la mayoría del código que escriba será solo para una de las abstracciones , lo que significa que es Fácil de probar y refactorizar.

No hay nada de malo en el campo User.IsAdmin ... a menos que también haya un campo User.Name . Esto indicaría que el concepto "Usuario" no está correctamente definido y esto es, lamentablemente, un error muy común entre los desarrolladores que están un poco mojados detrás de las orejas cuando se trata de seguridad. Por lo general, lo único que debe ser compartido por identidad y política es el ID de usuario, que, no por casualidad, es exactamente cómo se implementa tanto en Windows como en * nix modelos de seguridad.

Es completamente aceptable crear objetos de envoltura que encapsulan identidad y política. Por ejemplo, facilitaría la creación de una pantalla de panel de control en la que deba mostrar un mensaje de "saludo" además de varios widgets o enlaces a los que el usuario actual puede acceder. Siempre y cuando este envoltorio simplemente ajuste la información de identidad y política, y no pretenda poseerlo. En otras palabras, siempre que no se presente como una raíz agregada .

Un modelo de seguridad simplista siempre parece como una buena idea cuando estás diseñando una nueva aplicación, debido a YAGNI y todo eso, pero casi siempre termina volviéndote a morder después. porque, sorpresa sorpresa, se agregan nuevas funciones!

Entonces, si sabe qué es lo mejor para usted, mantendrá la información de autenticación y autorización por separado. Incluso si la "autorización" en este momento es tan simple como un indicador "IsAdmin", todavía estará mejor si no es parte de la misma clase o tabla que la información de autenticación, de modo que, si y cuando su política de seguridad necesite Cambie, no necesita realizar una cirugía reconstructiva en sus sistemas de autenticación, que ya funciona bien.

Bueno, al menos viola el principio de responsabilidad única . En caso de que haya cambios (varios niveles de administración, ¿aún más roles diferentes?), Este tipo de diseño sería bastante complicado y de difícil mantenimiento.

Considere la ventaja de separar el sistema de seguridad de la clase de usuario, para que ambos puedan tener un rol único y bien definido. Usted termina con un diseño general más limpio y fácil de mantener.

Creo que el problema, tal vez expresado de manera deficiente, es que pone demasiado peso en la clase User . No, no hay ningún problema de seguridad con el método User.isAdmin() , como se sugirió en esos comentarios. Después de todo, si alguien es lo suficientemente profundo en su código para inyectar código malicioso en una de sus clases principales, tiene graves problemas. Por otro lado, no tiene sentido que User decida si es un administrador para cada contexto. Imagina que agregas diferentes roles: moderadores para tu foro, editores que pueden publicar publicaciones en la página principal, escritores que pueden escribir contenido para que los editores publiquen, etc. Con el enfoque de ponerlo en el objeto User , terminarás con demasiados métodos y demasiada lógica viviendo en el User que no está directamente relacionado con la clase.

En su lugar, podría usar una enumeración de diferentes tipos de permisos y una clase PermissionsManager . Tal vez podría tener un método como PermissionsManager.userHasPermission(User, Permission) , que devuelve un valor booleano. En la práctica, podría parecer (en java):

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

Es esencialmente equivalente al método Rails before_filter , que proporciona un ejemplo de este tipo de verificación de permisos en el mundo real.

Object.isX () se usa para representar una relación clara entre el objeto y X, que se puede expresar como un resultado booleano, por ejemplo:

Water.isBoiling ()

Circuit.isOpen ()

User.isAdmin () asume un solo significado de 'Administrador' dentro de cada contexto del sistema , que un usuario es, en cada parte del sistema, un administrador.

Aunque suene lo suficientemente simple, un programa del mundo real casi nunca se ajustará a este modelo, sin duda habrá un requisito para que un usuario administre el recurso [X] pero no [Y], o para tipos más distintos de administradores ( administrador de [proyecto] frente a administrador de [sistema]).

Esta situación generalmente requiere una investigación de los requisitos. Rara vez, si alguna vez, un cliente querrá la relación que User.isAdmin () representa realmente, por lo que dudaría en implementar cualquier solución de este tipo sin una aclaración.

El cartel simplemente tenía un diseño diferente y más complicado en mente. En mi opinión, User.isAdmin() está bien . Incluso si más tarde introduces un modelo de permisos complicado, veo pocas razones para mover User.isAdmin() . Más adelante, es posible que desee dividir la clase de Usuario en un objeto que represente a un usuario conectado y un objeto estático que representa los datos sobre el usuario. O puede que no. Guarda mañana para mañana.

No es realmente un problema ...

No veo ningún problema con User.isAdmin() . Ciertamente lo prefiero a algo como PermissionManager.userHasPermission(user, permissions.ADMIN) , que en el santo nombre de SRP hace que el código sea menos claro y no agregue nada de valor.

Creo que algunos están interpretando el SRP un poco literalmente. Creo que está bien, incluso es preferible que una clase tenga una interfaz rica. SRP solo significa que un objeto debe delegar en los colaboradores cualquier cosa que no esté dentro de su única responsabilidad. Si el rol de administrador de un usuario es algo más complejo que un campo booleano, es muy posible que el objeto del usuario delegue en un PermissionsManager. Pero eso no significa que no sea una buena idea que un usuario mantenga su método isAdmin . De hecho, significa que cuando su aplicación se gradúa de simple a compleja, debe cambiar el código que utiliza el objeto Usuario. IOW, su cliente no necesita saber lo que realmente se necesita para responder a la pregunta de si un usuario es un administrador o no.

... pero ¿por qué realmente quieres saberlo?

Dicho esto, me parece que rara vez se necesita saber si un usuario es un administrador, excepto para poder responder alguna otra pregunta, como si un usuario tiene permiso para realizar alguna acción específica, como actualizar un Widget Si ese es el caso, preferiría tener un método en Widget, como isUpdatableBy(User user) :

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

De esa manera, un Widget es responsable de saber qué criterios deben cumplirse para que un usuario pueda actualizarlo, y un usuario es responsable de saber si es un administrador. Este diseño se comunica claramente acerca de sus intenciones y hace que sea más fácil pasar a una lógica de negocios más compleja cuando llegue ese momento.

[Editar]

El problema con no tiene User.isAdmin() y usa PermissionManager.userHasPermission(...)

Pensé que agregaría a mi respuesta para explicar por qué prefiero llamar a un método en el objeto Usuario a llamar a un método en un objeto PermissionManager si quiero saber si un usuario es un administrador (o tiene un rol de administrador) .

Creo que es justo suponer que siempre dependerá de la clase de Usuario donde sea necesario hacer la pregunta ¿este usuario es un administrador? Es una dependencia de la que no puede deshacerse. Pero si necesita pasar el usuario a un objeto diferente para hacer una pregunta al respecto, eso crea una nueva dependencia de ese objeto además del que ya tiene sobre el usuario. Si la pregunta se hace mucho, hay muchos lugares donde se crea una dependencia adicional, y es posible que tenga que cambiar muchos lugares si cualquiera de ellos depende de la dependencia.

Compare esto con mover la dependencia a la clase Usuario. Ahora, de repente, tiene un sistema donde el código del cliente (código que debe hacer la pregunta es este usuario y administrador ) no está acoplado a la implementación de cómo se responde a esta pregunta. Usted es libre de cambiar el sistema de permisos completamente, y solo tiene que actualizar un método en una clase para hacerlo. Todo el código del cliente sigue siendo el mismo.

Insistir en no tener un método isAdmin en el Usuario por temor a crear una dependencia en la clase de Usuario en el subsistema de permisos, es, en mi opinión, similar a gastar un dólar para ganar un centavo. Claro, usted evita una dependencia en la clase de Usuario, pero al costo de crear una en cada lugar donde necesita hacer la pregunta. No es una buena ganga.

Esta discusión me recordó a una publicación de blog de Eric Lippert, que me llamó la atención en una discusión similar sobre diseño, seguridad y corrección de clase.

¿Por qué están selladas muchas de las clases marco?

En particular, Eric plantea el punto:

  

4) Seguro. El punto central del polimorfismo es que puedes pasar objetos que parecen animales pero en realidad son jirafas. Hay posibles problemas de seguridad aquí.

     

Cada vez que implementas un método que toma una instancia de un tipo sin sellar, DEBES escribir que el método sea robusto frente a instancias potencialmente hostiles de ese tipo. No puedes confiar en las invariantes que sabes que son verdaderas de TUS implementaciones, ya que alguna página web hostil puede subclasificar tu implementación, anular los métodos virtuales para hacer cosas que enturbian tu lógica y pasarlas. Cada vez que sello una clase , Puedo escribir métodos que usen esa clase con la confianza de que sé lo que hace esa clase.

Esto puede parecer una tangente, pero creo que encaja con el punto que otros pósters han planteado sobre el SOLID principio de responsabilidad única . Considere la siguiente clase maliciosa como una razón para no implementar un método o propiedad User.IsAdmin .

public class MyUser : User
{

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

    // Anything else we can break today?

}

Por supuesto, es un poco exagerado: nadie sugirió aún hacer de IsAmdmin un método virtual, pero podría suceder si su arquitectura de usuario / rol terminara siendo extrañamente compleja. (O tal vez no. Considere public class Admin : User { ... } : tal clase podría tener exactamente ese código arriba). Poner un binario malicioso en su lugar no es un vector de ataque común y tiene mucho más potencial para el caos que una biblioteca de usuario poco clara. de nuevo, esto podría ser el error Escalamiento de privilegios que abre la puerta a los verdaderos chanchullos. Finalmente, si una instancia de objeto o binario malintencionado se abrió camino en el tiempo de ejecución, no es mucho más difícil imaginar que el "Privilegio o Sistema de seguridad" se reemplace de una manera similar.

Pero teniendo en cuenta el punto de Eric, si pones tu código de usuario en un tipo particular de sistema vulnerable, tal vez acabas de perder el juego.

Ah, y solo para ser precisos, estoy de acuerdo con el postulado de la pregunta: "Un usuario no debe decidir si es un administrador o no". Los privilegios o el sistema de seguridad deberían ". Un método User.IsAdmin es una mala idea. Si está ejecutando el código en el sistema, no tiene el control del código al 100%; en su lugar, debería hacer Permissions.IsAdmin(User user) .

El problema aquí es:

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

O bien, ha configurado su seguridad correctamente, en cuyo caso el método privilegiado debería verificar si la persona que llama tiene la autoridad suficiente, en cuyo caso la verificación es superflua. O bien, no lo ha hecho y está confiando en una clase sin privilegios para tomar sus propias decisiones sobre seguridad.