Was ist mit „Ein Benutzer sollte sich nicht entscheiden, ob es sich um einen Administrator handelt oder nicht. Das Privilegien- oder Sicherheitssystem sollte. “

Das in der Frage verwendete Beispiel für die Übergabe von Mindestdaten an eine Funktion gibt Aufschluss darüber, ob der Benutzer ein Administrator ist oder nicht. Eine häufige Antwort war:

user.isAdmin()

Dies führte zu einem Kommentar, der mehrmals wiederholt und mehrfach hochgestuft wurde:

Ein Benutzer sollte sich nicht entscheiden, ob es sich um einen Administrator handelt oder nicht. Das Privilegien- oder Sicherheitssystem sollte. Etwas, das eng mit einer Klasse verbunden ist, bedeutet nicht, dass es eine gute Idee ist, sie zu einem Teil dieser Klasse zu machen.

Ich antwortete,

Der Benutzer entscheidet nichts. Das Benutzerobjekt / die Tabelle speichert Daten über jeden Benutzer. Tatsächliche Benutzer können nicht alles an sich selbst ändern.

Dies war jedoch nicht produktiv. Es liegt eindeutig ein Unterschied in der Perspektive zugrunde, der die Kommunikation erschwert. Kann mir jemand erklären, warum user.isAdmin () schlecht ist, und eine kurze Skizze darüber zeichnen, wie es aussieht, als wäre es "richtig" gemacht worden?

Wirklich, ich sehe keinen Vorteil darin, die Sicherheit von dem zu schützenden System zu trennen. Jeder Sicherheitstext besagt, dass Sicherheit von Anfang an in ein System integriert und in jeder Phase der Entwicklung, Bereitstellung, Wartung und sogar am Ende der Lebensdauer berücksichtigt werden muss. Es ist nicht etwas, das an der Seite verschraubt werden kann. Bisher haben 17 Stimmen zu diesem Kommentar zugelegt, dass ich etwas Wichtiges verpasse.

Stellen Sie sich den Benutzer als Schlüssel und die Berechtigungen als Wert vor.

Unterschiedliche Kontexte haben möglicherweise unterschiedliche Berechtigungen für jeden Benutzer. Da die Berechtigungen kontextrelevant sind, müssten Sie den Benutzer für jeden Kontext ändern. Also ist es besser, wenn Sie diese Logik an einer anderen Stelle ablegen (z. B. in der Kontextklasse) und die erforderlichen Berechtigungen nachschlagen.

Ein Nebeneffekt ist, dass Ihr System dadurch möglicherweise sicherer wird, da Sie nicht vergessen können, das Admin-Flag an Stellen zu löschen, an denen es nicht relevant ist.

Dieser Kommentar war falsch formuliert.

Es geht nicht darum, ob das Benutzerobjekt "entscheidet", ob es sich um einen Administrator, einen Testbenutzer, einen Superbenutzer oder um etwas Ungewöhnliches handelt. Offensichtlich entscheidet es nichts davon, was das von Ihnen implementierte Softwaresystem im Allgemeinen tut. Der Punkt ist , ob die „user“ Klasse sollte repräsentiert die Rollen des Hauptes, die ihre Objekte darstellen, oder ob dies eine Verantwortung einer anderen Klasse.

Ich hätte mich nicht dafür entschieden, den ursprünglichen Kommentar so zu formulieren, wie er formuliert wurde, aber er identifiziert ein potenziell legitimes Problem.

Insbesondere sind die Bedenken, die eine Trennung rechtfertigen, die Authentifizierung gegenüber der Autorisierung .

Authentifizierung bezieht sich auf den Prozess der Anmeldung und des Abrufs einer Identität. Auf diese Weise wissen die Systeme, wer Sie sind , und werden für Dinge wie Personalisierung, Objekteigentum usw. verwendet.

Die Autorisierung bezieht sich auf das , was Sie tun dürfen , und dies hängt (im Allgemeinen) nicht davon ab , wer Sie sind . Stattdessen wird es durch einige Sicherheitsrichtlinien wie Rollen oder Berechtigungen festgelegt, die sich nicht um Dinge wie Ihren Namen oder Ihre E-Mail-Adresse kümmern.

