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

ShadowDom and Password managers #22

Closed
kant01ne opened this issue Nov 3, 2020 · 5 comments
Closed

ShadowDom and Password managers #22

kant01ne opened this issue Nov 3, 2020 · 5 comments
Assignees
Labels
enhancement New feature or request

Comments

@kant01ne
Copy link
Contributor

kant01ne commented Nov 3, 2020

Autocomplete for extensions such as 1Password, Dashlane and Lastpass does not work within shadow dom:

Without shadow dom:
image

With shadow dom
image

It only works when shadow dom is disabled. This is not an issue related to us directly as stated in https://1password.community/discussion/79137/shadow-dom-polymer-forms-do-not-fill-companion-extension-v4-7-4-now-supports-shadow-doms

Basically 1Password does a querySelectorAll which won't work with shadow Dom. That is their responsibility to catch shadow root dom component in the future with their extensions. It seems pretty straightforward and I don't understand why they haven't done such a thing yet (maybe not a top priority for them).

I think it's pretty important for us as an open source authentication solution to support password managers.
We can't provide support for password managers while shadow root is enabled at the moment. Password manager providers do not seem to be actively working on solving the issue.

Bunch of other open source frameworks are facing this issue: Ionic, Polymer.
Other resource on the subject: WICG/webcomponents#572

Solution proposed

In case they want support for password managers, we could provide an "obscure" config to allow users to disable shadow dom letting them know that doing such a thing might result in CSS clashes because they are removing CSS encapsulation.

