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

feat: add service accounts options to sso #852

Merged
merged 17 commits into from
Oct 9, 2024
Merged

Conversation

ldgriswold
Copy link
Contributor

@ldgriswold ldgriswold commented Oct 2, 2024

Description

This enables support for service account roles in keycloak for client credentials type grants
...

Related Issue

Fixes #851

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Other (security config, docs update, etc)

Checklist before merging

@ldgriswold ldgriswold requested a review from a team as a code owner October 2, 2024 17:05
@ldgriswold
Copy link
Contributor Author

ldgriswold commented Oct 2, 2024

@ldgriswold
Copy link
Contributor Author

@ldgriswold
Copy link
Contributor Author

Pushed a fix due to a trick in the system:

apiVersion: uds.dev/v1alpha1
kind: Package
metadata:
  name: {{ include "wfapi.fullname" . }}
spec:
  sso:
    - name: wfapi-creds
      clientId: wfapi
      standardFlowEnabled: false
      publicClient: true
      serviceAccountsEnabled: true #TODO: Add this to UDS Core
      attributes:
        oauth2.device.authorization.grant.enabled: "true"

Was allowing a public client to be created, now pepr catches the error:

image

@bburky
Copy link
Member

bburky commented Oct 3, 2024

I need to test this, but I think this is PR is fine.

We (@rjferguson21 ?) can do a follow up PR to implement a hypothetical authz: or something that is the same as authserviceServiceSelector: but only Istio JWT authn/zuthz, no authservice. This field would sit beside these other options.

This PR should work today for grant=client_credentials auth to a (mission app) workload that does internal JWT validation. I really don't want to encourage apps to do their own JWT validation and crypto though, so adding authz: would let Istio do the JWT auth and more safely implement grant=client_credentials auth workload with automatic Istio JWT validation. I would argue Keycloak service accounts primarily make sense for out-of-cluster machine-to-machine clients connecting to an in-cluster workload.

Very similarly, we could support istio authz on users using OAuth Device Flow to connect to an in-cluster workload with the same authz: field (generates the same istio resources as above, just a different client config).

However, if you need to authenticate in-cluster workloads to each other, I would really suggest instead using Kubernetes service account, Istio mTLS and a principals: AuthorizationPolicy. I think the service account name is in the x-forwarded-client-cert header. Alternately, you could use something like Kubernetes projected service account tokens or GitLab CI JWTs. If any of these options are chosen, no changes to the Package CR need to be made, just deploy some AuthorizationPolicy resources from the Helm chart. I don't think Keycloak should be needed for two in-cluster workloads communicating.

Anyways, that's all TODO and can be done as a followup of this PR.

@bburky
Copy link
Member

bburky commented Oct 4, 2024

I did a bunch of testing today with @ldgriswold , and we realized that you don't get an aud audience claim by default, you need a mapper.

But if you're using a mapper anyway, you can customize the claim. If you pick the same audience as your authservice client, this lets your service accounts access authservice protected apps with no other configuration than defining a custom protocolMapper. This implements "grant=client_credentials auth to workload with automatic Istio JWT validation".

I updated the docs to include this and cleaned up some other code

docs/configuration/uds-operator.md Outdated Show resolved Hide resolved
docs/configuration/uds-operator.md Outdated Show resolved Hide resolved
docs/configuration/uds-operator.md Show resolved Hide resolved
Co-authored-by: Micah Nagel <micah.nagel@defenseunicorns.com>
ldgriswold and others added 2 commits October 9, 2024 11:20
Co-authored-by: Luke Griswold <150048380+ldgriswold@users.noreply.github.com>
@mjnagel mjnagel enabled auto-merge (squash) October 9, 2024 17:48
@mjnagel mjnagel merged commit 1029162 into main Oct 9, 2024
22 checks passed
@mjnagel mjnagel deleted the feature/kc-service-accounts branch October 9, 2024 18:10
mjnagel pushed a commit that referenced this pull request Oct 11, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.29.0](v0.28.0...v0.29.0)
(2024-10-11)


### Features

