Ověřování oprávnění a loginu ve formuláři

Jak při odeslání, tak při výpisu je třeba ověřovat oprávnění. I bez toho, abys mi formulář zobrazil ho mohu odeslat, pokud vím, jaká data ode mne čeká. Stejně tak pokud už má uživatel stránku načtenou a mezi zobrazením formuláře a odesláním mu odebereš oprávnění.

Ahoj, myslím že řešit přihlášení/kontrolu až v metodě renderNěco() je už dost pozdě. Užitečnější je startup() nebo actionNěco() tam se to vyřeší rychleji a navíc tam jde i řešit co/kam vlastně dál. Navíc ten startup() zařízne všechno včetně formulářů.

Úplně nejrychlejší to bude v checkRequirements(), dost možná se některé objekty totiž ani nebudou muset instancovat.

Záleží, jak má fikusir aplikaci navrženou, ale většinou to je tak, že bývá podstránka fixnutá na určitou akci.
Tedy jakože je jedna podstránka na přidání něčeho, jedna na editaci něčeho atp.
Podle dnešních trendů máš mít ale na každou takovou akci samotný presenter..
No ovšem někdy se stane, že máš třeba video (co je komponenta jež můžeš zobrazit) a k němu komentáře, které můžeš mazat, editovat, přidávat zobrazovat…

Normálně by pro každou z těchto částí vypadalo řešení oprávnění jinak.

V případě, že máš na každou akci zvlášť presenter by prostě stačilo ve startup metodě zakázat celou práci s Presenterem a jeho komponentami, jak píše Marek Bartoš.

Pokud ale máš v presenteru více akcí, z nichž některé mohou uživatelé vykonávat a jiné ne, tak toto nebude fungovat a musíš vložit ověření do ‚action‘ metody, jak píší Ozzrel a Rick Strafy. Problém zde je, že pokud to ověříš pouze v action metodě, tak jsem schopný z akce do které mám přístup poslat validní request na odeslání formuláře z akce, do které přístup nemám a to proto, že komponenty nejsou vázány na žádnou akci presenteru, ale existují souběžně ve všech akcích, jak píše Marek Bartoš.

Ověřování až v komponentách je ale pozdě. Komponenta se musí vytvořit, nějak na něco napojit atp.. Pokud uživatel nemá oprávnění, tak by se komponenta neměla už ani vytvořit, natož tak po vytvoření sama oprávnění hlídat.

Nejlepší řešení, které jsem zatím viděl vypadalo takto:

class ArticlePresenter extends Presneter
{
	#[Inject] public ArticleModifyFormControlFactory $articleModifyFormControlFactory;
	#[Inject] public ArticleRepository $articleRepository;
	private ?Article $article = null; // Může být i ActiveRow
	private bool $canCreateArticleModifyForm = false;

	public function actionAdd(): void
	{
		if (!$this->user->isAllowed('article', 'add')) {
			$this->error('Not allowed', IResponse::S403_FORBIDDEN);
		}
		$this->canCreateArticleModifyForm = true;
	}

	public function actionEdit(int $id): void
	{
		$article = $this->articleRepository->getById($id);
		if (!$article) {
			$this->error('Not found', IResponse::S404_NOT_FOUND);
		}
		if (!$this->user->isAllowed($article, 'edit')) {
			$this->error('Not allowed', IResponse::S403_FORBIDDEN);
		}
		$this->canCreateArticleModifyForm = true;
	}

	public function actionDetail(int $id): void
	{
		$article = $this->articleRepository->getById($id);
		if (!$article) {
			$this->error('Not found', IResponse::S404_NOT_FOUND);
		}
		if (!$this->user->isAllowed($article, 'view')) {
			$this->error('Not allowed', IResponse::S403_FORBIDDEN);
		}
		$this->template->article = $article;
	}

	public function createComponentArticleModifyForm(): Control
	{
		if (!$this->canCreateArticleModifyForm) {
			$this->error('Not allowed', IResponse::S403_FORBIDDEN);
		}
		return $this->articleModifyFormControlFactory->create($this->article);
	}
}

Toto řešení povoluje v Presenteru Article jen 3 akce, tedy přidání, editaci a zobrazení. (Pro jednoduchost berme, že zobrazení daného článku je přímo v šabloně presenteru a proto máme jen 1 komponentu a to pro formulář.)

