From 79e07914e7a339069479618f1c0b604c5c69aba1 Mon Sep 17 00:00:00 2001 From: MNSOFT <137189378+devmnsoft@users.noreply.github.com> Date: Fri, 25 Sep 2026 10:20:36 -0300 Subject: [PATCH] Test review authorization against PostgreSQL --- .../CLIENT_FUNCTIONAL_AUDIT_2026-09-25.md | 16 +- .../Controllers/ReviewRequestsController.cs | 19 ++- .../ReviewAuthorizationBehaviorTests.cs | 137 ++++++++++++++++++ 3 files changed, 162 insertions(+), 10 deletions(-) create mode 100644 tests/Odca.IntegrationTests/ReviewAuthorizationBehaviorTests.cs diff --git a/docs/audits/CLIENT_FUNCTIONAL_AUDIT_2026-09-25.md b/docs/audits/CLIENT_FUNCTIONAL_AUDIT_2026-09-25.md index 7ecbb95..4bcac86 100644 --- a/docs/audits/CLIENT_FUNCTIONAL_AUDIT_2026-09-25.md +++ b/docs/audits/CLIENT_FUNCTIONAL_AUDIT_2026-09-25.md @@ -32,8 +32,20 @@ de persistência. `odca.has_tenant_permission`. A função canônica definida e concedida nas migrations é `odca.tenant_actor_has_permission(actor, tenant, permission)`. O helper passou a usar essa função, sem alias de compatibilidade, bypass ou mudança de grants. +3. **Contexto RLS persistente no pool:** as consultas de lista e detalhe de revisão + configuravam `odca.tenant_id` e `odca.user_id` no escopo da sessão. Ao devolver a + conexão ao pool, o próximo consumidor poderia herdar essa identidade. As duas rotas + agora abrem transação, usam `set_config(..., true)` e executam todas as consultas na + mesma transação; commit, rollback ou descarte removem o contexto antes do reuso. -Não é necessária migration para as duas correções: ambas são divergências no código da +Foi acrescentado um teste comportamental PostgreSQL que chama os métodos reais de +responsáveis e fila com a role restrita, executa a função canônica, diferencia revisor, +usuário sem permissão, vínculo bloqueado e organização suspensa, e força um pool de uma +conexão para comprovar que tenant/ator não permanecem depois da resposta. Ele permanece +condicionado ao banco descartável protegido pelas variáveis `ODCA_TEST_*` e não foi +executado nesta máquina, que continua sem SDK e PostgreSQL. + +Não é necessária migration para essas correções: elas são divergências no código da API em relação ao schema já canônico. A busca no checkout depois da correção não encontra consumidor executável de `has_tenant_permission`; permanece apenas uma menção histórica em documentação, que não é chamada pelo runtime. @@ -57,7 +69,7 @@ em documentação, que não é chamada pelo runtime. | Edição da minuta | `/estudio/minutas/{id}` | `tenant.contract_drafts.manage` | campos, blocos, pendências, comentários, autosave | draft + row version | **Ainda incompleta nesta auditoria** | Testar reload, idempotência e concorrência. | | Versão imutável / PDF | `/estudio/versoes/{id}` | leitura da minuta/documento | visualizar, gerar/baixar PDF, revisão | generated version + hash/PDF | **Bloqueada por dependência identificada** | Renderização e download precisam dos hosts; verificar bytes/hash e autorização. | | Participantes da versão | `/estudio/versoes/{id}#participantes` | gestão da minuta | incluir, editar, ordenar, retirar, confirmar e reabrir | preparation revisions/events | **Ainda incompleta nesta auditoria** | UI e API existem; confirmar contra PDF, histórico, comparação e replay no banco. | -| Solicitações de revisão — fila | `/organizacoes/{tenantId}/solicitacoes` | `tenant.reviews.read/decide` | filtros, escopos, responsáveis | leitura paginada | **Ainda incompleta** | Helper usa função canônica; testar perfis permitido/negado, vínculo bloqueado e tenant inativo. | +| Solicitações de revisão — fila | `/organizacoes/{tenantId}/solicitacoes` | `tenant.reviews.read/decide` | filtros, escopos, responsáveis | leitura paginada | **NÃO EXECUTADO neste agente; regressão automatizada preparada** | Métodos reais cobrem função canônica, responsáveis, fila vazia, permitido/negado, vínculo bloqueado, tenant inativo e limpeza do contexto RLS; CI com PostgreSQL precisa confirmar. | | Solicitação de revisão — detalhe | `/solicitacoes/{reviewId}` | `tenant.reviews.read/decide` | comentário, reatribuição, aprovar/ajustes | comments/steps/events/notifications | **Ainda incompleta** | O bloqueio 42883 foi removido; ainda requer requester e reviewer reais. | | Importações — lista | `/organizacoes/{tenantId}/importacoes` | `tenant.imports.read` | status, solicitante, minhas, revisão, datas | leitura paginada | **Ainda incompleta** | SQL 42601 corrigido; executar todas as combinações pedidas no PostgreSQL. | | Importação — revisão/preview | `/importacoes/{id}`, `/importacoes/preview/...` | `tenant.imports.manage/confirm`, documentos | diagnóstico, sugestões/manual, confirmar/cancelar | import/events/audit/contract | **Bloqueada por dependência identificada** | Scanner/OCR não podem ser simulados; testar original, estados e contrato resultante. | diff --git a/src/Odca.Api/Controllers/ReviewRequestsController.cs b/src/Odca.Api/Controllers/ReviewRequestsController.cs index cca28f2..22a9ad7 100644 --- a/src/Odca.Api/Controllers/ReviewRequestsController.cs +++ b/src/Odca.Api/Controllers/ReviewRequestsController.cs @@ -25,7 +25,8 @@ public async Task List(Guid tenantId, [FromQuery] string? status, scope = scope is "requested" or "assigned" or "all" ? scope : "requested"; if ((scope == "all" || assigneeId.HasValue) && !canManage) return Forbid(); if (from > to) return ValidationProblem("A data inicial deve ser anterior ou igual à data final."); - await SetTenant(connection, tenantId, actor.Value, ct); + await using var transaction = await connection.BeginTransactionAsync(ct); + await SetTenant(connection, tenantId, actor.Value, transaction, ct); page = Math.Max(1, page); pageSize = Math.Clamp(pageSize, 1, 100); const string where = """ r.tenant_id=@tenantId @@ -47,7 +48,7 @@ SELECT count(*)::integer FROM odca.contract_review_requests r JOIN odca.contracts c ON c.tenant_id=r.tenant_id AND c.id=r.contract_id WHERE {where} - """, args, cancellationToken: ct)); + """, args, transaction, cancellationToken: ct)); var rows = await connection.QueryAsync(new CommandDefinition($""" SELECT r.id AS Id,r.contract_id AS ContractId,r.requested_by AS RequestedBy,c.title AS Contract,r.status AS Status,requester.display_name AS Requester, assignee.display_name AS Assignee,r.opened_at AS OpenedAt,r.updated_at AS UpdatedAt,r.due_at AS DueAt,r.row_version AS Version, @@ -60,7 +61,8 @@ FROM odca.contract_review_requests r LEFT JOIN odca.contract_review_comments cm ON cm.tenant_id=r.tenant_id AND cm.review_id=r.id WHERE {where} GROUP BY r.id,c.title,requester.display_name,assignee.display_name ORDER BY r.updated_at DESC,r.id LIMIT @pageSize OFFSET @offset - """, new { args.tenantId,args.actor,args.status,args.scope,args.assigneeId,args.contractId,args.from,args.to,args.search,args.offset,args.pageSize,canManage }, cancellationToken: ct)); + """, new { args.tenantId,args.actor,args.status,args.scope,args.assigneeId,args.contractId,args.from,args.to,args.search,args.offset,args.pageSize,canManage }, transaction, cancellationToken: ct)); + await transaction.CommitAsync(ct); return Ok(new ReviewQueuePage(rows.Select(Map).ToArray(), page, pageSize, total)); } @@ -143,7 +145,8 @@ public async Task Detail(Guid tenantId, Guid reviewId, Cancellati await using var connection = await dataSource.OpenConnectionAsync(ct); if (!await Allowed(connection, actor.Value, tenantId, "tenant.reviews.read", ct)) return Forbid(); var internalAccess = await Allowed(connection, actor.Value, tenantId, "tenant.reviews.decide", ct); - await SetTenant(connection, tenantId, actor.Value, ct); + await using var transaction = await connection.BeginTransactionAsync(ct); + await SetTenant(connection, tenantId, actor.Value, transaction, ct); var row = await connection.QuerySingleOrDefaultAsync(new CommandDefinition(""" SELECT r.id AS Id,r.contract_id AS ContractId,r.requested_by AS RequestedBy,c.title AS Contract,r.status AS Status,requester.display_name AS Requester, assignee.display_name AS Assignee,r.instructions AS Instructions,r.opened_at AS OpenedAt,r.updated_at AS UpdatedAt, @@ -152,7 +155,7 @@ public async Task Detail(Guid tenantId, Guid reviewId, Cancellati JOIN odca.users requester ON requester.id=r.requested_by LEFT JOIN odca.contract_review_steps s ON s.tenant_id=r.tenant_id AND s.review_id=r.id AND s.status='current' LEFT JOIN odca.users assignee ON assignee.id=s.reviewer_id WHERE r.tenant_id=@tenantId AND r.id=@reviewId - """, new { tenantId, reviewId }, cancellationToken: ct)); + """, new { tenantId, reviewId }, transaction, cancellationToken: ct)); if (row is null) return NotFound(); if (!internalAccess && row.RequestedBy != actor.Value) return NotFound(); var messages = await connection.QueryAsync(new CommandDefinition(""" @@ -160,12 +163,13 @@ public async Task Detail(Guid tenantId, Guid reviewId, Cancellati cm.created_at AS CreatedAt,(cm.resolved_at IS NOT NULL) AS Resolved FROM odca.contract_review_comments cm JOIN odca.users u ON u.id=cm.author_id WHERE cm.tenant_id=@tenantId AND cm.review_id=@reviewId AND (@internalAccess OR cm.visibility='client') ORDER BY cm.created_at,cm.id - """, new { tenantId, reviewId, internalAccess }, cancellationToken: ct)); + """, new { tenantId, reviewId, internalAccess }, transaction, cancellationToken: ct)); var history = await connection.QueryAsync(new CommandDefinition(""" SELECT e.id AS Id,e.event_type AS Type,u.display_name AS Actor,e.occurred_at AS OccurredAt FROM odca.contract_review_events e JOIN odca.users u ON u.id=e.actor_id WHERE e.tenant_id=@tenantId AND e.review_id=@reviewId ORDER BY e.occurred_at,e.id - """, new { tenantId, reviewId }, cancellationToken: ct)); + """, new { tenantId, reviewId }, transaction, cancellationToken: ct)); + await transaction.CommitAsync(ct); return Ok(new ReviewDetail(row.Id,row.ContractId,row.Contract,row.Status,row.Requester,row.Assignee,row.Instructions,row.OpenedAt,row.UpdatedAt,row.DueAt,row.Version,row.DocumentVersionId,row.GeneratedVersionId,messages.ToArray(),history.ToArray())); } @@ -271,7 +275,6 @@ ON CONFLICT(tenant_id,deduplication_key) DO NOTHING private Guid? Actor()=>Guid.TryParse(User.FindFirstValue("sub"),out var id)?id:null; private static Task Allowed(NpgsqlConnection c,Guid actor,Guid tenant,string permission,CancellationToken ct)=>c.ExecuteScalarAsync(new CommandDefinition("SELECT odca.tenant_actor_has_permission(@actor,@tenant,@permission)",new{actor,tenant,permission},cancellationToken:ct)); - private static Task SetTenant(NpgsqlConnection c,Guid tenant,Guid actor,CancellationToken ct)=>c.ExecuteAsync(new CommandDefinition("SELECT set_config('odca.tenant_id',@tenant::text,false),set_config('odca.user_id',@actor::text,false)",new{tenant,actor},cancellationToken:ct)); private static Task SetTenant(NpgsqlConnection c,Guid tenant,Guid actor,NpgsqlTransaction tx,CancellationToken ct)=>c.ExecuteAsync(new CommandDefinition("SELECT set_config('odca.tenant_id',@tenant::text,true),set_config('odca.user_id',@actor::text,true)",new{tenant,actor},tx,cancellationToken:ct)); private static ReviewQueueItem Map(QueueRow r)=>new(r.Id,r.ContractId,r.Contract,r.Status,r.Requester,r.Assignee,r.OpenedAt,r.UpdatedAt,r.DueAt,r.Version,r.PublicMessages,r.PendingComments); private sealed record QueueRow(Guid Id,Guid ContractId,string Contract,string Status,string Requester,string? Assignee,DateTimeOffset OpenedAt,DateTimeOffset UpdatedAt,DateTimeOffset? DueAt,long Version,int PublicMessages,int PendingComments); diff --git a/tests/Odca.IntegrationTests/ReviewAuthorizationBehaviorTests.cs b/tests/Odca.IntegrationTests/ReviewAuthorizationBehaviorTests.cs new file mode 100644 index 0000000..efcb534 --- /dev/null +++ b/tests/Odca.IntegrationTests/ReviewAuthorizationBehaviorTests.cs @@ -0,0 +1,137 @@ +using System.Security.Claims; +using Dapper; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Npgsql; +using Odca.Api.Controllers; +using Odca.Contracts.Reviews; + +namespace Odca.IntegrationTests; + +public sealed class ReviewAuthorizationBehaviorTests(DatabaseFixture database) : IClassFixture +{ + private static readonly Guid TenantId = Guid.Parse("72000000-0000-0000-0000-000000000010"); + private static readonly Guid ReviewerId = Guid.Parse("72000000-0000-0000-0000-000000000001"); + private static readonly Guid ReaderId = Guid.Parse("72000000-0000-0000-0000-000000000002"); + private static readonly Guid BlockedId = Guid.Parse("72000000-0000-0000-0000-000000000003"); + private static readonly Guid ReviewerRoleId = Guid.Parse("72000000-0000-0000-0000-000000000011"); + + [Fact] + public async Task CanonicalPermissionAndAssigneeEndpointEnforceMembershipAndDoNotLeakRlsContext() + { + await SeedAsync(); + var builder = new NpgsqlConnectionStringBuilder(database.AppConnectionString) + { + MaxPoolSize = 1, + MinPoolSize = 0 + }; + await using var dataSource = NpgsqlDataSource.Create(builder.ConnectionString); + + await using (var connection = await dataSource.OpenConnectionAsync()) + { + Assert.True(await connection.ExecuteScalarAsync( + "SELECT odca.tenant_actor_has_permission(@actor,@tenant,'tenant.reviews.decide')", + new { actor = ReviewerId, tenant = TenantId })); + Assert.False(await connection.ExecuteScalarAsync( + "SELECT odca.tenant_actor_has_permission(@actor,@tenant,'tenant.reviews.decide')", + new { actor = ReaderId, tenant = TenantId })); + Assert.False(await connection.ExecuteScalarAsync( + "SELECT odca.tenant_actor_has_permission(@actor,@tenant,'tenant.reviews.decide')", + new { actor = BlockedId, tenant = TenantId })); + } + + var authorized = Controller(dataSource, ReviewerId); + var response = Assert.IsType(await authorized.Assignees(TenantId, default)); + var assignees = Assert.IsAssignableFrom>(response.Value); + var reviewer = Assert.Single(assignees); + Assert.Equal((ReviewerId, "Revisor sintético"), (reviewer.Id, reviewer.Name)); + + var queueResponse = Assert.IsType(await authorized.List( + TenantId, status: null, scope: "all", ct: default)); + var queue = Assert.IsType(queueResponse.Value); + Assert.Empty(queue.Items); + Assert.Equal(0, queue.Total); + + // The endpoint uses transaction-local RLS identity. With a one-connection pool, + // this proves that returning the physical connection does not expose its tenant + // or actor to the next borrower. + await using (var reused = await dataSource.OpenConnectionAsync()) + { + var context = await reused.QuerySingleAsync( + "SELECT current_setting('odca.tenant_id',true) AS \"Tenant\", current_setting('odca.user_id',true) AS \"Actor\""); + Assert.True(string.IsNullOrEmpty(context.Tenant)); + Assert.True(string.IsNullOrEmpty(context.Actor)); + } + + Assert.IsType(await Controller(dataSource, ReaderId).Assignees(TenantId, default)); + Assert.IsType(await Controller(dataSource, BlockedId).Assignees(TenantId, default)); + + await SetTenantStatusAsync("suspended"); + Assert.IsType(await Controller(dataSource, ReviewerId).Assignees(TenantId, default)); + } + + private static ReviewRequestsController Controller(NpgsqlDataSource dataSource, Guid actor) + { + var controller = new ReviewRequestsController(dataSource); + controller.ControllerContext = new ControllerContext + { + HttpContext = new DefaultHttpContext + { + User = new ClaimsPrincipal(new ClaimsIdentity( + [new Claim("sub", actor.ToString())], "integration-test")) + } + }; + return controller; + } + + private async Task SetTenantStatusAsync(string status) + { + await using var connection = new NpgsqlConnection(database.AdminConnectionString); + await connection.ExecuteAsync( + "UPDATE odca.tenants SET status=@status WHERE id=@tenant", new { status, tenant = TenantId }); + } + + private async Task SeedAsync() + { + const string sql = """ + DELETE FROM odca.member_roles WHERE tenant_id=@tenant; + DELETE FROM odca.role_permissions WHERE role_id=@role; + DELETE FROM odca.memberships WHERE tenant_id=@tenant; + DELETE FROM odca.roles WHERE tenant_id=@tenant; + DELETE FROM odca.tenants WHERE id=@tenant; + DELETE FROM odca.sessions WHERE user_id IN (@reviewer,@reader,@blocked); + DELETE FROM odca.users WHERE id IN (@reviewer,@reader,@blocked); + + INSERT INTO odca.users(id,email,email_normalized,login_normalized,display_name,password_hash,must_change_password,email_verified_at) + VALUES + (@reviewer,'reviewer-behavior@odca.local','REVIEWER-BEHAVIOR@ODCA.LOCAL','REVIEWER-BEHAVIOR@ODCA.LOCAL','Revisor sintético','not-used',false,now()), + (@reader,'reader-behavior@odca.local','READER-BEHAVIOR@ODCA.LOCAL','READER-BEHAVIOR@ODCA.LOCAL','Leitor sintético','not-used',false,now()), + (@blocked,'blocked-behavior@odca.local','BLOCKED-BEHAVIOR@ODCA.LOCAL','BLOCKED-BEHAVIOR@ODCA.LOCAL','Revisor bloqueado','not-used',false,now()); + INSERT INTO odca.tenants(id,business_code,display_name,status) + VALUES(@tenant,'REVIEW-BEHAVIOR','Organização sintética de revisão','active'); + INSERT INTO odca.memberships(tenant_id,user_id,status) + VALUES(@tenant,@reviewer,'active'),(@tenant,@reader,'active'),(@tenant,@blocked,'blocked'); + INSERT INTO odca.roles(id,scope_type,tenant_id,code,display_name,is_system) + VALUES(@role,'tenant',@tenant,'reviewer-behavior','Revisor',false); + INSERT INTO odca.role_permissions(role_id,permission_code) + VALUES(@role,'tenant.reviews.decide'),(@role,'tenant.reviews.read'); + INSERT INTO odca.member_roles(tenant_id,user_id,role_id,assigned_by) + VALUES(@tenant,@reviewer,@role,@reviewer),(@tenant,@blocked,@role,@reviewer); + """; + await using var connection = new NpgsqlConnection(database.AdminConnectionString); + await connection.ExecuteAsync(sql, new + { + tenant = TenantId, + reviewer = ReviewerId, + reader = ReaderId, + blocked = BlockedId, + role = ReviewerRoleId + }); + } + + private sealed class ConnectionContext + { + public string? Tenant { get; set; } + public string? Actor { get; set; } + } +}