Added login with password encrypted#272
Conversation
- Added EncryptionHelper to help with encryption. - Added basic password validation for the login. - Right now its not possible to just connect with username as before.
WalkthroughThe changes involve updates to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LoginHandler
participant Global
User->>LoginHandler: Login(username, password, machineID)
LoginHandler->>Global: Validate login
alt Login successful
LoginHandler-->>User: Send success message and character list
else Login failed
LoginHandler-->>User: Send error message
end
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Outside diff range and nitpick comments (3)
Maple2.Server.Core/Helpers/EncryptionHelper.cs (2)
7-14: LGTM with suggestion: Consider combining salt and hash for easier storage.The
HashPasswordmethod uses strong cryptographic practices:
- Utilizes
Rfc2898DeriveBytes(PBKDF2) with SHA256- High iteration count (100,000) for brute-force protection
- Appropriate hash length (32 bytes) and Base64 encoding for storage
Consider combining the salt and hash in the output for easier storage and retrieval. Here's a suggested implementation:
public static string HashPassword(string password, byte[] salt) { using (var rfc2898DeriveBytes = new Rfc2898DeriveBytes(password, salt, 100000, HashAlgorithmName.SHA256)) { byte[] hash = rfc2898DeriveBytes.GetBytes(32); // 32 bytes for SHA-256 byte[] hashBytes = new byte[48]; // 16 bytes for salt + 32 bytes for hash Array.Copy(salt, 0, hashBytes, 0, 16); Array.Copy(hash, 0, hashBytes, 16, 32); return Convert.ToBase64String(hashBytes); } }This change would require adjusting the
VerifyPasswordmethod accordingly.
23-29: LGTM with suggestion: Simplify salt generation.The
GenerateSaltmethod correctly uses a cryptographically secure random number generator to create a 16-byte salt, which is a suitable size for most applications.Consider simplifying the method using
RandomNumberGenerator.GetBytes()directly:public static byte[] GenerateSalt() { return RandomNumberGenerator.GetBytes(16); }This change reduces the number of lines while maintaining the same functionality and security level.
Maple2.Database/Model/Account.cs (1)
121-123: Approved: Database configuration updatesThe changes to the
Configuremethod are appropriate:
- Ensuring uniqueness for
Usernameis a good practice.- Marking
Passwordas required is correct.- Setting a default value for
MaxCharactersis appropriate.Consider adding a maximum length constraint to the
Passwordproperty to prevent potential issues with extremely long passwords:builder.Property(account => account.Password).IsRequired().HasMaxLength(255);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
Maple2.Server.World/Migrations/20221027102305_InitialCreate.csis excluded by!Maple2.Server.World/Migrations/*
📒 Files selected for processing (4)
- Maple2.Database/Model/Account.cs (7 hunks)
- Maple2.Model/Game/User/Account.cs (2 hunks)
- Maple2.Server.Core/Helpers/EncryptionHelper.cs (1 hunks)
- Maple2.Server.World/Service/GlobalService.cs (3 hunks)
🔇 Additional comments (5)
Maple2.Server.Core/Helpers/EncryptionHelper.cs (2)
1-6: LGTM: Class structure and namespace are well-defined.The
EncryptionHelperclass is appropriately placed in theMaple2.Server.Core.Helpersnamespace. The class structure as a public utility class with static methods is suitable for its purpose of providing encryption-related functions.
1-30: Overall assessment: Solid implementation with room for minor enhancements.The
EncryptionHelperclass successfully addresses the PR objective of enhancing security for user authentication. It provides a strong foundation for secure password handling with appropriate use of cryptographic functions.Key strengths:
- Use of PBKDF2 with SHA256 for password hashing
- High iteration count for brute-force protection
- Secure salt generation
Suggested improvements:
- Combine salt and hash for easier storage
- Use time-constant comparison for password verification
- Simplify salt generation method
These enhancements will further strengthen the security and maintainability of the authentication system.
Maple2.Model/Game/User/Account.cs (2)
33-33:⚠️ Potential issueImprove encapsulation and security of the
SaltpropertyWhile adding a salt for password hashing is a good practice, the current implementation exposes the salt publicly, which could potentially be misused.
Consider the following improvements:
- Make the
Saltproperty private or internal.- Generate and set the salt automatically when setting the password.
- Ensure the salt is unique for each user and is generated using a cryptographically secure random number generator.
Example refactoring:
private byte[] Salt { get; set; } public void SetPassword(string password) { Salt = GenerateSecureRandomSalt(); PasswordHash = HashPassword(password, Salt); }This approach ensures that the salt is properly encapsulated and can only be manipulated through controlled methods, improving overall security.
To ensure salt generation is implemented securely, run:
#!/bin/bash # Search for salt generation implementation rg -i 'generatesalt|createsalt|newsalt' --type csharp
Line range hint
1-43: Ensure comprehensive implementation of secure authenticationThe addition of
PasswordandSaltproperties to theAccountclass is a step towards improving authentication. However, a robust authentication system requires more than just these properties.Consider the following recommendations:
- Implement the
EncryptionHelpermentioned in the PR objectives to handle password hashing and verification.- Ensure password validation is implemented, possibly in a separate service.
- Review and update all authentication-related code to use these new properties securely.
- Implement proper error handling and logging for authentication attempts.
- Consider using a well-established authentication library or framework instead of building everything from scratch.
To get an overview of the current authentication implementation, run:
#!/bin/bash # Search for authentication-related code rg -i 'auth|login|password' --type csharp # Search for potential usage of encryption or hashing rg -i 'encrypt|hash|bcrypt|argon2|pbkdf2' --type csharpThis will help ensure that the new properties are being used correctly throughout the codebase and that proper encryption/hashing is being implemented.
Maple2.Server.World/Service/GlobalService.cs (1)
41-41:⚠️ Potential issueHandle potential
NullReferenceExceptionwhenrequest.Usernameis null in DEBUG modeSimilarly,
request.Usernamemay benullin DEBUG mode, leading to aNullReferenceExceptionwhen callingTrim(). Use a null-coalescing operator to ensure safe handling.Apply this diff to fix the issue:
- string username = request.Username.Trim().ToLower(); + string username = (request.Username ?? "").Trim().ToLower();Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
Maple2.Server.World/Service/GlobalService.cs (3)
31-35: Good addition of password validationThe explicit check for empty or whitespace-only passwords is a good security enhancement. The error response is consistent with the invalid username check.
Consider using a constant for the error message to maintain consistency and ease future updates:
private const string InvalidPasswordMessage = "Invalid Password."; // Then use it in the response Message = InvalidPasswordMessage
74-80: Secure password verification implementedThe implementation of password verification using the stored hash and salt is correct and secure. This is a crucial security feature for the login process.
For consistency with the earlier suggestion, consider using a constant for the error message:
private const string IncorrectPasswordMessage = "Incorrect Password."; // Then use it in the response Message = IncorrectPasswordMessage
87-87: Consider a more user-friendly error messageWhile the updated error message is more specific, it might be exposing internal implementation details to the user. Consider using a more user-friendly message that doesn't reveal system internals:
Message = "Unable to verify device. Please contact support if this issue persists."
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- Maple2.Server.Core/Helpers/EncryptionHelper.cs (1 hunks)
- Maple2.Server.World/Service/GlobalService.cs (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Maple2.Server.Core/Helpers/EncryptionHelper.cs
🔇 Additional comments (4)
Maple2.Server.World/Service/GlobalService.cs (4)
25-29: Improved error handling for invalid usernamesThe change from throwing an
RpcExceptionto returning a structuredLoginResponsefor invalid usernames is a good improvement. It provides better error handling and aligns with best practices for API design.
53-53: Secure storage of hashed password and saltThe account creation process now correctly stores the hashed password and salt instead of a plain text password. This is consistent with the earlier hashing implementation and follows security best practices.
Also applies to: 55-55
Line range hint
1-100: Overall assessment: Significant security improvementsThis PR successfully implements encrypted password login as intended. Key improvements include:
- Proper password hashing and salting for new accounts.
- Secure password verification for existing accounts.
- Improved error handling with specific error codes and messages.
These changes greatly enhance the security of the login process. The code is well-structured and follows good practices for the most part. A few minor suggestions have been made for consistency and user-friendliness, but they don't detract from the overall quality of the implementation.
48-50: Good implementation of salted password hashingThe use of salted hashes for password storage is a good security practice. However, ensure that the
EncryptionHelperclass is using strong, industry-standard cryptographic methods.Please verify the implementation of the
EncryptionHelperclass:✅ Verification successful
Verified: Secure Password Hashing Implementation
The
EncryptionHelperclass utilizesRfc2898DeriveByteswithSHA256and a high iteration count (100,000), aligning with industry-standard security practices for password hashing.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of EncryptionHelper # Test: Search for the EncryptionHelper class definition ast-grep --lang csharp --pattern $'class EncryptionHelper { $$$ }' # Test: Check for use of secure hashing algorithms rg --type csharp -i '(SHA256|SHA512|BCrypt|Argon2)'Length of output: 286
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (7)
Maple2.Server.Core/Maple2.Server.Core.csproj (1)
14-14: Approved: Addition of BCrypt.Net-Next packageThe inclusion of the BCrypt.Net-Next package (version 4.0.3) aligns well with the PR objectives of implementing login with encrypted passwords and basic password validation. BCrypt is a robust choice for password hashing, enhancing the security of user authentication.
Consider adding a comment explaining the purpose of this package in the context of your project. For example:
<!-- BCrypt.Net-Next: Used for secure password hashing in user authentication --> <PackageReference Include="BCrypt.Net-Next" Version="4.0.3" />This can help other developers understand the rationale behind including this package.
Maple2.Server.World/Service/GlobalService.cs (4)
31-35: Good addition of password validationThe new check for empty passwords enhances security and aligns with the PR's objective of implementing basic password validation. This is a good improvement.
However, consider using a more specific error message:
- Message = "Invalid Password." + Message = "Password cannot be empty."This change would provide more precise feedback to the user about why their login attempt failed.
48-52: Excellent implementation of secure password hashingThe introduction of BCrypt for password hashing is a significant security improvement. Using a work factor of 13 is a good balance between security and performance.
To further enhance this implementation:
Consider extracting the work factor to a constant or configuration value:
private const int BcryptWorkFactor = 13; // ... string hashedPassword = BCrypt.Net.BCrypt.HashPassword(password, BcryptWorkFactor);This would make it easier to adjust the work factor in the future if needed.
72-78: Secure password verification implementedThe addition of password verification using
BCrypt.Verifyis crucial for secure authentication. This change completes the secure password handling process and aligns well with the PR's security objectives.Consider updating the error message for clarity:
- Message = "Incorrect Password." + Message = "Invalid username or password."This change avoids revealing whether the username or password was incorrect, which is a security best practice.
81-81: Good addition of MachineId mismatch loggingAdding a warning log for MachineId mismatches is a good security practice. It helps in detecting potential unauthorized access attempts.
To make the log more informative:
- logger.Warning($"MachineId mismatch for account {account.Id}"); + logger.Warning($"MachineId mismatch for account {account.Id}. Expected: {account.MachineId}, Received: {machineId}");This change provides more context, making it easier to investigate any issues.
Maple2.Server.Login/PacketHandlers/LoginHandler.cs (2)
45-46: Remove Obsolete TODO CommentSince we're addressing the password logging issue, the associated TODO comment can be removed to keep the codebase clean.
Apply this diff to remove the TODO comment:
- // TODO: At some point remove this otherwise well leak of passwords
Line range hint
56-60: Address the TODO: Handle Already Logged-In UsersThe code contains a TODO comment about handling scenarios where the account is already logged into the game server. Implementing this check is important to prevent multiple logins from the same account, which could cause data inconsistencies or security vulnerabilities.
Would you like assistance in implementing this functionality, or should we open a GitHub issue to track this task?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
Maple2.Server.World/Migrations/20221027102305_InitialCreate.csis excluded by!Maple2.Server.World/Migrations/*
📒 Files selected for processing (4)
- Maple2.Database/Model/Account.cs (4 hunks)
- Maple2.Server.Core/Maple2.Server.Core.csproj (1 hunks)
- Maple2.Server.Login/PacketHandlers/LoginHandler.cs (2 hunks)
- Maple2.Server.World/Service/GlobalService.cs (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Maple2.Database/Model/Account.cs
🔇 Additional comments (5)
Maple2.Server.World/Service/GlobalService.cs (4)
25-29: Improved error handling for invalid usernamesThe change from raising an
RpcExceptionto returning aLoginResponsewith a specific error code and message is a good improvement. It provides more informative feedback to the client and aligns with the PR's objective of enhancing the login process.
41-41: Improved password handlingGreat job addressing the previous review comment. By removing the
ToLower()call, you've maintained the password's complexity and case sensitivity, which is crucial for security. TheTrim()operation is acceptable as it only removes leading/trailing whitespace.
85-85: Improved error message for MachineId mismatchThe updated error message "MachineId mismatch" is more specific and accurately describes the issue. This change is in line with the overall improvements in error handling and security measures implemented in this PR.
Line range hint
1-97: Overall excellent improvements to security and login processThis PR introduces significant enhancements to the login process and security measures:
- Improved error handling with specific error codes and messages.
- Implementation of secure password hashing using BCrypt.
- Proper password verification during login.
- Enhanced logging for potential security issues (MachineId mismatch).
These changes align well with the PR objectives and significantly improve the overall security of the system. The code is well-structured and follows good practices. The minor suggestions provided in the review comments can further refine the implementation.
Great work on enhancing the security of the login process!
Maple2.Server.Login/PacketHandlers/LoginHandler.cs (1)
6-6: [Approved]The addition of the
using Maple2.Server.Core.Helpers;directive is appropriate if helper methods from this namespace are utilized within the code.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
Maple2.Server.World/Service/GlobalService.cs (3)
31-35: Good addition of password validationThe new check for empty or whitespace-only passwords is a valuable security enhancement. It aligns well with the PR's objective of implementing basic password validation.
Consider using a constant for the error message to maintain consistency and ease future updates:
+ private const string INVALID_PASSWORD_MESSAGE = "Invalid Password."; if (string.IsNullOrWhiteSpace(request.Password)) { return Task.FromResult(new LoginResponse { Code = LoginResponse.Types.Code.ErrorPassword, - Message = "Invalid Password." + Message = INVALID_PASSWORD_MESSAGE }); }
48-52: Good implementation of password hashingThe use of BCrypt for password hashing is an excellent security practice. The work factor of 13 provides a good balance between security and performance.
Consider extracting the work factor to a constant for easier future adjustments:
+ private const int BCRYPT_WORK_FACTOR = 13; - string hashedPassword = BCrypt.Net.BCrypt.HashPassword(password, 13); + string hashedPassword = BCrypt.Net.BCrypt.HashPassword(password, BCRYPT_WORK_FACTOR);This change makes it easier to update the work factor in the future if needed.
72-78: Secure password verification implementedThe use of BCrypt for password verification is correct and secure. This implementation aligns well with the PR's objective of implementing basic password validation.
For consistency with the earlier suggestion, consider using a constant for the error message:
+ private const string INCORRECT_PASSWORD_MESSAGE = "Incorrect Password."; if (!isPasswordValid) { return Task.FromResult(new LoginResponse { Code = LoginResponse.Types.Code.ErrorPassword, - Message = "Incorrect Password." + Message = INCORRECT_PASSWORD_MESSAGE }); }This change maintains consistency in error message handling throughout the class.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (3)
Maple2.Server.World/Migrations/20241002170426_AddAccountPassword.Designer.csis excluded by!Maple2.Server.World/Migrations/*Maple2.Server.World/Migrations/20241002170426_AddAccountPassword.csis excluded by!Maple2.Server.World/Migrations/*Maple2.Server.World/Migrations/Ms2ContextModelSnapshot.csis excluded by!Maple2.Server.World/Migrations/*
📒 Files selected for processing (4)
- Maple2.Database/Model/Account.cs (4 hunks)
- Maple2.Model/Game/User/Account.cs (1 hunks)
- Maple2.Server.Login/PacketHandlers/LoginHandler.cs (2 hunks)
- Maple2.Server.World/Service/GlobalService.cs (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- Maple2.Database/Model/Account.cs
- Maple2.Model/Game/User/Account.cs
- Maple2.Server.Login/PacketHandlers/LoginHandler.cs
🔇 Additional comments (5)
Maple2.Server.World/Service/GlobalService.cs (5)
25-29: Improved error handling for invalid usernamesThe change from throwing an
RpcExceptionto returning a structuredLoginResponsefor invalid usernames is a good improvement. It provides better error handling and aligns with the PR's objective of enhancing the login process.
85-85: Improved error message for MachineId mismatchThe updated error message for MachineId mismatch is more concise and clear. This minor change improves the overall clarity of error messages in the login process.
Line range hint
1-96: Appropriate use of structured loggingThe current implementation correctly uses structured logging with Serilog, which is a good practice. There are no instances of string interpolation in logging statements, addressing the concern raised in a past review comment. This approach to logging is maintainable and provides better performance and flexibility for log analysis.
Line range hint
1-96: Summary of Login Implementation ChangesThe changes in this PR successfully implement login with encrypted passwords, meeting the stated objectives. Key improvements include:
- Enhanced security through proper password hashing and verification using BCrypt.
- Implementation of basic password validation, preventing empty passwords.
- Improved error handling with clear, structured responses for various scenarios.
- Maintenance of existing MachineId checks with clearer error messages.
The changes align well with the PR objectives and significantly improve the security of the login process. Minor suggestions have been made for code consistency and maintainability.
Overall, this is a solid implementation that enhances the security and functionality of the login process.
41-41:⚠️ Potential issueAvoid modifying passwords before hashing
Trimming the password could potentially alter the user's intended input and might lead to security issues if users rely on leading or trailing spaces in their passwords. It's generally recommended to preserve the exact password as entered by the user.
Apply this diff to fix the issue:
- string password = request.Password.Trim(); + string password = request.Password;This change ensures that the password is preserved exactly as the user entered it, maintaining its full complexity and avoiding potential security risks.
Maple2.Model/Game/User/Account.cs
Outdated
There was a problem hiding this comment.
Do we need Password on this Account? I would figure it's only used on DB Model. Once a user has logged in, it's no longer necessary.
There was a problem hiding this comment.
its in use inside GlobalService
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
Maple2.Database/Maple2.Database.csproj (2)
15-15: Approved: Addition of BCrypt.Net-Next packageThe inclusion of the BCrypt.Net-Next package (version 4.0.3) aligns well with the PR objectives of implementing password encryption. BCrypt is a robust and secure hashing algorithm suitable for password storage.
Consider adding a comment above the package reference to explain its purpose, e.g.:
<!-- For secure password hashing --> <PackageReference Include="BCrypt.Net-Next" Version="4.0.3" />This can help other developers understand the reason for including this package at a glance.
Line range hint
1-30: Project configuration looks good, minor suggestion for consistencyThe overall project configuration is well-structured and up-to-date:
- Targeting .NET 8.0 (latest version)
- Nullable reference types enabled
- Using C# 12
- Appropriate package references for the project's needs
For consistency, consider updating the Entity Framework Core packages to version 8.0 to match the target framework:
<PackageReference Include="Microsoft.EntityFrameworkCore" Version="8.0.0" /> <PackageReference Include="Microsoft.EntityFrameworkCore.Relational" Version="8.0.0" />This ensures you're using the latest features and improvements in EF Core that are compatible with .NET 8.0.
Maple2.Database/Storage/Game/GameStorage.User.cs (1)
39-47: Consider handling the case when the account is not found.The method currently returns
falseif the account is not found. However, it might be beneficial to differentiate between the cases when the account doesn't exist versus when the update fails for other reasons.Consider throwing a specific exception or returning a more descriptive result to indicate when the account is not found. For example:
public bool UpdateMachineId(long accountId, Guid machineId) { Model.Account? account = Context.Account.Find(accountId); if (account == null) { - return false; + throw new AccountNotFoundException($"Account not found for ID: {accountId}"); } account.MachineId = machineId; Context.Account.Update(account); return Context.TrySaveChanges(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
- Maple2.Database/Maple2.Database.csproj (1 hunks)
- Maple2.Database/Model/Account.cs (2 hunks)
- Maple2.Database/Storage/Game/GameStorage.User.cs (2 hunks)
- Maple2.Model/Game/User/Account.cs (1 hunks)
- Maple2.Server.Login/PacketHandlers/LoginHandler.cs (1 hunks)
- Maple2.Server.World/Service/GlobalService.cs (4 hunks)
✅ Files skipped from review due to trivial changes (1)
- Maple2.Model/Game/User/Account.cs
🚧 Files skipped from review as they are similar to previous changes (3)
- Maple2.Database/Model/Account.cs
- Maple2.Server.Login/PacketHandlers/LoginHandler.cs
- Maple2.Server.World/Service/GlobalService.cs
🔇 Additional comments (3)
Maple2.Database/Maple2.Database.csproj (1)
Line range hint
1-30: Verify implementation of login and password validation featuresThe addition of the BCrypt.Net-Next package is a good start for implementing password encryption. However, to fully meet the PR objectives, we need to ensure that the actual implementation of the login functionality and password validation exists in other files.
Please run the following script to check for the implementation of these features:
This will help us confirm that the login functionality and password validation have been implemented as intended.
✅ Verification successful
Login and Password Validation Implemented Successfully
The BCrypt.Net-Next package has been effectively integrated for password encryption, and both login functionality and password validation are implemented across the codebase.
- Password Validation: Utilizes BCrypt for hashing and verifying passwords (
Maple2.Database/Storage/Game/GameStorage.User.cs).- Login Functionality: Comprehensive login methods and handlers are present (
Maple2.Server.World/Service/GlobalService.cs,Maple2.Server.Login/LoginServer.cs, etc.).🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for the implementation of login and password validation features # Search for login-related code echo "Searching for login implementation:" rg --type csharp -i "login|authenticate|signin" -g "!*.csproj" # Search for password validation code echo "Searching for password validation implementation:" rg --type csharp -i "password.*(validation|verify|check)" -g "!*.csproj" # Search for BCrypt usage echo "Searching for BCrypt usage:" rg --type csharp "BCrypt" -g "!*.csproj"Length of output: 21827
Maple2.Database/Storage/Game/GameStorage.User.cs (2)
34-37: Ensure the password verification logic is secure.The password verification logic looks correct. It retrieves the account by ID, checks if the account exists, and then uses BCrypt to verify the provided password against the stored hash. This is a secure approach for password verification.
400-403: Verify the password hashing configuration is secure.The password hashing using BCrypt with a work factor of 13 is a secure approach. However, it's important to ensure that this work factor provides an acceptable balance between security and performance for your specific use case.
Consider running the following script to benchmark the password hashing performance and adjust the work factor if needed:
Ensure that the chosen work factor provides sufficient security while keeping the hashing time within acceptable limits for your application's performance requirements.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
Maple2.Database/Storage/Game/GameStorage.User.cs (1)
44-52: Consider handling the case when the account is not found.The
UpdateMachineIdmethod retrieves the account based on the providedaccountId. If the account is not found, it returnsfalse. However, it might be beneficial to differentiate between the cases when the account is not found and when the update operation fails.Consider throwing a specific exception or returning a more descriptive result to indicate that the account was not found. This can help the calling code handle the scenario appropriately.
public bool UpdateMachineId(long accountId, Guid machineId) { Model.Account? account = Context.Account.Find(accountId); if (account == null) { - return false; + throw new AccountNotFoundException($"Account not found for ID: {accountId}"); } account.MachineId = machineId; Context.Account.Update(account); return Context.TrySaveChanges(); }
Zintixx
left a comment
Zintixx
left a comment
There was a problem hiding this comment.
just some tiny formatting things
There was a problem hiding this comment.
add the trailing comma back pls. this is the formatting settings we're going with
3 participants
Summary by CodeRabbit
New Features
Passwordproperty for account management.Bug Fixes
Documentation