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

feat: link oidc credentials when login #3222

Closed

Conversation

splaunov
Copy link
Contributor

When user tries to login with OIDC for the first time but has already registered before with email/password a credentials identifier conflict may be detected by Kratos. In this case user needs to login with email/password first and then link OIDC credentials on a settings screen.
This PR simplifies UX and allows user to link OIDC credentials to existing account right in the login flow, without
switching to settings flow.

Related issue(s)

#2727

Checklist

  • [ x] I have read the contributing guidelines.
  • I have referenced an issue containing the design document if my change
    introduces a new feature.
  • [ x] I am following the
    contributing code guidelines.
  • [ x] I have read the security policy.
  • [ x] I confirm that this pull request does not address a security
    vulnerability. If this pull request addresses a security vulnerability, I
    confirm that I got the approval (please contact
    security@ory.sh) from the maintainers to push
    the changes.
  • [ x] I have added tests that prove my fix is effective or that my feature
    works.
  • I have added or changed the documentation.

@splaunov splaunov force-pushed the feature/link-credentials-when-login branch from eb99bba to be7259f Compare April 10, 2023 19:14
Copy link
Member

@aeneasr aeneasr left a comment

Choose a reason for hiding this comment

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

Thank you! Unfortunately, merging this would introduce some quite serious security vulnerabilities which allows account take over:

  1. Adam signs up with adam@example.org in Kratos
  2. Eve creates account at e.g. Twitter with adam@example.org
  3. Eve uses Twitter account to sign into Kratos, system things it's adam@example.org
  4. Account is taken over

There are some ways to prevent this problem by, for example, requiring email verification but not all services support returning these fields.

Emails, in general, can not be used to cross reference authentication between completely separate systems.

To improve UX, the best way would be:

  1. Sign up with adam@example.org and password
  2. Adam tries to sign in with Google account adam@example.org
  3. UI says: "You already have an account. Please sign in with your password, and link your Google account in the next step"

@splaunov
Copy link
Contributor Author

  1. Eve uses Twitter account to sign into Kratos, system things it's adam@example.org
  2. Account is taken over

Eve will be requested to login with email/password first. So, takeover is not possible.
Security-vice it's the same flow as already implemented in kratos. The only difference is that the settings UI is skipped. Credentials are auto-merged if user logins with both credentials.

@aeneasr
Copy link
Member

aeneasr commented Apr 11, 2023

I see! My bad, in that case, this looks like a very nice approach :) I'll try to get it running in the next couple of days locally

@hperl
Copy link
Contributor

hperl commented Oct 9, 2023

Thank you for your contribution! I merged the latest changes into your branch and opened a new PR here: #3563. I'll finish off the PR there.

@hperl hperl closed this Oct 9, 2023
aeneasr pushed a commit that referenced this pull request Nov 8, 2023
When user tries to login with OIDC for the first time but has already registered before with email/password a credentials identifier conflict may be detected by Kratos. In this case user needs to login with email/password first and then link OIDC credentials on a settings screen.
This PR simplifies UX and allows user to link OIDC credentials to existing account right in the login flow, without
switching to settings flow.

Closes #2727
Closes #3222
@aeneasr
Copy link
Member

aeneasr commented Nov 8, 2023

@splaunov thank you so much for this work! @hperl has done an amazing job of iterating over your work and we are happy to announce that the feature has landed on master!

moose115 pushed a commit to moose115/kratos that referenced this pull request Dec 7, 2023
When user tries to login with OIDC for the first time but has already registered before with email/password a credentials identifier conflict may be detected by Kratos. In this case user needs to login with email/password first and then link OIDC credentials on a settings screen.
This PR simplifies UX and allows user to link OIDC credentials to existing account right in the login flow, without
switching to settings flow.

Closes ory#2727
Closes ory#3222
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants