Repository: google/timesketch

Issue:
Surface user management via API client
**Is your feature request related to a problem? Please describe.**
I regularly want to create (and manage) users, via API, because I don't have direct CLI access to the server which hosts timesketch.

**Describe the solution you'd like**
I would like to have a feature, where I can create, list and modify users, when I have admin privileges within timesketch.

**Describe alternatives you've considered**
There is a feature that exists in `tsctl`, however, that limits user management to the time sketch server, prohibiting administration via API.


Discussion:
lo-chr: Giving this PR a bit more thought, I identified some security and data privacy issues. I will set this PR into draft mode again, fix the issues and submit the  PR again. Sorry for the inconvenience.
lo-chr: I changed the UserResource, so that it does not check for user privileges anymore.

Might be useful to limit details like mail address to admin users In the future.
berggren: Hi @lo-chr thanks for contributing! I'll review this on Friday, Jan 26.
lo-chr: I'll fix this over the weekend! Thank you for the review.
berggren: Thanks for contributing! 
lo-chr: Thank your for the reviews and advice. I learned one or two things. ☺️

Patch:
diff --git a/api_client/python/timesketch_api_client/client.py b/api_client/python/timesketch_api_client/client.py
index 0ffffe72..e9f13eee 100644
--- a/api_client/python/timesketch_api_client/client.py
+++ b/api_client/python/timesketch_api_client/client.py
@@ -451,6 +451,62 @@ class TimesketchApi:
         sketch_id = objects[0]["id"]
         return self.get_sketch(sketch_id)
 
+    def create_user(self, username, password):
+        """Create a new user.
+
+        Args:
+            username: Name of the user
+            password: Password of the user
+
+        Returns:
+            True if user created successfully.
+
+        Raises:
+            RuntimeError: If response does not contain an 'objects' key after
+                DEFAULT_RETRY_COUNT attempts.
+        """
+
+        retry_count = 0
+        objects = None
+        while True:
+            resource_url = "{0:s}/users/".format(self.api_root)
+            form_data = {"username": username, "password": password}
+            response = self.session.post(resource_url, json=form_data)
+            response_dict = error.get_response_json(response, logger)
+            objects = response_dict.get("objects")
+            if objects:
+                break
+            retry_count += 1
+
+            if retry_count >= self.DEFAULT_RETRY_COUNT:
+                raise RuntimeError("Unable to create a new user.")
+
+        return user.User(user_id=objects[0]["id"], api=self)
+
+    def list_users(self):
+        """Get a list of all users.
+
+        Yields:
+            User object instances.
+        """
+        response = self.fetch_resource_data("users/")
+
+        for user_dict in response.get("objects", [])[0]:
+            user_id = user_dict["id"]
+            user_obj = user.User(user_id=user_id, api=self)
+            yield user_obj
+
+    def get_user(self, user_id):
+        """Get a user.
+
+        Args:
+            user_id: Primary key ID of the user.
+
+        Returns:
+            Instance of a User object.
+        """
+        return user.User(user_id=user_id, api=self)
+
     def get_oauth_token_status(self):
         """Return a dict with OAuth token status, if one exists."""
         if not self.credentials:
diff --git a/api_client/python/timesketch_api_client/user.py b/api_client/python/timesketch_api_client/user.py
index 71a6181e..588a56d0 100644
--- a/api_client/python/timesketch_api_client/user.py
+++ b/api_client/python/timesketch_api_client/user.py
@@ -24,11 +24,16 @@ logger = logging.getLogger("timesketch_api.user")
 class User(resource.BaseResource):
     """User object."""
 
-    def __init__(self, api):
+    def __init__(self, api, user_id=None):
         """Initializes the user object."""
         self._object_data = None
-        resource_uri = "users/me/"
-        super().__init__(api, resource_uri)
+        if not user_id:
+            resource_uri = "users/me/"
+            super().__init__(api, resource_uri)
+        else:
+            self.id = user_id
+            self.api = api
+            super().__init__(api=api, resource_uri=f"users/{self.id}")
 
     def _get_data(self):
         """Returns dict from the first object of the resource data."""
diff --git a/timesketch/api/v1/resources/__init__.py b/timesketch/api/v1/resources/__init__.py
index a01161d1..2e3680b2 100644
--- a/timesketch/api/v1/resources/__init__.py
+++ b/timesketch/api/v1/resources/__init__.py
@@ -42,7 +42,10 @@ class ResourceMixin(object):
     group_fields = {"name": fields.String}
 
     user_fields = {
+        "id": fields.Integer,
         "username": fields.String,
+        "name": fields.String,
+        "email": fields.String,
         "admin": fields.Boolean,
         "active": fields.Boolean,
         "groups": fields.Nested(group_fields),
diff --git a/timesketch/api/v1/resources/user.py b/timesketch/api/v1/resources/user.py
index e825ac12..6496a4e2 100644
--- a/timesketch/api/v1/resources/user.py
+++ b/timesketch/api/v1/resources/user.py
@@ -46,6 +46,62 @@ class UserListResource(resources.ResourceMixin, Resource):
         """
         return self.to_json(User.query.all())
 
+    @login_required
+    def post(self):
+        """Handles POST request to the resource.
+
+        Returns:
+            User Object
+        """
+
+        if not current_user.admin:
+            abort(
+                HTTP_STATUS_CODE_FORBIDDEN,
+                "The user has no permissions to create other users.",
+            )
+        form = request.json
+        username = form.get("username", "")
+        password = form.get("password", "")
+
+        # Check provided username
+        if not username:
+            abort(
+                HTTP_STATUS_CODE_NOT_FOUND,
+                "No username provided, unable to create the user.",
+            )
+        if not isinstance(username, str):
+            abort(HTTP_STATUS_CODE_FORBIDDEN, "Username needs to be a string.")
+        # Check provided password
+        if not password:
+            abort(
+                HTTP_STATUS_CODE_NOT_FOUND,
+                "No password provided, unable to create the user.",
+            )
+        if not isinstance(password, str):
+            abort(HTTP_STATUS_CODE_FORBIDDEN, "Password needs to be a string.")
+
+        user = User.get_or_create(username=username, name=username)
+        user.set_password(plaintext=password)
+        # TODO: Take additional attributes of users into account
+        db_session.add(user)
+        db_session.commit()
+        return self.to_json(user)
+
+
+class UserResource(resources.ResourceMixin, Resource):
+    """Resource to get list of users."""
+
+    @login_required
+    def get(self, user_id):
+        """Handles GET request to the resource.
+
+        Returns:
+            Details of user
+        """
+
+        user = User.get_by_id(user_id)
+        return self.to_json(user)
+
 
 class GroupListResource(resources.ResourceMixin, Resource):
     """Resource to get list of groups."""
diff --git a/timesketch/api/v1/routes.py b/timesketch/api/v1/routes.py
index 14034ef0..663e4ef6 100644
--- a/timesketch/api/v1/routes.py
+++ b/timesketch/api/v1/routes.py
@@ -60,6 +60,7 @@ from .resources.searchindex import SearchIndexListResource
 from .resources.searchindex import SearchIndexResource
 from .resources.session import SessionResource
 from .resources.user import UserListResource
+from .resources.user import UserResource
 from .resources.user import GroupListResource
 from .resources.user import CollaboratorResource
 from .resources.user import LoggedInUserResource
@@ -170,6 +171,7 @@ API_ROUTES = [
     (SigmaRuleResource, "/sigmarules/<string:rule_uuid>/"),
     (SigmaRuleByTextResource, "/sigmarules/text/"),
     (LoggedInUserResource, "/users/me/"),
+    (UserResource, "/users/<int:user_id>/"),
     (GraphListResource, "/sketches/<int:sketch_id>/graphs/"),
     (GraphResource, "/sketches/<int:sketch_id>/graphs/<int:graph_id>/"),
     (GraphPluginListResource, "/graphs/"),


Test patch:
diff --git a/timesketch/api/v1/resources_test.py b/timesketch/api/v1/resources_test.py
index 56478b49..cde3fbe4 100644
--- a/timesketch/api/v1/resources_test.py
+++ b/timesketch/api/v1/resources_test.py
@@ -22,6 +22,7 @@ from timesketch.lib.definitions import HTTP_STATUS_CODE_BAD_REQUEST
 from timesketch.lib.definitions import HTTP_STATUS_CODE_CREATED
 from timesketch.lib.definitions import HTTP_STATUS_CODE_NOT_FOUND
 from timesketch.lib.definitions import HTTP_STATUS_CODE_OK
+from timesketch.lib.definitions import HTTP_STATUS_CODE_FORBIDDEN
 from timesketch.lib.testlib import BaseTest
 from timesketch.lib.testlib import MockDataStore
 
@@ -1092,3 +1093,73 @@ class ContextLinksResourceTest(BaseTest):
         self.assertIsNotNone(response)
         self.assertEqual(response.status_code, HTTP_STATUS_CODE_OK)
         self.assertDictEqual(data, expected_configuration)
+
+
+class UserListTest(BaseTest):
+    """Test UserListResource."""
+
+    def test_user_post_resource_admin(self):
+        """Authenticated request (admin user) to create another user."""
+        self.login_admin()
+
+        data = dict(username="testuser", password="testpassword")
+        response = self.client.post(
+            "/api/v1/users/",
+            data=json.dumps(data),
+            content_type="application/json",
+        )
+        self.assertIsNotNone(response)
+
+    def test_user_post_resource_without_admin(self):
+        """Authenticated request (no admin) to create another user,
+        which should not work."""
+        self.login()
+
+        data = dict(username="testuser", password="testpassword")
+        response = self.client.post(
+            "/api/v1/users/",
+            data=json.dumps(data),
+            content_type="application/json",
+        )
+        self.assertIsNotNone(response)
+        self.assertEqual(response.status_code, HTTP_STATUS_CODE_FORBIDDEN)
+
+    def test_user_post_resource_missing_username(self):
+        """Authenticated request (admin user) to create another user,
+        but with missing username, which should not work."""
+        self.login_admin()
+
+        data = dict(username="", password="testpassword")
+        response = self.client.post(
+            "/api/v1/users/",
+            data=json.dumps(data),
+            content_type="application/json",
+        )
+        self.assertIsNotNone(response)
+        self.assertEqual(response.status_code, HTTP_STATUS_CODE_NOT_FOUND)
+
+    def test_user_post_resource_missing_password(self):
+        """Authenticated request (admin user) to create another user,
+        but with missing password, which should not work."""
+        self.login_admin()
+
+        data = dict(username="testuser", password="")
+        response = self.client.post(
+            "/api/v1/users/",
+            data=json.dumps(data),
+            content_type="application/json",
+        )
+        self.assertIsNotNone(response)
+        self.assertEqual(response.status_code, HTTP_STATUS_CODE_NOT_FOUND)
+
+
+class UserTest(BaseTest):
+    """Test UserResource."""
+
+    def test_user_get_resource_admin(self):
+        """Authenticated request (admin user) to create another user."""
+        self.login_admin()
+
+        response = self.client.get("/api/v1/users/1/")
+        data = json.loads(response.get_data(as_text=True))
+        self.assertEqual(data["objects"][0]["username"], "test1")
diff --git a/timesketch/lib/testlib.py b/timesketch/lib/testlib.py
index 545e0d9c..bccc9c44 100644
--- a/timesketch/lib/testlib.py
+++ b/timesketch/lib/testlib.py
@@ -435,17 +435,20 @@ class BaseTest(TestCase):
         db_session.add(model)
         db_session.commit()
 
-    def _create_user(self, username, set_password=False):
+    def _create_user(self, username, set_password=False, set_admin=False):
         """Create a user in the database.
         Args:
             username: Username (string)
             set_password: Boolean value to decide if a password should be set
+            set_admin: Boolean value to decide if the user should be an admin
         Returns:
             A user (instance of timesketch.models.user.User)
         """
         user = User.get_or_create(username=username, name=username)
         if set_password:
             user.set_password(plaintext="test", rounds=4)
+        if set_admin:
+            user.admin = True
         self._commit_to_database(user)
         return user
 
@@ -616,6 +619,9 @@ class BaseTest(TestCase):
 
         self.user1 = self._create_user(username="test1", set_password=True)
         self.user2 = self._create_user(username="test2", set_password=False)
+        self.useradmin = self._create_user(
+            username="testadmin", set_password=True, set_admin=True
+        )
 
         self.group1 = self._create_group(name="test_group1", user=self.user1)
         self.group2 = self._create_group(name="test_group2", user=self.user1)
@@ -677,6 +683,14 @@ class BaseTest(TestCase):
             follow_redirects=True,
         )
 
+    def login_admin(self):
+        """Authenticate the test user with admin privileges."""
+        self.client.post(
+            "/login/",
+            data=dict(username="testadmin", password="test"),
+            follow_redirects=True,
+        )
+
     def test_unauthenticated(self):
         """
         Generic test for all resources. It tests that no