Co oznacza „użytkownik nie powinien decydować, czy jest administratorem, czy nie. System uprawnień lub bezpieczeństwa powinien. ”

Przykład użyty w pytaniu przekazuje absolutną minimalną ilość danych do funkcji, która dotyczy najlepszego sposobu ustalenia, czy użytkownik jest administratorem, czy nie. Jedną z powszechnych odpowiedzi było:

user.isAdmin()

To skłoniło do komentarza, który został kilkakrotnie powtórzony i wielokrotnie głosowany:

Użytkownik nie powinien decydować, czy jest administratorem, czy nie. System uprawnień lub bezpieczeństwa powinien. Coś ściśle związanego z klasą nie oznacza, że ​​dobrym pomysłem jest włączenie jej do klasy.

Odpowiedziałem,

Użytkownik nic nie decyduje. Obiekt / tabela użytkownika przechowuje dane o każdym użytkowniku. Faktyczni użytkownicy nie mogą zmienić wszystkiego o sobie.

Ale to nie było produktywne. Oczywiście istnieje zasadnicza różnica w perspektywie, która utrudnia komunikację. Czy ktoś może mi wyjaśnić, dlaczego user.isAdmin () jest zły, i narysować krótki szkic tego, jak to wygląda „zrobione”?

Naprawdę nie widzę korzyści z oddzielenia bezpieczeństwa od chronionego systemu. W każdym tekście dotyczącym bezpieczeństwa powiedziano, że zabezpieczenia muszą być projektowane od początku w systemie i rozważane na każdym etapie rozwoju, wdrażania, konserwacji, a nawet pod koniec okresu użytkowania. To nie jest coś, co można przykręcić z boku. Ale jak dotąd 17 głosów za tym komentarzem mówi, że brakuje mi czegoś ważnego.

Pomyśl o użytkowniku jako kluczu, a uprawnienia jako wartości.

Różne konteksty mogą mieć różne uprawnienia dla każdego użytkownika. Ponieważ uprawnienia dotyczą kontekstu, musisz zmienić użytkownika dla każdego kontekstu. Lepiej więc umieścić tę logikę gdzie indziej (np. Klasę kontekstową) i po prostu sprawdzić uprawnienia w razie potrzeby.

Efektem ubocznym jest zwiększenie bezpieczeństwa systemu, ponieważ nie można zapomnieć o usunięciu flagi administratora dla miejsc, w których nie ma to znaczenia.

Ten komentarz był źle sformułowany.

Nie chodzi o to, czy obiekt użytkownika „decyduje”, czy jest administratorem, użytkownikiem próbnym, superużytkownikiem, czy czymś niezwykłym. Oczywiście to nie decyduje o żadnej z tych rzeczy, tak jak cały system oprogramowania, tak jak Ty. Chodzi o to, czy klasa „user” powinna reprezentować role nazwy głównej, którą reprezentują jej obiekty, czy też jest to odpowiedzialność innej klasy.

Nie zdecydowałbym się wypowiedzieć oryginalnego komentarza w sposób, w jaki został sformułowany, ale identyfikuje on potencjalnie uzasadniony problem.

W szczególności obawy, które uzasadniają separację, to uwierzytelnianie vs. autoryzacja .

Uwierzytelnianie odnosi się do procesu logowania i uzyskiwania tożsamości. W ten sposób systemy wiedzą, kim jesteś , i są wykorzystywane do personalizacji, własności obiektu itp.

Autoryzacja odnosi się do tego, co wolno ci robić , a to (ogólnie) nie zależy od tego, kim jesteś . Zamiast tego zależy to od niektórych zasad bezpieczeństwa, takich jak role lub uprawnienia, które nie dbają o takie rzeczy, jak imię i nazwisko lub adres e-mail.

Ci dwaj mogą zmieniać się ortogonalnie. Na przykład możesz zmienić model uwierzytelniania, dodając dostawców OpenID / OpenAuth. Możesz także zmienić politykę bezpieczeństwa, dodając nową rolę lub zmieniając RBAC na ABAC.

Jeśli wszystko to zostanie ujęte w jedną klasę lub abstrakcję, wówczas kod bezpieczeństwa, który jest jednym z najważniejszych narzędzi ograniczania ryzyka , staje się, jak na ironię, wysokim ryzykiem.

Pracowałem z systemami, w których uwierzytelnianie i autoryzacja były zbyt ściśle powiązane. W jednym systemie istniały dwie równoległe bazy danych użytkowników, każda dla jednego rodzaju „roli”. Osoba lub zespół, który ją zaprojektował, najwyraźniej nigdy nie uważał, że pojedynczy użytkownik fizyczny może pełnić obie role lub że mogą istnieć pewne działania, które były wspólne dla wielu ról lub że mogą występować problemy z kolizjami ID użytkownika. Jest to wprawdzie skrajny przykład, ale praca z nim była / jest niezwykle bolesna.

