-
Notifications
You must be signed in to change notification settings - Fork 1
Feature/flask smorest auth #24
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
Conversation
WalkthroughAdds a new auth blueprint mounted at /auth with register, login, and refresh endpoints; moves category/subcategory/product blueprint imports from app.migrated_routes to app.routes; removes legacy auth endpoints from app/routes.py; adds AuthIn/AuthOut schemas; changes User.set_email to let normalization errors propagate; updates tests to expect 422 and a revised login message. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant API as Flask API (/auth)
participant DB as Database
participant JWT as JWT Manager
rect rgb(235,245,255)
note over Client,API: Register
Client->>API: POST /auth/register {email, password}
API->>API: Validate via AuthIn (trim email)
API->>DB: Create User, commit
alt Duplicate email
DB-->>API: IntegrityError
API-->>Client: 409 Conflict
else Invalid email
API-->>Client: 422 Unprocessable Entity
else Success
API-->>Client: 200 {message}
end
end
rect rgb(240,255,240)
note over Client,API: Login
Client->>API: POST /auth/login {email, password}
API->>DB: Lookup User by email
alt Auth fails
API-->>Client: 401 Invalid email or password
else Auth OK
API->>JWT: create_access_token(user.id)
API->>JWT: create_refresh_token(user.id)
API-->>Client: 200 {access_token, refresh_token}
end
end
rect rgb(255,250,235)
note over Client,API: Refresh
Client->>API: POST /auth/refresh (with refresh JWT)
API->>JWT: get_jwt_identity()
alt Invalid/expired refresh
API-->>Client: 401/422 error
else Valid
API->>JWT: create_access_token(identity)
API-->>Client: 200 {access_token}
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/routes/auth.py (1)
135-154: Align refresh response schema with the actual payload.
AuthOut(partial=("access_token",))still advertisesrefresh_tokenin the generated OpenAPI and doesn’t buy us anything at dump time—the refresh handler only returns an access token. This mismatch will confuse clients consuming the docs. Please narrow the schema (or define a dedicatedRefreshOut) so the spec mirrors reality.- @bp.response(200, AuthOut(partial=("access_token",))) + @bp.response(200, AuthOut(only=("access_token",)))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/__init__.py(1 hunks)app/models.py(1 hunks)app/routes.py(0 hunks)app/routes/auth.py(1 hunks)app/schemas.py(2 hunks)tests/test_auth.py(4 hunks)
💤 Files with no reviewable changes (1)
- app/routes.py
🧰 Additional context used
🧬 Code graph analysis (2)
app/routes/auth.py (2)
app/models.py (4)
User(18-59)set_password(54-56)set_email(50-52)check_password(58-59)app/schemas.py (2)
AuthIn(107-119)AuthOut(122-124)
app/schemas.py (1)
app/models.py (1)
User(18-59)
🪛 Ruff (0.13.3)
app/schemas.py
116-116: Unused method argument: kwargs
(ARG002)
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.
Actionable comments posted: 3
🧹 Nitpick comments (2)
app/routes/auth.py (2)
57-61: Do email validation before hashing to avoid wasted CPU.Hashing with scrypt is expensive; validate/normalize email first, then hash.
- user = User() - user.set_password(data["password"]) - - try: - user.set_email(data["email"]) + user = User() + try: + user.set_email(data["email"]) + user.set_password(data["password"])
47-54: Align OpenAPI responses with actual behavior (use 422 for validation errors).Docs list 400, but Marshmallow/Smorest and your code return 422 on invalid input.
- 400: - description: Invalid input. + 422: + description: Validation error.Repeat similarly for the Login section.
Also applies to: 104-112
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/routes/auth.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/routes/auth.py (3)
app/models.py (4)
User(18-59)set_password(54-56)set_email(50-52)check_password(58-59)app/schemas.py (2)
AuthIn(107-119)AuthOut(122-124)app/routes.py (3)
register(11-61)login(65-111)refresh(116-135)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: deploy-preview
Summary by CodeRabbit
New Features
Refactor
Tests