Qu'entend-on par «Un utilisateur ne devrait pas décider s'il s'agit d'un administrateur ou non. Le système de privilèges ou de sécurité devrait. "

L'exemple utilisé dans la question Transmettre les données minimales nues à une fonction permet de déterminer le meilleur moyen de déterminer si l'utilisateur est un administrateur ou non. Une réponse commune était:

user.isAdmin()

Cela a provoqué un commentaire qui a été répété plusieurs fois et voté plusieurs fois:

Un utilisateur ne devrait pas décider s'il s'agit d'un administrateur ou non. Le système de privilèges ou de sécurité devrait. Quelque chose est étroitement lié à une classe ne signifie pas que c'est une bonne idée de faire partie de cette classe.

J'ai répondu,

L'utilisateur ne décide rien. L'objet / la table utilisateur stocke des données sur chaque utilisateur. Les utilisateurs réels ne peuvent pas tout changer d'eux-mêmes.

Mais ce n'était pas productif. Clairement, il existe une différence de perspective sous-jacente qui rend la communication difficile. Quelqu'un peut-il m'expliquer pourquoi user.isAdmin () est mauvais et dessiner un bref aperçu de ce à quoi cela ressemble "bien"?

En réalité, je ne vois pas l'avantage de séparer la sécurité du système qu'il protège. Tout texte sur la sécurité indiquera que la sécurité doit être intégrée dans un système dès le début et prise en compte à chaque étape du développement, du déploiement, de la maintenance et même de la fin de vie. Ce n'est pas quelque chose qui peut être verrouillé sur le côté. Mais 17 votes jusqu'à présent sur ce commentaire indiquent qu'il me manque quelque chose d'important.

Pensez à l'utilisateur en tant que clé et aux autorisations en tant que valeur.

Des contextes différents peuvent avoir des autorisations différentes pour chaque utilisateur. Les autorisations étant liées au contexte, vous devez changer d'utilisateur pour chaque contexte. Il est donc préférable de placer cette logique ailleurs (par exemple, la classe de contexte) et de rechercher les autorisations si nécessaire.

Un effet secondaire est que cela pourrait rendre votre système plus sécurisé, car vous ne pouvez pas oublier d'effacer l'indicateur admin pour les endroits où il n'est pas pertinent.

Ce commentaire était mal formulé.

Il ne s'agit pas de savoir si l'objet utilisateur "décide" s'il s'agit d'un administrateur, d'un utilisateur test, d'un superutilisateur ou de tout autre élément entrant ou sortant de l'ordinaire. De toute évidence, rien de tout cela n’est décidé, c’est le système logiciel dans son ensemble, tel que vous le mettez en œuvre. Le point est de savoir si la classe "utilisateur" doit représenter les rôles du principal que représentent ses objets ou s'il s'agit d'une responsabilité d'une autre classe.

Je n'aurais pas choisi de formuler le commentaire original de la manière dont il était libellé, mais cela identifie un problème potentiellement légitime.

Plus précisément, les préoccupations qui justifient la séparation sont l’ authentification contre l’ autorisation .

L'authentification fait référence au processus de connexion et d'obtention d'une identité. C’est ainsi que les systèmes savent qui vous êtes et sont utilisés pour des tâches telles que la personnalisation, la propriété d’objets, etc.

L'autorisation fait référence à ce que vous êtes autorisé à faire , et ceci (généralement) n'est pas déterminé par votre identité . Au lieu de cela, il est déterminé par certaines règles de sécurité, telles que les rôles ou les autorisations, qui ne se soucient pas de choses telles que votre nom ou votre adresse électronique.

Ces deux peuvent changer orthogonalement l'un à l'autre. Par exemple, vous pouvez modifier le modèle d'authentification en ajoutant des fournisseurs OpenID / OpenAuth. Vous pouvez également modifier la stratégie de sécurité en ajoutant un nouveau rôle ou en passant de RBAC à ABAC.

Si tout cela est classé dans une classe ou une abstraction, votre code de sécurité, qui est l’un de vos principaux outils d’ atténuation des risques , devient, paradoxalement, à haut risque.

