From ff482585add6591d58dee78b77fc50383f194452 Mon Sep 17 00:00:00 2001 From: Jongmin Kim Date: Thu, 17 Mar 2022 22:11:04 +0900 Subject: [PATCH] refactor: change UserConfig model and authorization policy Signed-off-by: Jongmin Kim --- .../config/manager/user_config_manager.py | 10 +++--- .../config/model/user_config_model.py | 4 +-- .../config/service/domain_config_service.py | 2 +- .../config/service/user_config_service.py | 34 +++++++++++++------ test/factory/user_config_factory.py | 1 + test/service/test_user_config_service.py | 22 +++++++++--- 6 files changed, 50 insertions(+), 23 deletions(-) diff --git a/src/spaceone/config/manager/user_config_manager.py b/src/spaceone/config/manager/user_config_manager.py index 84dbe8f..ed612ee 100644 --- a/src/spaceone/config/manager/user_config_manager.py +++ b/src/spaceone/config/manager/user_config_manager.py @@ -24,7 +24,7 @@ def _rollback(user_config_vo): return user_config_vo def update_user_config(self, params): - user_config_vo: UserConfig = self.get_user_config(params['name'], params['domain_id']) + user_config_vo: UserConfig = self.get_user_config(params['name'], params['user_id'], params['domain_id']) return self.update_user_config_by_vo(params, user_config_vo) def update_user_config_by_vo(self, params, user_config_vo): @@ -36,12 +36,12 @@ def _rollback(old_data): return user_config_vo.update(params) - def delete_user_config(self, name, domain_id): - user_config_vo: UserConfig = self.get_user_config(name, domain_id) + def delete_user_config(self, name, user_id, domain_id): + user_config_vo: UserConfig = self.get_user_config(name, user_id, domain_id) user_config_vo.delete() - def get_user_config(self, name, domain_id, only=None): - return self.user_config_model.get(name=name, domain_id=domain_id, only=only) + def get_user_config(self, name, user_id, domain_id, only=None): + return self.user_config_model.get(name=name, user_id=user_id, domain_id=domain_id, only=only) def filter_user_configs(self, **conditions): return self.user_config_model.filter(**conditions) diff --git a/src/spaceone/config/model/user_config_model.py b/src/spaceone/config/model/user_config_model.py index 8d29202..795cf05 100644 --- a/src/spaceone/config/model/user_config_model.py +++ b/src/spaceone/config/model/user_config_model.py @@ -9,10 +9,10 @@ class UserConfigTag(EmbeddedDocument): class UserConfig(MongoModel): - name = StringField(max_length=255, unique_with='domain_id') + name = StringField(max_length=255, unique_with=['user_id', 'domain_id']) data = DictField() tags = ListField(EmbeddedDocumentField(UserConfigTag)) - user_id = StringField(max_length=40, default=None, null=True) + user_id = StringField(max_length=40) domain_id = StringField(max_length=40) created_at = DateTimeField(auto_now_add=True) updated_at = DateTimeField(auto_now=True) diff --git a/src/spaceone/config/service/domain_config_service.py b/src/spaceone/config/service/domain_config_service.py index 96fa004..f23939b 100644 --- a/src/spaceone/config/service/domain_config_service.py +++ b/src/spaceone/config/service/domain_config_service.py @@ -83,7 +83,7 @@ def set(self, params): if 'tags' in params: params['tags'] = utils.dict_to_tags(params['tags']) - domain_config_vos = self.domain_config_mgr.filter_domain_configs(domain_id=domain_id) + domain_config_vos = self.domain_config_mgr.filter_domain_configs(domain_id=domain_id, name=params['name']) if domain_config_vos.count() == 0: return self.domain_config_mgr.create_domain_config(params) diff --git a/src/spaceone/config/service/user_config_service.py b/src/spaceone/config/service/user_config_service.py index fd66301..320c27a 100644 --- a/src/spaceone/config/service/user_config_service.py +++ b/src/spaceone/config/service/user_config_service.py @@ -35,9 +35,7 @@ def create(self, params): user_config_vo (object) """ - user_type = self.transaction.get_meta('authorization.user_type') - if user_type == 'DOMAIN_OWNER': - raise ERROR_PERMISSION_DENIED() + self._check_permission(params['domain_id']) params['user_id'] = self.transaction.get_meta('user_id') @@ -63,6 +61,10 @@ def update(self, params): user_config_vo (object) """ + self._check_permission(params['domain_id']) + + params['user_id'] = self.transaction.get_meta('user_id') + if 'tags' in params: params['tags'] = utils.dict_to_tags(params['tags']) @@ -85,19 +87,19 @@ def set(self, params): user_config_vo (object) """ - domain_id = params['domain_id'] + self._check_permission(params['domain_id']) + + params['user_id'] = self.transaction.get_meta('user_id') user_type = self.transaction.get_meta('authorization.user_type') if user_type == 'DOMAIN_OWNER': raise ERROR_PERMISSION_DENIED() - params['user_id'] = self.transaction.get_meta('user_id') - user_id = params['user_id'] - if 'tags' in params: params['tags'] = utils.dict_to_tags(params['tags']) - user_config_vos = self.user_config_mgr.filter_user_configs(domain_id=domain_id, user_id=user_id) + user_config_vos = self.user_config_mgr.filter_user_configs(domain_id=params['domain_id'], + user_id=params['user_id'], name=params['name']) if user_config_vos.count() == 0: return self.user_config_mgr.create_user_config(params) @@ -119,7 +121,10 @@ def delete(self, params): None """ - self.user_config_mgr.delete_user_config(params['name'], params['domain_id']) + self._check_permission(params['domain_id']) + user_id = self.transaction.get_meta('user_id') + + self.user_config_mgr.delete_user_config(params['name'], user_id, params['domain_id']) @transaction(append_meta={'authorization.scope': 'USER'}) @check_required(['name', 'domain_id']) @@ -137,7 +142,9 @@ def get(self, params): user_config_vo (object) """ - return self.user_config_mgr.get_user_config(params['name'], params['domain_id'], params.get('only')) + user_id = self.transaction.get_meta('user_id') + + return self.user_config_mgr.get_user_config(params['name'], user_id, params['domain_id'], params.get('only')) @transaction(append_meta={ 'authorization.scope': 'USER', @@ -190,3 +197,10 @@ def stat(self, params): query = params.get('query', {}) return self.user_config_mgr.state_user_configs(query) + + def _check_permission(self, request_domain_id): + user_type = self.transaction.get_meta('authorization.user_type') + user_domain_id = self.transaction.get_meta('domain_id') + + if user_type == 'DOMAIN_OWNER' or request_domain_id != user_domain_id: + raise ERROR_PERMISSION_DENIED() diff --git a/test/factory/user_config_factory.py b/test/factory/user_config_factory.py index 8de0bf1..346377c 100644 --- a/test/factory/user_config_factory.py +++ b/test/factory/user_config_factory.py @@ -23,5 +23,6 @@ class Meta: } ] domain_id = utils.generate_id('domain') + user_id = utils.generate_id('user') updated_at = factory.Faker('date_time') created_at = factory.Faker('date_time') diff --git a/test/service/test_user_config_service.py b/test/service/test_user_config_service.py index a7e2409..f03425c 100644 --- a/test/service/test_user_config_service.py +++ b/test/service/test_user_config_service.py @@ -23,6 +23,7 @@ def setUpClass(cls): connect('test', host='mongomock://localhost') cls.domain_id = utils.generate_id('domain') + cls.user_id = utils.generate_id('user') cls.transaction = Transaction({ 'service': 'config', 'api_class': 'UserConfig' @@ -51,10 +52,12 @@ def test_create_user_config(self, *args): 'tags': { utils.random_string(): utils.random_string() }, - 'domain_id': utils.generate_id('domain') + 'domain_id': self.domain_id } self.transaction.method = 'create' + self.transaction.set_meta('user_id', self.user_id) + self.transaction.set_meta('domain_id', self.domain_id) user_config_svc = UserConfigService(transaction=self.transaction) user_config_vo = user_config_svc.create(params.copy()) @@ -69,7 +72,7 @@ def test_create_user_config(self, *args): @patch.object(MongoModel, 'connect', return_value=None) def test_update_user_config(self, *args): - new_user_config_vo = UserConfigFactory(domain_id=self.domain_id) + new_user_config_vo = UserConfigFactory(domain_id=self.domain_id, user_id=self.user_id) params = { 'name': new_user_config_vo.name, @@ -83,6 +86,8 @@ def test_update_user_config(self, *args): } self.transaction.method = 'update' + self.transaction.set_meta('user_id', self.user_id) + self.transaction.set_meta('domain_id', self.domain_id) user_config_svc = UserConfigService(transaction=self.transaction) user_config_vo = user_config_svc.update(params.copy()) @@ -104,10 +109,12 @@ def test_set_user_config(self, *args): 'tags': { utils.random_string(): utils.random_string() }, - 'domain_id': utils.generate_id('domain') + 'domain_id': self.domain_id } self.transaction.method = 'set' + self.transaction.set_meta('user_id', self.user_id) + self.transaction.set_meta('domain_id', self.domain_id) user_config_svc = UserConfigService(transaction=self.transaction) user_config_vo = user_config_svc.create(params.copy()) @@ -122,7 +129,7 @@ def test_set_user_config(self, *args): @patch.object(MongoModel, 'connect', return_value=None) def test_delete_user_config(self, *args): - new_user_config_vo = UserConfigFactory(domain_id=self.domain_id) + new_user_config_vo = UserConfigFactory(domain_id=self.domain_id, user_id=self.user_id) params = { 'name': new_user_config_vo.name, @@ -130,6 +137,8 @@ def test_delete_user_config(self, *args): } self.transaction.method = 'delete' + self.transaction.set_meta('user_id', self.user_id) + self.transaction.set_meta('domain_id', self.domain_id) user_config_svc = UserConfigService(transaction=self.transaction) result = user_config_svc.delete(params.copy()) @@ -137,7 +146,7 @@ def test_delete_user_config(self, *args): @patch.object(MongoModel, 'connect', return_value=None) def test_get_user_config(self, *args): - new_user_config_vo = UserConfigFactory(domain_id=self.domain_id) + new_user_config_vo = UserConfigFactory(domain_id=self.domain_id, user_id=self.user_id) params = { 'name': new_user_config_vo.name, @@ -145,6 +154,8 @@ def test_get_user_config(self, *args): } self.transaction.method = 'get' + self.transaction.set_meta('user_id', self.user_id) + self.transaction.set_meta('domain_id', self.domain_id) user_config_svc = UserConfigService(transaction=self.transaction) user_config_vo = user_config_svc.get(params.copy()) @@ -164,6 +175,7 @@ def test_list_user_configs_by_name(self, *args): } self.transaction.method = 'list' + self.transaction.set_meta('user_id', 'test_user') user_config_svc = UserConfigService(transaction=self.transaction) user_config_vos, total_count = user_config_svc.list(params.copy()) UserConfigsInfo(user_config_vos, total_count)