BLOG-94 Create user in DB when first login through OIDC #96

Merged
squid merged 4 commits from BLOG-94_create_user_when_first_login into main 2025-08-01 13:24:09 +08:00
Owner

Description

This PR introduces the functionality to persist user information in the database. When a user logs in via OIDC for the first time, a new user record is created. Subsequent logins will retrieve the existing user data from the database.

This change ensures that users have a persistent identity within our system, identified by their unique combination of OIDC issuer and subject ID.

Key Changes

  • User Persistence Logic:

    • In ExchangeAuthCodeUseCase, after successfully exchanging the authorization code, the logic now checks if the user exists in our database using their issuer and source_id.
    • If the user is not found (AuthError::UserNotFound), a new record is created in the user table.
    • The User entity returned by the use case now contains the internal database id.
  • Database Integration in Auth Feature:

    • Introduced a new UserDbService trait and its sqlx-based implementation, UserDbServiceImpl, to handle database operations for users.
    • The AuthRepository is extended to include methods for querying (get_user_by_source_id) and saving (save_user) users, delegating the calls to the new UserDbService.
    • The dependency injection container in server/src/container.rs has been updated to provide the UserDbServiceImpl to the AuthRepositoryImpl.
  • Domain and Data Model Updates:

    • The User domain entity now includes id (the database primary key) and issuer (from OIDC claims) to uniquely identify a user across different identity providers.
    • The UserResponseDto now exposes the internal id instead of the source_id.
  • Session Management:

    • The user's session now stores the database user_id (i32) instead of the entire user object. This is more efficient and secure.
    • Session keys have been centralized into a constants.rs file for better maintainability.

Database Changes

  • A new database migration has been added to create the user table.
  • The table includes columns for id, issuer, source_id, displayed_name, and email.
  • A UNIQUE index has been created on (source_id, issuer) to guarantee that each user from a specific identity provider is stored only once.

Refactoring

  • Minor refactoring in the image feature to change id: Option<i32> to id: i32 for consistency with the new User entity model.

Package Changes

No response

Screenshots

No response

Reference

Resolves #94

Checklist

  • A milestone is set
  • The related issuse has been linked to this branch
### Description This PR introduces the functionality to persist user information in the database. When a user logs in via OIDC for the first time, a new user record is created. Subsequent logins will retrieve the existing user data from the database. This change ensures that users have a persistent identity within our system, identified by their unique combination of OIDC issuer and subject ID. #### Key Changes * **User Persistence Logic**: * In `ExchangeAuthCodeUseCase`, after successfully exchanging the authorization code, the logic now checks if the user exists in our database using their `issuer` and `source_id`. * If the user is not found (`AuthError::UserNotFound`), a new record is created in the `user` table. * The `User` entity returned by the use case now contains the internal database `id`. * **Database Integration in Auth Feature**: * Introduced a new `UserDbService` trait and its `sqlx`-based implementation, `UserDbServiceImpl`, to handle database operations for users. * The `AuthRepository` is extended to include methods for querying (`get_user_by_source_id`) and saving (`save_user`) users, delegating the calls to the new `UserDbService`. * The dependency injection container in `server/src/container.rs` has been updated to provide the `UserDbServiceImpl` to the `AuthRepositoryImpl`. * **Domain and Data Model Updates**: * The `User` domain entity now includes `id` (the database primary key) and `issuer` (from OIDC claims) to uniquely identify a user across different identity providers. * The `UserResponseDto` now exposes the internal `id` instead of the `source_id`. * **Session Management**: * The user's session now stores the database `user_id` (`i32`) instead of the entire user object. This is more efficient and secure. * Session keys have been centralized into a `constants.rs` file for better maintainability. #### Database Changes * A new database migration has been added to create the `user` table. * The table includes columns for `id`, `issuer`, `source_id`, `displayed_name`, and `email`. * A **`UNIQUE` index** has been created on `(source_id, issuer)` to guarantee that each user from a specific identity provider is stored only once. #### Refactoring * Minor refactoring in the `image` feature to change `id: Option<i32>` to `id: i32` for consistency with the new `User` entity model. ### Package Changes _No response_ ### Screenshots _No response_ ### Reference Resolves #94 ### 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 13:05:11 +08:00
squid added 3 commits 2025-08-01 13:05:11 +08:00
- Introduced UserDbService and UserDbServiceImpl for user data handling.
- Added UserMapper for converting between User and UserRecord.
- Implemented user creation and retrieval in the database.
- Updated AuthRepository to include user-related methods.
- Enhanced OIDC claims processing to include issuer information.
- Created user table in the database with necessary fields and indexes.
BLOG-94 refactor: rename db mapper
All checks were successful
Frontend CI / build (push) Successful in 1m9s
Auto Comment On PR / add_improve_comment (pull_request) Successful in 16s
PR Title Check / pr-title-check (pull_request) Successful in 16s
d349fd672c
Collaborator