Microsoft i Sun / Oracle (Java) określają agregację informacji o uwierzytelnianiu i autoryzacji jako Principal Security . Nie jest idealny, ale działa dość dobrze. W .NET, na przykład, masz IPrincipal, który obudowujeIIdentity - dawna będąc polityka (autoryzacja) przedmiot, a drugi jest tożsamość (uwierzytelnianie). Można rozsądnie zakwestionować decyzję o umieszczeniu jednego w drugim, ale ważne jest to, że większość pisanego kodu będzie dotyczyła tylko jednej z abstrakcji, co oznacza, że ​​łatwo go przetestować i zmienić.

Nie ma nic złego w User.IsAdminpolu ... chyba że jest też User.Namepole. Oznaczałoby to, że koncepcja „użytkownika” nie jest właściwie zdefiniowana i jest to niestety bardzo częsty błąd programistów, którzy są nieco mokrzy za uszami, jeśli chodzi o bezpieczeństwo. Zazwyczaj jedyną rzeczą, która powinna być współdzielona przez tożsamość i zasady, jest identyfikator użytkownika, który nieprzypadkowo jest dokładnie taki, jak jest implementowany zarówno w modelach bezpieczeństwa Windows, jak i * nix.

Całkowicie dopuszczalne jest tworzenie obiektów opakowujących, które zawierają zarówno tożsamość, jak i zasady. Ułatwi to na przykład utworzenie ekranu deski rozdzielczej, na którym oprócz różnych widżetów lub łączy, do których bieżący użytkownik ma dostęp, należy wyświetlić komunikat „cześć”. Tak długo, jak to opakowanie po prostu otacza informacje dotyczące tożsamości i zasad i nie rości sobie prawa do ich posiadania. Innymi słowy, o ile nie jest prezentowany jako zagregowany katalog główny .

Uproszczony model bezpieczeństwa zawsze wydaje się dobrym pomysłem przy pierwszym projektowaniu nowej aplikacji, ze względu na YAGNI i tak dalej, ale prawie zawsze wraca, by cię ugryźć później, ponieważ, niespodzianka, nowe funkcje są dodawane!

Jeśli więc wiesz, co jest dla Ciebie najlepsze, informacje dotyczące uwierzytelniania i autoryzacji będą oddzielone. Nawet jeśli „autoryzacja” jest teraz tak prosta jak flaga „IsAdmin”, nadal będziesz lepiej, jeśli nie jest częścią tej samej klasy lub tabeli co informacje uwierzytelniające, więc jeśli i kiedy twoja polityka bezpieczeństwa musi zmiana, nie musisz wykonywać operacji rekonstrukcyjnych w systemach uwierzytelniania, które już działają dobrze.

Cóż, przynajmniej narusza zasadę pojedynczej odpowiedzialności . Czy powinny nastąpić zmiany (wiele poziomów administratora, jeszcze więcej różnych ról?) Tego rodzaju projekt byłby dość niechlujny i okropny w utrzymaniu.

Rozważ zaletę oddzielenia systemu bezpieczeństwa od klasy użytkownika, aby oba mogły mieć jedną, dobrze zdefiniowaną rolę. W efekcie powstaje czystszy, łatwiejszy w utrzymaniu projekt.

Myślę, że problemem, być może źle sformułowanym, jest to, że kładzie on zbyt duży nacisk na Userklasę. Nie, nie ma problemu z bezpieczeństwem posiadania metody User.isAdmin(), jak sugerowano w tych komentarzach. W końcu, jeśli ktoś jest wystarczająco głęboko w twoim kodzie, aby wstrzyknąć szkodliwy kod dla jednej z twoich podstawowych klas, masz poważne problemy. Z drugiej strony nie ma sensu Userdecydować, czy jest administratorem w każdym kontekście. Wyobraź sobie, że dodajesz różne role: moderatorzy forum, redaktorzy, którzy mogą publikować posty na stronie głównej, autorzy, którzy mogą pisać treści, które redaktorzy mogą publikować, i tak dalej. Podejście polegające na umieszczeniu go na Userobiekcie Userspowoduje powstanie zbyt wielu metod i zbyt dużej logiki, która nie będzie miała bezpośredniego związku z klasą.

Zamiast tego można użyć wyliczenia różnych typów uprawnień i PermissionsManagerklasy. Być może możesz mieć metodę typu PermissionsManager.userHasPermission(User, Permission)zwracającą wartość logiczną. W praktyce może wyglądać następująco (w java):

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

Zasadniczo jest to metoda before_filtermetody Rails , która stanowi przykład tego typu sprawdzania uprawnień w świecie rzeczywistym.

Object.isX () służy do przedstawienia wyraźnego związku między obiektem a X, który można wyrazić jako wynik boolowski, na przykład:

Water.isBoiling ()

Circuit.isOpen ()

User.isAdmin () zakłada jedno znaczenie „Administrator” w każdym kontekście systemu , że użytkownik jest, w każdej części systemu, administratorem.

Choć brzmi to dość prosto, rzeczywisty program prawie nigdy nie będzie pasował do tego modelu, z pewnością będzie wymóg, aby użytkownik administrował zasobem [X], ale nie [Y], lub bardziej wyraźnymi typami administratorów ([projekt ] administrator kontra administrator [systemowy]).

Ta sytuacja zazwyczaj wymaga zbadania wymagań. Rzadko, jeśli w ogóle, klient będzie chciał relacji, którą faktycznie reprezentuje User.isAdmin (), więc wahałbym się wdrożyć takie rozwiązanie bez wyjaśnienia.

Plakat miał na myśli jedynie inny i bardziej skomplikowany projekt. Moim zdaniem User.isAdmin()jest w porządku . Nawet jeśli później wprowadzisz skomplikowany model uprawnień, nie widzę powodu, by się poruszać User.isAdmin(). Później możesz podzielić klasę użytkownika na obiekt reprezentujący zalogowanego użytkownika i obiekt statyczny reprezentujący dane o użytkowniku. A może nie. Zaoszczędź jutro na jutro.

To naprawdę nie problem ...

Nie widzę problemu z User.isAdmin(). Z pewnością wolę to od czegoś takiego PermissionManager.userHasPermission(user, permissions.ADMIN), co w świętym imieniu SRP czyni kod mniej przejrzystym i nie dodaje nic wartościowego.

Myślę, że SRP jest interpretowany przez niektórych nieco dosłownie. Myślę, że to dobrze, nawet lepiej, jeśli klasa ma bogaty interfejs. SRP oznacza po prostu, że obiekt musi przekazać współpracownikom wszystko, co nie wchodzi w zakres jego pojedynczej odpowiedzialności. Jeśli rola administratora użytkownika jest czymś bardziej zaangażowanym niż pole boolowskie, bardzo dobrze może być delegowanie obiektu użytkownika na menedżera uprawnień. Ale to nie znaczy, że nie jest dobrym pomysłem dla użytkownika, aby zachować tę isAdminmetodę. W rzeczywistości oznacza to, że kiedy twoja aplikacja przechodzi z prostej do złożonej, musisz zmienić kod, który używa obiektu użytkownika. IOW, twój klient nie musi wiedzieć, co tak naprawdę jest potrzebne, aby odpowiedzieć na pytanie, czy użytkownik jest administratorem.

... ale dlaczego tak naprawdę chcesz to wiedzieć?

To powiedziawszy, wydaje mi się, że rzadko musisz wiedzieć, czy użytkownik jest administratorem, z wyjątkiem możliwości udzielenia odpowiedzi na inne pytanie, na przykład, czy użytkownik może wykonać określone działanie, na przykład zaktualizować widżet. W takim przypadku wolałbym mieć metodę na Widget, taką jak isUpdatableBy(User user):

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

W ten sposób widżet odpowiada za to, jakie kryteria musi spełnić, aby użytkownik mógł go zaktualizować, a użytkownik odpowiada za to, czy jest administratorem. Ten projekt jasno komunikuje o swoich zamiarach i ułatwia przejście do bardziej złożonej logiki biznesowej, jeśli i kiedy nadejdzie ten czas.

[Edytować]

Problemem nie mający User.isAdmin()i korzystaniaPermissionManager.userHasPermission(...)

Pomyślałem, że dodam do mojej odpowiedzi, aby wyjaśnić, dlaczego zdecydowanie wolę wywoływać metodę na obiekcie użytkownika niż wywoływać metodę na obiekcie PermissionManager, jeśli chcę wiedzieć, czy użytkownik jest administratorem (lub pełni rolę administratora).

Sądzę, że można założyć, że zawsze będziesz zależał od klasy User, gdziekolwiek musisz zadać pytanie, czy ten użytkownik jest administratorem? Zależności, której nie możesz się pozbyć. Ale jeśli musisz przekazać użytkownika do innego obiektu, aby zadać pytanie na ten temat, tworzy to nową zależność od tego obiektu w stosunku do tego, który już masz na Użytkowniku. Jeśli pytanie jest często zadawane, jest wiele miejsc, w których tworzysz dodatkową zależność, i jest wiele miejsc, które możesz zmienić, jeśli którakolwiek z zależności tego wymaga.

Porównaj to z przeniesieniem zależności do klasy User. Teraz nagle masz system, w którym kod klienta (kod, który musi zadać pytanie, to ten użytkownik jest administratorem ) nie jest powiązany z implementacją odpowiedzi na to pytanie. Możesz całkowicie zmienić system uprawnień i musisz tylko zaktualizować jedną metodę w jednej klasie, aby to zrobić. Cały kod klienta pozostaje taki sam.

Naleganie, by nie mieć isAdminmetody użytkownika z obawy przed stworzeniem zależności w klasie użytkownika od podsystemu uprawnień, jest moim zdaniem podobne do wydawania dolara na zarobienie grosza. Jasne, unikasz jednej zależności w klasie User, ale kosztem stworzenia jednej zależności w każdym miejscu, w którym musisz zadać pytanie. Nie jest to dobra okazja.

Dyskusja ta przypomniała mi post na blogu autorstwa Erica Lipperta, na który zwróciłem uwagę w podobnej dyskusji na temat projektu klasy, bezpieczeństwa i poprawności.

Dlaczego tak wiele klas ramowych jest zapieczętowanych?

W szczególności Eric podnosi kwestię:

4) Bezpieczne. sedno polimorfizmu polega na tym, że możesz ominąć przedmioty, które wyglądają jak Zwierzęta, ale w rzeczywistości są Żyrafami. Istnieją tutaj potencjalne problemy z bezpieczeństwem.

Za każdym razem, gdy implementujesz metodę, która pobiera instancję typu nieopieczętowanego, MUSISZ napisać tę metodę, aby była odporna na potencjalnie wrogie instancje tego typu. Nie możesz polegać na niezmiennikach, o których wiesz, że są prawdziwe w TWOICH implementacjach, ponieważ niektóre wrogie strony internetowe mogą podklasować twoją implementację, zastępować wirtualne metody robienia rzeczy, które psują twoją logikę i przekazują ją. Za każdym razem, gdy zamykam klasę , Potrafię pisać metody, które używają tej klasy z pewnością, że wiem, co robi ta klasa.

Może się to wydawać styczną, ale myślę, że zgadza się z punktem, w którym inni plakaty poruszali kwestię SOLIDNEJ zasady pojedynczej odpowiedzialności . Weź pod uwagę następującą złośliwą klasę jako przyczynę niewdrożenia User.IsAdminmetody lub właściwości.

public class MyUser : User
{

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

    // Anything else we can break today?

}

To prawda, że ​​jest to trochę IsAmdminskomplikowane : nikt jeszcze nie zaproponował stworzenia metody wirtualnej, ale może się to zdarzyć, jeśli twoja architektura użytkowników / ról stanie się dziwnie złożona. (A może nie. Zastanów się public class Admin : User { ... }: taka klasa może mieć dokładnie ten kod powyżej). Umieszczenie złośliwego pliku binarnego na miejscu nie jest częstym wektorem ataku i ma o wiele większy potencjał do chaosu niż niejasna biblioteka użytkowników - to znowu może to być błąd eskalacji uprawnień, który otwiera drzwi do prawdziwych shenaniganów. Wreszcie, jeśli złośliwe wystąpienie binarne lub obiektowe przedostało się do środowiska wykonawczego, wyobrażenie sobie, że „system przywilejów lub system bezpieczeństwa” został zastąpiony w podobny sposób, nie stanowi większego problemu.

Ale biorąc pod uwagę punkt Erica, jeśli umieścisz swój kod użytkownika w określonym systemie podatnym na ataki, być może po prostu przegrałeś.

Aha, a ściślej mówiąc, zgadzam się z postulatem w pytaniu: „Użytkownik nie powinien decydować, czy jest administratorem, czy nie. System przywilejów lub zabezpieczeń powinien. ” User.IsAdminMetoda jest złym pomysłem, jeśli uruchamiasz kod w systemie, nie masz 100% kontroli nad kodem - powinieneś to zrobić Permissions.IsAdmin(User user).

Problem tutaj:

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

Albo poprawnie skonfigurowałeś zabezpieczenia, w którym to przypadku uprzywilejowana metoda powinna sprawdzić, czy osoba dzwoniąca ma wystarczające uprawnienia - w takim przypadku sprawdzenie jest zbędne. Albo nie masz zaufania do klasy nieuprzywilejowanej i ufasz jej, że podejmie własne decyzje dotyczące bezpieczeństwa.