-
Notifications
You must be signed in to change notification settings - Fork 1
Add password validation for email auth #265
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,10 +31,10 @@ test: | |
| source ./venv/bin/activate && python3 -m pytest --verbosity=2 --showlocals --log-level=DEBUG | ||
|
|
||
| create-user: | ||
| python -m auth_backend user create --email test-user@profcomff.com --password string | ||
| python -m auth_backend user create --email test-user@profcomff.com --password string12 | ||
|
|
||
| create-admin: | ||
| source ./venv/bin/activate && python -m auth_backend user create --email test-admin@profcomff.com --password string | ||
| source ./venv/bin/activate && python -m auth_backend user create --email test-admin@profcomff.com --password string12 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. здесь соответственно тоже |
||
| source ./venv/bin/activate && python -m auth_backend scope create --name auth.group.create --comment auth.group.create --creator_email test-admin@profcomff.com | ||
| source ./venv/bin/activate && python -m auth_backend scope create --name auth.group.delete --comment auth.group.delete --creator_email test-admin@profcomff.com | ||
| source ./venv/bin/activate && python -m auth_backend scope create --name auth.group.read --comment auth.group.read --creator_email test-admin@profcomff.com | ||
|
|
@@ -61,7 +61,7 @@ create-admin: | |
| source ./venv/bin/activate && python -m auth_backend user_group create --email test-admin@profcomff.com | ||
|
|
||
| login-user: | ||
| curl -X 'POST' 'http://localhost:8000/email/login' -H 'accept: application/json' -H 'Content-Type: application/json' -d '{"email": "test-user@profcomff.com", "password": "string"}' | ||
| curl -X 'POST' 'http://localhost:8000/email/login' -H 'accept: application/json' -H 'Content-Type: application/json' -d '{"email": "test-user@profcomff.com", "password": "string12"}' | ||
|
|
||
| login-admin: | ||
| curl -X 'POST' 'http://localhost:8000/email/login' -H 'accept: application/json' -H 'Content-Type: application/json' -d '{"email": "test-admin@profcomff.com", "password": "string"}' | ||
| curl -X 'POST' 'http://localhost:8000/email/login' -H 'accept: application/json' -H 'Content-Type: application/json' -d '{"email": "test-admin@profcomff.com", "password": "string12"}' | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -96,13 +96,21 @@ foo@bar:~$ python -m auth_backend start | |
|
|
||
| ## Сценарий использования | ||
| ### Email: регистрация нового аккаунта | ||
| 1. Дернуть ручку `POST /email/registrate` . Вы передаете | ||
| 1. Дернуть ручку `POST /email/registration`. Вы передаете | ||
| ```json | ||
| { | ||
| "email": "string", // Почта | ||
| "password": "string" // Пароль | ||
| "email": "user@example.com", | ||
| "password": "Password1!" | ||
| } | ||
| ``` | ||
|
|
||
| Требования к новому паролю: | ||
| - длина от 8 до 32 символов; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. от min до max
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. от min до max |
||
| - разрешены латинские буквы `A-Z`, `a-z`, цифры `0-9` и стандартные ASCII-спецсимволы; | ||
| - пробелы, управляющие символы и символы вне ASCII (например, кириллица) запрещены. | ||
|
|
||
| Эти же требования применяются при смене и восстановлении пароля. При нарушении требований API возвращает `422 Unprocessable Entity`. | ||
|
|
||
| 3. На почту приходит письмо с линком на `GET /email/approve?token='...'`, если по ней перейти то почта будет подтверждена и регистрацию можно считать завершенной. | ||
|
|
||
| ### Email: вход в аккаунт | ||
|
|
@@ -118,16 +126,16 @@ foo@bar:~$ python -m auth_backend start | |
| 3. Вам придет письмо, где будет ссылка НА ФРОНТ(надо сделать это), в ссылке будет reset_token | ||
| 4. Токен надо передать в ручку `POST /email/reset/password` в заголовках, вместе с | ||
| ```json | ||
| {"new_password": ""} | ||
| {"new_password": "NewPassword1!"} | ||
| ``` | ||
| и пароль будет изменен | ||
|
|
||
| ### Email: Изменение пароля | ||
| 1. Если пароль не забыт, а просто надо его поменять. Тогда в `POST /email/reset/password/request` передается токен авторизации, в теле вы передаете | ||
| ```json | ||
| { | ||
| "password": "string", // старый пароль | ||
| "new_password": "string" // новый пароль | ||
| "password": "CurrentPassword1!", // старый пароль | ||
| "new_password": "NewPassword1!" // новый пароль | ||
| } | ||
| ``` | ||
| 3. Отправляете запрос и всё, пароль изменен, вам придет письмо с уведомлением о смене пароляю | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,10 +4,12 @@ | |
|
|
||
| from auth_backend.auth_plugins import Email | ||
| from auth_backend.models import AuthMethod, User | ||
| from auth_backend.schemas.types.password import validate_password | ||
| from auth_backend.utils.string import random_string | ||
|
|
||
|
|
||
| def create_user(email: str, password: str, session: Session) -> None: | ||
| password = validate_password(password) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. здесь можно без присваивания, ты же в validate_password никак не меняешь пароль, а всего лишь проверяешь
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. плюс в коде ниже есть проверка с print и exit |
||
| if ( | ||
| AuthMethod.query(session=session) | ||
| .filter(AuthMethod.value == email, AuthMethod.auth_method == "email") | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| import string | ||
| from typing import Any | ||
|
|
||
| from pydantic import GetCoreSchemaHandler, GetJsonSchemaHandler | ||
| from pydantic.json_schema import JsonSchemaValue | ||
| from pydantic_core import core_schema | ||
|
|
||
| PASSWORD_MIN_LENGTH = 8 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. надо эту переменную перенести в settings и здесь ее просто оттуда импортировать также, когда перенесешь это в settings надо еще в github/workflows в test и prod прописать аналогичные команды как здесь https://github.com/profcomff/auth-api/blob/main/.github/workflows/build_and_publish.yml#L147 |
||
| PASSWORD_MAX_LENGTH = 32 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. с этой переменной аналогично
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ниже переменные (с 10 строчки) оставить здесь, их нет смысла переносить в settings |
||
| PASSWORD_ALLOWED_CHARACTERS = string.ascii_letters + string.digits + string.punctuation | ||
| PASSWORD_PATTERN = r"^[\x21-\x7E]+$" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. а зачем эта константа если есть PASSWORD_ALLOWED_CHARACTERS
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ты ее в валидации используешь, ее же и используй в классе Password, не надо плодить константы одинаковые по сути |
||
| PASSWORD_REQUIREMENTS = ( | ||
| f"Password must be {PASSWORD_MIN_LENGTH}-{PASSWORD_MAX_LENGTH} characters long and contain only " | ||
| "ASCII letters, digits and punctuation. Spaces and non-ASCII characters are not allowed." | ||
| ) | ||
|
|
||
|
|
||
| def validate_password(value: str) -> str: | ||
| """Validate a newly created password according to the Auth API password policy.""" | ||
| if len(value) < PASSWORD_MIN_LENGTH: | ||
| raise ValueError(f"Password must be at least {PASSWORD_MIN_LENGTH} characters long") | ||
| if len(value) > PASSWORD_MAX_LENGTH: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. здесь лучше elif для более быстрой работы, если len(value)<min то больше max точно не будет |
||
| raise ValueError(f"Password must be at most {PASSWORD_MAX_LENGTH} characters long") | ||
| if any(character not in PASSWORD_ALLOWED_CHARACTERS for character in value): | ||
| raise ValueError( | ||
| "Password may contain only ASCII letters, digits and punctuation; " | ||
| "spaces and non-ASCII characters are not allowed" | ||
| ) | ||
| return value | ||
|
|
||
|
|
||
| class Password: | ||
| """Pydantic type for a password that is being created or replaced.""" | ||
|
|
||
| @classmethod | ||
| def __get_pydantic_core_schema__( | ||
| cls, | ||
| source: type[Any], | ||
| handler: GetCoreSchemaHandler, | ||
| ) -> core_schema.CoreSchema: | ||
| return core_schema.no_info_after_validator_function(validate_password, core_schema.str_schema()) | ||
|
|
||
| @classmethod | ||
| def __get_pydantic_json_schema__( | ||
| cls, core_schema_: core_schema.CoreSchema, handler: GetJsonSchemaHandler | ||
| ) -> JsonSchemaValue: | ||
| field_schema = handler(core_schema_) | ||
| field_schema.update( | ||
| type="string", | ||
| format="password", | ||
| minLength=PASSWORD_MIN_LENGTH, | ||
| maxLength=PASSWORD_MAX_LENGTH, | ||
| pattern=PASSWORD_PATTERN, | ||
| description=PASSWORD_REQUIREMENTS, | ||
| ) | ||
| return field_schema | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -58,7 +58,7 @@ def dbsession(): | |
| @pytest.fixture() | ||
| def user_id(client_auth: TestClient, dbsession): | ||
| time = datetime.datetime.utcnow() | ||
| body = {"email": f"user{time}@example.com", "password": "string"} | ||
| body = {"email": f"user{time}@example.com", "password": "string12"} | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. я бы лучше что-то более существенное прописал сюда, например "test-password" везде ниже тоже |
||
| client_auth.post("/email/registration", json=body) | ||
| db_user: AuthMethod = ( | ||
| dbsession.query(AuthMethod).filter(AuthMethod.value == body['email'], AuthMethod.param == 'email').one() | ||
|
|
@@ -78,7 +78,7 @@ def user_id(client_auth: TestClient, dbsession): | |
| def user(client_auth: TestClient, dbsession): | ||
| url = "/email/login" | ||
| time = datetime.datetime.utcnow() | ||
| body = {"email": f"user{time}@example.com", "password": "string", "scopes": []} | ||
| body = {"email": f"user{time}@example.com", "password": "string12", "scopes": []} | ||
| response = client_auth.post("/email/registration", json=body) | ||
| db_user: AuthMethod = ( | ||
| dbsession.query(AuthMethod).filter(AuthMethod.value == body['email'], AuthMethod.param == 'email').one() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -62,14 +62,14 @@ def test_unprocessable_jsons_with_token(client_auth: TestClient, dbsession: Sess | |
| response = client_auth.post( | ||
| f"{url}/request", | ||
| headers={"Authorization": auth_token}, | ||
| json={"password": "", "new_password": "changed"}, | ||
| json={"password": "", "new_password": "changed12"}, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. здесь какой нибудь "changed-pass" |
||
| ) | ||
| assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY | ||
|
|
||
| response = client_auth.post( | ||
| f"{url}/request", | ||
| headers={"Authorization": auth_token}, | ||
| json={"password": "", "new_password": "changed"}, | ||
| json={"password": "", "new_password": "changed12"}, | ||
| ) | ||
| assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY | ||
|
|
||
|
|
@@ -83,7 +83,21 @@ def test_unprocessable_jsons_with_token(client_auth: TestClient, dbsession: Sess | |
| response = client_auth.post( | ||
| f"{url}/request", | ||
| headers={"Authorization": auth_token}, | ||
| json={"password": body["password"], "new_password": "changed"}, | ||
| json={"password": body["password"], "new_password": "short7"}, | ||
| ) | ||
| assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY | ||
|
|
||
| response = client_auth.post( | ||
| f"{url}/request", | ||
| headers={"Authorization": auth_token}, | ||
| json={"password": body["password"], "new_password": "пароль123"}, | ||
| ) | ||
| assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY | ||
|
|
||
| response = client_auth.post( | ||
| f"{url}/request", | ||
| headers={"Authorization": auth_token}, | ||
| json={"password": body["password"], "new_password": "changed12"}, | ||
| ) | ||
| assert response.status_code == status.HTTP_200_OK | ||
|
|
||
|
|
@@ -113,6 +127,20 @@ def test_no_token(client_auth: TestClient, dbsession: Session, user_id: int): | |
| assert reset_token | ||
| auth_params = Email.get_auth_method_params(user_id, session=dbsession) | ||
|
|
||
| response = client_auth.post( | ||
| f"{url}", | ||
| headers={"reset-token": reset_token.value}, | ||
| json={"new_password": "short7"}, | ||
| ) | ||
| assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY | ||
|
|
||
| response = client_auth.post( | ||
| f"{url}", | ||
| headers={"reset-token": reset_token.value}, | ||
| json={"new_password": "пароль123"}, | ||
| ) | ||
| assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY | ||
|
|
||
| response = client_auth.post( | ||
| f"{url}", | ||
| headers={"reset-token": reset_token.value + "x"}, | ||
|
|
@@ -129,7 +157,7 @@ def test_no_token(client_auth: TestClient, dbsession: Session, user_id: int): | |
|
|
||
| response = client_auth.post( | ||
| "/email/login", | ||
| json={"email": auth_params["email"].value, "password": "string", "scopes": []}, | ||
| json={"email": auth_params["email"].value, "password": "string12", "scopes": []}, | ||
| ) | ||
| assert response.status_code == status.HTTP_401_UNAUTHORIZED | ||
|
|
||
|
|
@@ -147,21 +175,21 @@ def test_with_token(client_auth: TestClient, dbsession: Session, user): | |
| response = client_auth.post( | ||
| f"{url}/request", | ||
| headers={"Authorization": auth_token}, | ||
| json={"password": "wrong", "new_password": "changed"}, | ||
| json={"password": "wrong", "new_password": "changed12"}, | ||
| ) | ||
| assert response.status_code == status.HTTP_401_UNAUTHORIZED | ||
|
|
||
| response = client_auth.post( | ||
| f"{url}/request", | ||
| headers={"Authorization": auth_token + "wrong"}, | ||
| json={"password": body["password"], "new_password": "changed"}, | ||
| json={"password": body["password"], "new_password": "changed12"}, | ||
| ) | ||
| assert response.status_code == status.HTTP_403_FORBIDDEN | ||
|
|
||
| response = client_auth.post( | ||
| f"{url}/request", | ||
| headers={"Authorization": auth_token}, | ||
| json={"password": body["password"], "new_password": "changed"}, | ||
| json={"password": body["password"], "new_password": "changed12"}, | ||
| ) | ||
| assert response.status_code == status.HTTP_200_OK | ||
| reset_token = ( | ||
|
|
@@ -175,7 +203,7 @@ def test_with_token(client_auth: TestClient, dbsession: Session, user): | |
| ) | ||
| assert response.status_code == status.HTTP_401_UNAUTHORIZED | ||
|
|
||
| response = client_auth.post("/email/login", json={"email": body["email"], "password": "changed", "scopes": []}) | ||
| response = client_auth.post("/email/login", json={"email": body["email"], "password": "changed12", "scopes": []}) | ||
| assert response.status_code == status.HTTP_200_OK | ||
|
|
||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
мб коммент добавить сюда и в изменение ниже, что пароль должен удовлетворять min и max, просто при беглом изучении может быть неочевидно почему именно string12 а не просто string к примеру
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
вроде коммент здесь через хэштег