TryGhost / Ghost

Independent technology for modern publishing, memberships, subscriptions and newsletters.
https://ghost.org
MIT License
47.49k stars 10.36k forks source link

1Password can't autofill Member email address #16960

Open nk9 opened 1 year ago

nk9 commented 1 year ago

Issue Summary

Users cannot use a saved email or password on Ghost.io sites with 1Password. I've reproduced this on Firefox and Chrome with the 1Password browser plugin, and also iOS. It probably doesn't work with any password manager, given the reason for the bug, but I've only tested 1Password.

The problem is that the login form is shown to the user in an anonymous (domain-less) iframe. You can read more about how I worked out what the problem was with Ghost in this thread, and more about the technical specifics of the about:srcdoc issue in this other thread.

Suffice it to say, password managers are not able to determine the domain of the page hosting the form when that page is generated dynamically and set inside an anonymous iframe. Because one of the important goals of password managers is to prevent phishing, they are very particular about ensuring that they only suggest an item for completion when the domain of the page matches the saved domain. Since the login forms in Ghost are all hosted inside these anonymous frames, there is no domain, just about:srcdoc. In the case of desktop browsers, 1Password does use the domain in the URL bar to look for possible matches. But it still won't allow you to auto-fill into an anonymous iframe for the phishing reason described above.

It seems like the solution to this will have to involve either loading actual pages with a URL into the iframe, or else dispensing with the iframe entirely and just overlaying the form with JS/CSS.

The code which generates this page is probably here, although there are a couple of places which seem to generate anonymous iframes, so I'm not exactly sure which one is used by the login form(s).

Steps to Reproduce

  1. Create a Ghost.io login in your password manager. Set the email and password. Ensure that it's tied to the domain used by your Ghost.io site.
  2. Visit the Ghost.io site and attempt to auto-fill the username (email) and password.
  3. Notice that you can't autofill them. On iOS, it doesn't even find the correct item.

Ghost Version

5.50.2? whatever version is live on ghost.io

Node.js Version

0

How did you install Ghost?

Hosted by ghost.io

Database type

Other

Browser & OS version

Mac: Chrome, Firefox; iOS: Safari

Relevant log / error output

No response

Code of Conduct

github-actions[bot] commented 1 year ago

Hi there! If you're having any issue with a Ghost(Pro) site, please drop us an email on support@ghost.org and we'll be more than happy to give you a hand directly 🙂

naz commented 1 year ago

Hey @nk9 👋 From the issue description it seems like you are talking about member login forms. What's confusing here is that there's no concept of a "password" for members - they login through magic links. Are you talking about only saving an email part in the password manager or is there's something else missing?

nk9 commented 1 year ago

I included passwords because I wasn't sure if there was any circumstance where the login form doesn't use magic links. If it's only ever emails, then that's what this is about. Certainly that is the behavior I have seen myself.

Sorry for the confusion!

nk9 commented 1 year ago

Hi @naz, will you be reopening this issue? My understanding is that the Ghost(Pro) support team has determined that it's not something they can help with.

naz commented 1 year ago

@nk9 the described behavior with "passwords" created a lot of confusion as that's not how member logins work. Please be very specific and to the point about what the bug is. Would be amazing if you could record a video (Loom or alike) or even create a PR with a suggested fix for the behavior.

Are you expecting for 1password to autofill the member email address in the member "Sign in" form, correct?

nk9 commented 1 year ago

@naz Here is a video. I've explained what the problem is above, but I am not sure which solution you will want to use.

https://github.com/TryGhost/Ghost/assets/3646730/473cc732-95b0-449a-a6dc-6f125cf96dba

naz commented 1 year ago

Awasome! Thanks @nk9 ❤️ I think what happens here is that a different email field gets filled out in the background (like a subscribe form or something). Might be due to how 1Password looks up the email fields on the page. Are you keen to explore a possible solution?

nk9 commented 1 year ago

Thanks for reopening! The problem is what I explained before: the anonymous iframe. Please read the links in my original report. The two possible solutions that I can see are to use a real page, or to stop using an iframe, although perhaps you have some other ideas.

Can you confirm how many locations in the code generate a sign in page like this? Is the code I linked to in my original report the only code responsible for this?

naz commented 1 year ago

The code you've linked to in the Portal package is the right place where the iframe is generated and this is the preferred way for Ghost sites to use members. Having an iframe is not the only way to have a signin/signup forms. We allow for custom forms, which you could embed in the theme. For more information about custom forms check out these docs - https://ghost.org/docs/themes/members/#signup-forms.