Diese beiden können sich orthogonal zueinander ändern. Beispielsweise können Sie das Authentifizierungsmodell ändern, indem Sie OpenID / OpenAuth-Anbieter hinzufügen. Außerdem können Sie die Sicherheitsrichtlinie ändern, indem Sie eine neue Rolle hinzufügen oder von RBAC zu ABAC wechseln.

Wenn all dies in eine Klasse oder Abstraktion fällt, ist Ihr Sicherheitscode, der eines Ihrer wichtigsten Instrumente zur Risikominderung ist, ironischerweise ein hohes Risiko.

Ich habe mit Systemen gearbeitet, bei denen Authentifizierung und Autorisierung zu eng miteinander verbunden waren. In einem System gab es zwei parallele Benutzerdatenbanken für jeweils einen Typ von "Rolle". Die Person oder das Team, die / das es entworfen hat, hat anscheinend nie in Betracht gezogen, dass ein einzelner physischer Benutzer möglicherweise beide Rollen innehat, dass bestimmte Aktionen für mehrere Rollen gelten oder dass es Probleme mit Benutzer-ID-Kollisionen gibt. Dies ist ein zugegebenermaßen extremes Beispiel, aber es war / ist unglaublich schmerzhaft, damit zu arbeiten.

Microsoft und Sun / Oracle (Java) bezeichnen die Gesamtheit der Authentifizierungs- und Autorisierungsinformationen als Sicherheitsprinzipal . Es ist nicht perfekt, aber es funktioniert einigermaßen gut. In .NET, zum Beispiel, haben Sie IPrincipal, die kapselt das IIdentity- die ehemalige ein Wesen Politik (Autorisierung) Objekt , während die letztere ist eine Identität (Authentifizierung). Sie könnten die Entscheidung, eines in das andere zu setzen, vernünftigerweise in Frage stellen , aber das Wichtigste ist, dass der meiste Code, den Sie schreiben, nur für eine der Abstraktionen bestimmt ist, was bedeutet, dass es leicht zu testen und umzugestalten ist.

An einem User.IsAdminFeld ist nichts auszusetzen ... es sei denn, es gibt auch ein User.NameFeld. Dies würde darauf hinweisen, dass das "Benutzer" -Konzept nicht richtig definiert ist, und dies ist leider ein sehr häufiger Fehler bei Entwicklern, die in Bezug auf Sicherheit etwas hinter den Ohren liegen. In der Regel ist die Benutzer-ID das einzige, was von Identität und Richtlinie gemeinsam genutzt werden sollte. Dies ist nicht zufällig genau so, wie es in den Sicherheitsmodellen von Windows und * nix implementiert ist.

Es ist völlig akzeptabel, Wrapper-Objekte zu erstellen, die sowohl Identität als auch Richtlinie kapseln. Dies würde beispielsweise die Erstellung eines Dashboard-Bildschirms erleichtern, auf dem Sie neben verschiedenen Widgets oder Links, auf die der aktuelle Benutzer zugreifen darf, eine "Hallo" -Nachricht anzeigen müssen. Solange diese Wrapper nur hüllen die Identität und politische Informationen und erheben nicht den Anspruch , es zu besitzen. Mit anderen Worten, solange es nicht als Gesamtwurzel dargestellt wird .

Ein simplifiziertes Sicherheitsmodell scheint immer eine gute Idee zu sein, wenn Sie aufgrund von YAGNI und all dem zuerst eine neue Anwendung entwerfen, aber es kommt fast immer wieder auf Sie zu, um Sie später zu beißen, da überraschenderweise neue Funktionen hinzugefügt werden!

Wenn Sie also wissen, was für Sie am besten ist, trennen Sie Authentifizierungs- und Autorisierungsinformationen. Selbst wenn die "Autorisierung" im Moment so einfach wie ein "IsAdmin" -Flag ist, sind Sie immer noch besser dran, wenn sie nicht Teil derselben Klasse oder Tabelle wie die Authentifizierungsinformationen sind, und wenn Ihre Sicherheitsrichtlinie dies erfordert Sie müssen keine rekonstruktiven Eingriffe an Ihren Authentifizierungssystemen vornehmen, die bereits ordnungsgemäß funktionieren.