J'ai travaillé avec des systèmes où authentification et autorisation étaient trop étroitement couplées. Dans un système, il y avait deux bases de données d'utilisateurs parallèles, chacune pour un type de "rôle". La personne ou l'équipe qui l'a conçu ne s'est apparemment jamais rendu compte qu'un seul utilisateur physique pouvait occuper les deux rôles, qu'il pouvait y avoir certaines actions communes à plusieurs rôles ou qu'il pouvait y avoir des problèmes de collisions d'ID utilisateur. C’est certes un exemple extrême, mais c’était / était incroyablement pénible de travailler avec.

Microsoft et Sun / Oracle (Java) font référence à l' ensemble des informations d'authentification et d'autorisation en tant que principal de sécurité . Ce n'est pas parfait, mais cela fonctionne raisonnablement bien. Dans .NET, par exemple, vous avez IPrincipal, qui encapsule le IIdentity- le premier étant une politique objet (autorisation) , alors que celle - ci est une identité (authentification). Vous pouvez raisonnablement remettre en question la décision de placer l'un à l' intérieur de l'autre, mais l'important est que la plupart du code que vous écrivez ne soit utilisé que pour l'une des abstractions, ce qui signifie qu'il est facile de tester et de refactoriser.

Il n'y a rien de mal avec un User.IsAdminchamp ... à moins qu'il y ait aussi un User.Namechamp. Cela indiquerait que le concept "d'utilisateur" n'est pas correctement défini et qu'il s'agit malheureusement d'une erreur très courante parmi les développeurs qui sont un peu mouillés derrière les oreilles en matière de sécurité. Généralement, l' identité et la stratégie ne doivent être partagées que par l'ID utilisateur. Ce n'est pas une coïncidence si c'est exactement comment il est implémenté dans les modèles de sécurité Windows et * nix.

Il est tout à fait acceptable de créer des objets wrapper qui encapsulent à la fois l'identité et la stratégie. Par exemple, cela faciliterait la création d'un écran de tableau de bord dans lequel vous devez afficher un message "hello" en plus de divers widgets ou liens auxquels l'utilisateur actuel est autorisé à accéder. Tant que cette enveloppe juste enveloppe les informations d'identité et de la politique, et ne prétend pas posséder. En d'autres termes, tant que cela n'est pas présenté comme une racine globale .

Un modèle de sécurité simpliste semble toujours être une bonne idée lorsque vous concevez une nouvelle application, à cause de YAGNI et ainsi de suite, mais il finit presque toujours par vous revenir plus tard, car, surprise, de nouvelles fonctionnalités sont ajoutées!

Ainsi, si vous savez ce qui vous convient le mieux, vous allez séparer les informations d'authentification et d'autorisation. Même si "l'autorisation" est aussi simple qu'un indicateur "IsAdmin", vous serez toujours mieux s'il ne fait pas partie de la même classe ou du même tableau que les informations d'authentification. Ainsi, si et quand votre politique de sécurité doit change, vous n’avez pas besoin de faire de chirurgie reconstructive sur vos systèmes d’authentification qui fonctionne déjà bien.

Eh bien, à tout le moins, cela viole le principe de responsabilité unique . Devrait-il y avoir des changements (plusieurs niveaux d’administrateur, encore plus de rôles différents?), Ce type de conception serait assez compliqué et difficile à maintenir.

Considérez l'avantage de séparer le système de sécurité de la classe d'utilisateurs, afin que les deux puissent avoir un rôle unique et bien défini. Vous vous retrouvez avec plus propre, plus facile à maintenir la conception globale.

Je pense que le problème, peut-être mal formulé, est que cela donne trop de poids à la Userclasse. Non, il n'y a pas de problème de sécurité avec une méthode User.isAdmin(), comme suggéré dans ces commentaires. Après tout, si votre code est suffisamment profond pour injecter du code malveillant pour l’une de vos classes principales, vous rencontrez de graves problèmes. En revanche, il n’a aucun sens Userde décider s’il s’agit d’un administrateur pour chaque contexte. Imaginez que vous ajoutiez différents rôles: modérateurs pour votre forum, rédacteurs pouvant publier des publications sur la page d'accueil, rédacteurs pouvant rédiger du contenu que les rédacteurs publient, etc. Avec l'approche consistant à le mettre sur l' Userobjet, vous vous retrouverez avec trop de méthodes et trop de logique vivant sur des objets Userqui ne sont pas directement liés à la classe.

