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

[Password Managers] Some password manager icons do not show up on input fields #10274

Closed
3 tasks done
rianadon opened this issue Oct 15, 2021 · 81 comments · Fixed by #14040, #14065 or #14202
Closed
3 tasks done

[Password Managers] Some password manager icons do not show up on input fields #10274

rianadon opened this issue Oct 15, 2021 · 81 comments · Fixed by #14040, #14065 or #14202
Assignees
Labels
Accessibility Related to accessibility for people with disabilities Bug Current Bug in UI - Extra Attention

Comments

@rianadon
Copy link
Contributor

rianadon commented Oct 15, 2021

Checklist

  • I have updated to the latest available Home Assistant version.
  • I have cleared the cache of my browser.
  • I have tried a different browser to see if it is related to my browser.

Describe the issue you are experiencing

This issue is for discussing remaining password manager bugs that have arisen in spite of now having a polyfill in place to support password managers.

I'm continuing discussion from #3133, now that we have an issue specific to password manager quirks.

LastPass

I can reproduce @Nowaker's issue with LastPass. Their browser extension made the interesting design choice of adding their clickable icon to the background of the input element.
Screen Shot 2021-10-14 at 8 19 36 PM
It looks like there are also mouse move & click listeners somewhere that swap the image when you hover over the LastPass icon and know when you click on it.

LastPass successfully finds our polyfilled elements and adjust their background color (although the mouse move detection does not appear to work), but since they are hidden the LastPass icon will never be visible.

Safari

@soundstorm: Safari adds its password fill menu directly to the input element. This menu looks like it is the only way to save a password from the Home Assistant page. However, I see no other way to making this menu visible other than displaying the polyfill element.

The silver lining is that once a password is created within Safari, the autofocus lets you fill it in right away.

image

Note: it is possible to enter the credentials via Safari preferences, but this is a more roundabout way than using the menu.

Other password managers

@creack I tried the Bitwarden extension in Firefox and filling in the password works from the icon next to the address bar. It doesn't look like their extension adds icons like LastPass does, so I'm curious what is not working for you.

The only password manager for which the icon showing up does work is KeePassXC (last tested at the time I wrote the polyfill)

Solutions

I don't think there is any change we can make that will allow Safari and LastPass to show their password autocomplete icons. If anyone has a burning need to write a PR to fix this issue, I suggest adding a "my password manager doesn't work" button that un-hides the polyfilled elements.

@rianadon rianadon added the Bug Current Bug in UI - Extra Attention label Oct 15, 2021
@creack
Copy link

creack commented Oct 16, 2021

I didn't try the latest version, it may be already fixed. Bitwarden extension in Chrome simply wouldn't fill the fields on both the web ui and the mobile app.

@deviantintegral
Copy link

deviantintegral commented Nov 25, 2021

Something is up with filling on iOS too. Earlier today, I couldn't get autofill working in Safari at all. I'm using 1Password, but normally use the system-level filling so it should be the same behaviour for any password manager. Then, it worked, and I added the app to my home screen, but when logging in again (as session data is separated) there's no offer to fill at all.

As well, it's detecting the username field as normal text, and is auto-capitalizing the first letter when it shouldn't.

@leonelsr
Copy link

leonelsr commented Dec 3, 2021

Google Chrome's own password manager (without extensions) also doesn't work. The same happens on Chromium-based Microsoft Edge, vanilla password manager doesn't recognize the fields.

@Nowaker

This comment was marked as abuse.

@leonelsr
Copy link

leonelsr commented Dec 3, 2021

SOLUTION: Stop using fancy JavaScript shadow DOM / single page application / whatever comes after Web 2.0 frameworks to build a freaking login page. A simple HTML from the 90s will do better!

