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

Feature: Enable the Kubernetes Proxy Endpoint to be disabled via PermissionPolicy #16237

Merged
merged 6 commits into from Mar 21, 2023

Conversation

RubenV-dev
Copy link
Contributor

@RubenV-dev RubenV-dev commented Feb 7, 2023

Hey, I just made a Pull Request!

I am introducing a way to restrict access to the proxy-endpoint via integration with the Permissions framework. This is being done by:

  • Introducing basic-permissions that can be used by integrators via permission policy to restrict access to the kubernetes proxy endpoint entirely.
  • Require the need for two separate tokens to be provided to the kubernetes proxy endpoint via headers in the request.
    A backstage identity token to determine whether users have the correct permissions to use the endpoint to be provided as a value field to the Authorization header and a second token obtained from an auth provider that will be passed to authenticate with the kubernetes api when the proxy middleware redirects the request provided as a value field to the X-Kubernetes-Authorization header.

Context:

Here is my basic setup:
I enable permissions first.

OIDC Authority

I create an azure AD app registration with

$ export CLIENT_ID=$(az ad app create --display-name my-backstage --web-redirect-uris http://localhost:7007/api/auth/microsoft/handler/frame http://localhost:8000 | jq -r .appId)
$ export CLIENT_SECRET=$(az ad app credential reset --append --id $CLIENT_ID | jq -r .password)

OIDC Enabled Kind Cluster

I create a kind cluster with:

$ kind create cluster --config - <<EOF
apiVersion: kind.x-k8s.io/v1alpha4
kind: Cluster
kubeadmConfigPatches:
- |-
  kind: ClusterConfiguration
  apiServer:
    extraArgs:
      oidc-client-id: CLIENT-ID
      oidc-issuer-url: https://login.microsoftonline.com/TENANT-ID/v2.0
      oidc-username-claim: email
EOF

where CLIENT-ID is the ID for the app registration i created previously, and TENANT-ID is the ID of my azure AD tenant.

Setup RBAC on the cluster

kubectl create clusterrolebinding me-admin --user EMAIL-ADDRESS --clusterrole cluster-admin

where EMAIL_ADDRESS is the email of my azure account

Backstage

app-config

kubernetes:
  serviceLocatorMethod:
    type: 'multiTenant'
  clusterLocatorMethods:
    - type: config
      clusters:
        - name: kind
          url: https://127.0.0.1:PORT
          authProvider: oidc
          oidcTokenProvider: microsoft
          skipTLSVerify: true
          skipMetricsLookup: true
auth:
  environment: development
  providers:
    microsoft:
      development:
        clientId: ${CLIENT_ID}
        clientSecret: ${CLIENT_SECRET}
        tenantId: ${TENANT_ID}
catalog:
  locations:
    - type: file
      target: ../../kubernetes.yaml
      rules:
        - allow: [User, Component, Resource]

where PORT is my kind cluster port obtain by using kubectl cluster-info in your terminal. Other values were described above.
kubernetes.yaml

apiVersion: backstage.io/v1alpha1
kind: User
metadata:
  name: MYNAME
  annotations:
    'microsoft.com/email': EMAIL
spec:
  memberOf: []

where MYNAME and EMAIL match the name and email used to sign into microsoft.

I sign in using the Microsoft sign in resolver and on refresh find the refresh call under the network tab in my browser. Here i navigate to the response tab to obtain the providerInfo.accessToken to use as my X-Kubernetes-Authorization value and backstageIdentity.token as my Authorization header value.

Here i can call the proxy endpoint along with clustername as shown in unit tests and get a valid response.

After_Changes

I can create a permission policy like:
packages/backend/src/plugins/permissions.ts

class KubernetesDenyAllProxyEndpointPolicy implements PermissionPolicy {
  async handle(
    request: PolicyQuery,
    user?: BackstageIdentityResponse,
  ): Promise<PolicyDecision> {
    if (
      request.permission.name === 'kubernetes.proxy"
    ) {
      return {
        result: AuthorizeResult.DENY,
      };
    }
    return { result: AuthorizeResult.ALLOW };
  }
}

where the proxy endpoint returns 403 Forbidden messages even though I provided a valid oidc token in my request.

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)
  • All your commits have a Signed-off-by line in the message. (more info)

@backstage-goalie
Copy link
Contributor

backstage-goalie bot commented Feb 7, 2023

Changed Packages

