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

Integrate with oathkeeper via forward-auth interface #275

Merged

Conversation

natalian98
Copy link
Contributor

@natalian98 natalian98 commented Oct 24, 2023

Issue

This PR adds integration with Oathkeeper - the policy decision point in Identity and Access Proxy architecture, by supporting a new forwardAuth middleware.

In order to integrate Traefik into IAP, it must update routing rules for the applications that requested to be protected with the ForwardAuth middleware. Its definition must include headers and oathkeeper's decisions address.

Solution

This PR integrates traefik with oathkeeper with the use of forward-auth interface.
Oathkeeper provides Traefik with its decisions address, allowed return headers and application names that requested the IAP protection. This information is processed by Oathkeeper from auth-proxy relation.
Traefik updates the relation databag with app names it provides ingress for, so that Oathkeeper can compare the app names and evaluate if they can be protected using forwardAuth.
If an app is related with ingress-per-app/leader/unit, Traefik will update its routes and add the middleware.

Context

Traefik ForwardAuth upstream docs
IAP interfaces design

Testing Instructions

  1. Pack and deploy the charms.
    As forward-auth relies on auth-proxy relation data, I'm using zinc-k8s from this branch as a dummy example to test that the relation data is then transferred to traefik.
    Oathkeeper is built from this branch.
juju deploy ./traefik-k8s_ubuntu-20.04-amd64.charm traefik-ingress --trust --resource traefik-image=ghcr.io/canonical/traefik:2.10.4
juju deploy ./oathkeeper*.charm --resource oci-image=$(yq eval '.resources.oci-image.upstream-source' metadata.yaml) --trust
juju deploy ./zinc-k8s*.charm --resource zinc-image=ghcr.io/jnsgruk/zinc:0.4.9 --trust
  1. Add integrations to oathkeeper:
juju relate oathkeeper zinc-k8s:auth-proxy
juju relate oathkeeper traefik-ingress:experimental-forward-auth

At this point, if zinc was not related to ingress, oathkeeper will get blocked with message zinc-k8s is not related via ingress.
After you relate zinc with ingress, oathkeeper will get active:

juju relate zinc-k8s traefik-ingress
  1. Check the traefik routes:
juju ssh --container traefik traefik-ingress/0 bash
cd /opt/traefik/juju/

Zinc route should define and reference the middleware:

# cat juju_ingress_ingress_6_zinc-k8s.yaml
http:
  middlewares:
    juju-sidecar-forward-auth-oathkeeper:
      forwardAuth:
        address: https://oathkeeper.test21.svc.cluster.local:4456/decisions
        authResponseHeaders:
        - X-User
    juju-sidecar-noprefix-test21-zinc-k8s:
      stripPrefix:
        forceSlash: false
        prefixes:
        - /test21-zinc-k8s
  routers:
    juju-test21-zinc-k8s-router:
      entryPoints:
      - web
      middlewares:
      - juju-sidecar-forward-auth-oathkeeper
      - juju-sidecar-noprefix-test21-zinc-k8s
      rule: PathPrefix(`/test21-zinc-k8s`)
      service: juju-test21-zinc-k8s-service
    juju-test21-zinc-k8s-router-tls:
      entryPoints:
      - websecure
      middlewares:
      - juju-sidecar-forward-auth-oathkeeper
      - juju-sidecar-noprefix-test21-zinc-k8s
      rule: PathPrefix(`/test21-zinc-k8s`)
      service: juju-test21-zinc-k8s-service
      tls: {}
  services:
    juju-test21-zinc-k8s-service:
      loadBalancer:
        servers:
        - url: http://zinc-k8s.test21.svc.cluster.local:4080

Release Notes

  • add forward-auth integration with oathkeeper

@PietroPasotti
Copy link
Contributor

Thanks Natalia, will review when I have time after/during the sprint (or probably next week we can go through it together if you like).

@PietroPasotti
Copy link
Contributor

In order to make it clear that the feature is untested and might be unstable/experimental, we should rename the endpoint to experimental_forward_auth. The description should make it clear what it does and that it's insecure and that you're supposed to have traefik enabled as TLS termination endpoints.

For now you'll have to do juju relate traefik:experimental_forward_auth oathkeeper, and then when the feature becomes stable and oathkeeper has TLS, we can rename the interface to forward_auth.

Secondly, we will only enable the experimental forward auth feature behind a feature flag.
You'll need to juju config traefik enable_experimental_forward_auth=True in order to use it.
All code blocks introduced by this PR should be behind a:
if self.config["enable_experimental_forward_auth"]: guard so that we can easily filter out any buggy behaviour due to this feature.

If you relate traefik to oathkeeper but enable_experimental_forward_auth is False, Traefik will not enable the middleware.

@natalian98
Copy link
Contributor Author

In order to make it clear that the feature is untested and might be unstable/experimental, we should rename the endpoint to experimental_forward_auth. The description should make it clear what it does and that it's insecure and that you're supposed to have traefik enabled as TLS termination endpoints.

For now you'll have to do juju relate traefik:experimental_forward_auth oathkeeper, and then when the feature becomes stable and oathkeeper has TLS, we can rename the interface to forward_auth.

Secondly, we will only enable the experimental forward auth feature behind a feature flag. You'll need to juju config traefik enable_experimental_forward_auth=True in order to use it. All code blocks introduced by this PR should be behind a: if self.config["enable_experimental_forward_auth"]: guard so that we can easily filter out any buggy behaviour due to this feature.

If you relate traefik to oathkeeper but enable_experimental_forward_auth is False, Traefik will not enable the middleware.

Wouldn't it be enough to have the feature flag as a juju config? I'd prefer to avoid renaming the relation.

metadata.yaml Show resolved Hide resolved
metadata.yaml Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
tests/unit/test_charm.py Show resolved Hide resolved
@PietroPasotti
Copy link
Contributor

In order to make it clear that the feature is untested and might be unstable/experimental, we should rename the endpoint to experimental_forward_auth. The description should make it clear what it does and that it's insecure and that you're supposed to have traefik enabled as TLS termination endpoints.
For now you'll have to do juju relate traefik:experimental_forward_auth oathkeeper, and then when the feature becomes stable and oathkeeper has TLS, we can rename the interface to forward_auth.
Secondly, we will only enable the experimental forward auth feature behind a feature flag. You'll need to juju config traefik enable_experimental_forward_auth=True in order to use it. All code blocks introduced by this PR should be behind a: if self.config["enable_experimental_forward_auth"]: guard so that we can easily filter out any buggy behaviour due to this feature.
If you relate traefik to oathkeeper but enable_experimental_forward_auth is False, Traefik will not enable the middleware.

Wouldn't it be enough to have the feature flag as a juju config? I'd prefer to avoid renaming the relation.

We don't have to rename the interface, only the endpoint. That seems like a minor change.
We want to avoid the impression that traefik offers stable support for forward_auth. A config-toggled feature flag is a bit hidden: someone might skip reading the config, deploy traefik, relate forward-auth, and be surprised when it doesn't work.

metadata.yaml Outdated Show resolved Hide resolved
lib/charms/oathkeeper/v0/forward_auth.py Outdated Show resolved Hide resolved
lib/charms/oathkeeper/v0/forward_auth.py Outdated Show resolved Hide resolved
lib/charms/oathkeeper/v0/forward_auth.py Outdated Show resolved Hide resolved
lib/charms/oathkeeper/v0/forward_auth.py Outdated Show resolved Hide resolved
lib/charms/oathkeeper/v0/forward_auth.py Show resolved Hide resolved
lib/charms/oathkeeper/v0/forward_auth.py Show resolved Hide resolved
lib/charms/oathkeeper/v0/forward_auth.py Outdated Show resolved Hide resolved
lib/charms/oathkeeper/v0/forward_auth.py Outdated Show resolved Hide resolved
lib/charms/oathkeeper/v0/forward_auth.py Outdated Show resolved Hide resolved
lib/charms/oathkeeper/v0/forward_auth.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
@simskij simskij requested a review from sed-i November 28, 2023 15:27
@natalian98 natalian98 force-pushed the IAM-500-forward-auth-relation-implementation branch from 915e008 to 9d3bbc4 Compare November 29, 2023 13:54
Copy link
Contributor

@PietroPasotti PietroPasotti left a comment

Choose a reason for hiding this comment

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

A couple of points regarding the API changes made to the Traefik class, but the rest looks good to me!

src/charm.py Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
@sed-i
Copy link
Contributor

sed-i commented Dec 4, 2023

Tested successfully with the bundle below.

$ juju run trfk/0 show-proxied-endpoints
proxied-endpoints: '{"zinc": {"url": "http://10.43.8.206/auth2-zinc"}}'

$ curl http://10.43.8.206/auth2-zinc/version
{"version":"'0.4.9'","build":"0","commit_hash":"'249a843897ade8ca3f7e80ac6036b8ac0a27154a'","branch":"0","build_date":"'2023-09-13_09:50:04AM-GMT'"}%

$ curl http://10.43.8.206/auth2-zinc/versionn
{"error":{"code":401,"status":"Unauthorized","message":"The request could not be authorized"}}

@natalian98 do we have issues open to capture the gaps currently addressed by:

  • config option: enable_experimental_forward_auth: true
  • config option: dev: true
  • relation: trfk:experimental-forward-auth

bundle: kubernetes
applications:
  ok:
    # From https://github.com/canonical/oathkeeper-operator
    # 1e23771 - (HEAD -> address-forward-auth-comments, origin/address-forward-auth-comments) refactor
    charm: ./oathkeeper-operator/oathkeeper_ubuntu-22.04-amd64.charm
    scale: 1
    constraints: arch=amd64
    trust: true
    resources:
      oci-image: "ghcr.io/canonical/oathkeeper:0.40.6"
    options:
      # until tls is impl'd dev mode would resort to http decisions url
      dev: true
  trfk:
    # From https://github.com/canonical/traefik-k8s-operator/pull/275
    # cc35a85 - (HEAD -> IAM-500-forward-auth-relation-implementation, natalian98/IAM-500-forward-auth-relation-implementation) refactor: add experimental_forward_auth_enabled as an init arg
    charm: ./traefik-k8s-operator/traefik-k8s_ubuntu-20.04-amd64.charm
    series: focal
    scale: 1
    constraints: arch=amd64
    storage:
      configurations: kubernetes,1,1024M
    trust: true
    resources:
      traefik-image: "docker.io/ubuntu/traefik:2-22.04"
    options:
      enable_experimental_forward_auth: true
  zinc:
    # From https://github.com/natalian98/zinc-k8s-operator
    # 0c19865 - (HEAD -> test-proxy-interfaces, origin/test-proxy-interfaces) fix: auth-proxy
    charm: ./zinc-k8s-operator/zinc-k8s_ubuntu-22.04-amd64.charm
    scale: 1
    constraints: arch=amd64
    storage:
      data: kubernetes,1,1024M
    trust: true
    resources:
      zinc-image: "ghcr.io/jnsgruk/zinc:0.4.9"
relations:
- - ok:forward-auth
  - trfk:experimental-forward-auth
- - ok:auth-proxy
  - zinc:auth-proxy
- - zinc:ingress
  - trfk:ingress

Copy link
Contributor

@sed-i sed-i left a comment

Choose a reason for hiding this comment

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

Thanks @natalian98, this is looking great.
Just a few final questions mostly.

src/charm.py Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
tests/integration/test_forward_auth.py Show resolved Hide resolved
tests/integration/test_forward_auth.py Show resolved Hide resolved
@natalian98 natalian98 requested a review from sed-i December 5, 2023 09:34
@natalian98 natalian98 force-pushed the IAM-500-forward-auth-relation-implementation branch from e149762 to e50fc27 Compare December 5, 2023 11:04
Copy link
Contributor

@sed-i sed-i left a comment

Choose a reason for hiding this comment

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

Well done @natalian98, thank you for the stellar patience and persistence!

@sed-i sed-i merged commit 01bd1ce into canonical:main Dec 5, 2023
12 checks passed
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.

5 participants