Handle "Email address is taken" on Login page #3950

AlexTugarev · 2021-04-15T10:17:07Z

This PR adds a check for already taken email addresses on signup. You supposed to see a message on the Login page that explains why a signup with an Identity Provider didn't work, because you previously created a Gitpod account with a different IdP but matching email address.

The intention of this change is to minimize the unintentional creation of second Gitpod accounts, just by using a different IdP.

Try it yourself:

make sure you have a GH and a GL session and the primary email address is matching
sign up with one provider first
logout
try to sign up with the second provider
see the error message

––

This PR also fixes #3801

How to test:

go to GH setting and revoke access for the Gitpod Preview Environments OAuth app
try to sign up and decline(!)
see the error message

gtsiolis · 2021-04-15T10:20:07Z

Looking at this in a moment! 👀

Feel free to update the description if there are any particular steps to follow to help reviewing this.

AlexTugarev · 2021-04-15T10:31:50Z

gitlab.com uses a similar strategy BTW

gtsiolis

Thanks for adding this @AlexTugarev and the detailed write-up on how to review this! 🌟

FWIW, this pattern could become useful for tackling similar issues like #3836.

Left some minor comments below, feel free to resolve these here or in a follow up PR.

There are only three changes that would be great to go into this PR:

Center align the alert component.
Limit alert width using w-96.
Add some margin below the alert.

Pasting below how this could ideally look like.

gtsiolis · 2021-04-15T10:43:40Z

components/dashboard/src/Login.tsx

