Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Should the email be used to identify the user when connecting with Dex ? #4438

Closed
Calvinaud opened this issue Feb 14, 2024 · 4 comments
Closed

Comments

@Calvinaud
Copy link
Contributor

Question

Hello,

My question is around the way the user record is retrieve when connection through Dex.
At the moment, the email retrieve from the token is used to retrieve the record corresponding to the user in the DB:

In https://github.com/litmuschaos/litmus/blob/master/litmus-portal/authentication/pkg/user/repository.go#L48 it use the UserName which take the value of the email here: https://github.com/litmuschaos/litmus/blob/master/chaoscenter/authentication/api/handlers/rest/dex_auth_handler.go#L127

When looking at some documentation it's often not recommended to use the email for this (as I understand) since it's subject to change and not sure to be unique:

Should the way the record is retrieved be changed so it using the sub claim instead of the email ?

@SarthakJain26
Copy link
Contributor

SarthakJain26 commented Mar 1, 2024

Looks to be a good suggestion, we can take this. @Calvinaud would you like to contribute to this?

@SarthakJain26
Copy link
Contributor

@DarthBenro008 would you like to share your views on this

@DarthBenro008
Copy link
Contributor

Thanks for pointing this out @Calvinaud !

It is indeed true that its not the best practice and we need to use sub instead of email. But we have implemented email because, folks who logged in via dex can also login via normal auth. If we use sub, we loose this data correlation.

If the following cases are true, we can switch to sub from email for username.

  • users signed up via Dex are not allowed to login via normal auth
  • Dex users cannot access LitmusUI if admin opts to switch off Dex integration.

@SarthakJain26 can you please confirm if that's the case?

@SarthakJain26
Copy link
Contributor

@DarthBenro008 I think this should be fine. We can separate out accounts logged in via dex and normal logged in accounts. And based on this Dex users will not be able to access LitmusUI if admin opts to switch off Dex integration, which is an expected behaviour.
@Calvinaud any further thoughts on this?

@litmuschaos litmuschaos locked and limited conversation to collaborators Mar 14, 2024
@SarthakJain26 SarthakJain26 converted this issue into discussion #4519 Mar 14, 2024

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants