Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes cluster-admin seeing a completely empty UI #2574

Merged
merged 4 commits into from
Sep 9, 2022

Conversation

foot
Copy link
Contributor

@foot foot commented Aug 10, 2022

Fixes allowing cluster-admin and other users with * permissions in their RBAC rules from using the UI.

Prior to this our rule engine determined that cluster-admin couldn't see any namespaces as we didn't take * into account.

What changed?

Adds support for evaluating * in the various positions it can appear ({ "verbs": ["*"], "apiGroups": ["*"], "resources": ["*"] }) when evaluating the rules that determine which Namespaces we think a user has access to.

Why was this change made?

Its common for people to use cluster-admin when debugging permissions as cluster-admin should be able to see everything.

How was this change implemented?

Updates to our rules engine.

How did you validate the change?

Only with unit tests so far.

@foot
Copy link
Contributor Author

foot commented Aug 10, 2022

For reference this is what a SelfSubjectReviews response for cluster-admin looks like:

{
  "metadata": { "creationTimestamp": null },
  "spec": {},
  "status": {
    "resourceRules": [
      {
        "verbs": ["create"],
        "apiGroups": ["authorization.k8s.io"],
        "resources": ["selfsubjectaccessreviews", "selfsubjectrulesreviews"]
      },
      { "verbs": ["*"], "apiGroups": ["*"], "resources": ["*"] }
    ],
    "nonResourceRules": [
      {
        "verbs": ["get"],
        "nonResourceURLs": [
          "/healthz",
          "/livez",
          "/readyz",
          "/version",
          "/version/"
        ]
      },
      {
        "verbs": ["get"],
        "nonResourceURLs": [
          "/api",
          "/api/*",
          "/apis",
          "/apis/*",
          "/healthz",
          "/livez",
          "/openapi",
          "/openapi/*",
          "/readyz",
          "/version",
          "/version/"
        ]
      },
      { "verbs": ["*"], "nonResourceURLs": ["*"] }
    ],
    "incomplete": true,
    "evaluationError": "webhook authorizer does not support user rule resolution"
  }
}

@alexandermarston
Copy link
Contributor

alexandermarston commented Aug 21, 2022

👍 would be great to have this merged, fixes some issues we're having with cluster access

@foot any plans to push this through?

@foot
Copy link
Contributor Author

foot commented Sep 6, 2022

Sorry! Got distracted. Will try and wrap this up for the next release

@foot foot force-pushed the fixes-cluster-admin-querying branch from a4316ff to ea4d1b6 Compare September 7, 2022 10:55
@foot foot marked this pull request as ready for review September 7, 2022 11:00
@foot foot changed the title Fixes cluster admin querying Fixes cluster-admin seeing a completely empty UI Sep 7, 2022
@foot foot requested review from jpellizzari and a team September 8, 2022 09:05
Copy link
Contributor

@jpellizzari jpellizzari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, love the extra comments 👍

@foot foot merged commit f3d1fab into main Sep 9, 2022
@foot foot deleted the fixes-cluster-admin-querying branch September 9, 2022 10:31
@ozamosi ozamosi added the bug Something isn't working label Sep 14, 2022
This was referenced Sep 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants