Project

General

Profile

Segnalazione #863

Aggiungere permission checking a operation e API

Added by Elena Grandi 11 months ago. Updated 8 months ago.

Status:
Chiuso
Priority:
Normale
Assignee:
Start date:
06/28/2019
Due date:
% Done:

0%

Resolution:
fixed

Related issues

Related to fuss-manager - Segnalazione #862: form/view di login Chiuso 06/28/2019
Blocked by fuss-manager - Segnalazione #856: interfaccia con LDAP asincrona per tornado Chiuso 06/28/2019

Associated revisions

Revision a908b879 (diff)
Added by Elena Grandi 8 months ago

Pass user informations to operations. refs: #863

Revision 5728ad26 (diff)
Added by Elena Grandi 8 months ago

Start checking permissions for operations. refs: #863

Revision 8002fce0 (diff)
Added by Elena Grandi 8 months ago

Fix some tests and code errors. refs: #863

Revision 5374e745 (diff)
Added by Elena Grandi 8 months ago

Start testing permissions. refs: #863

Revision 8469c804 (diff)
Added by Elena Grandi 8 months ago

Further permission tests. refs: #863

Revision ce151ab6 (diff)
Added by Elena Grandi 8 months ago

Permissions on remaining ops. refs: #863

Revision 02ee4927 (diff)
Added by Elena Grandi 8 months ago

Permission checking on sync and async methods. refs: #863

Revision 3a0e70f6 (diff)
Added by Elena Grandi 8 months ago

Further webapi permissions. refs: #863

Revision 124717db (diff)
Added by Elena Grandi 8 months ago

Override the user if passed by js in decoded. refs: #863

Revision 3f870563 (diff)
Added by Elena Grandi 8 months ago

Use @wraps on decorators. refs: #863

Revision 65d962e3 (diff)
Added by Elena Grandi 8 months ago

Move checking for permissions to a common place. refs: #863

Revision a1aa0926
Added by Enrico Zini 8 months ago

Merging t863 into master. Refs: #863

History

#1 Updated by Elena Grandi 11 months ago

#2 Updated by Elena Grandi 11 months ago

#3 Updated by Elena Grandi 8 months ago

#4 Updated by Elena Grandi 8 months ago

#5 Updated by Elena Grandi 8 months ago

  • Status changed from Nuovo to Commenti
  • Assignee changed from Elena Grandi to Enrico Zini

Ripasso il ticket per review e merge.

Mi pare ci sia quasi tutto, solo non sono sicura se aggiungere in tests/test_webserver.py un test in più per le operations che testi un'operation che richieda permessi, perché non son sicura che funzioni correttamente (sospetto che al momento non dia un forbidden, ma un 500 o simili).

#6 Updated by Enrico Zini 8 months ago

  • Assignee changed from Enrico Zini to Elena Grandi

Bel branch, mi piace molto!

Alcuni commenti piú o meno estetici.

In manager/ops.py:58, suggerisco un decoded.pop("user", None) per buttar via 'user' se viene passato per sbaglio da javascipt, altrimenti il costruttore di Op lanca un'eccezione per un parametro passato due volte.

In alternativa, forse meglio, un override pari pari: decoded["user"] = user.

Nel decoratore permission_required, in caso user sia None, possiamo dare per scontato che il permesso non ci sia, e quindi semplificare i vari getattr successivi, o c'è la possibilità che vengano assegnati permessi all'utente None?

Nei decoratori, aggiungi un functools.wraps, cosí vengono preservati gli attributi della funzione originale nella funzione wrappata: https://docs.python.org/3.5/library/functools.html#functools.wraps

Visto che il controllo dei permessi è replicato nei due decoratori di ops e di webapi, propongo di spostarlo in una funzione unica in perms.py, cosí quella parte, abbastanza security sensitive, rimane in un posto solo per testarla, fare code review, e sistemare bug.

#7 Updated by Elena Grandi 8 months ago

Enrico Zini wrote:

In manager/ops.py:58, suggerisco un decoded.pop("user", None) per buttar via 'user' se viene passato per sbaglio da javascipt, altrimenti il costruttore di Op lanca un'eccezione per un parametro passato due volte.

In alternativa, forse meglio, un override pari pari: decoded["user"] = user.

fatto con l'alternativa override

Nel decoratore permission_required, in caso user sia None, possiamo dare per scontato che il permesso non ci sia, e quindi semplificare i vari getattr successivi, o c'è la possibilità che vengano assegnati permessi all'utente None?

Mi pare realistico che l'utente None possa avere ReadonlyBaseAccess.

Nei decoratori, aggiungi un functools.wraps, cosí vengono preservati gli attributi della funzione originale nella funzione wrappata: https://docs.python.org/3.5/library/functools.html#functools.wraps

fatto

Visto che il controllo dei permessi è replicato nei due decoratori di ops e di webapi, propongo di spostarlo in una funzione unica in perms.py, cosí quella parte, abbastanza security sensitive, rimane in un posto solo per testarla, fare code review, e sistemare bug.

fatto, anche questo.

#8 Updated by Elena Grandi 8 months ago

  • Assignee changed from Elena Grandi to Enrico Zini

#9 Updated by Enrico Zini 8 months ago

  • Status changed from Commenti to Chiuso
  • Resolution set to fixed

Perfetto, mergiato in master, chiudo.

Also available in: Atom PDF