V takto navrženém presenteru mluví akce samy za sebe, ale pro jistotu to ještě popíšu:

1. Pokud uživatel přijde na akci přidat, tak pokud nemá práva editora a tedy nemůže přidat nový článek, tak jej zastaví už funkce ‚actionAdd‘. V případě, že má práva editora, tak se nastaví proměnná povolující vytvořit danou komponentu formuláře na true, což znamená, že podmínka projde v pořádku a tedy se komponenta v klidu vykreslí.
2. Pokud uživatel přijde na akci editovat, tak presenter vytáhne daný článek z databáze a pokud článek neexistuje, nebo nemáme právo editovat tento konkrétní článek, tak nás samotná metoda ‚actionEdit‘ dále nepustí. Pokud ale článek existuje a právo máme, tak se opět nastaví proměnná povolující tvorbu komponenty a komponenta se vykreslí. (Komponenta je navržena tak, aby když jí jako parametr přijde null, jde o přidání, když ji přijde instance třídy, jde o editaci a o to, jestli může člověk tuto danou instanci editovat se už komponenta nestará (single responsibility. Komponenta se nemá starat o editaci a zároveň o práva.))
3. Pokud uživatel přijde ovšem do akce detail, tak nás zase action metoda nepustí, pokud článek neexistuje, nebo nemáme právo jej vidět… Ovšem jelikož v akci detail se komponenta formuláře nepoužívá, tak logicky se nenastaví povolení komponentu vytvořit a tovární metoda pro tvorbu komponenty se vůbec nezavolá.
4. Pokud uživatel nacházející se v akci přidání bude chtít odeslat formulář tak, aby se tvářil jako editace článku některého kolegy, ke kterému má i nemá přístup, tak mu toto aplikace nepovolí, protože komponenta, když je napsána tímto způsobem je spojena s danou akcí a to tak, že v akci add se nikdy nenaplní vnitřní proměnná ‚article‘ a tedy v rámci formuláře ať útočník dělá co chce se vždy bude jednat o přidání.
5. Pokud uživatel nacházející se v akci editace bude chtít odeslat formulář tak, aby editoval jinou instanci (jiný článek), tak by musel pozměnit parametr ID v URL adrese, jenže tímto pozměněním jej nepustí již Action metoda, jelikož na editaci tohoto článku nemá oprávnění. Pokud by chtěl poslat request, který by vypadal jako přidání článku, tak je toto opět nemožné, jelikož v akci edit se vždy naplní Presenterovská vnitřní proměnná ‚article‘, takže se bude vždy jednat o editaci. Může takto editovat sice jiný článek, ke keterému má přístup na editaci, ale to ublíží jen sám sobě. Něco jako kdyby jste chtěli ukrást si samo sobě peníze ze svého druhého bankovního účtu…
6. Pokud je v akci detail a pokusí se odeslat request, který odesílá formulář, tak se formulář bude tvářit, jako kdyby se snažil přidat článek. (Neplní se vnitřní proměnná article, takže si formulář myslí, že jde o přidání). Zde se stane to, že formulář se bude chovat jinak a tovární metoda ‚createComponentArticleModifyForm‘ se zavolá těsně po action metodě, která proběhne v pořádku (pokud je v detailu článku, který může vidět.) a ne až při vykreslování formuláře a to proto, že těsně po ‚action‘ metodách se volají signály (‚handle‘ metody), přičemž odeslání formuláře je taktéž signál a aby bylo možné formulář odeslat, je nutné jej nejdříve vytvořit… Jenže vzhledem k tomu, že v akci detail se nenastavuje proměnná ‚canCreateArticleModifyForm‘ na true, tak podmínka uvnitř továrničky na form selže a vyhodí error, který je vyhozen v signálu, tedy v čase, kdy se ještě nezačaly odesílat HTTP hlavičky na klienta a tedy je zde error správně, protože se neodeslaly ani hlavičky, ani se ještě nevytvořila komponenta formuláře, což šetří systémový čas a nemusí se nikde nic nijak odchytávat zevnitř komponent a komponenty se tím pádem stávají nezávislé na oprávněních.

