Simpler controller code with some refactoring

tracim/lib/core/user.py

@@ -46,7 +46,11 @@ class UserApi(object):
         """
         Get one user by user id
         """
-        return self._base_query().filter(User.user_id == user_id).one()
+        try:
+            user = self._base_query().filter(User.user_id == user_id).one()
+        except NoResultFound:
+            raise UserNotExist()
+        return user
 
     def get_one_by_email(self, email: str) -> User:
         """
@@ -54,7 +58,11 @@ class UserApi(object):
         :param email: Email of the user
         :return: one user
         """
-        return self._base_query().filter(User.email == email).one()
+        try:
+            user = self._base_query().filter(User.email == email).one()
+        except NoResultFound:
+            raise UserNotExist()
+        return user
 
     # FIXME - G.M - 24-04-2018 - Duplicate method with get_one.
     def get_one_by_id(self, id: int) -> User:
@@ -95,7 +103,7 @@ class UserApi(object):
                 return user
             else:
                 raise WrongUserPassword()
-        except (WrongUserPassword, NoResultFound):
+        except (WrongUserPassword, NoResultFound, UserNotExist):
             raise AuthenticationFailed()
 
     def can_see_private_info_of_user(self, user: User):

tracim/lib/core/userworkspace.py

@@ -120,8 +120,8 @@ class RoleApi(object):
         return self._session.query(UserRoleInWorkspace)\
             .filter(UserRoleInWorkspace.user_id == user_id)
 
-    def get_all_for_user(self, user_id):
-        return self._get_all_for_user(user_id).all()
+    def get_all_for_user(self, user: User):
+        return self._get_all_for_user(user.user_id).all()
 
     def get_all_for_user_order_by_workspace(
             self,
@@ -130,9 +130,9 @@ class RoleApi(object):
         return self._get_all_for_user(user_id)\
             .join(UserRoleInWorkspace.workspace).order_by(Workspace.label).all()
 
-    def get_all_for_workspace(self, workspace_id):
+    def get_all_for_workspace(self, workspace:Workspace):
         return self._session.query(UserRoleInWorkspace)\
-            .filter(UserRoleInWorkspace.workspace_id == workspace_id).all()
+            .filter(UserRoleInWorkspace.workspace_id == workspace.workspace_id).all()  # nopep8
 
     def save(self, role: UserRoleInWorkspace):
         self._session.flush()

tracim/lib/utils/authentification.py

@@ -4,6 +4,7 @@ from pyramid.request import Request
 from sqlalchemy.orm.exc import NoResultFound
 
 from tracim import TracimRequest
+from tracim.exceptions import UserNotExist
 from tracim.lib.core.user import UserApi
 from tracim.models import User
 
@@ -50,6 +51,6 @@ def _get_basic_auth_unsafe_user(
         if not login:
             return None
         user = uapi.get_one_by_email(login)
-    except NoResultFound:
+    except (NoResultFound, UserNotExist):
         return None
     return user

tracim/lib/utils/authorization.py

@@ -80,4 +80,4 @@ def require_workspace_role(minimal_required_role):
             raise InsufficientUserWorkspaceRole()
 
         return wrapper
-    return decorator
+    return decorator

tracim/lib/utils/request.py

@@ -74,7 +74,6 @@ class TracimRequest(Request):
             )
         self._current_user = user
 
-
 ###
 # Utils for TracimRequest
 ###
@@ -112,9 +111,10 @@ def get_workspace(
     """
     workspace_id = ''
     try:
-        if 'workspace_id' not in request.json_body:
-            raise WorkspaceNotFound('No workspace_id param in json body')
-        workspace_id = request.json_body['workspace_id']
+        if 'workspace_id' in request.matchdict:
+            workspace_id = request.matchdict['workspace_id']
+        if not workspace_id:
+            raise WorkspaceNotFound('No workspace_id param')
         wapi = WorkspaceApi(
             current_user=user,
             session=request.dbsession,

tracim/tests/functional/test_system.py

@@ -4,14 +4,13 @@ from tracim.tests import FunctionalTest
 
 class TestApplicationsEndpoint(FunctionalTest):
     def test_api__get_applications__ok_200__nominal_case(self):
-        # TODO need authorization ? check permissions ?
-        # self.testapp.authorization = (
-        #     'Basic',
-        #     (
-        #         'admin@admin.admin',
-        #         'admin@admin.admin'
-        #     )
-        # )
+        self.testapp.authorization = (
+            'Basic',
+            (
+                'admin@admin.admin',
+                'admin@admin.admin'
+            )
+        )
         res = self.testapp.get('/api/v2/system/applications', status=200)
         res = res.json_body
         application = res[0]
@@ -49,3 +48,17 @@ class TestApplicationsEndpoint(FunctionalTest):
         assert application['hexcolor'] == '#757575'
         assert application['is_active'] is True
         assert 'config' in application
+
+    def test_api__get_workspace__err_401__unregistered_user(self):
+        self.testapp.authorization = (
+            'Basic',
+            (
+                'john@doe.doe',
+                'lapin'
+            )
+        )
+        res = self.testapp.get('/api/v2/system/applications', status=401)
+        assert isinstance(res.json, dict)
+        assert 'code' in res.json.keys()
+        assert 'message' in res.json.keys()
+        assert 'details' in res.json.keys()

tracim/tests/functional/test_workspaces.py

@@ -74,20 +74,19 @@ class TestWorkspaceEndpoint(FunctionalTest):
         assert sidebar_entry['hexcolor'] == "#757575"
         assert sidebar_entry['icon'] == "calendar-alt"
 
-    # TODO - G.M - 22-05-2018 - Check if this feature is needed
-    # def test_api__get_workspace__err_403__unallowed_user(self):
-    #     self.testapp.authorization = (
-    #         'Basic',
-    #         (
-    #             'lawrence-not-real-email@fsf.local',
-    #             'foobarbaz'
-    #         )
-    #     )
-    #     res = self.testapp.get('/api/v2/workspaces/1', status=403)
-    #     assert isinstance(res.json, dict)
-    #     assert 'code' in res.json.keys()
-    #     assert 'message' in res.json.keys()
-    #     assert 'details' in res.json.keys()
+    def test_api__get_workspace__err_403__unallowed_user(self):
+        self.testapp.authorization = (
+            'Basic',
+            (
+                'lawrence-not-real-email@fsf.local',
+                'foobarbaz'
+            )
+        )
+        res = self.testapp.get('/api/v2/workspaces/1', status=403)
+        assert isinstance(res.json, dict)
+        assert 'code' in res.json.keys()
+        assert 'message' in res.json.keys()
+        assert 'details' in res.json.keys()
 
     def test_api__get_workspace__err_401__unregistered_user(self):
         self.testapp.authorization = (
@@ -103,7 +102,7 @@ class TestWorkspaceEndpoint(FunctionalTest):
         assert 'message' in res.json.keys()
         assert 'details' in res.json.keys()
 
-    def test_api__get_workspace__err_404__workspace_does_not_exist(self):
+    def test_api__get_workspace__err_403__workspace_does_not_exist(self):
         self.testapp.authorization = (
             'Basic',
             (
@@ -111,7 +110,7 @@ class TestWorkspaceEndpoint(FunctionalTest):
                 'admin@admin.admin'
             )
         )
-        res = self.testapp.get('/api/v2/workspaces/5', status=404)
+        res = self.testapp.get('/api/v2/workspaces/5', status=403)
         assert isinstance(res.json, dict)
         assert 'code' in res.json.keys()
         assert 'message' in res.json.keys()
@@ -139,21 +138,19 @@ class TestWorkspaceMembersEndpoint(FunctionalTest):
         assert user_role['user']['display_name'] == 'Global manager'
         assert user_role['user']['avatar_url'] is None  # TODO
 
-
-
-    # def test_api__get_workspace_members__err_400__unallowed_user(self):
-    #     self.testapp.authorization = (
-    #         'Basic',
-    #         (
-    #             'lawrence-not-real-email@fsf.local',
-    #             'foobarbaz'
-    #         )
-    #     )
-    #     res = self.testapp.get('/api/v2/workspaces/3/members', status=403)
-    #     assert isinstance(res.json, dict)
-    #     assert 'code' in res.json.keys()
-    #     assert 'message' in res.json.keys()
-    #     assert 'details' in res.json.keys()
+    def test_api__get_workspace_members__err_403__unallowed_user(self):
+        self.testapp.authorization = (
+            'Basic',
+            (
+                'lawrence-not-real-email@fsf.local',
+                'foobarbaz'
+            )
+        )
+        res = self.testapp.get('/api/v2/workspaces/3/members', status=403)
+        assert isinstance(res.json, dict)
+        assert 'code' in res.json.keys()
+        assert 'message' in res.json.keys()
+        assert 'details' in res.json.keys()
 
     def test_api__get_workspace_members__err_401__unregistered_user(self):
         self.testapp.authorization = (
@@ -169,7 +166,7 @@ class TestWorkspaceMembersEndpoint(FunctionalTest):
         assert 'message' in res.json.keys()
         assert 'details' in res.json.keys()
 
-    def test_api__get_workspace_members__err_404__workspace_does_not_exist(self):
+    def test_api__get_workspace_members__err_403__workspace_does_not_exist(self):
         self.testapp.authorization = (
             'Basic',
             (
@@ -177,7 +174,7 @@ class TestWorkspaceMembersEndpoint(FunctionalTest):
                 'admin@admin.admin'
             )
         )
-        res = self.testapp.get('/api/v2/workspaces/5/members', status=404)
+        res = self.testapp.get('/api/v2/workspaces/5/members', status=403)
         assert isinstance(res.json, dict)
         assert 'code' in res.json.keys()
         assert 'message' in res.json.keys()

tracim/tests/library/test_user_api.py

@@ -60,7 +60,7 @@ class TestUserApi(DefaultTest):
             session=self.session,
             config=self.config,
         )
-        with pytest.raises(NoResultFound):
+        with pytest.raises(UserNotExist):
             api.get_one_by_email('unknown')
 
     # def test_unit__get_all__ok__nominal_case(self):
@@ -156,4 +156,4 @@ class TestUserApi(DefaultTest):
             config=self.config,
         )
         with pytest.raises(AuthenticationFailed):
-            api.authenticate_user('unknown_user', 'wrong_password')
+            api.authenticate_user('admin@admin.admin', 'wrong_password')

tracim/views/core_api/system_controller.py

@@ -1,6 +1,9 @@
 # coding=utf-8
 from pyramid.config import Configurator
 
+from tracim.exceptions import NotAuthentificated, InsufficientUserProfile
+from tracim.lib.utils.authorization import require_profile
+from tracim.models import Group
 from tracim.models.applications import applications
 
 try:  # Python 3.5+
@@ -17,6 +20,9 @@ from tracim.views.core_api.schemas import ApplicationSchema
 class SystemController(Controller):
 
     @hapic.with_api_doc()
+    @hapic.handle_exception(NotAuthentificated, HTTPStatus.UNAUTHORIZED)
+    @hapic.handle_exception(InsufficientUserProfile, HTTPStatus.FORBIDDEN)
+    @require_profile(Group.TIM_USER)
     @hapic.output_body(ApplicationSchema(many=True),)
     def applications(self, context, request: TracimRequest, hapic_data=None):
         """

tracim/views/core_api/user_controller.py

@@ -30,27 +30,23 @@ class UserController(Controller):
         """
         Get list of user workspaces
         """
-        uid = hapic_data.path['user_id']
         app_config = request.registry.settings['CFG']
+
+        uid = hapic_data.path['user_id']
         uapi = UserApi(
             request.current_user,
             session=request.dbsession,
             config=app_config,
         )
+        user = uapi.get_one(uid)
+        if not uapi.can_see_private_info_of_user(user):
+            raise InsufficientUserProfile()
+
         wapi = WorkspaceApi(
             current_user=request.current_user,  # User
             session=request.dbsession,
             config=app_config,
         )
-        # TODO - G.M - 22-05-2018 - Refactor this in a more lib way( avoid
-        # try/catch and complex code here).
-        try:
-            user = uapi.get_one(uid)
-        except NoResultFound:
-            raise UserNotExist()
-        if not uapi.can_see_private_info_of_user(user):
-            raise InsufficientUserProfile()
-
         return [
             WorkspaceInContext(workspace, request.dbsession, app_config)
             for workspace in wapi.get_all_for_user(user)

tracim/views/core_api/workspace_controller.py

@@ -4,8 +4,10 @@ from pyramid.config import Configurator
 from sqlalchemy.orm.exc import NoResultFound
 
 from tracim.lib.core.userworkspace import RoleApi
+from tracim.lib.utils.authorization import require_workspace_role
 from tracim.models.context_models import WorkspaceInContext, \
     UserRoleWorkspaceInContext
+from tracim.models.data import UserRoleInWorkspace
 
 try:  # Python 3.5+
     from http import HTTPStatus
@@ -25,10 +27,11 @@ from tracim.views.core_api.schemas import WorkspaceSchema, UserSchema, \
 class WorkspaceController(Controller):
 
     @hapic.with_api_doc()
-    @hapic.input_path(WorkspaceIdPathSchema())
     @hapic.handle_exception(NotAuthentificated, HTTPStatus.UNAUTHORIZED)
-    #@hapic.handle_exception(InsufficientUserProfile, HTTPStatus.FORBIDDEN)
-    @hapic.handle_exception(WorkspaceNotFound, HTTPStatus.NOT_FOUND)
+    @hapic.handle_exception(InsufficientUserProfile, HTTPStatus.FORBIDDEN)
+    @hapic.handle_exception(WorkspaceNotFound, HTTPStatus.FORBIDDEN)
+    @require_workspace_role(UserRoleInWorkspace.READER)
+    @hapic.input_path(WorkspaceIdPathSchema())
     @hapic.output_body(WorkspaceSchema())
     def workspace(self, context, request: TracimRequest, hapic_data=None):
         """
@@ -41,19 +44,14 @@ class WorkspaceController(Controller):
             session=request.dbsession,
             config=app_config,
         )
-        # TODO - G.M - 22-05-2018 - Refactor this in a more lib way( avoid
-        # try/catch and complex code here).
-        try:
-            workspace = wapi.get_one(wid)
-        except NoResultFound:
-            raise WorkspaceNotFound()
-        return wapi.get_workspace_with_context(workspace)
+        return wapi.get_workspace_with_context(request.current_workspace)
 
     @hapic.with_api_doc()
-    @hapic.input_path(WorkspaceIdPathSchema())
     @hapic.handle_exception(NotAuthentificated, HTTPStatus.UNAUTHORIZED)
-    #@hapic.handle_exception(InsufficientUserProfile, HTTPStatus.FORBIDDEN)
-    @hapic.handle_exception(WorkspaceNotFound, HTTPStatus.NOT_FOUND)
+    @hapic.handle_exception(InsufficientUserProfile, HTTPStatus.FORBIDDEN)
+    @hapic.handle_exception(WorkspaceNotFound, HTTPStatus.FORBIDDEN)
+    @require_workspace_role(UserRoleInWorkspace.READER)
+    @hapic.input_path(WorkspaceIdPathSchema())
     @hapic.output_body(WorkspaceMemberSchema(many=True))
     def workspaces_members(
             self,
@@ -64,25 +62,15 @@ class WorkspaceController(Controller):
         """
         Get Members of this workspace
         """
-        wid = hapic_data.path['workspace_id']
         app_config = request.registry.settings['CFG']
         rapi = RoleApi(
             current_user=request.current_user,
             session=request.dbsession,
             config=app_config,
         )
-        wapi = WorkspaceApi(
-            current_user=request.current_user,
-            session=request.dbsession,
-            config=app_config,
-        )
-        try:
-            wapi.get_one(wid)
-        except NoResultFound:
-            raise WorkspaceNotFound()
         return [
             rapi.get_user_role_workspace_with_context(user_role)
-            for user_role in rapi.get_all_for_workspace(wid)
+            for user_role in rapi.get_all_for_workspace(request.current_workspace)
         ]
 
     def bind(self, configurator: Configurator) -> None: