From 43bce200edd86669048622242591fb262fa6f788 Mon Sep 17 00:00:00 2001 From: mohit Date: Wed, 17 Jan 2024 09:20:21 +0000 Subject: [PATCH 01/24] regex check for username --- .env.example | 2 +- api/api/adapter/dynamodb_adapter.py | 2 +- api/api/common/config/constants.py | 3 +- api/api/domain/user.py | 20 ++++-- api/requirements.txt | 2 +- api/test/api/domain/test_user_request.py | 65 ++++++++++++++++++- infrastructure/modules/app-cluster/main.tf | 1 + .../modules/app-cluster/variables.tf | 13 ++++ 8 files changed, 98 insertions(+), 10 deletions(-) diff --git a/.env.example b/.env.example index 95657e4..5c7cee3 100644 --- a/.env.example +++ b/.env.example @@ -8,7 +8,7 @@ RESOURCE_PREFIX=rapid DOMAIN_NAME=example.com COGNITO_USER_POOL_ID=11111111 LAYERS=raw,layer - +CUSTOM_USERNAME_REGEX=["regex_expression", "test_string"] # SDK Specific RAPID_CLIENT_ID= RAPID_CLIENT_SECRET= diff --git a/api/api/adapter/dynamodb_adapter.py b/api/api/adapter/dynamodb_adapter.py index 115277e..6ad04c9 100644 --- a/api/api/adapter/dynamodb_adapter.py +++ b/api/api/adapter/dynamodb_adapter.py @@ -190,7 +190,7 @@ def store_schema(self, schema: Schema) -> None: def validate_permissions(self, subject_permissions: List[str]) -> None: if not subject_permissions: raise UserError("At least one permission must be provided") - permissions_response = self._find_permissions(subject_permissions) + permissions_response = self._find_permissions(subjec`t_permissions) if not len(permissions_response) == len(subject_permissions): AppLogger.info(f"Invalid permission in {subject_permissions}") raise UserError( diff --git a/api/api/common/config/constants.py b/api/api/common/config/constants.py index 60f6ccb..e3dc92c 100644 --- a/api/api/common/config/constants.py +++ b/api/api/common/config/constants.py @@ -1,3 +1,5 @@ +import os + BASE_API_PATH = "/api" BASE_REGEX = "^[a-zA-Z0-9_-]" FILENAME_WITH_TIMESTAMP_REGEX = r"[a-zA-Z0-9:_\-]+.csv$" @@ -22,7 +24,6 @@ ) USERNAME_REGEX = "[a-zA-Z][a-zA-Z0-9@._-]{2,127}" - DEFAULT_JOB_EXPIRY_DAYS = 1 UPLOAD_JOB_EXPIRY_DAYS = 7 QUERY_JOB_EXPIRY_DAYS = 1 diff --git a/api/api/domain/user.py b/api/api/domain/user.py index e593a4d..c323bda 100644 --- a/api/api/domain/user.py +++ b/api/api/domain/user.py @@ -1,12 +1,17 @@ import re from typing import Optional, List - +import os from pydantic import BaseModel from api.common.config.auth import DEFAULT_PERMISSION, ALLOWED_EMAIL_DOMAINS -from api.common.config.constants import EMAIL_REGEX, USERNAME_REGEX +from api.common.config.constants import ( + EMAIL_REGEX, + USERNAME_REGEX, +) from api.common.custom_exceptions import UserError +CUSTOM_USERNAME_REGEX = os.environ.get("CUSTOM_USERNAME_REGEX") + class UserRequest(BaseModel): username: str @@ -19,8 +24,15 @@ def get_validated_username(self): https://docs.aws.amazon.com/cognito/latest/developerguide/limits.html """ if self.username is not None and re.fullmatch(USERNAME_REGEX, self.username): - return self.username - raise UserError("Invalid username provided") + if re.fullmatch(CUSTOM_USERNAME_REGEX, self.username): + return self.username + # I don't know if these messages need to be substantially different from a user point of view, but have tried for now anyways. + raise UserError( + "Your username does not match the requirements specified by your organisation. Please check the username and try again" + ) + raise UserError( + "This username is invalid. Please check the username and try again" + ) def get_permissions(self) -> List[str]: return self.permissions diff --git a/api/requirements.txt b/api/requirements.txt index 440de54..b22e35f 100644 --- a/api/requirements.txt +++ b/api/requirements.txt @@ -13,4 +13,4 @@ pydantic[email] python-multipart uvicorn requests -strenum +strenum \ No newline at end of file diff --git a/api/test/api/domain/test_user_request.py b/api/test/api/domain/test_user_request.py index 035f13c..9f4fd6f 100644 --- a/api/test/api/domain/test_user_request.py +++ b/api/test/api/domain/test_user_request.py @@ -4,6 +4,16 @@ from api.domain.user import UserRequest +@pytest.fixture +def custom_user_regex_default(): + return "[a-zA-Z][a-zA-Z0-9@._-]{2,127}" + + +@pytest.fixture +def custom_user_regex_non_default(): + return "^[A-Z][A-Za-z0-9]{3,50}$" + + class TestUserRequest: @pytest.mark.parametrize( "provided_username", @@ -16,9 +26,11 @@ class TestUserRequest: "S1234", ], ) - def test_get_validated_username(self, provided_username): + def test_get_validated_username(self, provided_username, custom_user_regex_default): request = UserRequest(username=provided_username, email="user@email.com") + # Overrwrite env variable on fn import + CUSTOM_USERNAME_REGEX = custom_user_regex_default try: validated_name = request.get_validated_username() assert validated_name == provided_username @@ -40,7 +52,56 @@ def test_get_validated_username(self, provided_username): "A" * 129, ], ) - def test_raises_error_when_invalid_username(self, provided_username): + def test_raises_error_when_invalid_username( + self, provided_username, custom_user_regex_default + ): + request = UserRequest(username=provided_username, email="user@email.com") + # Overrwrite env variable on fn import + CUSTOM_USERNAME_REGEX = custom_user_regex_default + with pytest.raises(UserError, match="Invalid username provided"): + request.get_validated_username() + + @pytest.mark.parametrize( + "provided_username", + [ + "Ttest123", + "TCheck", + "SamCheck", + "Sam123Check456", + "S1234", + ], + ) + def test_get_validated_username_custom_regex( + self, provided_username, custom_user_regex_non_default + ): + CUSTOM_USERNAME_REGEX = custom_user_regex_non_default + request = UserRequest(username=provided_username, email="user@email.com") + try: + validated_name = request.get_validated_username() + assert validated_name == provided_username + except UserError: + pytest.fail("An unexpected UserError was thrown") + + @pytest.mark.parametrize( + "provided_username", + [ + "", + " ", + "SOme naME", + "sOMe!name", + "-some-nAMe", + "(some)namE", + "1234", + "....", + "A" * 2, + "A" * 129, + ], + ) + def test_raises_error_when_invalid_username( + self, provided_username, custom_user_regex_non_default + ): + CUSTOM_USERNAME_REGEX = custom_user_regex_non_default + request = UserRequest(username=provided_username, email="user@email.com") with pytest.raises(UserError, match="Invalid username provided"): diff --git a/infrastructure/modules/app-cluster/main.tf b/infrastructure/modules/app-cluster/main.tf index 2d8e424..4912850 100644 --- a/infrastructure/modules/app-cluster/main.tf +++ b/infrastructure/modules/app-cluster/main.tf @@ -9,6 +9,7 @@ locals { "COGNITO_USER_POOL_ID" : var.cognito_user_pool_id, "RESOURCE_PREFIX" : var.resource-name-prefix, "COGNITO_USER_LOGIN_APP_CREDENTIALS_SECRETS_NAME" : var.cognito_user_login_app_credentials_secrets_name + "CUSTOM_USERNAME_REGEX" : var.custom_username_regex }, var.project_information) } diff --git a/infrastructure/modules/app-cluster/variables.tf b/infrastructure/modules/app-cluster/variables.tf index 47e7b7c..ef4f8f7 100644 --- a/infrastructure/modules/app-cluster/variables.tf +++ b/infrastructure/modules/app-cluster/variables.tf @@ -202,3 +202,16 @@ variable "layers" { description = "A list of the layers that the rAPId instance will contain" default = ["default"] } + +variable "custom_username_regex" { + type = list(string) + description = "A list containing the regex expression for conditional user validation and a string to test against." + default = ["[a-zA-Z][a-zA-Z0-9@._-]{2,127}", "AUser1"] + + validation { + condition = can(regex(var.regex_expression[0], var.regex_expression[1])) + error_message = "Your regex expression cannot be evaluated against your test string. Please check the expression and the test string." + } +} + + From ddf27a7017ae2ddccc0304c4ad768c105c10e921 Mon Sep 17 00:00:00 2001 From: mohit Date: Wed, 17 Jan 2024 10:54:06 +0000 Subject: [PATCH 02/24] some more changes --- api/api/adapter/dynamodb_adapter.py | 2 +- api/api/common/config/auth.py | 2 ++ api/api/domain/user.py | 9 +++++++-- api/batect.yml | 2 +- infrastructure/modules/app-cluster/variables.tf | 7 ++++--- 5 files changed, 15 insertions(+), 7 deletions(-) diff --git a/api/api/adapter/dynamodb_adapter.py b/api/api/adapter/dynamodb_adapter.py index 6ad04c9..115277e 100644 --- a/api/api/adapter/dynamodb_adapter.py +++ b/api/api/adapter/dynamodb_adapter.py @@ -190,7 +190,7 @@ def store_schema(self, schema: Schema) -> None: def validate_permissions(self, subject_permissions: List[str]) -> None: if not subject_permissions: raise UserError("At least one permission must be provided") - permissions_response = self._find_permissions(subjec`t_permissions) + permissions_response = self._find_permissions(subject_permissions) if not len(permissions_response) == len(subject_permissions): AppLogger.info(f"Invalid permission in {subject_permissions}") raise UserError( diff --git a/api/api/common/config/auth.py b/api/api/common/config/auth.py index c1f7aae..89685d9 100644 --- a/api/api/common/config/auth.py +++ b/api/api/common/config/auth.py @@ -18,6 +18,8 @@ COGNITO_RESOURCE_SERVER_ID = f"https://{DOMAIN_NAME}" COGNITO_USER_POOL_ID = os.environ["COGNITO_USER_POOL_ID"] +CUSTOM_USERNAME_REGEX = os.environ.get("CUSTOM_USERNAME_REGEX") + ALLOWED_EMAIL_DOMAINS = os.environ["ALLOWED_EMAIL_DOMAINS"] IDENTITY_PROVIDER_BASE_URL = ( diff --git a/api/api/domain/user.py b/api/api/domain/user.py index c323bda..ae23380 100644 --- a/api/api/domain/user.py +++ b/api/api/domain/user.py @@ -2,15 +2,20 @@ from typing import Optional, List import os from pydantic import BaseModel +from dotenv import load_dotenv -from api.common.config.auth import DEFAULT_PERMISSION, ALLOWED_EMAIL_DOMAINS +from api.common.config.auth import ( + DEFAULT_PERMISSION, + ALLOWED_EMAIL_DOMAINS, + CUSTOM_USERNAME_REGEX, +) from api.common.config.constants import ( EMAIL_REGEX, USERNAME_REGEX, ) from api.common.custom_exceptions import UserError -CUSTOM_USERNAME_REGEX = os.environ.get("CUSTOM_USERNAME_REGEX") +print(CUSTOM_USERNAME_REGEX) class UserRequest(BaseModel): diff --git a/api/batect.yml b/api/batect.yml index de84076..a3aeac6 100755 --- a/api/batect.yml +++ b/api/batect.yml @@ -23,7 +23,7 @@ containers: ALLOWED_EMAIL_DOMAINS: ${ALLOWED_EMAIL_DOMAINS:-} RESOURCE_PREFIX: ${RESOURCE_PREFIX:-prefix} LAYERS: ${LAYERS:-} - + CUSTOM_USERNAME_REGEX: ${CUSTOM_USERNAME_REGEX:-} tasks: runtime-environment: description: Build runtime environment diff --git a/infrastructure/modules/app-cluster/variables.tf b/infrastructure/modules/app-cluster/variables.tf index ef4f8f7..3162d27 100644 --- a/infrastructure/modules/app-cluster/variables.tf +++ b/infrastructure/modules/app-cluster/variables.tf @@ -205,11 +205,12 @@ variable "layers" { variable "custom_username_regex" { type = list(string) - description = "A list containing the regex expression for conditional user validation and a string to test against." - default = ["[a-zA-Z][a-zA-Z0-9@._-]{2,127}", "AUser1"] + description = "A list containing the regex expression for conditional user validation and a string to test against. The input must be a terraform list variable composed of two strings. The first string is the regex expression and the second string is the test string for said regular expression." + # Set default to a regex expression for basic api username validation + default = ["[a-zA-Z][a-zA-Z0-9@._-]{2,127}", "AUser1"] validation { - condition = can(regex(var.regex_expression[0], var.regex_expression[1])) + condition = can(regex(var.custom_username_regex[0], var.custom_username_regex[1])) error_message = "Your regex expression cannot be evaluated against your test string. Please check the expression and the test string." } } From 175db07cfc0ec714f1596ef4376e803d6d477a62 Mon Sep 17 00:00:00 2001 From: mohit Date: Wed, 17 Jan 2024 12:35:48 +0000 Subject: [PATCH 03/24] Mostly working tests --- api/api/domain/user.py | 11 ++-- api/test/api/adapter/test_cognito_adapter.py | 1 + api/test/api/domain/test_user_request.py | 58 +++++++++----------- 3 files changed, 32 insertions(+), 38 deletions(-) diff --git a/api/api/domain/user.py b/api/api/domain/user.py index ae23380..777d339 100644 --- a/api/api/domain/user.py +++ b/api/api/domain/user.py @@ -15,25 +15,24 @@ ) from api.common.custom_exceptions import UserError -print(CUSTOM_USERNAME_REGEX) - class UserRequest(BaseModel): username: str email: str permissions: Optional[List[str]] = DEFAULT_PERMISSION - def get_validated_username(self): + def get_validated_username( + self, custom_username_regex=os.environ.get("CUSTOM_USERNAME_REGEX") + ): """ We restrict further beyond Cognito limits: https://docs.aws.amazon.com/cognito/latest/developerguide/limits.html """ if self.username is not None and re.fullmatch(USERNAME_REGEX, self.username): - if re.fullmatch(CUSTOM_USERNAME_REGEX, self.username): + if re.fullmatch(custom_username_regex, self.username): return self.username - # I don't know if these messages need to be substantially different from a user point of view, but have tried for now anyways. raise UserError( - "Your username does not match the requirements specified by your organisation. Please check the username and try again" + "Your username does not match the requirements specified by your organisation" ) raise UserError( "This username is invalid. Please check the username and try again" diff --git a/api/test/api/adapter/test_cognito_adapter.py b/api/test/api/adapter/test_cognito_adapter.py index a43a7b8..c7384c0 100644 --- a/api/test/api/adapter/test_cognito_adapter.py +++ b/api/test/api/adapter/test_cognito_adapter.py @@ -248,6 +248,7 @@ def test_throws_error_when_client_app_name_has_not_been_changed_from_placeholder class TestCognitoAdapterUsers(BaseCognitoAdapter): def test_create_user(self): + CUSTOM_USERNAME_REGEX = "[a-zA-Z][a-zA-Z0-9@._-]{2,127}" cognito_response = { "User": { "Username": "user-name", diff --git a/api/test/api/domain/test_user_request.py b/api/test/api/domain/test_user_request.py index 9f4fd6f..3f042d1 100644 --- a/api/test/api/domain/test_user_request.py +++ b/api/test/api/domain/test_user_request.py @@ -4,16 +4,6 @@ from api.domain.user import UserRequest -@pytest.fixture -def custom_user_regex_default(): - return "[a-zA-Z][a-zA-Z0-9@._-]{2,127}" - - -@pytest.fixture -def custom_user_regex_non_default(): - return "^[A-Z][A-Za-z0-9]{3,50}$" - - class TestUserRequest: @pytest.mark.parametrize( "provided_username", @@ -30,9 +20,10 @@ def test_get_validated_username(self, provided_username, custom_user_regex_defau request = UserRequest(username=provided_username, email="user@email.com") # Overrwrite env variable on fn import - CUSTOM_USERNAME_REGEX = custom_user_regex_default try: - validated_name = request.get_validated_username() + validated_name = request.get_validated_username( + custom_username_regex=custom_user_regex_default + ) assert validated_name == provided_username except UserError: pytest.fail("An unexpected UserError was thrown") @@ -57,9 +48,13 @@ def test_raises_error_when_invalid_username( ): request = UserRequest(username=provided_username, email="user@email.com") # Overrwrite env variable on fn import - CUSTOM_USERNAME_REGEX = custom_user_regex_default - with pytest.raises(UserError, match="Invalid username provided"): - request.get_validated_username() + with pytest.raises( + UserError, + match="This username is invalid. Please check the username and try again", + ): + request.get_validated_username( + custom_username_regex=custom_user_regex_default + ) @pytest.mark.parametrize( "provided_username", @@ -74,10 +69,11 @@ def test_raises_error_when_invalid_username( def test_get_validated_username_custom_regex( self, provided_username, custom_user_regex_non_default ): - CUSTOM_USERNAME_REGEX = custom_user_regex_non_default request = UserRequest(username=provided_username, email="user@email.com") try: - validated_name = request.get_validated_username() + validated_name = request.get_validated_username( + custom_username_regex=custom_user_regex_non_default + ) assert validated_name == provided_username except UserError: pytest.fail("An unexpected UserError was thrown") @@ -85,27 +81,25 @@ def test_get_validated_username_custom_regex( @pytest.mark.parametrize( "provided_username", [ - "", - " ", - "SOme naME", - "sOMe!name", - "-some-nAMe", - "(some)namE", - "1234", - "....", - "A" * 2, - "A" * 129, + "username_name", + "username_name2", + "username@email.com", + "VA.li_d@na-mE", + "A....", ], ) - def test_raises_error_when_invalid_username( + def test_raises_error_when_invalid_username_custom_regex( self, provided_username, custom_user_regex_non_default ): - CUSTOM_USERNAME_REGEX = custom_user_regex_non_default - request = UserRequest(username=provided_username, email="user@email.com") - with pytest.raises(UserError, match="Invalid username provided"): - request.get_validated_username() + with pytest.raises( + UserError, + match="Your username does not match the requirements specified by your organisation", + ): + request.get_validated_username( + custom_username_regex=custom_user_regex_non_default + ) @pytest.mark.parametrize( "provided_email", From 49a4e7008cdc78c5d76fd6bbc39cc65497bbe699 Mon Sep 17 00:00:00 2001 From: mohit Date: Wed, 17 Jan 2024 12:35:55 +0000 Subject: [PATCH 04/24] mostly working tests --- api/test/api/domain/test_user_request.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/api/test/api/domain/test_user_request.py b/api/test/api/domain/test_user_request.py index 3f042d1..8a014da 100644 --- a/api/test/api/domain/test_user_request.py +++ b/api/test/api/domain/test_user_request.py @@ -4,6 +4,16 @@ from api.domain.user import UserRequest +@pytest.fixture +def custom_user_regex_default(): + return "[a-zA-Z][a-zA-Z0-9@._-]{2,127}" + + +@pytest.fixture +def custom_user_regex_non_default(): + return "^[A-Z][A-Za-z0-9]{3,50}$" + + class TestUserRequest: @pytest.mark.parametrize( "provided_username", From db5a914812f071c161c939b0a049582ecd058928 Mon Sep 17 00:00:00 2001 From: mohit Date: Wed, 17 Jan 2024 12:50:56 +0000 Subject: [PATCH 05/24] passed tests --- .env.example | 2 +- api/api/domain/user.py | 11 ++--- api/test/api/adapter/test_cognito_adapter.py | 12 ++++- api/test/api/domain/test_user_request.py | 50 +++++++------------- 4 files changed, 33 insertions(+), 42 deletions(-) diff --git a/.env.example b/.env.example index 5c7cee3..121c31c 100644 --- a/.env.example +++ b/.env.example @@ -8,7 +8,7 @@ RESOURCE_PREFIX=rapid DOMAIN_NAME=example.com COGNITO_USER_POOL_ID=11111111 LAYERS=raw,layer -CUSTOM_USERNAME_REGEX=["regex_expression", "test_string"] +CUSTOM_USERNAME_REGEX="regex_expression" # SDK Specific RAPID_CLIENT_ID= RAPID_CLIENT_SECRET= diff --git a/api/api/domain/user.py b/api/api/domain/user.py index 777d339..0b6f5e7 100644 --- a/api/api/domain/user.py +++ b/api/api/domain/user.py @@ -4,11 +4,7 @@ from pydantic import BaseModel from dotenv import load_dotenv -from api.common.config.auth import ( - DEFAULT_PERMISSION, - ALLOWED_EMAIL_DOMAINS, - CUSTOM_USERNAME_REGEX, -) +from api.common.config.auth import DEFAULT_PERMISSION, ALLOWED_EMAIL_DOMAINS from api.common.config.constants import ( EMAIL_REGEX, USERNAME_REGEX, @@ -21,14 +17,13 @@ class UserRequest(BaseModel): email: str permissions: Optional[List[str]] = DEFAULT_PERMISSION - def get_validated_username( - self, custom_username_regex=os.environ.get("CUSTOM_USERNAME_REGEX") - ): + def get_validated_username(self): """ We restrict further beyond Cognito limits: https://docs.aws.amazon.com/cognito/latest/developerguide/limits.html """ if self.username is not None and re.fullmatch(USERNAME_REGEX, self.username): + custom_username_regex = os.environ.get("CUSTOM_USERNAME_REGEX") if re.fullmatch(custom_username_regex, self.username): return self.username raise UserError( diff --git a/api/test/api/adapter/test_cognito_adapter.py b/api/test/api/adapter/test_cognito_adapter.py index c7384c0..1385219 100644 --- a/api/test/api/adapter/test_cognito_adapter.py +++ b/api/test/api/adapter/test_cognito_adapter.py @@ -1,8 +1,9 @@ from abc import ABC from unittest.mock import Mock - +from unittest import mock import pytest from botocore.exceptions import ClientError +import os from api.adapter.cognito_adapter import CognitoAdapter from api.common.config.auth import COGNITO_USER_POOL_ID, COGNITO_RESOURCE_SERVER_ID @@ -247,6 +248,9 @@ def test_throws_error_when_client_app_name_has_not_been_changed_from_placeholder class TestCognitoAdapterUsers(BaseCognitoAdapter): + @mock.patch.dict( + os.environ, {"CUSTOM_USERNAME_REGEX": "[a-zA-Z][a-zA-Z0-9@._-]{2,127}"} + ) def test_create_user(self): CUSTOM_USERNAME_REGEX = "[a-zA-Z][a-zA-Z0-9@._-]{2,127}" cognito_response = { @@ -321,6 +325,9 @@ def test_delete_user_fails_when_user_does_not_exist(self): ): self.cognito_adapter.delete_user("my_user") + @mock.patch.dict( + os.environ, {"CUSTOM_USERNAME_REGEX": "[a-zA-Z][a-zA-Z0-9@._-]{2,127}"} + ) def test_create_user_fails_in_aws(self): request = UserRequest( username="user-name", @@ -338,6 +345,9 @@ def test_create_user_fails_in_aws(self): ): self.cognito_adapter.create_user(request) + @mock.patch.dict( + os.environ, {"CUSTOM_USERNAME_REGEX": "[a-zA-Z][a-zA-Z0-9@._-]{2,127}"} + ) def test_create_user_fails_when_the_user_already_exist(self): request = UserRequest( username="user-name", diff --git a/api/test/api/domain/test_user_request.py b/api/test/api/domain/test_user_request.py index 8a014da..742559d 100644 --- a/api/test/api/domain/test_user_request.py +++ b/api/test/api/domain/test_user_request.py @@ -2,16 +2,8 @@ from api.common.custom_exceptions import UserError from api.domain.user import UserRequest - - -@pytest.fixture -def custom_user_regex_default(): - return "[a-zA-Z][a-zA-Z0-9@._-]{2,127}" - - -@pytest.fixture -def custom_user_regex_non_default(): - return "^[A-Z][A-Za-z0-9]{3,50}$" +from unittest import mock +import os class TestUserRequest: @@ -26,14 +18,15 @@ class TestUserRequest: "S1234", ], ) - def test_get_validated_username(self, provided_username, custom_user_regex_default): + @mock.patch.dict( + os.environ, {"CUSTOM_USERNAME_REGEX": "[a-zA-Z][a-zA-Z0-9@._-]{2,127}"} + ) + def test_get_validated_username(self, provided_username): request = UserRequest(username=provided_username, email="user@email.com") # Overrwrite env variable on fn import try: - validated_name = request.get_validated_username( - custom_username_regex=custom_user_regex_default - ) + validated_name = request.get_validated_username() assert validated_name == provided_username except UserError: pytest.fail("An unexpected UserError was thrown") @@ -53,18 +46,17 @@ def test_get_validated_username(self, provided_username, custom_user_regex_defau "A" * 129, ], ) - def test_raises_error_when_invalid_username( - self, provided_username, custom_user_regex_default - ): + @mock.patch.dict( + os.environ, {"CUSTOM_USERNAME_REGEX": "[a-zA-Z][a-zA-Z0-9@._-]{2,127}"} + ) + def test_raises_error_when_invalid_username(self, provided_username): request = UserRequest(username=provided_username, email="user@email.com") # Overrwrite env variable on fn import with pytest.raises( UserError, match="This username is invalid. Please check the username and try again", ): - request.get_validated_username( - custom_username_regex=custom_user_regex_default - ) + request.get_validated_username() @pytest.mark.parametrize( "provided_username", @@ -76,14 +68,11 @@ def test_raises_error_when_invalid_username( "S1234", ], ) - def test_get_validated_username_custom_regex( - self, provided_username, custom_user_regex_non_default - ): + @mock.patch.dict(os.environ, {"CUSTOM_USERNAME_REGEX": "^[A-Z][A-Za-z0-9]{3,50}$"}) + def test_get_validated_username_custom_regex(self, provided_username): request = UserRequest(username=provided_username, email="user@email.com") try: - validated_name = request.get_validated_username( - custom_username_regex=custom_user_regex_non_default - ) + validated_name = request.get_validated_username() assert validated_name == provided_username except UserError: pytest.fail("An unexpected UserError was thrown") @@ -98,18 +87,15 @@ def test_get_validated_username_custom_regex( "A....", ], ) - def test_raises_error_when_invalid_username_custom_regex( - self, provided_username, custom_user_regex_non_default - ): + @mock.patch.dict(os.environ, {"CUSTOM_USERNAME_REGEX": "^[A-Z][A-Za-z0-9]{3,50}$"}) + def test_raises_error_when_invalid_username_custom_regex(self, provided_username): request = UserRequest(username=provided_username, email="user@email.com") with pytest.raises( UserError, match="Your username does not match the requirements specified by your organisation", ): - request.get_validated_username( - custom_username_regex=custom_user_regex_non_default - ) + request.get_validated_username() @pytest.mark.parametrize( "provided_email", From 5ef359d5296d40ee611a22a25bd0e5748e5eb561 Mon Sep 17 00:00:00 2001 From: mohit Date: Wed, 17 Jan 2024 13:48:28 +0000 Subject: [PATCH 06/24] lint errors --- api/api/common/config/constants.py | 2 -- api/api/domain/user.py | 4 ++++ api/test/api/adapter/test_cognito_adapter.py | 1 - 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/api/api/common/config/constants.py b/api/api/common/config/constants.py index e3dc92c..4534e60 100644 --- a/api/api/common/config/constants.py +++ b/api/api/common/config/constants.py @@ -1,5 +1,3 @@ -import os - BASE_API_PATH = "/api" BASE_REGEX = "^[a-zA-Z0-9_-]" FILENAME_WITH_TIMESTAMP_REGEX = r"[a-zA-Z0-9:_\-]+.csv$" diff --git a/api/api/domain/user.py b/api/api/domain/user.py index 0b6f5e7..71373ce 100644 --- a/api/api/domain/user.py +++ b/api/api/domain/user.py @@ -4,6 +4,10 @@ from pydantic import BaseModel from dotenv import load_dotenv +try: + load_dotenv() +except: + pass from api.common.config.auth import DEFAULT_PERMISSION, ALLOWED_EMAIL_DOMAINS from api.common.config.constants import ( EMAIL_REGEX, diff --git a/api/test/api/adapter/test_cognito_adapter.py b/api/test/api/adapter/test_cognito_adapter.py index 1385219..c469cbc 100644 --- a/api/test/api/adapter/test_cognito_adapter.py +++ b/api/test/api/adapter/test_cognito_adapter.py @@ -252,7 +252,6 @@ class TestCognitoAdapterUsers(BaseCognitoAdapter): os.environ, {"CUSTOM_USERNAME_REGEX": "[a-zA-Z][a-zA-Z0-9@._-]{2,127}"} ) def test_create_user(self): - CUSTOM_USERNAME_REGEX = "[a-zA-Z][a-zA-Z0-9@._-]{2,127}" cognito_response = { "User": { "Username": "user-name", From b4bde6496c137478cd1479b75d76a8919337ea15 Mon Sep 17 00:00:00 2001 From: mohit Date: Thu, 1 Feb 2024 17:18:07 +0000 Subject: [PATCH 07/24] fix PR comments --- .env.example | 2 +- api/api/common/config/auth.py | 2 - api/api/common/config/constants.py | 4 ++ api/api/domain/user.py | 13 ++-- api/batect.yml | 2 +- api/test/api/adapter/test_cognito_adapter.py | 67 ++++++++++++++----- api/test/api/domain/test_user_request.py | 14 ++-- infrastructure/modules/app-cluster/main.tf | 2 +- .../modules/app-cluster/variables.tf | 17 ++--- 9 files changed, 78 insertions(+), 45 deletions(-) diff --git a/.env.example b/.env.example index 121c31c..37002c5 100644 --- a/.env.example +++ b/.env.example @@ -8,7 +8,7 @@ RESOURCE_PREFIX=rapid DOMAIN_NAME=example.com COGNITO_USER_POOL_ID=11111111 LAYERS=raw,layer -CUSTOM_USERNAME_REGEX="regex_expression" +CUSTOM_USER_NAME_REGEX="regex_expression" # SDK Specific RAPID_CLIENT_ID= RAPID_CLIENT_SECRET= diff --git a/api/api/common/config/auth.py b/api/api/common/config/auth.py index 89685d9..c1f7aae 100644 --- a/api/api/common/config/auth.py +++ b/api/api/common/config/auth.py @@ -18,8 +18,6 @@ COGNITO_RESOURCE_SERVER_ID = f"https://{DOMAIN_NAME}" COGNITO_USER_POOL_ID = os.environ["COGNITO_USER_POOL_ID"] -CUSTOM_USERNAME_REGEX = os.environ.get("CUSTOM_USERNAME_REGEX") - ALLOWED_EMAIL_DOMAINS = os.environ["ALLOWED_EMAIL_DOMAINS"] IDENTITY_PROVIDER_BASE_URL = ( diff --git a/api/api/common/config/constants.py b/api/api/common/config/constants.py index 4534e60..bf0e4de 100644 --- a/api/api/common/config/constants.py +++ b/api/api/common/config/constants.py @@ -1,3 +1,5 @@ +import os + BASE_API_PATH = "/api" BASE_REGEX = "^[a-zA-Z0-9_-]" FILENAME_WITH_TIMESTAMP_REGEX = r"[a-zA-Z0-9:_\-]+.csv$" @@ -38,3 +40,5 @@ FIRST_SCHEMA_VERSION_NUMBER = 1 SCHEMA_VERSION_INCREMENT = 1 + +CUSTOM_USER_NAME_REGEX = os.environ.get("CUSTOM_USER_NAME_REGEX") diff --git a/api/api/domain/user.py b/api/api/domain/user.py index 71373ce..2f47e9f 100644 --- a/api/api/domain/user.py +++ b/api/api/domain/user.py @@ -12,6 +12,7 @@ from api.common.config.constants import ( EMAIL_REGEX, USERNAME_REGEX, + CUSTOM_USER_NAME_REGEX, ) from api.common.custom_exceptions import UserError @@ -27,12 +28,14 @@ def get_validated_username(self): https://docs.aws.amazon.com/cognito/latest/developerguide/limits.html """ if self.username is not None and re.fullmatch(USERNAME_REGEX, self.username): - custom_username_regex = os.environ.get("CUSTOM_USERNAME_REGEX") - if re.fullmatch(custom_username_regex, self.username): + if CUSTOM_USER_NAME_REGEX is None: return self.username - raise UserError( - "Your username does not match the requirements specified by your organisation" - ) + else: + if re.fullmatch(CUSTOM_USER_NAME_REGEX, self.username): + return self.username + raise UserError( + "Your username does not match the requirements specified by your organisation." + ) raise UserError( "This username is invalid. Please check the username and try again" ) diff --git a/api/batect.yml b/api/batect.yml index a3aeac6..5daa08f 100755 --- a/api/batect.yml +++ b/api/batect.yml @@ -23,7 +23,7 @@ containers: ALLOWED_EMAIL_DOMAINS: ${ALLOWED_EMAIL_DOMAINS:-} RESOURCE_PREFIX: ${RESOURCE_PREFIX:-prefix} LAYERS: ${LAYERS:-} - CUSTOM_USERNAME_REGEX: ${CUSTOM_USERNAME_REGEX:-} + CUSTOM_USER_NAME_REGEX: ${CUSTOM_USER_NAME_REGEX:-} tasks: runtime-environment: description: Build runtime environment diff --git a/api/test/api/adapter/test_cognito_adapter.py b/api/test/api/adapter/test_cognito_adapter.py index c469cbc..08e50ae 100644 --- a/api/test/api/adapter/test_cognito_adapter.py +++ b/api/test/api/adapter/test_cognito_adapter.py @@ -10,7 +10,7 @@ from api.common.config.aws import DOMAIN_NAME from api.common.custom_exceptions import AWSServiceError, UserError from api.domain.client import ClientRequest, ClientResponse -from api.domain.user import UserResponse, UserRequest +from api.domain.user import UserResponse, UserRequest, CUSTOM_USER_NAME_REGEX class BaseCognitoAdapter(ABC): @@ -248,10 +248,51 @@ def test_throws_error_when_client_app_name_has_not_been_changed_from_placeholder class TestCognitoAdapterUsers(BaseCognitoAdapter): - @mock.patch.dict( - os.environ, {"CUSTOM_USERNAME_REGEX": "[a-zA-Z][a-zA-Z0-9@._-]{2,127}"} - ) def test_create_user(self): + cognito_response = { + "User": { + "Username": "TtestUser1", + "Attributes": [ + {"Name": "sub", "Value": "some-uu-id-b226-e5fd18c59b85"}, + {"Name": "email_verified", "Value": "True"}, + {"Name": "email", "Value": "user-name@example1.com"}, + ], + }, + "ResponseMetadata": { + "RequestId": "the-request-id-b368-fae5cebb746f", + "HTTPStatusCode": 200, + }, + } + expected_response = UserResponse( + username="TtestUser1", + email="user-name@example1.com", + permissions=["WRITE_PUBLIC", "READ_PRIVATE"], + user_id="some-uu-id-b226-e5fd18c59b85", + ) + request = UserRequest( + username="TtestUser1", + email="TtestUser1@example1.com", + permissions=["WRITE_PUBLIC", "READ_PRIVATE"], + ) + self.cognito_boto_client.admin_create_user.return_value = cognito_response + + actual_response = self.cognito_adapter.create_user(request) + self.cognito_boto_client.admin_create_user.assert_called_once_with( + UserPoolId=COGNITO_USER_POOL_ID, + Username="TtestUser1", + UserAttributes=[ + {"Name": "email", "Value": "TtestUser1@example1.com"}, + {"Name": "email_verified", "Value": "True"}, + ], + DesiredDeliveryMediums=[ + "EMAIL", + ], + ) + + assert actual_response == expected_response + + @mock.patch("api.domain.user.CUSTOM_USER_NAME_REGEX", None) + def test_create_user_no_custom_regex(self): cognito_response = { "User": { "Username": "user-name", @@ -324,13 +365,10 @@ def test_delete_user_fails_when_user_does_not_exist(self): ): self.cognito_adapter.delete_user("my_user") - @mock.patch.dict( - os.environ, {"CUSTOM_USERNAME_REGEX": "[a-zA-Z][a-zA-Z0-9@._-]{2,127}"} - ) def test_create_user_fails_in_aws(self): request = UserRequest( - username="user-name", - email="user-name@example1.com", + username="TtestUser1", + email="TtestUser1@example1.com", permissions=["WRITE_PUBLIC", "READ_PRIVATE"], ) @@ -340,17 +378,14 @@ def test_create_user_fails_in_aws(self): ) with pytest.raises( - AWSServiceError, match="The user 'user-name' could not be created" + AWSServiceError, match="The user 'TtestUser1' could not be created" ): self.cognito_adapter.create_user(request) - @mock.patch.dict( - os.environ, {"CUSTOM_USERNAME_REGEX": "[a-zA-Z][a-zA-Z0-9@._-]{2,127}"} - ) def test_create_user_fails_when_the_user_already_exist(self): request = UserRequest( - username="user-name", - email="user-name@example1.com", + username="TtestUser1", + email="TtestUser1@example1.com", permissions=["WRITE_PUBLIC", "READ_PRIVATE"], ) @@ -361,7 +396,7 @@ def test_create_user_fails_when_the_user_already_exist(self): with pytest.raises( UserError, - match="The user 'user-name' or email 'user-name@example1.com' already exist", + match="The user 'TtestUser1' or email 'TtestUser1@example1.com' already exist", ): self.cognito_adapter.create_user(request) diff --git a/api/test/api/domain/test_user_request.py b/api/test/api/domain/test_user_request.py index 742559d..389ebfc 100644 --- a/api/test/api/domain/test_user_request.py +++ b/api/test/api/domain/test_user_request.py @@ -18,13 +18,13 @@ class TestUserRequest: "S1234", ], ) - @mock.patch.dict( - os.environ, {"CUSTOM_USERNAME_REGEX": "[a-zA-Z][a-zA-Z0-9@._-]{2,127}"} + @mock.patch( + "api.domain.user.CUSTOM_USER_NAME_REGEX", "[a-zA-Z][a-zA-Z0-9@._-]{2,127}" ) def test_get_validated_username(self, provided_username): request = UserRequest(username=provided_username, email="user@email.com") - # Overrwrite env variable on fn import + # Overwrite env variable on fn import try: validated_name = request.get_validated_username() assert validated_name == provided_username @@ -46,8 +46,8 @@ def test_get_validated_username(self, provided_username): "A" * 129, ], ) - @mock.patch.dict( - os.environ, {"CUSTOM_USERNAME_REGEX": "[a-zA-Z][a-zA-Z0-9@._-]{2,127}"} + @mock.patch( + "api.domain.user.CUSTOM_USER_NAME_REGEX", "[a-zA-Z][a-zA-Z0-9@._-]{2,127}" ) def test_raises_error_when_invalid_username(self, provided_username): request = UserRequest(username=provided_username, email="user@email.com") @@ -68,7 +68,7 @@ def test_raises_error_when_invalid_username(self, provided_username): "S1234", ], ) - @mock.patch.dict(os.environ, {"CUSTOM_USERNAME_REGEX": "^[A-Z][A-Za-z0-9]{3,50}$"}) + @mock.patch("api.domain.user.CUSTOM_USER_NAME_REGEX", "^[A-Z][A-Za-z0-9]{3,50}$") def test_get_validated_username_custom_regex(self, provided_username): request = UserRequest(username=provided_username, email="user@email.com") try: @@ -87,7 +87,7 @@ def test_get_validated_username_custom_regex(self, provided_username): "A....", ], ) - @mock.patch.dict(os.environ, {"CUSTOM_USERNAME_REGEX": "^[A-Z][A-Za-z0-9]{3,50}$"}) + @mock.patch("api.domain.user.CUSTOM_USER_NAME_REGEX", "^[A-Z][A-Za-z0-9]{3,50}$") def test_raises_error_when_invalid_username_custom_regex(self, provided_username): request = UserRequest(username=provided_username, email="user@email.com") diff --git a/infrastructure/modules/app-cluster/main.tf b/infrastructure/modules/app-cluster/main.tf index 4912850..6dccdc1 100644 --- a/infrastructure/modules/app-cluster/main.tf +++ b/infrastructure/modules/app-cluster/main.tf @@ -9,7 +9,7 @@ locals { "COGNITO_USER_POOL_ID" : var.cognito_user_pool_id, "RESOURCE_PREFIX" : var.resource-name-prefix, "COGNITO_USER_LOGIN_APP_CREDENTIALS_SECRETS_NAME" : var.cognito_user_login_app_credentials_secrets_name - "CUSTOM_USERNAME_REGEX" : var.custom_username_regex + "CUSTOM_USER_NAME_REGEX" : var.custom_user_name_regex }, var.project_information) } diff --git a/infrastructure/modules/app-cluster/variables.tf b/infrastructure/modules/app-cluster/variables.tf index 3162d27..c0e4d6f 100644 --- a/infrastructure/modules/app-cluster/variables.tf +++ b/infrastructure/modules/app-cluster/variables.tf @@ -203,16 +203,9 @@ variable "layers" { default = ["default"] } -variable "custom_username_regex" { - type = list(string) - description = "A list containing the regex expression for conditional user validation and a string to test against. The input must be a terraform list variable composed of two strings. The first string is the regex expression and the second string is the test string for said regular expression." - # Set default to a regex expression for basic api username validation - default = ["[a-zA-Z][a-zA-Z0-9@._-]{2,127}", "AUser1"] - - validation { - condition = can(regex(var.custom_username_regex[0], var.custom_username_regex[1])) - error_message = "Your regex expression cannot be evaluated against your test string. Please check the expression and the test string." - } +variable "custom_user_name_regex" { + type = string + description = "A regex expression for conditional user validation." + default = null + nullable = true } - - From e67dec8f0412a0a7a1eeb6c7a3f262faf571abe7 Mon Sep 17 00:00:00 2001 From: mohit Date: Thu, 1 Feb 2024 17:21:25 +0000 Subject: [PATCH 08/24] removing unneeded load_dotenv --- api/api/domain/user.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/api/api/domain/user.py b/api/api/domain/user.py index 2f47e9f..bbfa492 100644 --- a/api/api/domain/user.py +++ b/api/api/domain/user.py @@ -2,12 +2,7 @@ from typing import Optional, List import os from pydantic import BaseModel -from dotenv import load_dotenv -try: - load_dotenv() -except: - pass from api.common.config.auth import DEFAULT_PERMISSION, ALLOWED_EMAIL_DOMAINS from api.common.config.constants import ( EMAIL_REGEX, From 963eea42c3fb068ae888f6b6589c39ea4403d25d Mon Sep 17 00:00:00 2001 From: mohit Date: Thu, 1 Feb 2024 17:29:39 +0000 Subject: [PATCH 09/24] fixing tests, not sure why they fail --- api/test/api/controller/test_schema.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/api/test/api/controller/test_schema.py b/api/test/api/controller/test_schema.py index f979dc9..b7c586f 100644 --- a/api/test/api/controller/test_schema.py +++ b/api/test/api/controller/test_schema.py @@ -61,10 +61,10 @@ def test_return_400_pydantic_error(self): assert response.status_code == 400 assert response.json() == { "details": [ - "layer -> Field required", - "domain -> Field required", - "dataset -> Field required", - "sensitivity -> Field required", + "metadata: layer -> Field required", + "metadata: domain -> Field required", + "metadata: dataset -> Field required", + "metadata: sensitivity -> Field required", ] } @@ -232,10 +232,10 @@ def test_return_400_when_request_body_invalid(self): assert response.status_code == 400 assert response.json() == { "details": [ - "layer -> Field required", - "domain -> Field required", - "dataset -> Field required", - "sensitivity -> Field required", + "metadata: layer -> Field required", + "metadata: domain -> Field required", + "metadata: dataset -> Field required", + "metadata: sensitivity -> Field required", ] } From 3796530edfa00e88ac4f170901218124c7197ffb Mon Sep 17 00:00:00 2001 From: mohit Date: Thu, 1 Feb 2024 17:32:11 +0000 Subject: [PATCH 10/24] remove unneeded packages --- api/api/domain/user.py | 1 - api/test/api/adapter/test_cognito_adapter.py | 3 +-- api/test/api/domain/test_user_request.py | 1 - 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/api/api/domain/user.py b/api/api/domain/user.py index bbfa492..f212c2b 100644 --- a/api/api/domain/user.py +++ b/api/api/domain/user.py @@ -1,6 +1,5 @@ import re from typing import Optional, List -import os from pydantic import BaseModel from api.common.config.auth import DEFAULT_PERMISSION, ALLOWED_EMAIL_DOMAINS diff --git a/api/test/api/adapter/test_cognito_adapter.py b/api/test/api/adapter/test_cognito_adapter.py index 08e50ae..4e94d25 100644 --- a/api/test/api/adapter/test_cognito_adapter.py +++ b/api/test/api/adapter/test_cognito_adapter.py @@ -3,14 +3,13 @@ from unittest import mock import pytest from botocore.exceptions import ClientError -import os from api.adapter.cognito_adapter import CognitoAdapter from api.common.config.auth import COGNITO_USER_POOL_ID, COGNITO_RESOURCE_SERVER_ID from api.common.config.aws import DOMAIN_NAME from api.common.custom_exceptions import AWSServiceError, UserError from api.domain.client import ClientRequest, ClientResponse -from api.domain.user import UserResponse, UserRequest, CUSTOM_USER_NAME_REGEX +from api.domain.user import UserResponse, UserRequest class BaseCognitoAdapter(ABC): diff --git a/api/test/api/domain/test_user_request.py b/api/test/api/domain/test_user_request.py index 389ebfc..d5bdbb3 100644 --- a/api/test/api/domain/test_user_request.py +++ b/api/test/api/domain/test_user_request.py @@ -3,7 +3,6 @@ from api.common.custom_exceptions import UserError from api.domain.user import UserRequest from unittest import mock -import os class TestUserRequest: From a3c337ed511f88edab88c37b1390c9a23a075bde Mon Sep 17 00:00:00 2001 From: mohit Date: Thu, 1 Feb 2024 22:11:53 +0000 Subject: [PATCH 11/24] add regex expression to env file --- .github/.github.env | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/.github.env b/.github/.github.env index ad8c623..d477b37 100644 --- a/.github/.github.env +++ b/.github/.github.env @@ -4,3 +4,4 @@ ALLOWED_EMAIL_DOMAINS=example1.com,example2.com LAYERS=raw,layer DOMAIN_NAME=example.com DATA_BUCKET=the-bucket +CUSTOM_USER_REGEX=^[A-Z][A-Za-z0-9]{3,50}$ \ No newline at end of file From 0b344021b17cc87cf29b835c5beea693760f1a8b Mon Sep 17 00:00:00 2001 From: mohit Date: Thu, 1 Feb 2024 22:28:31 +0000 Subject: [PATCH 12/24] fix --- .github/.github.env | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/.github.env b/.github/.github.env index d477b37..474ef06 100644 --- a/.github/.github.env +++ b/.github/.github.env @@ -4,4 +4,4 @@ ALLOWED_EMAIL_DOMAINS=example1.com,example2.com LAYERS=raw,layer DOMAIN_NAME=example.com DATA_BUCKET=the-bucket -CUSTOM_USER_REGEX=^[A-Z][A-Za-z0-9]{3,50}$ \ No newline at end of file +CUSTOM_USER_NAME_REGEX=^[A-Z][A-Za-z0-9]{3,50}$ \ No newline at end of file From ac4586fff61105f921969a8f1b46f6cbbffc38f0 Mon Sep 17 00:00:00 2001 From: mohit Date: Fri, 2 Feb 2024 09:22:57 +0000 Subject: [PATCH 13/24] quick check --- .github/.github.env | 2 +- api/api/common/config/constants.py | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/.github.env b/.github/.github.env index 474ef06..aa08094 100644 --- a/.github/.github.env +++ b/.github/.github.env @@ -4,4 +4,4 @@ ALLOWED_EMAIL_DOMAINS=example1.com,example2.com LAYERS=raw,layer DOMAIN_NAME=example.com DATA_BUCKET=the-bucket -CUSTOM_USER_NAME_REGEX=^[A-Z][A-Za-z0-9]{3,50}$ \ No newline at end of file +CUSTOM_USER_NAME_REGEX=[a-zA-Z][a-zA-Z0-9@._-]{2,127} \ No newline at end of file diff --git a/api/api/common/config/constants.py b/api/api/common/config/constants.py index bf0e4de..ff7b230 100644 --- a/api/api/common/config/constants.py +++ b/api/api/common/config/constants.py @@ -41,4 +41,7 @@ FIRST_SCHEMA_VERSION_NUMBER = 1 SCHEMA_VERSION_INCREMENT = 1 -CUSTOM_USER_NAME_REGEX = os.environ.get("CUSTOM_USER_NAME_REGEX") +CUSTOM_USER_NAME_REGEX_ENVIRON = os.environ.get("CUSTOM_USER_NAME_REGEX") +CUSTOM_USER_NAME_REGEX = ( + "" if len(CUSTOM_USER_NAME_REGEX_ENVIRON) == 0 else CUSTOM_USER_NAME_REGEX_ENVIRON +) From 3b62f18f65b61845bbe240b941428afa051473ea Mon Sep 17 00:00:00 2001 From: mohit Date: Fri, 2 Feb 2024 09:28:57 +0000 Subject: [PATCH 14/24] Trying to see regex env var in action context --- .github/workflows/dev.yml | 140 +++++++++++++++++++------------------- api/api/domain/user.py | 1 + 2 files changed, 71 insertions(+), 70 deletions(-) diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index a12d546..4d0c79b 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -17,26 +17,26 @@ jobs: - name: Log commit SHA run: echo $GITHUB_SHA - security-check: - needs: - - setup - runs-on: self-hosted - steps: - - name: Setup Python - uses: actions/setup-python@v4 - with: - python-version: '3.10' - cache: 'pip' - - - run: pip install -r requirements.txt - - - name: Run security checks - run: make security-check + # security-check: + # needs: + # - setup + # runs-on: self-hosted + # steps: + # - name: Setup Python + # uses: actions/setup-python@v4 + # with: + # python-version: '3.10' + # cache: 'pip' + + # - run: pip install -r requirements.txt + + # - name: Run security checks + # run: make security-check api-dev: needs: - setup - - security-check + # - security-check runs-on: self-hosted steps: - name: Checkout @@ -61,66 +61,66 @@ jobs: - name: API Tag and Upload run: make api-tag-and-upload - sdk-dev: - needs: - - setup - - security-check - runs-on: self-hosted - steps: - - name: Checkout - uses: actions/checkout@v3 - - - name: Populate .env with additional vars - run: | - echo "TWINE_USERNAME=${{ secrets.TWINE_USERNAME_TEST }}" >> .env - echo "TWINE_PASSWORD=${{ secrets.TWINE_PASSWORD_TEST }}" >> .env - echo "TWINE_NON_INTERACTIVE=true" >> .env - - - name: Setup Python - uses: actions/setup-python@v4 - with: - python-version: '3.10' - cache: 'pip' - - - name: Setup Python Environment - run: | - make sdk-setup - source sdk/.venv/bin/activate - - - name: SDK Test - run: make sdk-test - - # TODO: Add back in - # - name: SDK Test Deploy - # run: make sdk-release-test - - ui-dev: - needs: - - setup - - security-check - runs-on: self-hosted - steps: - - name: Checkout - uses: actions/checkout@v3 - - - name: Setup Node - uses: actions/setup-node@v3 - with: - node-version: 19 - - - name: Install UI Packages - run: make ui-setup - - - name: UI Test - run: make ui-test + # sdk-dev: + # needs: + # - setup + # - security-check + # runs-on: self-hosted + # steps: + # - name: Checkout + # uses: actions/checkout@v3 + + # - name: Populate .env with additional vars + # run: | + # echo "TWINE_USERNAME=${{ secrets.TWINE_USERNAME_TEST }}" >> .env + # echo "TWINE_PASSWORD=${{ secrets.TWINE_PASSWORD_TEST }}" >> .env + # echo "TWINE_NON_INTERACTIVE=true" >> .env + + # - name: Setup Python + # uses: actions/setup-python@v4 + # with: + # python-version: '3.10' + # cache: 'pip' + + # - name: Setup Python Environment + # run: | + # make sdk-setup + # source sdk/.venv/bin/activate + + # - name: SDK Test + # run: make sdk-test + + # # TODO: Add back in + # # - name: SDK Test Deploy + # # run: make sdk-release-test + + # ui-dev: + # needs: + # - setup + # - security-check + # runs-on: self-hosted + # steps: + # - name: Checkout + # uses: actions/checkout@v3 + + # - name: Setup Node + # uses: actions/setup-node@v3 + # with: + # node-version: 19 + + # - name: Install UI Packages + # run: make ui-setup + + # - name: UI Test + # run: make ui-test cleanup: needs: - setup - security-check - api-dev - - sdk-dev - - ui-dev + # - sdk-dev + # - ui-dev runs-on: self-hosted steps: - name: Checkout diff --git a/api/api/domain/user.py b/api/api/domain/user.py index f212c2b..a012240 100644 --- a/api/api/domain/user.py +++ b/api/api/domain/user.py @@ -29,6 +29,7 @@ def get_validated_username(self): return self.username raise UserError( "Your username does not match the requirements specified by your organisation." + + CUSTOM_USER_NAME_REGEX ) raise UserError( "This username is invalid. Please check the username and try again" From 873f4d7939dd508596655eec7b1a9c4254c5847f Mon Sep 17 00:00:00 2001 From: mohit Date: Fri, 2 Feb 2024 09:29:29 +0000 Subject: [PATCH 15/24] fixing typo --- api/api/common/config/constants.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/api/common/config/constants.py b/api/api/common/config/constants.py index ff7b230..ee7f694 100644 --- a/api/api/common/config/constants.py +++ b/api/api/common/config/constants.py @@ -43,5 +43,5 @@ CUSTOM_USER_NAME_REGEX_ENVIRON = os.environ.get("CUSTOM_USER_NAME_REGEX") CUSTOM_USER_NAME_REGEX = ( - "" if len(CUSTOM_USER_NAME_REGEX_ENVIRON) == 0 else CUSTOM_USER_NAME_REGEX_ENVIRON + None if len(CUSTOM_USER_NAME_REGEX_ENVIRON) == 0 else CUSTOM_USER_NAME_REGEX_ENVIRON ) From 19aea89db35cfd261c9f0af82d47565e7680a224 Mon Sep 17 00:00:00 2001 From: mohit Date: Fri, 2 Feb 2024 09:30:17 +0000 Subject: [PATCH 16/24] fix dev.yml --- .github/workflows/dev.yml | 141 +++++++++++++++++++------------------- 1 file changed, 71 insertions(+), 70 deletions(-) diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index 4d0c79b..e895b0b 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -17,26 +17,26 @@ jobs: - name: Log commit SHA run: echo $GITHUB_SHA - # security-check: - # needs: - # - setup - # runs-on: self-hosted - # steps: - # - name: Setup Python - # uses: actions/setup-python@v4 - # with: - # python-version: '3.10' - # cache: 'pip' - - # - run: pip install -r requirements.txt - - # - name: Run security checks - # run: make security-check + security-check: + needs: + - setup + runs-on: self-hosted + steps: + - name: Setup Python + uses: actions/setup-python@v4 + with: + python-version: '3.10' + cache: 'pip' + + - run: pip install -r requirements.txt + + - name: Run security checks + run: make security-check api-dev: needs: - setup - # - security-check + - security-check runs-on: self-hosted steps: - name: Checkout @@ -48,6 +48,7 @@ jobs: echo AWS_ACCOUNT=${{ secrets.AWS_ACCOUNT }} >> .env echo AWS_REGION=${{ secrets.AWS_REGION }} >> .env echo AWS_DEFAULT_REGION=${{ secrets.AWS_REGION }} >> .env + echo CUSTOM_USER_NAME_REGEX=[a-zA-Z][a-zA-Z0-9@._-]{2,127} >> .env - name: Build API Image run: make api-create-image @@ -61,66 +62,66 @@ jobs: - name: API Tag and Upload run: make api-tag-and-upload - # sdk-dev: - # needs: - # - setup - # - security-check - # runs-on: self-hosted - # steps: - # - name: Checkout - # uses: actions/checkout@v3 - - # - name: Populate .env with additional vars - # run: | - # echo "TWINE_USERNAME=${{ secrets.TWINE_USERNAME_TEST }}" >> .env - # echo "TWINE_PASSWORD=${{ secrets.TWINE_PASSWORD_TEST }}" >> .env - # echo "TWINE_NON_INTERACTIVE=true" >> .env - - # - name: Setup Python - # uses: actions/setup-python@v4 - # with: - # python-version: '3.10' - # cache: 'pip' - - # - name: Setup Python Environment - # run: | - # make sdk-setup - # source sdk/.venv/bin/activate - - # - name: SDK Test - # run: make sdk-test - - # # TODO: Add back in - # # - name: SDK Test Deploy - # # run: make sdk-release-test - - # ui-dev: - # needs: - # - setup - # - security-check - # runs-on: self-hosted - # steps: - # - name: Checkout - # uses: actions/checkout@v3 - - # - name: Setup Node - # uses: actions/setup-node@v3 - # with: - # node-version: 19 - - # - name: Install UI Packages - # run: make ui-setup - - # - name: UI Test - # run: make ui-test + sdk-dev: + needs: + - setup + - security-check + runs-on: self-hosted + steps: + - name: Checkout + uses: actions/checkout@v3 + + - name: Populate .env with additional vars + run: | + echo "TWINE_USERNAME=${{ secrets.TWINE_USERNAME_TEST }}" >> .env + echo "TWINE_PASSWORD=${{ secrets.TWINE_PASSWORD_TEST }}" >> .env + echo "TWINE_NON_INTERACTIVE=true" >> .env + + - name: Setup Python + uses: actions/setup-python@v4 + with: + python-version: '3.10' + cache: 'pip' + + - name: Setup Python Environment + run: | + make sdk-setup + source sdk/.venv/bin/activate + + - name: SDK Test + run: make sdk-test + + # TODO: Add back in + # - name: SDK Test Deploy + # run: make sdk-release-test + + ui-dev: + needs: + - setup + - security-check + runs-on: self-hosted + steps: + - name: Checkout + uses: actions/checkout@v3 + + - name: Setup Node + uses: actions/setup-node@v3 + with: + node-version: 19 + + - name: Install UI Packages + run: make ui-setup + + - name: UI Test + run: make ui-test cleanup: needs: - setup - security-check - api-dev - # - sdk-dev - # - ui-dev + - sdk-dev + - ui-dev runs-on: self-hosted steps: - name: Checkout From 3792323d2e08633ff2f70eb5930f6ef7c1f24a01 Mon Sep 17 00:00:00 2001 From: mohit Date: Fri, 2 Feb 2024 09:38:52 +0000 Subject: [PATCH 17/24] try again --- .github/workflows/dev.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index e895b0b..a12d546 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -48,7 +48,6 @@ jobs: echo AWS_ACCOUNT=${{ secrets.AWS_ACCOUNT }} >> .env echo AWS_REGION=${{ secrets.AWS_REGION }} >> .env echo AWS_DEFAULT_REGION=${{ secrets.AWS_REGION }} >> .env - echo CUSTOM_USER_NAME_REGEX=[a-zA-Z][a-zA-Z0-9@._-]{2,127} >> .env - name: Build API Image run: make api-create-image From 563babdb421413ff8e520d543f6fc543b5225cbc Mon Sep 17 00:00:00 2001 From: mohit Date: Fri, 2 Feb 2024 09:51:20 +0000 Subject: [PATCH 18/24] fix --- .github/.github.env | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/.github.env b/.github/.github.env index aa08094..507b6eb 100644 --- a/.github/.github.env +++ b/.github/.github.env @@ -3,5 +3,5 @@ RESOURCE_PREFIX=rapid ALLOWED_EMAIL_DOMAINS=example1.com,example2.com LAYERS=raw,layer DOMAIN_NAME=example.com +CUSTOM_USER_NAME_REGEX=[a-zA-Z][a-zA-Z0-9@._-]{2,127} DATA_BUCKET=the-bucket -CUSTOM_USER_NAME_REGEX=[a-zA-Z][a-zA-Z0-9@._-]{2,127} \ No newline at end of file From d51d9e89afcbd2fe5920be1b81b7a83f4f16bc75 Mon Sep 17 00:00:00 2001 From: mohit Date: Fri, 2 Feb 2024 09:59:50 +0000 Subject: [PATCH 19/24] test yml --- .github/workflows/dev.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index a12d546..1f60d00 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -45,9 +45,9 @@ jobs: - name: Populate .env with additional vars run: | cp ./.github/.github.env .env - echo AWS_ACCOUNT=${{ secrets.AWS_ACCOUNT }} >> .env - echo AWS_REGION=${{ secrets.AWS_REGION }} >> .env - echo AWS_DEFAULT_REGION=${{ secrets.AWS_REGION }} >> .env + echo \AWS_ACCOUNT=${{ secrets.AWS_ACCOUNT }} >> .env + echo \AWS_REGION=${{ secrets.AWS_REGION }} >> .env + echo \AWS_DEFAULT_REGION=${{ secrets.AWS_REGION }} >> .env - name: Build API Image run: make api-create-image From 8f32c4f9bc934efedd582e4f306af6e3262ef2a0 Mon Sep 17 00:00:00 2001 From: mohit Date: Fri, 2 Feb 2024 10:15:37 +0000 Subject: [PATCH 20/24] amend blocks --- docs/infrastructure/deployment.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/infrastructure/deployment.md b/docs/infrastructure/deployment.md index 01143ea..59f4640 100644 --- a/docs/infrastructure/deployment.md +++ b/docs/infrastructure/deployment.md @@ -56,7 +56,7 @@ There are also these optional inputs: - `catalog_disabled` - if set to `true` it will disable the rAPId internal data catalogue - `tags` - if provided, it will tag the resources with the defined value. Otherwise, it will default to "Resource = ' data-f1-rapid'" - +- `custom_user_name_regex` - Regex to allow custom filtering of usernames supplied when creating a new user. Defaults to null, in which case rAPId will default to its basic username validity checks. Once you apply the Terraform, a new instance of the application should be created. ## rAPId Full Stack From e870159acb0bf652f89d80fc9644ddbc72112090 Mon Sep 17 00:00:00 2001 From: mohit Date: Fri, 2 Feb 2024 10:17:29 +0000 Subject: [PATCH 21/24] forgot to amend rapid module --- infrastructure/modules/rapid/main.tf | 1 + infrastructure/modules/rapid/variables.tf | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/infrastructure/modules/rapid/main.tf b/infrastructure/modules/rapid/main.tf index 1a57b1e..35ee074 100644 --- a/infrastructure/modules/rapid/main.tf +++ b/infrastructure/modules/rapid/main.tf @@ -31,6 +31,7 @@ module "app_cluster" { ecs_cluster_arn = var.ecs_cluster_arn ecs_cluster_name = var.ecs_cluster_name ecs_cluster_id = var.ecs_cluster_id + custom_user_name_regex = var.custom_user_name_regex } module "auth" { diff --git a/infrastructure/modules/rapid/variables.tf b/infrastructure/modules/rapid/variables.tf index ea04014..c7975b0 100644 --- a/infrastructure/modules/rapid/variables.tf +++ b/infrastructure/modules/rapid/variables.tf @@ -165,3 +165,11 @@ variable "layers" { description = "A list of the layers that the rAPId instance will contain" default = ["default"] } + + +variable "custom_user_name_regex" { + type = string + description = "A regex expression for conditional user validation." + default = null + nullable = true +} From 2aed5f3e06fbf595b5a0f39d9ad82807fabb1c82 Mon Sep 17 00:00:00 2001 From: mohit Date: Mon, 5 Feb 2024 09:31:15 +0000 Subject: [PATCH 22/24] quick code comment responses --- api/api/common/config/constants.py | 5 +---- docs/infrastructure/deployment.md | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/api/api/common/config/constants.py b/api/api/common/config/constants.py index ee7f694..ebd97ce 100644 --- a/api/api/common/config/constants.py +++ b/api/api/common/config/constants.py @@ -41,7 +41,4 @@ FIRST_SCHEMA_VERSION_NUMBER = 1 SCHEMA_VERSION_INCREMENT = 1 -CUSTOM_USER_NAME_REGEX_ENVIRON = os.environ.get("CUSTOM_USER_NAME_REGEX") -CUSTOM_USER_NAME_REGEX = ( - None if len(CUSTOM_USER_NAME_REGEX_ENVIRON) == 0 else CUSTOM_USER_NAME_REGEX_ENVIRON -) +CUSTOM_USER_NAME_REGEX_ENVIRON = os.getenv("CUSTOM_USER_NAME_REGEX") diff --git a/docs/infrastructure/deployment.md b/docs/infrastructure/deployment.md index 59f4640..0ed3294 100644 --- a/docs/infrastructure/deployment.md +++ b/docs/infrastructure/deployment.md @@ -56,7 +56,7 @@ There are also these optional inputs: - `catalog_disabled` - if set to `true` it will disable the rAPId internal data catalogue - `tags` - if provided, it will tag the resources with the defined value. Otherwise, it will default to "Resource = ' data-f1-rapid'" -- `custom_user_name_regex` - Regex to allow custom filtering of usernames supplied when creating a new user. Defaults to null, in which case rAPId will default to its basic username validity checks. +- `custom_user_name_regex` - Regex that when supplied usernames must conform to when creating a new user. Defaults to none, in which case rAPId will default to it's basic username validity checks. Once you apply the Terraform, a new instance of the application should be created. ## rAPId Full Stack From c2b7452da1a54184fd682dbf98901cc0bec6504b Mon Sep 17 00:00:00 2001 From: mohit Date: Mon, 5 Feb 2024 09:51:44 +0000 Subject: [PATCH 23/24] wrong name for regex var, fix --- api/api/common/config/constants.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/api/common/config/constants.py b/api/api/common/config/constants.py index ebd97ce..33d38e2 100644 --- a/api/api/common/config/constants.py +++ b/api/api/common/config/constants.py @@ -41,4 +41,4 @@ FIRST_SCHEMA_VERSION_NUMBER = 1 SCHEMA_VERSION_INCREMENT = 1 -CUSTOM_USER_NAME_REGEX_ENVIRON = os.getenv("CUSTOM_USER_NAME_REGEX") +CUSTOM_USER_NAME_REGEX = os.getenv("CUSTOM_USER_NAME_REGEX") From 4c45df521e68fed5ddc5030a331c219177236f1d Mon Sep 17 00:00:00 2001 From: mohit Date: Mon, 5 Feb 2024 12:54:30 +0000 Subject: [PATCH 24/24] trying without slashes --- .github/.github.env | 2 +- .github/workflows/dev.yml | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/.github.env b/.github/.github.env index 507b6eb..5702873 100644 --- a/.github/.github.env +++ b/.github/.github.env @@ -3,5 +3,5 @@ RESOURCE_PREFIX=rapid ALLOWED_EMAIL_DOMAINS=example1.com,example2.com LAYERS=raw,layer DOMAIN_NAME=example.com -CUSTOM_USER_NAME_REGEX=[a-zA-Z][a-zA-Z0-9@._-]{2,127} DATA_BUCKET=the-bucket +CUSTOM_USER_NAME_REGEX=[a-zA-Z][a-zA-Z0-9@._-]{2,127} diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index 1f60d00..a12d546 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -45,9 +45,9 @@ jobs: - name: Populate .env with additional vars run: | cp ./.github/.github.env .env - echo \AWS_ACCOUNT=${{ secrets.AWS_ACCOUNT }} >> .env - echo \AWS_REGION=${{ secrets.AWS_REGION }} >> .env - echo \AWS_DEFAULT_REGION=${{ secrets.AWS_REGION }} >> .env + echo AWS_ACCOUNT=${{ secrets.AWS_ACCOUNT }} >> .env + echo AWS_REGION=${{ secrets.AWS_REGION }} >> .env + echo AWS_DEFAULT_REGION=${{ secrets.AWS_REGION }} >> .env - name: Build API Image run: make api-create-image