oAuthSecret stored in plain text

[guydog28]

Describe the bug

oAuthClientSecret is being stored in plain text in the CheCluster resource. We deploy Che via helm and GitOps with ArgoCD. This means cluster state is stored in Git. We used ExternalSecrets to keep all sensitive data out of the code base, but this does not seem to be possible with Che.

Suggestion:
Modify networking.auth.oAuthSecret to first check in the namespace for a secret with the provided name, and a key of 'secret', and if it does not exist, assume the value is the plaintext secret. This keeps things working with backward compatibility but allows us to keep sensitive oauth client secrets out of our git repo.

Che version

7.85@latest

Steps to reproduce

No steps required.

Expected behavior

Provide a kubernetes secret name in place of a plaintext oauth client secret.

Runtime

Kubernetes (vanilla)

Screenshots

No response

Installation method

other (please specify in additional context)

Environment

Amazon

Eclipse Che Logs

No response

Additional context

We store the CheCluster resource and supporting resources in git, and use ArgoCD to keep them in sync with the cluster.

[tolusha]

Hello @guydog28
Make sense for me.

Suggestion:
Modify networking.auth.oAuthSecret to first check in the namespace for a secret with the provided name, and a key of 'secret', and if it does not exist, assume the value is the plaintext secret. This keeps things working with backward compatibility but allows us to keep sensitive oauth client secrets out of our git repo.

[ibuziuk]

Adding help-wanted label. @guydog28 Would you be interested in contributing this functionality to Eclipse Che?

[guydog28]

Adding help-wanted label. @guydog28 Would you be interested in contributing this functionality to Eclipse Che?

Possibly. If someone wants to point me in the right direction for eclipse's requirements for contributing and the section of the architecture to look at for this, I can possibly dig in a bit.

[tolusha]

I think we can read the secret here [1] like this:

secret := &corev1.Secret{}
exists, err := deploy.GetNamespacedObject(ctx, ctx.CheCluster.Spec.Networking.Auth.OAuthSecret, secret)

If secret exists, then read some key from it, otherwise use ctx.CheCluster.Spec.Networking.Auth.OAuthSecret as plain text

[1] https://github.com/eclipse-che/che-operator/blob/32974f029ee66275cf1fa4c49cc6e7ae2c621f23/pkg/deploy/gateway/oauth_proxy.go#L103

[guydog28]

I think we can read the secret here [1] like this:

secret := &corev1.Secret{}
exists, err := deploy.GetNamespacedObject(ctx, ctx.CheCluster.Spec.Networking.Auth.OAuthSecret, secret)

If secret exists, then read some key from it, otherwise use ctx.CheCluster.Spec.Networking.Auth.OAuthSecret as plain text

[1] https://github.com/eclipse-che/che-operator/blob/32974f029ee66275cf1fa4c49cc6e7ae2c621f23/pkg/deploy/gateway/oauth_proxy.go#L103

I agree that is probably the right spot since it isn't done the same way in openshift. Is this something your team would do or is it still preferred that I do it? I've only written minimal Go, so it might take me a bit to get a working environment going and wrap my head around it and add tests and such.

[guydog28]

I created this:

eclipse-che/che-operator@main...guydog28:che-operator:main

But I lack the background to create proper unit tests that test the cases where:

1. The secret exists and the key oAuthSecret does not exist on the secret, and
2. The secret exists and the key oAuthSecret does exist on the secret

Mainly - how to mock the calls to get the secret and the key in the cluster for the tests.

Any help on that would be appreciated and then I can submit a PR.

[tolusha]

@guydog28
Sounds good, could you create a PR ?
I will provide comments there.

[guydog28]

@tolusha eclipse-che/che-operator#1836 Created there.

[tolusha]

@guydog28
Thank you for contribution.

[guydog28]

@guydog28 Thank you for contribution.

You are very welcome. Thanks for the help!

[ibuziuk]

@guydog28 thank you for the contribution \o/
Adding the issue to the upstream Release Notes:

Previously, when deployed on Kubernetes, `oAuthClientSecret` was stored in plain text in the CheCluster resource.   That was not convenient for the GitOps approach when the cluster state is stored in Git and managed by ArgoCD. Starting from this release the values for oAuthSecret can be configured using ExternalSecrets to keep all sensitive data out of the code base.

will you be able to contribute additional docs to https://github.com/eclipse-che/che-docs?