From c4659049aef1194b3b6c4f30dd202aab081278ee Mon Sep 17 00:00:00 2001 From: wallrony Date: Thu, 6 Oct 2022 15:46:38 -0300 Subject: [PATCH 1/3] Add duration limit flag (#283) Co-authored-by: vassalo --- e2e/slack/test_accessbot_slack_access.py | 51 +++++++++++++++++++ e2e/test_common.py | 1 + plugins/sdm/config_template.py | 1 + .../sdm/lib/helper/resource_grant_helper.py | 23 +++++++-- 4 files changed, 72 insertions(+), 4 deletions(-) diff --git a/e2e/slack/test_accessbot_slack_access.py b/e2e/slack/test_accessbot_slack_access.py index 4783152..e2185a6 100644 --- a/e2e/slack/test_accessbot_slack_access.py +++ b/e2e/slack/test_accessbot_slack_access.py @@ -194,6 +194,57 @@ def test_access_command_dont_grant_access_when_duration_flag_has_duration_equals mocked_testbot.push_message(f"access to Xxx --duration {duration}") assert "You need to enter a duration greater than zero" in mocked_testbot.pop_message() + def test_access_command_fails_with_duration_flag_value_and_invalid_duration_limit(self, mocked_testbot): + duration = '45m' + duration_limit = '40x' + accessbot = mocked_testbot.bot.plugin_manager.plugins['AccessBot'] + accessbot.config['DURATION_FLAG_LIMIT'] = duration_limit + mocked_testbot.push_message(f"access to Xxx --duration {duration}") + assert "invalid duration limit was defined" in mocked_testbot.pop_message() + + def test_access_command_with_duration_flag_value_lesser_than_duration_limit(self, mocked_testbot): + duration = '40m' + duration_limit = '45m' + accessbot = mocked_testbot.bot.plugin_manager.plugins['AccessBot'] + accessbot.config['DURATION_FLAG_LIMIT'] = duration_limit + mocked_testbot.push_message(f"access to Xxx --duration {duration}") + assert "valid request" in mocked_testbot.pop_message() + assert "access request" in mocked_testbot.pop_message() + + def test_access_command_with_duration_flag_value_equals_to_duration_limit(self, mocked_testbot): + duration = '40m' + duration_limit = '40m' + accessbot = mocked_testbot.bot.plugin_manager.plugins['AccessBot'] + accessbot.config['DURATION_FLAG_LIMIT'] = duration_limit + mocked_testbot.push_message(f"access to Xxx --duration {duration}") + assert "valid request" in mocked_testbot.pop_message() + assert "access request" in mocked_testbot.pop_message() + + def test_access_command_with_duration_flag_time_unit_different_than_duration_limit_unit(self, mocked_testbot): + duration = '40m' + duration_limit = '1h' + accessbot = mocked_testbot.bot.plugin_manager.plugins['AccessBot'] + accessbot.config['DURATION_FLAG_LIMIT'] = duration_limit + mocked_testbot.push_message(f"access to Xxx --duration {duration}") + assert "valid request" in mocked_testbot.pop_message() + assert "access request" in mocked_testbot.pop_message() + + def test_access_command_fails_with_duration_flag_value_greater_than_duration_limit(self, mocked_testbot): + duration = '45m' + duration_limit = '40m' + accessbot = mocked_testbot.bot.plugin_manager.plugins['AccessBot'] + accessbot.config['DURATION_FLAG_LIMIT'] = duration_limit + mocked_testbot.push_message(f"access to Xxx --duration {duration}") + assert "need to enter a duration lesser or equals" in mocked_testbot.pop_message() + + def test_access_command_fails_with_duration_flag_time_unit_different_than_duration_limit_unit(self, mocked_testbot): + duration = '65m' + duration_limit = '1h' + accessbot = mocked_testbot.bot.plugin_manager.plugins['AccessBot'] + accessbot.config['DURATION_FLAG_LIMIT'] = duration_limit + mocked_testbot.push_message(f"access to Xxx --duration {duration}") + assert "need to enter a duration lesser or equals" in mocked_testbot.pop_message() + def test_access_command_fails_for_unreachable_admin_users(self, mocked_testbot_with_no_admin_users): mocked_testbot_with_no_admin_users.push_message("access to Xxx") assert "no active Slack Admin" in mocked_testbot_with_no_admin_users.pop_message() diff --git a/e2e/test_common.py b/e2e/test_common.py index c50a7be..7d2bf82 100644 --- a/e2e/test_common.py +++ b/e2e/test_common.py @@ -82,6 +82,7 @@ def create_config(): 'APPROVERS_CHANNEL_TAG': None, 'ALLOW_RESOURCE_ACCESS_REQUEST_RENEWAL': False, 'ENABLE_BOT_STATE_HANDLING': False, + 'DURATION_FLAG_LIMIT': None, } diff --git a/plugins/sdm/config_template.py b/plugins/sdm/config_template.py index ac78643..5aad3ac 100644 --- a/plugins/sdm/config_template.py +++ b/plugins/sdm/config_template.py @@ -34,6 +34,7 @@ 'APPROVERS_CHANNEL_TAG': os.getenv("SDM_APPROVERS_CHANNEL_TAG"), 'ALLOW_RESOURCE_ACCESS_REQUEST_RENEWAL': str(os.getenv("SDM_ALLOW_RESOURCE_ACCESS_REQUEST_RENEWAL", "")).lower() == 'true', 'ENABLE_BOT_STATE_HANDLING': str(os.getenv("SDM_ENABLE_BOT_STATE_HANDLING", "")).lower() == 'true', + 'DURATION_FLAG_LIMIT': os.getenv('SDM_DURATION_FLAG_LIMIT'), } def get(): diff --git a/plugins/sdm/lib/helper/resource_grant_helper.py b/plugins/sdm/lib/helper/resource_grant_helper.py index f089855..14aebb2 100644 --- a/plugins/sdm/lib/helper/resource_grant_helper.py +++ b/plugins/sdm/lib/helper/resource_grant_helper.py @@ -2,7 +2,7 @@ from grant_request_type import GrantRequestType from .base_grant_helper import BaseGrantHelper from ..exceptions import PermissionDeniedException -from ..util import VALID_TIME_UNITS +from ..util import VALID_TIME_UNITS, convert_duration_flag_to_timedelta class ResourceGrantHelper(BaseGrantHelper): @@ -55,13 +55,28 @@ def duration_flag_validator(self, value: str): match = re.match(r'^\d+[a-zA-Z]?$', value) if not match: raise Exception('You need to enter a valid duration, e.g. 60m, 2h, etc.') - time_unit_match = re.search(r'[a-zA-Z]', value) - short_time_unit = time_unit_match.group() if time_unit_match else 'm' - if not VALID_TIME_UNITS.get(short_time_unit): + short_time_unit = self.get_short_time_unit_from_duration(value) + if short_time_unit is None: formatted_valid_time_units = ', '.join(VALID_TIME_UNITS.keys()) raise Exception(f'You need to enter a valid duration unit. Valid units are: {formatted_valid_time_units}.') duration = int(re.search(r'\d+', value).group()) if duration == 0: raise Exception('You need to enter a duration greater than zero.') + duration_limit_var = self.__bot.config['DURATION_FLAG_LIMIT'] + if duration_limit_var is not None: + short_limit_time_unit = self.get_short_time_unit_from_duration(duration_limit_var) + if short_limit_time_unit is None: + self.__bot.log.error(f'Invalid duration limit: unparseable time unit from "{duration_limit_var}"') + raise Exception("An invalid duration limit was defined.") + duration_timedelta = convert_duration_flag_to_timedelta(value) + duration_limit_timedelta = convert_duration_flag_to_timedelta(duration_limit_var) + if duration_timedelta > duration_limit_timedelta: + raise Exception(f"You need to enter a duration lesser or equals to {duration_limit_var}") return True + def get_short_time_unit_from_duration(self, duration): + time_unit_match = re.search(r'[a-zA-Z]', duration) + short_time_unit = time_unit_match.group() if time_unit_match else 'm' + if not VALID_TIME_UNITS.get(short_time_unit): + return None + return short_time_unit From 363487cd9efc14ff723b0d9ed76434eb9b88f85b Mon Sep 17 00:00:00 2001 From: wallrony Date: Thu, 13 Oct 2022 15:05:51 -0300 Subject: [PATCH 2/3] Add multiarch build for arm image (#298) Co-authored-by: vassalo --- .github/workflows/main-release.yml | 13 +++++++------ Dockerfile | 3 +-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/main-release.yml b/.github/workflows/main-release.yml index f6b7a0a..fb61b7c 100644 --- a/.github/workflows/main-release.yml +++ b/.github/workflows/main-release.yml @@ -49,11 +49,12 @@ jobs: git submodule update # Adding version echo __version__=\"$IMAGE_TAG\" > plugins/sdm/_version.py - # Building docker image - docker build -t $ECR_REGISTRY/$ECR_REPOSITORY:$IMAGE_TAG . - # Pushing image to ECR + # Authenticating to ECR aws ecr-public get-login-password --region us-east-1 | docker login --username AWS --password-stdin public.ecr.aws - docker push $ECR_REGISTRY/$ECR_REPOSITORY:$IMAGE_TAG - docker tag $ECR_REGISTRY/$ECR_REPOSITORY:$IMAGE_TAG $ECR_REGISTRY/$ECR_REPOSITORY:latest - docker push $ECR_REGISTRY/$ECR_REPOSITORY:latest + # Building docker image + docker run --rm --privileged multiarch/qemu-user-static --reset -p yes + docker buildx create --name multiarch --driver docker-container --use + # Building docker image for multiple platforms and pushing to ECR + docker buildx build --push --platform "linux/amd64,linux/arm64" -t $ECR_REGISTRY/$ECR_REPOSITORY:$IMAGE_TAG . + docker buildx build --push --platform "linux/amd64,linux/arm64" -t $ECR_REGISTRY/$ECR_REPOSITORY:latest . echo "::set-output name=image::$ECR_REGISTRY/$ECR_REPOSITORY:$IMAGE_TAG" diff --git a/Dockerfile b/Dockerfile index f031519..0856ab4 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM continuumio/miniconda3 +FROM python:3.9 ENV ERRBOT_DIR=/errbot ENV SDM_DOCKERIZED=true @@ -7,7 +7,6 @@ RUN mkdir -p $ERRBOT_DIR WORKDIR $ERRBOT_DIR COPY requirements/common.txt ./requirements.txt -RUN apt update -y && apt install -y gcc RUN pip install \ --no-cache-dir \ --disable-pip-version-check \ From 87a93755542dc3259b2341d330b69a1abd4054c6 Mon Sep 17 00:00:00 2001 From: vassalo Date: Mon, 17 Oct 2022 11:28:47 -0300 Subject: [PATCH 3/3] Refactor DURATION_FLAG_LIMIT to GRANT_TIMEOUT_LIMIT and make it consistent with the rest of the env vars and add docs (#283) Co-authored-by: wallrony --- .../CONFIGURE_ACCESSBOT.md | 1 + e2e/slack/test_accessbot_slack_access.py | 32 +++++++------------ e2e/test_common.py | 2 +- plugins/sdm/config_template.py | 2 +- .../sdm/lib/helper/resource_grant_helper.py | 13 +++----- requirements/common.txt | 1 + 6 files changed, 21 insertions(+), 30 deletions(-) diff --git a/docs/configure_accessbot/CONFIGURE_ACCESSBOT.md b/docs/configure_accessbot/CONFIGURE_ACCESSBOT.md index beebb8f..ab92c05 100644 --- a/docs/configure_accessbot/CONFIGURE_ACCESSBOT.md +++ b/docs/configure_accessbot/CONFIGURE_ACCESSBOT.md @@ -54,6 +54,7 @@ You just need to remove the "SDM_" prefix when configuring them. Here's a usage * **SDM_ENABLE_BOT_STATE_HANDLING**. Boolean flag to enable persistent grant requests. When enabled, all grant requests will be synced in a local file, that way if AccessBot goes down, all ongoing requests will be restored. Default = false * **SDM_ENABLE_RESOURCES_FUZZY_MATCHING**. Flag to enable fuzzy matching for resources when a perfect match is not found. Default = true * **SDM_GRANT_TIMEOUT**. Timeout in minutes for an access grant. Default = 60 min +* **SDM_GRANT_TIMEOUT_LIMIT**. Timeout limit in minutes for an access grant when using the `--duration` flag. Disabled by default * **SDM_GROUPS_TAG**. User tag to be used for specifying the groups a user belongs to. Disabled by default ([see below](#user-groups) for more info about using tags) * **SDM_HIDE_RESOURCE_TAG**. Resource tag to be used for hiding available resources, meaning that they are not going to be shown nor accessible. Ideally set value to `true` or `false` (e.g. `hide-resource=true`). If there's no value, it's interpreted as `true`. Disabled by default ([see below](#using-tags) for more info about using tags) * **SDM_HIDE_ROLE_TAG**. Role tag to be used for hiding available roles, meaning that they are not going to be shown nor accessible. Ideally set value to `true` or `false` (e.g. `hide-role=true`). If there's no value, it's interpreted as `true`. Disabled by default ([see below](#using-tags) for more info about using tags) diff --git a/e2e/slack/test_accessbot_slack_access.py b/e2e/slack/test_accessbot_slack_access.py index e2185a6..7c9d6a1 100644 --- a/e2e/slack/test_accessbot_slack_access.py +++ b/e2e/slack/test_accessbot_slack_access.py @@ -194,54 +194,46 @@ def test_access_command_dont_grant_access_when_duration_flag_has_duration_equals mocked_testbot.push_message(f"access to Xxx --duration {duration}") assert "You need to enter a duration greater than zero" in mocked_testbot.pop_message() - def test_access_command_fails_with_duration_flag_value_and_invalid_duration_limit(self, mocked_testbot): - duration = '45m' - duration_limit = '40x' - accessbot = mocked_testbot.bot.plugin_manager.plugins['AccessBot'] - accessbot.config['DURATION_FLAG_LIMIT'] = duration_limit - mocked_testbot.push_message(f"access to Xxx --duration {duration}") - assert "invalid duration limit was defined" in mocked_testbot.pop_message() - def test_access_command_with_duration_flag_value_lesser_than_duration_limit(self, mocked_testbot): duration = '40m' - duration_limit = '45m' + duration_limit = '45' accessbot = mocked_testbot.bot.plugin_manager.plugins['AccessBot'] - accessbot.config['DURATION_FLAG_LIMIT'] = duration_limit + accessbot.config['GRANT_TIMEOUT_LIMIT'] = duration_limit mocked_testbot.push_message(f"access to Xxx --duration {duration}") assert "valid request" in mocked_testbot.pop_message() assert "access request" in mocked_testbot.pop_message() def test_access_command_with_duration_flag_value_equals_to_duration_limit(self, mocked_testbot): duration = '40m' - duration_limit = '40m' + duration_limit = '40' accessbot = mocked_testbot.bot.plugin_manager.plugins['AccessBot'] - accessbot.config['DURATION_FLAG_LIMIT'] = duration_limit + accessbot.config['GRANT_TIMEOUT_LIMIT'] = duration_limit mocked_testbot.push_message(f"access to Xxx --duration {duration}") assert "valid request" in mocked_testbot.pop_message() assert "access request" in mocked_testbot.pop_message() def test_access_command_with_duration_flag_time_unit_different_than_duration_limit_unit(self, mocked_testbot): - duration = '40m' - duration_limit = '1h' + duration = '1h' + duration_limit = '60' accessbot = mocked_testbot.bot.plugin_manager.plugins['AccessBot'] - accessbot.config['DURATION_FLAG_LIMIT'] = duration_limit + accessbot.config['GRANT_TIMEOUT_LIMIT'] = duration_limit mocked_testbot.push_message(f"access to Xxx --duration {duration}") assert "valid request" in mocked_testbot.pop_message() assert "access request" in mocked_testbot.pop_message() def test_access_command_fails_with_duration_flag_value_greater_than_duration_limit(self, mocked_testbot): duration = '45m' - duration_limit = '40m' + duration_limit = '40' accessbot = mocked_testbot.bot.plugin_manager.plugins['AccessBot'] - accessbot.config['DURATION_FLAG_LIMIT'] = duration_limit + accessbot.config['GRANT_TIMEOUT_LIMIT'] = duration_limit mocked_testbot.push_message(f"access to Xxx --duration {duration}") assert "need to enter a duration lesser or equals" in mocked_testbot.pop_message() def test_access_command_fails_with_duration_flag_time_unit_different_than_duration_limit_unit(self, mocked_testbot): - duration = '65m' - duration_limit = '1h' + duration = '2h' + duration_limit = '60' accessbot = mocked_testbot.bot.plugin_manager.plugins['AccessBot'] - accessbot.config['DURATION_FLAG_LIMIT'] = duration_limit + accessbot.config['GRANT_TIMEOUT_LIMIT'] = duration_limit mocked_testbot.push_message(f"access to Xxx --duration {duration}") assert "need to enter a duration lesser or equals" in mocked_testbot.pop_message() diff --git a/e2e/test_common.py b/e2e/test_common.py index 7d2bf82..5f08c22 100644 --- a/e2e/test_common.py +++ b/e2e/test_common.py @@ -82,7 +82,7 @@ def create_config(): 'APPROVERS_CHANNEL_TAG': None, 'ALLOW_RESOURCE_ACCESS_REQUEST_RENEWAL': False, 'ENABLE_BOT_STATE_HANDLING': False, - 'DURATION_FLAG_LIMIT': None, + 'GRANT_TIMEOUT_LIMIT': None, } diff --git a/plugins/sdm/config_template.py b/plugins/sdm/config_template.py index 5aad3ac..64d523a 100644 --- a/plugins/sdm/config_template.py +++ b/plugins/sdm/config_template.py @@ -34,7 +34,7 @@ 'APPROVERS_CHANNEL_TAG': os.getenv("SDM_APPROVERS_CHANNEL_TAG"), 'ALLOW_RESOURCE_ACCESS_REQUEST_RENEWAL': str(os.getenv("SDM_ALLOW_RESOURCE_ACCESS_REQUEST_RENEWAL", "")).lower() == 'true', 'ENABLE_BOT_STATE_HANDLING': str(os.getenv("SDM_ENABLE_BOT_STATE_HANDLING", "")).lower() == 'true', - 'DURATION_FLAG_LIMIT': os.getenv('SDM_DURATION_FLAG_LIMIT'), + 'GRANT_TIMEOUT_LIMIT': os.getenv('SDM_GRANT_TIMEOUT_LIMIT'), } def get(): diff --git a/plugins/sdm/lib/helper/resource_grant_helper.py b/plugins/sdm/lib/helper/resource_grant_helper.py index 14aebb2..0fa0cd8 100644 --- a/plugins/sdm/lib/helper/resource_grant_helper.py +++ b/plugins/sdm/lib/helper/resource_grant_helper.py @@ -3,6 +3,7 @@ from .base_grant_helper import BaseGrantHelper from ..exceptions import PermissionDeniedException from ..util import VALID_TIME_UNITS, convert_duration_flag_to_timedelta +from readabledelta import readabledelta class ResourceGrantHelper(BaseGrantHelper): @@ -62,16 +63,12 @@ def duration_flag_validator(self, value: str): duration = int(re.search(r'\d+', value).group()) if duration == 0: raise Exception('You need to enter a duration greater than zero.') - duration_limit_var = self.__bot.config['DURATION_FLAG_LIMIT'] - if duration_limit_var is not None: - short_limit_time_unit = self.get_short_time_unit_from_duration(duration_limit_var) - if short_limit_time_unit is None: - self.__bot.log.error(f'Invalid duration limit: unparseable time unit from "{duration_limit_var}"') - raise Exception("An invalid duration limit was defined.") + duration_limit = self.__bot.config['GRANT_TIMEOUT_LIMIT'] + if duration_limit is not None: duration_timedelta = convert_duration_flag_to_timedelta(value) - duration_limit_timedelta = convert_duration_flag_to_timedelta(duration_limit_var) + duration_limit_timedelta = convert_duration_flag_to_timedelta(f"{duration_limit}m") if duration_timedelta > duration_limit_timedelta: - raise Exception(f"You need to enter a duration lesser or equals to {duration_limit_var}") + raise Exception(f"You need to enter a duration lesser or equals to {readabledelta(duration_limit_timedelta)}") return True def get_short_time_unit_from_duration(self, duration): diff --git a/requirements/common.txt b/requirements/common.txt index 4250406..f0c5573 100644 --- a/requirements/common.txt +++ b/requirements/common.txt @@ -9,3 +9,4 @@ fuzzywuzzy python-levenshtein-wheels memoization prometheus_client +readabledelta