/improve

/improve
Collaborator

PR Code Suggestions

CategorySuggestion                                                                                                                                    Impact
Possible issue
Improve error handling for user retrieval

The current error handling for get_user_by_source_id only checks for
AuthError::UserNotFound. If a different database error occurs, it is not propagated,
leading to a silent failure and potentially returning an unsaved user. Use a match
statement to explicitly handle all Result variants, propagating unexpected errors.

backend/feature/auth/src/application/use_case/exchange_auth_code_use_case.rs [49-62]

-let saved_user = self
+let saved_user_result = self
     .auth_repository
     .get_user_by_source_id(&logged_in_user.issuer, &logged_in_user.source_id)
     .await;
 
-if saved_user.err() == Some(AuthError::UserNotFound) {
-    let id = self
-        .auth_repository
-        .save_user(logged_in_user.clone())
-        .await?;
-    logged_in_user.id = id;
+match saved_user_result {
+    Ok(user) => {
+        logged_in_user.id = user.id;
+    },
+    Err(AuthError::UserNotFound) => {
+        let id = self
+            .auth_repository
+            .save_user(logged_in_user.clone())
+            .await?;
+        logged_in_user.id = id;
+    },
+    Err(e) => {
+        return Err(e);
+    }
 }
 
 Ok(logged_in_user)
Suggestion importance[1-10]: 9

__

Why: The original code only handled AuthError::UserNotFound, potentially leading to silent failures if get_user_by_source_id returned other errors. The suggested match statement correctly propagates all errors and ensures the id is updated for existing users, significantly improving robustness.

High
## PR Code Suggestions ✨ <!-- --> <table><thead><tr><td><strong>Category</strong></td><td align=left><strong>Suggestion&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </strong></td><td align=center><strong>Impact</strong></td></tr><tbody><tr><td rowspan=1>Possible issue</td> <td> <details><summary>Improve error handling for user retrieval</summary> ___ **The current error handling for <code>get_user_by_source_id</code> only checks for <br><code>AuthError::UserNotFound</code>. If a different database error occurs, it is not propagated, <br>leading to a silent failure and potentially returning an unsaved user. Use a <code>match</code> <br>statement to explicitly handle all <code>Result</code> variants, propagating unexpected errors.** [backend/feature/auth/src/application/use_case/exchange_auth_code_use_case.rs [49-62]](https://git.squidspirit.com/squid/blog/src/branch/BLOG-94_create_user_when_first_login/backend/feature/auth/src/application/use_case/exchange_auth_code_use_case.rs#L49-L62) ```diff -let saved_user = self +let saved_user_result = self .auth_repository .get_user_by_source_id(&logged_in_user.issuer, &logged_in_user.source_id) .await; -if saved_user.err() == Some(AuthError::UserNotFound) { - let id = self - .auth_repository - .save_user(logged_in_user.clone()) - .await?; - logged_in_user.id = id; +match saved_user_result { + Ok(user) => { + logged_in_user.id = user.id; + }, + Err(AuthError::UserNotFound) => { + let id = self + .auth_repository + .save_user(logged_in_user.clone()) + .await?; + logged_in_user.id = id; + }, + Err(e) => { + return Err(e); + } } Ok(logged_in_user) ``` <details><summary>Suggestion importance[1-10]: 9</summary> __ Why: The original code only handled `AuthError::UserNotFound`, potentially leading to silent failures if `get_user_by_source_id` returned other errors. The suggested `match` statement correctly propagates all errors and ensures the `id` is updated for existing users, significantly improving robustness. </details></details></td><td align=center>High </td></tr></tr></tbody></table>
squid added 1 commit 2025-08-01 13:10:56 +08:00
BLOG-94 fix: improve error handling for user retrieval
All checks were successful
Frontend CI / build (push) Successful in 1m8s
PR Title Check / pr-title-check (pull_request) Successful in 16s
ef7db7424a
Author
Owner

The current error handling for get_user_by_source_id only checks for
AuthError::UserNotFound. If a different database error occurs, it is not propagated,
leading to a silent failure and potentially returning an unsaved user. Use a match
statement to explicitly handle all Result variants, propagating unexpected errors.

backend/feature/auth/src/application/use_case/exchange_auth_code_use_case.rs [49-62]

-let saved_user = self
+let saved_user_result = self
     .auth_repository
     .get_user_by_source_id(&logged_in_user.issuer, &logged_in_user.source_id)
     .await;
 
-if saved_user.err() == Some(AuthError::UserNotFound) {
-    let id = self
-        .auth_repository
-        .save_user(logged_in_user.clone())
-        .await?;
-    logged_in_user.id = id;
+match saved_user_result {
+    Ok(user) => {
+        logged_in_user.id = user.id;
+    },
+    Err(AuthError::UserNotFound) => {
+        let id = self
+            .auth_repository
+            .save_user(logged_in_user.clone())
+            .await?;
+        logged_in_user.id = id;
+    },
+    Err(e) => {
+        return Err(e);
+    }
 }
 
 Ok(logged_in_user)

Addressed in ef7db7424a

> **The current error handling for <code>get_user_by_source_id</code> only checks for <br><code>AuthError::UserNotFound</code>. If a different database error occurs, it is not propagated, <br>leading to a silent failure and potentially returning an unsaved user. Use a <code>match</code> <br>statement to explicitly handle all <code>Result</code> variants, propagating unexpected errors.** > > [backend/feature/auth/src/application/use_case/exchange_auth_code_use_case.rs [49-62]](https://git.squidspirit.com/squid/blog/src/branch/BLOG-94_create_user_when_first_login/backend/feature/auth/src/application/use_case/exchange_auth_code_use_case.rs#L49-L62) > > ```diff > -let saved_user = self > +let saved_user_result = self > .auth_repository > .get_user_by_source_id(&logged_in_user.issuer, &logged_in_user.source_id) > .await; > > -if saved_user.err() == Some(AuthError::UserNotFound) { > - let id = self > - .auth_repository > - .save_user(logged_in_user.clone()) > - .await?; > - logged_in_user.id = id; > +match saved_user_result { > + Ok(user) => { > + logged_in_user.id = user.id; > + }, > + Err(AuthError::UserNotFound) => { > + let id = self > + .auth_repository > + .save_user(logged_in_user.clone()) > + .await?; > + logged_in_user.id = id; > + }, > + Err(e) => { > + return Err(e); > + } > } > > Ok(logged_in_user) > ``` Addressed in https://git.squidspirit.com/squid/blog/commit/ef7db7424a7586bc39310f8cb2571b5acd6b3332
squid merged commit 9c88b4bb55 into main 2025-08-01 13:24:09 +08:00
squid deleted branch BLOG-94_create_user_when_first_login 2025-08-01 13:24:09 +08:00
Sign in to join this conversation.
No description provided.