Save new user based on lowercase email #7629
Conversation
kubermatic-bot
added
release-note-none
ahmadhamzh
force-pushed
the
7617-save-user-using-lowercase-email
branch
from
94f98ed to
15d62bf
Compare
| user := &kubermaticv1.User{ | ||
| ObjectMeta: metav1.ObjectMeta{ | ||
| Name: fmt.Sprintf("%x", sha256.Sum256([]byte(email))), | ||
| Name: fmt.Sprintf("%x", sha256.Sum256([]byte(strings.ToLower(email)))), | ||
| }, | ||
| Spec: kubermaticv1.UserSpec{ | ||
| Name: name, | ||
| Email: email, | ||
| Email: strings.ToLower(email), | ||
| Groups: groups, | ||
| }, | ||
| } |
There was a problem hiding this comment.
Move the lower case conversion on top of the method after:
if len(name) == 0 || len(email) == 0 {
return nil, apierrors.NewBadRequest("Email, ID and Name cannot be empty when creating a new user resource")
}
email = strings.ToLower(email)
...
...There was a problem hiding this comment.
We should also copy this change over to https://github.com/kubermatic/dashboard/blob/main/modules/api/pkg/provider/kubernetes/user.go#L136
// UpdateUser updates user.
func (p *UserProvider) UpdateUser(ctx context.Context, user *kubermaticv1.User) (*kubermaticv1.User, error) {
// make sure the first patch doesn't override the status
status := user.Status.DeepCopy()
user.Spec.Email = strings.ToLower(user.Spec.Email)
if err := p.runtimeClient.Update(ctx, user); err != nil {
return nil, err
}
...
...
ahmadhamzh
force-pushed
the
7617-save-user-using-lowercase-email
branch
from
15d62bf to
f8a59fe
Compare
|
/cherry-pick release/v2.28 |
|
@ahmadhamzh: once the present PR merges, I will cherry-pick it on top of In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
LGTM label has been added. Git tree hash: 8d1bb13fff8e3084138a116714b2d58e5167147e
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ahmedwaleedmalik 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 |
kubermatic-bot
added
the
approved
Waseem826
added
the
code-freeze-approved
kubermatic-bot
removed
the
do-not-merge/code-freeze
|
/retest |
|
@ahmadhamzh: new pull request created: #7634 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@ahmadhamzh: new pull request created: #7635 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
4 participants
What this PR does / why we need it:
Use email in lowercase to generate name when creating a user object.
Which issue(s) this PR fixes:
Fixes #7617
What type of PR is this?
/kind bug
Special notes for your reviewer:
I tried to reproduce the issue but couldn’t.
I signed in with GitHub using an email with capital initials, granted a viewer role, then logged out and logged in again with Google using the same email in lowercase.
No new user was created, and the role was preserved across both logins, so both logins are treated as the same user.
I also tried adding two dummy users in the Dex config. Both had the same email, but one with capital initials. When I tried adding them to the same project, I got the error: user is already in the project. However, when I log in with either user, I can see that both are granted the same role.
project members
add an existing member with capital initials
Does this PR introduce a user-facing change? Then add your Release Note here:
Documentation: