Enables CORS and updates package versions#75
Conversation
Improves API security by adding the `Unauthorized` status code to endpoint definitions and enables CORS to allow requests from the frontend application running on `http://localhost:4200`. Updates package versions to their latest stable releases, including `Google.Apis.Gmail.v1`, `Microsoft.VisualStudio.Azure.Containers.Tools.Targets`, `Scalar.AspNetCore`, `SonarAnalyzer.CSharp`, and `MSTest` packages. The `CreateUser` endpoint is now accessible without authentication. Adds functionality to update basic user information. Adds functionality to change user password.
jldsilva
added
.NET
WalkthroughAtualiza dependências, adiciona respostas 401 a múltiplos endpoints, expande JWT com claims Name e TelegramChatId, adiciona endpoints e métodos para atualizar info básica do usuário e trocar senha, implementa persistência correspondente e configura política CORS. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant API as API Endpoint<br/>(UserPasswordEndpoints)
participant AppService as UserPasswordAppService
participant Repo as UserPasswordRepository
participant DB as PostgreSQL
Client->>API: PATCH /users/password/change<br/>(UserPasswordViewModel)
API->>AppService: ChangePasswordAsync(viewModel)
AppService->>AppService: Validate input
AppService->>AppService: Hash password (PBKDF2)
AppService->>Repo: ChangePasswordAsync(userPassword)
Repo->>DB: UPDATE invoice_reminder.user_password<br/>SET password_hash, password_salt, updated_at WHERE user_id
DB-->>Repo: rows affected
Repo-->>AppService: boolean result
AppService-->>API: Result<bool>
API-->>Client: 200 OK / 400 Bad Request / 500 Error
sequenceDiagram
actor Client
participant API as API Endpoint<br/>(UserEndpoints)
participant AppService as UserAppService
participant Repo as UserRepository
participant DB as PostgreSQL
Client->>API: PATCH /users/basic-info<br/>(UserViewModel)
API->>AppService: UpdateBasicUserInfoAsync(viewModel)
AppService->>AppService: Validate input
AppService->>Repo: UpdateBasicUserInfoAsync(user)
Repo->>DB: UPDATE invoice_reminder.user<br/>SET name, email, telegram_chat_id, updated_at WHERE id
DB-->>Repo: rows affected
Repo-->>AppService: boolean result
AppService-->>API: Result<bool>
API-->>Client: 200 OK / 400 Bad Request / 500 Error
Estimated code review effort🎯 4 (Complexo) | ⏱️ ~75 minutos Possibly related PRs
Poema
🚥 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
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 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: 5
🤖 Fix all issues with AI agents
In `@InvoiceReminder.API/Program.cs`:
- Around line 14-21: The CORS origin is hardcoded in the AddCors setup
(builder.Services.AddCors / policy "CorsPolicy" using
WithOrigins("http://localhost:4200")); change it to read allowed origins from
configuration (e.g., IConfiguration) so environments can override via
appsettings.json or env vars: add a "Cors:AllowedOrigins" array to
configuration, load that value in Program.cs, and call
builder.WithOrigins(allowedOrigins) (or
builder.WithOrigins(allowedOriginsArray)) when defining the "CorsPolicy" instead
of the literal string; keep AllowAnyMethod/AllowCredentials/WithHeaders as-is.
In `@InvoiceReminder.Application/AppServices/UserAppService.cs`:
- Around line 88-90: The current app service returns
Result<bool>.Success(result) even when result is false from
UpdateBasicUserInfoAsync, causing a 200 OK with false which is ambiguous; change
the method in UserAppService (the method that calls
_repository.UpdateBasicUserInfoAsync) to check the boolean result and return
Result<bool>.Failure(...) when result is false (include an appropriate error
code/message) and return Result<bool>.Success(true) only when result is true,
preserving the existing cancellationToken usage and exception paths.
In `@InvoiceReminder.Application/AppServices/UserPasswordAppService.cs`:
- Around line 83-111: ChangePasswordAsync currently calls
_repository.ChangePasswordAsync which uses Dapper and commits immediately,
breaking atomicity with _unitOfWork.SaveChangesAsync and also lacks verification
of the existing password; update the flow so the password change happens inside
the same EF/Core unit-of-work transaction and validate the old password before
changing: either (A) modify _repository.ChangePasswordAsync to use the EF
DbContext (load the user entity, verify viewModel.OldPassword against stored
PasswordHash/PasswordSalt, set new hash/salt and return without committing) so
SaveChangesAsync performs the atomic commit, or (B) if you must keep Dapper,
have ChangePasswordAsync obtain the current IDbTransaction/DbConnection from the
unit of work and pass that transaction into the repository Dapper call so the
UPDATE participates in the same transaction, and add a check to verify the old
password prior to executing the update (use the same hash/salt logic used in
other paths).
In `@InvoiceReminder.Authentication/Jwt/JwtObject.cs`:
- Around line 5-10: O DTO JwtObject está expondo dados sensíveis (UserId, Name,
Email, TelegramChatId) no corpo da resposta; remove TelegramChatId da classe
JwtObject (ou torná-lo interno/privado) e, se o frontend precisar do chat id,
mova-o para as claims do JWT quando o token é criado (emitido pela rotina que
preenche AuthenticationToken) ou crie um Response DTO separado sem
TelegramChatId para as respostas HTTP; atualize os pontos que instanciam
JwtObject (procurar usos de JwtObject, AuthenticationToken e Authenticated) para
usar o novo DTO ou para armazenar TelegramChatId apenas nas claims do token.
In `@InvoiceReminder.Data/Repository/UserRepository.cs`:
- Around line 149-194: O UpdateBasicUserInfoAsync captura todas as exceções em
um DataLayerException, ocultando conflitos de unicidade no índice
idx_user_email; antes do catch(Exception) adicione um catch específico para a
exceção do driver (por exemplo Npgsql.PostgresException) e verifique SqlState ==
"23505" ou ConstraintName/Message contendo "idx_user_email"; nesse caso registre
a ocorrência e lance uma exceção específica de conflito (por exemplo
EmailAlreadyExistsException ou um ConflictException do domínio) para que o
caller possa retornar 409, mantendo o log/contexto similar ao existente;
preserve o catch(OperationCanceledException) e deixe o catch(Exception) genérico
como fallback.
🧹 Nitpick comments (6)
InvoiceReminder.API/Program.cs (1)
14-21: Sombreamento do parâmetrobuilder.Na linha 15, o parâmetro do lambda também se chama
builder, sombreando a variávelWebApplicationBuilderdo escopo externo (linha 7). Isso pode causar confusão e dificultar a manutenção.♻️ Renomeie o parâmetro do lambda
builder.Services.AddCors(opt => - opt.AddPolicy("CorsPolicy", builder => - builder.WithOrigins("http://localhost:4200") + opt.AddPolicy("CorsPolicy", policy => + policy.WithOrigins("http://localhost:4200") .AllowAnyMethod() .AllowCredentials() .WithHeaders("Content-Type", "Authorization")InvoiceReminder.Authentication/Abstractions/UserClaims.cs (1)
8-8: Inconsistência:TelegramChatIdusasetenquanto todas as outras propriedades usaminit.As propriedades
Id,Nameeinit, garantindo imutabilidade após a construção do record.TelegramChatIdcomsetquebra essa consistência e permite mutação inesperada.♻️ Correção sugerida
- public required long TelegramChatId { get; set; } + public required long TelegramChatId { get; init; }InvoiceReminder.API/Endpoints/UserEndpoints.cs (1)
139-156: Risco de over-posting:UserViewModelcompleto no endpoint de atualização parcial.
MapUpdateBasicUserInfoaceitaUserViewModelque incluiPassword,UserPassword,Invoices,JobSchedulese outras propriedades que não deveriam ser modificáveis por este endpoint. Embora o repositório só atualizename,telegram_chat_id, um DTO dedicado (ex:UpdateBasicUserInfoViewModel) deixaria o contrato da API mais explícito e evitaria confusão para consumidores da API.InvoiceReminder.UnitTests.Infrastructure/Authentication/JwtProviderTests.cs (1)
66-90: Cobertura de teste não verifica os novos campos doJwtObject.O teste valida que o token é gerado com sucesso, mas não verifica se os novos campos (
UserId,Name,TelegramChatId) estão corretamente preenchidos noJwtObjectretornado. Considere adicionar asserções para esses campos.♻️ Sugestão de asserções adicionais
result.ShouldSatisfyAllConditions(() => { _ = result.ShouldNotBeNull(); result.Authenticated.ShouldBe(true); _ = result.AuthenticationToken.ShouldNotBeNull(); result.Expiration.ShouldNotBe(default); + result.UserId.ShouldBe(user.Id); + result.Name.ShouldBe(user.Name); + result.Email.ShouldBe(user.Email); + result.TelegramChatId.ShouldBe(user.TelegramChatId); });InvoiceReminder.Authentication/Jwt/JwtProvider.cs (1)
57-66:Namenão é validado, diferente deId.O método valida
user.Id(linha 27) euser.Email(linha 32), mas não validauser.Name, mesmo que seja uma propriedaderequiredemUserClaims. Para consistência, considere adicionar validação paraName.♻️ Sugestão de validação
if (string.IsNullOrWhiteSpace(user.Email)) { throw new ArgumentException("User Email cannot be null or empty.", nameof(user)); } + + if (string.IsNullOrWhiteSpace(user.Name)) + { + throw new ArgumentException("User Name cannot be null or empty.", nameof(user)); + }InvoiceReminder.Application/AppServices/UserPasswordAppService.cs (1)
83-95: Considere verificar a senha atual antes de permitir a alteração.O método
ChangePasswordAsyncnão valida a senha atual do usuário antes de aplicar a nova senha. Embora o endpoint possa estar protegido porRequireAuthorization, uma verificação da senha atual é uma prática de segurança padrão para fluxos de alteração de senha, prevenindo uso indevido de sessões ativas comprometidas.
Configures CORS to allow specific origins from the configuration, enabling integration with the frontend application. This change ensures that the API can securely handle requests from the specified frontend origin. Also, refactors the `UserAppService` and `UserPasswordAppService` to handle update failures more gracefully, returning a failure result with a descriptive message. Finally, simplifies JwtObject and includes telegramChatId in jwt claims.
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 (1)
InvoiceReminder.API/Program.cs (1)
50-56:⚠️ Potential issue | 🟡 MinorReordenar middleware:
UseHttpsRedirectiondeve vir antes deUseCors.A documentação oficial da Microsoft recomenda que
UseHttpsRedirectionseja executado no início do pipeline, antes do middleware de roteamento e segurança. Atualmente,UseCors(linha 50) está posicionado antes deUseHttpsRedirection(linha 54), o que significa que requisições HTTP não-seguras serão processadas pelo middleware CORS antes de serem redirecionadas para HTTPS, expondo headers CORS em respostas não criptografadas.A ordem recomendada é:
UseHttpsRedirection→UseCors→UseAuthentication→UseAuthorization.🔧 Correção sugerida
+app.UseHttpsRedirection(); app.UseCors("CorsPolicy"); app.RegisterEndpoints(); app.UseExceptionHandler(); app.UseStatusCodePages(); -app.UseHttpsRedirection(); app.UseAuthentication(); app.UseAuthorization();
🤖 Fix all issues with AI agents
In `@InvoiceReminder.Application/AppServices/UserPasswordAppService.cs`:
- Around line 103-105: The ternary return is inverted: when the repository call
result (variable result) is true you should return success, otherwise failure;
update the return in UserPasswordAppService (the method that currently returns
Result<bool>) to return Result<bool>.Success(result) or
Result<bool>.Success(true) when result is true and Result<bool>.Failure("Failed
to change the password.") when result is false so the success/failure mapping of
the boolean result is correct.
In `@InvoiceReminder.Data/Repository/UserRepository.cs`:
- Around line 180-191: Replace the dead catch for DbUpdateException in the
UpdateBasicUserInfoAsync path with a provider-agnostic
System.Data.Common.DbException so Dapper/ADO.NET DB errors are caught; update
the catch clause from DbUpdateException to DbException, add a using
System.Data.Common at the top, and keep the existing contextualInfo/logging and
rethrow as DataLayerException (preserving ex as inner) so database-specific
errors are handled correctly in UserRepository.UpdateBasicUserInfoAsync.
🧹 Nitpick comments (1)
InvoiceReminder.API/Program.cs (1)
15-27: A leitura da configuração dentro do lambda doAddPolicyfunciona, mas é preferível movê-la para fora.Ler
builder.Configurationdentro do delegate doAddPolicyfunciona neste caso porque obuilderainda está em escopo. No entanto, o padrão mais idiomático é ler a configuração fora do lambda e capturá-la na closure, o que melhora a clareza e facilita testes.Além disso, se
allowedOriginsresultar em array vazio (fallback[]), nenhuma origem será permitida e as requisições CORS falharão silenciosamente. Considere adicionar um log de aviso ou uma validação.♻️ Sugestão de refatoração
+var allowedOrigins = builder.Configuration.GetSection("Cors:AllowedOrigins").Get<string[]>() ?? []; + builder.Services.AddCors(opt => opt.AddPolicy("CorsPolicy", policy => - { - var allowedOrigins = builder.Configuration.GetSection("Cors:AllowedOrigins").Get<string[]>() ?? []; - - _ = policy - .AllowAnyMethod() - .AllowCredentials() - .WithOrigins(allowedOrigins) - .WithHeaders("Content-Type", "Authorization"); - } + policy + .AllowAnyMethod() + .AllowCredentials() + .WithOrigins(allowedOrigins) + .WithHeaders("Content-Type", "Authorization") ) );
1 participant
Improves API security by adding the
Unauthorizedstatus code to endpoint definitions and enables CORS to allow requests from the frontend application running onhttp://localhost:4200.Updates package versions to their latest stable releases, including
Google.Apis.Gmail.v1,Microsoft.VisualStudio.Azure.Containers.Tools.Targets,Scalar.AspNetCore,SonarAnalyzer.CSharp, andMSTestpackages.The
CreateUserendpoint is now accessible without authentication.Adds functionality to update basic user information.
Adds functionality to change user password.
Summary by CodeRabbit
Notas de Lançamento
New Features
Chores