Skip to content

Add e2e tests for webhook grants passing through authz gates #4512

Closed

Description

@mjudeikis-bot

Context

PR #4507 ("Restructure authz chain") splits the authorization chain in pkg/server/options/authorization.go (Authorization.ApplyTo) into three groups: bypass, grant and gate. The only new e2e coverage is TestMaximalPermissionPolicyWebhookEscalation. Nothing tests the other gates, so a later reorder of the blocks or a change to the gate decision handling would not fail any test.

Requested in #4507 (comment) (follow-up to the review comment at #4507 (comment)).

What exists today

  • pkg/server/options/authorization.go: the final authorizer closure sits inline in ApplyTo. It composes the bypass authorizers, the grant authorizers (RBAC, webhook) and the gates (gate-maxpermissionpolicy, gate-systemcrd, gate-requiredgroups).
  • The e2e suite only covers the maximal permission policy gate against a webhook grant.

What should be added

An e2e test (against a kcp server configured with an authorization webhook) proving that a webhook decision cannot get around the gates:

  • A webhook Allow for a user not in the groups listed in the workspace's required-groups annotation ends in a Deny.
  • A webhook Allow for update/patch on apiexports/status and apibindings/status (apis.kcp.io) ends in a Deny.
  • A webhook Deny short-circuits before the gates (the gates are not consulted and the request is denied).

Optionally, extract the closure into a function, e.g. chainAuthorizer(bypass, preGate, grants, gates authorizer.Authorizer) authorizer.Authorizer, so the decision table can also be unit tested.

Checklist

  • Look at how TestMaximalPermissionPolicyWebhookEscalation sets up the webhook and reuse its helpers.
  • Add an e2e case: webhook Allow plus the required-groups annotation, user lacks the group, expect Forbidden.
  • Add an e2e case: webhook Allow on apiexports/status and apibindings/status, expect Forbidden.
  • Add an e2e case: webhook Deny, expect Forbidden and that the gates do not change the outcome.
  • (Optional) Extract the closure and add a table-driven unit test over (bypass, grants, gates) decisions.

Activity

  1. added
    good first issueDenotes an issue ready for a new contributor, according to the "help wanted" guidelines.
    on Oct 29, 2026
  2. souvik-biswas-dev commented on Oct 30, 2026

    @souvik-biswas-dev
    Contributor

    Hi, I'd like to take this. My plan is to add the three e2e cases following TestMaximalPermissionPolicyWebhookEscalation's setup, and I'll skip the optional refactor unless you want it. Is #4507 merged or still changing?

  3. moved this from New to Done in kcpon Nov 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    good first issueDenotes an issue ready for a new contributor, according to the "help wanted" guidelines.

    Type

    No type

    Projects

    • Status
      Done

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions