Používat traity, nebo dědičnost?

@Bulldog je vidět, že už to máš prostě vymakaný. Není to teda úplně jednoduché v presenterech nette neprůstřelně autorizovat akce a komponenty. Díky za ukázky, joo jsou různé cesty a je potřeba vybrat tu optimální.

Bulldog napsal(a):

která mi přetěžuje presenterovskou protected function createComponent(string $name): ?Nette\ComponentModel\IComponent a automaticky na základě řetězce $name kontroluje, jestli má uživatel přístup v momentě vytvoření komponenty, takže si ten if v každé CreateComponent<name> metodě můžu odpustit a vím, že to mám secure.

To už je velmi blízko k traitě SecurePresenter, o které jsem psal někdy před 2 lety ve vlákně o autorizaci. Já v té traitě řeším i volání všech action, renderů a handlů. Z reflexe, resp. názvu metody, se poskládá resource. V databázi se to překlopí na resource_key, protože velmi často chce člověk „grupovat“ více metod pod jedno oprávnění.

V presenteru pak odpadnou všechny IFy na isAllowed(), všechny metody jsou chráněné, na nic není šance zapomenout.

Bulldog napsal(a):

To zní hodně fajn. Jediné co, tak bych to asi nedával do databáze.

Databáze je úložiště jako každé jiné. A vzhledem k tempu, jakým jde vývoj, oprávnění vznikají nová a nová, pověřený zákazník si sám chce naklikávat kam koho pustit, co s čím spojit… se nám ta databáze osvědčuje.
Navíc používáme něco jako „sniffování“, kdy se do té db insertují nová oprávnění, pokud byla požadována a neexistovala.
Vývojář tomu jen přiřadí resource_key a už to žije samo.
Ale samozřejmě je nad tou db cache, protože po oprávněních se sahá často, ale mění se zřídka.

Nicméně jak tam řešíš přístupy k entitám?

To je dobrá otázka, čekal jsem, že přijde :) Pokud se vyžaduje oprávnění ne jen „mohu/nemohu“, ale i „mohu jen vybrané“, musí na to model implementoval interface s testovacími metodami, které vrací a) seznam povolených id entit, b) bool pro konkrétní id.
Takže velmi podobně, jako jsi sám uváděl ve své ukázce. Nad obecnou entitou zavoláš konkrétní metodu.

Já když jsem to zkoušel naposledy, tak jsem našel problém ten, že se mi dost často měnilo, jak vypadá která podmínka, protože jednou tahám data z jedné tabulky, jednou z jiné, jindy musím ověřit přístup ke celé skupině dat atd

To je ale záležitost modelu, kam sahá a podle čeho to vyhodnocuje. Model prostě poskytuje data a poskytuje dostupná id. Někdo nad ním (v tomto případě presenter) to zpracuje a pokud je požadované id mezi povolenými, pustí tě, nebo zamítne. Ano, jako vždy je to něco za něco, pro tuto automatiku máme úzus, že id entity musí jít v parametru $id. Jiný to nezkontroluje. 99 % případů ale všichni používají právě parametr id. A když náhodou někdo pojmenuje parametr jinak, tak si prostě if na isAllowed napíše sám. Dveře k tomu zavřeny nejsou. Proč si ale těch 99 % případů nevychytat bez psaní…

Editoval Kamil Valenta (22. 10. 2022 20:00)

Bulldog napsal(a):

Vývojář tomu jen přiřadí resource_key a už to žije samo.

Tomu nerozumím jak to myslíš :D

Je to v kontextu předchozího postu. Ta traita SecurePresenter „snifne“ nový resource ve chvíli, kdy se zavolá nějaká nová actiona, render, nebo handle. A vývojář jen určí, jestli se to grupne do již existujícího oprávnění (alias Article:edit), nebo zda vznikne nové.

A to je přesně to, co nějak nerozumím. Chápu, že si předáváte ID pojmenováno vždy jako $id. Ale moc nerozumím tomu, jak poznáš, ze které databázové tabulky máš to ID brát? Respektive jestli je to ID článku, nebo ID komentáře atp. To je ta věc na které jsem ztroskotal. :)

Nemusíš se ohlížet na název presenteru (to bych ani nedělal). Ten presenter má přece injectovanou nějakou respository. Jí se ptáš a je ti jedno, do jakých tabulek ona sahá. Je to jen o konvenci. Tak jako ty máš u handleDelete $this->repository->getById($id); Stejný princip.

Bulldog napsal(a):
Ale většinou tu repository v Presenteru nemám nazvanou čistě repository, jelikož jich tam v rámci jednoho presenteru může být více.

Jasně, může mít více services, ale ani moc nepamatuju případ, kdy by jeden presenter operoval nad více entitami jako takovými. O to více, když je často řeč o jednoakčních presenterech…
Takže ostatní service ať se injectují do libovolného jména, ale ta jedna, podle které se ověřuje oprávnění, je „repository“.

Ale u presenterů, které sdružují více těch komponent a potřeboval bych ověřit více oprávnění naráz, tak tam si dovolit nazvat to obecně repository nemůžu.

Neviděl jsem to, tak nesoudím. Takto z té věty to na mě působí zvláštně, že jeden presenter sdružuje tak moc odlišné věci. Kdyby to ale v našich projektech nastávalo, tak bych si asi v těchto případech předával info o atributu, ve kterém je související model injectovaný, anotací dané metody, u které se oprávnění kontroluje…

@Bulldog Tvoje příspěvky byly tak obsáhlé a fundamentální, že se k nim po dovolené znovu vracím pro inspiraci.

Tedy mazat články, které mohu mazat můžu odkudkoliv, ale jen ty které mohu mazat

Přesně tak, naprostý souhlas. Místo ověřování při vytváření komponenty, nebo v action akce je daleko lepší ověřovat co nejblíže samotnému mazání – tedy v handleru komponenty. To se docela dost přibližuje mojí myšlence – autorizovat TĚSNĚ před vlastním provedením v modelu.

Modelu (nebo handleru který model volá) je úplně jedno, zda uživatel použil legální akci a existující tlačítko v aplikaci, nebo si sám hacknul svoji aplikaci manipulací s POST – pokud oprávnění má tak se mazání provede a pokud maže nějakým hackováním přičemž totéž může docílit legálně tlačítkem, tak na tom nezáleží, je to v pořádku.

// Tvoje autorizace v handleru

public function handleDelete(int $id)
{
	$order = $this->orderRepository->getById($id);

       if (!$order) {
           $this->error(404);
       }
	if (!$this->user->isAllowed($order, 'delete')) {
            $this->error(401);
        }
	$this->orderRepository->delete($order);	// autorizace předchází mazání - 99% neprůstřelné
	$this->redirect('grid');
}

Moje poznámka 99% neprůstřelné znamená, že mazání OrderRepository může teoreticky vyvolat i nějaký automatizovaný proces (složitější aplikace) který volá OrderRepository přímo a obejde handleDelete().

Nebo:

// moje autorizace v modelu - velmi velmi podobné jako v handleru

class OrderRepository
{
	......

	public function delete(ActiveRow $order)
	{
		....
		if (!$this->user->isAllowed($order, 'delete')) {
            		throw new App\Exceptions\NotAllowedException;
        	}
		$order->delete();  // autorizace předchází mazání - 100% neprůstřelné
	}
}

Zde píši 100%, protože pokud nejde model obejít jinudy, tak tohle je ve složité aplikaci 100%.

Ovšem jak Jsi psal, čistý jednoduchý model je také cenná deviza. Takže nakonec se mě začíná líbit spíš než moje validace v modelu Tvoje myšlenka validovat oprávnění v obslužných metodách komponent/formulářů. Zkombinovat to samozřejmě s validací základní úrovně oprávnění uživatele pro presenter jako takový ve startup(), v akcích nevalidovat, nemá to cenu, jde to stejně obejít, nebo pokud to uděláme neprůstřelně, tak je tam na můj vkus příliš mnoho kódu, ale oprávnění ke konkrétnímu záznamu a akci validovat v handleru. To mě v této chvíli přijde jako dobrá koncepce.