@@ -126,6 +136,12 @@ export function Login() {
                        </div>
                    </div>
                </div>
+                {emailTaken && (<div className="bg-gitpod-kumquat-light rounded-md p-3 text-gitpod-red text-sm mb-2">


nitpick: Alerts like this can use a larger radius to differentiate them from inputs or other elements as we do in the garbage collection message in the workspaces lists.

Some thoughts on this:

This pattern will become more clear and consistent as its product design evolves.

Similarly, colors used for each alert type will also become more clear as design guidelines shape up.

Suggested change

{emailTaken && (<div className="bg-gitpod-kumquat-light rounded-md p-3 text-gitpod-red text-sm mb-2">

{emailTaken && (<div className="bg-gitpod-kumquat-light rounded-xl p-3 text-gitpod-red text-sm mb-2">

Could we add some reuseable components for that, please? Each time I pick one example from somewhere else, it's the wrong one.

Strong vote for YES!

This is common due to a) some design experimentations added on purpose in some pages and b) our MVC approach with helps us ship faster and unblock code reviews. The need to introduce a reusable component library will become more clear on the way as a) the product evolves and b) team members try to add or improve product features on more areas.

For this iteration, going with the rounded-xl class should suffice. WDYT?

Sure! ☝🏻 updated the screenshots in the description above.

gtsiolis · 2021-04-15T10:43:54Z

components/dashboard/src/Login.tsx

+                {emailTaken && (<div className="bg-gitpod-kumquat-light rounded-md p-3 text-gitpod-red text-sm mb-2">
+                    {`Email address is already in use. Please log in with ${emailTaken.otherUser.authHost}.`}
+                </div>)}
+                {errorMessage && (<div className="bg-gitpod-kumquat-light rounded-md p-3 text-gitpod-red text-sm mb-2">


nitpick: Radius fix.

Suggested change

{errorMessage && (<div className="bg-gitpod-kumquat-light rounded-md p-3 text-gitpod-red text-sm mb-2">

{errorMessage && (<div className="bg-gitpod-kumquat-light rounded-xl p-3 text-gitpod-red text-sm mb-2">

gtsiolis · 2021-04-15T10:46:21Z

components/dashboard/src/Login.tsx

@@ -126,6 +136,12 @@ export function Login() {
                        </div>
                    </div>
                </div>
+                {emailTaken && (<div className="bg-gitpod-kumquat-light rounded-md p-3 text-gitpod-red text-sm mb-2">


suggestion: What do you think of making this alert component dismissible?

Given that the Login page is just a stop between here and there, I'd personally never try to dismiss it, but if you think this is useful 🤷🏻‍♂️ let's add it. Also it's dismissed on next try to connect.

It should be fine as non-dismissible for now as the task is blocked if no action is taken. Usually, an alert component is non-dismissible when the user is prevented from completing a task until a system-condition is met. Since this is user-driven and not relying on a system condition, a dismissible alert is probably the correct approach here. 💡

However, we can tackle this in a follow up issue going through all alerts and make sure we make dismissible the ones needed. Your call! 📞

However, we can tackle this in a follow up issue going through all alerts and make sure we make dismissible the ones needed. Your call! 📞

sounds good!

components/dashboard/src/Login.tsx

gtsiolis · 2021-04-15T10:54:36Z

components/dashboard/src/Login.tsx

@@ -126,6 +136,12 @@ export function Login() {
                        </div>
                    </div>
                </div>
+                {emailTaken && (<div className="bg-gitpod-kumquat-light rounded-md p-3 text-gitpod-red text-sm mb-2">
+                    {`Email address is already in use. Please log in with ${emailTaken.otherUser.authHost}.`}


suggestion: Ideally, we could have a title text for briefly explaining what happened and a body text to clarify what needs to be done. This way we can also make the title more prominent. We can tackle this separately and leave this as is for now in one piece.

* already taken email addresses should be rejected on signup * OAuth errors (e.g. access_denied) should be propagated

AlexTugarev requested review from gtsiolis and csweichel April 15, 2021 10:17

This was referenced Apr 15, 2021

Handle same email address on signup #3943

Closed

[server/oauth] fix access_denied parsing #3915

Closed

gtsiolis added aspect: authentication This is a broad, abstract, and almost impractical category that we have yet to sort out. component: dashboard type: improvement Improves an existing feature or existing code labels Apr 15, 2021

gtsiolis added this to the April 2021 milestone Apr 15, 2021

gtsiolis approved these changes Apr 15, 2021

View reviewed changes

AlexTugarev force-pushed the at/email-taken branch from 5e51ea9 to df6f5e9 Compare April 15, 2021 13:17

[server/login] handle errors on login

5095aa9

* already taken email addresses should be rejected on signup * OAuth errors (e.g. access_denied) should be propagated

AlexTugarev force-pushed the at/email-taken branch from df6f5e9 to 5095aa9 Compare April 15, 2021 13:46

AlexTugarev merged commit 81d7ff4 into main Apr 15, 2021

AlexTugarev deleted the at/email-taken branch April 15, 2021 14:07

gtsiolis mentioned this pull request Apr 19, 2021

[dashboard] Add design specs for the login redirection page #3589

Closed

jankeromnes mentioned this pull request May 10, 2021

Seed Changelog v2 with exciting / user-visible changes from the April / May cycles #4187

Closed

gtsiolis mentioned this pull request May 17, 2021

Feedback Issue: New dashboard 💬 #3727

Closed

AlexTugarev mentioned this pull request Jun 29, 2021

Handle deletions and re-creation of GitHub accounts #4656

Closed

gtsiolis mentioned this pull request Jul 7, 2021

Show helpful explanation when opening a workspace and the git provider integration is missing #4696

Closed

gtsiolis mentioned this pull request Oct 26, 2021

Cannot add a new project when having two accounts #6268

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle "Email address is taken" on Login page #3950

Handle "Email address is taken" on Login page #3950

AlexTugarev commented Apr 15, 2021 •

edited

Loading

gtsiolis commented Apr 15, 2021 •

edited

Loading

AlexTugarev commented Apr 15, 2021

gtsiolis left a comment •

edited by AlexTugarev

Loading

gtsiolis Apr 15, 2021

AlexTugarev Apr 15, 2021

gtsiolis Apr 15, 2021

AlexTugarev Apr 15, 2021

gtsiolis Apr 15, 2021

gtsiolis Apr 15, 2021 •

edited

Loading

AlexTugarev Apr 15, 2021

gtsiolis Apr 15, 2021

AlexTugarev Apr 15, 2021

gtsiolis Apr 15, 2021

	{emailTaken && (<div className="bg-gitpod-kumquat-light rounded-md p-3 text-gitpod-red text-sm mb-2">
	{emailTaken && (<div className="bg-gitpod-kumquat-light rounded-xl p-3 text-gitpod-red text-sm mb-2">

	{errorMessage && (<div className="bg-gitpod-kumquat-light rounded-md p-3 text-gitpod-red text-sm mb-2">
	{errorMessage && (<div className="bg-gitpod-kumquat-light rounded-xl p-3 text-gitpod-red text-sm mb-2">

Handle "Email address is taken" on Login page #3950

Handle "Email address is taken" on Login page #3950

Conversation

AlexTugarev commented Apr 15, 2021 • edited Loading

gtsiolis commented Apr 15, 2021 • edited Loading

AlexTugarev commented Apr 15, 2021

gtsiolis left a comment • edited by AlexTugarev Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gtsiolis Apr 15, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlexTugarev commented Apr 15, 2021 •

edited

Loading

gtsiolis commented Apr 15, 2021 •

edited

Loading

gtsiolis left a comment •

edited by AlexTugarev

Loading

gtsiolis Apr 15, 2021 •

edited

Loading