From 740df09b9d73ef98f71f99a2d1423bf30db51ce7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= <sybren@stuvel.eu>
Date: Thu, 8 Jun 2017 11:35:33 +0200
Subject: [PATCH] User edit form: prevent accidentally revoking roles

Prevent accidentally revoking roles that were not part of the form.
---
 pillar/web/users/forms.py  | 19 ++++++++++++++++---
 pillar/web/users/routes.py | 12 ++++++++----
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/pillar/web/users/forms.py b/pillar/web/users/forms.py
index fb272fdf..15293c4a 100644
--- a/pillar/web/users/forms.py
+++ b/pillar/web/users/forms.py
@@ -59,8 +59,21 @@ class UserSettingsEmailsForm(Form):
         'Notifications', choices=choices, coerce=int)
 
 
+# TODO: refactor roles to be obtainable from the Pillar application.
 class UserEditForm(Form):
-    role_choices = [('admin', 'admin'),
-                    ('subscriber', 'subscriber'),
-                    ('demo', 'demo')]
+    ROLES = [
+        'admin',
+        'badger',
+        'demo',
+        'flamenco-admin',
+        'flamenco_manager',
+        'flamenco-user',
+        'homeproject',
+        'protected',
+        'service',
+        'subscriber',
+        'svner',
+        'urler',
+    ]
+    role_choices = [(r, r) for r in ROLES]
     roles = SelectMultipleField('Roles', choices=role_choices)
diff --git a/pillar/web/users/routes.py b/pillar/web/users/routes.py
index 1b23492c..b60e7b95 100644
--- a/pillar/web/users/routes.py
+++ b/pillar/web/users/routes.py
@@ -236,7 +236,7 @@ def users_edit(user_id):
 def _users_edit(form, user, api):
     """Performs the actual user editing."""
 
-    from pillar.api.service import role_to_group_id, ROLES_WITH_GROUPS
+    from pillar.api.service import role_to_group_id
 
     current_user_roles = set(user.roles or [])
     current_user_groups = set(user.groups or [])
@@ -244,11 +244,15 @@ def _users_edit(form, user, api):
     roles_in_form = set(form.roles.data)
 
     granted_roles = roles_in_form - current_user_roles
-    revoked_roles = ROLES_WITH_GROUPS - roles_in_form
+    revoked_roles = set(UserEditForm.ROLES) - roles_in_form
 
     # role_to_group_id contains ObjectIDs, but the SDK works with strings.
-    granted_groups = {str(role_to_group_id[role]) for role in granted_roles}
-    revoked_groups = {str(role_to_group_id[role]) for role in revoked_roles}
+    granted_groups = {str(role_to_group_id[role])
+                      for role in granted_roles
+                      if role in role_to_group_id}
+    revoked_groups = {str(role_to_group_id[role])
+                      for role in revoked_roles
+                      if role in role_to_group_id}
 
     user.roles = list((current_user_roles - revoked_roles).union(granted_roles))
     user.groups = list((current_user_groups - revoked_groups).union(granted_groups))