Repository navigation
fix: serve the console account pages to an IAM user for its own account only - #734
Open
LeeroyHannigan wants to merge 6 commits into
Open
LeeroyHannigan wants to merge 6 commits into
LeeroyHannigan wants to merge 6 commits into
Conversation
…unt only
The account, user, group and role pages under
/console/accounts/{account_id} checked only that a console session existed.
An IAM user who requested another account's ID read that account's name
and the names of its users, groups and roles, and from the user, group and
role pages each user's access key IDs, policy names, groups and tags, and
each role's trust policy.
Every handler under that path, GET and POST, now gets its session from
require_account_session. It redirects an IAM-user session to /console when
the account ID in the path is not the session's own account. The check runs
before the handler looks up the account in the path, so another account's
ID and an ID that no account has get the same answer. An admin session
keeps access to every account.
…s only The console home page read dashboard_counts() for every session, so an IAM user saw the number of accounts and admin users in the whole deployment. The handler now reads the counts for admin sessions only, and the page shows them to admin sessions only. An IAM-user session gets a card that links to its own account.
… account only Accounts A and B each hold users, a group, a role, inline policies, an access key and a tag. Logged in as an IAM user of account B, every GET page and every POST form under /console/accounts/<A> redirects to /console, names nothing in A, and changes nothing in A, and an account ID that no account has gets the same answer as A's. In account B the user reads the account page and its own user page, manages its own access keys, and is refused every other change. An admin reads the pages of both accounts. The home page shows the account and admin counts to the admin only. The pages of B's other users, groups and roles are not checked either way: #731 tracks which of them an IAM-user session should read. The fixture reads each account's state once, and each form test reads it once after its POST and compares it with the last state read, so a form that gets through fails only its own test.
…session
The security model now says that every page and form under
/console/accounts/{account_id} serves an IAM-user session its own account
only and redirects it to /console for any other account, that the handler
compares the account IDs before it looks up the account in the path, so
another account's ID and an ID that no account has get the same answer,
and that the home page shows the deployment's account and admin counts to
admin sessions only. It also says that in its own account an IAM-user
session currently reads every user, group and role page, which the
management API refuses the same user and #731 tracks, and that
/console/metrics-data serves every account's table metrics to every
console session until pull request #727 merges.
The upgrade manual has an entry for the change. It names the four pages
under /console/accounts/{account_id} that checked only for a session, and
says where each refused form for another account redirected before: 8 of
the 17 to a page of that account, the delete account form to
/console/accounts, and the 2 access key forms and 6 policy forms to
/console. The admin guide, the getting started guide and the architecture
guide say that the home page counts are for admin sessions only.
LeeroyHannigan
requested review from
amrith,
c33howard,
jcshepherd,
pdf-amzn,
robinnsc and
yesyayen
as code owners
November 1, 2026 16:34
5 of 8 tasks
… message The refusal helper in the account_scope tests formatted the session's CallerIdentity into the message of the panic it raises when the check lets a session in. Code scanning reports that as clear-text logging of a sensitive value. The message now names the account only.
…essage Code scanning still reported the refusal helper's panic as clear-text logging after the session was taken out of its message, because the message named the account. The helper now calls expect_err with a fixed message, so the failure prints nothing from the request.
4 of 8 tasks
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The console pages and forms under
/console/accounts/{account_id}now serve an IAM-user session its own account only. An IAM-user session no longer sees the deployment's account and admin counts on the console home page.require_account_session, in the newcrates/server/src/console/pages/account_scope.rs, gets the console session the wayrequire_sessiondoes. For an IAM-user session it then compares the account ID in the path with the session's own account, and when the two differ it returns a redirect to/console. fix: serve the console metrics dashboard to admin sessions only #727 gives an IAM user the same redirect on the metrics page. The comparison comes before the handler looks up the account in the path, so another account's ID and an ID that no account has get the same answer. An admin session passes for every account.require_session. They cover the account page and its delete form, the user, group and role pages with their create, delete and membership forms, the two access key forms, and the three shared policy helpers behind the nine policy routes.dashboard()readsdashboard_counts()for admin sessions only. An IAM-user session gets a card that links to its own account in place of the counts.crates/server/src/console/pages/mod.rschanges by one line,mod account_scope;.The security model now says what an IAM-user console session can read and change, and names the reads that #731 and #727 track. The admin guide, the getting started guide and the architecture guide say the home page counts are for admin sessions only, and the upgrade manual has a v0.1.15 entry.
A trial merge of this branch with each of #704, #718 and #727 has no conflicts.
Why
account_detail,user_detail,group_detailandrole_detailchecked only that a console session existed, then read whatever account the path named.list_accountsalready limits an IAM user to its own account, but an account ID missing from that list could still be read by ID. The pages also answered differently for an account that exists and one that does not, so they told an IAM user which account IDs exist.Reproduction on main's console code (
f49c5f823), SQLite backend, 2026-11-01. Each request was sent twice and got the same answer both times.ownerandpeer, a group withowneras its member, a role, an inline policy on the user, the group and the role, an access key forownerand a tag onowner. Create account B with an IAM user that has a console password.GET /console/accounts/<A>returns 200 with A's name and the names of both users, the group and the role.GET /console/accounts/<A>/users/ownerreturns 200 with the access key ID, the policy name, the group name, and the tag key and value. The group page shows the member and the group's policy name, and the role page shows the role's policy name.The same requests on this branch get 303 with
Location: /consoleand an empty body, for A's ID and for the missing ID alike, and the home page shows no count.On main every POST handler under these routes already checks for an admin session itself, except the two access key handlers, which also accept the IAM user named in the path. B's IAM user posted all 17 forms in its own account, each with a valid CSRF token and a complete form, against main's console code and against this branch. Every one answered 303 and changed nothing. In that run the access key forms named another user of B.
Closes #724
Testing done
New tests:
crates/server/src/console/pages/account_scope.rs, 5 tests. An IAM user may use its own account. Another account's ID gets 303 to/consolewith an empty body. Another account's ID and a missing ID get the same status,Locationheader and body. IDs that only resemble the user's own are refused: one digit short, one digit long, a leading zero, a leading space, a trailing slash and the empty string. An admin may use every account.crates/server/src/console/pages/account_pages_tests.rs, 2 tests. An IAM user's home page has no count and links to its own account, even when the counts are passed in. An admin's home page shows both counts.tests/test_console_account_scope.py, 61 cases against a server, with accounts A and B set up as in the reproduction and every name given a random suffix. As B's IAM user, each of the 10 GET pages and 17 POST forms under A's ID answers 303 to/console, and an ID that no account has gets the same answer. The GET answers name nothing in A, and the POST forms leave A's users, groups, roles, access keys, policies and group members as they were. In account B the user reads B's account page and its own user page, and creates and deletes its own access key. All 17 forms refuse it there too. An admin reads the account, user, group and role pages of both accounts. The home page shows the counts to the admin and not to the IAM user.Commands, all on this branch:
cargo +1.97.0 fmt --all -- --check: clean.cargo +1.97.0 clippy --locked --workspace --all-targets -- -D warnings: clean. With-W clippy::pedanticthe server crate has 107 warnings, 87 in the library and 20 more in its tests. Three are in files this PR touches, and all three are in lines it does not change:too_many_linesongroup_detailanduser_detail, andsimilar_namesinuser_detail.cargo test --locked -p extenddb-serveron Rust 1.99.0 and on 1.88.0: 244 passed on each.cargo check --locked --workspace --all-targets: clean.tests/test_console_account_scope.py61 passed on each.tests/test_console_integration.py,tests/test_console_cache_coherence.py,tests/test_console_settings_redaction.pyandtests/test_cross_account_isolation.py46 passed on each.Negative controls:
/console/accountsor a page of account A, the missing-ID answers differ from A's for seven pages, and the home page shows the counts. The 37 that pass are cases where main already behaves this way: the policy and access key routes, which redirect an IAM user to/consoleon main too, the 17 forms in the user's own account, the user's own account page, user page and access keys, and the admin reads.Checklist
cargo test --workspace)cargo fmt --check)cargo clippy -- -W clippy::pedantic)Storagetrait, auth model, on-diskformat, or public CLI surface, an RFC has been accepted or is linked
below. Otherwise, an ADR captures the decision (link below).
ADR / RFC: n/a
Breaking changes
IAM users lose the console pages of every account but their own, and the account and admin counts on the console home page. Those pages redirect them to
/console. A refused form for another account now redirects to/consoletoo. On main, 8 of the 17 forms redirected to a page of that account (the forms that create and delete users, groups and roles, and that add and remove group members), the delete account form redirected to/console/accounts, and the 2 access key forms and 6 policy forms redirected to/console. Admin sessions are unchanged.Not covered by this change
arn:aws:iam::*:user/*, and the console's own create handler refuses that principal. Submitting the form unchanged on SQLite on 2026-11-01 failed twice with "Invalid trust policy: wildcard is not allowed in a principal ARN". It is filed as Console: the New Role form's default trust policy is refused by the console's own create handler #730./console/metrics-datastill serves every console session the table metrics of every account. fix: serve the console metrics dashboard to admin sessions only #727 limits it to admin sessions. The security model paragraph this PR adds says the feed serves every account until fix: serve the console metrics dashboard to admin sessions only #727 merges, and fix: serve the console metrics dashboard to admin sessions only #727 should delete that sentence.account_scopeand the home page renderer, and none calls a handler, so onlytests/test_console_account_scope.pychecks that the 21 handlers make the call, for the routes it lists by hand in itsPAGESandFORMStables. A route added under/console/accounts/{account_id}must callrequire_account_sessionand needs an entry in those tables.By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache License 2.0 and I agree to the Developer Certificate of
Origin (DCO). See CONTRIBUTING.md for details.