SuperTokens.init({
  appName: "",
  {...},
  useShadowDom: false // default true
  • They should care about Supertokens components styling mainly as we are using a solution that makes sure there is no clashes with their CSS EVEN IF shadow root is disabled.
  • Only CSS that we are loading and that would be loaded on their website is the fontFamily Rubik and it should not impact their website but they should make sure.
@rishabhpoddar
Copy link
Member

Okay. A few points:

  • shadowDom should go in the emailpassword level, not in the init level
  • We do not need to mention this in the docs as it's hard to communicate. If someone really has this problem, they can msg us, and then we can tell them and then add it to the docs later perhaps.

@rishabhpoddar
Copy link
Member

rishabhpoddar commented Nov 21, 2020

@NkxxkN I had a discussion about this with Advait and he is of the opinion that the lack of a password manager not recognising is very obvious. So we should document this.

@makerboiAdi also suggested that we can have some indication of this in the demo app:
An idea for how to document this is that you could put it in the demo. Like a little line at the bottom saying "Why didnt google autosuggest a password?".

In order to do the above though, we will have to maintain two repos of the demo app - one for the actual site that will have the above info, and one for as a sample repo for people to see while coding as a guide. Not sure if it's worth it as of yet.

I am reopening this issue for discussion

@rishabhpoddar rishabhpoddar reopened this Nov 21, 2020
@kant01ne
Copy link
Contributor Author

kant01ne commented Nov 21, 2020

Documentation for shadowDom option:

I agree it is important and should be documented somehow.

An idea for how to document this is that you could put it in the demo. Like a little line at the bottom saying "Why didnt google autosuggest a password?".

It's not really "Google" or "Google Chrome". Shadow dom is preventing the rendering of password managers such as:

  • LastPass
  • Dashlane
  • 1Password

In order to do the above though, we will have to maintain two repos of the demo app - one for the actual site that will have the above info, and one for as a sample repo for people to see while coding as a guide. Not sure if it's worth it as of yet.

I don't think we should have two repos.
One thing I did in the test app to allow/disallow such configs is to leverage GET params.

See https://github.com/supertokens/supertokens-auth-react/blob/master/test/react-test-app/src/App.js#L16 how the dark theme can be switch on.
We could use the same trick for shadowDom in the demo app.

Why are we trying to hide the documentation in the demo app though? I think this is a real subject and this could benefit from a proper section in the detailed guide in my opinion.

Pull request adding autocomplete password suggestion:

In current version we do not have Browser autosuggest setup correctly: it didn't include new password suggestion.
This PR adds a better autocomplete suggestion. #59

State of the art for password auto complete:

TL;DR:

  • Autocomplete is hard and inconsistent between browsers! It will be hard to get it right for all of them!
  • Password manager have different ways

Research

Browser researched: Chrome / Firefox / Safari / Opera
Password managers: LastPass / Dashlane

Results

In the results displayed below, I never managed to get the Chrome autosuggest password to work though I'm pretty sure that I've seen it before. If you find a way to make it work, or see something that I've done wrong please let me know so I can complete.

Firefox:

  • No Shadow Dom:
    • Browser Autosuggest Email autocomplete="on"
    • Browser Autosuggest Current Password autocomplete="current-password"
    • Browser Autosuggest New Password autocomplete="new-password"
    • Password Managers
  • Shadow Dom
    • Browser Autosuggest Email autocomplete="on"
    • Browser Autosuggest Current Password autocomplete="current-password"
    • Browser Autosuggest New Password autocomplete="new-password"
    • Password Managers

Chrome:

  • No Shadow Dom:
    • Browser Autosuggest Email autocomplete="on"
    • Browser Autosuggest Current Password autocomplete="current-password"
    • Browser Autosuggest New Password autocomplete="new-password"
    • Password Managers
  • Shadow Dom
    • Browser Autosuggest Email autocomplete="on"
    • Browser Autosuggest Current Password autocomplete="current-password"
    • Browser Autosuggest New Password autocomplete="new-password"
    • Password Managers

Safari:

  • No Shadow Dom:
    • Browser Autosuggest Email autocomplete="on"
    • Browser Autosuggest Current Password autocomplete="current-password"
      => Instead new-password is suggested...
    • Browser Autosuggest New Password autocomplete="new-password"
    • Password Managers
  • Shadow Dom
    • Browser Autosuggest Email autocomplete="on"
    • Browser Autosuggest Current Password autocomplete="current-password"
    • Browser Autosuggest New Password autocomplete="new-password"
    • Password Managers

Opera:

  • No Shadow Dom:
    • Browser Autosuggest Email autocomplete="on"
    • Browser Autosuggest Current Password autocomplete="current-password"
    • Browser Autosuggest New Password autocomplete="new-password"
    • Password Managers
  • Shadow Dom
    • Browser Autosuggest Email autocomplete="on"
    • Browser Autosuggest Current Password autocomplete="current-password"
    • Browser Autosuggest New Password autocomplete="new-password"
    • Password Managers

Examples of good use cases:**

Dashlane on Chrome
Dashlane-3

Autosuggest on Firefox (Shadow dom or not)

Safari Strong Password
safari-strong-password

Lastpass on Chrome with error fields
lastpass-with-error-fields

Firefox Email Password suggestion
lastpass-with-error-fields

Opera without shadow dom

Password Managers inconsistencies:

Password managers have different ways to display their UI in form fields, making it a case by case issue if we want to make sure there are no conflict with error fields / password managers UI. (Unless we come up with a UI that is keeping the right side of the inputs for password managers and not insert input success/error icons).

LastPass Click
lastpass-click

  • Clicking in the red circle area does not work.
  • You need to click on the green circle area to display the LastPass background. This is because of the current implementation to make sure that field errors and password manager are not shown on top of each other. This can be fixed but will require extra work.

Dashlane with error fields
Dashlane-3

Dashlane seems to be aware of this issue and tries to solve it by moving their icon themselves... with limited results....

@rishabhpoddar
Copy link
Member

Thanks. Will discuss this over a call in detail

@kant01ne
Copy link
Contributor Author

kant01ne commented Nov 24, 2020

TODO:

  • Implement new designs in 0.2 with input error /!\ emoji close to text error. form validation on blur #73
  • Create a doc section about password manager (and turning off shadowDom option)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants