fix: service accounts are managed at /service-accounts, not /users, and the internal one of the metrics service is only listed.

debian-package/src/usr/lib/vpn-router/web

@@ -12,7 +12,11 @@ Pages:
            runs, as JSON
   /users   the users in the router's directory, whether or not a P2S system is enabled: list,
            add, change, set the password, disable, enable, remove, the OpenVPN client
-           profile, and TOTP enrolment (see users())
+           profile, and TOTP enrolment (see users()); service accounts are not among them
+  /service-accounts
+           the service accounts: list, add, change the e-mail address, set the password,
+           disable, enable, unlock and remove; the internal one is only listed (see
+           service_accounts())
   /groups  the groups of the router's directory: list, add, rename or describe, remove, and
            add and take out members (see groups())
   /account the account of the signed-in user: its details, its password and its TOTP
@@ -28,7 +32,7 @@ Pages:
            the databases of each PostgreSQL cluster on the router with their sizes, as JSON (see
            postgresql_databases()); /config has the clusters' settings
 
-/config, /metrics/history, /postgresql, /users, /groups, /site-to-site and /account need an HS256 JWT in an Authorization: Bearer header signed
+/config, /metrics/history, /postgresql, /users, /service-accounts, /groups, /site-to-site and /account need an HS256 JWT in an Authorization: Bearer header signed
 with vpnrouter_auth.WEB_KEY (see vpnrouter_auth.verify). A token of /auth names a user in sub;
 /account serves that user, and the other paths need a member of vpnrouter_users.ADMIN_GROUP. A
 token of vpn-router-token has the admin claim and names no user; it may call every path but
@@ -441,11 +445,9 @@ def users(state, method, path, body):
 
       GET    /users                   the users
       POST   /users                   add a user: {"name", "password", "first_name", "last_name", "email"};
-                                      without the names a Manager-only account, and with "service": true a
-                                      service account
+                                      without the names a Manager-only account
       PUT    /users/<name>            change any of {"first_name", "last_name", "email"}
-      PUT    /users/<name>/password   set the password: {"password": ...}; refused for the internal service
-                                      account, whose password vpn-router-setup keeps
+      PUT    /users/<name>/password   set the password: {"password": ...}
       POST   /users/<name>/disable    lock the user out of OpenVPN
       POST   /users/<name>/enable     let the user in again
       POST   /users/<name>/unlock     end a lockout after failed logins
@@ -454,6 +456,8 @@ def users(state, method, path, body):
       POST   /users/<name>/otp        a new TOTP secret and its otpauth URI; nothing is stored
       PUT    /users/<name>/otp        store a TOTP secret: {"secret", "code"}, code a code of the secret
       DELETE /users/<name>/otp        remove the TOTP token
+
+    Service accounts are served by /service-accounts: they are not listed, and a request naming one is 404.
     """
     if body == 'invalid' or (body is not None and not isinstance(body, dict)):
         return 400, {'error': 'the body must be a JSON object'}
@@ -462,9 +466,9 @@ def users(state, method, path, body):
     route = (method, len(parts), parts[2] if len(parts) == 3 else None)
     name = parts[1] if len(parts) > 1 else None
     actions = {
-        ('GET', 1, None): lambda connection: vpnrouter_users.list_users(connection),
-        ('POST', 1, None): lambda connection: vpnrouter_users.add(connection, body.get('name'), body.get('password'), body,
-                                                                  service=body.get('service') is True),
+        ('GET', 1, None): lambda connection: {
+            'users': [user for user in vpnrouter_users.list_users(connection)['users'] if not user['service']]},
+        ('POST', 1, None): lambda connection: vpnrouter_users.add(connection, body.get('name'), body.get('password'), body),
         ('PUT', 2, None): lambda connection: vpnrouter_users.update(connection, name, body),
         ('PUT', 3, 'password'): lambda connection: vpnrouter_users.passwd(connection, name, body.get('password')),
         ('POST', 3, 'disable'): lambda connection: vpnrouter_users.disable(connection, name),
@@ -481,12 +485,10 @@ def users(state, method, path, body):
                   ('DELETE', 2, None): 'remove'}
     if route not in actions:
         return 404, {'error': f'no route {method} {path}'}
-    if route == ('POST', 1, None) and not isinstance(body.get('service', False), bool):
-        return 400, {'error': 'service must be true or false'}
-    if route == ('PUT', 3, 'password') and name == vpnrouter_users.metrics_account():
-        return 400, {'error': f'{name} is the internal service account of the metrics service; vpn-router-setup keeps its password'}
     try:
         with vpnrouter_users.session() as connection:
+            if name is not None and vpnrouter_users.service(vpnrouter_users.entry(connection, name)):
+                raise vpnrouter_users.Failure(f'{name} is a service account; see /service-accounts', 3)
             document = actions[route](connection)
         if operations.get(route) in vpnrouter_auth.REVOKING:
             vpnrouter_auth.revoke(vpnrouter.load_config(), name)
@@ -502,6 +504,56 @@ def users(state, method, path, body):
     return (201 if route == ('POST', 1, None) else 200), document
 
 
+def service_accounts(method, path, body):
+    """Answer a /service-accounts request with vpnrouter_users: the HTTP status and the JSON document.
+
+      GET    /service-accounts                   the service accounts: {"service_accounts": [...]}
+      POST   /service-accounts                   add one: {"name", "password", "email"?}
+      PUT    /service-accounts/<name>            change the e-mail address: {"email"}; "" removes it
+      PUT    /service-accounts/<name>/password   set the password: {"password": ...}
+      POST   /service-accounts/<name>/disable    lock it out
+      POST   /service-accounts/<name>/enable     let it in again
+      POST   /service-accounts/<name>/unlock     end a lockout after failed logins
+      DELETE /service-accounts/<name>            remove it
+
+    A name that is not a service account is 404. The internal one (vpnrouter_users.metrics_account), which
+    vpn-router-setup adds and keeps, is only listed: every other request on it is refused with 403.
+    """
+    if body == 'invalid' or (body is not None and not isinstance(body, dict)):
+        return 400, {'error': 'the body must be a JSON object'}
+    body = body or {}
+    parts = path.strip('/').split('/')
+    route = (method, len(parts), parts[2] if len(parts) == 3 else None)
+    name = parts[1] if len(parts) > 1 else None
+    email = {'email': body.get('email')}
+    actions = {
+        ('GET', 1, None): lambda connection: {
+            'service_accounts': [user for user in vpnrouter_users.list_users(connection)['users'] if user['service']]},
+        ('POST', 1, None): lambda connection: vpnrouter_users.add(connection, body.get('name'), body.get('password'), email,
+                                                                  service=True),
+        ('PUT', 2, None): lambda connection: vpnrouter_users.update(connection, name, email),
+        ('PUT', 3, 'password'): lambda connection: vpnrouter_users.passwd(connection, name, body.get('password')),
+        ('POST', 3, 'disable'): lambda connection: vpnrouter_users.disable(connection, name),
+        ('POST', 3, 'enable'): lambda connection: vpnrouter_users.enable(connection, name),
+        ('POST', 3, 'unlock'): lambda connection: vpnrouter_users.unlock(connection, name),
+        ('DELETE', 2, None): lambda connection: vpnrouter_users.remove(connection, name),
+    }
+    if route not in actions:
+        return 404, {'error': f'no route {method} {path}'}
+    if name is not None and name == vpnrouter_users.metrics_account():
+        return 403, {'error': f'{name} is the internal service account of the metrics service; only vpn-router-setup changes it'}
+    try:
+        with vpnrouter_users.session() as connection:
+            if name is not None and not vpnrouter_users.service(vpnrouter_users.entry(connection, name)):
+                raise vpnrouter_users.Failure(f'{name} is not a service account', 3)
+            document = actions[route](connection)
+    except vpnrouter_users.Failure as failure:
+        return USER_STATUS.get(failure.status, 500), {'error': str(failure)}
+    except (ldap.LDAPError, OSError) as exc:
+        return 500, {'error': f'the user directory failed: {exc}'}
+    return (201 if route == ('POST', 1, None) else 200), document
+
+
 def groups(state, method, path, body):
     """Answer a /groups request with vpnrouter_users: the HTTP status and the JSON document.
 
@@ -511,6 +563,9 @@ def groups(state, method, path, body):
       DELETE /groups/<name>                   remove the group; its members are kept
       POST   /groups/<name>/members           add a member: {"user": ...}
       DELETE /groups/<name>/members/<user>    take a member out; the last member cannot leave
+
+    The internal service account (vpnrouter_users.metrics_account) is not put in or taken out of a group: such a
+    request is refused with 403.
     """
     if body == 'invalid' or (body is not None and not isinstance(body, dict)):
         return 400, {'error': 'the body must be a JSON object'}
@@ -518,6 +573,11 @@ def groups(state, method, path, body):
     parts = path.strip('/').split('/')
     name = parts[1] if len(parts) > 1 else None
     route = (method, len(parts), parts[2] if len(parts) > 2 else None)
+    internal = vpnrouter_users.metrics_account()
+    members = body.get('members') if route == ('POST', 1, None) else [body.get('user')] if route == ('POST', 3, 'members') \
+        else [parts[3]] if route == ('DELETE', 4, 'members') else []
+    if internal is not None and isinstance(members, list) and internal in members:
+        return 403, {'error': f'{internal} is the internal service account of the metrics service; only vpn-router-setup changes it'}
     actions = {
         ('GET', 1, None): lambda connection: vpnrouter_users.list_groups(connection),
         ('POST', 1, None): lambda connection: vpnrouter_users.group_add(
@@ -680,7 +740,8 @@ def metrics_history(view):
 
 
 def changes_admins(connection, method, parts, body):
-    """Whether a /users or /groups request creates, renames to, changes or removes ADMIN_GROUP, or removes a member of it."""
+    """Whether a /users, /service-accounts or /groups request creates, renames to, changes or removes ADMIN_GROUP, or
+    removes a member of it."""
     admins = vpnrouter_users.ADMIN_GROUP
     if parts[0] == 'groups':
         if method == 'POST' and len(parts) == 1:
@@ -688,7 +749,7 @@ def changes_admins(connection, method, parts, body):
         if method != 'GET' and len(parts) > 1 and parts[1] == admins:
             return True
         return method == 'PUT' and len(parts) == 2 and body.get('name') == admins
-    if parts[0] == 'users' and method == 'DELETE' and len(parts) == 2:
+    if parts[0] in ('users', 'service-accounts') and method == 'DELETE' and len(parts) == 2:
         return admins in vpnrouter_users.groups_of(connection, parts[1])
     return False
 
@@ -802,7 +863,7 @@ class Handler(BaseHTTPRequestHandler):
         # A body that is not read must not be taken for the next request.
         if self.command not in ('GET', 'HEAD'):
             self.close_connection = True
-        directory = next((name for name in ('users', 'groups', 'account', 'site-to-site')
+        directory = next((name for name in ('users', 'service-accounts', 'groups', 'account', 'site-to-site')
                           if path == f'/{name}' or path.startswith(f'/{name}/')), None)
         protected = path in ('/config', '/metrics/history', '/postgresql') or directory is not None
         signing = path in ('/auth', '/auth/refresh', '/auth/logout')
@@ -843,6 +904,9 @@ class Handler(BaseHTTPRequestHandler):
         elif directory == 'site-to-site':
             code, document = site_to_site(self.command, path, payload)
             body, kind = json.dumps(document, indent=2).encode() + b'\n', 'application/json'
+        elif directory == 'service-accounts':
+            code, document = service_accounts(self.command, path, payload)
+            body, kind = json.dumps(document, indent=2).encode() + b'\n', 'application/json'
         elif directory is not None:
             code, document = (users if directory == 'users' else groups)(state, self.command, path, payload)
             body, kind = json.dumps(document, indent=2).encode() + b'\n', 'application/json'