Open manoelhc opened 1 month ago
This pull request introduces a new feature for user login with username and password. It includes significant changes to the user model to add email validation, updates to existing tests, and the addition of new tests for authentication. New routers and helper functions for handling authentication and JWT tokens are also included.
Files | Changes |
---|---|
src/test/test_users.py src/test/test_auth.py |
Updated user tests to include email validation and added new tests for authentication. |
src/routers/user.py src/routers/auth.py |
Updated user router to handle email and added new auth router for login and password reset. |
src/models/user.py src/models/auth.py |
Updated user model to include email and added new auth model for authentication. |
src/helpers/auth.py src/helpers/user.py src/helpers/jwt.py |
Added helper functions for authentication, user validation, and JWT token handling. |
The recent changes enhance the FastAPI application by improving database connectivity, authentication features, and user management processes. Notable modifications include the addition of email validation, robust password handling mechanisms, and refined user creation logic. Several new helper functions and models support these features, alongside updates to existing environment configurations and testing frameworks. Overall, these updates contribute to a more secure and flexible application architecture.
Files | Change Summary |
---|---|
.env.local |
Renamed DATABASE_UR to DATABASE_URL and updated its value for file-based SQLite database. |
.github/workflows/.../snorkell-auto-documentation.yml |
Minor formatting changes to branch_name parameter. |
.whitesource |
Added a newline at the end of the file for coding standards. |
Dockerfile |
Reordered COPY commands and added a new COPY for manocorp package. |
justfile |
Updated environment variables and volume mappings for improved configuration. |
requirements-dev.txt |
Reordered dependencies, moving pytest-asyncio above pytest . |
requirements.txt |
Added blake3==0.4.1 and email-validator==2.2.0 dependencies. |
src/app.py |
Added import for auth router, expanding API's routing capabilities. |
src/config.py |
Introduced new environment variables for authentication and testing. |
src/helpers/auth.py |
New file for password management functions including hashing and token generation. |
src/helpers/jwt.py |
New file for JWT encoding and decoding functionalities. |
src/helpers/user.py |
New file for user validation functions including username and email checks. |
src/migrations/__init__.py |
Enhanced seed_db function to dynamically create users and link with authentication records. |
src/models/auth.py |
Introduced several authentication-related models using SQLModel. |
src/models/user.py |
Updated UserCreate and User classes to include email validation and improved username handling. |
src/routers/auth.py |
New endpoints for password resets and user login, enhancing authentication features. |
src/routers/user.py |
Modified create_user function to improve user creation flow and link with authentication. |
src/test/test_auth.py |
Comprehensive tests for authentication flow, covering password resets and logins. |
src/test/test_users.py |
Updated user creation tests to include email validation and expanded checks for invalid usernames. |
sequenceDiagram
participant User
participant AuthService
participant Database
User->>AuthService: Request login
AuthService->>Database: Verify credentials
Database-->>AuthService: Return user data
AuthService-->>User: Return JWT token
sequenceDiagram
participant User
participant AuthService
participant Database
User->>AuthService: Request password reset
AuthService->>Database: Verify user and token
Database-->>AuthService: Confirmation of user
AuthService-->>User: Confirm password reset
Tag v0.38.0-pr134
(branch: HEAD
, SHA: 056ae28
) Added.
β±οΈ Estimated effort to review [1-5] | 4, because the PR introduces a significant amount of new functionality, including authentication, password management, and JWT handling, which requires careful review of both the implementation and the associated tests. |
π§ͺ Relevant tests | Yes |
β‘ Possible issues | Possible Bug: The password reset functionality should ensure that the reset token is unique and securely generated to prevent unauthorized access. |
Possible Bug: The password hashing function should be reviewed to ensure it meets security standards and is resistant to attacks. | |
π Security concerns | - Sensitive information exposure: Ensure that sensitive information such as passwords and tokens are not logged or exposed in error messages. |
Category | Suggestion | Score |
Testing |
Expand the invalid username tests to cover more edge cases___ **Ensure that the test cases for invalid usernames and emails cover all edge cases toimprove robustness.** [src/test/test_users.py [60-82]](https://github.com/manoelhc/test-actions/pull/134/files#diff-697871d06d13be1b42d4b3c9a6e51cb9fc0c9098fc0b632f33585ba6ca28887bR60-R82) ```diff -response = client.post( - "/user", - json={"username": "t", "email": config.TEST_USEREMAIL}, -) -assert response.status_code == 422 +for username in ["t", "ttt&", "ttt,asd", "ttt-test", "tt*"]: + response = client.post( + "/user", + json={"username": username, "email": config.TEST_USEREMAIL}, + ) + assert response.status_code == 422 ``` Suggestion importance[1-10]: 9Why: Expanding the test cases for invalid usernames and emails is crucial for ensuring the robustness of the application, making this a significant improvement. | 9 |
Performance |
Refactor user creation tests to reduce redundancy___ **Consider using a loop or a helper function to reduce code duplication when creatingmultiple users in tests.** [src/test/test_users.py [153-168]](https://github.com/manoelhc/test-actions/pull/134/files#diff-697871d06d13be1b42d4b3c9a6e51cb9fc0c9098fc0b632f33585ba6ca28887bR153-R168) ```diff -response = client.post( - "/user", - json={"username": "test_duplicate_user", "email": "test.duplicate.user.first@gmail.com"}, -) -assert response.status_code == 200 -response = client.post( - "/user", - json={"username": "test_duplicate_user", "email": "test.duplicate.user.second@gmail.com"}, -) +for i in range(2): + response = client.post( + "/user", + json={"username": "test_duplicate_user", "email": f"test.duplicate.user.{i+1}@gmail.com"}, + ) + assert response.status_code == (200 if i == 0 else 400) ``` Suggestion importance[1-10]: 8Why: This suggestion effectively addresses code duplication, which is important for maintainability and readability, making it a valuable improvement. | 8 |
Security |
Use a different method to generate the password to ensure security___ **Thepassword and reset_token fields in the Auth instance should be generated securely and not reused for both fields.** [src/routers/user.py [76-77]](https://github.com/manoelhc/test-actions/pull/134/files#diff-838fdfe65292cfb0194482f911a9d79a5717cc137be8fa29511c2f46db33ea9dR76-R77) ```diff reset_token=auth.get_password_token(), -password=auth.get_password_token(), +password=auth.password_generator(), ``` Suggestion importance[1-10]: 8Why: This suggestion addresses a security concern by recommending different methods for generating the `password` and `reset_token`, which is crucial for maintaining secure authentication practices. | 8 |
Maintainability |
Modify the username in the test to ensure uniqueness___ **Ensure that the username used in the test case is unique to avoid potential conflicts withexisting users in the database.** [src/test/test_users.py [42]](https://github.com/manoelhc/test-actions/pull/134/files#diff-697871d06d13be1b42d4b3c9a6e51cb9fc0c9098fc0b632f33585ba6ca28887bR42-R42) ```diff -json={"username": "T3sT_create_user", "email": "manoelhc@gmail.com"}, +json={"username": "unique_T3sT_create_user", "email": "manoelhc@gmail.com"}, ``` Suggestion importance[1-10]: 7Why: While ensuring unique usernames is a good practice, the suggestion does not address a critical issue in the code and is more of a style improvement. | 7 |
Eliminate commented-out code to enhance code readability___ **Remove commented-out code to improve code clarity and maintainability.** [src/test/test_users.py [18]](https://github.com/manoelhc/test-actions/pull/134/files#diff-697871d06d13be1b42d4b3c9a6e51cb9fc0c9098fc0b632f33585ba6ca28887bR18-R18) ```diff -# seed_db() +# (remove this line) ```Suggestion importance[1-10]: 6Why: Removing commented-out code can enhance readability, but this suggestion does not address any functional issues in the code. | 6 | |
Modify the
___
**The | 4 | |
Best practice |
Change the default datetime to use UTC to prevent timezone-related issues___ **Thecreated_at and updated_at fields should use datetime.utcnow() instead of datetime.now() to avoid timezone issues.**
[src/models/auth.py [24-25]](https://github.com/manoelhc/test-actions/pull/134/files#diff-a40b54e0c1cd083a4e232c0850019984b534f46abb9f26773595149e610b9b49R24-R25)
```diff
-created_at: datetime = Field(default=datetime.now())
-updated_at: datetime | None = Field(default=datetime.now(), nullable=True)
+created_at: datetime = Field(default=datetime.utcnow())
+updated_at: datetime | None = Field(default=datetime.utcnow(), nullable=True)
```
Suggestion importance[1-10]: 7Why: This suggestion improves best practices by recommending the use of UTC for timestamps, which is important for avoiding timezone-related issues, although it is not a critical bug. | 7 |
Possible bug |
Validate the
___
**Ensure that the | 3 |
Here's the code health analysis summary for commits 01f42a8..923cde5
. View details on DeepSource β.
Analyzer | Status | Summary | Link |
---|---|---|---|
Python | β Failure | β 37 occurences introduced | View Check β |
Test coverage | β οΈ Artifact not reported | Timed out: Artifact was never reported | View Check β |
π‘ If youβre a repository administrator, you can configure the quality gates from the settings.
38 new problems were found
Inspection name | Severity | Problems |
---|---|---|
Unsatisfied package requirements |
πΆ Warning | 7 |
Unsatisfied package requirements |
β½οΈ Notice | 23 |
PEP 8 coding style violation |
β½οΈ Notice | 4 |
Problematic nesting of decorators |
β½οΈ Notice | 2 |
Using equality operators to compare with None |
β½οΈ Notice | 1 |
Unused local symbols |
β½οΈ Notice | 1 |
π‘ Qodana analysis was run in the pull request mode: only the changed files were checked βοΈ View the detailed Qodana report
New dependencies detected. Learn more about Socket for GitHub βοΈ
Package | New capabilities | Transitives | Size | Publisher |
---|---|---|---|---|
pypi/blake3@0.4.1 | None | 0 |
5.24 MB | oconnor663 |
pypi/email-validator@2.2.0 | environment, eval, filesystem | 0 |
205 kB | Joshua.Tauberer |
Description
Changes walkthrough π
6 files
app.py
Integrate authentication router into the application
src/app.py - Added `auth` router to the FastAPI application.
auth.py
Add password management utilities
src/helpers/auth.py - Implemented password hashing and token generation functions.
jwt.py
Implement JWT token handling
src/helpers/jwt.py - Added functions to encode and decode JWT tokens.
auth.py
Define authentication model and validation
src/models/auth.py
Auth
model for user authentication.auth.py
Add authentication endpoints
src/routers/auth.py - Implemented `/auth/login` and `/auth/password` endpoints.
user.py
Enhance user creation with authentication
src/routers/user.py - Updated user creation to include authentication logic.
2 files
config.py
Update configuration for authentication
src/config.py
PASSWORD_SALT
andTEST_USERNAME
,TEST_USEREMAIL
environmentvariables.
.env.local
Correct database URL in environment configuration
.env.local - Fixed `DATABASE_URL` variable.
2 files
test_auth.py
Implement tests for authentication features
src/test/test_auth.py - Added tests for authentication endpoints.
test_users.py
Enhance user tests with email checks
src/test/test_users.py - Updated user tests to include email validation.
Summary by Sourcery
Implement user login and password reset functionalities, enhance user creation with email validation, and update Docker run commands. Add corresponding tests for new features and enhance existing user tests.
New Features:
Enhancements:
Build:
Tests:
Summary by CodeRabbit