Zumindest verstößt es gegen das Prinzip der Einzelverantwortung . Sollte es Änderungen geben (mehrere Administratorebenen, noch mehr verschiedene Rollen?), Wäre diese Art von Design ziemlich chaotisch und schrecklich zu pflegen.

Berücksichtigen Sie den Vorteil der Trennung des Sicherheitssystems von der Benutzerklasse, sodass beide eine einzelne, genau definierte Rolle haben können. Sie erhalten ein saubereres und insgesamt leichter zu pflegendes Design.

Ich denke, das Problem, das vielleicht schlecht formuliert ist, ist, dass es der UserKlasse zu viel Gewicht gibt . Nein, es gibt keine Sicherheitsprobleme mit einer Methode User.isAdmin(), wie in diesen Kommentaren vorgeschlagen. Wenn jemand tief genug in Ihrem Code steckt, um schädlichen Code für eine Ihrer Kernklassen einzufügen, haben Sie schließlich ernsthafte Probleme. Auf der anderen Seite ist es nicht sinnvoll Userzu entscheiden, ob es sich bei jedem Kontext um einen Administrator handelt. Stellen Sie sich vor, Sie fügen verschiedene Rollen hinzu: Moderatoren für Ihr Forum, Redakteure, die Beiträge auf der Startseite veröffentlichen können, Autoren, die Inhalte für die Redakteure verfassen können, und so weiter. Mit dem Ansatz, es auf das UserObjekt zu setzen, werden Sie am Ende zu viele Methoden und zu viel Logik haben, die auf dem Userbasieren, was nicht direkt mit der Klasse zusammenhängt.

Stattdessen könnten Sie eine Aufzählung verschiedener Arten von Berechtigungen und eine PermissionsManagerKlasse verwenden. Vielleicht könnten Sie eine Methode haben PermissionsManager.userHasPermission(User, Permission), die einen booleschen Wert zurückgibt. In der Praxis könnte es so aussehen (in Java):

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

Es entspricht im Wesentlichen der Rails-Methode before_filter, die ein Beispiel für diese Art der Berechtigungsprüfung in der realen Welt darstellt.

Object.isX () wird verwendet, um eine eindeutige Beziehung zwischen dem Objekt und X darzustellen, die als boolesches Ergebnis ausgedrückt werden kann. Beispiel:

Wasser.istKochen ()

Circuit.isOpen ()

User.isAdmin () nimmt in jedem Kontext des Systems eine einzelne Bedeutung von 'Administrator' an, dh ein Benutzer ist in jedem Teil des Systems ein Administrator.

Obwohl es sich einfach anhört, wird ein Programm aus der realen Welt so gut wie nie in dieses Modell passen. Es wird jedoch höchstwahrscheinlich erforderlich sein, dass ein Benutzer [X] -Ressourcen verwaltet, jedoch nicht [Y], oder dass es sich um spezifischere Administratortypen handelt (ein [Projekt] ] Administrator gegen einen [System] Administrator).

Diese Situation erfordert normalerweise eine Untersuchung der Anforderungen. In seltenen Fällen möchte ein Client die Beziehung, die User.isAdmin () tatsächlich darstellt, so dass ich zögern würde, eine solche Lösung ohne Klärung zu implementieren.

Das Plakat hatte lediglich ein anderes und komplizierteres Design im Sinn. Meiner Meinung nach User.isAdmin()ist in Ordnung . Auch wenn Sie später ein kompliziertes Berechtigungsmodell einführen, sehe ich wenig Grund, umzuziehen User.isAdmin(). Später möchten Sie möglicherweise die Benutzerklasse in ein Objekt, das einen angemeldeten Benutzer darstellt, und ein statisches Objekt, das die Daten über den Benutzer darstellt, aufteilen. Oder du darfst nicht. Sparen Sie morgen für morgen.

Kein wirkliches Problem ...

Ich sehe kein Problem mit User.isAdmin(). Ich bevorzuge es auf jeden Fall so etwas wie PermissionManager.userHasPermission(user, permissions.ADMIN), was im heiligen Namen von SRP den Code weniger klar macht und nichts von Wert hinzufügt.

