Fix groups with OIDC connector

[konstl000]

What does this PR accomplish?

Bug Fix

closes #6435

Changes proposed by this PR:

Notes to reviewer:

Since Dex has the option InsecureEnableGroups set to false by default and concourse did not overwrite this, the OIDC connector ignores the groups coming from the upstream provider. This seems to be pretty bad for concourse, since its current RBAC model heavily relies on groups and many users (including myself) use those to manage access.
Adding an --oidc-enable-groups flag allows to override this behavior (if necessary) and thus to use groups again.

Release Note

• Adds an --oidc-disable-groups flag that disables fetching groups claims from an upstream OIDC provider. By default, the groups claim is fetched (as with previous version of Concourse)
• This was fixed in 6.7.4 as part of Fix OIDC connector not fetching groups claim #6448.

Contributor Checklist

• Followed Code of conduct, Contributing Guide & avoided Anti-patterns
• Signed all commits
• Added tests (Unit and/or Integration)
• Updated Documentation
• Added release note (Optional)

Reviewer Checklist

• Code reviewed
• Tests reviewed
• Documentation reviewed
• Release notes reviewed
• PR acceptance performed
• New config flags added? Ensure that they are added to the
  BOSH and
  Helm packaging; otherwise, ignored for
  the integration
  tests
  (for example, if they are Garden configs that are not displayed in the
  --help text).

[aoldershaw]

@xtremerui do we want this as an opt-in feature, or should it be opt-out instead (i.e. disable-groups instead of enable-groups? Making it opt-in means it'll be breaking (compared to 6.7.2)

[aoldershaw]

Or do we even need to provide the ability to opt-out?

[konstl000]

Just thought, it would make sense, since upstream dex does this ...

[xtremerui]

Good call. Making it opt-out is better for backward compatibility but at the same time opt-in flag makes user to aware that enabling groups is insecure.

Given existing concourse users of OIDC connectors still have a chance to add the flag when they upgrading Concourse, IMO keep it opt-in is more aligned to upstream DEX.

[aoldershaw]

I don't think enabling groups is actually insecure here. See dexidp/dex@21ab30d

There's been some discussion in dexidp/dex#1065 regarding what to do about refreshing groups. As it stands today dex doesn't update any of the claims on refresh (groups would just be another one). The main concern with enabling it is that group claims may change more frequently.

So the insecure behaviour is around using the OIDC connector with refresh tokens, since those are long-lived and groups can change, but that change won't be propagated. But Concourse doesn't support refresh tokens, so it's no different from any other connector (when your groups change, you have to log out and log back in).

Also, being more aligned with upstream Dex doesn't necessarily make sense here - using groups is pretty central to Concourse's auth system, whereas Dex is just meant to be a general purpose identity provider (whose downstream services may or may not care about groups - but in Concourse's case, groups is a pretty core concept for configuring auth)

[xtremerui]

@konstl000 ok thx!

[konstl000]

Now it is opt-out and is called --oidc-disable-groups

[aoldershaw]

That would be a breaking change too, at least for folks that are as lazy as me and just used to include groups in the openID scope

Ah sorry, I didn't see #6436 (comment) before I added my most recent comment - so just to be clear, in 6.7.2 and earlier, you didn't need to set --oidc-scopes to include groups - it picked them up by default?

[konstl000]

Yes, if the OIDC provider had groups in the openid scope, the earlier versions of Concourse would pick them up and use them without the --oidc-scopes flag. This is also how our production Concourse is configured at the moment.

[xtremerui]

Yes, if the OIDC provider had groups in the openid scope, the earlier versions of Concourse would pick them up and use them without the --oidc-scopes flag.

So it depends on how OIDC provider returns claims based on scopes. In @konstl000 case Keycloak is configured to return groups claims by one of the default scopes [openid, profile, email] so --oidc-scopes is not needed. I agree that --oidc-scopes and disable groups claim should then be decoupled to achieve maximum flexibility.

[taylorsilva]

@konstl000 FYI we're enabling 2FA for the Concourse org. We can see you don't have 2FA enabled. You'll be kicked from the Concourse org when we flip the switch in a few minutes. If you enable 2FA then you can re-join the Concourse org.

[konstl000]

@taylorsilva, I just activated the 2FA. What are the further steps?

[taylorsilva]

I actually don't think you'll need to do anything. You'll get added back automatically the next time the governance repo's github actions run https://github.com/concourse/governance/