Enhances security and updates dependencies#72
Merged
Conversation
Improves user authentication with password hashing using Argon2id. Updates essential .NET packages and other dependencies to the latest versions. Refactors endpoint parameter order for consistency and readability.
jldsilva
added
.NET
WalkthroughAdiciona entidade e persistência UserPassword com hashing Argon2, migrações e mapeamento EF; introduz endpoints e serviço para CRUD de senhas; adapta fluxo de login para ValidateUserPasswordAsync e UserClaims; amplia testes e atualiza versões de pacotes/SDK. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant LoginEndpoint as "Login Endpoint"
participant UserAppService as "User App Service"
participant UserRepository as "User Repository"
participant PasswordExt as "Password Extension (Argon2)"
participant JwtProvider as "JWT Provider"
Client->>LoginEndpoint: POST /login (email, password)
LoginEndpoint->>UserAppService: ValidateUserPasswordAsync(email, password)
UserAppService->>UserRepository: GetByEmailAsync(email)
UserRepository-->>UserAppService: User + UserPassword
UserAppService->>PasswordExt: VerifyPassword(password, hash, salt)
PasswordExt-->>UserAppService: isValid
alt senha válida
UserAppService-->>LoginEndpoint: Result.Success(UserViewModel)
LoginEndpoint->>JwtProvider: Generate(UserClaims)
JwtProvider-->>LoginEndpoint: JwtObject
LoginEndpoint-->>Client: 200 OK + JWT
else senha inválida
UserAppService-->>LoginEndpoint: Result.Failure
LoginEndpoint-->>Client: 400 Bad Request
end
sequenceDiagram
participant Client
participant UserPasswordEndpoint as "UserPassword Endpoint"
participant AuthService as "Authorization Service"
participant UserPasswordAppService as "UserPassword App Service"
participant PasswordExt as "Password Extension (Argon2)"
participant Repository as "UserPassword Repository"
participant UnitOfWork as "Unit of Work"
Client->>UserPasswordEndpoint: POST /api/user_password (payload)
UserPasswordEndpoint->>AuthService: CheckAuthorization()
alt autorizado
UserPasswordEndpoint->>UserPasswordAppService: AddAsync(viewModel)
UserPasswordAppService->>PasswordExt: HashPassword(plaintext)
PasswordExt-->>UserPasswordAppService: (hash, salt)
UserPasswordAppService->>Repository: AddAsync(entity)
UserPasswordAppService->>UnitOfWork: SaveChangesAsync()
UnitOfWork-->>UserPasswordAppService: success
UserPasswordAppService-->>UserPasswordEndpoint: Result.Success
UserPasswordEndpoint-->>Client: 201 Created
else não autorizado
UserPasswordEndpoint-->>Client: 401 Unauthorized
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutos Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
InvoiceReminder.Domain/Extensions/UserExtensions.cs (1)
14-24: Evite sobrescrever senha comnulle valide oUserId.Hoje qualquer chamada que não forneça
UserPasswordpode apagar a senha existente, e umUserPasswordcomUserIddiferente pode ser associado ao usuário errado.🐛 Sugestão de ajuste
if (!result.TryGetValue(user.Id, out var existingUser)) { - user.UserPassword = parameters.UserPassword; + if (parameters.UserPassword != null) + { + if (parameters.UserPassword.UserId != user.Id) + throw new InvalidOperationException("UserPassword.UserId não corresponde ao User.Id."); + user.UserPassword = parameters.UserPassword; + } parameters.Invoice.AddIfNotExists(user.Invoices); parameters.JobSchedule.AddIfNotExists(user.JobSchedules); parameters.EmailAuthToken.AddIfNotExists(user.EmailAuthTokens); parameters.ScanEmailDefinition.AddIfNotExists(user.ScanEmailDefinitions); @@ else { - existingUser.UserPassword = parameters.UserPassword; + if (parameters.UserPassword != null) + { + if (parameters.UserPassword.UserId != existingUser.Id) + throw new InvalidOperationException("UserPassword.UserId não corresponde ao User.Id."); + existingUser.UserPassword = parameters.UserPassword; + } parameters.Invoice.AddIfNotExists(existingUser.Invoices); parameters.JobSchedule.AddIfNotExists(existingUser.JobSchedules); parameters.EmailAuthToken.AddIfNotExists(existingUser.EmailAuthTokens); parameters.ScanEmailDefinition.AddIfNotExists(existingUser.ScanEmailDefinitions); }InvoiceReminder.Authentication/Jwt/JwtProvider.cs (1)
23-31: Adicione validações deIde
ClaimlançaArgumentNullExceptionse o valor for nulo, eGuid.Emptyresulta em umsubsemanticamente inválido. Embora haja validação upstream, é importante garantir essa invariante aqui.🐛 Sugestão de ajuste
public JwtObject Generate(UserClaims user) { ArgumentNullException.ThrowIfNull(user); + if (user.Id == Guid.Empty) + throw new ArgumentException("O Id do usuário é inválido.", nameof(user)); + if (string.IsNullOrWhiteSpace(user.Email)) + throw new ArgumentException("O email do usuário é inválido.", nameof(user)); var claims = new Claim[] { new (JwtRegisteredClaimNames.Sub, user.Id.ToString()), new (JwtRegisteredClaimNames.Email, user.Email) };
🤖 Fix all issues with AI agents
In `@Directory.Packages.props`:
- Around line 35-37: The new PackageVersion entries for itext,
itext.bouncy-castle-adapter and itext.bouncy-castle-fips-adapter
(Version="9.5.0") introduce breaking changes: update logging dependencies to
SLF4J 2.x compatible implementations (e.g., logback ≥1.3.x) and ensure your
build (nuget/msbuild) aligns; search code for usages of IConformanceLevel and
replace with the new PdfConformance / PdfAConformance / PdfUAConformance enums;
audit places that accessed itext internals as public fields and refactor to use
the new public getters/setters; run and fix compile errors in classes that
instantiate or configure PDF/A/UA conformance, bouncy-castle adapters, or any
direct itext types to confirm compatibility before merging.
In `@InvoiceReminder.API/Endpoints/UserPasswordEndpoints.cs`:
- Around line 23-36: UserPasswordViewModel.PasswordHash is JsonIgnore and
remains null so calling .HashPassword() in AddAsync, BulkInsertAsync and
UpdateAsync causes NullReferenceException; create an input DTO like
CreateUserPasswordRequest(Guid UserId, string Password) and update the POST
handler (the endpoint.MapPost delegate) to accept that DTO, compute the
PasswordHash from DTO.Password (calling HashPassword on the non-null plain
password) and then map/populate a UserPasswordViewModel instance with the
computed hash before calling appService.AddAsync; apply the same pattern for
bulk insert and update endpoints (accept request DTOs carrying raw passwords,
compute hashes server-side, and pass fully populated ViewModels to
BulkInsertAsync and UpdateAsync) while keeping PasswordHash as JsonIgnore on the
ViewModel.
In `@InvoiceReminder.Application/AppServices/UserAppService.cs`:
- Around line 31-33: Replace the predictable tempPassword creation in
UserAppService (the tempPassword variable that currently uses
DateTime.Now.ToString and the subsequent HashPassword() call) with a
cryptographically secure random password generator (use RandomNumberGenerator/
RNGCryptoServiceProvider to produce a 12–16 character alphanumeric/URL-safe
string), then pass that generated password into the existing HashPassword() call
to produce pHash and pSalt; set the user flag that enforces password change on
first login (e.g., RequirePasswordChange / MustChangePassword) and ensure the
generated temp password is delivered to the user only via a secure channel
(email/secure link).
- Around line 58-61: A validação atual assume que entity.UserPassword existe e
pode lançar NullReferenceException; update the check in UserAppService (after
retrieving entity via _repository.GetByEmailAsync) to ensure entity is not null
AND entity.UserPassword is not null before calling password.VerifyPassword —
i.e., include a guard like entity?.UserPassword != null (or check
entity.UserPassword != null) in the isValid condition so VerifyPassword only
receives non-null PasswordHash and PasswordSalt from entity.UserPassword.
In `@InvoiceReminder.Application/AppServices/UserPasswordAppService.cs`:
- Line 29: Fix typos in the error message strings returned by
Result<UserPasswordViewModel>.Failure: change "The provided obejct data was
Null." (and any other occurrences of "obejct" or incorrect capitalization like
"Null") to a correct, consistent message such as "The provided object data was
null." Update all instances in UserPasswordAppService where
Result<UserPasswordViewModel>.Failure is called (refer to the Failure(...)
invocations around the lines noted) so the spelling and casing are correct and
consistent.
- Around line 23-60: Add null/empty/whitespace validation for the password
before calling HashPassword in AddAsync, BulkInsertAsync and UpdateAsync: check
viewModel.PasswordHash (and for BulkInsertAsync each viewModel.PasswordHash)
using string.IsNullOrWhiteSpace and return Result<T>.Failure with an appropriate
message instead of calling HashPassword when invalid; do the same in UpdateAsync
(the method referenced as UpdateAsync) so HashPassword is only invoked with a
non-empty string and avoids throwing ArgumentException.
In `@InvoiceReminder.Application/ViewModels/InvoiceViewModel.cs`:
- Around line 12-29: The JSON property orders are duplicated for Bank and
Beneficiary causing non-deterministic serialization; update the
JsonPropertyOrder attributes so they are unique and sequential: keep Bank as
JsonPropertyOrder(3), change Beneficiary to JsonPropertyOrder(4), change Amount
to JsonPropertyOrder(5), change Barcode to JsonPropertyOrder(6), and change
DueDate to JsonPropertyOrder(7) by updating the attributes on the corresponding
properties (Bank, Beneficiary, Amount, Barcode, DueDate).
In `@InvoiceReminder.Authentication/Extensions/StringHashExtension.cs`:
- Around line 10-29: HashPassword and VerifyPassword create Argon2id instances
but never dispose them; wrap the Argon2id creation in a using (or a using
declaration) to ensure Dispose is called (or alternatively call Dispose in a
finally block). Locate the Argon2id instantiation in the HashPassword method and
the analogous creation in VerifyPassword and change them to use a using scope so
the Argon2id (IDisposable) is released after computing GetBytes.
In `@InvoiceReminder.Data/Persistence/EntitiesConfig/UserPasswordConfig.cs`:
- Around line 10-48: Explicitly configure the one-to-one relationship inside
UserPasswordConfig.Configure: use builder.HasOne<User>().WithOne(u =>
u.UserPassword).HasForeignKey<UserPassword>(x =>
x.UserId).OnDelete(DeleteBehavior.Cascade) (adjust the navigation lambda names
to the actual properties if they differ) so the relationship is declared rather
than inferred by convention.
In `@InvoiceReminder.Domain/Entities/User.cs`:
- Line 8: Adicionar a configuração explícita de relacionamento one-to-one em
UserPasswordConfig: use builder.HasOne<User>() combinado com .WithOne(x =>
x.UserPassword), configure a FK com .HasForeignKey<UserPassword>(x => x.UserId)
e marque como obrigatório (.IsRequired()), e crie um índice único em UserId via
.HasIndex(...).IsUnique(); por fim, verifique que a entidade User contém a
propriedade de navegação UserPassword (public virtual UserPassword UserPassword
{ get; set; }).
In `@InvoiceReminder.UnitTests.API/Endpoints/LoginEndpointTests.cs`:
- Around line 205-206: The test currently asserts that GetByEmailAsync and JWT
Generate were not called but misses asserting that password validation was
skipped; add a DidNotReceive assertion for the mocked method
ValidateUserPasswordAsync (the mock that exposes ValidateUserPasswordAsync) to
ensure it is not invoked when email/password are empty—use the same Arg.Any<>
matchers for its parameters (e.g. Arg.Any<string>(), Arg.Any<string>(),
Arg.Any<CancellationToken>()) so the test ensures ValidateUserPasswordAsync is
not called.
In
`@InvoiceReminder.UnitTests.Application/AppServices/UserPasswordAppServiceTests.cs`:
- Around line 157-217: The current assertion in
BulkInsertAsync_Should_Hash_All_Passwords_Before_Inserting (in
UserPasswordAppServiceTests) uses viewModels.All(v =>
plainPasswords.Contains(v.PasswordHash)) which only fails if every password
stayed the same; change it to assert that none of the resulting PasswordHash
values match any original plain password by replacing that check with either
viewModels.Any(v => plainPasswords.Contains(v.PasswordHash)).ShouldBeFalse() or
viewModels.All(v => !plainPasswords.Contains(v.PasswordHash)).ShouldBeTrue(), so
the test guarantees all passwords were hashed before calling
UserPasswordAppService.BulkInsertAsync.
- Around line 68-80: The mock for the repository AddAsync currently returns the
adapted viewModel with an unhashed PasswordHash; change the mock for
_repository.AddAsync(Arg.Any<UserPassword>(), Arg.Any<CancellationToken>()) to
return the actual UserPassword argument passed into the method (use the
call-info return overload such as Returns(callInfo =>
callInfo.Arg<UserPassword>())) so the test receives the entity whose password
was hashed by UserPasswordAppService before AddAsync is invoked; keep the
existing _unitOfWork.SaveChangesAsync mock as-is.
In
`@InvoiceReminder.UnitTests.Infrastructure/Authentication/StringHashExtensionTests.cs`:
- Around line 227-242: The test VerifyPassword_ModifiedHash_ShouldReturnFalse
can be flaky because the naive modification (hash[..^2] + "AB") may produce the
same string; update the test to deterministically create a different hash string
before calling VerifyPassword by altering a character at a known position that
is guaranteed to change (for example flip a non-padding Base64 char or iterate
until a different char is produced), referencing the existing HashPassword()
call to obtain (hash, salt) and using VerifyPassword(modifiedHash, salt) to
assert false.
- Around line 244-259: The test
VerifyPassword_ModifiedSaltWithValidBase64_ShouldReturnFalse is flaky because it
assumes replacing the last two characters with "AB" will change the salt; if the
salt already ends with "AB" the test doesn't modify it. Update the test to
ensure the modifiedSalt is always different: after calling
password.HashPassword(), mutate salt in a deterministic way (e.g., change the
last character to a different valid Base64 character or loop to pick a
replacement until modifiedSalt != salt) and then call
password.VerifyPassword(hash, modifiedSalt) expecting false; reference the
HashPassword and VerifyPassword helpers to locate the code to change.
♻️ Duplicate comments (1)
InvoiceReminder.Data/Repository/UserRepository.cs (1)
103-119: Mesmo cuidado com o multi-mapping aqui.Esta chamada repete o padrão do método anterior; aplique a mesma estabilização do
select/splitOnsugerida lá.
🧹 Nitpick comments (9)
InvoiceReminder.API/Endpoints/ScanEmailDefinitionEndpoints.cs (1)
79-96: Considere usar uma rota mais explícita para evitar ambiguidade.A rota
/{email}/{id}pode ser confundida com a rota/{id}do endpointMapGetScanEmailDefinitiondependendo do valor do parâmetro. Embora o ASP.NET Core consiga distinguir pelos tipos (Guid vs string), uma rota mais explícita como/getby-sender/{email}/{id}seria mais clara e consistente com o padrão usado emgetby-userid.💡 Sugestão de refatoração
- _ = endpoint.MapGet("/{email}/{id}", + _ = endpoint.MapGet("/getby-sender/{email}/{id}",InvoiceReminder.UnitTests.API/Endpoints/UserPasswordEndpointsTests.cs (1)
22-37: Descarte oCustomWebApplicationFactorye oHttpClientao final do teste.Hoje eles ficam vivos até o GC, o que pode acumular sockets/handlers em execuções longas.
♻️ Sugestão de ajuste
-public sealed class UserPasswordEndpointsTests +public sealed class UserPasswordEndpointsTests : IDisposable { private readonly HttpClient _client; + private readonly CustomWebApplicationFactory<Program> _factory; private readonly IAuthorizationService _authorizationService; private readonly IUserPasswordAppService _userPasswordAppService; private readonly Faker<UserPasswordViewModel> _userPasswordViewModelFaker; private const string basepath = "/api/user_password"; @@ - public UserPasswordEndpointsTests() + public UserPasswordEndpointsTests() { - var factory = new CustomWebApplicationFactory<Program>(); - var serviceProvider = factory.Services; + _factory = new CustomWebApplicationFactory<Program>(); + var serviceProvider = _factory.Services; - _client = factory.CreateClient(); + _client = _factory.CreateClient(); _authorizationService = serviceProvider.GetRequiredService<IAuthorizationService>(); _userPasswordAppService = serviceProvider.GetRequiredService<IUserPasswordAppService>(); @@ .RuleFor(u => u.CreatedAt, faker => faker.Date.Past().ToUniversalTime()) .RuleFor(u => u.UpdatedAt, faker => faker.Date.Recent().ToUniversalTime()); } + + public void Dispose() + { + _client.Dispose(); + _factory.Dispose(); + }InvoiceReminder.Data/Repository/UserRepository.cs (2)
30-31: Evite carregar senha em consultas genéricas sem necessidade.Ao incluir
user_passwordno_querybase, qualquer chamada deGetByEmailAsync/GetByIdAsyncpassa a trazerPasswordHash/PasswordSalt. Garanta que isso nunca é serializado/logado nas camadas superiores; se o hash só é necessário no login, considere uma consulta dedicada ou projeção sem senha.
48-64: Multi-mapping comselect *ficou mais frágil com o novo join.Com mais uma tabela, o
splitOn: "id"+select *fica sensível à ordem de colunas (principalmente seuser_passwordnão começar porid). Sugiro listar colunas explicitamente e usar aliases nosplitOn(ex.:InvoiceId, JobScheduleId, UserPasswordId, EmailAuthTokenId, ScanEmailDefinitionId) para estabilizar o mapeamento.InvoiceReminder.Authentication/Abstractions/UserClaims.cs (1)
3-6: Torne os campos obrigatórios para evitar claims inválidas.Hoje
Idpodem ficar no valor padrão se o chamador esquecer de preencher; isso pode quebrar a geração do JWT. Sugiro usarrequired(ou construtor com validação) para forçar inicialização.💡 Sugestão de ajuste
public record UserClaims { - public Guid Id { get; init; } - public string Email { get; init; } + public required Guid Id { get; init; } + public required string Email { get; init; } }InvoiceReminder.Domain/Extensions/UserParameters.cs (1)
9-9: Declare a nulabilidade deUserPasswordse ele for opcional.Se muitas operações não alteram senha, tornar o campo nullable deixa a intenção explícita e evita sobrescritas acidentais.
💡 Sugestão de ajuste
- public UserPassword UserPassword { get; set; } + public UserPassword? UserPassword { get; set; }InvoiceReminder.UnitTests.Infrastructure/Authentication/JwtProviderTests.cs (1)
90-91: Nome do método de teste inconsistente.O nome do método ainda referencia
UserViewModel, mas agora o parâmetro é do tipoUserClaims. Considere renomear para manter consistência.♻️ Sugestão de correção
[TestMethod] -public void JwtProvider_Generate_ShouldThrowArgumentNullExceptionWhenUserViewModelIsNull() +public void JwtProvider_Generate_ShouldThrowArgumentNullExceptionWhenUserClaimsIsNull()InvoiceReminder.API/AuthenticationSetup/LoginRequest.cs (1)
3-7: Boa prática de imutabilidade aplicada.A mudança para
inittorna o record imutável após a criação, o que é uma boa prática para objetos de requisição. Considere adicionar atributos de validação para garantir a integridade dos dados de entrada.♻️ Sugestão de validação
+using System.ComponentModel.DataAnnotations; + namespace InvoiceReminder.API.AuthenticationSetup; public record LoginRequest { + [Required] + [EmailAddress] public string Email { get; init; } + + [Required] public string Password { get; init; } }InvoiceReminder.Data/Persistence/EntitiesConfig/UserPasswordConfig.cs (1)
6-6: Considerar mover InternalsVisibleTo para arquivo centralizado.O atributo
InternalsVisibleToestá sendo adicionado individualmente em cada arquivo de configuração. Considere movê-lo para um arquivo centralizado comoAssemblyInfo.csou um arquivo de propriedades do projeto para evitar duplicação.
Improves user authentication by implementing password hashing and validation. Adds email validation to the login request and enhances the JWT token generation process. Also refactors endpoint definitions.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
InvoiceReminder.Data/Migrations/20260109013522_Initial_Create.cs (1)
82-104: Remova seeds de credenciais da migração ou proteja com verificação de ambiente.A inserção de hashes/salts fixos será aplicada em qualquer ambiente (dev, staging, produção). Se essa migração rodar em produção, contas com credenciais públicas e conhecidas serão criadas, comprometendo a segurança do sistema. Isso contradiz a recomendação no README de nunca compartilhar credenciais em repositórios públicos.
Opções:
- Extrair seeds para migração separada/condicional apenas para desenvolvimento
- Adicionar verificação
if (environment == Development)na migração- Remover seeds da migração e documentar setup manual de credenciais para dev
InvoiceReminder.UnitTests.API/Endpoints/ScanEmailDefinitionEndpointsTests.cs (1)
428-444: Corrigir chamada de método ao gerar email no teste.Sem os parênteses, ocorre erro de compilação (method group).
🛠️ Sugestão de ajuste
- var email = _faker.Internet.Email; + var email = _faker.Internet.Email();
🤖 Fix all issues with AI agents
In `@InvoiceReminder.Application/AppServices/UserAppService.cs`:
- Around line 22-35: The AddAsync method can NRE/throw when
viewModel.UserPassword or its PasswordHash is null/empty before calling
HashPassword; update AddAsync in UserAppService to validate
viewModel.UserPassword and viewModel.UserPassword.PasswordHash (and optionally
PasswordSalt) and return Result<UserViewModel>.Failure with a clear message if
missing, only call HashPassword when PasswordHash is non-null/non-empty, then
assign UserId, PasswordHash and PasswordSalt on the viewModel.UserPassword after
a successful hash.
In `@InvoiceReminder.Authentication/Extensions/StringHashExtension.cs`:
- Around line 10-51: The current Argon2 settings in HashPassword and
VerifyPassword set DegreeOfParallelism = 8 which can be overly aggressive;
change DegreeOfParallelism to a safer default (e.g., 1) or make it configurable
and derived from the runtime (e.g., use Math.Max(1, Environment.ProcessorCount /
N) or a configured value) so you can tune p per-deployment, and ensure you
document/measure the resulting hash latency on target hardware before rollout;
update both HashPassword and VerifyPassword to use the same configurable
parameter and keep MemorySize and Iterations as-is until you benchmark them.
♻️ Duplicate comments (3)
InvoiceReminder.Application/AppServices/UserAppService.cs (1)
60-63: Evitar NRE quandoUserPasswordestiver ausente.
Se não houver registro emuser_password,entity.UserPasswordseránulle o acesso direto quebra.🛠️ Sugestão de ajuste
- var entity = await _repository.GetByEmailAsync(email, cancellationToken); - var isValid = entity is not null && - password.VerifyPassword(entity.UserPassword.PasswordHash, entity.UserPassword.PasswordSalt); + var entity = await _repository.GetByEmailAsync(email, cancellationToken); + if (entity?.UserPassword is null) + { + return Result<UserViewModel>.Failure("User not Found."); + } + var isValid = password.VerifyPassword( + entity.UserPassword.PasswordHash, + entity.UserPassword.PasswordSalt);InvoiceReminder.Application/AppServices/UserPasswordAppService.cs (2)
27-30: Corrigir typos e capitalização nas mensagens de erro.Há ocorrências de “obejct” e “Null”; padronize para “object” e “null” para consistência.
🛠️ Sugestão de ajuste
- return Result<UserPasswordViewModel>.Failure("The provided obejct data was Null."); + return Result<UserPasswordViewModel>.Failure("The provided object data was null."); @@ - return Result<int>.Failure("The provided object data was Null or Empty."); + return Result<int>.Failure("The provided object data was null or empty."); @@ - return Result<UserPasswordViewModel>.Failure("The provided object data was Null."); + return Result<UserPasswordViewModel>.Failure("The provided object data was null.");Also applies to: 54-56, 93-96
93-101: Valide a senha antes de chamar HashPassword no UpdateAsync.Sem esse check, uma senha nula/vazia pode gerar
ArgumentExceptione virar 500.🛠️ Sugestão de ajuste
if (viewModel is null) { return Result<UserPasswordViewModel>.Failure("The provided object data was Null."); } + + if (string.IsNullOrWhiteSpace(viewModel.PasswordHash)) + { + return Result<UserPasswordViewModel>.Failure("Password is required."); + } (var pHash, var pSalt) = viewModel.PasswordHash.HashPassword();
🧹 Nitpick comments (1)
InvoiceReminder.Application/ViewModels/UserPasswordViewModel.cs (1)
1-14: Adicionar validação explícita paraPasswordHash.
Sem validação, entradas vazias podem chegar ao hashing. Recomendo restaurar validações de obrigatoriedade/tamanho para falhar cedo no binding. Confirme também se o pipeline aplica DataAnnotations (ApiController/ModelState).♻️ Sugestão de ajuste
+using System.ComponentModel.DataAnnotations; using System.Text.Json.Serialization; @@ + [Required] + [StringLength(100, MinimumLength = 6)] [JsonIgnore(Condition = JsonIgnoreCondition.WhenWriting)] public string PasswordHash { get; set; }
Improves password hashing by introducing a configurable parallelism factor for Argon2id. This change allows adjusting the computational effort required for password hashing, improving security against brute-force attacks. The parallelism factor is read from configuration, allowing for flexible adjustment based on available resources. It also adds new dependency injection for IConfigurationService This refactor introduces a helper method, GetMaxDegreeOfParallelism, that dynamically sets the degree of parallelism based on the server's processor count and the configured parallelism factor, ensuring optimal resource usage and security.
Adds data annotations to enforce password complexity. Requires a minimum length of 6 and a maximum of 100 characters.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Improves user authentication with password hashing using Argon2id.
Updates essential .NET packages and other dependencies to the latest versions.
Refactors endpoint parameter order for consistency and readability.
Summary by CodeRabbit
Novos Recursos
Melhorias
Atualizações
Tests
✏️ Tip: You can customize this high-level summary in your review settings.