Ich denke, SRP wird von einigen ein wenig zu wörtlich interpretiert. Ich denke, es ist in Ordnung, noch besser für eine Klasse, eine reichhaltige Oberfläche zu haben. SRP bedeutet lediglich, dass ein Objekt alles an Mitarbeiter delegieren muss, was nicht in seine alleinige Verantwortung fällt. Wenn die Administratorrolle eines Benutzers mehr als ein boolesches Feld ist, ist es möglicherweise sinnvoll, das Benutzerobjekt an einen PermissionsManager zu delegieren. Das bedeutet aber nicht, dass es für einen Benutzer auch keine gute Idee ist, seine isAdminMethode beizubehalten. Tatsächlich bedeutet dies, dass Sie den Code, der das Benutzerobjekt verwendet, ändern müssen, wenn Ihre Anwendung von einfach zu komplex wechselt. IOW, Ihr Kunde muss nicht wissen, was es wirklich braucht, um die Frage zu beantworten, ob ein Benutzer ein Administrator ist oder nicht.

... aber warum willst du das wirklich wissen?

Das heißt, es scheint mir, dass Sie selten wissen müssen, ob ein Benutzer ein Administrator ist, es sei denn, Sie können eine andere Frage beantworten, z. B. ob ein Benutzer eine bestimmte Aktion ausführen darf, z. B. das Aktualisieren eines Widgets. Wenn dies der Fall ist, würde ich es vorziehen, eine Methode in Widget zu haben, wie zum Beispiel isUpdatableBy(User user):

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

Auf diese Weise muss ein Widget wissen, welche Kriterien erfüllt sein müssen, damit es von einem Benutzer aktualisiert wird, und ein Benutzer muss wissen, ob es sich um einen Administrator handelt. Dieses Design kommuniziert klar über seine Absichten und macht es einfacher, zu komplexeren Geschäftslogiken überzugehen, wenn und wann dies der Fall ist.

[Bearbeiten]

Das Problem nicht zu haben User.isAdmin()und zu benutzenPermissionManager.userHasPermission(...)

Ich dachte, ich würde meiner Antwort hinzufügen, um zu erklären, warum ich es sehr bevorzuge, eine Methode für das Benutzerobjekt aufzurufen, anstatt eine Methode für ein PermissionManager-Objekt aufzurufen, wenn ich wissen möchte, ob ein Benutzer ein Administrator ist (oder eine Administratorrolle hat).

Ich denke, es ist fair anzunehmen, dass Sie immer von der Benutzerklasse abhängen werden, wo immer Sie die Frage stellen müssen. Ist dieser Benutzer ein Administrator? Das ist eine Abhängigkeit, die man nicht loswerden kann. Wenn Sie den Benutzer jedoch an ein anderes Objekt übergeben müssen, um eine Frage zu stellen, wird eine neue Abhängigkeit von diesem Objekt über die Abhängigkeit erstellt, die Sie bereits vom Benutzer haben. Wenn die Frage häufig gestellt wird, ist dies eine Menge Stellen, an denen Sie eine zusätzliche Abhängigkeit erstellen, und es sind viele Stellen, an denen Sie möglicherweise Änderungen vornehmen müssen, wenn eine der Abhängigkeiten dies erfordert.

Vergleichen Sie dies mit dem Verschieben der Abhängigkeit in die Benutzerklasse. Jetzt haben Sie plötzlich ein System, in dem der Client-Code (Code, der die Frage stellen muss, ist dieser Benutzer ein Administrator ) nicht an die Implementierung gekoppelt ist, wie diese Frage beantwortet wird. Es steht Ihnen frei, das Berechtigungssystem vollständig zu ändern, und Sie müssen nur eine Methode in einer Klasse aktualisieren, um dies zu tun. Der gesamte Client-Code bleibt unverändert.