Au lieu de cela, vous pouvez utiliser une énumération de différents types d'autorisations et une PermissionsManagerclasse. Peut-être que vous pourriez avoir une méthode comme PermissionsManager.userHasPermission(User, Permission)renvoyer une valeur booléenne. En pratique, cela pourrait ressembler à (en java):

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

C'est essentiellement équivalent à la méthode Rails before_filter, qui fournit un exemple de ce type de vérification des autorisations dans le monde réel.

Object.isX () est utilisé pour représenter une relation claire entre l'objet et X, qui peut être exprimée sous la forme d'un résultat booléen, par exemple:

L'eau bouillonne ()

Circuit.isOpen ()

User.isAdmin () assume un sens unique de "Administrateur" dans chaque contexte du système , qu'un utilisateur est, dans chaque partie du système, un administrateur.

Bien que cela semble assez simple, un programme du monde réel ne correspondra presque jamais à ce modèle, mais il sera certainement nécessaire pour un utilisateur d’administrer une ressource [X] mais pas de [Y], ou pour des types d’administrateurs plus distincts (un [projet]). ] administrateur contre un administrateur [système]).

Cette situation nécessite généralement une enquête sur les exigences. Rarement, voire jamais, un client voudra-t-il la relation que représente réellement User.isAdmin (), aussi j'hésiterais-il à mettre en œuvre une telle solution sans clarification.

L'affiche avait simplement une conception différente et plus compliquée à l'esprit. À mon avis, ça User.isAdmin()va . Même si vous introduisez plus tard un modèle d'autorisations compliqué, je ne vois aucune raison de changer User.isAdmin(). Plus tard, vous voudrez peut-être scinder la classe User en un objet représentant un utilisateur connecté et un objet statique représentant les données relatives à l'utilisateur. Ou vous ne pouvez pas. Économiser demain pour demain.

Pas vraiment un problème ...

Je ne vois pas de problème avec User.isAdmin(). Je le préfère certainement à quelque chose du genre PermissionManager.userHasPermission(user, permissions.ADMIN), qui, dans le saint nom de SRP, rend le code moins clair et n’ajoute rien de valeur.

Je pense que SRP est interprété un peu littéralement par certains. Je pense que c'est bien, même préférable pour une classe d'avoir une interface riche. SRP signifie simplement qu'un objet doit déléguer à des collaborateurs tout ce qui ne relève pas de sa seule responsabilité. Si le rôle d'administrateur d'un utilisateur est plus complexe qu'un champ booléen, il serait peut-être judicieux que l'objet utilisateur délègue une délégation à un Autorisateur. Mais cela ne veut pas dire que ce n’est pas aussi une bonne idée pour un utilisateur de conserver sa isAdminméthode. En fait, cela signifie que lorsque votre application passe du plus simple au plus complexe, vous devez changer le code qui utilise l'objet Utilisateur. IOW, votre client n'a pas besoin de savoir ce qu'il faut vraiment pour répondre à la question de savoir si un utilisateur est un administrateur ou non.

... mais pourquoi voulez-vous vraiment savoir?

Cela dit, il me semble que vous avez rarement besoin de savoir si un utilisateur est un administrateur, sauf pour pouvoir répondre à une autre question, par exemple si un utilisateur est autorisé à effectuer une action spécifique, par exemple la mise à jour d'un widget. Si tel est le cas, je préférerais avoir une méthode sur Widget, comme isUpdatableBy(User user):

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

De cette façon, un widget est responsable de savoir quels critères doivent être satisfaits pour qu'il soit mis à jour par un utilisateur, et un utilisateur est tenu de savoir s'il s'agit d'un administrateur. Cette conception communique clairement sur ses intentions et facilite le passage à une logique métier plus complexe, le cas échéant.

[Modifier]

Le problème de ne pas avoir User.isAdmin()et utiliserPermissionManager.userHasPermission(...)

J'ai pensé ajouter quelque chose à ma réponse pour expliquer pourquoi je préfère fortement appeler une méthode sur l'objet User que d'appeler une méthode sur un objet PermissionManager si je veux savoir si un utilisateur est un administrateur (ou a un rôle d'administrateur).

