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

🐛 BUG: Wrong password prompt when already logged in! #1443

Open
1 task done
SimonRiemertzon opened this issue Sep 18, 2024 · 4 comments · May be fixed by dailydotdev/apps#3599
Open
1 task done

🐛 BUG: Wrong password prompt when already logged in! #1443

SimonRiemertzon opened this issue Sep 18, 2024 · 4 comments · May be fixed by dailydotdev/apps#3599

Comments

@SimonRiemertzon
Copy link

SimonRiemertzon commented Sep 18, 2024

What went wrong? 🤔

Setup:

  • Mac M1
  • Arc Browser
  • Outlook Desktop
  1. Get my email with articles
  2. Cmd click on some of them
  3. Go into Arc Browser,
  4. Get log in prompt
  5. Enter credentials and press enter, now logged in
  6. Read article
  7. Switch to another tab, that also has a Daily Dev article on it
  8. Get log in prompt again
  9. Here something goes wrong! I enter credentials and press enter, GET EMAIL AND PASSWORD NOT RECOGNIZED IN OUR SYSTEM or the like.
  10. Reload the page
  11. Now logged in

Expected Behavior

I would like the website to recognize that I logged in on another tab and just reload or remove the login overlay when I'm entering my credentials , and pressing login again.

Steps to Reproduce Issue

Assuming this is a problem that is present on many different browsers. 

1. Get email with articles
2. Cmd click on some of them
3. Go into Arc Browser, 
4. Get log in prompt
5. Enter credentials and press enter, now logged in
6. Read article
7. Switch to another tab, that also has a Daily Dev article on it
8. Get log in prompt again
9. Enter credentials and press enter, `GET EMAIL AND PASSWORD NOT RECOGNIZED IN OUR SYSTEM` or the like. 
10. Reload the page
11. Now logged in

Solution Proposed

Check if user is logged in somehow, and if that is the case, reload or remove login overlay.

This is probably an edge-case where two tabs are open at the same time. One is logged in and the other is prompting the user for the sign-in. Since there are two instances of a user trying to log in to this at the same time, something goes wrong when communicating with the database or backend. Or it could be a cache / cookies issue, which then might be a browser issue? I'm not a webdeveloper myself so I'm not sure. Sorry to be vague but from my description you might understand what is going on. Happy bug hunting!

Tab that is logged in
Switched tab that hasn't realized that it is logged in, and give misleading prompt

Screenshots

No response

Environment

Mac M1 OS 14.6.1 (23G93) - Arc Browser Version 1.60.0 (53678) Chromium Engine Version 128.0.6613.138

Browsers

Other

OS

Mac

Version of daily.dev

No response

Additional Context

No response

Code of Conduct

  • I follow the conditions of this project.
@CR4ZED
Copy link

CR4ZED commented Sep 19, 2024

Hi Team, I took some time to dig into the current setup and wanted to share what I found.

Right now, we’re using the window object to post messages when a user logs in (see callback.tsx#L28 ). I think it might be better to use the Broadcast Channel API to let all open tabs know about the login.

The issue with postMessage is that it only works if the current window has an opener, which is null when users open new tabs with cmd+click or by clicking a link with target="_blank".
Screenshot 2024-09-19 at 9 49 56 PM
Switching to the Broadcast Channel API would ensure that all tabs receive the login notification, no matter how they were opened.

What do you think?

@SimonRiemertzon
Copy link
Author

It sounds like that is the solution. But I'm not so familiar with web development that I can just tell. 😅 But I would code in the change as a suggestion! :D

@idoshamun
Copy link
Member

Hi @CR4ZED,

The suggested solution sounds good!
Feel free to submit a PR and @sshanzel can help you with everything you need

@CR4ZED
Copy link

CR4ZED commented Sep 29, 2024

sure, I'll a raise a PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants