Skip to content
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

Добавил проверку на наличие скоупа print.file.send #88

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

BombinBM
Copy link

В коде добавил опциональность для ввода номера билета, а так же проверку на наличие скоупа внутри имеющихся у юзера. в выводе оставил только имя юзера.

В коде добавил опциональность для ввода номера билета, а так же проверку на наличие скоупа внутри имеющихся у юзера. в выводе оставил только имя юзера.
Copy link

💩 Code linting failed, use black and isort to fix it.

Copy link

github-actions bot commented Oct 26, 2024

Code Coverage

Coverage Report
FileStmtsMissCoverMissing
print_service
   __main__.py330%1–5
   base.py12467%6–9
   exceptions.py52787%21, 40, 45, 60, 65, 70, 77
print_service/routes
   admin.py492647%28–29, 32–36, 39–43, 50–56, 63–69
   auth.py440%1–8
   exc_handlers.py59985%34, 58, 70, 80, 124, 136, 144, 154, 166
   file.py1352184%50, 56, 58, 136–137, 145–146, 190, 198–199, 201, 204–205, 210–211, 221–224, 269, 278
   qrprint.py1127137%39–48, 53–59, 62–70, 73–82, 86–107, 110, 113–116, 124–127, 135–148
   user.py51492%67, 115–117
print_service/utils
   __init__.py58395%44, 86–87
TOTAL65815277% 

Summary

Tests Skipped Failures Errors Time
23 1 💤 0 ❌ 0 🔥 1.161s ⏱️

@Temmmmmo
Copy link
Member

Temmmmmo commented Nov 3, 2024

линтинг упал, пропиши
black .
isort .

@Temmmmmo
Copy link
Member

Temmmmmo commented Nov 3, 2024

тебе нужен был файл file.py
там нужно было разобраться с одной ручкой, отвечающей за отправку файлов
метод check_union_member тебе вероятно не нужен)

В POST запросе check_union_member вернул код в состояние до первого комита.
Для функции send написал одну дополнительную функцию, чтоб проверять наличие скоупа через Depends, добавил опциональный ввод поля number и проверку его наличия уже в самой функции send
Copy link

💩 Code linting failed, use black and isort to fix it.

@BombinBM BombinBM self-assigned this Nov 16, 2024
Copy link

@parfenovma parfenovma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

лучше ветки на англ называть
Uploading image.png…

print_service/routes/file.py Show resolved Hide resolved
print_service/routes/file.py Outdated Show resolved Hide resolved
if not has_send_scope and inp.number is None:
raise HTTPException(
status_code=400,
detail="Поле number обязательно для пользователей без скоупа print.file.send",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

не юзер френдли ошибка. Она же на фронт пойдет, зачем нам писать служебную информацию?
И наверное это 403 всё-таки

print_service/routes/file.py Outdated Show resolved Hide resolved
print_service/routes/file.py Outdated Show resolved Hide resolved
else:
user = user.filter(
func.upper(UnionMember.surname) == inp.surname.upper(),
)
if not user:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

вот тут добавим также проверку на наличие скоупа

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

то есть если у чела не прописаны ни номер билета, ни фио, ни студак, но при этом есть скоуп, то все хорошо, ошибку не рейзим

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if user is None

@@ -96,6 +98,10 @@ class ReceiveOutput(BaseModel):


# endregion
def has_send_scope(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

давай не будем это выносить в отдельную функцию

async def send(
inp: SendInput,
has_send_scope: bool = Depends(has_send_scope),
settings: Settings = Depends(get_settings),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

депендс пропишем прям тут, это норм

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

депендс от юнионаутха я имею в виду

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

назовем его user, тогда в дальнейшем оттуда мы сможем еще получать user_id

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

auth_user

@@ -63,7 +64,8 @@ class SendInput(BaseModel):
description='Фамилия',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

смотри, surname тоже становится необязательным для пользователей со скоупом, давай учтем это

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

str | None
default = None

@@ -46,10 +46,11 @@ class UpdateUserList(BaseModel):
)
async def check_union_member(
surname: constr(strip_whitespace=True, to_upper=True, min_length=1),
number: constr(strip_whitespace=True, to_upper=True, min_length=1),
number: Optional[str] = constr(strip_whitespace=True, to_upper=True, min_length=1),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

не уверен, что тут надо делать поле опциональным
т.к. для пользователей со скоупом в принципе не планируется прокидывание проверки на присутствие в профсоюзе

Сделал все для ревью
Copy link

github-actions bot commented Dec 4, 2024

💩 Code linting failed, use black and isort to fix it.

@BombinBM BombinBM requested a review from Temmmmmo December 9, 2024 12:16
func.upper(UnionMember.surname) == inp.surname.upper(),
).one_or_none()

if inp.number is not None:
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and inp.surname is not None

func.upper(UnionMember.surname) == inp.surname.upper(),
).one_or_none()
else:
if not "print.file.send" in [scope["name"] for scope in user.get('session_scopes')]:
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

elif

Осталось только разобраться с базой данных и тестами
Сделал локальную проверку на наличие скоупа print.file.send
Пришлось вынести user = user.one_or_none() так как при разных условиях возвращаются разные типы данных (запрос в базу или UnionMember), а далее для создания пина нужен именно UnionMember.

Изменение в файле __main__.py предложил сделать Стас, чтоб автоматически при обновлениях кода обновлялось и приложение в вебе
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Добавление аутха в принтер как второго метода авторизации
3 participants