* add base and identity layers
([#853](#853))
([b3f532a](b3f532a))
* add logging functional layer
([#861](#861))
([c1a67b9](c1a67b9))
* add metrics-server functional layer
([#865](#865))
([290367a](290367a))
* add monitoring layer
([#872](#872))
([5ecb040](5ecb040))
* add nightly testing for rke2
([#808](#808))
([c401419](c401419))
* add service accounts options to sso
([#852](#852))
([1029162](1029162))
* backup and restore layer, ui layer, runtime security layer
([#862](#862))
([b1d8015](b1d8015))
* grafana-ha
([#838](#838))
([d532d76](d532d76))


### Bug Fixes

* broken readme link
([#899](#899))
([6e47b11](6e47b11))
* **ci:** switch to larger runners to resolve ci disk space issues
([#882](#882))
([1af0401](1af0401))
* snapshot ci version modification and tasks for publish
([#877](#877))
([f01e5bd](f01e5bd))
* support for anywhere network policies in cilium
([#884](#884))
([5df0737](5df0737))


### Miscellaneous

* cleanup license parsing for github
([#881](#881))
([43c98ce](43c98ce))
* **deps:** update chainctl action to v0.2.3
([#864](#864))
([d782b59](d782b59))
* **deps:** update checkout action to v4.2.0
([#825](#825))
([29d1c98](29d1c98))
* **deps:** update dependency defenseunicorns/lula to v0.8.0
([#841](#841))
([fe36150](fe36150))
* **deps:** update githubactions
([#866](#866))
([44f8ea5](44f8ea5))
* **deps:** update grafana to 11.2.1
([#836](#836))
([11383c1](11383c1))
* **deps:** update grafana to v11.2.2
([#867](#867))
([06ed2c3](06ed2c3))
* **deps:** update loki nginx image to v1.27.2
([#894](#894))
([df7d427](df7d427))
* **deps:** update loki to v3.2.0
([#791](#791))
([d3c60b5](d3c60b5))
* **deps:** update metrics-server chart to v3.12.2
([#873](#873))
([e2e61ce](e2e61ce))
* **deps:** update pepr to v0.37.1
([#843](#843))
([68abcb2](68abcb2))
* **deps:** update pepr to v0.37.2
([#850](#850))
([b51f659](b51f659))
* **deps:** update prometheus operator to 0.77.1
([#819](#819))
([0864b33](0864b33))
* **deps:** update prometheus-stack
([#855](#855))
([c791c24](c791c24))
* **deps:** update prometheus-stack helm-charts to v64.0.0
([#849](#849))
([50a2588](50a2588))
* **deps:** update runtime to v0.6.0
([#897](#897))
([89ae6e2](89ae6e2))
* **deps:** update support-deps
([#890](#890))
([26ea612](26ea612))
* **deps:** update test-infra
([#875](#875))
([583f07c](583f07c))
* **deps:** update test-infra to v6.9.0
([#848](#848))
([ef9d317](ef9d317))
* **deps:** update uds to v0.17.0
([#859](#859))
([1489fef](1489fef))
* **deps:** update zarf to v0.41.0
([#857](#857))
([a390c3d](a390c3d))
* **docs:** update doc structure for site refresh
([#895](#895))
([1946a9a](1946a9a))
* fix broken link in docs
([#845](#845))
([3078a5b](3078a5b))
* fix license header references
([#901](#901))
([cf38b82](cf38b82))
* handle upgrade path for functional layers, add doc for usage
([#896](#896))
([70d6b1b](70d6b1b))
* regroup 'support dependencies' in renovate config
([#885](#885))
([640d859](640d859))
* update license
([#878](#878))
([b086170](b086170))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
docandrew pushed a commit that referenced this pull request Oct 17, 2024
## Description
This enables support for service account roles in keycloak for client
credentials type grants
...

## Related Issue

Fixes #851

## Type of change

- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Other (security config, docs update, etc)

## Checklist before merging

- [x] Test, docs, adr added or updated as needed
- [x] [Contributor
Guide](https://github.com/defenseunicorns/uds-template-capability/blob/main/CONTRIBUTING.md)
followed

---------

Co-authored-by: Blake Burkhart <blake@defenseunicorns.com>
Co-authored-by: Micah Nagel <micah.nagel@defenseunicorns.com>
docandrew pushed a commit that referenced this pull request Oct 17, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.29.0](v0.28.0...v0.29.0)
(2024-10-11)


### Features

* add base and identity layers
([#853](#853))
([b3f532a](b3f532a))
* add logging functional layer
([#861](#861))
([c1a67b9](c1a67b9))
* add metrics-server functional layer
([#865](#865))
([290367a](290367a))
* add monitoring layer
([#872](#872))
([5ecb040](5ecb040))
* add nightly testing for rke2
([#808](#808))
([c401419](c401419))
* add service accounts options to sso
([#852](#852))
([1029162](1029162))
* backup and restore layer, ui layer, runtime security layer
([#862](#862))
([b1d8015](b1d8015))
* grafana-ha
([#838](#838))
([d532d76](d532d76))


### Bug Fixes

* broken readme link
([#899](#899))
([6e47b11](6e47b11))
* **ci:** switch to larger runners to resolve ci disk space issues
([#882](#882))
([1af0401](1af0401))
* snapshot ci version modification and tasks for publish
([#877](#877))
([f01e5bd](f01e5bd))
* support for anywhere network policies in cilium
([#884](#884))
([5df0737](5df0737))


### Miscellaneous

* cleanup license parsing for github
([#881](#881))
([43c98ce](43c98ce))
* **deps:** update chainctl action to v0.2.3
([#864](#864))
([d782b59](d782b59))
* **deps:** update checkout action to v4.2.0
([#825](#825))
([29d1c98](29d1c98))
* **deps:** update dependency defenseunicorns/lula to v0.8.0
([#841](#841))
([fe36150](fe36150))
* **deps:** update githubactions
([#866](#866))
([44f8ea5](44f8ea5))
* **deps:** update grafana to 11.2.1
([#836](#836))
([11383c1](11383c1))
* **deps:** update grafana to v11.2.2
([#867](#867))
([06ed2c3](06ed2c3))
* **deps:** update loki nginx image to v1.27.2
([#894](#894))
([df7d427](df7d427))
* **deps:** update loki to v3.2.0
([#791](#791))
([d3c60b5](d3c60b5))
* **deps:** update metrics-server chart to v3.12.2
([#873](#873))
([e2e61ce](e2e61ce))
* **deps:** update pepr to v0.37.1
([#843](#843))
([68abcb2](68abcb2))
* **deps:** update pepr to v0.37.2
([#850](#850))
([b51f659](b51f659))
* **deps:** update prometheus operator to 0.77.1
([#819](#819))
([0864b33](0864b33))
* **deps:** update prometheus-stack
([#855](#855))
([c791c24](c791c24))
* **deps:** update prometheus-stack helm-charts to v64.0.0
([#849](#849))
([50a2588](50a2588))
* **deps:** update runtime to v0.6.0
([#897](#897))
([89ae6e2](89ae6e2))
* **deps:** update support-deps
([#890](#890))
([26ea612](26ea612))
* **deps:** update test-infra
([#875](#875))
([583f07c](583f07c))
* **deps:** update test-infra to v6.9.0
([#848](#848))
([ef9d317](ef9d317))
* **deps:** update uds to v0.17.0
([#859](#859))
([1489fef](1489fef))
* **deps:** update zarf to v0.41.0
([#857](#857))
([a390c3d](a390c3d))
* **docs:** update doc structure for site refresh
([#895](#895))
([1946a9a](1946a9a))
* fix broken link in docs
([#845](#845))
([3078a5b](3078a5b))
* fix license header references
([#901](#901))
([cf38b82](cf38b82))
* handle upgrade path for functional layers, add doc for usage
([#896](#896))
([70d6b1b](70d6b1b))
* regroup 'support dependencies' in renovate config
([#885](#885))
([640d859](640d859))
* update license
([#878](#878))
([b086170](b086170))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

Keycloak support for client_credentials grant via Service Account Roles
4 participants