Wenn ich darauf bestehe, keine isAdminMethode für den Benutzer zu haben, weil ich befürchte, eine Abhängigkeit in der Benutzerklasse vom Berechtigungssubsystem zu schaffen, ist dies meiner Meinung nach vergleichbar damit, einen Dollar auszugeben, um einen Cent zu verdienen. Sicher, Sie vermeiden eine Abhängigkeit in der Benutzerklasse, müssen jedoch an jeder Stelle, an der Sie die Frage stellen müssen, eine Abhängigkeit erstellen. Kein gutes Geschäft.

Diese Diskussion erinnerte mich an einen Blogbeitrag von Eric Lippert, auf den ich in einer ähnlichen Diskussion über Klassendesign, Sicherheit und Korrektheit aufmerksam wurde.

Warum sind so viele Framework-Klassen versiegelt?

Insbesondere bringt Eric Folgendes zur Sprache:

4) Sicher. Der springende Punkt beim Polymorphismus ist, dass Sie Objekte herumreichen können, die wie Tiere aussehen, aber tatsächlich Giraffen sind. Hier gibt es potenzielle Sicherheitsprobleme.

Jedes Mal, wenn Sie eine Methode implementieren, die eine Instanz eines nicht versiegelten Typs verwendet, MÜSSEN Sie diese Methode so schreiben, dass sie gegenüber potenziell feindlichen Instanzen dieses Typs robust ist. Sie können sich nicht auf Invarianten verlassen, von denen Sie wissen, dass sie für IHRE Implementierungen zutreffend sind, da eine feindliche Webseite Ihre Implementierung unterklassifizieren, die virtuellen Methoden überschreiben kann, um Dinge zu tun, die Ihre Logik durcheinander bringen, und sie weitergeben. Jedes Mal, wenn ich eine Klasse versiegele Ich kann Methoden schreiben, die diese Klasse verwenden, mit der Gewissheit, dass ich weiß, was diese Klasse tut.

Dies mag wie eine Tangente erscheinen, aber ich denke, es passt genau zu dem Punkt, den andere Plakate über das SOLID -Prinzip der Einzelverantwortung angesprochen haben . Betrachten Sie die folgende böswillige Klasse als Grund, eine User.IsAdminMethode oder Eigenschaft nicht zu implementieren .

public class MyUser : User
{

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

    // Anything else we can break today?

}

Zugegeben, es ist ein bisschen IsAmdminkompliziert : Noch hat niemand vorgeschlagen, eine virtuelle Methode zu entwickeln, aber es könnte passieren, dass Ihre Benutzer- / Rollenarchitektur bizarr komplex wird. (Oder vielleicht auch nicht. Bedenken Sie public class Admin : User { ... }, dass in einer solchen Klasse möglicherweise genau der oben genannte Code enthalten ist.) Das Einrichten einer schädlichen Binärdatei ist kein häufiger Angriffsmechanismus und birgt viel mehr Chaospotenzial als eine obskure Benutzerbibliothek Es könnte sich um den Fehler der Privilegieneskalation handeln , der den echten Spielereien die Tür öffnet. Wenn schließlich eine böswillige Binär- oder Objektinstanz in die Laufzeit gelangt ist, ist es nicht viel schwieriger, sich vorzustellen, dass das "Berechtigungs- oder Sicherheitssystem" auf ähnliche Weise ersetzt wird.

Aber wenn Sie Ihren Benutzercode in einem bestimmten verwundbaren System ablegen, haben Sie das Spiel vielleicht einfach verloren.

Um genau zu sein, stimme ich dem Postulat in der Frage zu: „Ein Benutzer sollte sich nicht entscheiden, ob es sich um einen Administrator handelt oder nicht. Das Privilegien- oder Sicherheitssystem sollte. “Eine User.IsAdminMethode ist eine schlechte Idee, wenn Sie den Code auf einem System ausführen, das Sie nicht zu 100% kontrollieren, sondern tun sollten Permissions.IsAdmin(User user).

Das Problem ist hier:

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

Entweder haben Sie Ihre Sicherheit richtig eingestellt, in welchem ​​Fall die privilegierte Methode prüfen soll, ob der Anrufer über ausreichende Berechtigungen verfügt - in welchem ​​Fall die Prüfung überflüssig ist. Oder Sie haben keine und vertrauen darauf, dass eine nicht privilegierte Klasse ihre eigenen Sicherheitsentscheidungen trifft.