Skip to content

feat: define the group-scope primitive for Editors (#990 slice 0) - #1045

Merged
milkway merged 1 commit into
mainfrom
feat/990-slice0-editor-scope
Jul 29, 2026
Merged

feat: define the group-scope primitive for Editors (#990 slice 0)#1045
milkway merged 1 commit into
mainfrom
feat/990-slice0-editor-scope

Conversation

@milkway

@milkway milkway commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Fatia 0 de 5 do plano da #990. Não muda o comportamento de nenhuma tela — entrega só o primitivo, para as fatias seguintes optarem handler por handler sem cada uma inventar sua própria regra de acesso.

Implementada por agente codex exec (gpt-5.6-sol, reasoning high); diff e gate revisados e reexecutados por mim.

O que entra

crates/ruscker-admin/src/scope.rs: o extrator EditorScope resolve {role, actor, groups, unscoped} e expõe os predicados que as fatias 1–3 vão consumir — may_touch_spec, may_touch_user, may_assign_groups, may_assign_role e merge_preserving_out_of_scope.

O modelo reusa o que já roda em produção: o escopo do Editor são os grupos dele, mesma ideia do Spec::access_allows que o proxy e a landing usam. Nada de ownership por app — o #517 rejeitou isso explicitamente.

Decisões que ficaram documentadas no código

  • Fail-closed. Conta inexistente, banco ausente ou lookup com erro ⇒ escopo vazio mas ainda escopado, nunca unscoped. O Editor nesse estado vê apenas apps abertos, e o erro vai para o log.
  • Break-glass intacto. A sessão por token (actor: None, papel Admin) segue irrestrita: acesso de emergência tem de funcionar mesmo com o banco de contas danificado.
  • Lookup por request, não na sessão. set_groups não revoga sessão, então cachear grupos na sessão manteria vivo um acesso já revogado — tirar alguém de um grupo tem de valer no próximo clique. Há precedente do mesmo formato no must_change_password_guard.
  • Spec restrito só por access-users fica Admin-only. is_open() exige access-groups E access-users vazios, então uma ACL de usuário nomeado não oferece fronteira de grupo de onde derivar autoridade de Editor. Documentado como decisão deliberada, não deixado como acidente.

Testes

Dez testes de unidade, um por invariante, nomeados pelo que protegem: Admin e token irrestritos; Editor alcança app aberto e app com grupo em comum, mas não o de outro time nem o restrito só por access-users; Editor nunca toca conta Admin nem usuário sem grupo em comum; não concede grupo que não tem nem papel Admin; merge_preserving_out_of_scope preserva o grupo de fora do escopo tanto ao adicionar quanto ao remover; escopo vazio só alcança apps abertos; e a filiação é relida a cada resolução.

Duas observações da minha revisão (não bloqueantes, registradas para as próximas fatias)

  1. merge_preserving_out_of_scope ordena a lista também no caminho Admin, então a ordem gravada dos grupos passa de "ordem do formulário" para alfabética quando isso for ligado na fatia 2. Determinístico, bom para teste, mas é mudança sutil e melhor estar escrita do que descoberta depois.
  2. O extrator acrescenta um segundo lookup por request em páginas que já pagam o do must_change_password_guard — aquele tem o cache do Perf: must_change_password_guard faz SELECT + valida sessão 2x por request /admin/* #903, este não. Irrelevante aqui (nada ligado), mas é o momento de anotar: nas fatias 1–3 toda página escopada vai usá-lo, e se aparecer pressão de latência o cache existente é o padrão a copiar.

Gate reexecutado por mim na branch: cargo test limpo, clippy --all-targets -- -D warnings sem warning, i18n-check OK.

🤖 Generated with Claude Code

Resolve Editor memberships per request and fail closed so group revocation takes effect immediately. Centralize spec, user, assignment, and merge decisions without changing any handler behavior.
@milkway

milkway commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Segunda passada da revisão

Este é o primitivo em que as outras quatro fatias se apoiam, então fui atrás de buraco de verdade. O código passou; o achado é sobre a fatia 1, e já reforcei o prompt do agente com ele.

Armadilha encontrada em specs.rs::index: a lista de apps não itera Spec. Ela faz o SELECT enxuto do #588, produzindo SpecRow, e depois enriquece cada linha com metadados de catalog::effective_specs_cached (logo, access_groups, restricted). Se o id da linha não estiver nesse mapa, row.access_groups fica vazio — e um app restrito pareceria aberto para um predicado aplicado sobre a linha.

Como may_touch_spec trata app aberto como Editor-global (decisão do lead), aplicar o predicado no SpecRow em vez de no Spec efetivo seria um bypass silencioso: exatamente o tipo de coisa que passa em review porque o teste feliz continua verde. A instrução para a fatia 1 é resolver sempre contra o Spec efetivo e falhar fechado quando um id não resolver — ausência de metadado não é "app aberto".

Verificações que passaram nesta passada:

  • Viewer não alcança o extrator (o RequireEditor reutilizado rejeita antes).
  • Comparação de grupo é case-sensitive, igual ao Spec::access_allows que já governa landing e proxy — consistente com o comportamento em produção, em vez de inventar normalização só no painel.
  • access_groups: Some([]) com access_users preenchido cai em Admin-only, como documentado.
  • Nenhum handler, rota ou template referencia o novo extrator: a promessa de "fatia sem mudança de comportamento" se sustenta no diff.

CI verde nos 5 checks; gate local reexecutado por mim antes do push.

@milkway
milkway merged commit be02ffb into main Jul 29, 2026
5 checks passed
@milkway
milkway deleted the feat/990-slice0-editor-scope branch July 29, 2026 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant