Skip to content

Distinguish 404 from 403 in authz-roles policy test snapshot#10559

Merged
david-crespo merged 3 commits into
mainfrom
diff-404-403
Jun 10, 2026
Merged

Distinguish 404 from 403 in authz-roles policy test snapshot#10559
david-crespo merged 3 commits into
mainfrom
diff-404-403

Conversation

@david-crespo

Copy link
Copy Markdown
Contributor

Built on top of #10550. Big diff but it's all in the snapshot. This snapshot was originally added in #1123 and I don't see a rationale given there for collapsing 404 and 403. I don't see a downside, it just gives you more info. Will take advantage of this in my Oso to Cedar port.

@david-crespo

Copy link
Copy Markdown
Contributor Author

@davepacheco curious if you can think of a reason you might have not wanted to distinguish 404s and 403s in this authz snapshot test. The discussion in the PR is about how long it takes to run.

@davepacheco

Copy link
Copy Markdown
Collaborator

By "snapshot" you mean the file containing the test's expected output, right? I can't think offhand of a particular reason not to distinguish these here.

Base automatically changed from policy-test-builtins to main June 10, 2026 19:06
The conferred-roles test only registered Fleet-rooted resources (Fleet,
IpPoolList), so it never exercised a conferring user reaching a *different*
Silo than their own. Add a second Silo ("other-silo") to the resource set.

This also gives the test 404-vs-403 coverage: a user with no path to the
other Silo can not see it (404, rendered ∅), while a user whose main-Silo
role confers a Fleet role can (403/visible, rendered ✘/✔). Regenerate
authz-conferred-roles.out accordingly.
@david-crespo david-crespo enabled auto-merge (squash) June 10, 2026 19:27
@david-crespo david-crespo merged commit 72562b7 into main Jun 10, 2026
18 checks passed
@david-crespo david-crespo deleted the diff-404-403 branch June 10, 2026 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants