BLOG-86 Checking authentication before uploading image #101

squid · 2025-08-01T18:11:21+08:00

squid commented

2025-08-01 18:11:21 +08:00

Description

This PR introduces a generic authentication middleware to protect application routes. The primary goal is to prevent unauthenticated users from uploading images.

Changes Implemented

Authentication Middleware:
- Created a new auth_middleware that checks the user's session for a valid user_id.
- If a user_id exists, it's added to the request extensions, making it available to downstream handlers.
UserId Extractor:
- A UserId type that implements FromRequest has been added.
- This allows route handlers to declaratively require authentication by simply adding user_id: UserId as a parameter. If the user is not logged in, the extractor automatically returns an ErrorUnauthorized response.
Route Protection:
- The upload_image_handler now includes the UserId extractor, securing the endpoint.
- A new /auth/me route has been added for easily verifying the logged-in user's ID during development and testing.
Minor Refinements:
- The logout_handler now uses session.clear() for more robust session termination.
- Corrected the default Redis URL from redis://127.0.1:6379 to redis://127.0.0.1:6379.

Package Changes

No response

Screenshots

No response

Reference

Resolves #86

Checklist

A milestone is set
The related issuse has been linked to this branch

### Description This PR introduces a generic authentication middleware to protect application routes. The primary goal is to prevent unauthenticated users from uploading images. #### Changes Implemented * **Authentication Middleware**: * Created a new `auth_middleware` that checks the user's session for a valid `user_id`. * If a `user_id` exists, it's added to the request extensions, making it available to downstream handlers. * **`UserId` Extractor**: * A `UserId` type that implements `FromRequest` has been added. * This allows route handlers to declaratively require authentication by simply adding `user_id: UserId` as a parameter. If the user is not logged in, the extractor automatically returns an `ErrorUnauthorized` response. * **Route Protection**: * The `upload_image_handler` now includes the `UserId` extractor, securing the endpoint. * A new `/auth/me` route has been added for easily verifying the logged-in user's ID during development and testing. * **Minor Refinements**: * The `logout_handler` now uses `session.clear()` for more robust session termination. * Corrected the default Redis URL from `redis://127.0.1:6379` to `redis://127.0.0.1:6379`. ### Package Changes _No response_ ### Screenshots _No response_ ### Reference Resolves #86 ### Checklist - [x] A milestone is set - [x] The related issuse has been linked to this branch

squid added this to the 0.3 milestone 2025-08-01 18:11:21 +08:00

squid added 4 commits 2025-08-01 18:11:22 +08:00

BLOG-86 fix: redis URL in session configuration to use correct localhost address 87982fd480

BLOG-86 feat: implement authentication middleware and update routes for user session management 0e8eb809a5

BLOG-86 feat: add auth dependency and integrate UserId in image upload handler a72a567d6a

BLOG-86 refactor: update UserId struct to encapsulate user ID and improve access method

Frontend CI / build (push) Successful in 1m9s

Details

Auto Comment On PR / add_improve_comment (pull_request) Successful in 16s

Details

PR Title Check / pr-title-check (pull_request) Successful in 17s

Details

eb7a3cf985

bot commented

2025-08-01 18:11:29 +08:00

/improve

bot commented

2025-08-01 18:11:34 +08:00

PR Code Suggestions ✨

Category	Suggestion	Impact
Possible issue	Block unauthenticated requests in middleware The `auth_middleware` currently allows requests to proceed if `session.get` returns an error or if the `user_id` is not found in the session. This bypasses authentication for unauthenticated or malformed session requests. The middleware should instead return an `ErrorUnauthorized` in these cases to properly enforce authentication. backend/feature/auth/src/framework/web/auth_middleware.rs [21-32] `-if user_id.is_err() { - return next.call(req).await; -} +let user_id = session.get::<i32>(SESSION_KEY_USER_ID) + .map_err(\|e\| { + log::error!("Session error: {:?}", e); + ErrorUnauthorized("Session error") + })? + .ok_or_else(\|\| ErrorUnauthorized("Not authenticated"))?; -let user_id = user_id.unwrap(); -if user_id.is_none() { - return next.call(req).await; -} - -let user_id = user_id.unwrap(); req.extensions_mut().insert(user_id); next.call(req).await` Suggestion importance[1-10]: 10 __ Why: The current implementation allows unauthenticated requests to proceed if `session.get` fails or returns `None`, which is a critical security vulnerability. The suggestion correctly enforces authentication by returning `ErrorUnauthorized` in these cases.	High

## PR Code Suggestions ✨  <table><thead><tr><td><strong>Category</strong></td><td align=left><strong>Suggestion                                                                                                                                    </strong></td><td align=center><strong>Impact</strong></td></tr><tbody><tr><td rowspan=1>Possible issue</td> <td> <details><summary>Block unauthenticated requests in middleware</summary> ___ **The <code>auth_middleware</code> currently allows requests to proceed if <code>session.get</code> returns an <br>error or if the <code>user_id</code> is not found in the session. This bypasses authentication <br>for unauthenticated or malformed session requests. The middleware should instead <br>return an <code>ErrorUnauthorized</code> in these cases to properly enforce authentication.** [backend/feature/auth/src/framework/web/auth_middleware.rs [21-32]](https://git.squidspirit.com/squid/blog/src/branch/BLOG-86_image_can_only_be_uploaded_by_logged_in_user/backend/feature/auth/src/framework/web/auth_middleware.rs#L21-L32) ```diff -if user_id.is_err() { - return next.call(req).await; -} +let user_id = session.get::<i32>(SESSION_KEY_USER_ID) + .map_err(|e| { + log::error!("Session error: {:?}", e); + ErrorUnauthorized("Session error") + })? + .ok_or_else(|| ErrorUnauthorized("Not authenticated"))?; -let user_id = user_id.unwrap(); -if user_id.is_none() { - return next.call(req).await; -} - -let user_id = user_id.unwrap(); req.extensions_mut().insert(user_id); next.call(req).await ``` <details><summary>Suggestion importance[1-10]: 10</summary> __ Why: The current implementation allows unauthenticated requests to proceed if `session.get` fails or returns `None`, which is a critical security vulnerability. The suggestion correctly enforces authentication by returning `ErrorUnauthorized` in these cases. </details></details></td><td align=center>High </td></tr></tr></tbody></table>

squid added 1 commit 2025-08-01 18:24:12 +08:00

BLOG-86 refactor: simplify auth middleware by removing unnecessary error handling and integrating user ID retrieval

Frontend CI / build (push) Successful in 1m8s

Details

PR Title Check / pr-title-check (pull_request) Successful in 16s

Details

04c9d4dcf0

squid commented

2025-08-01 18:24:50 +08:00

The auth_middleware currently allows requests to proceed if session.get returns an
error or if the user_id is not found in the session. This bypasses authentication
for unauthenticated or malformed session requests. The middleware should instead
return an ErrorUnauthorized in these cases to properly enforce authentication.

backend/feature/auth/src/framework/web/auth_middleware.rs [21-32]
-if user_id.is_err() {
-    return next.call(req).await;
-}
+let user_id = session.get::<i32>(SESSION_KEY_USER_ID)
+    .map_err(|e| {
+        log::error!("Session error: {:?}", e);
+        ErrorUnauthorized("Session error")
+    })?
+    .ok_or_else(|| ErrorUnauthorized("Not authenticated"))?;
 
-let user_id = user_id.unwrap();
-if user_id.is_none() {
-    return next.call(req).await;
-}
-
-let user_id = user_id.unwrap();
 req.extensions_mut().insert(user_id);
 next.call(req).await

Addressed in 04c9d4dcf0.

Removed redoundent middleware.

> **The <code>auth_middleware</code> currently allows requests to proceed if <code>session.get</code> returns an <br>error or if the <code>user_id</code> is not found in the session. This bypasses authentication <br>for unauthenticated or malformed session requests. The middleware should instead <br>return an <code>ErrorUnauthorized</code> in these cases to properly enforce authentication.** > > [backend/feature/auth/src/framework/web/auth_middleware.rs [21-32]](https://git.squidspirit.com/squid/blog/src/branch/BLOG-86_image_can_only_be_uploaded_by_logged_in_user/backend/feature/auth/src/framework/web/auth_middleware.rs#L21-L32) > > ```diff > -if user_id.is_err() { > - return next.call(req).await; > -} > +let user_id = session.get::<i32>(SESSION_KEY_USER_ID) > + .map_err(|e| { > + log::error!("Session error: {:?}", e); > + ErrorUnauthorized("Session error") > + })? > + .ok_or_else(|| ErrorUnauthorized("Not authenticated"))?; > > -let user_id = user_id.unwrap(); > -if user_id.is_none() { > - return next.call(req).await; > -} > - > -let user_id = user_id.unwrap(); > req.extensions_mut().insert(user_id); > next.call(req).await > ``` Addressed in 04c9d4dcf073cca02b4d5c2779a5a2bf576f6ceb. Removed redoundent middleware.

squid merged commit 197d7773ef into main

2025-08-01 18:26:39 +08:00

squid deleted branch BLOG-86_image_can_only_be_uploaded_by_logged_in_user

2025-08-01 18:26:39 +08:00

squid referenced this issue from a commit

2025-08-01 18:26:41 +08:00

BLOG-86 Checking authentication before uploading image (#101)

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: squid/blog#101