Repository navigation
fix: CT-28 blacklist policy, FE-18/19/20 2FA, money formatting and booking-payment e2e (#777-#780) - #820
Merged
Conversation
A blacklisted administrator could still create, approve, reject and execute governance proposals because require_admin only checked membership of the admin list. Blacklist checks now run at the single authorisation choke point that every privileged call goes through, and again at execution, where a blacklisted signer's stored approval stops counting towards the quorum and a blacklisted proposer cannot have their proposal run. Self-blacklisting is rejected so one admin cannot lock governance out of its only account, and the new error semantics are covered by role-style tests for creation, approval, rejection, execution and direct admin actions. The test module's merge-damaged tail (unclosed delimiters and spliced test bodies from merges 97b99c8, 3510a28 and 8649a50) is rebuilt so the CT-23 to CT-30 tests compile and run again. 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
…ment e2e (FE-18/19/20) FE-19: `formatNaira` in lib/utils is now the single locale-aware kobo-to-naira formatter. It splits kobo from the whole naira with integer arithmetic before formatting, so nothing is rounded or scaled twice and values past MAX_SAFE_INTEGER/100 stay exact. Money fields are typed as KoboAmount / NairaAmount and every ad-hoc `x / 100` currency call site (bookings, payments, invoices, admin and dashboard summaries, workspaces) now uses the one formatter. FE-18: the 2FA sign-in challenge and setup modal expose their loading, invalid code, backup-code and recovery outcomes: errors are tied to the code input with aria-invalid/aria-describedby plus role=alert, submission is disabled and aria-busy while pending, progress and success are announced through polite live regions, the modal is a labelled dialog that closes on Escape and moves focus into itself, and an expired challenge offers a way back to sign in. Both surfaces describe a failure through the shared `describeTwoFactorError`. FE-20: Playwright coverage for the booking-to-payment journey runs the real form against a mocked API and provider, asserting a successful payment, a provider that closes without paying, a workspace picked from the catalogue, and a retry after a failed initialization that never creates a second booking. The files the touched flows depend on were also repaired: merge-damaged splices in lib/apiClient.ts, app/invoices and app/workspaces, the `""/lib` import prefixes and a `celup` state literal in app/settings, and BookingForm calling a `markProviderOpened` the payment hook never exposed. With those fixed the frontend type-checks cleanly again. 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
|
@MatanOga01 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
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
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.
What this PR does
Resolves the four issues assigned to me, each with tests, in one PR.
Closes #777
Closes #778
Closes #779
Closes #780
#780 (CT-28) — Define blacklist behavior for admins and multisig signers
Policy: blacklisting an administrator or multisig signer revokes their governance power until another admin lifts it.
require_adminis the single choke point every privileged call passes through, so the blacklist is now checked there — a blacklisted admin cannot create proposals, vote, or use any direct admin action.execute_proposalapplies the policy again at execution time: proposals raised by a blacklisted administrator do not run, and approvals cast by blacklisted signers stop counting towards the quorum. Filtering instead of failing outright means the remaining signers can still reach the threshold, so blacklisting one signer cannot freeze governance permanently; lifting the blacklist restores the stored approval without re-voting.blacklist_userrefuses self-blacklisting: in single-admin mode that would have left the only account able to lift the restriction blacklisted.UserBlacklisted(134) is reused (no error-code churn); its description now covers privileged actions in general.access_control_tests.rs) cover creation, approval, rejection, quorum exclusion, execution blocking + recovery, direct admin actions, and the self-blacklist guard.contracts/access_control/src/access_control_tests.rsalso had to be repaired to be able to add any test: merges97b99c8,3510a28and8649a50left the tail of the file with unclosed delimiters and test bodies spliced into each other, so the crate did not compile at all. The CT-23…CT-30 tests are rebuilt from those fragments.#778 (FE-19) — Standardize currency and amount formatting
formatNaira(kobo, options)infrontend/lib/utils.tsis now the single locale-aware formatter. It splits kobo from whole naira with integer arithmetic before formatting, so a value is never divided, rounded or scaled twice —MAX_SAFE_INTEGERkobo renders₦90,071,992,547,409.91where dividing first renders…409.90.KoboAmount/NairaAmountdocument and type the money carried byPayment,BookingandPriceEstimate.x / 100+toLocaleString/Intl.NumberFormatcall site is replaced by the shared formatter: bookings, payment step, invoices, admin bookings/workspaces/analytics, member dashboard stats and workspace cards/detail. The two places that displayedmaximumFractionDigits: 0(i.e. silently dropped kobo) now show the exact amount; the analytics summary keeps compact notation through the same formatter.#777 (FE-18) — Complete accessible 2FA challenge states
app/(auth)/verify-2fa/page.tsx): the error is rendered inline and associated with the code input (aria-invalid,aria-describedby,role="alert"), the field is focused and selected after a rejection, submission is disabled andaria-busywhile pending, and progress/backup-code/success outcomes go through a polite live region. A missing or expiredtempTokenshows a recovery notice with a link back to sign-in.useConfirm2faexportsdescribeTwoFactorError, which distinguishes invalid code, rate limiting and an expired challenge instead of echoing a raw message.TwoFactorModalis a labelledrole="dialog" aria-modalthat closes on Escape, moves focus into itself, ties the code error to the input, announces verification and backup-code outcomes, and reports disable/setup failures inline. Backup codes are exposed as a labelled group and the copy actions announce themselves.#779 (FE-20) — Add end-to-end coverage for the booking-to-payment journey
frontend/e2e/booking-payment.spec.ts(Playwright) drives the real form against a mocked API and Paystack popup:POST /bookings, twoPOST /payments/initialize).frontend/playwright.config.ts(bootsnext devwith the API base URL pointed at the same origin) and atest:e2escript.BookingFormgained theid/htmlForpair for the workspace select, and the workspace label/rate is now formatted by the shared formatter.Merge damage repaired along the way
These files could not be parsed on
mainand are on the paths the issues touch, so they are repaired here (maindoes not type-check without these):frontend/lib/apiClient.ts(missing method brace swallowed a doc comment),frontend/app/invoices/page.tsx(a spliced fetch-based download inside theapiClient.downloadflow),frontend/app/settings/page.tsx(""/lib/...import prefixes and acelupstate literal),frontend/app/workspaces/page.tsx(duplicated query destructuring andhasActiveFilters), andBookingFormcalling amarkProviderOpenedthatuseInitializePaymentnever exposed.Verification
cd frontend && npx tsc --noEmitcd frontend && npx vitest runcd frontend && npx playwright testcd frontend && npm run lintcd contracts && cargo fmt -p access_control -- --checkcd contracts && cargo clippy -p access_control --all-targets --all-featurestoo_many_argumentswarning on the contract wrappercd contracts && cargo test -p access_controlPre-existing failures this PR neither causes nor fixes
cargo test -p access_controlreports 25 failures. I ran the same suite on a clean worktree of60f467e(the last commit whose test module compiled, before the corruption above): it fails identically there (49 passed / 25 failed), so these are pre-existing. All 25 are auth-harness issues in untouched tests — tests that never callenv.mock_all_auths(), and tests that invoke several privileged entry points inside oneenv.as_contractframe, whichmock_all_authsrejects with "frame is already authorized" now thatrequire_adminbinds the caller's signature. Fixing them means restructuring ~25 unrelated tests, so it is left out of an issue-scoped PR — happy to follow up (or fold it in) if you prefer.The workspace also still fails to build as a whole independently of this PR:
contracts/membership_token/src/lib.rshas an unclosed delimiter andcontracts/manage_hub/src/multisig_guards.rs(tests) containsenv.as(|| {, socargo test --all/cargo build --allfail before reachingaccess_control. Everything above was verified with-p access_control.