feat: added endpoints: auth, pofile, account, keyring #5

Open
ivan.dev wants to merge 9 commits from VORKOUT-4 into master
Member
No description provided.
ivan.dev added 1 commit 2025-04-17 15:46:15 +05:00
ivan.dev requested review from cyrussmeat 2025-04-17 15:46:21 +05:00
ivan.dev requested review from vlad.dev 2025-04-17 15:46:21 +05:00
cyrussmeat reviewed 2025-04-17 16:14:02 +05:00
@ -0,0 +49,4 @@
return user, password
async def upgrade_old_refresh_token(connection: AsyncConnection, user) -> Optional[User]:
Owner

Он все текущие токены пометит устаревшими у аккаунта?

Тут момент - он должен экспайрить конкретный рефреш токен, т.к. один Пользователь может заходить с нескольких устройств.

Он все текущие токены пометит устаревшими у аккаунта? Тут момент - он должен экспайрить конкретный рефреш токен, т.к. один Пользователь может заходить с нескольких устройств.
Author
Member

Как при решении этой проблемы привязать рефреш токен к конкретной сессии?
Когда пользователь заходит(логин/пароль) я могу получить только все ключи по его ID.

Как при решении этой проблемы привязать рефреш токен к конкретной сессии? Когда пользователь заходит(логин/пароль) я могу получить только все ключи по его ID.
Owner

Ты прав в отсутствии привязки. А я про то, что в update_query надо ставить фильтрацию по expiry как минимум, т.е. при генерации нового рефреша не губить скопом все старые как минимум.

По поводу сессионной привязки я подумаю, стоит ли её организовывать по идентификатору сессию, который по идее должен тоже быть у нас на руках.

Ты прав в отсутствии привязки. А я про то, что в update_query надо ставить фильтрацию по expiry как минимум, т.е. при генерации нового рефреша не губить скопом все старые как минимум. По поводу сессионной привязки я подумаю, стоит ли её организовывать по идентификатору сессию, который по идее должен тоже быть у нас на руках.
ivan.dev marked this conversation as resolved
cyrussmeat requested changes 2025-04-17 16:42:30 +05:00
@ -0,0 +39,4 @@
connection: AsyncConnection = Depends(get_connection_dep),
Authorize: AuthJWT = Depends()):
current_user = AccessTokenValidadtion(Authorize)
Owner

Я вижу тут повторяющийся в каждом методе паттерн "взять текущего пользователя/получить его данные".
Не думал сделать общий механизм через middleware?

@app.middleware("http")

Я вижу тут повторяющийся в каждом методе паттерн "взять текущего пользователя/получить его данные". Не думал сделать общий механизм через middleware? @app.middleware("http")
ivan.dev marked this conversation as resolved
@ -0,0 +102,4 @@
update_values = put_key_data_validator(keyring_update,keyring)
if update_values is None:
Owner

А это разве ошибка, если изменений не было, но попытались сохранить данные?
Оно должно проходить без лишнего крика.

А это разве ошибка, если изменений не было, но попытались сохранить данные? Оно должно проходить без лишнего крика.
ivan.dev marked this conversation as resolved
@ -0,0 +3,4 @@
from api.schemas.endpoints.account import UserUpdate, Role, Status
from api.schemas.endpoints.account_keyring import AccountKeyringUpdate, StatusKey, TypeKey
def put_user_data_validator(update_data: UserUpdate, user) -> Optional[dict]:
Owner

По названию оно должно заниматься валидацией через DTO/модель, т.е. проверять поля на их соответствие типам и размерностям.

Тут скорее просится название put_user_data_changes() как вариант оптимизации избыточности вставки данных, но не проверки валидности.

Это не отменяет необходимости валидации данных. Либо Pydantic в модели, либо DTO с валидацией - ваш выбор.

По названию оно должно заниматься валидацией через DTO/модель, т.е. проверять поля на их соответствие типам и размерностям. Тут скорее просится название put_user_data_changes() как вариант оптимизации избыточности вставки данных, но не проверки валидности. Это не отменяет необходимости валидации данных. Либо Pydantic в модели, либо DTO с валидацией - ваш выбор.
Author
Member

Функция сравнивает то что пришло с таблицей Pydantic AccountKeyringUpdate(api\schemas\endpoints\account_keyring) где все точно такие же поля только все необязательные, это в данном случае и выполняет функцию валидации. Это не верное решение?

Функция сравнивает то что пришло с таблицей Pydantic AccountKeyringUpdate(api\schemas\endpoints\account_keyring) где все точно такие же поля только все необязательные, это в данном случае и выполняет функцию валидации. Это не верное решение?
Owner

У Pydantic же есть model_validate() и @validator() директива, в которой реализуется не только проверка размерности по типу данных, но и всякие дополнительные нюансы, к примеру длина логина, его состав (e-mail) и прочие дополнительные ограничения.

В классике там оно приходит уже провалидированным в методе:
https://stackoverflow.com/questions/78157897/sqlmodel-and-pydantic-data-validation

У Pydantic же есть model_validate() и @validator() директива, в которой реализуется не только проверка размерности по типу данных, но и всякие дополнительные нюансы, к примеру длина логина, его состав (e-mail) и прочие дополнительные ограничения. В классике там оно приходит уже провалидированным в методе: https://stackoverflow.com/questions/78157897/sqlmodel-and-pydantic-data-validation
ivan.dev marked this conversation as resolved
vlad.dev reviewed 2025-04-18 12:21:37 +05:00
@ -0,0 +52,4 @@
@api_router.post("/{user_id}")
async def post_account(
Member

Вот сейчас еще раз посмотрел, кажется, что лучше все-таки создание аккаунта делать на простой пост, без path параметра, а то как-то глупо получается, что мы хардкодим id, хотя у нас там автоинкремент идет)
@cyrussmeat какой роут использовать для создания пользователя, просто http://localhost:8000/api/v1/account/?

Вот сейчас еще раз посмотрел, кажется, что лучше все-таки создание аккаунта делать на простой пост, без `path` параметра, а то как-то глупо получается, что мы хардкодим `id`, хотя у нас там автоинкремент идет) @cyrussmeat какой роут использовать для создания пользователя, просто `http://localhost:8000/api/v1/account/`?
ivan.dev marked this conversation as resolved
@ -0,0 +78,4 @@
@api_router.put("/{user_id}")
async def put_account(
Member

По поводу имен, я думаю, что лучше вместо put, post использовать более наглядные update и create

По поводу имен, я думаю, что лучше вместо put, post использовать более наглядные update и create
ivan.dev marked this conversation as resolved
@ -0,0 +107,4 @@
@api_router.post("/refresh")
def refresh(
Member

Тут access_token же должен возвращаться теле ответа, разве нет?

Тут `access_token` же должен возвращаться теле ответа, разве нет?
Author
Member

Новый(сгенерированный) или старый?

Новый(сгенерированный) или старый?
Member

Генерируешь новый access_token на основе валидного refresh
По сути, надо просто поменять ответ, вместо проставления access_token в куки, вернуть его в теле

Генерируешь новый `access_token` на основе валидного `refresh` По сути, надо просто поменять ответ, вместо проставления `access_token` в куки, вернуть его в теле
ivan.dev marked this conversation as resolved
@ -39,6 +40,7 @@ async def init():
create_key_query = account_keyring_table.insert().values(
owner_id=user_id,
key_type=KeyType.PASSWORD,
key_id=KeyIdGenerator()
Member

Тут потерял запятую после key_id=KeyIdGenerator()

Тут потерял запятую после `key_id=KeyIdGenerator()`
ivan.dev marked this conversation as resolved
vlad.dev reviewed 2025-04-18 12:40:17 +05:00
@ -0,0 +130,4 @@
user_update = UserUpdate(status=Status.DELETED.value)
update_values = put_user_id_validator(user_update,user)
Member

put_user_id_validator выдает у меня ошибку, что имя не определено

`put_user_id_validator` выдает у меня ошибку, что имя не определено
ivan.dev marked this conversation as resolved
ivan.dev added 1 commit 2025-04-18 17:31:46 +05:00
ivan.dev added 3 commits 2025-04-23 18:49:37 +05:00
ivan.dev added 1 commit 2025-04-23 18:54:32 +05:00
ivan.dev added 1 commit 2025-04-23 19:30:50 +05:00
vlad.dev reviewed 2025-04-24 12:35:43 +05:00
@ -0,0 +103,4 @@
if update_values is None:
return user
user_update_data = User.model_validate({**user.model_dump(), **update_values})
Member

А вот где неиспользуемые переменные, можешь объяснить зачем они, я просто не особо понял?
Вот к примеру user_update_data и authorize_user, это какой-то задел на будущее?

А вот где неиспользуемые переменные, можешь объяснить зачем они, я просто не особо понял? Вот к примеру `user_update_data` и `authorize_user`, это какой-то задел на будущее?
Author
Member

Да, если логика поменяется.

Да, если логика поменяется.
ivan.dev marked this conversation as resolved
ivan.dev added 1 commit 2025-04-29 21:17:15 +05:00
vlad.dev requested changes 2025-05-05 12:15:42 +05:00
Dismissed
@ -0,0 +27,4 @@
@api_router.get("/{user_id}")
async def get_profile(
Member

Тут, наверное, надо переделать
Профиль не должен зависеть от id, то есть, можно переделать роут на просто /profile
Сам пользователя можно брать из загрузки jwt, вроде там уже есть login

Тут, наверное, надо переделать Профиль не должен зависеть от `id`, то есть, можно переделать роут на просто `/profile` Сам пользователя можно брать из загрузки jwt, вроде там уже есть `login`
ivan.dev marked this conversation as resolved
@ -0,0 +46,4 @@
@api_router.put("/{user_id}")
async def update_profile(
Member

Тут тоже самое, что и с get_profile, только вот по поводу обновления полей
@cyrussmeat какие поля пользователь не может сам себе обновить? Роль?

Тут тоже самое, что и с `get_profile`, только вот по поводу обновления полей @cyrussmeat какие поля пользователь не может сам себе обновить? Роль?
Owner

Роль и Логин

Роль и Логин
ivan.dev marked this conversation as resolved
ivan.dev added 1 commit 2025-05-12 21:20:39 +05:00
vlad.dev approved these changes 2025-05-12 21:51:34 +05:00
This pull request can be merged automatically.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin VORKOUT-4:VORKOUT-4
git checkout VORKOUT-4
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: Vorkout/connect#5
No description provided.