V případech, kdy komponenta má v sobě komponentu se kterou na základě oprávnění můžeme pracovat je to také jednoduché. Například seznam mých článků, kde seznam je jedna komponenta a jednotlivý řádek/buňka je druhá komponenta. Zde chceme, abychom nemohli mazat články, které nejsou naše a tedy například abychom uvnitř Element komponenty mohli volat signál ‚handleDelete()‘ jen nad Elementy, které mazat můžeme.
Řešení je jednoduché.
Do první komponenty (Gridu chcete-li) si pošleme kolekci článků, které chceme zobrazit. Správnost této kolekce ověřuje již presenter, takže pro nevalidní data Grid komponentu vůbec nevytvoří… Tato Grid komponenta poté na validních datech vytvoří podkomponentu, nad kterou se zavolá daný signál. Pokud Grid komponenta ve své validní kolekci nemá element s daným ID (nemá danou podkomponentu, nad kterou chceme signál volat), nemůže tedy danou podkomponentu vytvořit a zobrazí se 404…

EDIT 1:
V případě, že by se v akci detail plnila vnitřní proměnná ‚article‘, tak se nic nemění. Formulář by si myslel sice, že jde o editaci, ale díky nenastavené proměnné ‚canCreate…‘ by se formulář vůbec nevytvořil, takže no stress.

EDIT 2:
A aby nevypadal presenter zaneřáděně a daly se použít komponenty ve více presenterech, tak je dobré cpát obslužné věci do trait. :)

Editoval Polki (8. 9. 2021 17:46)

@KamilValenta

(co když někdo u nějaké actiony zapomene)

tak tímto můžeš argumentovat všude. Co když uživatel zapomene zaheslovat databázi? Co když zapomene vypnout debug mód? Co když zapomene nastavit práva v checkRequirements?…

BTW. tohle řešení na to ovšem myslí a je blbuvzdorné…

Představ si pro tu komponentu traitu:

trait PresenterTrait
{
	#[Inject] public ControlFactory $articleModifyFormControlFactory;
	private ?Article $article = null; // Může být i ActiveRow
	private bool $canCreateArticleModifyForm = false;

	public function createComponentArticleModifyForm(): Control
	{
		if (!$this->canCreateArticleModifyForm) {
			$this->error('Not allowed', IResponse::S403_FORBIDDEN);
		}
		return $this->articleModifyFormControlFactory->create($this->article);
	}
}

a Presenter:

class ArticlePresenter extends Presneter
{
    use PresenterTrait;

	#[Inject] public ArticleRepository $articleRepository;

	public function actionAdd(): void
	{
		if (!$this->user->isAllowed('article', 'add')) {
			$this->error('Not allowed', IResponse::S403_FORBIDDEN);
		}
		$this->canCreateArticleModifyForm = true;
	}

	public function actionEdit(int $id): void
	{
		$article = $this->articleRepository->getById($id);
		if (!$article) {
			$this->error('Not found', IResponse::S404_NOT_FOUND);
		}
		if (!$this->user->isAllowed($article, 'edit')) {
			$this->error('Not allowed', IResponse::S403_FORBIDDEN);
		}
		$this->canCreateArticleModifyForm = true;
	}

	public function actionDetail(int $id): void
	{
		$article = $this->articleRepository->getById($id);
		if (!$article) {
			$this->error('Not found', IResponse::S404_NOT_FOUND);
		}
		if (!$this->user->isAllowed($article, 'view')) {
			$this->error('Not allowed', IResponse::S403_FORBIDDEN);
		}
		$this->template->article = $article;
	}
}

1. ‚actionEdit‘ nezapomeneš napsat, protože vyžaduje parametr
2. ‚actionDetail‘ nezapomeneš napsat, protože vyžaduje parametr
3. ‚actionAdd‘ můžeš zapomenout napsat, ale to je v pořádku, protože v případě, že se na ni bude chtít KDOKOLIV dostat aniž by byly uvnitř ošetřená práva, bude mít zakázáno komponentu vytvořit a tedy vyskočí Error, který na to upozorní, takže si programátor v klidu vzpomene.

Výsledek: Jelikož jsou komponenty ve výchozím stavu zakázány a povolujeme je jen tam, kde je chceme pomocí action metod, nemůže nastat, že by někdo tam, kde chce komponentu použít zapomněl action metodu s právy napsat a zároveň když by opomněl, tak žádná bezpečnostní díra nehrozí. Nikdy.

Oproti tomu, jak by asi vypadalo checkRequirements?

class ArticlePresenter extends Presneter
{
	#[Inject] public ArticleRepository $articleRepository;

	public function checkRequirements($element): void
	{
        parent::checkRequirements($element);

		if ($this->isLinkCurrent('Article:add') && !$this->user->isAllowed('article', 'add')) {
			$this->error('Not allowed', IResponse::S403_FORBIDDEN);
		} elseif ($this->isLinkCurrent('Article:edit')) {
             $id = $this->getParameter('id');
             $article = $this->articleRepository->getById($id);
             if (!$article) {
			     $this->error('Not found', IResponse::S404_NOT_FOUND);
		     }
             if (!$this->user->isAllowed($article, 'edit') {
                 $this->error('Not allowed', IResponse::S403_FORBIDDEN);
             }
        } elseif ($this->isLinkCurrent('Article:detail')) {
             $id = $this->getParameter('id');
             $article = $this->articleRepository->getById($id);
             if (!$article) {
			     $this->error('Not found', IResponse::S404_NOT_FOUND);
		     }
             if (!$this->user->isAllowed($article, 'view') {
                 $this->error('Not allowed', IResponse::S403_FORBIDDEN);
             }
        }
        if ($this->isLinkCurrent('Nějak ověřím, jestli je odeslán signál od komponenty/odeslání formu????') &&
                  !$this->isLinkCurrent('Article:detail') &&
                  !$this->isLinkCurrent('Article:add')) {
             $this->error('Not allowed', IResponse::S403_FORBIDDEN);
        }
        if ...// další komponenty, které mohou být vykresleny za různých podmínek spolu i odděleně od formuláře a mají handly???
	}
}

Jinak si to moc nedokážu představit, jelikož každá akce ověřuje co uživatel může vůči jinému privilege a taky jedna to dělá prostě nad řetězcem jako resource, jedna nad entitou jako resource, takže se to horko těžko bude nějak sjednocovat a v takovém rozvětveném ifování se upřímně dá vyznat hůře, než v oddělených action metodách a tedy je to hůře udržitelné (co kdyby bylo akcí více? řekněme 40 a pak se nějaké privilege změní? Navíc ověřovat tady třeba to, jestli se náhodou nevolá signál komponenty, kterou volat nemám je řekněme strašně nepřehledné, složitě se přemýšlí co všechno a kde musím jak ověřit pro jaký případ a dá se do toho lehce zamotat a tedy něco opomenout. Jednoduše vyznat se ve stromě ifů je peklo, zatímco když je to v action metodách, tak hned člověk ví, kterou action metodu změnit, jelikož ví, pro co se změnilo privilege…). Proto se dnešní programy píší tak, aby v nich bylo co nejméně ifů a když, tak jen s jedním příkazem uvnitř o jednom zanoření. (Tím jsou myšleny takové ty ify, které ověří validitu vstupu a kdyžtak hned vyhodí výjimku, nebo ukončí vykonávání funkce.)

Řešení s checkRequirements se hodí do projektů, kde Presenter = Action. Tam je to super, protože je jeden jediný if a to ten, který by byl v action metodě je už v checkRequirements, která se volá o dost dříve, než action metoda.

PS:

V checkRequirements() si pokryju všechny actiony z jednoho místa. Takže zapomenout na žádnou ani nemohu.

z několika nezávislých zkušeností, kdy jsem seděl s nováčky a zkoušel je vím, že když se to nováček snažil dávat na jedno místo (do startupu, nebo do checkRequirements), tak hned, jak napsali kód, co byl v zadání, tak jsem jim řekl, aby připsali novou metodu, která něco dělá a má nějaká oprávnění.
Kromě 1 člověka VŠICHNI napsali action metodu, kde si předali parametry, napsali šablonu, zkusili refresh stránky, zjistili, že jsou na to vidět odkazy, tak odkazy skryly, ale upravit podle toho tu checkRequirements/startup už zapomněli, takže jsem jim na tu stránku přes odkaz normálně došel. V 99% se shodovaly i odpovědi, když jsem se zeptal, proč tedy neošetřili i přímý přístup přes URL. Odpovědi zněly nějak takto: „Já jsem to chtěl/a udělat nakonec a jelikož ověřování se děje ve startupu, tak jsem na to pak zapoměl/a, a myslel/a si, že už to tam je…“

Takže ze zkušenosti nováčkům dělá problém dopisovat nový kód do starých už hotových metod, které fungují. Naopak, když psali ověření do action metod, tak na to nezapomněli nikdy (opravdu ani jeden), protože věděli, že když už píšou action metodu pro předání parametrů, tak do ní rovnou rubnou i autorizaci/resourceGainCheck atp. a dávalo jim to i větší smysl, protože každá akce se opravdu starala jen sama o sebe a ne o ostatní…
Startup/checkRequirements tedy používáme jen pro ověření globálních pravidel, jako že musíš být admin na přístup do AdminModulu…

Aby sis nemyslel, že je to nějaký zeronumber, tak těch nováčků už jsem takto odzkoušel kolem 90.

Editoval Polki (8. 9. 2021 22:20)

@KamilValenta Ale myslím si, že pořád jsou naše obě řešení ještě v normě.

Viděl jsem v jedné firmě borce Lukáše, který se tvářil, že zná vše, všechny a všude byl a razil názor že každý presenter má jen jednu akci a taky každý presenter musí být v separátním modulu a taky, že všechny callbacky formulářů má v jedné třídě FormService a jeho presenter vypadal nějak takto:

namespace App\Modules\UserModule\Presenters;

use ...;

final class SettingPresenter extends AbstractPresenter
{
	/** @var UserFormFactory @inject */
	public $userFormFactory;

	/** @var UserSettingFormFactory @inject */
	public $userSettingFormFactory;

	/** @var UserSettingFormService @inject */
	public $userSettingFormService;

	protected function createComponentUserForm(): Form
	{
		$form = $this->userFormFactory->create();
		$form->onAnchor[] = function (Form $form): void {
			$this->userFormFactory->setDefaultsByEntity($form, $this->user->identity);
		};

		return $form;
	}

	protected function createComponentUserSettingForm(): Form
	{
		$form = $this->userSettingFormFactory->create();
		$form->onAnchor[] = function (Form $form): void {
			$this->userSettingFormFactory->setDefaultsByEntity($form, $this->user->identity);
		};
		$form->onSuccess[] = function (Form $form, ArrayHash $values): void {
			$this->userSettingFormService->processForm($form, $values, $this->user->identity);
		};
		$form->onSuccess[] = function (Form $form, ArrayHash $values): void {
			$this->flashMessage('Nastavení změněno.', 'success');
			$this->redirect('this');
		};
		$form->onError[] = function (Form $form): void {
			foreach ($form->errors as $error) {
				$this->flashMessage($error, 'danger');
			}
		};

		return $form;
	}
}

Takže vlastně nastavoval callbacky, které se mají provést až v dané createComponent metodě, což jsem viděl bylo použito i v action metodách, takže se danému formuláři v action metodě nastavil success callback, což mělo za následek to, že když byl uživatel ve špatné akci, tak sice form odeslal, ale nic se nestalo, jelikož form neměl navěšenou onSuccess metodu… :D

EDIT 1:
userSettingFormService má v sobě jen 1 metodu a to processForm. No a pro každý existující formulář existuje taková třída zaregistrovaná jako service s 1 metodou jako callbackem formu :)

Editoval Polki (8. 9. 2021 22:42)

Ano, v momentě, kdy si checkRequirements() napíšeš takto, tak tím skutečně můžeš demonstrovat jeho nevhodnost.
Pokud ho ale napíšeš nějak takto:

public function checkRequirements($element)
    {
        if (!$this->user->isLoggedIn()) {
            $this->getHttpResponse()->setCode(\Nette\Http\Response::S401_UNAUTHORIZED);
            $this->forward(':User:Auth:unauthorized', $this->storeRequest());
        }

        if (!$this->user->isAllowed($this->getReflection()->getName(), $this->formatActionMethod($this->action))) {
            $this->getHttpResponse()->setCode(\Nette\Http\Response::S403_FORBIDDEN);
            $this->forward(':User:Auth:forbidden');
        }

        parent::checkRequirements($element);
    }

tak tím můžeš demonstrovat jeho vhodnost. Je to součástí traity, která obdobným způsobem testuje i signály a rendery.
Takže skutečně nemusím checkRequirements() rozšiřovat, je obecný a pokrývá všechny actiony, handly a rendery.
Maximálně ho některé projekty ještě dědí a rozšiřují o getParameter() – to pro případ, že nechci rozslišovat jen mám / nemám přístup, ale i mám přístup na zázmam…

Kamil Valenta napsal(a):

@Polki moc nechápu, proč bych si měl checkRequirements() zanášet nějakým testováním, zda entita existuje a případně vykopávat 404. To má dělat router. A pokud mi to router pustil a neposlal to do 404, tak je URL legitimní. Myslím, že do sebe mícháš věci, které k sobě nepatří…

To je diskutabilní.
Existují názory, kdy by router měl sahat do databáze pro entity a zjišťovat, jestli existují a na základě toho házet 404, nebo do Presenteru posílat přímo Entitu a odkaz generovat přímo z Entity.

Na druhou stranu existují názory, že router by se měl starat opravdu jen o routování a o nějakých entitách by neměl vůbec nic tušit, natož se na ně dotazovat do databáze. Validní URL podle tohoto je ta, která odpovídá požadovanému formátu i když záznam neexistuje a existenci záznamu zjistíme až tam, kde s ním chceme pracovat, tedy v presenteru.

Většina lidí preferuje 2 způsob a to tedy, že router se má starat jen o vzhled URL adres a netahat nic z DB a to i kvůli rychlosti. (například špatně navržený router může injectovat zbytečně mnoho services, může dělat zbytečné dotazy do DB a v neposlední řadě, pokud nemá uživatel právo manipulovat s nějakou resource, tak toto právo můžeme ověřit ještě před samotným dotazem do DB a ušetřit tak system time. Příklad je třeba že pokud jdeš na URL http://mujweb.cz/article/edit/7 a můžeš editovat články jen pokud jsi přihlášený, tak ověření přihlášení provádíš až v presenteru, přičemž v případě, že přihlášený nejsi úplně zbytečně v routě taháš článek s id 7, který poté předáváš do presenteru, jelikož presenter vyhodí error, not allowed…)
Pokud ale vezmeme v potaz i to, že máš použitý první způsob a děláš dotazy do DB rovnou v routeru, tak tvoje řešení je značně neúplné.

Vysvětlení:

if (!$this->user->isAllowed( // v pořádku.
        $this->getReflection()->getName(), // Tímto spojíš natvrdo název presenteru s názvem resource. To ale není pravda, jelikož na jedné stránce (v rámci 1 presenteru) můžeš mít ověření klidně vícero různých resources. Příkladem je třeba opět můj presenter s články. Pokud jsem v detailu článku, tak mám hned 2 resources. 1 je samotný článek a 2 jsou komentáře, kdy můžu třeba komentáře přidávat jen pokud jsem přihlášený/mám odběr/mám předplatné a stejné podmínky mohou být pro článek, ale každá může být jiná. Tímto řešením máš sice jednoduchý chceckRequirements, ale nechci si ani představovat, jak hrůzný a NEznovupoužitelný musí být ACL.
        $this->formatActionMethod($this->action), // Stejné jako předchozí řádek. Natvrdo spojuješ názvy akcí s privileges, což taky není optimální a navíc můžeš v rámci 1 akce chtít různě povolit/zakázat různé signály a touto konstrukcí se ptáš jen konkrétní presenter/akce. Zapomínáš ale, že hierarchie resource/privilege != presenter/action
    )) {
    $this->getHttpResponse()->setCode(\Nette\Http\Response::S403_FORBIDDEN);
    $this->forward(':User:Auth:forbidden');
}

BTW: handly nepokrývá. (nevím, co máš v metodě formatActionMethod, takže to nemůžu říct narovinu, ale ve výchozím stavu ne nepokrývá) a dělat si v ACL všechny možné kombinace preseter/action/signal které mohou nastat mi přijde jako systime sebevražda, ve které udělat chybu je snad ještě jednodužší, než v těch ifech, protože na nějakou kombinaci určitě někdy zapomeneš.

Editoval Polki (9. 9. 2021 1:09)

Kamil Valenta napsal(a):

Ano, v momentě, kdy si checkRequirements() napíšeš takto, tak tím skutečně můžeš demonstrovat jeho nevhodnost.
Pokud ho ale napíšeš nějak takto:

public function checkRequirements($element)
    {
        if (!$this->user->isLoggedIn()) {
            $this->getHttpResponse()->setCode(\Nette\Http\Response::S401_UNAUTHORIZED);
            $this->forward(':User:Auth:unauthorized', $this->storeRequest());
        }

        if (!$this->user->isAllowed($this->getReflection()->getName(), $this->formatActionMethod($this->action))) {
            $this->getHttpResponse()->setCode(\Nette\Http\Response::S403_FORBIDDEN);
            $this->forward(':User:Auth:forbidden');
        }

        parent::checkRequirements($element);
    }

tak tím můžeš demonstrovat jeho vhodnost. Je to součástí traity, která obdobným způsobem testuje i signály a rendery.
Takže skutečně nemusím checkRequirements() rozšiřovat, je obecný a pokrývá všechny actiony, handly a rendery.
Maximálně ho některé projekty ještě dědí a rozšiřují o getParameter() – to pro případ, že nechci rozslišovat jen mám / nemám přístup, ale i mám přístup na zázmam…

Tvoje řešení funguje jen do té doby, dokud nevyběhneš ze seznamu action. U mě je běžné, že například default, show, detail mají stejnou action read.

Jojo, už jsem si vybral :) Ověřování jsem prozatím přesunul do action a zároveň nechal v událostech komponenty. Asi existuje lepší řešení, ale v mých podmínkách si s tímto vystačím. Moc díky všem, diskuze je poučná

Ne, router nemá co autentikovat (a myslel jsem přesně to, co jsem napsal, tak já to většinou mám…)
Router má říct, jestli je URL legitimní, pokud ano, tak ji přeložit na presenter/action, pokud ne, nechat propadnout na 404.

O autentikaci a autorizaci se má starat ten, kdo odbavuje request, což je cotroller, resp. presenter.

Pokud to Ty vidíš jinak, tak to já respektuju a nemám potřebu Ti to vyvracet.

@mbrecher
Nejvíc bezpečné je udělat si na každou jednu akci samostatný Presenter a tomu oprávnění řešit v startup, případně v checkRequirements, které se volá dřív, jak píše Kamil.

Pokud rozdělíš jen Presentery na veřejné a neveřejné, tak můžeš zase narazit.
Zkus si přepsat do startupu následující kód:

class ArticlePresenter extends Presenter
{
	#[Inject] public ArticleRepository $articleRepository;
	#[Inject] public ArticleFormFactory $articleFormFactory;
	private ?Article $article = null;

	protected function startup()
	{
		parent::startup(); // TODO: Change the autogenerated stub
		if (!$this->user->isAllowed('Article', 'view')) {
			$this->error('Bohužel nemáte oprávnění toto vidět', 401);
		}
	}

	public function actionAdd() {
		if (!$this->user->isAllowed('Article', 'add')) {
			$this->error('Bohužel nemáte oprávnění toto vidět', 401);
		}
	}

	public function actionEdit(int $id) {
		$this->article = $this->articleRepository->getById($id);
		if (!$this->article) {
			$this->error('Bohužel daný článek neexistuje', 404);
		}
		if (!$this->user->isAllowed($this->article, 'edit')) {
			$this->error('Bohužel nemáte oprávnění tento článek editovat', 401);
		}
	}

	public function createComponentArticleForm(): Form // jeden formulář jak pro editaci, tak pro přidání
	{
		return $this->articleFormFactory($this->article, function() {
			$this->flashMessage('Úspěšně odesláno', 'success');
			$this->redirect('this');
		});
	}
}

Tady to bude trošku složitější. Tento Presenter je neveřejný, jak píšeš, ale obsahuje 2 rozdílné akce, která má kadá úplně odlišná práva, jelikož řešíš, jestli článek, co edituješ náleží opravdu tobě.
Je to jako tady na fóru. Vytvářet odpovědi a editovat je můžeš jen když jsi přihlášený, tedy Presenter je neveřejný. Zároveň ale nesmíš být schopen editovat odpovědi jiných lidí. Kdyby jsi to mohl dělat, bylo by to blbé.

Řešení je:

1. na každou akci jiný presenter (to ovšem bude presenterů jako blázen a vyznat se v tom není úplně ok máme to na projektu odzkoušené a mít v jedné složce 73 presenterů není úplně ok – a to nepočítám traity kvůli duplicitě kódu)
2. narvat to do startupu všechno, ale to bude ifování jako pes, přičemž budeme zjišťovat, kdy kdo co může dělat, jelikož někdy článek je, jindy není, zjistit na základě jména akce jestli má být apod…
3. mít prostě na jednu logickou část jeden presenter, tedy na všechnu práci s články bude presenter Article, který bude mít třeba 5 akcí, kde KAŽDOU tuto akci je třeba ověřit úplně stejně, jako v bodě 1, akorát nebude ověření rozkouskováno do 5 presenterů, ale bude v 1 a v tomto presenteru také dodělat ověření v KAŽDÉ createComponent metodě a v KAŽDÉ handle metodě s tím, že vykonávání VŠEHO vždy defaultně ZAKÁZAT a povolovat to jen tam, kde se to opravdu může použít.

Každopádně dobře jsi rozebral, jak to vidíš ty. Je to zase jiný pohled na věc, který vnese trošku světla do problematiky.

Editoval Polki (8. 10. 2021 1:00)

Polki napsal(a):

@mbrecher
Ještě jsem zapomněl zmínit, že máš pravdu a když má presenter jen 1 akci, tak toto:

<?php

declare(strict_types=1);

namespace App\Presenters;

use Nette\Application\UI\Presenter;

final class HomepagePresenter extends Presenter
{

	public function actionDefault(): void
	{
		if ($this->user->isLoggedIn()) {
			$this->error();
		}
	}

	public function handleSignal(): void
	{
		bdump('ahoj');
	}

}

Je nedostatečné, protože teoreticky se to dá obejít přes neexistující akci, například když do URL zadám /homepage/neexistujici-akce?do=signal
tak Nette správně vyhodnotí, že Page not found. Missing template 'D:\wamp64\www\testNette31\app\Presenters\templates\Homepage\neexistujiciAkce.latte'., ale handle metoda se provede a řetězec ahoj se dumpne.

Řešení problému s neexistujícími akcemi je to, že z routeru odebereš obecnou routu a necháš routing routovat vždy jen ty routy, které existují. Takže v tomto případě když bude router vypadat třeba takto:

final class RouterFactory
{
	use Nette\StaticClass;

	public static function createRouter(Nette\Security\User $user): RouteList
	{
		$router = new RouteList;
		$router->addRoute('homepage', 'Homepage:default');
		//$router->addRoute('<presenter>/<action>[/<id>]', 'Homepage:default'); // odebrána obecná routa
		return $router;
	}
}

tak se nikdo přes neexistující akci k signálu nedostane, jelikož ten dotaz na tu URL vyhodí No route for HTTP request.
Otázka potom je, co si chce člověk vybrat.

1. univerzální routu, 1 presenter = 1 akce, ověřování ve startup metodě
2. konkrétní routy, 1 presenter = 1 akce, ověřování v action metodě, kde už máme přístupné parametry z URL
3. dělat to nějakým jiným způsobem z těch, které jsou definované v minulém komentáři.

Prezentery s jednou akcí můžeš řešit i tak, že upravíš metody ve výchozím prezenteru či traite tomu určené, které vrací aktuální název akce.

Počítáno s tím, že v routě máš výchozí akci default.

https://github.com/…resenter.php#L494

