swarming: switch to a 'capability focused' ACL system

appengine/swarming/server/acl.py

Index: appengine/swarming/server/acl.py
diff --git a/appengine/swarming/server/acl.py b/appengine/swarming/server/acl.py
index c28e54f447af06bf3cb383e043ef32b033fc3be1..0d4c436ff54758708dc66c54a38c0a9b1ae4bc75 100644
--- a/appengine/swarming/server/acl.py
+++ b/appengine/swarming/server/acl.py
@@ -9,25 +9,40 @@ from components import utils
 from server import config
 
 
-def is_admin():
-  admins = config.settings().auth.admins_group
-  return auth.is_group_member(admins) or auth.is_admin()
+def _is_admin():
+  """The admins group is a super set of the privileged users group."""

[Vadim Sh., 2017/07/24 23:07:26]

This statement is false. 

A group is a set of users, by definition.

Admins group (a set of users) is not a super set of privileged users group.
"admins_group" group doesn't include all users in "privileged_users_group"
group.

[M-A Ruel, 2017/07/25 13:46:38]

Reworded

+  group = config.settings().auth.admins_group
+  return auth.is_group_member(group) or auth.is_admin()
 
 
-def is_privileged_user():
-  priv_users = config.settings().auth.privileged_users_group
-  return auth.is_group_member(priv_users) or is_admin()
+def _is_privileged_user():
+  """The privileged users group is a super set of the users group."""
+  group = config.settings().auth.privileged_users_group
+  return auth.is_group_member(group) or _is_admin()
 
 
-def is_user():
-  users = config.settings().auth.users_group
-  return auth.is_group_member(users) or is_privileged_user()
+def _is_user():
+  group = config.settings().auth.users_group
+  return auth.is_group_member(group) or _is_privileged_user()
 
 
-def is_bootstrapper():
+def _is_view_all_bots():
+  group = config.settings().auth.view_all_bots_group
+  return auth.is_group_member(group) or _is_privileged_user()
+
+
+def _is_view_all_tasks():
+  group = config.settings().auth.view_all_tasks_group
+  return auth.is_group_member(group) or _is_privileged_user()
+
+
+def _is_bootstrapper():
   """Returns True if current user have access to bot code (for bootstrap)."""
   bot_group = config.settings().auth.bot_bootstrap_group
-  return is_admin() or auth.is_group_member(bot_group)
+  return _is_admin() or auth.is_group_member(bot_group)
+
+
+### Capabilities
 
 
 def is_ip_whitelisted_machine():
@@ -39,44 +54,107 @@ def is_ip_whitelisted_machine():
       auth.bots_ip_whitelist(), auth.get_peer_ip(), False)
 
 
-def is_bot():
-  # TODO(vadimsh): Get rid of this. Swarming jobs will use service accounts
-  # associated with the job when calling Swarming, not the machine IP.
-  return is_ip_whitelisted_machine() or is_admin()
+def can_access():
+  """Minimally authenticated user."""
+  return (
+      is_ip_whitelisted_machine() or _is_user() or
+      _is_view_all_bots() or _is_view_all_tasks())
+
+
+#### Config
+
+
+def can_view_config():
+  """Can view the configuration data."""
+  return _is_admin()
+
+
+def can_edit_config():
+  """Can edit the configuration data.
+
+  Only super users can edit the configuration data.
+  """
+  return _is_admin()
+
+
+#### Bot
+
 
+def can_create_bot():
+  """Can create (bootstrap) a bot."""
+  return _is_admin() or _is_bootstrapper()
 
-def is_bot_or_user():
-  # TODO(vadimsh): Get rid of this. Swarming jobs will use service accounts
-  # associated with the job when calling Swarming, not the machine ID itself.
-  return is_bot() or is_user()
 
+def can_edit_bot():
+  """Can terminate, delete a bot.
 
-def is_bot_or_privileged_user():
-  # TODO(vadimsh): Get rid of this. Swarming jobs will use service accounts
-  # associated with the job when calling Swarming, not the machine ID itself.
-  return is_bot() or is_privileged_user()
+  Bots can terminate other bots. This may change in the future.
+  """
+  return is_ip_whitelisted_machine() or _is_privileged_user()

[Vadim Sh., 2017/07/24 23:07:26]

I think it should be _is_admin instead of _is_privileged_user.

Why would users terminate bots? As a reminder, in our main case 'privileged
users' is all googlers.

(As I said before, I think we should get rid of 'privileged' vs 'non-priviliged'
users distinction. We are not using it in practice. Almost all our users are
privileged. It just confuses things. If we want to hide something from
non-googlers we should just run it on internal Swarming instance. Trying to use
privileged vs non-priviliged difference to hide stuff on single Swarming
instance is dangerous, at least until we redo UI to be inherently multi-tenant).

[M-A Ruel, 2017/07/25 13:46:38]

So that any Googler can quickly take a bot out of rotation, including build
sheriffs.
Removing groups is out of scope for this CL.

 
 
-def is_bot_or_admin():
-  """Returns True if current user can execute user-side and bot-side calls."""
-  # TODO(vadimsh): Get rid of this. Swarming jobs will use service accounts
-  # associated with the job when calling Swarming, not the machine ID itself.
-  return is_bot() or is_admin()
+def can_view_bot():
+  """Can view bot.
+
+  Bots can view other bots. This may change in the future.
+  """
+  return (
+      is_ip_whitelisted_machine() or _is_privileged_user() or
+      _is_view_all_bots())
+
+
+#### Task
+
+
+def can_create_task():
+  """Can create a task.
+
+  Swarming is reentrant, a bot can create a new task as part of a task. This may
+  change in the future.
+  """
+  return is_ip_whitelisted_machine() or _is_user()
 
 
 def can_schedule_high_priority_tasks():
   """Returns True if the current user can schedule high priority tasks."""
-  return is_bot() or is_privileged_user()
+  # TODO(maruel): Deny priority < 100 task creation instead of silently
+  # downgrading the priority.
+  return is_ip_whitelisted_machine() or _is_privileged_user()
+
+
+def can_edit_task(task):
+  """Can 'edit' tasks, like cancelling.
+
+  Since bots can create tasks, they can also cancel them. This may change in the
+  future.
+  """
+  return (
+      is_ip_whitelisted_machine() or _is_privileged_user() or
+      auth.get_current_identity() == task.authenticated)
+
+
+def can_edit_all_tasks():
+  """Can 'edit' a batch of tasks, like cancelling."""
+  return _is_privileged_user()
+
+
+def can_view_task(task):
+  """Can view a single task.
+
+  It is possible that the user can only see a subset of the tasks.

[Vadim Sh., 2017/07/24 19:16:47]

this line no longer applies

[M-A Ruel, 2017/07/25 13:46:38]

Done.

+  """
+  return (
+      is_ip_whitelisted_machine() or _is_view_all_tasks() or
+      _is_privileged_user() or
+      auth.get_current_identity() == task.authenticated)
+
+
+def can_view_all_tasks():
+  """Can view all tasks."""
+  return _is_view_all_tasks() or _is_privileged_user()
 
 
-def get_user_type():
-  """Returns a string describing the current access control for the user."""
-  if is_admin():
-    return 'admin'
-  if is_privileged_user():
-    return 'privileged user'
-  if is_user():
-    return 'user'
+### Other
 
 
 def bootstrap_dev_server_acls():