home-assistant / frontend

:lollipop: Frontend for Home Assistant
https://demo.home-assistant.io
Other
4.03k stars 2.75k forks source link

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

Closed rianadon closed 1 year ago

rianadon commented 3 years ago

Checklist

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.

creack commented 3 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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!

leonelsr commented 2 years ago

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 commented 2 years ago

Ok, I understand that putting such a request in h1 (or markdown's #) letters doesn't seem to be a very polite approach

Before I answer that, let's look at the background first:

Oct 23, 2019 - 8 upvotes - @Nowaker - I guess the login page should be a regular HTML login page if SPA makes the basics overly complicated.

Sep 24, 2020 - 29 upvotes - @Nowaker - Can we simply stop using the latest tech that is password filler and usability unfriendly and go back to normal HTML? We don't need the 2020 tech on the login page. 1995 tech sans the gradients and shadows will do...

Apr 18, 2021 - 19 upvotes - @Nowaker - The login page should be a regular HTML login page if SPA makes the basics overly complicated.

Apr 18, 2021 - 30 upvotes - @johanson - The founder seems to suggest it wont' be fixed - #1622. I agree with @Nowaker, common sense would be the login page to work with password managers, not aiming for to be 100% x framework.

It's been almost three years since #3133 was reported, and two years since not using SPA for the login page was suggested, and still counting.

Secondly, password manager support ticket (#3133) is the TOP1 most upvoted ticket of all times for HA frontend - 172 upvotes, ticket is closed, and the problems are still arising. The second most upvoted ticket is #4614 with only 37 upvotes.

So, commenting on what you said - of course, it wasn't polite, but closing and locking the TOP1 most upvoted ticket wasn't polite either. The use of h1 is just a tribute to these 172 upvotes, since the maintainers do not consider "use 90s tech on login page for simplicity" a valid request, as evidenced in #1622.

rianadon commented 2 years ago

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[bot] commented 2 years ago

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.

Nowaker commented 2 years ago

bad bot

leonelsr commented 2 years ago

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

Nowaker commented 2 years ago

And the login page isn't always just a username & password; there are a few different configurations.

With usage rate of what, 5% at most? If any of these are enabled in that particular HA setup, sure, whatever, render the fancy dandy Web 3.0 rainbow.

Otherwise, I'll repeat my words again: 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!

iBobik commented 2 years ago

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

github-actions[bot] commented 2 years ago

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.

Nowaker commented 2 years ago

bad bot

Nowaker commented 2 years ago

172 upvotes on the previous ticket: https://github.com/home-assistant/frontend/issues/3133 - we won't go away.

kjake commented 2 years ago

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

blakek commented 2 years ago

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 commented 2 years ago

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.

RosemaryOrchard commented 2 years ago

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

Just to confirm, this is correct. The Apple Developer Documentation shows it. Ideally, it would be added to the 2FA token and password change fields too.

steverep commented 2 years ago

Just to confirm, this is correct. The Apple Developer Documentation shows it. Ideally, it would be added to the 2FA token and password change fields too.

Yes, I can add it there and any other relevant places, just haven't gotten to it yet.

steverep commented 1 year ago

The autocomplete attribute has now been added in the following locations, and is in the current beta release for 2022.11. At a minimum, I expect autofill on Safari/iOS to now work for all of these because it does not require the polyfill for shadow DOM support.

However, please note that only the main login form currently uses the polyfill for shadow DOM, so browsers/extensions that still don't support shadow DOM won't see any effect in the other areas. I know there are active bug reports on this for Chrome, Firefox, and popular extensions like BitWarden.

Please test with your particular password manager and report any unexpected results here.

RosemaryOrchard commented 1 year ago

Auto fill on iOS (specifically the iPad app) was broken today on 2022.11.0b6 as the 2FA code auto filled into the password field.

Nowaker commented 1 year ago

However, please note that only the main login form currently uses the polyfill for shadow DOM, so browsers/extensions that still don't support shadow DOM won't see any effect in the other areas. I know there are active bug reports on this for Chrome, Firefox, and popular extensions like BitWarden.

We have an active bug report in Home Assistant:

Please 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!

steverep commented 1 year ago

@RosemaryOrchard can you inspect the DOM and verify the value of the autocomplete attribute is "current-password"? I don't see how it could be anything else but I'll check to make sure I didn't mess up. You are using a standard Home Assistant login correct?

If you can verify the value is correct, the culprit is probably whatever password manager you are using.

RosemaryOrchard commented 1 year ago

Sadly inspecting the DOM of the iOS app on an iPad is not something that is easy/possible to accomplish. I am using 1Password, but through the native iOS autofill. That said, it pops up Safari window which does not run any extensions or modifications, so it can't be one of those interfering with it.

steverep commented 1 year ago

You can use this extension to inspect on iOS:

https://apps.apple.com/us/app/web-inspector/id1584825745

RosemaryOrchard commented 1 year ago

You cannot use the inspection as it's an in app Safari, that means it uses WKWebView. For clarity: You can't even use the native Safari keyboard shortcuts for autofill, extensions are not available (as stated above), and it doesn't even share cookies with Safari.

If I inspect in Safari itself, I can see the HTML which seems fine, but autofill does not work. 8A4F87A3-EDE3-4C84-A4CA-6A9C68F7CBF1 ha_login.txt.

steverep commented 1 year ago

You cannot use the inspection as it's an in app Safari, that means it uses WKWebView.

Right I was asking to inspect in Safari. I would not expect different autofill behavior in the web view.

Auto fill on iOS (specifically the iPad app) was broken today on 2022.11.0b6 as the 2FA code auto filled into the password field.

Are you saying that the username filled correctly? I'm confused now because the screenshot says nothing was auto-filled. Is that an error from 1Password?

steverep commented 1 year ago

@RosemaryOrchard I did make one more change to make sure that the fields have a name attribute in addition to autocomplete. This is in 2022.11.0. Please let me know if that makes a difference.

FWIW, I tested all the locations I listed with BitWarden on iOS and autofills performed as expected.

If anyone uses iCloud Keychain, some feedback on how that behaves in 2022.11.0 would be appreciated (or any other specific password manager for that matter).

steverep commented 1 year ago

FYI - the Chromium issue for autofilling inside shadow DOM was just marked fixed

pomeloy commented 1 year ago

Auto fill is still broken on HA Frontend 20221213.0 with Safari 15.5 on macOS and Safari on iOS 16.1.1.

steverep commented 1 year ago

Auto fill is still broken on HA Frontend 20221213.0 with Safari 15.5 on macOS and Safari on iOS 16.1.1.

With what password manager and on which page?

pomeloy commented 1 year ago

Using the default iCloud keychain on page homeassistant:8123/auth/authorize?response_type=code&redirect_uri=http://homeassistant:8123/?auth_callback=1&client_id=http://homeassistant:8123/&state=...

steverep commented 1 year ago

@pomeloy please explain your symptoms and steps on iOS because I cannot reproduce any issue. Are you able to invoke autofill from the keyboard popup? Or is it just not autofilling after you invoke it?

pomeloy commented 1 year ago

Actually there is no popup or autofill sign at all neither on various iOS devices nor on macOS so I am not able to invoke any autofill mechanism.

steverep commented 1 year ago
  1. Just for clarity, please verify that autofill is turned on and Keychain is selected in Settings  > Passwords > Password Options
  2. Tap either the username or password field first to get the keyboard. At the top of the keyboard you should see a button to select a password to autofill. Is that not the case?
iBobik commented 1 year ago

I can confirm no password manager started on homepage. Having iCloud Keychain and Bitwarden. Just used it to login on GitHub, so I am sure it works.

iOS 16.1.1 Home Assistant 2022.12.6

259CC8A1-0316-43A1-89FC-5BE4DA4EC72C

steverep commented 1 year ago

Thanks for the verification @iBobik. It seems I'm making the mistake of testing with Firefox on iOS instead of Safari thinking the results would be the same as they both use webkit, but it only works on the former.

m33x commented 1 year ago

There is progress on Safari's side too. https://hachyderm.io/@rmondello/109752442226984975

steverep commented 1 year ago

For anyone using bitWarden, they just closed their issue regarding shadow DOM as completed (https://github.com/bitwarden/clients/issues/713).

Coder84619 commented 1 year ago

I'm having this issue with 1Password on Mac. Using Safari (Ventura) and Vivaldi for Mac. Neither browser let 1Password auto-fill credentials.

m33x commented 1 year ago

Double checked that. For me, the same setup works flawlessly. (Ofc, the 1Password icon does not show up, only after the autofill).

Server:
Home Assistant 2023.3.0
Browser:
macOS Ventura 13.2.1 (22D68)
Safari Version 16.3 (18614.4.6.1.6)
1Password for Safari Version 2.7.0 (20238)

https://user-images.githubusercontent.com/883685/223154950-73f7152f-75d7-4fdc-b34e-1a1546584394.mp4

Coder84619 commented 1 year ago

@m33x The missing 1P icon is exactly the problem. On literally 99.9999% of other sites it shows up.

SFoskett commented 1 year ago

Still having issues in 1Password for Safari on iOS. Same password works perfectly on Safari for Mac.

Eric13246 commented 1 year ago

I am experiencing this today on Firefox with LastPass on UI version 20230802.0. I have found a blog post that shows a possible solution to this problem here https://www.thisdot.co/blog/a-tale-of-form-autofill-litelement-and-the-shadow-dom/

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.

revolter commented 1 year ago

Workaround for username and password on the Apple side: add and delete some random letter in both fields.

Unfortunately, the 2FA code autofill doesn't seem to have any workaround.

steverep commented 1 year ago

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.

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...

First, the base text field component comes from an external package with complicated stylesheets, so it would have to be hacked to move it and errors would almost certainly arise. Plus even on the login form, it's not just one shadow DOM but several nested ones, so you'd have to deal with potential conflicts from several components up the tree. The alternative would be to use much simpler fields, which requires rewriting how the values are grabbed and sent to core for authentication, and that results in an a11y and UX trade off due to a lack of UI consistency. A hybrid approach is already in place where there are hidden fields in the light DOM designed to catch autocompleted values and pass them to the visible web components, but it only seems to work for some password managers and not others.

Now lets say I spend the time to get past all that and it's approved for production, I've still only developed something that works on a couple fields on one page and not on the many others throughout the app that also have the potential to be autocompleted. Moreover, the eventual goal would be to revert my entire set of changes when password managers, most of which have paid developers, get around to supporting fields within shadow DOMs, which have been around as part of the spec for many years now.

Continuing that last point, there also has been recent activity from sever managers, making this less and less an impactful thing to work on:

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.

SFoskett commented 1 year ago

I very much appreciate your answer, as it provides much needed perspective from a volunteer developer. That being said, it’s a little bit puzzling to me as a user that I don’t encounter this issue with any other systems. I believe you when you say that the issue is with the password software not the web application. But why don’t other systems have the same problem? And why has this not been a high priority for the two major password managers to fix? That’s really a rhetorical question, though I am curious.

I am a paid 1Password subscriber, and in fact, am an enterprise user, and will definitely submit this to them to fix. What specifically should I be asking for? “Shadow DOM support?” should I refer them to this thread? I am not (this kind of) developer, but I want to help.

Thank you for the time and energy you have invested into improving home assistant! And thank you for this answer.

Roos-AID commented 1 year ago

Steve,

the fix mentioned for Safari 16.6 does not solve the problem for the Apple password manager.

When the Apple password manager has autofilled the fields, following error is thrown : Login attempt or request with invalid authentication from ipxxxxx (ipxxxx). Requested URL: '/auth/login_flow/144acb813613299cd87799xxxxxxxxxx'. (Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/16.6 Safari/605.1.15)

The only workaround is to have it autofilled, manually ad a space (or any other char) at end of userid and pw and remove this. The UI then sees the value, without this the autofill string is not transferred to the core for authentication.

Met vriendelijk groet,

Rob Roos

Op 8 aug. 2023, om 21:03 heeft Steve Repsher @.***> het volgende geschreven:

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.

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...

First, the base text field component comes from an external package with complicated stylesheets, so it would have to be hacked to move it and errors would almost certainly arise. Plus even on the login form, it's not just one shadow DOM but several nested ones, so you'd have to deal with potential conflicts from several components up the tree. The alternative would be to use much simpler fields, which requires rewriting how the values are grabbed and sent to core for authentication, and that results in an a11y and UX trade off due to a lack of UI consistency. A hybrid approach is already in place where there are hidden fields in the light DOM designed to catch autocompleted values and pass them to the visible web components, but it only seems to work for some password managers and not others.

Now lets say I spend the time to get past all that and it's approved for production, I've still only developed something that works on a couple fields on one page and not on the many others throughout the app that also have the potential to be autocompleted. Moreover, the eventual goal would be to revert my entire set of changes when password managers, most of which have paid developers, get around to supporting fields within shadow DOMs, which have been around as part of the spec for many years now.

Continuing that last point, there also has been recent activity from sever managers, making this less and less an impactful thing to work on:

bitWarden has a fixed release from earlier this year (bitwarden/clients#713 https://github.com/bitwarden/clients/issues/713) Chrome has fixed their issue but looks to be working through their alpha/beta process (Issue 649162: Related inputs separated by shadow root do not autofill https://bugs.chromium.org/p/chromium/issues/detail?id=649162) Safari seems to have possibly fixed in 16.6 (Bug 172567 - Safari autofill doesn't work in form inputs that are in a shadow root https://bugs.webkit.org/show_bug.cgi?id=172567) Firefox has a bug report but no recent activity (Bug 1629226 - [meta] Make password manager work with Shadow DOM https://bugzilla.mozilla.org/show_bug.cgi?id=1629226). I would encourage folks to add their vote and/or comment on it. 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.

— Reply to this email directly, view it on GitHub https://github.com/home-assistant/frontend/issues/10274#issuecomment-1670141427, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFQ5PY5ECW3MD6GRTHJFZ2TXUKEPDANCNFSM5GBCDB3A. You are receiving this because you are subscribed to this thread.

steverep commented 1 year ago

I am a paid 1Password subscriber, and in fact, am an enterprise user, and will definitely submit this to them to fix. What specifically should I be asking for? “Shadow DOM support?” should I refer them to this thread? I am not (this kind of) developer, but I want to help.

@SFoskett yes basically you would be asking for support for autofilling fields within shadow DOMs that are properly marked up with the autocomplete attribute. And I would definitely point them to the links I provided showing their competitors actively working on it.

Steve, the fix mentioned for Safari 16.6 does not solve the problem for the Apple password manager. When the Apple password manager has autofilled the fields, following error is thrown : Login attempt or request with invalid authentication from ...

@Roos-AID that's a separate issue likely caused by the hybrid approach I mentioned. See #16976.