-
Notifications
You must be signed in to change notification settings - Fork 20
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(auth): deploy openshift auth proxy #799
feat(auth): deploy openshift auth proxy #799
Conversation
Currently fails like this:
|
Works now in the most basic case: openshift-oauth-proxy is always deployed, never oauth2_proxy. Basic auth via htpasswd is not yet implemented. SubjectAccessReview and TokenReview (therefore Bearer auth) are not yet implemented. This is deploying the upstream proxy image which has the URL decoding/redirecting bug, so many features don't work and it's tough to test much but I suspect that report generation and "View in Grafana" probably aren't working yet, either. Tests are failing so must currently be skipped during build. |
I guess the (Cluster)Role applied to the Pod, or the authproxy container, needs to gain the permission to create SubjectAccessReviews. |
It looks to me like it should already have such permissions: cryostat-operator/config/rbac/role.yaml Line 82 in 53d2b73
@ebaron what am I missing here? |
@ebaron this is still quite rough and incomplete, but I think it's a reasonable starting point for the OpenShift OAuth Proxy. The TLS configuration stuff is a little beyond me right now, but after this I'll try configuring the Basic htpasswd auth on this proxy container as a follow-up PR. Once that's done I can try switching out between the OpenShift OAuth Proxy and OAuth2 Proxy depending on where the Operator is running. |
Is it working after adding it to the cryostat_role.yaml as well? |
After this commit the auth proxy stopped complaining like that in its logs and the behaviour started doing what I expected, ie same as in cryostatio/cryostat-helm#132 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed this line in the OAuth Proxy README:
- Requires no additional permissions to be granted for the proxy service account
It seems like if don't use the delegate URLs option, then we somehow don't need SAR permissions on the service account. This would be great since we could likely remove the entire Cluster Role and Cluster Role Binding for Cryostat's service account. Perhaps we could accomplish the same thing by using the --bypass-auth-for
option instead like this:
diff --git a/internal/controllers/common/resource_definitions/resource_definitions.go b/internal/controllers/common/resource_definitions/resource_definitions.go
index e05c691d..55e2f899 100644
--- a/internal/controllers/common/resource_definitions/resource_definitions.go
+++ b/internal/controllers/common/resource_definitions/resource_definitions.go
@@ -632,12 +632,8 @@ func NewOpenShiftAuthProxyContainer(cr *model.CryostatInstance, specs *ServiceSp
`--openshift-sar=[{"group":"","name":"","namespace":"%s","resource":"pods","subresource":"exec","verb":"create","version":""}]`,
cr.InstallNamespace,
),
- fmt.Sprintf(
- `--openshift-delegate-urls={"/health":{},"/api":{"group":"","name":"","namespace":"%s","resource":"pods","subresource":"exec","verb":"create","version":""}, "/storage":{"group":"","name":"","namespace":"%s","resource":"pods","subresource":"exec","verb":"create","version":""}, "/grafana":{"group":"","name":"","namespace":"%s","resource":"pods","subresource":"exec","verb":"create","version":""} }`,
- cr.InstallNamespace,
- cr.InstallNamespace,
- cr.InstallNamespace,
- ),
+
+ `--bypass-auth-for=^/health$`,
"--proxy-prefix=/oauth2",
}
// if tls != nil {
I tried it in OpenShift and it seemed to work. Does it work for you too?
I can test it, but from what I understand from the proxy's docs, that config flag is required for the proxy to handle Bearer authorization headers, which we need for our Agent. |
Tested it and it does look like this flag is required for the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understood on the agent use-case. It wasn't immediately clear to me what the delegate URLs option was for. I'm still not totally clear how they get away without the SAR permission in the normal case, but this seems like our only option.
We could use an entry here for RELATED_IMAGE_OPENSHIFT_OAUTH_PROXY
: https://github.com/cryostatio/cryostat-operator/blob/main/hack/image_tag_patch.yaml.in
Then we'd need another make bundle
.
I think in the normal interactive case the proxy doesn't need the SAR permission because it is only acting as a redirector to send the client to the OpenShift cluster's internal SSO provider. That's what performs the SAR, and if it passes, issues a new restricted token which the client stores as a cookie. Then it redirects the user back to the authproxy, which now receives the cookie, passes it back to the internal SSO provider which approves it, and then the authproxy begins forwarding the user's request to the upstream service (Cryostat container). |
As for why the TokenReview for |
Welcome to Cryostat! 👋
Before contributing, make sure you have:
main
branch[chore, ci, docs, feat, fix, test]
git commit -S -m "YOUR_COMMIT_MESSAGE"
Related to #710
Based on (merged in) #800
Description of the change:
Deploys
openshift-oauth-proxy
in the Pod and points the existing Service (and Route or Ingress) at it, rather than at Cryostat directly. The auth proxy is always configured to require one particular set of Roles/permissions for both interactive users as well as programmatic clients, and always delegates to OpenShift SSO. In this PR it cannot be disabled and it cannot be augmented with htpasswd Basic auth. It also does not handle TLS.Motivation for the change:
In Cryostat 3.0 there should always be an auth proxy placed in front of the operand containers, since in the new architecture the authz responsibility is always delegated to such an auth proxy and never performed by the Cryostat container. The OpenShift OAuth Proxy with SubjectAccessReview and TokenReview configuration in this manner is very close to what Cryostat <= 2.4 did and generally allows users and clients to authenticate and access the application and API with the same permissions as before.
How to manually test:
OPERATOR_IMG=quay.io/andrewazores/cryostat-operator:3.0.0-openshift-oauth-proxy-3 DISABLE_SERVICE_TLS=true DEPLOY_NAMESPACE=$(oc project -q) make install deploy
enableCertManager: false
developer
. You should see a "permission denied" page. Click "Log in" again and this time log in with a privileged user, ex.kubeadmin
. You should get in to the application and be able to perform some basic functionality ex view Security certs/credentials lists (many still broken due toHTTP 301
redirection responses mangleLocation
header openshift/oauth-proxy#272)http --follow cryostat-sample-cryostat3-helm.apps-crc.testing/api/v3/tls/certs
produces a login page response, whilehttp --follow --auth-type=bearer --auth=$(oc whoami -t) cryostat-sample-cryostat3-helm.apps-crc.testing/api/v3/tls/certs
returns the list response (assuming you are logged in withoc
as a privileged user).http --follow --auth-type=bearer --auth=FAKE$(oc whoami -t) cryostat-sample-cryostat3-helm.apps-crc.testing/api/v3/tls/certs
(a bad/corrupted token) should result in the login page response.