feat: enable authservice integration #201
Conversation
There was a problem hiding this comment.
Could you also add a renovate packageRule to group the authservice PRs?
See the istio one as an example: https://github.com/defenseunicorns/uds-core/blob/authservice-pepr/renovate.json#L162-L167
| @@ -438,13 +438,14 @@ export interface Sso { | |||
| */ | |||
| description?: string; | |||
| /** | |||
| * Whether the SSO client is enabled | |||
| * Labels to match pods to automatically protect with authservice. Leave empty to disable | |||
There was a problem hiding this comment.
I think an empty selector of {} like: enableAuthserviceSelector: {} should enable Authservice for all Pods in a namespace, correct?
I'd like to include this in the docs and recommend it as a default. I'd rather people only write narrow selectors if they need to exclude some Pods.
And this should probably say "leave null to disable" instead of "empty", to disambiguate between {} which is actually all.
(I may have this wrong and Istio selector: { labelSelector: {} } and selector: nil may actually be different. I want to make sure we provide a way to let users do "all Pods in namespace")
There was a problem hiding this comment.
enableAuthserviceSelector: {} should enable authservice for all pods in the namespace, but I am curious to hear why that would be your recommendation for the default?
Does that lean into an assumption of there being a single pod/deployment in the namespace? If a user deployed an application with an internal API or database, how would we recommend they authenticate?
There was a problem hiding this comment.
Just that I'd encourage auth on all webservers that are reachable (in-cluster/SSRF and external) and don't already have auth. If you're careful to write good policies it's fine to exclude things, but it's a safe default to encourage I think.
I'd recommend docs something like:
Use
enableAuthserviceSelector: {}to add authservice to your app. If you need to exclude some Pods, edit it as needed and verify that the excluded Pods are not reachable externally or from other namespaces without authentication.
Doing auth on {} is also future proof to the app changing in the future and adding new Pods that don't match the old selector.
There was a problem hiding this comment.
(there's also the actual "zero trust" answer, but it only applies to custom developed apps: Encourage people to develop apps that don't access their backend unauthenticated. Have them pass the user's JWT from the frontend Pod to the backend/db Pod as auth. In this setup, you can reuse istio JWT authn/authz checking... but you actually don't want the CUSTOM authservice authz. Not sure if this is something we want to build support for, but it actually results in good design of custom apps which create a great audit log in isto access logs)
| ### Changes from the DoD Platform One Big Bang package | ||
|
|
||
| This package differs from the [DoD Platform One Big Bang package](https://repo1.dso.mil/big-bang/product/packages/authservice) in a few key ways: | ||
|
|
||
| * Leverages the [authservice-go](https://github.com/tetrateio/authservice-go) project which is the successor to the original [authservice](https://github.com/istio-ecosystem/authservice) project. |
There was a problem hiding this comment.
nit: BB has switched to 1.0.0 already - https://repo1.dso.mil/big-bang/product/packages/authservice/-/blob/main/chart/values.yaml?ref_type=heads#L62
There was a problem hiding this comment.
Updated to 1.0.1, which is the latest from that repo.
| repository: registry1.dso.mil/ironbank/istio-ecosystem/authservice-go | ||
| tag: "1.0.2" |
There was a problem hiding this comment.
We should probably point to standard authservice now as the tetrate go-fork is archived - https://github.com/tetrateio/authservice-go - all future updates should end up in the main repo.
There was a problem hiding this comment.
Updated to point to the non-archived repo.
src/authservice/chart/values.yaml
Outdated
| @@ -45,9 +45,9 @@ global: | |||
|
|
|||
| oidc: | |||
| # -- OpenID Connect hostname. Assumption of Keycloak based on URL construction | |||
| host: login.uds.dev | |||
| host: sso.uds.dev | |||
There was a problem hiding this comment.
Should use domain zarf var here.
## Description Improve / Add validations for package deployments and testing. Keycloak validations are captured on this [branch](https://github.com/defenseunicorns/uds-core/tree/authservice-pepr) / this [PR](defenseunicorns/uds-core#201). ## Related Issue Fixes # [109](defenseunicorns/uds-core#109) ## Type of change - [x] 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 - [x] Test, docs, adr added or updated as needed - [x] [Contributor Guide Steps](https://github.com/defenseunicorns/uds-template-capability/blob/main/CONTRIBUTING.md)(https://github.com/defenseunicorns/uds-template-capability/blob/main/CONTRIBUTING.md#submitting-a-pull-request) followed
Description
To test:
See https://github.com/defenseunicorns/uds-core/blob/authservice-pepr/docs/IDAM.md