Package Name Package Path Changeset Bump Current Version
example-backend packages/backend none v0.2.81
@backstage/plugin-kubernetes-backend plugins/kubernetes-backend minor v0.9.4
@backstage/plugin-kubernetes-common plugins/kubernetes-common patch v0.6.1

@github-actions
Copy link
Contributor

github-actions bot commented Feb 7, 2023

Uffizzi Preview deployment-14728 was deleted.

Copy link
Member

@jamieklassen jamieklassen left a comment

Choose a reason for hiding this comment

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

Can we add a permission integration router so that the plugin announces the permissions it supports at an endpoint like /api/kubernetes/.well-known/backstage/permissions/metadata? I did something similar in #15150.

.changeset/eleven-bats-tease.md Outdated Show resolved Hide resolved
plugins/kubernetes-common/src/permissions.ts Outdated Show resolved Hide resolved
plugins/kubernetes-backend/src/service/KubernetesProxy.ts Outdated Show resolved Hide resolved
@Rugvip Rugvip self-requested a review February 9, 2023 16:20
@benjdlambert benjdlambert removed their request for review February 9, 2023 17:28
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Feb 9, 2023
Copy link
Member

@Rugvip Rugvip left a comment

Choose a reason for hiding this comment

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

Thank you! 👍

Couple of smaller things but also want to get some clarity on how we're able to evolve the integration

.changeset/eleven-bats-tease.md Outdated Show resolved Hide resolved
plugins/kubernetes-backend/src/service/KubernetesProxy.ts Outdated Show resolved Hide resolved
plugins/kubernetes-backend/api-report.md Outdated Show resolved Hide resolved
plugins/kubernetes-common/src/permissions.ts Outdated Show resolved Hide resolved
plugins/kubernetes-common/src/permissions.ts Outdated Show resolved Hide resolved
const app = express().use('/mountpath', proxy.createRequestHandler());
const app = express().use(
'/mountpath',
proxy.createRequestHandler({ permissionApi }),

Check failure

Code scanning / CodeQL

Missing rate limiting

This route handler performs [authorization](1), but is not rate-limited.
@vinzscam vinzscam self-requested a review February 13, 2023 21:33
@RubenV-dev RubenV-dev force-pushed the proxy-restrict branch 2 times, most recently from 6300864 to c08b209 Compare February 13, 2023 22:46
@github-actions
Copy link
Contributor

This PR has been automatically marked as stale because it has not had recent activity from the author. It will be closed if no further activity occurs. If the PR was closed and you want it re-opened, let us know and we'll re-open the PR so that you can continue the contribution!

@github-actions github-actions bot added the stale label Feb 21, 2023
@freben freben removed the stale label Feb 22, 2023
Copy link
Member

@Rugvip Rugvip left a comment

Choose a reason for hiding this comment

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

Awesome, thank you! 😁

Just a couple of small nits but basically ready to :shipit:

We'll hold of on merging this until after the next mainline release since it's fairly large, so after next Tuesday.

.changeset/eleven-bats-tease.md Outdated Show resolved Hide resolved
docs/features/kubernetes/proxy.md Outdated Show resolved Hide resolved
plugins/kubernetes-backend/src/service/KubernetesProxy.ts Outdated Show resolved Hide resolved
@Rugvip Rugvip added the merge-after-release This is a bit too scary to merge until after the next release label Mar 9, 2023
Signed-off-by: Ruben Vallejo <rvallejo@vmware.com>
Signed-off-by: Ruben Vallejo <rvallejo@vmware.com>
…ssion, reword changeset, add permission parameter to install doc

Signed-off-by: Ruben Vallejo <rvallejo@vmware.com>
… available to the plugin

Signed-off-by: Ruben Vallejo <rvallejo@vmware.com>
…ew changes to api-reports and changeset along with docs

Signed-off-by: Ruben Vallejo <rvallejo@vmware.com>
Signed-off-by: Ruben Vallejo <rvallejo@vmware.com>
@Rugvip Rugvip removed the merge-after-release This is a bit too scary to merge until after the next release label Mar 21, 2023
Copy link
Member

@Rugvip Rugvip left a comment

Choose a reason for hiding this comment

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

Nice! Let's :shipit: 😁

@Rugvip Rugvip merged commit 333fc6a into backstage:master Mar 21, 2023
@github-actions
Copy link
Contributor

Thank you for contributing to Backstage! The changes in this pull request will be part of the 1.13.0 release, scheduled for Tue, 18 Apr 2023.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants