Skip to content

fix: improved 'use-identity-login' error message for users trying to signInn using different provider #20582

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

romitg2
Copy link
Contributor

@romitg2 romitg2 commented Apr 7, 2025

…t provider

What does this PR do?

improves 'use-identity-login' error message for users trying to signin using different provider.

Before Change: (was like)

Screenshot 2025-04-05 at 4 16 09 AM

After Change:

Screenshot 2025-04-07 at 3 01 54 PM

how I solved?
  • passed additional query parameter passing identity provider which is managing user acccount.
changes made:
  • added additional query parameter for "user-identity-login" type of error which gives provider to which email is linked to (CAL, GOOGLE, SAML)
  • based on this provider render specific error message for user.

How should this be tested?

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.
Screenshot 2025-04-05 at 4 33 50 AM
redirects to this page: (i added query parameter which passes original provider information to error page)

Copy link

vercel bot commented Apr 7, 2025

@Romitgabanii is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

@graphite-app graphite-app bot requested a review from a team April 7, 2025 09:34
@graphite-app graphite-app bot added the community Created by Linear-GitHub Sync label Apr 7, 2025
@github-actions github-actions bot added authentication area: authentication, auth, google sign in, password, SAML, password reset, can't log in 🐛 bug Something isn't working labels Apr 7, 2025
Copy link

graphite-app bot commented Apr 7, 2025

Graphite Automations

"Add consumer team as reviewer" took an action on this PR • (04/07/25)

1 reviewer was added to this PR based on Keith Williams's automation.

"Add community label" took an action on this PR • (04/07/25)

1 label was added to this PR based on Keith Williams's automation.

"Add ready-for-e2e label" took an action on this PR • (04/08/25)

1 label was added to this PR based on Keith Williams's automation.

@romitg2
Copy link
Contributor Author

romitg2 commented Apr 7, 2025

@hariombalhara

@dosubot dosubot bot added the i18n area: i18n, translations label Apr 7, 2025
@CLAassistant
Copy link

CLAassistant commented Apr 7, 2025

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
3 out of 4 committers have signed the CLA.

✅ romitg2
✅ TusharBhatt1
✅ anikdhabal
❌ Romitgabanii
You have signed the CLA already but the status is still pending? Let us recheck it.

retrogtx
retrogtx previously approved these changes Apr 8, 2025
Copy link
Contributor

github-actions bot commented Apr 8, 2025

E2E results are ready!

@romitg2
Copy link
Contributor Author

romitg2 commented Apr 8, 2025

Hey @anikdhabal @retrogtx

Sorry to say but i missed to commit one file in this PR, and currently it's in process of merging.
how can we fix it?

Screenshot 2025-04-08 at 6 03 12 PM

@hariombalhara
Copy link
Member

@romitg2 You add/commit/push that file, we will re-review and approve

@@ -950,7 +950,7 @@ export const getOptions = ({
return true;
}
} else if (existingUserWithEmail.identityProvider === IdentityProvider.CAL) {
return "/auth/error?error=use-password-login";
return `/auth/error?error=use-identity-login&provider=${existingUserWithEmail.identityProvider}`;
Copy link
Member

Choose a reason for hiding this comment

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

I think the error code use-identity-login doesn't make sense in this case as it is email and password case and not identity provider.

How about we change the error to something generic like, "wrong-provider" and then we can handle all cases under this error instead of use-identity-login

: provider === IdentityProvider.CAL
? "Email and Password"
: provider === IdentityProvider.SAML
? "SAML"
Copy link
Member

@hariombalhara hariombalhara Apr 9, 2025

Choose a reason for hiding this comment

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

Suggested change
? "SAML"
? "SAML(like Okta)"

Copy link
Member

@hariombalhara hariombalhara left a comment

Choose a reason for hiding this comment

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

Great work so far @romitg2 !! Left some comments to be addressed

@romitg2
Copy link
Contributor Author

romitg2 commented Apr 9, 2025

Great work so far @romitg2 !! Left some comments to be addressed

Thanks.

Fixed and Tested.

Image Image

@romitg2 romitg2 requested a review from hariombalhara April 9, 2025 05:57
@TusharBhatt1 TusharBhatt1 enabled auto-merge (squash) April 14, 2025 05:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
authentication area: authentication, auth, google sign in, password, SAML, password reset, can't log in 🐛 bug Something isn't working community Created by Linear-GitHub Sync i18n area: i18n, translations ready-for-e2e
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CAL-5430] Improve 'use-identity-login' error
7 participants