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

TextInput: Password management browser extensions incompatible with shadow DOM #225

Open
kentjas1 opened this issue Sep 22, 2021 · 12 comments

Comments

@kentjas1
Copy link

Expected behavior
When leveraging password management browser extensions (e.g. 1Password, LastPass), the browser plug in/extension can populate the un/pw fields so I don't have to manually enter.

Actual behavior
The browser extension doesn't recognize Pharos input fields to populate values.

Steps to reproduce the issue

  1. Go to the Admin login page
  2. Attempt to use a password management extension to populate the username/password fields.
  3. Observe the extension not working.

Pharos version
Pharos v10.7.0

Your environment

  • OS: MacOS
  • Browser: Chrome
  • Version: 93.0.4577.82

Additional information
The assumption here is that because web components are not technically in the DOM, the browser extension doesn't have access/detect to the fields. This may not be something that needs direct attention/can be solved but more of a discussion on if this browser extension support is something to be desired.

@daneah daneah changed the title Password management browser extensions incompatible with web component inputs. TextInput: Password management browser extensions incompatible with shadow DOM Sep 22, 2021
@daneah
Copy link
Member

daneah commented Sep 23, 2021

This research suggests that adding the appropriate autocomplete attributes will give some compatibility, with hopefully growing compatibility moving forward.

@michael-iden
Copy link
Contributor

Short narrative on the user value and business risk:

To me not supporting PW managers is almost a knock against our JSTOR brand and by extension pharos. It feels antiquated.

Recent market research saying 20-24% of people use password managers. Also note that the “saved into browser” may also not work (did not for me on Firefox) and thats 24% from this first report. That means login is harder for everyone using pharos inputs for nearly 50% of users.
https://www.security.org/digital-safety/password-manager-annual-report/
https://www.comparitech.com/blog/information-security/password-statistics/#:~:text=24%25%20of%20people%20use%20a%20password%20manager&text=While%20many%20users%20say%20they,they%20used%20a%20password%20manager.f

This has lower usage numbers but from 2016 (6+ years old). And combining browsers as the password manager takes it to 30%
https://www.pewresearch.org/internet/2017/01/26/2-password-management-and-mobile-security/

@michael-iden
Copy link
Contributor

michael-iden commented Sep 27, 2022

I couldn't get any consistent behavior from the various browsers when trying the autocomplete workaround behavior listed above.

Chrome:
https://groups.google.com/a/chromium.org/g/blink-dev/c/RY9leYMu5hI?pli=1
https://chromestatus.com/feature/5769419581030400

Firefox:

Webkit in various forms:

Getting a little support here and there for the browser's own password manager but its not consistent enough for me in any browser to say its at feature parity with non-shadow DOM elements

@adamdehaven
Copy link

adamdehaven commented Sep 28, 2022

For what it's worth, I've been following this thread since it's inception and have solved the issue for my Vue lib. I don't utilize your repo, but followed along to see how you talked through it.

TL;DR password managers need to support traversing the shadow DOM when searching for valid elements, but most choose not to.

I currently work mainly in Vue and have managed to get all password managers to work with components mounted within a shadow DOM by teleporting the elements out of the shadow DOM on mount with Vue's native <Teleport /> component.

This works for my scenario because I need to package my Vue components as native web components so that they can ship with their own version of Vue and other dependencies in the bundle that differ from the host application.

I believe you could do the same with a React Portal.

There are some necessary adjustments needed, such as explicitly scoping your component styles to a wrapper class, which prevent the styles from portentously leaking into the host application. (I did this utilizing sass:meta by wrapping @include meta.load-css("./components")) in a parent class.

In my own library, there are no vulnerabilities associated with the components "exiting" the isolation of the shadow DOM; however, this obviously isn't the case for every type of component.

Edit: added some extra detail

@daneah
Copy link
Member

daneah commented Sep 28, 2022

Thanks for the detailed tactic suggestions @adamdehaven! We'll take them under advisement. Because we're using Lit here I'm less sure of our faculties for a teleport/portal-esque approach; we'd potentially need to manage that ourselves if we hope to have consumers continue using these transparently agnostic of framework.

@adamdehaven
Copy link

Ah. I suggested React only because I saw it in your dependency tree.

@adamdehaven
Copy link

FWIW I explored the other, eh, "hacks" outlined above and can confidently say I wouldn't go that direction.

Also, don't inject "hidden" HTML elements into the DOM to try and produce a workaround; it becomes a nightmare for users that utilize screen-readers and other assistive tech.

@daneah
Copy link
Member

daneah commented Sep 28, 2022

Ah. I suggested React only because I saw it in your dependency tree.

We do provide light wrappers around the web components so that React plays a bit more nicely by default, but ultimately we're providing these to folks using React, Vue, Angular, and frameworkless.

@michaelwarren1106
Copy link

following along here too for my own design system of lit components. i’m thinking of an approach where the input/label are light dom children of a shadow dom component so that password managers can find the inputs.

my concerns are how the value gets added and detecting that. i do t think there’s an event fired when the value is auto filled right? if there was, id add a listener to the light dom input and when it’s fired, copy the value into my shadow dom input or something.

the cool thing about shadow dom and light dom children is that they aren’t shown if there no slot. so they would just need to be screen readers hidden in favor of the shadow dom input.

so that just leaves figuring out if there’s an event to attach to, or something that a mutationobserver could observe.

@daneah
Copy link
Member

daneah commented Oct 26, 2022

@michaelwarren1106 That's an interesting tactic! I'd hope there are still input and change events fired appropriately for the inputs that get autofilled, but I've honestly not worked enough with it to say if that's the case. I would really hope a MutationObserver would be able to catch it, in the worst case! If you go further down this road we'd appreciate any findings you can share! I'll try to find some time to investigate on our end.

@michaelwarren1106
Copy link

i suspect (without testing it at all) that password managers are just setting the value property directly, which won’t automatically emit any events. if the mgrs also manually emit events that would be great, but might be a security risk?

of course all the password managers don’t have any dev docs describing how they work or listing events they emit etc

@michael-iden
Copy link
Contributor

michael-iden commented Dec 27, 2023

It appears that something has been updated between 1Password and Chrome that allows for autofilling credential inputs within the shadow DOM. Pharos input change events were also firing for me after password manager autofill, which allowed for input validation

edit: Looks like there was a recent 1Password fix highlighted in this post from 12/8
https://1password.community/discussion/143571/usability-digest-dec-2023-improved-autofill-reliability-lock-state-and-item-title-generation#:~:text=Improved%20Autofilling%20for%20Sites%20with%20Shadow%20DOM%20Elements

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: No status
Development

No branches or pull requests

5 participants