akuity / kargo

Application lifecycle orchestration
https://kargo.akuity.io/
Apache License 2.0
1.59k stars 137 forks source link

OIDC Claims not mapped correctly #2717

Open YgorCastor opened 1 week ago

YgorCastor commented 1 week ago

Checklist

Description

Starting at 0.9.0, looks like the user claims are not being mapped correctly to Service Account permissions.

Steps to Reproduce

  1. Create a OIDC connection through DEX
  2. Add a claim to the viewer/admin groups
  3. Login

Version

0.9.0

Logs

kargo-api

time="2024-10-10T12:08:14Z" level=error msg="finished unary call" connect.code=permission_denied connect.duration=112.6313ms connect.method=ListProjects connect.service=akuity.io.kargo.service.v1alpha1.KargoService connect.start_time="2024-10-10T12:08:14Z" error="permission_denied: projects.kargo.akuity.io is forbidden: list is not permitted"

kargo-dex

time="2024-10-10T12:33:14Z" level=info msg="login successful: connector \"onelogin\", username=\"Ygor Castor\", preferred_username=\"ygor.castor\", email=\"ygor.castor@xxx.xxx\"

Resources

service-account

apiVersion: v1
kind: ServiceAccount
metadata:
  annotations:
    rbac.kargo.akuity.io/claim.emails: ygor.castor@xxx.xxx
  labels:
    app.kubernetes.io/instance: cd
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/name: kargo
    app.kubernetes.io/version: v0.9.0
    helm.sh/chart: kargo-0.9.0
  name: kargo-admin
  namespace: gitops
krancour commented 1 week ago

emails is not a valid claim.

rbac.kargo.akuity.io/claim.emails should be rbac.kargo.akuity.io/claim.email

YgorCastor commented 1 week ago

emails is not a valid claim.

rbac.kargo.akuity.io/claim.emails should be rbac.kargo.akuity.io/claim.email

With claim.emails is working as expected, however looks like groups are not working:

apiVersion: v1
kind: ServiceAccount
metadata:
  annotations:
    rbac.kargo.akuity.io/claim.groups: APP_1L_Access-GitOps-Entertainment-Users

This do not work on 0.9.0

apiVersion: v1
kind: ServiceAccount
metadata:
  annotations:
    rbac.kargo.akuity.io/groups: APP_1L_Access-GitOps-Entertainment-Users

This works on 0.8.8

@krancour

krancour commented 1 week ago

Apologies... I zeroed right in on the rbac.kargo.akuity.io/claim.emails: ygor.castor@xxx.xxx in the ServiceAccount you pasted (which is wrong) and overlooked your steps to reproduce.

Now that I see this is groups-related, I can take a guess that you're not using Dex and I believe you may have run into this:

https://github.com/akuity/kargo/issues/2675 https://github.com/akuity/kargo/pull/2702

krancour commented 1 week ago

If your issue turned out to be what I was guessing it was, then v0.9.1 should solve your problem.

If you are possibly able to confirm this, that would be excellent!

mmclane commented 4 days ago

I am using Dex,

I just upgrade to v0.9.1 and I am seeing the same error message and I can no longer see my test pipeline (I only have one at the moment).

I see this in the logs of the kargo-api pod.

time="2024-10-16T13:10:48Z" level=error msg="finished unary call" connect.code=permission_denied connect.duration="864.605µs" connect.method=ListProjects connect.service=akuity.io.kargo.service.v1alpha1.KargoService connect.start_time="2024-10-16T13:10:48Z" error="permission_denied: projects.kargo.akuity.io is forbidden: list is not permitted"

Note, I don't get this error when I login with the Admin account, only when I login with my Google Account through Dex.

krancour commented 4 days ago

@YgorCastor @mmclane new guess...

I fear we may have failed to document a breaking change in the chart.

What once looked like this:

admins:
      ## @param api.oidc.admins.subs Subjects having any of these values in their sub claim will automatically be Kargo admins.
      subs: []
      ## @param api.oidc.admins.emails Subjects having any of these values in their email claim will automatically be Kargo admins.
      emails: []
      ## @param api.oidc.admins.groups Subjects having any of these values in their groups claim will automatically be Kargo admins.
      groups: []

    viewers:
      ## @param api.oidc.viewers.subs Subjects having any of these values in their sub claim will automatically receive read-only access to all Kargo resources.
      subs: []
      ## @param api.oidc.viewers.emails Subjects having any of these values in their email claim will automatically receive read-only access to all Kargo resources.
      emails: []
      ## @param api.oidc.viewers.groups Subjects having any of these values in their groups claim will automatically receive read-only access to all Kargo resources.
      groups: []

Now looks like this:

    admins:
      ## @param api.oidc.admins.claims Subjects having any of these claims will automatically be Kargo admins.
      claims: {}
        # sub:
        # - alice
        # - bob
        # email:
        # - alice@example.com
        # - bob@examples.com
        # groups:
        # - kargo-admin

    viewers:
      ## @param api.oidc.viewers.claims Subjects having any of these claims will automatically receive read-only access to all Kargo resources.
      claims: {}
        # sub:
        # - alice
        # - bob
        # email:
        # - alice@example.com
        # - bob@examples.com
        # groups:
        # - kargo-viewer

Perhaps your own values file(s) are using the old structure and not the newer one?

mmclane commented 4 days ago

@YgorCastor @mmclane new guess...

I fear we may have failed to document a breaking change in the chart.

What once looked like this:

admins:
      ## @param api.oidc.admins.subs Subjects having any of these values in their sub claim will automatically be Kargo admins.
      subs: []
      ## @param api.oidc.admins.emails Subjects having any of these values in their email claim will automatically be Kargo admins.
      emails: []
      ## @param api.oidc.admins.groups Subjects having any of these values in their groups claim will automatically be Kargo admins.
      groups: []

    viewers:
      ## @param api.oidc.viewers.subs Subjects having any of these values in their sub claim will automatically receive read-only access to all Kargo resources.
      subs: []
      ## @param api.oidc.viewers.emails Subjects having any of these values in their email claim will automatically receive read-only access to all Kargo resources.
      emails: []
      ## @param api.oidc.viewers.groups Subjects having any of these values in their groups claim will automatically receive read-only access to all Kargo resources.
      groups: []

Now looks like this:

    admins:
      ## @param api.oidc.admins.claims Subjects having any of these claims will automatically be Kargo admins.
      claims: {}
        # sub:
        # - alice
        # - bob
        # email:
        # - alice@example.com
        # - bob@examples.com
        # groups:
        # - kargo-admin

    viewers:
      ## @param api.oidc.viewers.claims Subjects having any of these claims will automatically receive read-only access to all Kargo resources.
      claims: {}
        # sub:
        # - alice
        # - bob
        # email:
        # - alice@example.com
        # - bob@examples.com
        # groups:
        # - kargo-viewer

Perhaps your own values file(s) are using the old structure and not the newer one?

This was EXACTLY what my problem was. I just fixed it and was coming to share that when I saw your reply. I just had to add claims: and tab over groups.

krancour commented 4 days ago

Glad we nailed it.

I'll update the v0.9.0 release notes with this.

YgorCastor commented 3 hours ago

Yes! it works ok now!