From b001987665c83c4dbef120b31fe26750dd1a6bc8 Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Wed, 28 Oct 2026 23:44:14 +0200 Subject: [PATCH] feat: revoke button and notices for scheduled grants (part 2 of #108) - The message of a scheduled grant keeps a Revoke button until the grant ends. A Revoke ends the grant (grant_ended, cause revoked). It passes the user, chat, data and message checks; it skips the changed-message check, because it only takes a right away, except on a message whose request may still wait. - When a grant ends for any cause, also a later Deny, its message says how and when, and the button goes away. - A grant that reaches its end with uses left sends a "not used" notice. - On a broker stop, one notice lists the dropped grants. The stop denies every request before it updates the messages and sends the notice. Refs #108 --- DESIGN.md | 111 ++++++- THREAT-MODEL.md | 25 +- cmd/nodkey/broker.go | 4 +- docs/manual-telegram-test.md | 41 +++ docs/setup.md | 19 +- internal/approval/approval.go | 16 +- internal/approval/approver.go | 32 +- internal/approval/decide.go | 9 + internal/approval/grants.go | 201 +++++++++++- internal/approval/preapprove_test.go | 1 + internal/approval/request.go | 6 +- internal/approval/revoke_test.go | 464 +++++++++++++++++++++++++++ internal/audit/event.go | 7 +- internal/broker/broker.go | 8 + internal/broker/preapprove.go | 66 ++++ internal/broker/revoke_test.go | 30 ++ internal/broker/stopnotice_test.go | 127 ++++++++ internal/telegram/core_test.go | 7 +- internal/telegram/fake_test.go | 2 +- internal/telegram/poll.go | 36 +++ internal/telegram/revoke_test.go | 440 +++++++++++++++++++++++++ internal/telegram/telegram.go | 54 +++- internal/telegram/text.go | 76 ++++- 23 files changed, 1734 insertions(+), 48 deletions(-) create mode 100644 internal/approval/revoke_test.go create mode 100644 internal/broker/revoke_test.go create mode 100644 internal/broker/stopnotice_test.go create mode 100644 internal/telegram/revoke_test.go diff --git a/DESIGN.md b/DESIGN.md index 64264ba..1ab9ab8 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -554,7 +554,16 @@ expires 14:07:00 (120 s) A scheduled grant (`nodkey preapprove`, section 8) shows the secrets, or the key, hosts and remote user, the number of uses and the start and the end with the date and the zone. It has only "Approve" and "Deny"; the -message is in section 8. +message is in section 8. After "Approve", the message keeps one **Revoke** +button until the grant ends. Its data is `r:` and the request ID, as for +the other buttons. It passes the checks of "Telegram details" below: +the approver's user ID, the private chat, not an inline message, data of +the strict form, and in the core the grant of the request ID on that +message. It skips the check of a changed message, because a Revoke only +takes a right away. It never raises the alert about a changed message, +except on a message whose request may still wait: there it is refused +as a changed message, with the alert. The alert still comes from the +other taps. A notice of a "never ask" use (section 12) has no buttons and no expiry. A notice of a use of a scheduled grant is the same, with the second line @@ -570,6 +579,22 @@ allowed by a "never ask" rule, without a tap command: restic backup /srv ``` +A notice that a scheduled grant ended with uses left (section 8) is the +same, with the second line `not used: grant ID ended with 1 of 1 uses +left`. It goes to the notice chat without a sound: + +``` +๐Ÿ”” Pre-approval ยท request m4k2 +not used: grant 5Zq0v9xS3JkT1wYb7nQe2A ended with 1 of 1 uses left +โœ… machine: laptop +โœ… secrets: backup-token +โœ… uses: 1 +โœ… from: 2026-10-28 03:00:00 UTC +โœ… until: 2026-10-28 04:00:00 UTC +โ€” claimed by the agent โ€” +reason: job nightly-backup +``` + A notice about the version of a client (section 10) has no buttons either. It goes to the notice chat without a sound, once for each client in each start of the broker, rarely more than once (see "Telegram @@ -913,7 +938,14 @@ Not masked, for example: 14:05:12, for 1 use(s) until 2026-10-28 04:00:00 UTC" (a scheduled grant), "โ›” denied by you at 14:02:41", "โŒ› expired โ€” denied", "โ›” cancelled by the client" or - "โ›” denied: the broker stopped taking answers". An edit without a + "โ›” denied: the broker stopped taking answers". The message of a + scheduled grant is edited twice more (section 8): when the grant + starts, to add the Revoke button, and when the grant ends, to remove + it and add a line that says how and when it ended, such as + "โ›” revoked by you at 2026-10-28 03:10:00 UTC, with 1 use(s) left". A tap on + the Approve or Deny button of that request on such a message is a late + tap. A button of another request on it is refused as a changed message, + with the alert. An edit without a keyboard (`reply_markup`) removes the buttons too: the manual test of issue #17 showed this on 2026-10-15 ([docs/manual-telegram-test.md](docs/manual-telegram-test.md), check @@ -1314,7 +1346,7 @@ use without a tap when one grant covers it fully: **End.** A grant ends when its uses reach 0, when its end time comes, when the client cancels it, when a later Deny covers it, or when the broker stops. Each end writes one `grant_ended` entry with the cause (`used_up`, -`ended`, `cancelled`, `denied` or `stopped`) and the uses left. A timer +`ended`, `cancelled`, `denied`, `revoked` or `stopped`) and the uses left. A timer ends a grant at its end time; a request never finds a grant that is past its end. @@ -1326,21 +1358,64 @@ its end. - **A later Deny.** A Deny for the same client and any secret or key of a grant ends it, also before its start (as the Deny of a grant of "Approve N min" does, above). A Deny that comes after the tap but before the - grant starts stops it from starting. + grant starts stops it from starting; the message then keeps the + approval line, as for every grant that did not start. The message of a + grant that started says "ended by a later Deny", and the Revoke button + goes. +- **Revoke on the phone.** After the grant started, its message has one + **Revoke** button (section 7). A tap on it ends the grant at once, with + cause `revoked`, through the same end path as the other causes, and the + phone shows "Revoked." The tap counts only if the checks of section 7 + pass (a changed message does not matter for a Revoke), and then only if + the grant is still open and the tap is on its own message. Any other Revoke changes nothing: the phone shows "This + request has ended." (the grant is gone: ended, used up, cancelled or + denied), "This button does not count." (the message is not the + grant's); each + is a `foreign_tap` entry with its cause (section 9), as for the other + buttons. A Revoke only takes a right away, so a forged or replayed one + can only end a grant; the data is still bound to the request ID and to + the message, so it cannot name another grant. The grant can be revoked + before its start too. +- **The message at the end.** At every end (`used_up`, `ended`, + `cancelled`, `denied`, `revoked`, `stopped`) the broker edits the + message of the grant: no button, and a line that says how and when it + ended, with the uses left. The edits of one grant run one after the + other, in the order of the events, in a goroutine of the core, so a + slow Telegram never delays a request. A failed edit is logged; the + audit log has the end anyway. The edit that adds the Revoke button is + tried again after 10 and 20 seconds, while the grant lives (3 tries in + all). A Revoke tap on a message whose request still waits is refused as a + changed message, with the alert: such a message has no Revoke button. +- **Not used.** A grant that reaches its end time with uses left (cause + `ended`, `uses_left` above 0) sends one silent notice to the notice chat + (section 7). It does not count toward the limit of 6 per client, because + no request caused it, but it obeys the limit of 20 in all in any 60 + seconds of the Telegram code (the limit of the notices of uses). A + grant that was used up, cancelled, revoked, denied or dropped by a stop + sends none. If the notice is over the limit or cannot be sent, the + broker logs it. Nothing else changes: the `grant_ended` entry is + written before. - **A restart** drops all grants, as for every other grant. At a stop, the broker writes `grant_ended` with cause `stopped` for each grant that - is open. + is open, edits their messages, and sends one **stop notice** (section + 15, step 2 of the stop). It lists each dropped grant: its ID, the + client, the secrets or the key, and the uses left, in at most 20 lines. + It is one `Inform` call to the notice chat, without a sound, with its + own count (3 in any 60 seconds). It has a time limit of 5 seconds, so a + stop never waits for a Telegram that does not answer. With no scheduled + grant, nothing is sent. **Answers to the open questions of issue 108.** 1. A broker restart drops the scheduled grants, so a job asks as usual. - One notice that lists the dropped grants is planned (issue 111). + One notice lists the dropped grants (above). Keeping them across a + restart is planned (issue 111). 2. Several secrets in one run: one tap makes one grant for all of them, as above. A run that asks for a secret outside it is not covered. 3. Cancel by the client: yes, as above. 4. The clock set back: the uses already made still count. -5. A later Deny cancels a scheduled grant. The Deny message will say so - (a later change). +5. A later Deny cancels a scheduled grant, and the message of the grant + says so (above). 6. The `window` of a rule does not cap a scheduled grant. Only the limits of `[preapprove]` do. @@ -1415,7 +1490,7 @@ The fields of each event, after `seq`, `time`, `event` and `prev`: | `request` | the request fields; `message`: the exact text the approver saw, if a message was sent | | `decision` | the request fields; `user_id`: the Telegram user that answered; `answer`: `approve_once`, `approve_window` or `deny`; `window_minutes` for `approve_window` | | `grant_used` | the request fields; `via`: `grant`, `never_rule` or `scheduled` (then `grant_id` is in the request fields) | -| `grant_ended` | the request fields of the `preapprove` request that made the grant, with its `grant_id`; `cause`: `used_up`, `ended`, `cancelled`, `denied` or `stopped`; `uses_left` | +| `grant_ended` | the request fields of the `preapprove` request that made the grant, with its `grant_id`; `cause`: `used_up`, `ended`, `cancelled`, `denied`, `revoked` or `stopped`; `uses_left` | | `secret_sent`, `signed` | the request fields | | `denied` | the request fields; `cause`: `approver`, `expired`, `cancelled`, `not_allowed`, `limit` or `error` | | `foreign_tap` | `user_id`, `chat_id`, `message_id`, `data`: the button data, at most 64 bytes, and `cause`: why it did not count (section 7). An entry with cause `edited_message` can belong to a Deny that counted: its `data` is `d:` and the request ID, and that request has a `decision` entry with `deny` | @@ -1462,7 +1537,12 @@ grant comes when the grant ends. If a request ended it (the last use, or a Deny), the entry comes after the entries of that request that the approval writes (`request`, `decision`, `grant_used`, `denied`), and before `secret_sent` or `signed`, which the broker writes later. It names -the request that made the grant, not the request that ended it. +the request that made the grant, not the request that ended it. A Revoke +tap that counts writes only this entry, with cause `revoked`, and no +`decision` entry: the cause says who ended the grant, because only the +approver's tap can do it, and the tap passed the same checks as every +answer. A Revoke that does not count writes a `foreign_tap` entry, as for +the other buttons. A request that the broker denies before the approval core sees it gets a request ID from the broker, in the same form, and the entries `request` @@ -2472,7 +2552,12 @@ order: step, it sends no request and no notice. 2. The polling: the `getUpdates` call that waits is cancelled and its connection closed (section 7). From now on no answer counts: every - request that waits for the person is denied. + request that waits for the person is denied. The scheduled grants are + dropped, with their `grant_ended` entries and the edits of their + messages (together at most 5 seconds). Then the broker sends the stop + notice about them, if there are any (section 8), with a limit of 5 + seconds. The notice is sent after the polling stopped and before the + service stops, so it never delays more than 10 seconds. 3. The service: new requests get `503`, and the requests in progress end and get `503` too (section 10). The broker waits for them, and writes the `foreign_tap` entries that still wait. A notice about the version @@ -2488,7 +2573,9 @@ order: The stop takes at most 80 seconds. A request may need up to 30 seconds to take its answer, and the alert about an audit log that cannot be -written up to 30 seconds more. The tries of the alerts of the start run +written up to 30 seconds more. The updates of the messages of scheduled +grants (5 seconds) and the stop notice (5 seconds) add up to 10 seconds +after the polling stopped. The tries of the alerts of the start run at the same time as these steps, and end within 30 seconds. After 80 seconds the broker ends without the rest of the stop, and exits with 1. The example unit gives it 90 seconds (`TimeoutStopSec=`); then systemd diff --git a/THREAT-MODEL.md b/THREAT-MODEL.md index 707c22a..0400935 100644 --- a/THREAT-MODEL.md +++ b/THREAT-MODEL.md @@ -981,7 +981,21 @@ rule (DESIGN.md section 12, decision 3 B) again, but smaller. grants or rules. A run that asks for one more secret needs a tap. - A Deny of the same client for a secret of the grant ends it, also before its start. The client can cancel its own grant, which only takes a right - away. Another client's ID and an unknown ID get the same answer, so a + away. So can the person, with the **Revoke** button on the message of the + grant. A forged or replayed Revoke tap can only take a right away, so it + cannot start or widen a grant. A Revoke skips the check of a changed + message (T6), because it only takes a right away, and it never raises + the alert about a changed message, except on a message whose request + may still wait: there it is refused as a changed message, with the + alert. The alert still comes from the other taps. Every other check stays: the approver's user ID, the private chat, + not an inline message, strict button data, and in the core the grant of + the request ID on that very message. So a Revoke also counts on a + message that Telegram sends without content (older than 48 hours). A + tap on a grant message with the button of another request is refused + as a changed message, with the alert. A Revoke on a grant that has ended is + refused as a late tap. A Revoke that does not count is a `foreign_tap` + entry. One that counts is the `grant_ended` entry with cause `revoked`. + Another client's ID and an unknown ID get the same answer, so a client cannot learn which grants others hold. - The agent can fill the phone with requests for grants. They count as other requests toward the limits per client (DESIGN.md section 8), and @@ -990,7 +1004,14 @@ rule (DESIGN.md section 12, decision 3 B) again, but smaller. clock reaches it. A wall clock that someone sets forward on the broker host can start a grant early (T16 covers that host). One that is set back cannot make a grant last longer, and cannot give more uses. -- Grants live in memory. A broker restart drops them. +- Grants live in memory. A broker restart drops them. A grant that ends + with uses left sends a silent "not used" notice, and a stop sends one + notice that lists the dropped grants, so the person learns that a job + will ask for a tap. These notices are the broker's own text, with names + from the config and checked IDs; the "not used" notice also shows the + claimed reason below the claimed line, cleaned and masked as in every + message. If they cannot be sent, they are + logged; the audit log has every end. - A grant must not outlive the client that asked for it. If the client hangs up, or its answer cannot be written, after the grant started, the broker cancels the grant and writes `grant_ended` (`cancelled`), as for diff --git a/cmd/nodkey/broker.go b/cmd/nodkey/broker.go index 7f4a6a6..5e0b528 100644 --- a/cmd/nodkey/broker.go +++ b/cmd/nodkey/broker.go @@ -69,7 +69,9 @@ const ( // tries of the alert messages of the start that are in progress run at // the same time, for at most alertTimeout (sendAlerts). The example // unit sets TimeoutStopSec=90s, so the broker ends on its own first and - // logs why. + // logs why. The stop of the approver also takes up to 5 seconds for the + // updates of the messages of scheduled grants, and up to 5 seconds for + // the stop notice about them: 10 seconds more. stopTimeout = 80 * time.Second ) diff --git a/docs/manual-telegram-test.md b/docs/manual-telegram-test.md index b711ee5..d6dfc14 100644 --- a/docs/manual-telegram-test.md +++ b/docs/manual-telegram-test.md @@ -539,6 +539,44 @@ grep -cF -f "$T/telegram-token" "$T/broker.log" "$T/state/audit.jsonl" `grep` reads the token from the file, so it is not in the list of processes. Expected: it prints `0` for both files. +### Check 14: Scheduled grants (optional) + +This checks the Revoke button, the notice of a grant that was not used, +and the notice of a stop. In `$T/broker.toml`, add `preapprove = true` +to the section of `demo-always`, add an empty `[preapprove]` section, +and start the broker again. Then, terminal 2, with `NOW` and `LATER` as +times in the next 10 minutes (for example `2026-10-28T14:05` and +`2026-10-28T14:08`, in the local time of the broker): + +```sh +HOME="$T/home" "$T/nodkey" preapprove --secret demo-always \ + --from NOW --until LATER --uses 2 --reason "check 14" +``` + +1. Tap **Approve**. Look at the message. +2. Tap **Revoke**. Terminal 2 prints nothing more. Then + `ask demo-always "check 14, after the revoke"` asks for a tap. Tap + **Deny**. +3. Do step 1 again. Wait until `LATER`. +4. Do step 1 again. Stop the broker with Ctrl-C. + +Expected: + +- Step 1: the message has the line `๐Ÿ‘ approved by you at ...` and one + **Revoke** button. +- Step 2: the phone shows `Revoked.`. The message has no button, and a + line `โ›” revoked by you at ..., with 2 use(s) left`. A second tap on + the old button is not possible; if you can make one, the phone shows + `This request has ended.`. +- Step 3: after `LATER`, the message has no button and a line `โŒ› ended at + ... with 2 of 2 use(s) left`. A silent notice comes: `๐Ÿ”” + Pre-approval ...` with `not used: grant ... ended with 2 of 2 uses + left`. +- Step 4: the message has the line `โ›” dropped: the broker stopped at + ...`. One notice comes, `โ„น๏ธ nodkey: The broker stopped. It dropped 1 + scheduled grant(s) ...`, with the grant ID, `test`, `demo-always` and + the uses left. The broker still stops within a few seconds. + ## Clean up 1. Terminal 1: stop the broker with Ctrl-C, if it still runs. @@ -580,3 +618,6 @@ raw answer of the Bot API, and the lines of the log with `user_id` or | 11 | revoked token (optional) | exit code 77; no alert | | | | 12 | `audit verify` with the anchors | ok, all anchors match | | | | 13 | the bot token in the logs | `0` for `broker.log` and for `audit.jsonl` | | | +| 14 | Revoke on a scheduled grant | `Revoked.`; button gone; `revoked by you` | | | +| 14 | grant ends with uses left | `ended at ... with 2 of 2 use(s) left`; silent notice `not used` | | | +| 14 | stop with a scheduled grant | `dropped: the broker stopped`; one notice that lists the grant | | | diff --git a/docs/setup.md b/docs/setup.md index 502a7ab..b415c10 100644 --- a/docs/setup.md +++ b/docs/setup.md @@ -1357,10 +1357,13 @@ nodkey preapprove --secret backup-token \ The message shows the secret, the uses and the time, in the broker's local time. After **Approve**, the command prints the ID of the grant and -ends. At 03:00 the job runs `nodkey run` as usual, and gets the secret -without a tap. Each use sends a silent notice to the notice chat. When -the uses are gone, or at 04:00, the grant is gone. To end it earlier, -for example when the job is removed: +ends. The message keeps one **Revoke** button. At 03:00 the job runs +`nodkey run` as usual, and gets the secret without a tap. Each use sends +a silent notice to the notice chat. When the uses are gone, or at 04:00, +the grant is gone, and the message shows how and when it ended. If the +job did not run, or ran late, and a use is left at 04:00, a silent notice +says `not used`. To end the grant earlier, tap **Revoke** on the phone. +The client can also cancel it, for example when the job is removed: ```sh nodkey preapprove --cancel ID @@ -1383,9 +1386,11 @@ Things to know: `[preapprove]`: how far ahead it may start (`max_ahead`, 7 days), how long it is open (`max_open`, 8 hours), how many uses (`max_uses`, 10) and how many grants a machine holds (`max_grants`, 5). -- A broker restart drops all grants. The job then asks as usual. A - **Deny** for the same secret or key ends the grant, also before its - start. +- A broker restart drops all grants. The job then asks as usual. When + the broker stops, one notice lists the grants that it dropped: the ID, + the machine, the secret or key, and the uses left. A **Deny** for the + same secret or key ends the grant, also before its start; the message + of the grant says so. ## 12. Check the audit log diff --git a/internal/approval/approval.go b/internal/approval/approval.go index e016003..6382250 100644 --- a/internal/approval/approval.go +++ b/internal/approval/approval.go @@ -78,6 +78,11 @@ const MaxGrantHosts = 20 // drops a login after LoginGraceTime. const endTimeout = 5 * time.Second +// stopMessagesTimeout bounds all updates of the messages of scheduled +// grants when the core stops (dropScheduled), so a stop never waits for a +// Telegram that does not answer. +const stopMessagesTimeout = 5 * time.Second + // Options are the optional parts of a Core. type Options struct { // Clock is the time source. The default is the real clock. @@ -112,10 +117,13 @@ type Core struct { // watchGrants hands them to onEnded. endedQueue []audit.GrantEnded onEnded func(audit.GrantEnded) - grantWake chan struct{} // wakes watchGrants; it has room for one - decisions uint64 // counts the answers that counted - running bool // Run has started - stopped bool // Run has returned: no answer can come any more + // msgQueue holds the updates of the messages of scheduled grants that + // ended, until watchGrants makes them. + msgQueue []msgUpdate + grantWake chan struct{} // wakes watchGrants; it has room for one + decisions uint64 // counts the answers that counted + running bool // Run has started + stopped bool // Run has returned: no answer can come any more } // New returns a core that decides requests by the rules of cfg and asks diff --git a/internal/approval/approver.go b/internal/approval/approver.go index 343c462..30959ac 100644 --- a/internal/approval/approver.go +++ b/internal/approval/approver.go @@ -25,11 +25,19 @@ type Approver interface { // the buttons and shows how the request ended. p is the prompt that // Ask got. The core calls End once for each message that Ask sent. // An error does not change the decision; the core logs it. + // + // The core also calls End for the message of a scheduled grant, after + // the grant started and after it ended (DESIGN.md section 7). The + // first call has e.Revoke set: the message then keeps one Revoke + // button. The last call has e.GrantEnd set: the message then shows + // how and when the grant ended, and has no button. The core makes + // these calls one after the other, in this order. End(ctx context.Context, m MessageID, p Prompt, e Ending) error - // Notify sends a silent notice that a "never ask" rule allowed the - // request p without a tap (DESIGN.md section 12). The prompt has no - // buttons. ctx ends as for Ask. If Notify returns an error, the core + // Notify sends a silent notice that a "never ask" rule or a scheduled + // grant allowed the request p without a tap (DESIGN.md section 12), or + // that a scheduled grant ended with uses left (p.GrantID and + // p.UsesLeft are set; see Prompt). The prompt has no buttons. ctx ends as for Ask. If Notify returns an error, the core // denies the request, so a use without a tap is never unseen. // // If a limit of the approver itself stops the notice, Notify returns @@ -135,6 +143,10 @@ type Prompt struct { // RemoteUser or Secrets say what it covers. Uses int From, Until time.Time + // GrantID and UsesLeft are set only in the notice that a scheduled + // grant ended with uses left: the grant's ID and the uses it had left. + GrantID string + UsesLeft int // Via is set in a notice of a use that a scheduled grant allowed // (audit.ViaScheduled). It is empty in a notice of a "never ask" use. Via audit.Via @@ -171,6 +183,17 @@ type Ending struct { Cause audit.Cause // At is when the request ended. At time.Time + // Revoke is set when the request is a scheduled grant that has + // started: the message keeps one Revoke button (audit.AnswerRevoke) + // until the grant ends. + Revoke bool + // GrantEnd is set when the scheduled grant of the message has ended, + // and says why. UsesLeft is the number of uses left. At is when it + // ended. The message then has no button. + GrantEnd audit.EndCause + UsesLeft int + // Approved is when the person approved the grant, if GrantEnd is set. + Approved time.Time } // Answer is a button press that the approver passes to the core. @@ -178,7 +201,8 @@ type Answer struct { // RequestID is the request ID from the button. RequestID string // Action is the button: audit.AnswerApproveOnce, - // audit.AnswerApproveWindow or audit.AnswerDeny. + // audit.AnswerApproveWindow, audit.AnswerDeny or audit.AnswerRevoke. + // A Revoke names the request that made the scheduled grant. Action audit.Answer // Message is the message the button is on. Message MessageID diff --git a/internal/approval/decide.go b/internal/approval/decide.go index 4585c0c..c7df242 100644 --- a/internal/approval/decide.go +++ b/internal/approval/decide.go @@ -385,6 +385,11 @@ func (c *Core) ask(ctx context.Context, p *pending) *Result { } if p.msg != "" { c.end(ctx, p, prompt, res) + if res.plan != nil { + // The grant keeps the message, with a Revoke button, once it + // starts (startScheduled). + res.msg, res.prompt = p.msg, prompt + } } return res } @@ -520,6 +525,9 @@ func (c *Core) Answer(a Answer) error { if a.UserID <= 0 || a.UserID != c.cfg.Telegram.UserID { return ErrNotApprover } + if a.Action == audit.AnswerRevoke { + return c.revoke(a) + } p := c.waiting[a.RequestID] if p == nil { if q := c.ids[a.RequestID]; q != nil && q.state == stateAsking { @@ -639,6 +647,7 @@ func (c *Core) makeGrantID(p *pending) error { func (c *Core) approvePlan(p *pending, res *Result) { res.Subject.GrantID = p.plan.id g := *p.plan + g.reqID = p.id g.origin = res.Subject g.origin.Secrets = slices.Clone(res.Subject.Secrets) g.origin.Hosts = slices.Clone(res.Subject.Hosts) diff --git a/internal/approval/grants.go b/internal/approval/grants.go index 806320e..ec670bd 100644 --- a/internal/approval/grants.go +++ b/internal/approval/grants.go @@ -2,6 +2,7 @@ package approval import ( "context" + "log/slog" "slices" "time" @@ -43,8 +44,43 @@ type grant struct { // origin is the subject of the request that made a scheduled grant, // for its audit entries. origin audit.Subject + // reqID is the ID of the request that made a scheduled grant. The + // Revoke button carries it. + reqID string + // msg is the message of that request and prompt is the prompt of it. + // They are set when the grant starts, and are "" for a grant whose + // message the core does not know. + msg MessageID + prompt Prompt + // approved is when the person approved it. + approved time.Time } +// msgUpdate is a call to the approver about the message of a scheduled +// grant: End, and for a grant that ended with uses left also Notify. The +// calls wait in Core.msgQueue, so they run one after the other, in the +// order of the events. +type msgUpdate struct { + msg MessageID + prompt Prompt + ending Ending + // notice says that the approver also gets a notice that the grant + // ended with uses left. grantID is the grant's ID for it. + notice bool + grantID string + // tries counts the failed tries of an update that adds a Revoke button. + tries int +} + +// The tries of the update that adds a Revoke button: it can fail, such as +// by a timeout, and the grant may live for days. After a failed try, the +// next one comes after holdFirstWait, then twice as long, at most +// holdTries in all, and only while the grant lives (staleHold). +const ( + holdTries = 3 + holdFirstWait = 10 * time.Second +) + // scheduled reports whether g comes from nodkey preapprove. func (g *grant) scheduled() bool { return g.id != "" } @@ -85,12 +121,34 @@ func (c *Core) endWhere(cause func(*grant) audit.EndCause) []audit.GrantEnded { } if g.scheduled() { ended = append(ended, audit.GrantEnded{Subject: g.origin, Cause: why, UsesLeft: max(g.uses, 0)}) + c.queueMessage(g, why) } return true }) return ended } +// queueMessage queues the update of the message of g, which ended with +// cause why: no button, and how and when it ended. A grant that ended +// by time with uses left also queues a notice. Core.mu must be held. +func (c *Core) queueMessage(g *grant, why audit.EndCause) { + if g.msg == "" { + return + } + at := c.clock.Wall() + if why == audit.EndEnded { + at = g.end.wall + } + left := max(g.uses, 0) + c.msgQueue = append(c.msgQueue, msgUpdate{ + msg: g.msg, + prompt: g.prompt, + ending: Ending{Answer: audit.AnswerApproveOnce, At: at, GrantEnd: why, UsesLeft: left, Approved: g.approved}, + notice: why == audit.EndEnded && left > 0, grantID: g.id, + }) + c.wakeGrants() +} + // sweepGrants removes the grants that have ended. The entries of the // scheduled ones wait for the goroutine of watchGrants. Core.mu must be // held. @@ -141,6 +199,104 @@ func (c *Core) takeEnded() ([]audit.GrantEnded, func(audit.GrantEnded)) { return evs, c.onEnded } +// deliverMessages makes the queued calls about the messages of scheduled +// grants, one after the other, until ctx ends. What it did not make stays +// in the queue. A call that fails is logged; the audit log has the end of +// the grant anyway. +func (c *Core) deliverMessages(ctx context.Context) { + for ctx.Err() == nil { + // The entries of the audit log first: a slow Telegram must not + // delay them. + c.deliverEnded() + c.mu.Lock() + if len(c.msgQueue) == 0 { + c.mu.Unlock() + return + } + u := c.msgQueue[0] + c.msgQueue = c.msgQueue[1:] + stale := u.ending.Revoke && c.staleHold(u) + c.mu.Unlock() + if stale { + continue + } + if !c.updateMessage(ctx, u) { + // The stop cut the call short: the next delivery makes it. + c.mu.Lock() + c.msgQueue = slices.Insert(c.msgQueue, 0, u) + c.mu.Unlock() + } + } +} + +// staleHold reports whether u, an update that adds a Revoke button, is +// not needed any more: its grant is gone or past its end, or the update +// of its end waits. Core.mu must be held. +func (c *Core) staleHold(u msgUpdate) bool { + now := c.now() + if !slices.ContainsFunc(c.grants, func(g *grant) bool { return g.msg == u.msg && now.before(g.end) }) { + return true + } + return slices.ContainsFunc(c.msgQueue, func(q msgUpdate) bool { return q.msg == u.msg && q.ending.GrantEnd != "" }) +} + +// retryHold queues the update u, which adds a Revoke button and failed, +// again after a wait, unless it has used its tries. It does not block: the +// wait is in a goroutine of its own, which ends when the core stops. +func (c *Core) retryHold(u msgUpdate) { + u.tries++ + if u.tries >= holdTries { + return + } + wait := holdFirstWait << (u.tries - 1) + t := c.clock.TimerAt(c.now().mono.Add(wait)) + go func() { + defer t.Stop() + select { + case <-t.C(): + case <-c.stopCtx.Done(): + return + } + c.mu.Lock() + c.msgQueue = append(c.msgQueue, u) + c.mu.Unlock() + c.wakeGrants() + }() +} + +// updateMessage makes the calls of u. It reports false when ctx ended +// before the message was updated. +func (c *Core) updateMessage(ctx context.Context, u msgUpdate) bool { + ectx, cancel := context.WithTimeout(ctx, endTimeout) + defer cancel() + if err := c.approver.End(ectx, u.msg, u.prompt, u.ending); err != nil { + if ctx.Err() != nil { + return false + } + c.logger.LogAttrs(ectx, slog.LevelWarn, "approval: cannot update the message of a scheduled grant", + slog.String("request_id", u.prompt.RequestID), slog.Any("error", err)) + if u.ending.Revoke { + c.retryHold(u) + } + } + if !u.notice { + return true + } + // The grant did not run, or ran late. The approver's own limit of + // notices applies; the limit per client does not, because the person + // did not ask for this notice (DESIGN.md section 8). + n := u.prompt + n.Expires, n.TimeLeft, n.Once, n.WindowMinutes = time.Time{}, 0, false, 0 + n.GrantID, n.UsesLeft = u.grantID, u.ending.UsesLeft + nctx, ncancel := context.WithTimeout(ctx, endTimeout) + defer ncancel() + if err := c.approver.Notify(nctx, n); err != nil { + c.logger.LogAttrs(nctx, slog.LevelWarn, "approval: cannot send the notice of a scheduled grant that was not used", + slog.String("request_id", n.RequestID), slog.Any("error", err)) + } + return true +} + // deliverEnded gives the queued entries to the function of OnGrantEnded. func (c *Core) deliverEnded() { evs, f := c.takeEnded() @@ -163,6 +319,7 @@ func (c *Core) watchGrants(ctx context.Context) { next, ok := c.nextEnd() c.mu.Unlock() c.deliverEnded() + c.deliverMessages(c.stopCtx) var timer <-chan time.Time var t Timer if ok { @@ -199,7 +356,8 @@ func (c *Core) nextEnd() (time.Time, bool) { // dropScheduled ends every scheduled grant with cause stopped, when the // core stops: grants live only in memory, so a restart would lose them -// anyway (DESIGN.md section 8). +// anyway (DESIGN.md section 8). It hands out the audit entries; the +// updates of the messages wait for FlushMessages. func (c *Core) dropScheduled() { c.mu.Lock() c.queueEnded(c.endWhere(func(g *grant) audit.EndCause { @@ -212,6 +370,42 @@ func (c *Core) dropScheduled() { c.deliverEnded() } +// FlushMessages makes the updates of the messages of scheduled grants that +// are still waiting, such as those of the grants that a stop dropped. The +// broker calls it after Run returned and after it denies every request, +// because Telegram can be slow: the calls take at most +// stopMessagesTimeout together, so a stop never waits for a Telegram that +// does not answer. +func (c *Core) FlushMessages() { + ctx, cancel := context.WithTimeout(context.Background(), stopMessagesTimeout) + defer cancel() + c.deliverMessages(ctx) +} + +// revoke ends the scheduled grant of the request a.RequestID, for the +// Revoke button. It takes rights away only, but it passes the same checks +// as every answer: the caller checked the user; here the grant must exist +// and the message must be its own. A grant that has ended is unknown. +func (c *Core) revoke(a Answer) error { + now := c.now() + c.sweepGrants(now) + i := slices.IndexFunc(c.grants, func(g *grant) bool { return g.scheduled() && g.reqID == a.RequestID }) + if i < 0 { + return ErrUnknownRequest + } + g := c.grants[i] + if g.msg == "" || a.Message != g.msg { + return ErrWrongMessage + } + c.queueEnded(c.endWhere(func(x *grant) audit.EndCause { + if x == g { + return audit.EndRevoked + } + return "" + })) + return nil +} + // CancelGrant ends the scheduled grant id of client at the client's // request, without a tap. It returns the audit entry, which the caller // writes. It reports false for an ID that no grant of client has, and the @@ -341,5 +535,10 @@ func (c *Core) startScheduled(r *Result, now moment) { } c.grants = append(c.grants, g) r.live = true + if r.msg != "" { + g.msg, g.prompt, g.approved = r.msg, r.prompt, r.at + c.msgQueue = append(c.msgQueue, msgUpdate{msg: g.msg, prompt: g.prompt, + ending: Ending{Answer: audit.AnswerApproveOnce, At: r.at, Revoke: true}}) + } c.wakeGrants() } diff --git a/internal/approval/preapprove_test.go b/internal/approval/preapprove_test.go index 1db55dd..da05484 100644 --- a/internal/approval/preapprove_test.go +++ b/internal/approval/preapprove_test.go @@ -73,6 +73,7 @@ func (e *env) grant(r *approval.Request, n int) *approval.Result { if !res.GrantStarted() { e.t.Fatal("the grant did not start") } + synctest.Wait() // the message gets its Revoke button return res } diff --git a/internal/approval/request.go b/internal/approval/request.go index 6b4d942..76c8fed 100644 --- a/internal/approval/request.go +++ b/internal/approval/request.go @@ -96,8 +96,10 @@ type Result struct { GrantFrom, GrantUntil time.Time core *Core - at time.Time // when the request was decided, on the wall clock - plan *grant // the scheduled grant of an approved KindPreapprove request + at time.Time // when the request was decided, on the wall clock + plan *grant // the scheduled grant of an approved KindPreapprove request + msg MessageID // with plan: the message of the request, and its prompt + prompt Prompt usedID string // the scheduled grant that allowed the request ended []audit.GrantEnded // entries of grants that this request ended vias []audit.Via // what allowed each secret or key without a tap diff --git a/internal/approval/revoke_test.go b/internal/approval/revoke_test.go new file mode 100644 index 0000000..6d9a730 --- /dev/null +++ b/internal/approval/revoke_test.go @@ -0,0 +1,464 @@ +package approval_test + +// Tests of the Revoke button, of the update of the message of a scheduled +// grant, and of the notice of a grant that was not used (DESIGN.md +// sections 7 and 8). + +import ( + "context" + "errors" + "testing" + "testing/synctest" + "time" + + "github.com/code6x/nodkey/internal/approval" + "github.com/code6x/nodkey/internal/approval/approvaltest" + "github.com/code6x/nodkey/internal/audit" + "github.com/code6x/nodkey/internal/config" +) + +// updates returns the updates of the message n (counted from 1) after the +// core made them all. +func (e *env) updates(n int) []approvaltest.Ended { + e.t.Helper() + synctest.Wait() + msg := e.fake.Asked()[n-1].Message + var out []approvaltest.Ended + for _, u := range e.fake.Ended() { + if u.Message == msg { + out = append(out, u) + } + } + return out +} + +// lastUpdate checks that the message n has had count updates, and returns +// the last. +func (e *env) lastUpdate(n, count int) approval.Ending { + e.t.Helper() + ups := e.updates(n) + if len(ups) != count { + e.t.Fatalf("message %d had %d updates, want %d", n, len(ups), count) + } + return ups[len(ups)-1].Ending +} + +// TestRevokeEndsGrant: after Approve the message keeps a Revoke button. A +// Revoke tap ends the grant at once with cause revoked, writes its entry, +// updates the message, and a later use needs a tap. It sends no notice. +func TestRevokeEndsGrant(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e := preEnv(t) + defer e.stop() + var rec endedRecorder + e.core.OnGrantEnded(rec.add) + g := e.grant(preSecrets("laptop", 3, 0, time.Hour, "github-token"), 1) + // The approval shows first; then the message keeps one Revoke button. + if hold := e.lastUpdate(1, 2); !hold.Revoke || hold.GrantEnd != "" { + t.Fatalf("second update %+v; want Revoke", hold) + } + e.uses(secretsReq("laptop", "github-token"), g.GrantID) + e.tap(1, audit.AnswerRevoke) + synctest.Wait() + got := rec.all() + if len(got) != 1 || got[0].Cause != audit.EndRevoked || got[0].UsesLeft != 2 || got[0].GrantID != g.GrantID || + got[0].RequestID != g.RequestID { + t.Fatalf("ended entries %+v", got) + } + end := e.lastUpdate(1, 3) + if !end.Approved.Equal(noon) { + t.Errorf("approved at %v, want %v", end.Approved, noon) + } + if end.Revoke || end.GrantEnd != audit.EndRevoked || end.UsesLeft != 2 { + t.Errorf("last update %+v; want the end by revoke, no button", end) + } + e.needsMessage(secretsReq("laptop", "github-token"), 2) + for _, n := range e.fake.Notices() { + if n.GrantID != "" { + t.Errorf("a revoke sent a not used notice: %+v", n) + } + } + }) +} + +// TestRevokeBeforeStart: a grant that has not started yet can be revoked. +func TestRevokeBeforeStart(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e := preEnv(t) + defer e.stop() + e.grant(preSecrets("laptop", 1, time.Hour, 2*time.Hour, "github-token"), 1) + e.tap(1, audit.AnswerRevoke) + e.clock.Advance(time.Hour) + e.needsMessage(secretsReq("laptop", "github-token"), 2) + }) +} + +// TestRevokeRefused: a Revoke that is not the approver's, not on the +// message of the grant, for a request that made no grant, or after the +// grant ended, changes nothing. +func TestRevokeRefused(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e := preEnv(t) + defer e.stop() + g := e.grant(preSecrets("laptop", 2, 0, time.Hour, "github-token"), 1) + for _, tc := range []struct { + name string + change func(*approval.Answer) + want error + }{ + {"other user", func(a *approval.Answer) { a.Action = audit.AnswerRevoke; a.UserID = approverID + 1 }, approval.ErrNotApprover}, + {"no user", func(a *approval.Answer) { a.Action = audit.AnswerRevoke; a.UserID = 0 }, approval.ErrNotApprover}, + {"other message", func(a *approval.Answer) { a.Action = audit.AnswerRevoke; a.Message = "message-99" }, approval.ErrWrongMessage}, + {"no message", func(a *approval.Answer) { a.Action = audit.AnswerRevoke; a.Message = "" }, approval.ErrWrongMessage}, + {"unknown request", func(a *approval.Answer) { a.Action = audit.AnswerRevoke; a.RequestID = "AAAAAAAAAAAAAAAAAAAAAA" }, approval.ErrUnknownRequest}, + {"the grant ID is not the request ID", func(a *approval.Answer) { a.Action = audit.AnswerRevoke; a.RequestID = g.GrantID }, approval.ErrUnknownRequest}, + } { + if err := e.answer(1, tc.change); !errors.Is(err, tc.want) { + t.Errorf("%s: error %v, want %v", tc.name, err, tc.want) + } + } + e.uses(secretsReq("laptop", "github-token"), g.GrantID) + e.uses(secretsReq("laptop", "github-token"), g.GrantID) // both uses: the grant stayed + + // A Revoke after the grant is used up, ended or revoked is late. + revoke := func(a *approval.Answer) { a.Action = audit.AnswerRevoke } + if err := e.answer(1, revoke); !errors.Is(err, approval.ErrUnknownRequest) { + t.Errorf("revoke after used up: %v", err) + } + e.grant(preSecrets("laptop", 2, 0, time.Hour, "npm-token"), 2) + e.grant(preSecrets("laptop", 2, 0, 2*time.Hour, "db-password"), 3) + e.clock.Advance(time.Hour) + if err := e.answer(2, revoke); !errors.Is(err, approval.ErrUnknownRequest) { + t.Errorf("revoke after the end: %v", err) + } + e.tap(3, audit.AnswerRevoke) + if err := e.answer(3, revoke); !errors.Is(err, approval.ErrUnknownRequest) { + t.Errorf("second revoke: %v", err) + } + if got := len(e.updates(3)); got != 3 { + t.Errorf("%d updates of message 3, want 3: a refused revoke changes nothing", got) + } + }) +} + +// TestRevokeNeedsTheMessageOfTheGrant: the tap of a request that still +// waits for its first answer is not a Revoke. +func TestRevokeWaitingRequest(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e := preEnv(t) + defer e.stop() + ch := e.decide(preSecrets("laptop", 1, 0, time.Hour, "github-token")) + mustWait(t, ch) + e.asked(1) + err := e.answer(1, func(a *approval.Answer) { a.Action = audit.AnswerRevoke }) + if !errors.Is(err, approval.ErrUnknownRequest) { + t.Errorf("revoke of a waiting request: %v", err) + } + e.tap(1, audit.AnswerApproveOnce) // it still waits + checkAllowed(t, mustResult(t, ch)) + }) +} + +// TestGrantEndChangesMessage: every cause of the end removes the button +// and says how and when the grant ended, in the last update of the message. +func TestGrantEndChangesMessage(t *testing.T) { + use := func(e *env, g *approval.Result) { e.uses(secretsReq("laptop", "github-token"), g.GrantID) } + for _, tc := range []struct { + cause audit.EndCause + left int + do func(e *env, g *approval.Result) + }{ + {audit.EndUsedUp, 0, func(e *env, g *approval.Result) { use(e, g) }}, + {audit.EndEnded, 1, func(e *env, g *approval.Result) { e.clock.Advance(time.Hour) }}, + {audit.EndCancelled, 1, func(e *env, g *approval.Result) { e.core.CancelGrant("laptop", g.GrantID) }}, + {audit.EndRevoked, 1, func(e *env, g *approval.Result) { e.tap(1, audit.AnswerRevoke) }}, + {audit.EndDenied, 1, func(e *env, g *approval.Result) { + ch := e.decide(secretsReq("laptop", "github-token", "npm-token")) + mustWait(e.t, ch) + e.tap(2, audit.AnswerDeny) + mustResult(e.t, ch) + }}, + {audit.EndStopped, 1, func(e *env, g *approval.Result) { e.stop(); e.core.FlushMessages() }}, + } { + t.Run(string(tc.cause), func(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e := preEnv(t) + defer e.stop() + g := e.grant(preSecrets("laptop", 1, 0, time.Hour, "github-token"), 1) + tc.do(e, g) + end := e.lastUpdate(1, 3) + if end.Revoke || end.GrantEnd != tc.cause || end.UsesLeft != tc.left || end.At.IsZero() { + t.Errorf("last update %+v; want the end by %s, %d left, no button", end, tc.cause, tc.left) + } + }) + }) + } +} + +// TestNotUsedNotice: a grant that reaches its end with uses left sends one +// silent notice. It is not sent when the uses ran out, or on cancel, +// revoke, a later Deny or a stop. The notice does not count for the limit +// per client. +func TestNotUsedNotice(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e := preEnv(t) + defer e.stop() + notUsed := func() []approval.Prompt { + synctest.Wait() + var out []approval.Prompt + for _, n := range e.fake.Notices() { + if n.GrantID != "" { + out = append(out, n) + } + } + return out + } + // Used up, cancelled, revoked, denied: no notice. + a := e.grant(preSecrets("laptop", 1, 0, time.Hour, "github-token"), 1) + e.uses(secretsReq("laptop", "github-token"), a.GrantID) + b := e.grant(preSecrets("laptop", 1, 0, time.Hour, "github-token"), 2) + e.core.CancelGrant("laptop", b.GrantID) + e.grant(preSecrets("laptop", 1, 0, time.Hour, "github-token"), 3) + e.tap(3, audit.AnswerRevoke) + e.grant(preSecrets("laptop", 1, 0, time.Hour, "github-token"), 4) + ch := e.decide(secretsReq("laptop", "github-token", "npm-token")) + mustWait(t, ch) + e.tap(5, audit.AnswerDeny) + mustResult(t, ch) + e.clock.Advance(approval.DenyPause) + if got := notUsed(); len(got) != 0 { + t.Fatalf("%d not used notices without a reason: %+v", len(got), got) + } + // The end with 2 of 3 uses left. + c := e.grant(preSecrets("laptop", 3, 0, time.Hour, "github-token", "npm-token"), 6) + e.uses(secretsReq("laptop", "github-token", "npm-token"), c.GrantID) + e.clock.Advance(time.Hour) + got := notUsed() + if len(got) != 1 { + t.Fatalf("%d not used notices, want 1", len(got)) + } + n := got[0] + if n.GrantID != c.GrantID || n.UsesLeft != 2 || n.Uses != 3 || n.Client != "laptop" || n.Once || n.WindowMinutes != 0 || + !n.Expires.IsZero() || n.Kind != audit.KindPreapprove || len(n.Secrets) != 2 || n.Via != "" { + t.Errorf("notice %+v", n) + } + e.clock.Advance(time.Hour) + if got := notUsed(); len(got) != 1 { + t.Errorf("%d notices after more time, want 1", len(got)) + } + }) +} + +// TestNotUsedNoticeLimits: the notice does not count toward the limit of 6 +// per client, and one that the approver cannot send is only logged: the +// message is updated anyway. +func TestNotUsedNoticeLimits(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e := preEnv(t) + defer e.stop() + var off time.Duration + for i := range 8 { + e.grant(preSecrets("laptop", 1, off, off+time.Hour, "github-token"), i+1) + off += time.Hour + e.clock.Advance(time.Hour) + } + // A client with many not used notices still has all its uses. + g := e.grant(preSecrets("laptop", 1, off, off+time.Hour, "github-token"), 9) + e.uses(secretsReq("laptop", "github-token"), g.GrantID) + + e.fake.FailNotify(errors.New("telegram is down")) + e.grant(preSecrets("laptop", 2, off, off+time.Hour, "npm-token"), 10) + e.clock.Advance(time.Hour) + synctest.Wait() + end := e.lastUpdate(10, 3) + if end.GrantEnd != audit.EndEnded || end.UsesLeft != 2 { + t.Errorf("last update %+v", end) + } + }) +} + +// blockingApprover is a fake whose End waits until its context ends, as a +// Telegram that does not answer. +type blockingApprover struct { + *approvaltest.Approver + block chan struct{} // closed: End blocks + revokeBlock bool // only an End that adds a Revoke button blocks +} + +func (b *blockingApprover) End(ctx context.Context, m approval.MessageID, p approval.Prompt, e approval.Ending) error { + select { + case <-b.block: + if b.revokeBlock && !e.Revoke { + return b.Approver.End(ctx, m, p, e) + } + <-ctx.Done() + return ctx.Err() + default: + return b.Approver.End(ctx, m, p, e) + } +} + +// TestStopDoesNotHangOnMessages: when Telegram does not answer, the stop +// of the core waits at most a few seconds for the updates of the messages. +func TestStopDoesNotHangOnMessages(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + cfg, _ := testConfig(t) + cfg.Preapprove = config.DefaultPreapprove() + s := cfg.Secrets["github-token"] + s.Rule.Preapprove = true + cfg.Secrets["github-token"] = s + clock := approvaltest.NewClock(noon) + b := &blockingApprover{Approver: approvaltest.NewApprover(approverID), block: make(chan struct{})} + core, err := approval.New(cfg, b, approval.Options{Clock: clock}) + if err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan error, 1) + go func() { done <- core.Run(ctx) }() + synctest.Wait() + var rec endedRecorder + core.OnGrantEnded(rec.add) + ch := make(chan *approval.Result, 1) + go func() { ch <- core.Decide(context.Background(), preSecrets("laptop", 1, 0, time.Hour, "github-token")) }() + synctest.Wait() + asked := b.Asked() + if len(asked) != 1 { + t.Fatalf("asked %d messages", len(asked)) + } + if err := b.Tap(asked[0].Prompt.RequestID, audit.AnswerApproveOnce); err != nil { + t.Fatal(err) + } + synctest.Wait() + res := <-ch + res.StartGrants() + synctest.Wait() + close(b.block) // Telegram stops answering + start := time.Now() + cancel() + <-done + // Run itself does not wait for Telegram: the broker denies every + // request before it flushes the messages. + if d := time.Since(start); d != 0 { + t.Errorf("Run took %v after the stop", d) + } + core.FlushMessages() + if d := time.Since(start); d > 10*time.Second { + t.Errorf("the stop took %v", d) + } + if got := rec.all(); len(got) != 1 || got[0].Cause != audit.EndStopped { + t.Errorf("ended entries %+v", got) + } + }) +} + +// TestStopSkipsRevokeEdit: at a stop, the edit that adds a Revoke button +// is cut short, but it does not take the time of the flush: its grant is +// gone, so the edit of the end goes out. +func TestStopSkipsRevokeEdit(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + cfg, _ := testConfig(t) + cfg.Preapprove = config.DefaultPreapprove() + s := cfg.Secrets["github-token"] + s.Rule.Preapprove = true + cfg.Secrets["github-token"] = s + b := &blockingApprover{Approver: approvaltest.NewApprover(approverID), block: make(chan struct{}), revokeBlock: true} + core, err := approval.New(cfg, b, approval.Options{Clock: approvaltest.NewClock(noon)}) + if err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan error, 1) + go func() { done <- core.Run(ctx) }() + synctest.Wait() + ch := make(chan *approval.Result, 1) + go func() { ch <- core.Decide(context.Background(), preSecrets("laptop", 1, 0, time.Hour, "github-token")) }() + synctest.Wait() + if err := b.Tap(b.Asked()[0].Prompt.RequestID, audit.AnswerApproveOnce); err != nil { + t.Fatal(err) + } + synctest.Wait() + close(b.block) // the edit that adds the button now hangs + (<-ch).StartGrants() + synctest.Wait() + cancel() + <-done + start := time.Now() + core.FlushMessages() + if d := time.Since(start); d != 0 { + t.Errorf("the flush took %v", d) + } + var last approval.Ending + for _, u := range b.Ended() { + last = u.Ending + } + if last.GrantEnd != audit.EndStopped || last.Revoke { + t.Errorf("last update %+v; want the end by a stop", last) + } + }) +} + +// holdUpdates counts the updates of the message n that add a Revoke button. +func (e *env) holdUpdates(n int) int { + count := 0 + for _, u := range e.updates(n) { + if u.Ending.Revoke { + count++ + } + } + return count +} + +// TestRevokeEditRetried: an edit that adds the Revoke button and fails is +// tried again after a wait, while the grant lives. +func TestRevokeEditRetried(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e := preEnv(t) + defer e.stop() + e.fake.FailEnd(errors.New("timeout")) + e.grant(preSecrets("laptop", 1, 0, time.Hour, "github-token"), 1) + if got := e.holdUpdates(1); got != 1 { + t.Fatalf("%d tries, want 1", got) + } + e.fake.FailEnd(nil) + e.clock.Advance(10 * time.Second) + synctest.Wait() + if got := e.holdUpdates(1); got != 2 { + t.Errorf("%d tries after the wait, want 2", got) + } + e.clock.Advance(time.Minute) + synctest.Wait() + if got := e.holdUpdates(1); got != 2 { + t.Errorf("%d tries; a good edit must not be repeated", got) + } + }) +} + +// TestRevokeEditRetriesAreBounded: a failing edit is tried at most +// holdTries times, and no more once the grant has ended. +func TestRevokeEditRetriesAreBounded(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + e := preEnv(t) + defer e.stop() + e.fake.FailEnd(errors.New("timeout")) + e.grant(preSecrets("laptop", 1, 0, time.Hour, "github-token"), 1) + for range 10 { + e.clock.Advance(time.Minute) + synctest.Wait() + } + if got := e.holdUpdates(1); got != 3 { + t.Errorf("%d tries, want 3", got) + } + // A grant that ended is not given a button. + g := e.grant(preSecrets("laptop", 1, 10*time.Minute, time.Hour, "npm-token"), 2) + if got := e.holdUpdates(2); got != 1 { + t.Fatalf("%d tries, want 1", got) + } + e.core.CancelGrant("laptop", g.GrantID) + e.clock.Advance(time.Minute) + synctest.Wait() + if got := e.holdUpdates(2); got != 1 { + t.Errorf("%d tries after the grant ended, want 1", got) + } + }) +} diff --git a/internal/audit/event.go b/internal/audit/event.go index 4a82aaf..136de49 100644 --- a/internal/audit/event.go +++ b/internal/audit/event.go @@ -48,6 +48,10 @@ const ( AnswerApproveOnce Answer = "approve_once" AnswerApproveWindow Answer = "approve_window" AnswerDeny Answer = "deny" + // AnswerRevoke is the Revoke button of a scheduled grant. It never + // has a decision entry: its result is a grant_ended entry with cause + // revoked. + AnswerRevoke Answer = "revoke" ) // Via says what allowed a request without a tap. @@ -83,6 +87,7 @@ const ( EndCancelled EndCause = "cancelled" // the client cancelled it EndDenied EndCause = "denied" // the person denied a later request for it EndStopped EndCause = "stopped" // the broker stopped + EndRevoked EndCause = "revoked" // the person pressed Revoke ) // TapCause says why a button press did not count (DESIGN.md section 7). @@ -454,7 +459,7 @@ func (e GrantEnded) validate() error { return errors.New("it needs a preapprove request, a grant_id and a number of uses left") } switch e.Cause { - case EndUsedUp, EndEnded, EndCancelled, EndDenied, EndStopped: + case EndUsedUp, EndEnded, EndCancelled, EndDenied, EndStopped, EndRevoked: return nil } return errors.New("cause is not known") diff --git a/internal/broker/broker.go b/internal/broker/broker.go index 02ad1e2..8dca644 100644 --- a/internal/broker/broker.go +++ b/internal/broker/broker.go @@ -398,13 +398,21 @@ func (s *Service) Run(ctx context.Context, core *approval.Core, alert AlertFunc, defer s.coreStopped.Store(true) // A scheduled grant that ends by itself has no request to carry its // entry (DESIGN.md section 8). + var dropped droppedGrants core.OnGrantEnded(func(ev audit.GrantEnded) { if _, err := s.append(ev); err != nil { s.log.Warn("broker: cannot write the grant_ended entry of a scheduled grant") } + dropped.note(ev) }) err := core.Run(ctx) s.coreStopped.Store(true) // first: deny every request from now on + // The grants are dropped by now. Their messages are updated, and one + // notice lists them, each with a short time limit. This comes after the + // line above, so a slow Telegram never leaves the broker open to + // requests that a grant would allow. + core.FlushMessages() + s.sendStopNotice(ctx, notice, dropped.list()) switch { case errors.Is(err, telegram.ErrPollConflict): s.log.Error("broker: another program polls with the bot token, or a webhook is set, so the token may be " + diff --git a/internal/broker/preapprove.go b/internal/broker/preapprove.go index 816d055..5432d0a 100644 --- a/internal/broker/preapprove.go +++ b/internal/broker/preapprove.go @@ -3,9 +3,12 @@ package broker import ( "context" "errors" + "fmt" "log/slog" "net/http" "slices" + "strings" + "sync" "time" "github.com/code6x/nodkey/internal/api" @@ -179,3 +182,66 @@ func (rq *request) cancelGrant(r *http.Request) { } rq.answerJSON(api.PreapproveCancelAnswer{ID: req.ID}) } + +// stopNoticeTimeout bounds the stop notice, so a stop never waits long for +// a Telegram that does not answer. It is a variable only so that a test +// can make it short. +var stopNoticeTimeout = 5 * time.Second + +// maxStopNoticeGrants is the most grants that the stop notice lists. The +// rest is counted. +const maxStopNoticeGrants = 20 + +// droppedGrants collects the scheduled grants that a stop dropped. +type droppedGrants struct { + mu sync.Mutex + evs []audit.GrantEnded +} + +// note keeps ev if it is the end of a grant by the stop. +func (d *droppedGrants) note(ev audit.GrantEnded) { + if ev.Cause != audit.EndStopped { + return + } + d.mu.Lock() + defer d.mu.Unlock() + d.evs = append(d.evs, ev) +} + +// list returns the notes. +func (d *droppedGrants) list() []audit.GrantEnded { + d.mu.Lock() + defer d.mu.Unlock() + return slices.Clone(d.evs) +} + +// sendStopNotice sends one notice that lists the scheduled grants that the +// stop dropped: ID, client, secrets or key, and uses left (DESIGN.md +// section 8). It sends nothing when there are none. It makes one try, with +// stopNoticeTimeout; a notice that does not go out is logged, and the +// grant_ended entries are in the audit log. +func (s *Service) sendStopNotice(ctx context.Context, notice NoticeFunc, dropped []audit.GrantEnded) { + if notice == nil || len(dropped) == 0 { + return + } + defer s.recoverGoroutine("sending the stop notice") + var b strings.Builder + fmt.Fprintf(&b, "The broker stopped. It dropped %d scheduled grant(s); jobs that need them will ask for a tap:", len(dropped)) + for i, ev := range dropped { + if i == maxStopNoticeGrants { + fmt.Fprintf(&b, "\n... and %d more", len(dropped)-i) + break + } + what := ev.Key + if len(ev.Secrets) > 0 { + what = strings.Join(ev.Secrets, ", ") + } + fmt.Fprintf(&b, "\n%s: %s, %s, %d use(s) left", ev.GrantID, ev.Client, what, ev.UsesLeft) + } + nctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), stopNoticeTimeout) + defer cancel() + if err := notice(nctx, b.String()); err != nil { + s.log.Warn("broker: cannot send the stop notice about the dropped scheduled grants", + slog.String("error", errorText(err))) + } +} diff --git a/internal/broker/revoke_test.go b/internal/broker/revoke_test.go new file mode 100644 index 0000000..cf8ef06 --- /dev/null +++ b/internal/broker/revoke_test.go @@ -0,0 +1,30 @@ +package broker + +import ( + "testing" + + "github.com/code6x/nodkey/internal/audit" +) + +// TestPreapproveRevoke: a Revoke tap ends the grant and writes one +// grant_ended entry with cause revoked. A later tap writes nothing. +func TestPreapproveRevoke(t *testing.T) { + e := newEnv(t, envOptions{config: preapproveOn}) + c := e.httpClient("laptop") + g := e.preapprove(c, preReq(2, "github-token"), 1) + before := len(e.audit.all()) + id := e.waitAsked(1)[0].Prompt.RequestID + e.tap(id, audit.AnswerRevoke) + events := e.audit.waitFor(t, before+1)[before:] + mustEvents(t, events, "grant_ended:revoked") + end := events[0].(audit.GrantEnded) + if end.GrantID != g.ID || end.UsesLeft != 2 || end.RequestID != id { + t.Errorf("entry %+v", end) + } + if err := e.approver.Tap(id, audit.AnswerRevoke); err == nil { + t.Error("a second Revoke counted") + } + if got := len(e.audit.all()); got != before+1 { + t.Errorf("%d entries, want %d", got, before+1) + } +} diff --git a/internal/broker/stopnotice_test.go b/internal/broker/stopnotice_test.go new file mode 100644 index 0000000..1da78de --- /dev/null +++ b/internal/broker/stopnotice_test.go @@ -0,0 +1,127 @@ +package broker + +import ( + "net/http" + "strings" + "testing" + "time" + + "github.com/code6x/nodkey/internal/api" + "github.com/code6x/nodkey/internal/audit" +) + +// Tests of the stop notice: one notice that lists the scheduled grants that +// a stop dropped (DESIGN.md sections 8 and 15). + +// stopAndWait stops Run, as nodkey broker does, and waits until it returned. +func (e *env) stopAndWait() { + e.t.Helper() + e.stopRun() + select { + case err := <-e.runErr: + _ = err + e.runErr <- nil // the cleanup waits for it + case <-time.After(waitLimit): + e.t.Fatal("Run did not return") + } +} + +// TestStopNoticeListsGrants: a stop sends one notice with the ID, client, +// secret and uses left of each grant that it dropped, and not of one that +// ended before. +func TestStopNoticeListsGrants(t *testing.T) { + e := newEnv(t, envOptions{config: preapproveOn}) + c := e.httpClient("laptop") + a := e.preapprove(c, preReq(3, "github-token"), 1) + b := e.preapprove(c, preReq(2, "npm-token"), 2) + gone := e.preapprove(c, preReq(1, "github-token"), 3) + if _, ok := e.core.CancelGrant("laptop", gone.ID); !ok { + t.Fatal("the grant was not there") + } + e.stopAndWait() + sent := e.notices.wentOut() + if len(sent) != 1 { + t.Fatalf("%d notices, want 1: %q", len(sent), sent) + } + msg := sent[0] + for _, want := range []string{ + "dropped 2 scheduled grant(s)", + a.ID + ": laptop, github-token, 3 use(s) left", + b.ID + ": laptop, npm-token, 2 use(s) left", + } { + if !strings.Contains(msg, want) { + t.Errorf("notice %q does not hold %q", msg, want) + } + } + if strings.Contains(msg, gone.ID) { + t.Errorf("notice %q lists a grant that ended before the stop", msg) + } +} + +// TestStopNoticeNoGrants: a stop with no grant sends no notice. +func TestStopNoticeNoGrants(t *testing.T) { + e := newEnv(t, envOptions{config: preapproveOn}) + c := e.httpClient("laptop") + g := e.preapprove(c, preReq(1, "github-token"), 1) + e.clock.Advance(2 * time.Hour) // the grant ends by itself + e.audit.waitFor(t, 3) + _ = g + e.stopAndWait() + if tries, _ := e.notices.tried(); len(tries) != 0 { + t.Errorf("tries %q, want none", tries) + } +} + +// TestStopNoticeTelegramSilent: when Telegram does not answer, the stop +// waits only as long as the time limit of the notice, and logs it. +func TestStopNoticeTelegramSilent(t *testing.T) { + old := stopNoticeTimeout + stopNoticeTimeout = 100 * time.Millisecond + t.Cleanup(func() { stopNoticeTimeout = old }) + e := newEnv(t, envOptions{config: preapproveOn}) + c := e.httpClient("laptop") + e.preapprove(c, preReq(1, "github-token"), 1) + e.notices.set(func(n *noticeLog) { n.gate = make(chan struct{}) }) // never opens + start := time.Now() + e.stopAndWait() + if d := time.Since(start); d > 5*time.Second { + t.Errorf("the stop took %v", d) + } + tries, _ := e.notices.tried() + if len(tries) != 1 || len(e.notices.wentOut()) != 0 { + t.Errorf("tries %q, sent %q", tries, e.notices.wentOut()) + } + if !strings.Contains(e.logs.String(), "cannot send the stop notice") { + t.Error("the broker did not log the notice that did not go out") + } +} + +// TestStopDeniesBeforeTheNoticeGoesOut: while the stop notice waits for +// Telegram, the broker already denies every request, also one that a grant +// would allow. +func TestStopDeniesBeforeTheNoticeGoesOut(t *testing.T) { + old := stopNoticeTimeout + stopNoticeTimeout = 3 * time.Second + t.Cleanup(func() { stopNoticeTimeout = old }) + e := newEnv(t, envOptions{config: preapproveOn}) + c := e.httpClient("laptop") + e.preapprove(c, preReq(1, "github-token"), 1) + // A grant of "Approve 5 min" for another secret. + p := e.start(c, api.PathSecrets, secretsRequest("NPM_TOKEN", "npm-token")) + e.tap(e.waitAsked(2)[1].Prompt.RequestID, audit.AnswerApproveWindow) + mustStatus(t, p.wait(t), http.StatusOK) + e.notices.set(func(n *noticeLog) { n.gate = make(chan struct{}) }) // never opens + e.stopRun() + deadline := time.Now().Add(waitLimit) + for tries, _ := e.notices.tried(); len(tries) == 0; tries, _ = e.notices.tried() { + if time.Now().After(deadline) { + t.Fatal("the stop notice was not tried") + } + time.Sleep(time.Millisecond) + } + res := e.post(t.Context(), c, api.PathSecrets, secretsRequest("NPM_TOKEN", "npm-token")) + if res.status == http.StatusOK { + t.Error("the grant allowed a request while the broker stopped") + } + e.stopAndWait() +} diff --git a/internal/telegram/core_test.go b/internal/telegram/core_test.go index 4d13651..af6f033 100644 --- a/internal/telegram/core_test.go +++ b/internal/telegram/core_test.go @@ -40,11 +40,12 @@ type coreEnv struct { // and backup-token (never, with a notice, for ci only). func coreConfig() *config.Broker { return &config.Broker{ - Telegram: testTelegramConfig(), - Clients: map[string]config.Client{"laptop": {Name: "laptop"}, "ci": {Name: "ci"}}, + Telegram: testTelegramConfig(), + Preapprove: config.DefaultPreapprove(), + Clients: map[string]config.Client{"laptop": {Name: "laptop"}, "ci": {Name: "ci"}}, Secrets: map[string]config.Secret{ "github-token": {Name: "github-token", Clients: []string{"laptop"}, - Rule: config.Rule{Ask: config.AskWindow, Window: 15 * time.Minute}}, + Rule: config.Rule{Ask: config.AskWindow, Window: 15 * time.Minute, Preapprove: true}}, "db-password": {Name: "db-password", Clients: []string{"laptop"}, Rule: config.Rule{Ask: config.AskAlways}}, "backup-token": {Name: "backup-token", Clients: []string{"ci"}, Rule: config.Rule{Ask: config.AskNever, Notify: true}}, diff --git a/internal/telegram/fake_test.go b/internal/telegram/fake_test.go index c9579e0..36e228a 100644 --- a/internal/telegram/fake_test.go +++ b/internal/telegram/fake_test.go @@ -267,7 +267,7 @@ func (f *fakeTelegram) ServeHTTP(w http.ResponseWriter, r *http.Request) { writeBody(w, http.StatusOK, `{"ok":true,"result":`+string(msg)+`}`) case "editMessageText", "editMessageReplyMarkup": msg, _ := json.Marshal(map[string]any{ - "message_id": call.num("message_id"), "date": 1, + "message_id": call.num("message_id"), "date": 1, "edit_date": 2, "chat": map[string]any{"id": call.num("chat_id"), "type": "private"}, "text": "the text", }) diff --git a/internal/telegram/poll.go b/internal/telegram/poll.go index a70bc00..2fbdfcb 100644 --- a/internal/telegram/poll.go +++ b/internal/telegram/poll.go @@ -41,6 +41,7 @@ var errPanic = errors.New("telegram: polling stopped after a panic; no answers c const ( tapCountedApprove = "Approved." tapCountedDeny = "Denied." + tapCountedRevoke = "Revoked." tapTooEarly = "Not ready yet. Tap again in a moment." tapEnded = "This request has ended." tapExpired = "This request has expired." @@ -239,6 +240,8 @@ func tapResult(action audit.Answer, err error) (string, audit.TapCause) { switch { case err == nil && action == audit.AnswerDeny: return tapCountedDeny, "" + case err == nil && action == audit.AnswerRevoke: + return tapCountedRevoke, "" case err == nil: return tapCountedApprove, "" case errors.Is(err, approval.ErrTooEarly): @@ -297,6 +300,37 @@ func (a *Approver) check(q *callbackQuery) (approval.Answer, audit.TapCause) { // it, so this is a late press. func (a *Approver) changed(ctx context.Context, q *callbackQuery, ans approval.Answer) (handled, deny bool) { sent, known := a.lookup(ans.Message) + if ans.Action == audit.AnswerRevoke { + // A Revoke only takes a right away, so a changed message is no + // reason to refuse it on the message of a grant (held), on one that + // ended, or on one that this approver does not know (too old). The + // other checks stay: the user, the chat, the data, and in the core + // the grant of the request ID on this very message. But a message + // whose request may still wait has no Revoke button: such a button + // was put there by someone else, as a changed message. + if known && sent.final == "" && !sent.held { + a.answerTap(ctx, q, tapChanged) + a.report(ctx, q, audit.TapEditedMessage) + a.alertChanged(ctx, q) + return true, false + } + return false, false + } + if known && sent.final == "" && sent.held { + // The request of this message is decided; End gave it a Revoke + // button. Only a Revoke can count on it. A button of its request is + // a late tap, such as the second tap of a double tap. A button of + // another request is not on this message by the broker's doing. + if ans.RequestID != sent.heldID { + a.answerTap(ctx, q, tapChanged) + a.report(ctx, q, audit.TapEditedMessage) + a.alertChanged(ctx, q) + return true, false + } + a.answerTap(ctx, q, tapEnded) + a.report(ctx, q, audit.TapUnknownRequest) + return true, false + } edited := q.Message.EditDate != 0 if known && sent.final == "" && !sameKeyboard(sent.keyboard, q.Message.ReplyMarkup) { edited = true @@ -465,6 +499,8 @@ func cutData(s string) string { // after the restart. func (a *Approver) ended(ctx context.Context, m *message) { chatID, msgID := m.Chat.ID, m.MessageID + a.editMu.Lock() + defer a.editMu.Unlock() sent, known := a.lookup(messageID(chatID, msgID)) switch { case known && sent.final == "": diff --git a/internal/telegram/revoke_test.go b/internal/telegram/revoke_test.go new file mode 100644 index 0000000..6004dea --- /dev/null +++ b/internal/telegram/revoke_test.go @@ -0,0 +1,440 @@ +package telegram + +import ( + "strings" + "testing" + "time" + + "github.com/code6x/nodkey/internal/approval" + "github.com/code6x/nodkey/internal/audit" + "github.com/code6x/nodkey/internal/claim" +) + +// Tests of the Revoke button of a scheduled grant, of the lines that tell +// how a grant ended, and of the notice of a grant that was not used +// (DESIGN.md sections 7 and 8). + +// TestRevokeData: the Revoke button carries the request ID, as the other +// buttons do, and only that. +func TestRevokeData(t *testing.T) { + p := preapprovePrompt() + kb := revokeKeyboard(p) + if len(kb.InlineKeyboard) != 1 || len(kb.InlineKeyboard[0]) != 1 || + kb.InlineKeyboard[0][0].Text != "Revoke" || kb.InlineKeyboard[0][0].CallbackData != dataRevoke+p.RequestID { + t.Fatalf("keyboard %+v", kb) + } + action, id, ok := parseData(kb.InlineKeyboard[0][0].CallbackData) + if !ok || action != audit.AnswerRevoke || id != p.RequestID { + t.Errorf("parseData = %q, %q, %v", action, id, ok) + } + for _, bad := range []string{"r:", "r:" + p.RequestID + "x", "R:" + p.RequestID, "x:" + p.RequestID, + "r:" + strings.Repeat("!", requestIDLen), "r:" + testID(3)[:21] + "=", ""} { + if _, _, ok := parseData(bad); ok { + t.Errorf("parseData(%q) was accepted", bad) + } + } +} + +// TestGrantEndLines: every end of a grant says how and when, in the last +// lines of the message, and the approval stays above it. +func TestGrantEndLines(t *testing.T) { + p := preapprovePrompt() + p.Uses = 3 + at := time.Date(2026, 10, 28, 3, 30, 0, 0, time.UTC) + for cause, want := range map[audit.EndCause]string{ + audit.EndUsedUp: "all 3 use(s) used", + audit.EndEnded: "ended at 2026-10-28 03:30:00 UTC with 2 of 3 use(s) left", + audit.EndCancelled: "cancelled by the client at 2026-10-28 03:30:00 UTC", + audit.EndDenied: "ended by a later Deny at 2026-10-28 03:30:00 UTC", + audit.EndRevoked: "revoked by you at 2026-10-28 03:30:00 UTC", + audit.EndStopped: "the broker stopped at 2026-10-28 03:30:00 UTC", + } { + approved := at.Add(-time.Hour) + tx := endingText(p, approval.Ending{Answer: audit.AnswerApproveOnce, At: at, Approved: approved, GrantEnd: cause, UsesLeft: 2}) + got := tx.plain.String() + if !strings.Contains(got, "\n๐Ÿ‘ approved by you at 02:30:00, for 3 use(s) until ") || !strings.Contains(got, want) || + strings.Contains(got, "expires") || !strings.HasPrefix(got, "๐Ÿ—“ Pre-approval") { + t.Errorf("%s: message\n%s\nwant %q", cause, got, want) + } + } +} + +// TestNotUsedNoticeText: the notice names the grant and the uses left, and +// the prompt checks refuse a notice that is not one. +func TestNotUsedNoticeText(t *testing.T) { + p := noticeOf(preapprovePrompt()) + p.Uses, p.GrantID, p.UsesLeft = 3, testID(9), 2 + tx, err := noticeText(p) + if err != nil { + t.Fatal(err) + } + got := tx.plain.String() + want := "๐Ÿ”” Pre-approval ยท request m4k2\nnot used: grant " + p.GrantID + " ended with 2 of 3 uses left\n" + + "โœ… machine: laptop\nโœ… secrets: backup-token\nโœ… uses: 3\n" + if !strings.HasPrefix(got, want) || strings.Contains(got, "[") || strings.Contains(got, "expires") { + t.Errorf("notice\n%s\nwant it to start with\n%s", got, want) + } + for name, change := range map[string]func(*approval.Prompt){ + "no grant ID": func(p *approval.Prompt) { p.GrantID = "" }, + "a bad grant ID": func(p *approval.Prompt) { p.GrantID = "short" }, + "no uses left": func(p *approval.Prompt) { p.UsesLeft = 0 }, + "more left than had": func(p *approval.Prompt) { p.UsesLeft = 4 }, + "a via": func(p *approval.Prompt) { p.Via = audit.ViaScheduled }, + "a button": func(p *approval.Prompt) { p.Once = true }, + } { + q := p + change(&q) + if _, err := noticeText(q); err == nil { + t.Errorf("%s: the notice was accepted", name) + } + } + // The ID of a grant is for a notice only. + m := preapprovePrompt() + m.GrantID = testID(9) + if _, _, err := promptMessage(m); err == nil { + t.Error("a message with a grant ID was accepted") + } + s := secretsPrompt() + s.UsesLeft = 1 + if _, _, err := promptMessage(s); err == nil { + t.Error("a secrets message with uses left was accepted") + } +} + +// grantEnv starts a core with a fake Telegram, asks for a scheduled grant +// of github-token with uses uses from now to an hour from now, taps +// Approve, and starts the grant. It returns the ID of the request, the +// number of its message, and the result. It waits for the Revoke button. +func (ce *coreEnv) grantEnv(t *testing.T, uses int) (string, int64, *approval.Result) { + t.Helper() + req := &approval.Request{ + Client: "laptop", Kind: audit.KindPreapprove, Secrets: []string{"github-token"}, Uses: uses, + NotBefore: noon, NotAfter: noon.Add(time.Hour), Claim: approval.Claim{Reason: "job nightly-backup"}, + } + n := len(ce.f.callsOf("sendMessage")) + 1 + ch := ce.decide(req) + id, msg := ce.asked(t, n) + ce.f.tap(goodTap(msg, dataOnce+id)) + res := waitResult(t, ch) + if !res.Allowed { + t.Fatalf("result %+v, want allowed", res) + } + edits := len(ce.edits(msg)) + res.StartGrants() + if !res.GrantStarted() { + t.Fatal("the grant did not start") + } + ce.f.waitFor("the Revoke button", func() bool { + return len(ce.f.callsOfLocked("editMessageText")) > edits && strings.Contains(string(ce.f.messages[msg].markup), dataRevoke+id) + }) + return id, msg, res +} + +// TestCoreRevoke: the message of an approved scheduled grant keeps one +// Revoke button. A tap on it ends the grant; the message shows that and +// has no button; a second tap is late; and a use needs a tap again. +func TestCoreRevoke(t *testing.T) { + ce := newCoreEnv(t, newFake(t)) + var ended []audit.GrantEnded + endedCh := make(chan audit.GrantEnded, 4) + ce.core.OnGrantEnded(func(ev audit.GrantEnded) { endedCh <- ev }) + id, msg, res := ce.grantEnv(t, 2) + edits := ce.edits(msg) + if len(edits) != 2 || !strings.Contains(edits[1], "๐Ÿ‘ approved by you") || strings.Contains(edits[1], "revoked") { + t.Fatalf("edits %q", edits) + } + if got := ce.f.messages[msg].markup; string(got) != `{"inline_keyboard":[[{"text":"Revoke","callback_data":"`+dataRevoke+id+`"}]]}` { + t.Errorf("buttons %s", got) + } + cb := ce.f.tap(goodTap(msg, dataRevoke+id)) + if got := ce.f.waitAnswer(cb).str("text"); got != tapCountedRevoke { + t.Errorf("the phone shows %q", got) + } + var ev audit.GrantEnded + select { + case ev = <-endedCh: + case <-time.After(waitLimit): + t.Fatal("no grant_ended entry") + } + ended = append(ended, ev) + if ev.Cause != audit.EndRevoked || ev.GrantID != res.GrantID || ev.UsesLeft != 2 { + t.Errorf("entry %+v", ev) + } + ce.f.waitFor("the end of the message", func() bool { return len(ce.f.callsOfLocked("editMessageText")) >= 3 }) + last := ce.edits(msg)[2] + if !strings.Contains(last, "revoked by you at") || !strings.Contains(last, "2 use(s) left") || + !emptyMarkup(ce.f.messages[msg].markup) { + t.Errorf("last edit %q", last) + } + // A second tap is late: the phone says so, the message is shown again, + // and the tap is reported. + cb = ce.f.tap(goodTap(msg, dataRevoke+id)) + if got := ce.f.waitAnswer(cb).str("text"); got != tapEnded { + t.Errorf("the second tap shows %q", got) + } + ce.f.waitHandled(cb) + if taps := ce.taps.all(); len(taps) != 1 || taps[0].Cause != audit.TapUnknownRequest { + t.Errorf("reports %+v", taps) + } + if len(ended) != 1 || len(endedCh) != 0 { + t.Errorf("%d entries; want 1", len(ended)+len(endedCh)) + } + // The grant is gone: the use asks. + ch := ce.decide(secretsRequest("github-token")) + ce.asked(t, 2) + ce.f.tap(goodTap(ce.f.callsOf("sendMessage")[1].MsgID, dataDeny+ce.f.callsOf("sendMessage")[1].requestID())) + checkDeniedBy(t, waitResult(t, ch)) +} + +// TestCoreRevokeRefused: a Revoke from another user, in another chat, on +// another message or on a message that someone else edited does not count. +// The grant stays and can be used. +func TestCoreRevokeRefused(t *testing.T) { + ce := newCoreEnv(t, newFake(t)) + id, msg, res := ce.grantEnv(t, 1) + other := msg + 100 + for name, tc := range map[string]struct { + spec tapSpec + cause audit.TapCause + }{ + "other user": {tapSpec{from: approverID + 1, chatID: approverID, chatType: "private", msgID: msg, data: dataRevoke + id}, audit.TapOtherUser}, + "other chat": {tapSpec{from: approverID, chatID: noticeChatID, chatType: "supergroup", msgID: msg, data: dataRevoke + id}, audit.TapOtherChat}, + "inline": {tapSpec{from: approverID, inline: true, data: dataRevoke + id}, audit.TapInlineMessage}, + "bad data": {goodTap(msg, dataRevoke+id+"x"), audit.TapBadData}, + "other message": {tapSpec{from: approverID, chatID: approverID, chatType: "private", msgID: other, data: dataRevoke + id, + bare: true}, audit.TapWrongMessage}, + } { + before := len(ce.taps.all()) + cb := ce.f.tap(tc.spec) + ce.f.waitHandled(cb) + taps := ce.taps.all() + if len(taps) != before+1 || taps[before].Cause != tc.cause { + t.Errorf("%s: reports %+v, want cause %s", name, taps[before:], tc.cause) + } + for _, c := range ce.f.callsOf("editMessageText") { + if strings.Contains(c.str("text"), "revoked") { + t.Errorf("%s: the message says the grant was revoked", name) + } + } + } + // The grant still works: its use sends a notice, and needs no tap. + r := waitResult(t, ce.decide(secretsRequest("github-token"))) + if !r.Allowed || len(r.Events()) < 3 { + t.Fatalf("result %+v; want a use of the grant %s", r, res.GrantID) + } +} + +// TestCoreRevokeSkipsEditedCheck: a Revoke only takes a right away, so it +// counts on a message that someone else edited, on one with an edit date +// that the broker does not know, and on one that Telegram sends without +// content (older than 48 hours). It raises no alert. +func TestCoreRevokeSkipsEditedCheck(t *testing.T) { + for name, tc := range map[string]struct { + prepare func(f *fakeTelegram, msg int64) + spec func(msg int64, data string) tapSpec + }{ + "edited by someone": {func(f *fakeTelegram, msg int64) { f.editByOther(msg, nil, true) }, goodTap}, + "unknown edit date": {nil, func(msg int64, data string) tapSpec { s := goodTap(msg, data); s.edited = true; return s }}, + "no content": {nil, func(msg int64, data string) tapSpec { s := goodTap(msg, data); s.bare = true; return s }}, + } { + t.Run(name, func(t *testing.T) { + f := newFake(t) + ce := newCoreEnv(t, f) + id, msg, _ := ce.grantEnv(t, 1) + if tc.prepare != nil { + tc.prepare(f, msg) + } + messages := len(f.callsOf("sendMessage")) + cb := f.tap(tc.spec(msg, dataRevoke+id)) + if got := f.waitAnswer(cb).str("text"); got != tapCountedRevoke { + t.Errorf("the phone shows %q", got) + } + f.waitHandled(cb) + if got := len(f.callsOf("sendMessage")); got != messages { + t.Errorf("%d messages (an alert?) went out", got-messages) + } + if taps := ce.taps.all(); len(taps) != 0 { + t.Errorf("reports %+v", taps) + } + f.waitFor("the end of the message", func() bool { return len(f.callsOfLocked("editMessageText")) >= 3 }) + // The grant is gone: the use asks, and the person denies it. + ch := ce.decide(secretsRequest("github-token")) + id2, m2 := ce.asked(t, 2) + f.tap(goodTap(m2, dataDeny+id2)) + checkDeniedBy(t, waitResult(t, ch)) + }) + } +} + +// TestCoreGrantEndMessages: each end of a grant edits its message, with no +// button, and a later Deny says so. +func TestCoreGrantEndMessages(t *testing.T) { + ce := newCoreEnv(t, newFake(t)) + _, msg, _ := ce.grantEnv(t, 1) + // A later Deny for the secret ends the grant. + ch := ce.decide(&approval.Request{Client: "laptop", Kind: audit.KindSecrets, Secrets: []string{"db-password", "github-token"}}) + id, m := ce.asked(t, 2) + ce.waitTimers(t, 2) // the grant's and the request's + ce.f.tap(goodTap(m, dataDeny+id)) + checkDeniedBy(t, waitResult(t, ch)) + ce.f.waitFor("the end of the message", func() bool { + return len(ce.f.callsOfLocked("editMessageText")) >= 4 && emptyMarkup(ce.f.messages[msg].markup) + }) + var last string + for _, c := range ce.f.callsOf("editMessageText") { + if c.num("message_id") == msg { + last = c.str("text") + } + } + if !strings.Contains(last, "ended by a later Deny at") || !strings.Contains(last, "1 use(s) left") { + t.Errorf("last edit of the grant message: %q", last) + } +} + +// TestCoreNotUsedNotice: a grant that reaches its end with uses left sends +// one silent notice to the notice chat. A grant that was used up sends +// none. +func TestCoreNotUsedNotice(t *testing.T) { + ce := newCoreEnv(t, newFake(t)) + _, msg, res := ce.grantEnv(t, 2) + ce.clock.Advance(time.Hour) + ce.f.waitFor("the notice", func() bool { return len(ce.f.callsOfLocked("sendMessage")) >= 2 }) + notice := ce.f.callsOf("sendMessage")[1] + text := notice.str("text") + if notice.num("chat_id") != noticeChatID || notice.field("disable_notification") != "true" || notice.has("reply_markup") || + !strings.Contains(text, claim.HTML("not used: grant "+res.GrantID+" ended with 2 of 2 uses left")) { + t.Errorf("notice %s", notice.Raw) + } + ce.f.waitFor("the end of the message", func() bool { return emptyMarkup(ce.f.messages[msg].markup) }) + if last := ce.edits(msg); !strings.Contains(last[len(last)-1], "ended at") { + t.Errorf("edits %q", last) + } + // Used up: no notice. The use sends its own notice only. + n := len(ce.f.callsOf("sendMessage")) + ce.clock.Advance(time.Minute) + req := &approval.Request{ + Client: "laptop", Kind: audit.KindPreapprove, Secrets: []string{"github-token"}, Uses: 1, + NotBefore: noon.Add(time.Hour + time.Minute), NotAfter: noon.Add(2 * time.Hour), Claim: approval.Claim{Reason: "job"}, + } + ch := ce.decide(req) + id, m := ce.asked(t, n+1) + ce.f.tap(goodTap(m, dataOnce+id)) + r := waitResult(t, ch) + r.StartGrants() + waitResult(t, ce.decide(secretsRequest("github-token"))) + ce.f.waitFor("the use notice", func() bool { return len(ce.f.callsOfLocked("sendMessage")) >= n+2 }) + for _, c := range ce.f.callsOf("sendMessage")[n:] { + if strings.Contains(c.str("text"), "not used") { + t.Errorf("a grant that was used up sent %s", c.Raw) + } + } +} + +// TestCoreLateTapOnGrantMessage: the second tap of a double tap on Approve, +// or a tap on Deny, that reaches the broker after the message got its +// Revoke button, is a late tap. It is no sign of an edit: no alert goes +// out, and the Revoke button stays. +func TestCoreLateTapOnGrantMessage(t *testing.T) { + f := newFake(t) + ce := newCoreEnv(t, f) + id, msg, _ := ce.grantEnv(t, 1) + alerts := len(f.callsOf("sendMessage")) + for _, data := range []string{dataOnce + id, dataDeny + id} { + cb := f.tap(goodTap(msg, data)) + if got := f.waitAnswer(cb).str("text"); got != tapEnded { + t.Errorf("the phone shows %q for %s", got, data) + } + f.waitHandled(cb) + } + if got := len(f.callsOf("sendMessage")); got != alerts { + t.Errorf("%d messages went out after the late taps, want none", got-alerts) + } + if emptyMarkup(f.messages[msg].markup) { + t.Error("a late tap removed the Revoke button") + } + cb := f.tap(goodTap(msg, dataRevoke+id)) + if got := f.waitAnswer(cb).str("text"); got != tapCountedRevoke { + t.Errorf("the Revoke shows %q", got) + } +} + +// TestCoreOtherRequestButtonOnGrantMessage: a button of another request on +// a grant message is not the broker's doing. The tap is refused as a +// changed message, with the alert, and the Revoke button stays. +func TestCoreOtherRequestButtonOnGrantMessage(t *testing.T) { + f := newFake(t) + ce := newCoreEnv(t, f) + id, msg, _ := ce.grantEnv(t, 1) + messages := len(f.callsOf("sendMessage")) + cb := f.tap(goodTap(msg, dataOnce+testID(5))) + if got := f.waitAnswer(cb).str("text"); got != tapChanged { + t.Errorf("the phone shows %q", got) + } + f.waitHandled(cb) + if taps := ce.taps.all(); len(taps) != 1 || taps[0].Cause != audit.TapEditedMessage { + t.Errorf("reports %+v", taps) + } + f.waitFor("the alert", func() bool { return len(f.callsOfLocked("sendMessage")) > messages }) + if emptyMarkup(f.messages[msg].markup) || id == "" { + t.Error("the Revoke button went") + } +} + +// TestGrantEndMessageFits: the message of a grant with the longest body +// and the longest last lines is not over the limit of Telegram. +func TestGrantEndMessageFits(t *testing.T) { + // Find a body that fills the room of a message with one last line, so + // that two last lines would not fit there. + var p approval.Prompt + for n := 1; n <= 64; n++ { + q := preapprovePrompt() + q.Secrets = nil + for range 32 { + q.Secrets = append(q.Secrets, strings.Repeat("s", n)) + } + q.Uses = 1 << 20 + q.Claim = approval.Claim{Vars: strings.Repeat("๐Ÿ˜€", 400), Command: strings.Repeat("๐Ÿ˜€", 2000), + Dir: strings.Repeat("๐Ÿ˜€", 600), Reason: strings.Repeat("๐Ÿ˜€", 1000)} + if tx, err := fitBody(q, false, maxTextUnits-lastLineRoom); err == nil && tx.units() > maxTextUnits-lastLineRoom-60 { + p = q + break + } + } + if p.Client == "" { + t.Fatal("no body of the size to test") + } + at := time.Date(2026, 10, 28, 3, 30, 0, 0, time.UTC) + for _, cause := range []audit.EndCause{audit.EndEnded, audit.EndStopped, audit.EndDenied} { + tx := endingText(p, approval.Ending{Answer: audit.AnswerApproveOnce, At: at, Approved: at, GrantEnd: cause, UsesLeft: 1 << 20}) + if n := tx.units(); n > maxTextUnits || !strings.Contains(tx.plain.String(), "use(s) left") { + t.Errorf("%s: %d units", cause, n) + } + } + tx, _, err := promptMessage(p) + if err != nil || tx.units() > maxTextUnits { + t.Errorf("message: %v, %d units", err, tx.units()) + } +} + +// TestCoreRevokeOnWaitingMessage: someone with the bot token puts Revoke +// data on the message of a request that still waits. A tap on it skips +// nothing: it is refused as a changed message, with the alert, and the +// request still waits. +func TestCoreRevokeOnWaitingMessage(t *testing.T) { + f := newFake(t) + ce := newCoreEnv(t, f) + ch := ce.decide(secretsRequest("github-token")) + id, msg := ce.asked(t, 1) + messages := len(f.callsOf("sendMessage")) + cb := f.tap(goodTap(msg, dataRevoke+id)) + if got := f.waitAnswer(cb).str("text"); got != tapChanged { + t.Errorf("the phone shows %q", got) + } + f.waitHandled(cb) + if taps := ce.taps.all(); len(taps) != 1 || taps[0].Cause != audit.TapEditedMessage { + t.Errorf("reports %+v", taps) + } + f.waitFor("the alert", func() bool { return len(f.callsOfLocked("sendMessage")) > messages }) + f.tap(goodTap(msg, dataDeny+id)) // it still waits + checkDeniedBy(t, waitResult(t, ch)) +} diff --git a/internal/telegram/telegram.go b/internal/telegram/telegram.go index c254b9d..45cd9ca 100644 --- a/internal/telegram/telegram.go +++ b/internal/telegram/telegram.go @@ -11,6 +11,12 @@ // or the URL: an error names the Bot API method and a fixed reason // (THREAT-MODEL.md T19). // +// The message of a scheduled grant that started also has a Revoke button. +// A Revoke only takes a right away, so it skips the check of a changed +// message and never raises its alert, except on a message whose request +// may still wait: there it is refused as a changed message, with the +// alert. The other checks stay. +// // A button press counts only if all of these are true (DESIGN.md section // 7): it is from the configured user ID, in the private chat with that // user, on the message of its request, for a request that waits, before @@ -142,6 +148,11 @@ type Approver struct { running atomic.Bool // Run runs + // editMu orders the edit that adds a Revoke button (End) and the edit + // that shows a result again after a late tap (ended), so that the + // second cannot remove the button that the first adds. + editMu sync.Mutex + mu sync.Mutex // sent holds the messages that Ask sent. order holds its keys, oldest // first; there are at most maxRemembered. @@ -168,6 +179,11 @@ type sentMessage struct { keyboard [][]inlineButton // final is the HTML text that End gave the message, or "" before End. final string + // held is true once End gave the message its Revoke button: the + // request of the message is decided, and only a Revoke can count on + // it. heldID is the request ID in that button. + held bool + heldID string } // reportWindow counts the reports of refused taps of one budget in one @@ -329,6 +345,22 @@ func (a *Approver) End(ctx context.Context, m approval.MessageID, p approval.Pro return errors.New("telegram: End: the message ID is not one that Ask returned") } html := endingText(p, e).html.String() + if e.Revoke { + // The message of a scheduled grant that started: it keeps the + // Revoke button. A Revoke needs no check of the message (see + // changed). The grant's end sets the final text. + kb := revokeKeyboard(p) + a.editMu.Lock() + defer a.editMu.Unlock() + a.hold(m, p.RequestID) + return a.editWith(ctx, chatID, msgID, html, kb) + } + if e.GrantEnd != "" { + // The end of a grant: a late tap must not write the approval text + // again after this edit (see ended). + a.editMu.Lock() + defer a.editMu.Unlock() + } // First, so that a tap that comes now sees that the broker itself // changes this message, and leaves the result as it is. a.setFinal(m, html) @@ -350,7 +382,7 @@ func (a *Approver) Notify(ctx context.Context, p approval.Prompt) error { return err } if !a.takeNotice() { - a.log.LogAttrs(ctx, slog.LevelWarn, "telegram: too many notices in the last minute; the use is denied", + a.log.LogAttrs(ctx, slog.LevelWarn, "telegram: too many notices in the last minute; the use is denied, or the notice is dropped", slog.String("request_id", p.RequestID), slog.Int("limit", NoticesPerMinute)) return ErrNoticeLimit } @@ -420,16 +452,22 @@ func (a *Approver) Inform(ctx context.Context, msg string) error { // edit replaces the text of a message with html, and removes its buttons. func (a *Approver) edit(ctx context.Context, chatID, msgID int64, html string) error { + return a.editWith(ctx, chatID, msgID, html, emptyKeyboard()) +} + +// editWith replaces the text of a message with html and its buttons with +// kb. +func (a *Approver) editWith(ctx context.Context, chatID, msgID int64, html string, kb inlineKeyboard) error { err := a.api.call(ctx, "editMessageText", editMessageTextParams{ ChatID: chatID, MessageID: msgID, Text: html, ParseMode: "HTML", LinkPreviewOptions: noPreview, - ReplyMarkup: emptyKeyboard(), + ReplyMarkup: kb, }, nil) if errors.Is(err, errNotModified) { - return nil // it shows this text already, without buttons + return nil // it shows this text already, with these buttons } return err } @@ -441,6 +479,16 @@ func (a *Approver) remember(m approval.MessageID, keyboard [][]inlineButton) { a.entryLocked(m).keyboard = keyboard } +// hold notes that the message m has a Revoke button for the request id, +// after End gave it its first result: its request is a scheduled grant that +// started. The message has no final text until the grant ends. +func (a *Approver) hold(m approval.MessageID, id string) { + a.mu.Lock() + defer a.mu.Unlock() + e := a.entryLocked(m) + e.keyboard, e.final, e.held, e.heldID = revokeData(id), "", true, id +} + // setFinal notes the text that End gives the message m. func (a *Approver) setFinal(m approval.MessageID, html string) { a.mu.Lock() diff --git a/internal/telegram/text.go b/internal/telegram/text.go index cc0f9dc..7ddf674 100644 --- a/internal/telegram/text.go +++ b/internal/telegram/text.go @@ -80,6 +80,9 @@ const ( dataOnce = "o:" dataWindow = "w:" dataDeny = "d:" + // dataRevoke is the Revoke button of a scheduled grant. It carries the + // ID of the request that made the grant. + dataRevoke = "r:" dataLen = 2 + requestIDLen ) @@ -103,7 +106,7 @@ func isBase64URL(c byte) bool { return 'A' <= c && c <= 'Z' || 'a' <= c && c <= 'z' || '0' <= c && c <= '9' || c == '-' || c == '_' } -// parseData reads button data strictly: "o:", "w:" or "d:", and a +// parseData reads button data strictly: "o:", "w:", "d:" or "r:", and a // request ID. Anything else is not valid. func parseData(data string) (audit.Answer, string, bool) { if len(data) != dataLen { @@ -117,6 +120,8 @@ func parseData(data string) (audit.Answer, string, bool) { action = audit.AnswerApproveWindow case dataDeny: action = audit.AnswerDeny + case dataRevoke: + action = audit.AnswerRevoke default: return "", "", false } @@ -235,7 +240,8 @@ func checkPrompt(p approval.Prompt, notice bool) error { case !namePattern.MatchString(p.Client): return bad("the client name") } - if p.Kind != audit.KindPreapprove && (p.Uses != 0 || !p.From.IsZero() || !p.Until.IsZero()) { + if p.Kind != audit.KindPreapprove && (p.Uses != 0 || !p.From.IsZero() || !p.Until.IsZero() || + p.GrantID != "" || p.UsesLeft != 0) { return bad("a span of uses outside a scheduled grant") } switch { @@ -263,8 +269,14 @@ func checkPrompt(p approval.Prompt, notice bool) error { named := len(p.Secrets) > 0 && allNames(p.Secrets) && p.Key == "" && len(p.Hosts) == 0 && p.RemoteUser == "" keyed := len(p.Secrets) == 0 && namePattern.MatchString(p.Key) && len(p.Hosts) > 0 && len(p.Hosts) <= approval.MaxGrantHosts && allNames(p.Hosts) && remoteUserPattern.MatchString(p.RemoteUser) - if notice || (!named && !keyed) || p.HostKey != "" || !p.GrantEnds.IsZero() || !p.Once || p.WindowMinutes != 0 || - p.Uses < 1 || p.From.IsZero() || !p.From.Before(p.Until) { + if (!named && !keyed) || p.HostKey != "" || !p.GrantEnds.IsZero() || p.WindowMinutes != 0 || + p.Uses < 1 || p.From.IsZero() || !p.From.Before(p.Until) || p.Via != "" { + return bad("the fields of a scheduled grant") + } + // The notice that a grant ended with uses left names the grant; + // the message of the request does not. + if notice != validRequestID(p.GrantID) || (notice && (p.UsesLeft < 1 || p.UsesLeft > p.Uses)) || + (!notice && (p.GrantID != "" || p.UsesLeft != 0 || !p.Once)) { return bad("the fields of a scheduled grant") } default: @@ -347,7 +359,10 @@ func body(p approval.Prompt, notice bool, cv claimedValues, c caps) *text { t := &text{} t.write(title(p, notice)) if notice { - if p.Via == audit.ViaScheduled { + if p.Kind == audit.KindPreapprove { + t.write("\nnot used: grant " + p.GrantID + " ended with " + strconv.Itoa(p.UsesLeft) + " of " + + strconv.Itoa(p.Uses) + " uses left") + } else if p.Via == audit.ViaScheduled { t.write("\n" + scheduledNoticeLine) } else { t.write("\n" + noticeLine) @@ -440,6 +455,17 @@ func keyboard(p approval.Prompt) inlineKeyboard { return inlineKeyboard{InlineKeyboard: [][]inlineButton{row}} } +// revokeKeyboard returns the one Revoke button of the message of a +// scheduled grant. +func revokeKeyboard(p approval.Prompt) inlineKeyboard { + return inlineKeyboard{InlineKeyboard: revokeData(p.RequestID)} +} + +// revokeData returns the rows of the Revoke button of the request id. +func revokeData(id string) [][]inlineButton { + return [][]inlineButton{{{Text: "Revoke", CallbackData: dataRevoke + id}}} +} + // buttonsLine shows the buttons of k as one line of text, as DESIGN.md // section 7 does: "[ Approve once ] [ Deny ]". func buttonsLine(k inlineKeyboard) string { @@ -458,13 +484,23 @@ func expiryLine(p approval.Prompt) string { return "expires " + p.Expires.Format(clockFormat) + " (" + strconv.FormatInt(secs, 10) + " s)" } +// bodyLimit is the most UTF-16 code units of the body of the message of +// p. The last lines need room: one, or for a scheduled grant two, because +// its message ends with the line of its approval and the line of its end. +func bodyLimit(p approval.Prompt) int { + if p.Kind == audit.KindPreapprove { + return maxTextUnits - 2*lastLineRoom + } + return maxTextUnits - lastLineRoom +} + // promptMessage returns the message that Ask sends for p: its text and // its buttons. func promptMessage(p approval.Prompt) (*text, inlineKeyboard, error) { if err := checkPrompt(p, false); err != nil { return nil, inlineKeyboard{}, err } - t, err := fitBody(p, false, maxTextUnits-lastLineRoom) + t, err := fitBody(p, false, bodyLimit(p)) if err != nil { return nil, inlineKeyboard{}, err } @@ -477,7 +513,7 @@ func promptMessage(p approval.Prompt) (*text, inlineKeyboard, error) { func endingText(p approval.Prompt, e approval.Ending) *text { var t *text if checkPrompt(p, false) == nil { - t, _ = fitBody(p, false, maxTextUnits-lastLineRoom) + t, _ = fitBody(p, false, bodyLimit(p)) } if t == nil { // Ask could not have sent a message for p. Show the result anyway. @@ -491,6 +527,9 @@ func endingText(p approval.Prompt, e approval.Ending) *text { // resultLine says how the request of p ended (DESIGN.md section 7). func resultLine(p approval.Prompt, e approval.Ending) string { at := e.At.Format(clockFormat) + if e.GrantEnd != "" { + return grantEndLines(p, e) + } switch e.Answer { case audit.AnswerApproveOnce: if p.Kind == audit.KindPreapprove { @@ -517,6 +556,29 @@ func resultLine(p approval.Prompt, e approval.Ending) string { return "โ›” denied" } +// grantEndLines says how and when the scheduled grant of p ended: the +// line of its approval, then the line of its end. +func grantEndLines(p approval.Prompt, e approval.Ending) string { + approved := resultLine(p, approval.Ending{Answer: e.Answer, At: e.Approved}) + at := e.At.Format(dateTimeFormat) + var end string + switch e.GrantEnd { + case audit.EndUsedUp: + end = "โœ… all " + strconv.Itoa(p.Uses) + " use(s) used; the grant ended at " + at + case audit.EndEnded: + end = "โŒ› ended at " + at + " with " + strconv.Itoa(e.UsesLeft) + " of " + strconv.Itoa(p.Uses) + " use(s) left" + case audit.EndCancelled: + end = "โ›” cancelled by the client at " + at + ", with " + strconv.Itoa(e.UsesLeft) + " use(s) left" + case audit.EndDenied: + end = "โ›” ended by a later Deny at " + at + ", with " + strconv.Itoa(e.UsesLeft) + " use(s) left" + case audit.EndRevoked: + end = "โ›” revoked by you at " + at + ", with " + strconv.Itoa(e.UsesLeft) + " use(s) left" + default: // audit.EndStopped + end = "โ›” dropped: the broker stopped at " + at + ", with " + strconv.Itoa(e.UsesLeft) + " use(s) left" + } + return approved + "\n" + end +} + // noticeText returns the text of the notice of a "never ask" use. func noticeText(p approval.Prompt) (*text, error) { if err := checkPrompt(p, true); err != nil {