    public static function formatActionMethod(string $action): string{
      if($action !== parent::DEFAULT_ACTION){
          throw new Nette\Application\BadRequestException();
      }

        return 'action';
    }

https://github.com/…resenter.php#L503

    public static function formatRenderMethod(string $view): string{
        return 'render';
    }

Výhoda? Nemusíš používat názvy metod actionDefault ani renderDefault, ale pouze action a render.

Editoval Marek Znojil (8. 10. 2021 9:23)

Polki napsal(a):

@MarekZnojil
U toho, když ma 1 presenter 1 akci to samozřejmě znemožní zkusit použít jiné akce, než je default, takže to taky zabrání spuštění signálu z jiné akce.

Ovšem najdou se i lidé, kteří řeknou, ze když je to v tomto případě jednoznačné, že existuje jen akce default, tak by se o to měl starat router, takže buď odebrat obecnou routu, nebo ji tam hodit filtr na akci, který pusti pouze default akci, takže se vubec nebude případně tvořit presenter.
Nicméně přejmenování metod na čistě action a render mi přijde stylové.

Máš pravdu a souhlasím s tebou. Pouze jsem chtěl poukázat i na tuto možnost, jelikož tímto vyřešíš oba případy. Omezíš akce prezenteru pouze na jednu a zkrátíš název metod.

Správnější (?) přístup z logiky věci řešit jaká akce má být zobrazena je v routě. Řešení na které jsem poukázal je spíše pohodlné, protože budeš mít jednu traitu (nebo BasePresenter), kterou si použiješ dle libosti a nemuší řešit individuální routy pro každý prezenter zvlášť u kterého chceš, aby byl ‚single action‘.