Validaci v modelu bych úplně neházel do koše, ale muselo by se to architektonicky vymyslet tak, aby se neporušovaly různé ty SOLID a jiné patterny a aby se model nezpřehlednil a dal se testovat. Kdyby se na modelu makalo, časem by se něco uspokojivého našlo – nějaká validační modelová vrstva nad jednoduchými repositáři. Ale má to asi smysl jedině u složitých projektů, kde jsou i nějaké automatizované procesy co volají model.

Tak díky všem za velmi podnětnou diskuzi, která mne posunula o kus dál :). Upřímně řečeno, když jsem si uvědomil bezpečnostní potíže validace oprávnění k záznamu v akci, tak mne sice napadlo autorizovat při vytváření komponenty, což se mě pro komplikovaný kód nelíbilo a pak mě napadlo autorizovat v modelu. Autorizovat v handleru mě nenapadlo, přitom je to tak dobrý nápad.

Editoval m.brecher (5. 11. 2022 15:47)

@mbrecher
Uff. Mám pár dotazů :)

Moje poznámka 99% neprůstřelné znamená, že mazání OrderRepository může teoreticky vyvolat i nějaký automatizovaný proces (složitější aplikace) který volá OrderRepository přímo a obejde handleDelete().

Tenhle automatizovaný proces snad má svoje oprávnění, které když je porušeno, tak k mazání snad nedojde ne?

nebo pokud to uděláme neprůstřelně, tak je tam na můj vkus příliš mnoho kódu

To si střílíš do vlastních řad mi přijde. Pokud validuješ až v modelu, tak ten kód validace tam stejně je a je ho stejně hodně. Mnohdy i více, jelikož musíš autorizovat přístupy ke všem možným metodám místo aby jsi prostě veškerou práci zakázal hned na začátku jedním ifem. A jako bonus tu tvou autorizaci v modelu musíš pak stejně v presenterech/komponentách nějak odchytávat try-catchem, takže ve výsledku je při sutorizaci v modelu daleko více kódu, akorát místo jednoho místa je rozházený různě po aplikaci ne?

Ale má to asi smysl jedině u složitých projektů, kde jsou i nějaké automatizované procesy co volají model.

Znova. Tyhle procesy mají nějaké své endpointy a jejich oprávnění se snad řeší tam ne? Nenapadá mě způsob, jak bych tohle mohl obejít.

