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

Use hashFromEnv in dex component for static user passwords #2229

Merged
merged 1 commit into from
Dec 26, 2023

Conversation

mvoitko
Copy link
Contributor

@mvoitko mvoitko commented Jun 29, 2022

Which issue is resolved by this Pull Request:
Resolves #

Description of your changes:

Checklist:

  • Unit tests pass:
    Make sure you have installed kustomize == 3.2.1
    1. make generate-changed-only
    2. make test

@mvoitko mvoitko force-pushed the fix/use-hashFromEnv-in-dex branch from 12595f1 to 14c5e1c Compare June 29, 2022 11:06
@mvoitko
Copy link
Contributor Author

mvoitko commented Jun 29, 2022

@krishnadurai could you please review?

@google-oss-prow
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: krishnadurai, mvoitko

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mvoitko
Copy link
Contributor Author

mvoitko commented Jul 26, 2022

@krishnadurai @yanniszark Could you please add the lgtm label so I merge the PR?

@umka1332
Copy link

Hi @mvoitko, can you please explain the purpose of this change.

@mvoitko
Copy link
Contributor Author

mvoitko commented Aug 14, 2022

@umka1332 The goal is to be a little bit more secure and not to store passwords in plain text in the source code. Also there was a comment

@juliusvonkohout
Copy link
Member

Please put this on the agenda of the next manifest wg meeting in two weeks and join us for a discussion

@mvoitko
Copy link
Contributor Author

mvoitko commented Sep 1, 2023

Please put this on the agenda of the next manifest wg meeting in two weeks and join us for a discussion

I'm sorry, I don't know how to do this. Could you please help with the link to the agenda and the meeting?

@juliusvonkohout
Copy link
Member

Yes of course. https://www.kubeflow.org/docs/about/community/#kubeflow-community-calendars here you can find all community meetings and join slack and the channel manifests-wg as well. In the calendar entry you can also find the meeting notes on google documents.

@juliusvonkohout
Copy link
Member

juliusvonkohout commented Dec 26, 2023

/lgtm

and the owner @krishnadurai approved

@google-oss-prow google-oss-prow bot added the lgtm label Dec 26, 2023
@google-oss-prow google-oss-prow bot merged commit cd991e6 into kubeflow:master Dec 26, 2023
1 check passed
@mvoitko mvoitko deleted the fix/use-hashFromEnv-in-dex branch December 27, 2023 12:06
Copy link
Contributor

@kromanow94 kromanow94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR made Kubeflow not usable through WEB UI because of auth misconfiguration.

@mvoitko can you fix those two simple issues?

metadata:
name: dex-passwords
data:
DEX_USER_PASSWORD: $2y$12$4K/VkmDd1q1Orb3xAt82zu8gk7Ad6ReFR4LCP9UeYE90NLiN9Df72
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This value should be encrypted with base64 or provided as stringData.DEX_USER_PASSWORD.

Using this in current form presents this error on kubectl apply:

Error from server (BadRequest): error when creating "STDIN": Secret in version "v1" cannot be handled as a Secret: illegal base64 data at input byte 52

@@ -0,0 +1,6 @@
apiVersion: v1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is not added to kustomization.yaml in resources section. Installing Kubeflow with this version renders dex as CreateContainerConfigError:

$ kubectl  -n auth describe pod -l app=dex
(...)
Events:
  Type     Reason     Age                From               Message
  ----     ------     ----               ----               -------
  Normal   Scheduled  39s                default-scheduler  Successfully assigned auth/dex-86f655645d-qc92d to kind-worker2
  Normal   Pulled     11s (x4 over 38s)  kubelet            Container image "ghcr.io/dexidp/dex:v2.36.0" already present on machine
  Warning  Failed     11s (x4 over 38s)  kubelet            Error: secret "dex-passwords" not found

@juliusvonkohout
Copy link
Member

@kromanow94 you can also create a PR and I can approve :-) this here was approved by a probably stale owner @krishnadurai by accident then.

kromanow94 added a commit to kromanow94/kubeflow-manifests that referenced this pull request Jan 2, 2024
The dex-passwords secret didn't have the secret defined properly and the secret wasn't added to the dex kustomization file.
google-oss-prow bot pushed a commit that referenced this pull request Jan 2, 2024
The dex-passwords secret didn't have the secret defined properly and the secret wasn't added to the dex kustomization file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants