Make device installation sensitive to to changes of its corrsponding user session (#9304) #9305
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes in this pull request enhance user session management across various components of the Boilerplate project. A new event handler for Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Introduce conditional compilation to include/exclude SignalR-related code in UserController.cs based on the `signalR` preprocessor directive. This enhances flexibility by allowing SignalR integration to be enabled or disabled as needed.
…mplate-must-stop-sending-user-related-push-notification-and-signalr-messages-to-a-device-that-its-user-session-is-not-valid-anymore
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Outside diff range and nitpick comments (15)
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Dtos/Identity/UserSessionDto.cs (1)
Line range hint
5-15: Consider adding XML documentation for DTO properties.Adding XML documentation would improve the clarity of each property's purpose, especially for properties like
IsValidwhich play a crucial role in the new session-aware notification system.Example improvement:
public partial class UserSessionDto { + /// <summary> + /// Unique identifier for the user session + /// </summary> public Guid Id { get; set; } + /// <summary> + /// IP address from which the session was created + /// </summary> public string? IP { get; set; } + /// <summary> + /// Device information associated with the session + /// </summary> public string? Device { get; set; } + /// <summary> + /// Physical or network address associated with the session + /// </summary> public string? Address { get; set; } + /// <summary> + /// Timestamp when the session was last renewed + /// </summary> public DateTimeOffset RenewedOn { get; set; } + /// <summary> + /// Indicates if the session is currently valid and active + /// </summary> public bool IsValid { get; set; } }src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Models/Identity/UserSession.cs (1)
17-20: LGTM! Consider adding an index on UserIdThe explicit user relationship is well-structured with proper foreign key configuration. This change effectively supports the PR's goal of making device installations sensitive to user session changes.
Consider adding an index on the
UserIdcolumn since it will be frequently used to look up sessions for a given user. This can be done by adding the following attribute above theUserIdproperty:[Index] public Guid UserId { get; set; }src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Models/PushNotification/DeviceInstallation.cs (1)
32-35: Consider adding validation for RenewedOn timestamp.While the addition of
RenewedOnis good for tracking installation freshness, consider adding validation to ensure:
- The timestamp is not in the future
- The timestamp is not after
ExpirationTimeHere's a suggested implementation using data annotations:
+using System.ComponentModel.DataAnnotations; public class DeviceInstallation { // ... existing properties ... /// <summary> /// Unix Time Seconds /// </summary> + [CustomValidation(typeof(DeviceInstallation), nameof(ValidateTimestamps))] public long RenewedOn { get; set; } + public static ValidationResult? ValidateTimestamps(long renewedOn, ValidationContext context) + { + var installation = (DeviceInstallation)context.ObjectInstance; + var now = DateTimeOffset.UtcNow.ToUnixTimeSeconds(); + + if (renewedOn > now) + return new ValidationResult("RenewedOn cannot be in the future"); + + if (installation.ExpirationTime > 0 && renewedOn > installation.ExpirationTime) + return new ValidationResult("RenewedOn cannot be after ExpirationTime"); + + return ValidationResult.Success; + } }src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Services/SharedPubSubMessages.cs (1)
29-30: Add XML documentation for the SESSION_REVOKED constant.For consistency with other constants in this file and to clarify its purpose in session management, consider adding XML documentation.
+ /// <summary> + /// Event triggered when a user session is revoked, prompting client-side session termination. + /// </summary> public const string SESSION_REVOKED = nameof(SESSION_REVOKED);src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Settings/SessionsSection.razor.cs (1)
Line range hint
1-93: Consider extracting session and presence management to dedicated servicesThe component could benefit from better separation of concerns:
- Extract session management logic into an
ISessionManagerservice- Move presence calculation to a reusable
IPresenceManagerserviceThis would improve maintainability and enable reuse across other components.
Would you like me to provide an example implementation of these services?
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Data/AppDbContext.cs (1)
23-23: LGTM! Consider adding XML documentation.The addition of the
UserSessionsDbSet property is well-placed and follows the established patterns in the codebase. Consider adding XML documentation to describe its purpose and relationship with user session management.+ /// <summary> + /// Gets or sets the user sessions in the database. + /// Used for tracking active user sessions and managing device notifications. + /// </summary> public DbSet<UserSession> UserSessions { get; set; } = default!;src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/PageTests/IdentityPagesTests.cs (2)
72-73: LGTM! Consider adding clarifying comments.The change from
ReloadAsync()to explicit session loading viaReference(u => u.Sessions).LoadAsync()aligns well with the PR's focus on session management. This makes the test's intent clearer by explicitly loading only the required navigation property.Consider adding a comment explaining why we specifically load the Sessions:
+ // Explicitly load Sessions to verify the session count after sign-in await dbContext.Entry(user).Reference(u => u.Sessions).LoadAsync(); Assert.AreEqual(1, user.Sessions.Count);
78-79: LGTM! Consider consolidating session verifications.The session loading and verification logic is correct but duplicated.
Consider extracting a helper method to reduce duplication:
+ private static async Task AssertSessionCount(AppDbContext dbContext, User user, int expectedCount) + { + await dbContext.Entry(user).Reference(u => u.Sessions).LoadAsync(); + Assert.AreEqual(expectedCount, user.Sessions.Count); + }Then use it in both places:
- await dbContext.Entry(user).Reference(u => u.Sessions).LoadAsync(); - Assert.AreEqual(1, user.Sessions.Count); + await AssertSessionCount(dbContext, user, expectedCount: 1); var mainHomePage = await identityHomePage.SignOut(); await mainHomePage.AssertSignOut(); - await dbContext.Entry(user).Reference(u => u.Sessions).LoadAsync(); - Assert.AreEqual(0, user.Sessions.Count); + await AssertSessionCount(dbContext, user, expectedCount: 0);src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/PushNotificationService.cs (1)
63-65: Informative Comments Enhance Code ReadabilityThe added comments effectively explain the rationale behind filtering devices based on session expiration, improving code maintainability and understanding for future developers.
However, consider correcting minor punctuation for clarity:
-// userRelatedPush: If the BearerTokenExpiration is 14 days, it’s not practical to send push notifications to a device where the user hasn't used the app for over 14 days. +// userRelatedPush: If the BearerTokenExpiration is 14 days, it's not practical to send push notifications to a device where the user hasn't used the app for over 14 days.src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Identity/UserController.cs (1)
Line range hint
64-64: Use constants for claim type keysTo improve maintainability and reduce the risk of typos, consider defining constants for claim type keys like
"session-id". This helps avoid magic strings scattered throughout your code.src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Data/Migrations/AppDbContextModelSnapshot.cs (3)
242-271: Enhance Validation for UserSession Entity PropertiesThe
UserSessionentity introduces properties likeAddress,Device, andIPwithout specified length constraints or required fields. To ensure data integrity and prevent potential issues with data storage, consider adding validation attributes.Apply the following enhancements:
- Add
[Required]attributes to properties that should not acceptnullvalues.- Specify maximum lengths using
[MaxLength]for string properties to limit the size of data stored.
257-258: Consistent Nullability for Date PropertiesThe
RenewedOnproperty is defined as nullable (long?), whereasStartedOnis non-nullable (long). IfRenewedOnshould always have a value after the session starts or upon renewal, consider making it non-nullable for consistency and to simplify null checks in the codebase.Also applies to: 260-261
857-861: Review Loading Strategy for Sessions Navigation PropertyThe addition of the
Sessionsnavigation property in theUserentity allows for easy access to a user's sessions. To optimize performance:
- Consider configuring the loading strategy (eager, lazy, or explicit loading) based on how frequently this navigation property is accessed.
- Be mindful of potential performance impacts, especially if a user has many sessions.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Data/Migrations/20241121162743_InitialMigration.Designer.cs (2)
763-770: Review the relationship betweenUserSessionandUserentitiesThe
UserSessionentity is configured to have a foreign key relationship with theUserentity. Confirm that the cascade delete behavior (OnDelete(DeleteBehavior.Cascade)) is appropriate for your use case, as deleting a user would remove all associated sessions.Consider whether a different delete behavior might be more suitable to prevent unintentional data loss.
861-863: Verify the navigation propertySessionsin theUserentityThe
Userentity'sSessionsnavigation property is introduced. Ensure that lazy loading, eager loading, or explicit loading strategies are correctly implemented when accessing user sessions to optimize performance and avoid unintended data retrieval.If necessary, configure the loading behavior using
.UseLazyLoadingProxies()or include statements in queries.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (18)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/ClientAppCoordinator.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Settings/SessionsSection.razor.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Identity/IdentityController.ResetPassword.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Identity/IdentityController.cs(7 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Identity/UserController.cs(5 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/PushNotification/PushNotificationController.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Data/AppDbContext.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Data/Configurations/Identity/UserConfiguration.cs(0 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Data/Migrations/20241121162743_InitialMigration.Designer.cs(16 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Data/Migrations/20241121162743_InitialMigration.cs(6 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Data/Migrations/AppDbContextModelSnapshot.cs(15 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Models/Identity/User.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Models/Identity/UserSession.cs(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Models/PushNotification/DeviceInstallation.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/PushNotificationService.cs(3 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Dtos/Identity/UserSessionDto.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Services/SharedPubSubMessages.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Tests/PageTests/IdentityPagesTests.cs(1 hunks)
💤 Files with no reviewable changes (1)
- src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Data/Configurations/Identity/UserConfiguration.cs
✅ Files skipped from review due to trivial changes (1)
- src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Models/Identity/User.cs
🔇 Additional comments (33)
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Dtos/Identity/UserSessionDto.cs (1)
5-5: LGTM! Verify usages of the renamed property.
The property rename from SessionUniqueId to Id aligns with standard entity naming conventions and supports the new session-aware device installation system.
Let's verify all usages of this DTO to ensure the property rename has been consistently applied:
✅ Verification successful
Property rename from SessionUniqueId to Id has been consistently applied
Based on the code analysis:
- The
UserSessionDto.Idproperty is correctly used inUserController.csfor session management operations - The mapping is properly configured in
IdentityMapper.csusing Mapperly - The property is correctly referenced in
SessionsSection.razor.csfor UI operations - The interface
IUserController.csuses the DTO with the renamed property
No references to the old property name SessionUniqueId were found, and all code consistently uses the new Id property, maintaining proper functionality for session management and device installation relationships.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to SessionUniqueId
# and verify Id property usage in UserSessionDto
# Check for any remaining references to old property name
rg "SessionUniqueId"
# Find usages of UserSessionDto to verify property access
ast-grep --pattern 'UserSessionDto $var = $_'
Length of output: 184
Script:
#!/bin/bash
# Let's try a more comprehensive search approach
# 1. Search for any references to UserSessionDto class
rg -l "UserSessionDto"
# 2. Search for any property access patterns that might use Id or SessionUniqueId
rg "\.SessionUniqueId|\.Id.*UserSession" -A 2 -B 2
# 3. Look for UserSessionDto class definition and usages with AST
ast-grep --pattern 'class $_ {
$$$
UserSessionDto $$$
$$$
}'
# 4. Search for method parameters or return types using UserSessionDto
ast-grep --pattern 'UserSessionDto $_($$$)'
Length of output: 6208
Script:
#!/bin/bash
# Let's examine the content of files that reference UserSessionDto
# Check the content of the files that use UserSessionDto
echo "=== Checking UserController.cs ==="
cat "src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Identity/UserController.cs"
echo "=== Checking IdentityMapper.cs ==="
cat "src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Mappers/IdentityMapper.cs"
echo "=== Checking SessionsSection.razor.cs ==="
cat "src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Settings/SessionsSection.razor.cs"
echo "=== Checking IUserController.cs ==="
cat "src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Controllers/Identity/IUserController.cs"
Length of output: 21254
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Models/Identity/UserSession.cs (2)
Line range hint 1-21: Well-structured class with appropriate property definitions
The class effectively combines session metadata, timing information, and user relationship. The use of nullable properties and DateTimeOffset shows good attention to detail.
5-5: Verify the impact of renaming SessionUniqueId to Id
The property rename follows better naming conventions, but this is a breaking change that requires careful verification.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Models/PushNotification/DeviceInstallation.cs (2)
20-35: Verify data migration strategy for existing installations.
The model changes look good, but ensure there's a migration strategy to handle existing device installations:
- How will existing installations be associated with the correct user sessions?
- What value will be set for the new
RenewedOnfield for existing records?
Let's check for migration files:
20-23: LGTM! Verify cascade behavior for session revocation.
The change from UserId to UserSessionId aligns well with the PR objectives, enabling better control over notification delivery based on session validity.
Let's verify the cascade behavior when sessions are revoked:
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Services/SharedPubSubMessages.cs (1)
30-30: LGTM! The constant addition aligns with session management requirements.
The new SESSION_REVOKED constant follows the established pattern using nameof for type-safety and is appropriately placed in the SignalREvents class.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/PushNotification/PushNotificationController.cs (2)
30-30: LGTM: RequestPush change aligns with session-awareness goal
The addition of false parameter correctly indicates this dev-only endpoint sends non-user-related pushes, aligning with the PR's goal of making notifications session-aware.
Let's verify if production push notification paths are properly session-aware:
✅ Verification successful
All production push notification paths are properly session-aware
Based on the search results, all production RequestPush calls in IdentityController correctly set userRelatedPush: true and include a proper customDeviceFilter that filters by user ID. The only instance with userRelatedPush: false is in the dev-only PushNotificationController, which is appropriate.
- ✓ IdentityController.ResetPassword.cs:
userRelatedPush: truewith user ID filter - ✓ IdentityController.cs (2 instances): Both use
userRelatedPush: truewith user ID filter - ✓ PushNotificationController.cs: Dev-only endpoint correctly uses
userRelatedPush: false
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check all RequestPush calls to ensure they specify userRelatedPush parameter
rg -A 3 "RequestPush\("
# Check PushNotificationService implementation
ast-grep --pattern 'class PushNotificationService {
$$$
RequestPush($$$) {
$$$
}
}'
Length of output: 4364
Line range hint 8-8: Review security implications of [AllowAnonymous]
The controller is marked with [AllowAnonymous] which could pose security risks. While this might be necessary for initial device registration, consider:
- Adding rate limiting to prevent abuse
- Implementing device verification
- Requiring authentication for deregistration
Let's check if there are any security measures in place:
✅ Verification successful
[AllowAnonymous] is appropriate for device registration with built-in security measures
The [AllowAnonymous] attribute on PushNotificationController is justified because:
-
The service layer (
PushNotificationService) implements session-aware security:- Tracks user authentication state via
User.IsAuthenticated() - Associates devices with user sessions through
UserSessionId - Maintains proper session relationships in the database
- Tracks user authentication state via
-
Device registration flow is secure:
- Devices are linked to user sessions when authenticated
- Devices can register without authentication for initial setup
- Session-user relationships are properly maintained in the database schema
-
Production push notifications are session-aware:
userRelatedPushparameter controls session-based delivery- Device installations are properly linked to user sessions
- Session invalidation automatically affects push notification delivery
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for rate limiting or security middleware
rg -A 5 "UseRateLimiting|ThrottlePolicy"
# Check for device verification in the service
ast-grep --pattern 'class PushNotificationService {
$$$
RegisterDevice($$$) {
$$$
}
}'
Length of output: 257
Script:
#!/bin/bash
# Let's try a broader search for the service implementation and security measures
# Search for PushNotification service/controller implementations
rg -l "PushNotification" --type cs
# Once we find relevant files, let's check their content
rg -A 10 "RegisterDevice|DeregisterDevice" --type cs
# Look for any security middleware or authentication configuration
rg -A 5 "UseAuthentication|AddAuthentication|ConfigureAuth" --type cs
# Check for any custom security attributes or filters
rg ":\s*AuthorizeAttribute|SecurityFilter" --type cs
Length of output: 24336
Script:
#!/bin/bash
# Let's check the PushNotificationService implementation for security measures
rg -A 20 "class PushNotificationService" --type cs
# Check if there's any user validation in the service
rg "User\.IsAuthenticated|GetSessionId|UserSession" --type cs
# Look for any rate limiting configuration
rg -A 5 "RateLimiting|ThrottlePolicy|AddRateLimiter" --type cs
# Check for any security-related attributes on the controller
rg -B 5 "class PushNotificationController" --type cs
Length of output: 23674
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Settings/SessionsSection.razor.cs (1)
42-42: LGTM! Verify integration with session revocation flow
The changes to session identification and revocation look good. The code properly prevents revoking the current session and handles concurrent operations.
Let's verify the integration with the session revocation flow:
Also applies to: 48-48
✅ Verification successful
The verification results show a complete end-to-end session revocation flow:
- The
SessionsSection.razor.cscomponent properly prevents revoking the current session - The server's
UserControllerbroadcasts theSESSION_REVOKEDevent to the specific user - The
ClientAppCoordinatorhandles the event by checking if the revoked session matches the current user's session
Let me verify one more aspect to ensure the session revocation is properly integrated with the authentication state:
Session revocation flow integration verified successfully
The verification confirms proper integration of the session revocation flow:
- The
SessionsSectioncomponent correctly retrieves and compares the current session ID - The
ClientAppCoordinatorhandles revoked sessions by triggering sign-out when the current session is revoked - Session ID handling is consistent across the codebase using the
GetSessionIdextension method - The server properly broadcasts session revocation events to affected users
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the session revocation handling across the codebase
# Check for SESSION_REVOKED event handler implementation
rg -A 5 "SESSION_REVOKED"
# Check for other components that might be affected by session revocation
ast-grep --pattern 'RevokeSession($_, $_)'
Length of output: 2307
Script:
#!/bin/bash
# Check how the authentication state is handled during session revocation
ast-grep --pattern 'AuthenticationStateProvider.NotifyAuthenticationStateChanged'
# Check for session-related extensions used in the flow
rg -A 3 "GetSessionId"
Length of output: 3489
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Identity/IdentityController.ResetPassword.cs (2)
60-60: Verify the security implications of session-based filtering
The change to use UserSession for filtering devices is a security improvement, but we should verify that this doesn't create any edge cases where critical password reset notifications might be missed.
Let's check the session management implementation:
Consider documenting this security-critical behavior in the API documentation or swagger annotations to make it clear that password reset notifications are session-dependent.
✅ Verification successful
Session-based filtering for password reset notifications is secure and appropriate
The code review shows that the session-based filtering is implemented correctly and securely for password reset notifications:
- The filter
d => d.UserSession!.UserId == user.Idensures notifications only go to devices with active sessions belonging to the user - The implementation is consistent with other security-sensitive operations in the codebase
- Password reset tokens are independently validated and have their own lifecycle management
- The notification is just a supplementary feature - the main reset password flow works through email/SMS tokens that are sent through verified channels
The current implementation actually enhances security by ensuring password reset notifications only reach devices with valid active sessions, while not compromising the core password reset functionality that works through verified email/SMS channels.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check how sessions are managed during sign-out/revocation
ast-grep --pattern 'void $_(UserSession $_, $$$) {
$$$
}'
# Look for other security-sensitive operations that might need similar session checks
rg -A 5 "RequestPush.*userRelatedPush:\s*false"
Length of output: 111
Script:
#!/bin/bash
# Let's check the implementation of RequestPush and session handling
ast-grep --pattern 'RequestPush($$$) {
$$$
}'
# Check how UserSession is used in push notifications
rg -A 5 "UserSession.*RequestPush"
# Look for password reset related push notifications
rg -A 5 "password.*RequestPush"
# Check session management in identity controller
fd "Identity.*Controller" --exec cat {}
Length of output: 38981
Script:
#!/bin/bash
# Let's check how UserSession is used in device filtering across the codebase
rg -A 3 "customDeviceFilter.*UserSession.*UserId"
# Check for any potential session invalidation during password reset
rg -A 5 "ResetPassword.*UserSession"
# Look for any device or session management during password changes
ast-grep --pattern 'ResetPassword($$$) {
$$$
UserSession $$$
$$$
}'
Length of output: 2440
60-60: 🛠️ Refactor suggestion
Verify null safety in UserSession access
While the change to use UserSession!.UserId aligns with the PR's goal of making notifications session-aware, the null-forgiving operator (!) might mask potential null reference issues. Consider adding explicit null handling.
Let's verify the null safety guarantees:
Consider this safer alternative:
-sendMessagesTasks.Add(pushNotificationService.RequestPush(message: message, userRelatedPush: true, customDeviceFilter: d => d.UserSession!.UserId == user.Id, cancellationToken: cancellationToken));
+sendMessagesTasks.Add(pushNotificationService.RequestPush(message: message, userRelatedPush: true, customDeviceFilter: d => d.UserSession?.UserId == user.Id, cancellationToken: cancellationToken));✅ Verification successful
The null-forgiving operator is appropriate in this context
After reviewing the implementation, the use of the null-forgiving operator (!) in d.UserSession!.UserId is safe because:
- The
RequestPushmethod already filters devices based onuserRelatedPushparameter, which ensures only devices with valid sessions within the bearer token expiration window are included. - The
DeviceInstallationmodel showsUserSessionis properly linked via foreign key relationship. - The
customDeviceFilteris only applied after the base query filters out expired sessions.
The current implementation provides sufficient null-safety guarantees through the query filters, making the null-forgiving operator a valid choice.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if there are any places where devices can be created without UserSession
ast-grep --pattern 'new Device {
$$$
}'
# Check if there are any places where UserSession can be null
rg -A 2 "UserSession\s+=\s+null"
Length of output: 194
Script:
#!/bin/bash
# Let's try a different approach to check device creation and user session handling
# Check for Device class/interface definition and its properties
ast-grep --pattern 'class Device {
$$$
}'
# Also check interface definition
ast-grep --pattern 'interface Device {
$$$
}'
# Look for UserSession property/field definitions and usage
rg -l "UserSession"
# Check for device creation or registration patterns
rg -A 3 "Device\s+="
rg -A 3 "CreateDevice"
Length of output: 5619
Script:
#!/bin/bash
# Let's check the DeviceInstallation model and PushNotificationService implementation
# Check DeviceInstallation model since it was found in the files
cat "src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Models/PushNotification/DeviceInstallation.cs"
# Check PushNotificationService implementation
cat "src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/PushNotificationService.cs"
# Check UserSession model
cat "src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Models/Identity/UserSession.cs"
Length of output: 6493
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Data/AppDbContext.cs (1)
23-23: Verify UserSession entity configuration.
Since this is a new entity, ensure its configuration is properly defined.
✅ Verification successful
Let me gather more information about the UserSession entity and its configuration.
Let me check the migration files to see how the UserSession entity is configured in the database schema.
Entity configuration for UserSession is properly defined
The UserSession entity is well-configured in the database schema with:
- Primary key (Id)
- Required relationships (UserId foreign key to Users table with cascade delete)
- All necessary properties (IP, Device, Address, StartedOn, RenewedOn)
- Proper navigation properties (User relationship)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for UserSession entity configuration and its relationship with User entity
# Look for UserSession configuration in Configurations folder
echo "Searching for UserSession configuration..."
rg -l "UserSession" --type cs src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Data/Configurations/
# Look for entity configuration in OnModelCreating
ast-grep --pattern 'protected override void OnModelCreating(ModelBuilder modelBuilder)
{
$$$
modelBuilder.Entity<UserSession>()$$$
$$$
}'
Length of output: 367
Script:
#!/bin/bash
# Search for UserSession class definition and any configuration
echo "Searching for UserSession class definition..."
rg -A 10 "class UserSession" --type cs
# Search for any files containing UserSession in the Data directory
echo -e "\nSearching for UserSession in Data directory..."
rg -l "UserSession" --type cs src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Data/
# Check if there's any IEntityTypeConfiguration implementation for UserSession
echo -e "\nSearching for IEntityTypeConfiguration..."
ast-grep --pattern 'class $_ : IEntityTypeConfiguration<UserSession>
{
$$$
}'
Length of output: 3738
Script:
#!/bin/bash
# Check the initial migration for UserSession configuration
echo "Checking migration for UserSession configuration..."
rg -A 20 "UserSessions" src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Data/Migrations/20241121162743_InitialMigration.cs
# Check model snapshot for UserSession configuration
echo -e "\nChecking model snapshot for UserSession configuration..."
rg -A 20 "entity.HasKey|UserSession" src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Data/Migrations/AppDbContextModelSnapshot.cs
Length of output: 9228
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/ClientAppCoordinator.cs (2)
208-217: Verify integration with device and notification services
The session revocation changes should be coordinated with device registration and push notification services to ensure proper cleanup.
Let's verify the integration points:
✅ Verification successful
Based on the search results, I can now generate the final response since I have enough information about the integration between session management, device registration, and authentication:
Session revocation is properly integrated with device and notification services
The session revocation handler in ClientAppCoordinator.cs is correctly integrated with:
- Device registration cleanup: When a session is revoked, the UserController.SignOut method cleans up device registrations by setting UserSessionId to null in DeviceInstallations table
- Push notification service: The PushNotificationService already handles user session ID during device registration and maintains proper session-device mapping
- Authentication state: The AuthenticationManager properly handles SignOut, triggers AuthenticationStateChanged events, and coordinates with the rest of the system
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for device registration patterns
rg -A 5 'RegisterDevice|UnregisterDevice'
# Search for push notification handling
rg -A 5 'pushNotificationService'
# Search for authentication state changes
rg -A 5 'AuthenticationStateChanged|SignOut'
Length of output: 63328
208-217: Review SignalR reconnection strategy after session revocation
The current implementation might lead to unnecessary reconnection attempts after session revocation. Consider these architectural improvements:
- Stop reconnection attempts after session revocation
- Clear the connection state after session revocation
- Ensure the handler isn't registered multiple times on reconnection
Let's verify the SignalR reconnection behavior:
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/PushNotificationService.cs (4)
12-16: Dependency Injection Updates Look Good
The added injections for IHttpContextAccessor and ServerApiSettings are appropriate for the new functionalities added to the service.
38-39: Assignment of userSessionId and Timestamp Fields
Assigning userSessionId, RenewedOn, and ExpirationTime enhances the tracking of device installations relative to user sessions. This looks good and aligns with the PR objectives.
56-59: Addition of userRelatedPush Parameter Enhances Functionality
The introduction of the userRelatedPush parameter allows the RequestPush method to differentiate between user-specific and general push notifications, which improves control over notification dispatch.
69-70: Ensure Consistent Time Units in Timestamp Calculations
When calculating (now - dev.RenewedOn) and comparing it to BearerTokenExpiration.TotalSeconds, ensure that all time units are consistent and that there are no potential overflows or type mismatches.
To confirm that the time units and data types are consistent, consider reviewing the types of dev.RenewedOn, now, and BearerTokenExpiration.TotalSeconds.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Identity/UserController.cs (2)
85-85: Duplicate comment: Use constants for claim type keys
Refer to the previous comment about using constants for claim type keys to enhance maintainability.
94-94: Verify that notifying all user clients upon session revocation is intended
The current implementation sends the SESSION_REVOKED event to all clients associated with the user via Clients.User(userId.ToString()). If the intention is to notify only the specific session that was revoked, consider targeting that session to avoid disrupting other active sessions.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Identity/IdentityController.cs (4)
98-98: User session creation is appropriate
The code correctly creates a new user session using the user's ID and device information.
137-142: User session and user updates are correctly handled
The user session is added to the context, and the user's TwoFactorTokenRequestedOn is reset. Changes are persisted correctly.
165-179: Proper initialization of UserSession and addition of session claims
The CreateUserSession method initializes a new UserSession with all required properties and adds the session ID to the session claims, which is essential for session management.
203-203: User session retrieval is handled correctly
The code retrieves the user session based on the current session ID and appropriately handles the case where the session might not exist.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Data/Migrations/20241121162743_InitialMigration.cs (5)
219-241: Ensure proper handling of sensitive user data in UserSessions table
The UserSessions table stores potentially sensitive information such as IP, Device, and Address. Please ensure that the storage and handling of this data comply with privacy regulations (e.g., GDPR). Consider encrypting sensitive fields or limiting the stored data to what's strictly necessary.
[security_issue]
229-239: Verify cascade delete behavior on UserSessions foreign key
The foreign key constraint on UserSessions uses onDelete: ReferentialAction.Cascade, which means deleting a user will remove all associated sessions. Confirm that this behavior aligns with the application requirements, as it could impact session logs and auditing.
335-337: Index on UserSessionId enhances query performance in DeviceInstallations
Adding an index on the UserSessionId column in the DeviceInstallations table is beneficial for query optimization when retrieving installations associated with a user session.
400-403: Index on UserId improves lookup efficiency in UserSessions
Creating an index on the UserId column in the UserSessions table will facilitate faster queries when accessing sessions for a specific user.
436-438: Confirm that the Down migration fully reverts the Up migration
The Down method drops the UserSessions table. Ensure that all related constraints, foreign keys, and indexes introduced in the Up method are properly reversed to maintain database consistency during rollbacks.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Data/Migrations/AppDbContextModelSnapshot.cs (1)
40-40: Verify Compatibility of PartitionKeyNames Annotation
The change from .HasAnnotation("Cosmos:PartitionKeyName", "Id") to .HasAnnotation("Cosmos:PartitionKeyNames", new List<string> { "Id" }) across multiple entities reflects an update in how partition keys are specified for Cosmos DB.
Please verify that:
- This change is compatible with the current version of the Entity Framework Core Cosmos DB provider.
- The application correctly handles queries and operations with the new pluralized partition key annotation.
Also applies to: 110-110, 216-216, 309-309, 617-617, 638-638, 664-664, 690-690, 715-715, 734-734, 757-757
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Data/Migrations/20241121162743_InitialMigration.Designer.cs (3)
244-274: Confirm the configuration of the UserSession entity
The new UserSession entity has been added with properties and relationships. Review the entity configuration to ensure that all necessary properties are included and correctly configured, such as data types, nullability, and relationships.
42-42: Verify the use of Cosmos:PartitionKeyNames annotation
The migration updates the partition key annotation from Cosmos:PartitionKeyName to Cosmos:PartitionKeyNames with a list of keys. Ensure that this change is compatible with the version of the Cosmos DB EF Core provider being used and that the provider supports multiple partition keys in this manner.
Run the following script to check for the usage and compatibility of Cosmos:PartitionKeyNames in the codebase:
Also applies to: 112-112, 218-218, 311-311, 589-589, 619-619, 640-640, 666-666, 692-692, 717-717, 736-736, 759-759
✅ Verification successful
Cosmos:PartitionKeyNames is correctly used in this initial migration
This is an initial migration in a boilerplate template that properly uses the Cosmos:PartitionKeyNames annotation. The evidence shows:
- The project uses the latest EF Core Cosmos provider without a fixed version (floating version)
- There's no previous usage of the singular
PartitionKeyNameannotation - The annotation is consistently used across all entities in both the migration and snapshot files
- Each entity has a single partition key defined (e.g., "Id", "UserId", "CategoryId", "Platform", "RoleId") which is a valid configuration
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for usages of 'Cosmos:PartitionKeyNames' in the codebase
rg --fixed-strings 'Cosmos:PartitionKeyNames'
Length of output: 5381
Script:
#!/bin/bash
# Check for package references to determine the Cosmos DB provider version
rg -g "*.csproj" "Microsoft.EntityFrameworkCore.Cosmos"
# Check for any previous usage of PartitionKeyName (singular) in migrations
rg --fixed-strings 'Cosmos:PartitionKeyName'
# Look for DbContext configuration to see how Cosmos is configured
ast-grep --pattern 'protected override void OnModelCreating(ModelBuilder $_) {
$$$
}'
Length of output: 5771
Script:
#!/bin/bash
# Check for the specific version of EF Core Cosmos package in csproj files
rg -g "*.csproj" -A 5 -B 5 '<PackageReference.*Microsoft.EntityFrameworkCore.Cosmos'
# Look for any entity configurations that might show how partition keys are set up
ast-grep --pattern 'modelBuilder.Entity<$_>()
$$$'
Length of output: 2674
571-573: Update associated references due to DeviceInstallation changes
The DeviceInstallation entity now includes a UserSessionId property replacing or adding to the previous UserId. Ensure that all references and usages of DeviceInstallation in the codebase are updated to accommodate this change, preventing potential runtime errors due to mismatches.
To verify, search for all usages of DeviceInstallation that may use UserId:
Also applies to: 578-589
...Client/Boilerplate.Client.Core/Components/Pages/Authorized/Settings/SessionsSection.razor.cs
Show resolved
Hide resolved
| hubConnection.On<Guid>(SignalREvents.SESSION_REVOKED, async (userSessionId) => | ||
| { | ||
| var user = (await AuthenticationStateTask).User; | ||
|
|
||
| if (user.IsAuthenticated() && user.GetSessionId() == userSessionId) | ||
| { | ||
| await AuthenticationManager.SignOut(CurrentCancellationToken); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and logging for session revocation
While the session revocation logic is correct, consider these improvements for robustness:
- Add error handling for the SignOut operation
- Add logging for session revocation events
- Consider potential race conditions with concurrent session revocation events
hubConnection.On<Guid>(SignalREvents.SESSION_REVOKED, async (userSessionId) =>
{
- var user = (await AuthenticationStateTask).User;
+ try
+ {
+ logger.LogInformation("Session revocation request received for session {SessionId}", userSessionId);
+ var user = (await AuthenticationStateTask).User;
- if (user.IsAuthenticated() && user.GetSessionId() == userSessionId)
- {
- await AuthenticationManager.SignOut(CurrentCancellationToken);
+ if (user.IsAuthenticated() && user.GetSessionId() == userSessionId)
+ {
+ logger.LogWarning("Current session {SessionId} was revoked, signing out user {UserId}",
+ userSessionId, user.GetUserId());
+ await AuthenticationManager.SignOut(CurrentCancellationToken);
+ }
+ }
+ catch (Exception ex)
+ {
+ logger.LogError(ex, "Error handling session revocation for session {SessionId}", userSessionId);
+ ExceptionHandler.Handle(ex);
}
});Committable suggestion skipped: line range outside the PR's diff.
...rplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/PushNotificationService.cs
Show resolved
Hide resolved
| using Boilerplate.Server.Api.SignalR; | ||
| using Microsoft.AspNetCore.SignalR; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Wrap using directives within conditional compilation directives
To prevent compilation errors when signalR is not enabled, consider wrapping the using statements for SignalR within the same conditional compilation directives as the dependent code.
Apply this diff to wrap the using directives:
+#if (signalR == true)
using Boilerplate.Server.Api.SignalR;
using Microsoft.AspNetCore.SignalR;
+#endif📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| using Boilerplate.Server.Api.SignalR; | |
| using Microsoft.AspNetCore.SignalR; | |
| #if (signalR == true) | |
| using Boilerplate.Server.Api.SignalR; | |
| using Microsoft.AspNetCore.SignalR; | |
| #endif |
| @@ -271,7 +268,7 @@ public async Task SendOtp(IdentityRequestDto request, string? returnUrl = null, | |||
| //#endif | |||
|
|
|||
| //#if (notification == true) | |||
| sendMessagesTasks.Add(pushNotificationService.RequestPush(message: pushMessage, customDeviceFilter: d => d.UserId == user.Id, cancellationToken: cancellationToken)); | |||
| sendMessagesTasks.Add(pushNotificationService.RequestPush(message: pushMessage, userRelatedPush: true, customDeviceFilter: d => d.UserSession!.UserId == user.Id, cancellationToken: cancellationToken)); | |||
There was a problem hiding this comment.
Add null check to prevent NullReferenceException in customDeviceFilter
Dereferencing d.UserSession without a null check may cause a NullReferenceException if UserSession is null. Consider adding a null check in the lambda expression.
Apply this diff to add a null check:
-sendMessagesTasks.Add(pushNotificationService.RequestPush(message: pushMessage, userRelatedPush: true, customDeviceFilter: d => d.UserSession!.UserId == user.Id, cancellationToken: cancellationToken));
+sendMessagesTasks.Add(pushNotificationService.RequestPush(message: pushMessage, userRelatedPush: true, customDeviceFilter: d => d.UserSession != null && d.UserSession.UserId == user.Id, cancellationToken: cancellationToken));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| sendMessagesTasks.Add(pushNotificationService.RequestPush(message: pushMessage, userRelatedPush: true, customDeviceFilter: d => d.UserSession!.UserId == user.Id, cancellationToken: cancellationToken)); | |
| sendMessagesTasks.Add(pushNotificationService.RequestPush(message: pushMessage, userRelatedPush: true, customDeviceFilter: d => d.UserSession != null && d.UserSession.UserId == user.Id, cancellationToken: cancellationToken)); |
| table.PrimaryKey("PK_DeviceInstallations", x => x.InstallationId); | ||
| table.ForeignKey( | ||
| name: "FK_DeviceInstallations_UserSessions_UserSessionId", | ||
| column: x => x.UserSessionId, | ||
| principalTable: "UserSessions", | ||
| principalColumn: "Id"); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Specify onDelete behavior for foreign key to UserSessions
To maintain data integrity, explicitly define the onDelete behavior for the FK_DeviceInstallations_UserSessions_UserSessionId foreign key. If cascading delete is desired, set onDelete: ReferentialAction.Cascade; otherwise, use Restrict or SetNull as appropriate.
Apply the following change to specify the delete behavior:
table.ForeignKey(
name: "FK_DeviceInstallations_UserSessions_UserSessionId",
column: x => x.UserSessionId,
principalTable: "UserSessions",
principalColumn: "Id"
+ onDelete: ReferentialAction.Cascade);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| table.PrimaryKey("PK_DeviceInstallations", x => x.InstallationId); | |
| table.ForeignKey( | |
| name: "FK_DeviceInstallations_UserSessions_UserSessionId", | |
| column: x => x.UserSessionId, | |
| principalTable: "UserSessions", | |
| principalColumn: "Id"); | |
| }); | |
| table.PrimaryKey("PK_DeviceInstallations", x => x.InstallationId); | |
| table.ForeignKey( | |
| name: "FK_DeviceInstallations_UserSessions_UserSessionId", | |
| column: x => x.UserSessionId, | |
| principalTable: "UserSessions", | |
| principalColumn: "Id", | |
| onDelete: ReferentialAction.Cascade); | |
| }); |
| .IsRequired() | ||
| .HasColumnType("TEXT"); | ||
|
|
||
| b.Property<Guid?>("UserId") | ||
| b.Property<Guid?>("UserSessionId") |
There was a problem hiding this comment.
Ensure Mandatory Association with UserSession
In the DeviceInstallation entity, the UserSessionId property is currently nullable (Guid?). If every DeviceInstallation must be associated with a UserSession, it's important to enforce this at the database level.
Apply this diff to make UserSessionId non-nullable:
-b.Property<Guid?>("UserSessionId")
+ b.Property<Guid>("UserSessionId")This change will:
- Enforce the presence of a
UserSessionIdfor allDeviceInstallationrecords. - Help maintain data integrity by preventing orphaned device installations.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| b.Property<Guid?>("UserSessionId") | |
| b.Property<Guid>("UserSessionId") |
| b.HasOne("Boilerplate.Server.Api.Models.Identity.UserSession", "UserSession") | ||
| .WithMany() | ||
| .HasForeignKey("UserId"); | ||
| .HasForeignKey("UserSessionId"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Configure Required Relationship and Cascade Delete
The relationship between DeviceInstallation and UserSession is currently optional and does not specify delete behavior. If a DeviceInstallation should always be linked to a UserSession, and you want to remove device installations when their associated user session is deleted, you should configure the relationship accordingly.
Apply this diff to enforce the required relationship and cascade delete:
-b.HasOne("Boilerplate.Server.Api.Models.Identity.UserSession", "UserSession")
- .WithMany()
- .HasForeignKey("UserSessionId");
+ b.HasOne("Boilerplate.Server.Api.Models.Identity.UserSession", "UserSession")
+ .WithMany()
+ .HasForeignKey("UserSessionId")
+ .OnDelete(DeleteBehavior.Cascade)
+ .IsRequired();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| b.HasOne("Boilerplate.Server.Api.Models.Identity.UserSession", "UserSession") | |
| .WithMany() | |
| .HasForeignKey("UserId"); | |
| .HasForeignKey("UserSessionId"); | |
| b.HasOne("Boilerplate.Server.Api.Models.Identity.UserSession", "UserSession") | |
| .WithMany() | |
| .HasForeignKey("UserSessionId") | |
| .OnDelete(DeleteBehavior.Cascade) | |
| .IsRequired(); |
| @@ -6,7 +6,7 @@ | |||
| namespace Boilerplate.Server.Api.Data.Migrations; | |||
|
|
|||
| [DbContext(typeof(AppDbContext))] | |||
| [Migration("20241107182721_InitialMigration")] | |||
| [Migration("20241121162743_InitialMigration")] | |||
There was a problem hiding this comment.
Avoid modifying existing migrations; create a new migration instead
Modifying an existing migration (InitialMigration) after it has been applied can lead to inconsistencies between the codebase and the database schema. It's best practice to create a new migration to reflect changes in the model.
| b.HasOne("Boilerplate.Server.Api.Models.Identity.UserSession", "UserSession") | ||
| .WithMany() | ||
| .HasForeignKey("UserId"); | ||
| .HasForeignKey("UserSessionId"); | ||
|
|
||
| b.Navigation("User"); | ||
| b.Navigation("UserSession"); |
There was a problem hiding this comment.
Ensure nullable foreign key in DeviceInstallation is handled correctly
The UserSessionId in DeviceInstallation is a nullable GUID. Ensure that the code accounts for the possibility of UserSessionId being null to avoid null reference exceptions when accessing the UserSession navigation property.
Apply this diff to add null checks where necessary:
- var sessionInfo = deviceInstallation.UserSession.SessionInfo;
+ var sessionInfo = deviceInstallation.UserSession?.SessionInfo;Committable suggestion skipped: line range outside the PR's diff.
ysmoradi
deleted the
9304-boilerplate-project-template-must-stop-sending-user-related-push-notification-and-signalr-messages-to-a-device-that-its-user-session-is-not-valid-anymore
branch
This closes #9304
Summary by CodeRabbit
Release Notes
New Features
UserSessionentity to manage user sessions in the database.SESSION_REVOKEDto improve session-related messaging.Bug Fixes
Refactor
UserSessionstructure for better clarity and functionality.Tests