Ok, I understand that putting such a request in h1 (or markdown's #) letters doesn't seem to be a very polite approach 😅 Also nobody will simply stop using frameworks and stuff. But, ffs, he does have a point!

At least, let's please care having proper and working polyfills (also called "shims") always in place, in order to cover most current use cases and scenarios.

I'm not even talking about backwards compatibility (as in IE11 down to IE6... whatsoever), but password managers are increasingly becoming popular these days, as digital security is such a big concern for everyone.

They (even "vanilla" ones) make it easier for users to generate, set and use passwords that are unique for each service, app or website. But usually those are very hard to remember OR type, and ain't nobody likes having to copy and paste stuff anywhere!!!

All that to say this should be treated as a serious, big and important UX bug, because it is!!! 😬

Thanks! Best Regards.

@Nowaker

This comment was marked as off-topic.

@rianadon
Copy link
Contributor Author

rianadon commented Dec 5, 2021

I don't like moderating here but I would like to clear some things up before the discussion becomes off-topic.

Also nobody will simply stop using frameworks and stuff. But, ffs, he does have a point!

This is why the login page is not a simple page. Using the same framework & components as the rest of HA makes the page easier to maintain. And the login page isn't always just a username & password; there are a few different configurations.

At least, let's please care having proper and working polyfills (also called "shims") always in place, in order to cover most current use cases and scenarios.

There is a polyfill that, until @leonelsr's comment, I believed covered the Google Chrome builtin manager and a few of the most popular password managers. This issue is for tracking the remaining password managers with which the polyfill does not work. I've edited the first comment in this chain to make this clearer, but I worry not everyone is reading it.

I hope we can keep discussion to the polyfill and how to improve it, since talking about re-writing the frontend with a separate set of components gets us nowhere.

@github-actions
Copy link

github-actions bot commented Mar 5, 2022

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates.
Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍
This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label Mar 5, 2022
@Nowaker

This comment was marked as abuse.

@github-actions github-actions bot removed the stale label Mar 6, 2022
@leonelsr
Copy link

Chromium (Chrome and Edge, at least) built-in password manager still doesn't work.

@Nowaker

This comment was marked as abuse.

@iBobik
Copy link

iBobik commented Apr 21, 2022

Here Safari iCloud Keychain and LasPass not offering password save nor filling manually saved password.

@github-actions
Copy link

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates.
Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍
This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label Jul 20, 2022
@Nowaker

This comment was marked as abuse.

@Nowaker

This comment was marked as abuse.

@github-actions github-actions bot removed the stale label Jul 20, 2022
@kjake
Copy link

kjake commented Jul 20, 2022

Ugh, this is annoying. This doesn't work in Chrome, or the iOS companion app (Safari).

@steverep steverep added the Accessibility Related to accessibility for people with disabilities label Aug 31, 2022
@steverep steverep moved this to Triage in Accessibility Aug 31, 2022
@steverep steverep moved this from Triage to Todo in Accessibility Aug 31, 2022
@blakek
Copy link

blakek commented Sep 12, 2022

Repeating what I said in #11001

The input elements should use the HTML autocomplete attribute. I don't know Polymer very well, but I believe if the autocomplete attribute were accepted here and the autocomplete type provided by the forms (don't know where to find that) that'd fix this issue.

@steverep
Copy link
Member

Adding autocomplete is my intended fix from an accessibility standpoint, which I suspect would fix the Safari/iOS issue.

As far as getting icons to show up and work for third-party browser extensions, I'd be inclined to throw the issue back to the extension if autocomplete doesn't work.

@Nowaker

This comment was marked as off-topic.

@chrgraham

This comment was marked as abuse.

@Nowaker

This comment was marked as duplicate.

@raman325

This comment was marked as off-topic.

@Coder84619

This comment was marked as off-topic.

@Eric13246
Copy link

Eric13246 commented Aug 30, 2023

I feel this would be a good compromise to render the login form only in the 'light DOM' while keeping the rest as web components contained in shadow DOMs.

@steverep That's been suggested as part of this issue before and my 2 🪙 as a volunteer developer is that's much easier said than done. And at least when I think about it, the motivation factor is very low. Maybe putting that in words would help[...]

