Do we need popups for login?

From inception of Discourse social logins on desktop would always use a “popup” window for logging in.

The intent was to be “less” distracting keeping you on the actual site while the “logging in” act was happening.

I think it makes no sense to carry this for a few reasons.

It is already inconsistent with the way “discourse SSO” works, it can be flaky on browsers that don’t allow popups (my Firefox complains and requires a click to enable it), the code for posting messages is complex and hard to maintain plus we already are requiring an extra click on the popup now.

How would you feel about throwing away the “pop up a window for social login” code and only having 1 consistent way of logging on be it SSO/social login or what not?

@codinghorror / @david / @eviltrout ?

26 Likes

I agree we should get rid of the popups, and make everything consistent. This would let us throw away a lot of code as well :tada:

The issues you described in Firefox are fixable, but as you said the popup code is complex and prone to bugs like this.

I can work on this later this week if you like?

18 Likes

I would be delighted :grin:

10 Likes

I’ve created a PR for this

https://github.com/discourse/discourse/pull/8092

Once it eventually hits stable, we should remove the redundant “full screen” configuration options from plugins (e.g. OAuth2, OIDC)

15 Likes

Nice… :+1:t3: :slot_machine: :hugs:

11 Likes

This is changing some fairly critical parts of our authentication code, so the branch is now deployed to Meta for testing.

If anyone has some time to spare, please try logging in using a few different providers, and disconnecting/reconnecting accounts in your preferences. If you find any issues, let me know and I’ll investigate :face_with_monocle:

Also a great chance to try out our new Discord Authentication, which is configured here on Meta :tada:

6 Likes

@jomaxro could you assist with testing?

3 Likes

Certainly! I’ll try all the accounts I have.

6 Likes

Tested logging in with Facebook, Google, Twitter, and Discord, all successful. Also tested disconnected and re-connecting the same accounts from user preferences. Both processes smooth.

I cannot test GitHub as I have 2FA on the Discourse account linked to my GitHub.

5 Likes

Did some further testing (above tests were all in Chrome on Windows). I confirmed login works with IE, Edge, and Firefox on Windows as well. Discovered 2 minor bugs, one related to logout, and one with account connection in IE, but both have been confirmed to exist prior to this change.

5 Likes

Thanks for testing @jomaxro.

PR here. It’s a core bug, which was made worse because facebook insist on adding a hash to the callback URL.

This was a caching issue which was only affecting IE11. Should be fixed by this commit. Deploying that fix to Meta now :mantelpiece_clock:

12 Likes

This is sounding great! Maybe we can merge after the next beta is cut.

6 Likes

A post was split to a new topic: Google One-Tap Sign In

This is now merged

https://github.com/discourse/discourse/commit/d2bceff133ac152678a1407d45fea260a0fe8536

9 Likes

This topic was automatically closed after 6 days. New replies are no longer allowed.