feature/auth-hardening #24
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feature/auth-hardening"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Fixes #16
Doklady přes
/media/jsou pořád dostupné bez přihlášení (issue #15). Chtělo by to přidat routu, která ověřuje práva, a defaultní routu/media/smazat.@ -51,6 +51,25 @@ Ve webovém rozhraní je možné požádat o výpis z bankovního účtu u Fio b**Důležité**: Pokud se požadují data starší než 90 dní, je nutné manuálně autorizovat API klíč v internetovém bankovnictví Fio banky.## OprávěníDrobnost: překlep
@ -54,0 +63,4 @@| **Přihlášený** | Základní | Může prohlížet semináře, ke kterým má přístup || **Účetní (accountant)** | Rozšířený | Může spravovat transakce, proplácení, importovat výpisy z banky || **Správce (staff)** | Administrativní | Může spravovat uživatele a jejich oprávnění || **Superuživatel** | Plný | Má všechny práva včetně správy Django adminu |Pár drobností k tomu, co je tady napsáno:
is_staffnestačí, je na to potřeba ještěauth.change_usernebo být superuživatelemTakhle je to teď více odpovídající realitě:
Seminar.organizers)budget.accountant)is_staff)@ -79,0 +79,4 @@def admin_required(view_func):"""Restricts view to staff users only."""Nebyl by (vzhledem ke komentáři a kódu) lepší název
staff_required?@ -0,0 +58,4 @@<ul class="unstyled-list text-small">{% for perm in user_info.permissions %}<li>{{ perm }}</li>{% endfor %}Superuživatel tady nebude mít nic vypsaného, i když má všechna práva. Možná speciálně v permissions vypsat že je to superuživatel?
@ -0,0 +48,4 @@def test_record_lookup_enumeration_blocked(self):"""Test that records cannot be enumerated without authentication."""# This simulates the attack described in AUTH.mdAUTH.md? Žádné nevidím. Na necommitnuté soubory by se nemělo nic odkazovat.
@ -0,0 +60,4 @@self.assertEqual(response.status_code, 302)# No records should be foundself.assertEqual(len(found_records), 0)Tenhle assert projde vždycky, do
found_recordsse nic nikde nepřidává.@ -320,3 +321,4 @@@login_requireddef record_lookup(request):@login_requiredje málo, teď to pořád vrátí všechny záznamy z daného dne všem přihlášeným, i když v README se píše:Buď to chce předefinovat co kdo může (v README), nebo tady přidat lepší kontrolu / filtrování, nebo to omezit jenom na accountant.
@ -326,3 +328,4 @@Requires authentication to prevent enumeration attacks."""code = request.GET.get("code", "").strip()(Forgejo neumí dát komentář níž, tak ho dávám sem)
record_lookuppořád spadne na nevalidním vstupu. Za regex bych přidal něco jako:@ -449,0 +464,4 @@perms = set()for group in user.groups.all():perms.update(group.permissions.values_list("codename", flat=True))perms.update(user.user_permissions.values_list("codename", flat=True))Pozor,
prefetch_relatedse tu nevyužije:values_list()vytvoří nový queryset a ten pošle vlastní dotaz, takže se na každého uživatele (a každou jeho skupinu) pošle dotaz navíc. Přednačtená data čte jen.all().Stačí přednačíst i práva skupin a pole vytáhnout v Pythonu:
a
@ -449,0 +471,4 @@"is_staff": user.is_staff,"is_superuser": user.is_superuser,"permissions": sorted(perms),})V tomhle výpisu úplně schází semináře, jejich je uživatel členem.
Dál tu schází třeba stav, jestli je aktivní nebo ne (když už máme to políčko
is_activev databázi).A ještě: systém práv je teď v aplikaci hrozně překomplikovaný, protože Django si tam dogenerovává tunu vlastních práv, která ale nikde nepoužíváme. Jestli dobře počítám, tak jich je přes 50 různých (třeba
budget.{add,change,delete,view}_{seminar,event,record,banktransaction,virtualtransaction}+ podobně rozsáhlé sety nainvoicer.aauth.).Bylo by pěkné systém práv nějak rozumně popsat, zbavit se tohohle hrozného balastu automatických práv a používat jenom naše práva. Přemýšlím, jestli pak nedává smysl mít práva scoped = můžeš mít právo
accountant(*)nebo třeba jenomaccountant(KSP)a mít to všechno navázané na tohle 🤷 Ale to by chtělo asi nejdřív zkusit někam sepsat.Současná práva viz dotaz do SQLite:
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.