Thank you for this insightful explanation. It's true that it could be fixed, but too much hacks would be required to make that work properly. It does seem like the problem lies more in the hands of password manager vendors than in HA's hands.

I hear folks mentioning LastPass and 1Password not working, and from a quick search of their community forums, support has been requested but not implemented. If you're a user of either, especially a paid one, I'd certainly be putting continued pressure on them.

I am a paying customer of LastPass, so I'll definitely mention that problem to them. Meanwhile, I guess I could try Bitwarden, considering they fixed that specific problem.

@sharkwhal
Copy link

It does seem like the problem lies more in the hands of password manager vendors than in HA's hands.

So, in all seriousness and with all due respect, is the official direction “Let’s wait another 5 years and see what happens?”

At what point does it become worthwhile to accept the reality of the present and prioritize a solution? Surely many of the other recent UX improvements have been much more complex than this?

@iBobik
Copy link

iBobik commented Aug 30, 2023

As a developer who worked on a login form I confirm it is pain in the ass to make this work.

But we does not need to wait another 5 years - all major browsers support passkeys and we can skip login form thanks to it.

Let's focus on this.

@Eric13246
Copy link

@sharkwhal I understand the frustration it can cause. It's true that waiting for a big company to do something specific is wishful thinking, but at the same time we should not compromise code quality for something that would take long to develop and not be reusable somewhere else in the project. Maybe it's just an opportunity to ditch aging password managers? Who knows!

@iBobik But we does not need to wait another 5 years - all major browsers support passkeys and we can skip login form thanks to it.

Yes please! I did not notice if HA's admin panel has an option for passwordless login or FIDO key support, but I agree this could be a better avenue.

@Nowaker

This comment was marked as abuse.

@sharkwhal
Copy link

I hope this doesn’t come across as argumentative, because that’s not my intent at all; I’m just trying to understand the decisions.

I’m not following the concern about reusability elsewhere in the project. There’s literally only one place I’d expect a password manager to work and that’s the login page. Not to mention this is the one part of the project that might be seen every time a user accesses HA, and is the first impression a new user will get.

You’ve already seen a comment above from a first time user whose first impression of HA was this login page issue. There are countless other forum & Reddit posts along with the many upvotes on prior GitHub issues.

Isn’t there a point where the poor UX and consistent user feedback justifies the time needed to develop a quality solution?

@chrgraham
Copy link

chrgraham commented Aug 30, 2023

I'm surprised whatever UI framework is being leveraged doesn't support password managers out of the box, along with accessibility.

I did contact 1Password with a form-fill report, link to the repo, link to the Docker compose so they can spin it up quick for testing, and a link to this ticket.

@sharkwhal
Copy link

@chrgraham - there is also this topic in their forum
It seems they have an internal issue filed, but I’d imagine it’s quite low in priority. However I’m sure if it was their login page affected that would be a different story.

@steverep
Copy link
Member

I’m not following the concern about reusability elsewhere in the project. There’s literally only one place I’d expect a password manager to work and that’s the login page. Not to mention this is the one part of the project that might be seen every time a user accesses HA, and is the first impression a new user will get.

Fair question - please see my comment above after implementing the autocomplete attribute for other places autofill should work. Yes, the login form would certainly get the most utilization, but it should also work for password changes, two-factor setup, and any of the hundreds of integrations (or any blueprints) that need usernames, passwords, API keys, emails, etc. as part of their setup. They can do that via the text selector.

@steverep
Copy link
Member

steverep commented Aug 30, 2023

@chrgraham - there is also this topic in their forum

That post is extremely helpful. It makes it clear that 1password actually does work with the polyfill (i.e. hidden light DOM) in place, but the crux of the problem is that their injected buttons are not on top. They are being overlapped by the "real" shadow DOM elements.

I wonder if they could fix the problem by simply adding a high CSS z-index to their styles. Any 1password user could probably test this by finding their injected DOM in the browser inspector, and adding an attribute like style="z-index: 99;".

@sharkwhal
Copy link