Je pense qu'il est juste de supposer que vous allez toujours dépendre de la classe User chaque fois que vous devez vous poser la question suivante : cet utilisateur est-il un administrateur? C'est une dépendance dont vous ne pouvez vous débarrasser. Mais si vous devez passer l'utilisateur à un autre objet pour lui poser une question, cela crée une nouvelle dépendance à cet objet en plus de celle que vous avez déjà sur l'utilisateur. Si la question est souvent posée, il y a beaucoup d'endroits où vous créez une dépendance supplémentaire et vous devrez peut-être changer d'endroit si l'une des dépendances l'exige.

Comparez cela avec le déplacement de la dépendance dans la classe User. Maintenant, vous avez soudainement un système où le code client (le code qui doit poser la question à cet utilisateur est un administrateur ) n'est pas associé à la mise en œuvre de la réponse donnée à cette question. Vous êtes libre de changer complètement le système de permission et il vous suffit de mettre à jour une méthode dans une classe pour le faire. Tout le code client reste le même.

Insister sur le fait de ne pas avoir de isAdminméthode sur l'utilisateur, de peur de créer une dépendance de la classe User vis-à-vis du sous-système des autorisations, équivaut, à mon avis, à dépenser un dollar pour gagner un centime. Bien sûr, vous évitez une dépendance dans la classe User, mais au prix de la créer à chaque endroit où vous devez poser la question. Pas une bonne affaire.

Cette discussion m'a rappelé un billet de blog d'Eric Lippert, qui m'a été signalé lors d'une discussion similaire sur la conception, la sécurité et la correction des classes.

Pourquoi autant de classes-cadres sont-elles scellées?

Eric soulève le point en particulier:

4) sécurisé. le polymorphisme a pour objectif principal de pouvoir faire passer des objets qui ressemblent à des animaux mais qui sont en fait des girafes. Il y a des problèmes de sécurité potentiels ici.

Chaque fois que vous implémentez une méthode qui prend une instance d'un type non scellé, vous DEVEZ écrire cette méthode pour qu'elle soit robuste face à des instances potentiellement hostiles de ce type. Vous ne pouvez pas compter sur des invariants que vous savez être vrais dans VOTRE implémentation, car une page Web hostile peut sous-classer votre implémentation, remplacer les méthodes virtuelles pour effectuer des tâches qui gâchent votre logique et les transmet. Chaque fois que je scelle une classe , Je peux écrire des méthodes qui utilisent cette classe avec la certitude de savoir ce que cette classe fait.

Cela peut sembler une tangente, mais je pense que cela rejoint le point que d'autres affiches ont soulevé à propos du principe de responsabilité unique SOLID . Considérez la classe malveillante suivante comme une raison de ne pas implémenter une méthode ou une propriété.User.IsAdmin

public class MyUser : User
{

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

    // Anything else we can break today?

}

Certes, c'est un peu exagéré: personne n'a encore suggéré de créer IsAmdminune méthode virtuelle, mais cela pourrait arriver si votre architecture utilisateur / rôle devenait étrangement complexe. (Ou peut-être pas. Pensez public class Admin : User { ... }: une telle classe pourrait avoir exactement le code ci-dessus.) La mise en place d’un binaire malveillant n’est pas un vecteur d’attaque courant et a beaucoup plus de potentiel de chaos qu’une obscure bibliothèque utilisateur. pourrait être le bug d' escalade de privilège qui ouvre la porte aux véritables manigances. Enfin, si une instance binaire ou d'objet malveillante se retrouvait dans le temps d'exécution, imaginer le "système de privilège ou de sécurité" remplacé de la même manière ne serait pas trop compliqué.

Mais pour reprendre le point de vue d’Eric, si vous mettez votre code utilisateur dans un type particulier de système vulnérable, vous perdez peut-être tout simplement la partie.

Oh, et pour être précis, je suis d'accord avec le postulat de la question: «Un utilisateur ne devrait pas décider s'il s'agit d'un administrateur ou non. Les privilèges ou le système de sécurité devraient. »Une User.IsAdminméthode est une mauvaise idée si vous exécutez le code sur le système, vous ne gardez pas le contrôle à 100% du code - vous devriez plutôt le faire Permissions.IsAdmin(User user).

Le problème ici est:

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

Soit vous avez correctement configuré votre sécurité, auquel cas la méthode privilégiée doit vérifier si l’appelant dispose des droits suffisants - auquel cas la vérification est superflue. Ou bien vous n’avez pas fait confiance à une classe non privilégiée pour prendre ses propres décisions en matière de sécurité.