#8 N'importe qui peut modifier un autre utilisateur en étant déconnecté !

Closed
opened 4 months ago by Eragon · 19 comments
Eragon commented 4 months ago

Si je suis déconnecté et que je vais sur la page admin/edit-account/1 je peut éditer le compte principal, lui changer le mot de passe, son pseudo, toutes les donnés accessibles et modifiables normalement uniquement par un admin

Si je suis déconnecté et que je vais sur la page `admin/edit-account/1` je peut éditer le compte principal, lui changer le mot de passe, son pseudo, toutes les donnés accessibles et modifiables normalement uniquement par un admin
Lephenixnoir commented 4 months ago
Owner

Cette page est protégée à juste titre par le filtre @priv_required('edit-account'). Cependant le filtre priv_required est actuellement désactivé à des fins de test par le paramètre LOGIN_DISABLED dans Config.py (le nom n’est plus très approprié).

La raison est simplement qu’on travaille régulièrement avec des bases de données où il n’y a pas de compte administrateur par défaut, donc aucun compte n’a de privilèges et aucun compte ne peux les attribuer.

Une façon correcte de réactiver le filtre serait de s’assurer qu’un compte administrateur existe toujours.

Cette page est protégée à juste titre par le filtre `@priv_required('edit-account')`. Cependant le filtre `priv_required` est actuellement désactivé à des fins de test par le paramètre `LOGIN_DISABLED` dans `Config.py` (le nom n'est plus très approprié). La raison est simplement qu'on travaille régulièrement avec des bases de données où il n'y a pas de compte administrateur par défaut, donc aucun compte n'a de privilèges et aucun compte ne peux les attribuer. Une façon correcte de réactiver le filtre serait de s'assurer qu'un compte administrateur existe toujours.
Eragon commented 4 months ago
Poster

Ok, on peut fermer l’issue, mais à mettre dans la doc du coup.
Ou alors modifier la bdd par défaut pour avoir un utilisateur admin par défaut.

Ok, on peut fermer l'issue, mais à mettre dans la doc du coup. Ou alors modifier la bdd par défaut pour avoir un utilisateur admin par défaut.
Eragon commented 4 months ago
Poster

Par contre, petit bug, dans la navbar, j’ai le lien vers le panel d’administration même quand je n’ai pas la permission de l’utiliser.
C’est rien puisque après on à une 403 mais c’est un peu nul…

Par contre, petit bug, dans la navbar, j'ai le lien vers le panel d'administration même quand je n'ai pas la permission de l'utiliser. C'est rien puisque après on à une 403 mais c'est un peu nul...
Lephenixnoir commented 3 months ago
Owner

Ça c’est un bug par contre. Facile à corriger. Tu veux tenter ta chance ? ^^

N’oublie pas d’activer la vérification des permissions dans models/users.py pour tester.

Ça c'est un bug par contre. Facile à corriger. Tu veux tenter ta chance ? ^^ N'oublie pas d'activer la vérification des permissions dans `models/users.py` pour tester.
Eragon commented 3 months ago
Poster

Je crois que j’ai réglé ça, mais je n’ai pas push la modif… je bosse plus trop dessus en ce moment, j’arrive pas à me focus sur mon BAC mais je fout rien d’utile non-plus, le 26 Juin je vais essayer de revenir bosser sérieusement sur la v5, j’aurais fini mes épreuves du BAC

Je crois que j'ai réglé ça, mais je n'ai pas push la modif... je bosse plus trop dessus en ce moment, j'arrive pas à me focus sur mon BAC mais je fout rien d'utile non-plus, le 26 Juin je vais essayer de revenir bosser sérieusement sur la v5, j'aurais fini mes épreuves du BAC
Lephenixnoir commented 3 months ago
Owner

Pas de problème, le fait est que les droits sont mal gérés actuellement. Bon courage pour ton bac :smiley:

Pas de problème, le fait est que les droits sont mal gérés actuellement. Bon courage pour ton bac :smiley:
Darks referenced this issue from a commit 3 months ago
Lephenixnoir commented 3 months ago
Owner

Je crois que t’as pas lu tout le problème Darks. Actuellement les privilèges ne sont même pas consultés !

Je crois que t'as pas lu tout le problème Darks. Actuellement les privilèges ne sont même pas consultés !
Darks commented 3 months ago
Owner

Chaque accès à une page du panel d’administration est vérifié via le décorateur @priv_required. Ce décorateur se charge de vérifier que l’utilisateur est connecté et a bien les droits pour accéder à la page, quelque soit la méthode (GET, POST ou whatever).

priv_required.py

def priv_required(*perms):
    """
    Requires the user to be an authenticated member with privileges [perms].
    Calls :attr:`LoginManager.unauthorized` if the user is not authenticated,
    and a 403 if some of the privileges are missing.

    Example:
        @app.route('/admin')
        @priv_required('access-admin-board')
        def admin_board():
            pass

    It can be convenient to globally turn off authentication when unit testing.
    Setting the `LOGIN_DISABLED` configuration variable to `True` will silence
    this decorator.
    """
    def decorated_view(func):
        @wraps(func)
        def wrapped(*args, **kwargs):
            if request.method in EXEMPT_METHODS:
                return func(*args, **kwargs)
            elif app.config.get('LOGIN_DISABLED'):
                return func(*args, **kwargs)
            elif not current_user.is_authenticated:
                return app.login_manager.unauthorized()
            else:
                for p in perms:
                    if not current_user.priv(p):
                        # TODO: Add error message and privilege name
                        abort(403)
            return func(*args, **kwargs)
        return wrapped
    return decorated_view

On remarque que trois cas de figure sont gérés :

  • le type de requête fait qu’on a pas besoin d’authentification (OPTIONS uniquement) ;
  • on a manuellement désactivé la protection ;
  • tout va bien et on check les droits.

Les privilèges sont donc bien consultés, mais ignorés à des fins de tests. Bien entendu il faudra modifier la ligne dans config.py avant tout passage en prod.

Si vous voulez vraiment tester les privilèges, je vous invite à désactiver en local le flag, puis tester.

Chaque accès à une page du panel d'administration est vérifié via le décorateur `@priv_required`. Ce décorateur se charge de vérifier que l'utilisateur est connecté et a bien les droits pour accéder à la page, quelque soit la méthode (`GET`, `POST` ou whatever). [priv_required.py](/devs/PCv5/src/branch/master/app/utils/priv_required.py) ```python def priv_required(*perms): """ Requires the user to be an authenticated member with privileges [perms]. Calls :attr:`LoginManager.unauthorized` if the user is not authenticated, and a 403 if some of the privileges are missing. Example: @app.route('/admin') @priv_required('access-admin-board') def admin_board(): pass It can be convenient to globally turn off authentication when unit testing. Setting the `LOGIN_DISABLED` configuration variable to `True` will silence this decorator. """ def decorated_view(func): @wraps(func) def wrapped(*args, **kwargs): if request.method in EXEMPT_METHODS: return func(*args, **kwargs) elif app.config.get('LOGIN_DISABLED'): return func(*args, **kwargs) elif not current_user.is_authenticated: return app.login_manager.unauthorized() else: for p in perms: if not current_user.priv(p): # TODO: Add error message and privilege name abort(403) return func(*args, **kwargs) return wrapped return decorated_view ``` On remarque que trois cas de figure sont gérés : - le type de requête fait qu'on a pas besoin d'authentification (`OPTIONS` **uniquement**) ; - on a manuellement désactivé la protection ; - tout va bien et on check les droits. Les privilèges sont donc bien consultés, mais ignorés à des fins de tests. Bien entendu il faudra modifier la ligne dans `config.py` avant tout passage en prod. Si vous voulez vraiment tester les privilèges, je vous invite à désactiver en local le flag, puis tester.
Eragon commented 3 months ago
Poster

J’ai désactivé le flag sur ma version locale je crois bien, mais le second problème que j’ai fait remonter est l’affichage du lien vers le panel d’administration même lorsqu’on à pas les droits pour y accéder, ce lien ne sert à rien si on est pas autorisé à y accéder mais il serait mieux de le cacher, et après vérification je n’ai pas patché ça, je ne sais pas comment vérifier si un utilisateur à cette permission ou pas(je ne connais aps le nom de la permission)

J'ai désactivé le flag sur ma version locale je crois bien, mais le second problème que j'ai fait remonter est l'affichage du lien vers le panel d'administration même lorsqu'on à pas les droits pour y accéder, ce lien ne sert à rien si on est pas autorisé à y accéder mais il serait mieux de le cacher, et après vérification je n'ai pas patché ça, je ne sais pas comment vérifier si un utilisateur à cette permission ou pas(je ne connais aps le nom de la permission)
Lephenixnoir commented 3 months ago
Owner

Je suis bien d’accord avec cette analyse Darks, et je reconnais ne pas avoir été clair.

Je pense qu’on devrait avoir le flag LOGIN_DISABLED désactivé même pendant les tests et surtout sur la branche master du dépôt, parce que sinon c’est pas sérieux du tout.

Eragon : Normalement Darks a corrigé ça dans son commit d’hier.

Je suis bien d'accord avec cette analyse Darks, et je reconnais ne pas avoir été clair. Je pense qu'on devrait avoir le flag `LOGIN_DISABLED` désactivé même pendant les tests et surtout sur la branche master du dépôt, parce que sinon c'est pas sérieux du tout. Eragon : Normalement Darks a corrigé ça dans son commit d'hier.
Eragon commented 3 months ago
Poster

Ok, et je suis ok pour que le flag soit désactivé par défaut

Ok, et je suis ok pour que le flag soit désactivé par défaut
Lephenixnoir commented 3 months ago
Owner

Je rouvre pour qu’on fasse les modifications appropriées ce soir, il faudra ajouter une ou deux fonctions pour éviter qu’on se retrouve sans compte administrateur et sans possibilité d’en recréer un.

Je rouvre pour qu'on fasse les modifications appropriées ce soir, il faudra ajouter une ou deux fonctions pour éviter qu'on se retrouve sans compte administrateur et sans possibilité d'en recréer un.
Eragon commented 3 months ago
Poster

Une fonction à appeler dans le code, ou un script de déploiement à lancer manuellement qui fasse ça ? Genre un fichier create_defaultusers.sh dans le dossier scripts qui, via le code du site, crée les utilisateurs qu’il faut(le bot, le compte communautaire, et possiblement les groupes(ce qui permettrais d’initialiser le bdd sans avoir lancé le site) Est-ce que c’est une bonne idée? Est-ce que je suis clair ?
Sinon en effet une fonction qui, permette de créer un utilisateur avec l’id 1 qui soit super-admin(utilisateur au nom de root en toute logique, penser à bloquer les noms semblables pour les autres utilisateurs)

Une fonction à appeler dans le code, ou un script de déploiement à lancer manuellement qui fasse ça ? Genre un fichier `create_defaultusers.sh` dans le dossier `scripts` qui, via le code du site, crée les utilisateurs qu'il faut(le bot, le compte communautaire, et possiblement les groupes(ce qui permettrais d'initialiser le bdd sans avoir lancé le site) Est-ce que c'est une bonne idée? Est-ce que je suis clair ? Sinon en effet une fonction qui, permette de créer un utilisateur avec l'id 1 qui soit super-admin(utilisateur au nom de `root` en toute logique, penser à bloquer les noms semblables pour les autres utilisateurs)
Lephenixnoir commented 3 months ago
Owner

Je pensais à remplacer la variable LOGIN_DISABLED par une variable DEV_MODE qui donne accès une unique page spéciale dans laquelle on peut nettoyer la base, attribuer des privilèges, et d’autres opérations spéciales. Ce serait une sorte de god mode local.

Je pensais à remplacer la variable `LOGIN_DISABLED` par une variable `DEV_MODE` qui donne accès une unique page spéciale dans laquelle on peut nettoyer la base, attribuer des privilèges, et d'autres opérations spéciales. Ce serait une sorte de god mode local.
Lephenixnoir self-assigned this 3 months ago
Lephenixnoir commented 3 months ago
Owner

J’ai introduit un master script dans le commit a3b867bab5 qui manipule l’application depuis la ligne de commande et est effectivement un god mode local.

Il contient deux fonctionnalités :

  1. Réinitialiser tous les groupes et les comptes communautaires
  2. Affilier n’importe quel membre à n’importe quel groupe

Et c’est tout. Ce script ne sert qu’à mettre une installation de Planète Casio en route, les autres tâches d’administration doivent se faire depuis le panel admin de l’interface web.

J'ai introduit un *master script* dans le commit a3b867bab5 qui manipule l'application depuis la ligne de commande et est effectivement un god mode local. Il contient deux fonctionnalités : 1. Réinitialiser tous les groupes et les comptes communautaires 2. Affilier n'importe quel membre à n'importe quel groupe Et c'est tout. Ce script ne sert qu'à mettre une installation de Planète Casio en route, les autres tâches d'administration doivent se faire depuis le panel admin de l'interface web.
Eragon commented 3 months ago
Poster

Parfait, on ne peut pas l’utiliser depuis le site ? Si c’est ça, je ne pense pas qu’on puisse faire plus sécure comme install

Parfait, on ne peut pas l'utiliser depuis le site ? Si c'est ça, je ne pense pas qu'on puisse faire plus sécure comme install
Lephenixnoir commented 3 months ago
Owner

Non, on ne peut pas l’utiliser depuis le site. Une installation de Planète Casio se fait comme ça :

  1. Installer le serveur web et la bdd
  2. Utiliser le script maître pour initialiser les groupes et quelques utilisateurs
  3. Lancer le serveur web (mais personne n’a les droits admin sur Planète Casio)
  4. Créer un premier compte
  5. Utiliser le script maître pour lui donner les droits admin
  6. Faire toute la suite par l’interface du site

Et il n’y a pas de variable de config’ à changer entre le mode de dev’ et la production.

Non, on ne peut pas l'utiliser depuis le site. Une installation de Planète Casio se fait comme ça : 1. Installer le serveur web et la bdd 2. Utiliser le script maître pour initialiser les groupes et quelques utilisateurs 3. Lancer le serveur web (mais personne n'a les droits admin sur Planète Casio) 4. Créer un premier compte 5. Utiliser le script maître pour lui donner les droits admin 6. Faire toute la suite par l'interface du site Et il n'y a pas de variable de config' à changer entre le mode de dev' et la production.
Eragon commented 3 months ago
Poster

Comment un admin peut donner des permissions aux autres membres ?
Je n’ai pas vu ça dans le panel d’administration(pour ajouter un modo, un dev…etc)

Comment un admin peut donner des permissions aux autres membres ? Je n'ai pas vu ça dans le panel d'administration(pour ajouter un modo, un dev...etc)
Lephenixnoir commented 3 months ago
Owner

En effet ce n’est pas encore implémenté, je vais le faire bientôt dans la page Groupes et Privilèges !

En effet ce n'est pas encore implémenté, je vais le faire bientôt dans la page Groupes et Privilèges !
Sign in to join this conversation.
No Milestone
No Assignees
3 Participants
Due Date

No due date set.

Dependencies

This issue currently doesn't have any dependencies.

Loading…
Cancel
Save
There is no content yet.