Co se mě hlavně nelíbí – musíme si zavést parametr navíc, kam přeneseme výsledky autorizace v akcích a autorizovat de facto podruhé (DRY :( ) před vytvořením komponenty.

Ne nutně. S Kamilem jsme se tu bavili o tom, že se ten kód dá vyextrahovat někam ven a používat globální pravidla pro víceakční presentery a nebo to jednoduše vyřešit pomocí jednoakčních presenterů, kde tento problém odpadá.

Tohle poslední řešení sice vyžaduje také napsání jednoho ifu, ale ten kód navíc není roztahaný po celém presenteru jako s tím parametrem navíc $orderFormRight.

Hlavně pokud se nepletu, tak tyto tvé inovace zabíjí znovupoužitelnost. V případě, že by jsi tu komponentu potřeboval použít i v jiných presenterech, tak budeš muset přepisovat jako blázen, kde se to má jak použít. Mě větší smysl dává, jak jsme se bavili s Kamilem mít nějaké jednotné místo, kterému řekneš, které komponenty se mohou použít a je to. Nebo jak by jsi řešil znovupoužitelnost?

Validaci v modelu bych úplně neházel do koše, ale muselo by se to architektonicky vymyslet tak, aby se neporušovaly různé ty SOLID a jiné patterny a aby se model nezpřehlednil a dal se testovat.

Mě na tom pořád vadí to, že musíš postavit celé město, v něm každý dům, hotel, knihovny, zavést dopravu, naplnit lidmi, kteří se budou o všechno starat a nakonec zjistit, že ten, kvůli komu jsme to stavěli nemá do města přístup a tak město zase zboříme a to jen proto, že ten, kdo se o přístup stará je radní, který potřebuje radnici, která aby fungovala potřebuje město, protože radnice bez města nemá smysl a to potřebuje obyvatele… Místo aby byl hned na začátku člověk, který řekne kašlem na to město stavět nebudeme, protože ten týpek stejně do něho nemůže.

Když něco takového vidím, tak mám chuť vyskočit z okna a jít makat radši jako prodavač do Lidlu. :D

Nebo jak řešíš tenhle obrovský overload? Jedna firma v Olomouci (C#) mi řekla, že to řeší nákupem silnějšího HW, jelikož najmout kvalitní programátory, kteří píšou rychlý kód je dražší, než to nechat udělat blbě studenty a koupit silnější kompy. To mi ale přijde jako dost na sílu řešení.

Jak to tedy děláš ty, aby tvoje autorizace v modelu nezpomalovala aplikaci?

@mbrecher

No to město je dost přehnané.

Jestli jo, tak mi prosím ukaž celý kód, jak autorizuješ, jestli má daný uživatel právo přidávat/editovat články až modelu.

Stačí klidně vytvořit testovací repo na gitu kam to nahraješ, aby se nespamovala tady diskuze hromadami kódu.
mělo by vytvoření těch pár tříd na prezentaci zabrat max 5 minut, tak snad to ukážeš, protože mě taková optimalizace fakt zajímá a jestli to bude fakt lazy, tak se domluvíme i na nějaké odměně a naučíš mě to prosím.

EDIT

Jediný výkonnostní rozdíl je zavolání metody, jeden jediný řádek PHP kódu, žádné město

To taky není úplně pravda, jelikož v případě, že používáš správně accessory, tak jsou služby vytvářené až v případě, že je potřebuješ, což v případě autorizace v Presenteru může vyústit v to, že se nevytvoří, jelikož je nepotřebuješ, ale při autorizaci v modelu se budou muset vytvořit a ano, je to celé město, protože: (Tučně jsou označeny společné věci, které můžeme ignorovat, jelikož se vykonají vždy.)

Autorizace v Presenteru s Accessorem.

1. Vytáhne se daný Presenter
2. Předá se mu Accessor
3. Předá se mu Nette\Security\User
4. Useru se předá ACL
5. Zavolá se startup metoda, která ověří, jestli má uživatel přístup do aplikace $user->isAllowed('app', 'view')
6. Vrátí se 404

Autorizace v modelu:

1. Vytáhne se daný Presenter
2. Vytvoří se model, který se předá Presenteru
3. Tento model potřebuje Nette\Database\Explorer, takže se vytvoří
4. Explorer potřebuje Nette\Database\Connection, Nette\Database\Structure, Nette\Database\Conventions a Nette\Caching\IStorage (IStorage je jediná, kterou potřebuje i Security, takže tu nebudeme řešit.), takže se všechny vytvoří.
5. Connection potřebuje 4 řetězce, které se kopírují
6. Structure potřebuje Nette\Database\Connection (už vytvořená) a Nette\Caching\IStorage (Už vytvořené – ale stejně v konstruktoru opět volá new Nette\Caching\Cache)
7. Conventions je interface, takže se musí vytvořit a předat vše, co potřebuje konkrétní třída.
8. Presenteru se předá Nette\Security\User
9. Useru se předá ACL
10. Zavolá se render?? (otazníky, protože netuším, odkud ten model voláš, ale jinde, než v render to nedává smysl, když ta data, co vrací budeš vykreslovat, protože mít to v actioně by znamenalo, že by jsi měl extra proměnnou, přes kterou by jsi to dostával do renderu, což sám popisuješ, že proměnnou navíc nechceš, takže nadále budu předpokládat, že se volá v renderu až.) metoda, která zavolá model o data, který vrátí Exception
11. Render ale mezitím vytvořil šablonu, takže se vytvořily instance LinkGeneratoru, Translatoru, LatteEngine, registrovaly se všechny filtry
12. Odeslaly se hlavičky na klienta s HTTP kódem 200 (vytvoření extra response) a klient očekává odpověď (zaprasení bandwidth a mismatch návratových kódů)
13. Pak se Try/Catchem odchytila výjimka z modelu a znovu se vyhodila jiná (Nette\Application\BadRequestException)
14. Konečně 404, která však zmátla příjemce, jelikož v půlce stránky dostal najednou jiný kód…

Suma sumárum.
Presenter a jeho závislosti je vytvořen vždy, takže koukáme na to, co se děje v Presenteru.

Pokud máme autorizaci jen v presenteru, tak se vytvořil 1 objekt (Accessor) a zavolala 1 metoda (startup) a aplikace skončila.

V případě, že autorizuju v modelu, tak se navíc vytvořilo jestli dobře počítám minimálně 11 extra objektů (Model, Explorer, Connection, Structure, Conventions, Cache – v Connection, NotAllowedException, LinkGenerator, Translator, LatteEngine, Response s 200) a to nepočítám co všechno ještě na pozadí latte vytváří a co přesně bere konkrétní implementace Conventions… No ale naoko se zavolají jen 2 metody (render a metoda z modelu)

A promiň tedy, že jsem to nazval jako zbytečně vytvořené město, ale ano, přijde mi ta analogie správná, když místo 1 objektu a 1 metody musím tvořit minimálně 11 objektů a volat 2 metody.

Editoval Bulldog (5. 11. 2022 20:11)

@mbrecher

Bohužel je to tak, že i když autorizuješ v presenteru a model při 403 nepoužiješ, tak všechny ty služby co jsi v tom dlouhém seznamu uvedl se includují a vytvářejí se instance, přesně podle DI kaskády kdy si každá služba nechá od DIC předat svoje závislosti. Udělej si také testy a schválně napiš jak jsi dopadnul.

To je přesně důvod, proč několikrát píšu při použití Accessoru

To je interface, který vytváří třídy lazy. A můžeš ho mít jak v Presenteru, tak klidně až v Modelu.
Pokud ho máš v presenteru nad Modelem, tak by vypadal nějak takto:
Model:

class ArticleRepo
{
    use SmartObject;

    public function __construct(private readonly Explorer $explorer) { }

    public function findAll(): Selection
    {
        return $this->explorer->table('article');
    }
}

Accessor

interface ArticleRepoAccessor
{
    public function get(): ArticleRepo;
}

Presenter

class HomepagePresenter extends Nette\Application\UI\Presenter
{
    public function __construct(private readonly ArticleRepoAccessor $articleRepoAccessor)
    {
    }

	public function startup(): void
	{
		if (!$this->user->isAllowed('Article', 'view')) {
			$this->error('Not allowed', 401);
		}
		parent::startup();
	}

	public function renderDefault(): void
	{
		$this->template->articles = $this->articleRepoAccessor->get()->findAll();	// Až teď se vytvoří instance třídy ArticleRepo
	}
}

A vytvoří se jen instance třídy ArticleRepoAccessor. Tedy jak jsem psal, nic dalšího se zbytečně nevytvoří. Zkus si to.

Nicméně tento příklad pořád předpokládá, že všechno mrskáš na hulváta na presenter. Pokud ale správně používáš komponenty, tak accessory nepotřebuješ, jelikož se všechny tyhle závislosti na modelech vkládají až do komponent až v momentě, kdy komponentu pomocí továrny vytváříš, takže reálně se do Presenteru natáhne jenom továrna a všechny modelové třídy se nevytváří, pokud se nevytvoří komponenta, která se nevytvoří díky autorizaci v Presenteru ;)

Tedy:
Komponenta

class ArticleGridControl extends Control
{
    public function __construct(private readonly ArticleRepo $articleRepo) { }

    public function render(): void
    {
        $this->template->articles = $this->articleRepo->findAll();
    }
}

Továrna

interface ArticleGridControlFactory
{
    public function create(): ArticleGridControl;
}

Presenter

class HomepagePresenter extends Nette\Application\UI\Presenter
{
    public function __construct(private readonly ArticleGridControlFactory $articleGridControlFactory)
    {
    }

	public function startup(): void
	{
		if (!$this->user->isAllowed('Article', 'view')) {
			$this->error('Not allowed', 401);
		}
		parent::startup();
	}

	public function createComponentArticleGrid(): Nette\Application\UI\Control
    {
        return $this->articleGridControlFactory->create();
    }
}

ArticleRepo:
stejný jako předchozí

A výsledek:

Všimni si ale, že všechno ostatní krom označeného je stejné a taky, že ArticleRepo se ani v jednom případě nevytvořilo…

Pokud ale udělám ověření až v ArticleRepository:
Repo:

class ArticleRepo
{
    use SmartObject;

    public function __construct(
        private readonly Explorer $explorer,
        private readonly User $user,
    ) { }

    public function findAll(): Selection
    {
        if (!$this->user->isAllowed('Article', 'view')) {
            throw new \Exception('...');
        }
        return $this->explorer->table('article');
    }
}

Presenter:

class HomepagePresenter extends Nette\Application\UI\Presenter
{
    public function __construct(private readonly ArticleRepo $articleRepo)
    {
    }

    public function renderDefault(): void
    {
        try {
            $this->template->articles = $this->articleRepo->findAll();
        } catch (\Exception $e) {
            $this->error('Not Allowed', 403);
        }
    }
}

Výsledek:

A tady si všimni, že sice celkem je registrováno o 1 službu (Accessor, nebo továrnu) méně, ale zato se tvoří 4 další instance připojení k databázi, což i otestuje, jestli databáze existuje a připojení je platný atp…
Takže jak vidíme, tak přidání ‚zbytečné‘ třídy navíc může kód dost zjednodušit. To je ale obecný problém a ne fičura Nette

Atd. atd.

2. nebo v render<View>,

To je právě ten problém. V případě, že od sebe Presentery dědíš, tak pokud v BasePresenteru máš metodu beforeRender, ve které nastavuješ nějaké globální věci, tak v ten moment tvoříš šablony, což je úplně zbytečné, když pak v konkrétní render metodě vyhodíš 404, nebo 401 atp. Pak vznikají takovéto chyby, kdy se třeba error vypíše přes stránku a návratový HTTP kód je chybný…:

Což je například obří problém z hlediska SEO a nebo GoogleAnalytics, kterému vlastně říkáš, že je vše ok a tak se error stránky zaindexují.

kde předává do šablony k vykreslení objekt ActiveRow, který v sobě data nemá, ale data z databáze získá až při vykreslování

To právě není pravda. Takhle se chová Selection, ale ActiveRow už v sobě ta data má. Dokonce ti Tracy vypíše, že se dotaz udělal. Zkus si to :)

Moc mě to už nebaví abych pravdu řekl.

Editoval Bulldog (5. 11. 2022 23:18)

@mbrecher
Abych pravdu řekl, je toho dost.

1. Články se mažou, ale odkazy zůstávají a lidi na ně klikají → overload
2. Oprávnění uživatele se mění (například vyprší mu předplatné), ale on má odkazy v oblíbených, nebo záložkách → overload
3. Dojde při přestávce na kafe k automatickému odhlášení → overload
4. Pokud na stránku existují odkazy a tedy ji najdou boti a ti uvidí, že se používá Nette, tak začnou zkoušet automatizovaně formulářové signály napříč stránkami → overload + možnost nechtěnného zobrazení privátních informací – jelikož než se dostaneš k vytažení resource, kterou zakazuješ v modelu, už se mohly natáhnout a odeslat informace, které vykresluješ předtím a ke kterým má přístup…
5. Error stránky nevrací správný kód → overload z opakované indexace + zmatení vyhledávačů a zabijárna SEO

a můžeme pokračovat. Ale to už je asi mimo Nette.

Editoval Bulldog (6. 11. 2022 13:10)