I wonder if they could fix the problem by simply adding a high CSS z-index to their styles. Any 1password user could probably test this by finding their injected DOM in the browser inspector, and adding an attribute like style="z-index: 99;".

Looks like this works:
Screenshot 2023-08-30 at 7 47 03 PM

@steverep
Copy link
Member

steverep commented Sep 1, 2023

@sharkwhal could you please paste that DOM as text? Specifically, I want to see exactly where 1password is injecting its content.

If anyone could do the same for LastPass, that would help as well.

@gjohansson-ST
Copy link
Member

If anyone could do the same for LastPass, that would help as well.

Did a test with lastpass and it doesn't seem to inject anything anywhere.
HA:
image
Working example (github):
image

@steverep
Copy link
Member

steverep commented Sep 1, 2023

Thanks @gjohansson-ST. I'm confused by that though because the OP says it does detect the polyfill but things are hidden similar to 1password. @rianadon can you clarify here. Or maybe their approach has changed?

For GH, what is the parent element for their injected div?

@gjohansson-ST
Copy link
Member

They inject <div data-lastpass-root="" style="position: absolute !important; top: 0px !important; left: 0px !important; height: 0px !important; width: 0px !important;"><div data-lastpass-infield="true" style="position: absolute !important; top: 0px !important; left: 0px !important;"></div></div>directly in body.
Then <div data-lastpass-icon-root="true" style="position: relative !important; height: 0px !important; width: 0px !important; float: left !important;"></div> into the form. The latter seems to control the icons LP uses for fetching their info.

Attached is the github blank form loaded for lastpass input
lp github.txt

@steverep
Copy link
Member

I made some changes to the polyfill to improve compatibility based on my testing with both LastPass and 1Password. It should be in 2023.10.0.

I tested their extensions on Windows in both Chrome and Firefox. Most of the changes were to appease LastPass, which was refusing to recognize the polyfill fields (for some very illogical and unpublished reasons). The technical stuff is in #17830 (comment).

➕ The good news is that LastPass will now autofill when invoked from the context or extension menus (1Password already does this fine). Additionally, I seem to have some better success getting 1Password's icon to show up by tabbing past the login button, but it's quite finicky.

➖ The bad news is that LastPass' extension is putting its icon off screen so that's unusable (again see link above for technical details). While LastPass seems to render its icons whenever the form is recognized, 1Password's icon seems to mainly be triggered by focusing one of the fields. Since the polyfill form never receives focus, that's a problem.

I know folks are also having issues with Safari/iOS, which I did not test. And honestly, it's not clear to me whether the password managers, Safari, or both are the prime factors. Based on the history of comments in this issue, I'd guess Safari is the bigger factor as some success is commented right around the time Safari was adding support for shadow roots.

Anyway, I'd be remiss for not lastly mentioning some irony: neither LastPass nor 1Password look for input fields in shadow roots, yet both of their extensions inject menus and fields in shadow roots... 🙃

@steverep steverep mentioned this issue Sep 24, 2023
8 tasks
@piitaya
Copy link
Member

piitaya commented Sep 27, 2023

We succeeded to remove shadow dom from the auth page (#18015).
So, with 2023.10 release... password managers should be fully supported! 🎉

I tested with 1Password and multiple browsers. The feature is in beta today so you can test it with other password managers 🙂

@Coder84619
Copy link

I'll wait until the GA release, but THANK YOU!!!!!!!!!!!

@Nowaker
Copy link

Nowaker commented Sep 28, 2023

Beautiful, @piitaya! Finally, pragmatism prevailed! That's a big surprise - I really didn't expect it!

@raman325
Copy link
Contributor

I believe this can be closed now. If anyone has any remaining issues, it's probably best to open a new thread

@github-project-automation github-project-automation bot moved this from In Progress to Done in Accessibility Oct 14, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Oct 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Accessibility Related to accessibility for people with disabilities Bug Current Bug in UI - Extra Attention
Projects
Status: Done