Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR modifies the user registration flow by adding authentication requirements and changing the default activation status. The signup endpoint is uncommented and now requires authentication, while new users are created in an inactive state by default.
Key changes:
- New users are now created with
active = Falseinstead ofactive = True - The
/signupendpoint now requires authentication via JWT token withLLEIDAHACKERuser type - Email notification system for new user creation is re-enabled
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/impl/LleidaHacker/service.py | Changed default user activation status to inactive with warning comment |
| src/impl/LleidaHacker/router_v1.py | Uncommented and modified signup endpoint to require authentication and send verification emails |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| def signup(payload: LleidaHackerCreate, token: BaseToken = Depends(JWTBearer())): | ||
| if not token.check([UserType.LLEIDAHACKER]): | ||
| raise AuthenticationException("Not authorized") | ||
|
|
There was a problem hiding this comment.
The signup endpoint should not require authentication. Requiring a JWT token to create a new user account creates a chicken-and-egg problem where users cannot sign up without already being authenticated. This breaks the typical user registration flow.
| def signup(payload: LleidaHackerCreate, token: BaseToken = Depends(JWTBearer())): | |
| if not token.check([UserType.LLEIDAHACKER]): | |
| raise AuthenticationException("Not authorized") | |
| def signup(payload: LleidaHackerCreate): |
| ) | ||
| new_lleidahacker.password = get_password_hash(payload.password) | ||
| new_lleidahacker.active = True | ||
| new_lleidahacker.active = False # IMPORTANT DO NOT ACTIVATE USER AUTOMATICALLY !!!!!!!!!!!!! @Big_Lolo |
There was a problem hiding this comment.
[nitpick] The comment uses excessive punctuation and mentions a specific person. Comments should be professional and explain the reasoning rather than using emphatic styling. Consider: '# Users must be manually activated by administrators for security reasons' or reference a configuration setting/documentation.
| new_lleidahacker.active = False # IMPORTANT DO NOT ACTIVATE USER AUTOMATICALLY !!!!!!!!!!!!! @Big_Lolo | |
| new_lleidahacker.active = False # Users must be manually activated by administrators for security reasons |
No description provided.