naz commented 1 year ago

Thanks a lot for putting this much effort into the issue!

macauleydev commented 1 year ago

Hi @nk9 and @naz! Thanks for the helpful descriptions & links so far.

I reproduced this behavior with Bitwarden in Firefox, so I believe the analysis above is correct and the use of iframe in Ghost’s sign-in forms has the unintended side effect of preventing autofill from any phishing-aware password manager.

It looks to me like iframe was introduced to the Ghost Portal package 3 years ago by @rishabhgrg in this commit and was likely based on react-frame-component. However, I’m not sure if the component author’s arguments for rendering to an iframe (”the main benefit is style encapsulation”) are relevant to Ghost’s sign-in forms, or that the benefits of iframe outweigh the drawback of preventing autofill.

Ideally someone with more React experience than myself (and perhaps @rishabhgrg if he reads this) could test or advise on whether (and exactly how) the iframe could be safely eliminated from the Portal's rendering of the sign-in form.

susannakosic commented 1 year ago

Hi, I'm very new to this community and just starting exploring Ghost. I saw this "good first start issue" and investigated a bit what would happen just by replacing the iframe by a simple div. Seems like SignIn popup is working fine (loom here, although I can see a bunch of SigninFlow tests failing and I don't know yet why. draft PR

vadimavdeev commented 1 year ago

I also tried working on this issue. I thought Shadow DOM would be the way to go because it provides style encapsulation, which seems to be the main reason to use iframes. I did a quick Google search to see if autofill in Shadow DOM is supported, and found this discussion in 1Password community, and this announcement from Dashlane, both of which seemed to suggest that it is. So I made the changes in my fork to replace iframe with Shadow DOM. Unfortunately, autofill with 1Password did not work. I joined 1Password Developers Slack workspace to ask about Shadow DOM support, but somebody beat me to it, and the answer was no, it is not currently supported.

I also tried to workaround the about:srcdoc url in iframe by setting it’s src attribute to the result of:

URL.createObjectURL(new Blob(['<!DOCTYPE html>'], { type: 'text/html' }))

but that didn’t work either.

Those are things that required minimal code changes and project expertise. There are other ways to go about this problem, of course, like server-rendering the login form to keep it in an iframe, or replacing iframe with a div, but those require some deeper understanding of the platform than what I have. I’d be happy to help if/when the core team makes a decision on how to solve this properly. I hope I’ve at least saved you some time investigating the options.

susannakosic commented 1 year ago

I've open the PR for review. Tests are passing. I've only replaced the iframe by a div for the sign-in page. We ll wait for reviewers feedback.

TanyyshaJ commented 1 year ago

Hey can u assign this issue to me!

nick2432 commented 11 months ago

can i work on this?

nk9 commented 11 months ago

It looks like @susannakosic has code which fixes this, it just needs to be reviewed. Any update on that, contributors? @naz

github-actions[bot] commented 7 months ago

Our bot has automatically marked this issue as stale because there has not been any activity here in some time.

The issue will be closed soon if there are no further updates, however we ask that you do not post comments to keep the issue open if you are not actively working on a PR.

We keep the issue list minimal so we can keep focus on the most pressing issues. Closed issues can always be reopened if a new contributor is found. Thank you for understanding 🙂

nk9 commented 7 months ago

This isn't stale, there's code which needs review. @naz

muratcorlu commented 6 months ago

Unfortunately, autofill with 1Password did not work. I joined 1Password Developers Slack workspace to ask about Shadow DOM support, but somebody beat me to it, and the answer was no, it is not currently supported.

@vadimavdeev I can not reach the slack message but I worked on a design system project that uses web components, we could still manage to provide autofill functionality. Generally problem occurs when you want to have custom input components (even though there are still some solutions by using ElementInternals). But, autofilling for a native form that is completely inside a single shadowroot, should not be a problem, I believe.

I think it's best to make all of those "portal elements" (including comments, floating subscribe button etc) web components, since it has many benefits for template developers to be able to customize them freely with just CSS.

github-actions[bot] commented 2 months ago

Our bot has automatically marked this issue as stale because there has not been any activity here in some time.

The issue will be closed soon if there are no further updates, however we ask that you do not post comments to keep the issue open if you are not actively working on a PR.

We keep the issue list minimal so we can keep focus on the most pressing issues. Closed issues can always be reopened if a new contributor is found. Thank you for understanding 🙂

nk9 commented 2 months ago

As before, this issue isn't stale. @naz when will the PR get a code review?