Giter Club home page Giter Club logo

Comments (52)

fbiel avatar fbiel commented on July 28, 2024 22

Personally, I don't think this should be labeled as a feature-request, this is a bug. Users are really confused about this.

Currently, there is no point in using the password component.

from amplify-ui.

adamjmarkham avatar adamjmarkham commented on July 28, 2024 21

This is a blocker for us (and probably most people) to upgrade our version of Amplify without a workaround. Looks like this bug seems to have been there for a while, any news on when it will be fixed?

from amplify-ui.

24601 avatar 24601 commented on July 28, 2024 10

The use of the shadow DOM is highly problematic, not just for password managers, but for testability, as DOM selectors need to be manually altered to work around the shadow DOM.

This should no doubt be a bug.

from amplify-ui.

wlee221 avatar wlee221 commented on July 28, 2024 9

With regards to our current components (before we publish the rewrite), we are looking at workaround that’ll allow saving username/password and autofilling credentials based on that (branch PR aws-amplify/amplify-js#8181).

Kapture 2021-04-20 at 16 46 09 (1)

This workaround does not support dropdown for multiple credentials, but this will at least allow for a credential save and autofill (edit: on chrome and firefox) while we work on the rewrite. Please let us know with any feedback or suggestions. We'll work on getting the workaround tested and reviewed meanwhile.

from amplify-ui.

wlee221 avatar wlee221 commented on July 28, 2024 8

Hello all, we just merged in the password manager workaround (pr aws-amplify/amplify-js#8181). This is now available in unstable npm tag and will be published in the next release cycle. To use this workaround, you need to wrap your authenticator with AmplifyAuthContainer:

<AmplifyAuthContainer>
  <AmplifyAuthenticator>
    ...
  </AmplifyAuthenticator>
</AmplifyAuthContainer>

(or with amplify-auth-container on vue/angular). Under the hood, AmplifyAuthContainer contains a hidden form that is not shadowed and thus gets autofilled. Then we use event listeners to send that autofill down to the amplify-authenticator.

This workaround works for Chrome and Firefox. Safari does not trigger credentials save even when form shadows are disabled, so this will be a limitation until our rewrite. Please let us know with any questions or issues with this solution. We'll keep this issue open to track this until this is completely resolved.

from amplify-ui.

p0fi avatar p0fi commented on July 28, 2024 7

Any news on this topic? This is a really annoying bug!

from amplify-ui.

richiewebgate avatar richiewebgate commented on July 28, 2024 3

same issue with aws-amplify-angular

from amplify-ui.

eddiekeller avatar eddiekeller commented on July 28, 2024 3

Hi @strugman , we have added support for password managers to our new Authenticator, which is currently under the @next tag.

Link to our documentation - https://ui.docs.amplify.aws/ui/components/authenticator?platform=react

And a link to a small test checking for support - https://github.com/aws-amplify/amplify-ui/blob/main/packages/e2e/features/ui/components/authenticator/sign-up-with-username.feature#L30-L33

If you'd like, give it a try and see if the new version works better for you!

from amplify-ui.

anantoghosh avatar anantoghosh commented on July 28, 2024 2

Vaadin login component handles this by not using the shadow dom for the form
https://github.com/vaadin/vaadin-login/blob/9e76baf5e8a7dccf5e65116d0fc91b49ea3bdf2a/src/vaadin-login-form.html#L50

from amplify-ui.

PeterBurner avatar PeterBurner commented on July 28, 2024 2

does not work with @aws-amplify/ui-vue either

from amplify-ui.

TAConsulting avatar TAConsulting commented on July 28, 2024 2

Any updates on this?

from amplify-ui.

cburkins avatar cburkins commented on July 28, 2024 2

I'm currently enjoying the simplicity of using the withAuthenticator HOC. As such, am I easily able to deploy this ? I'm not sure how to apply "you need to wrap your authenticator with AmplifyAuthContainer"

from amplify-ui.

wlee221 avatar wlee221 commented on July 28, 2024 2

@cburkins, we've given this more thought and we will be modifying the HOC to include this workaround by default. We'll create a PR soon.

from amplify-ui.

wlee221 avatar wlee221 commented on July 28, 2024 2

@ClaudioVR, No let me clarify this a little bit.

<amplify-auth-container>
  <amplify-authenticator></amplify-authenticator>
</amplify-auth-container>

I have a docs PR aws-amplify/docs#3224 that I'll merge soon.

from amplify-ui.

wlee221 avatar wlee221 commented on July 28, 2024 2

It will. The rewrite will stick with the best html practices to support a broad variety of password managers, which really boils down to not using shadow DOM.

from amplify-ui.

wlee221 avatar wlee221 commented on July 28, 2024 2

Hello, apologies on the late response here. The new authenticator is being actively developed in a separate repo aws-amplify/amplify-ui repository. This version will have full support for both browser and third party password managers (addressed specifically in #357. We are looking to make our first pre-release soon, after which you can try new versions for feedbacks or early adoption. The progress can be tracked in the ui repo.

This new version makes multiple improvements that are listed in detail in RFC: Authenticator@next on top of password manager support, so we'd appreciate any feedback there. Thanks for your patience while we work through it, and as always feedbacks / contributions are welcome.

from amplify-ui.

Purii avatar Purii commented on July 28, 2024 1

Seems that the usage of shadow DOM is the root cause: https://bugs.chromium.org/p/chromium/issues/detail?id=770175

from amplify-ui.

fdobrovolny avatar fdobrovolny commented on July 28, 2024 1

@ryan-mars The issue is using of shadow dom for the form (not using plain HTML for the form). I tried to force the components to be in compatibility mode.

It works on Firefox. On chrome, it only saves the password but it won't let you use it.

It could be hacked together with this https://developers.google.com/web/fundamentals/security/credential-management to make it work but it's not a solution.

from amplify-ui.

DavidNelsonCordelia avatar DavidNelsonCordelia commented on July 28, 2024 1

Any progress on this? its a unpleasant missing feature.

from amplify-ui.

wlee221 avatar wlee221 commented on July 28, 2024 1

You're welcome! We just merged aws-amplify/amplify-js#8243 to support withAuthenticator -- you shouldn't need to update any code to use this workaround. This has some restrictions as stated before, but hopefully this will alleviate some of the accessibility pain while we work on the rewrite.

The fix will hit unstable soon and will be published in the next release cycle. We'll fix this issue more completely on our rewrite (without hidden forms πŸ˜„ ) and close this issue then. I'll mark this back to feature-request as a workaround has been provided. Thanks for your patience!

from amplify-ui.

hrietman avatar hrietman commented on July 28, 2024 1

@patti-berchi do you mean you got the password-manager working? I am using 1password, latest versions
...
"aws-amplify": "^4.2.1",
"@aws-amplify/ui-components": "^1.7.0",
"@aws-amplify/ui-vue": "^1.1.1",
...

Reinstalled everything; added even the formfields with autocomplete; no luck sofar.

@wlee221 how long for the version without the shadow-dom (or is it released and I missed it completely .... )

from amplify-ui.

ericclemmons avatar ericclemmons commented on July 28, 2024 1

@markcarroll Yes we do! https://github.com/aws-amplify/amplify-ui/milestone/1

from amplify-ui.

kjellski avatar kjellski commented on July 28, 2024

This looks like a bug to every end-user - any hints on how to help fix this here?

from amplify-ui.

fdobrovolny avatar fdobrovolny commented on July 28, 2024

So I tried to fix this. Following upon @anantoghosh I googled and found out that the most likely issue is that the form fields are scattered across different shadow DOM roots.

I also noticed this commit aws-amplify/amplify-js@538ca66

So I tried to undo some of it works. And instead of using shadow dom for the whole thing I tried to force it to use fallback (scoped: true https://stenciljs.com/docs/component#component-decorator).

When I applied this to authenticator and everything underneath this have happend:

Pokus-1

<amplify-authenticator>
  <amplify-sign-in slot="sign-in" hideSignUp="true" [formFields]="this.signInFormFields"></amplify-sign-in>
</amplify-authenticator>

Then I tried to apply it only from form down. Which initialy seemed to be working. Both chrome and firefox now saves the password properly. Unfortunately in chrome you can not use saved credentials, on firefox this works.

I'm out of ideas maybe someone can make it work.

fdobrovolny/amplify-js@fe91c7e

from amplify-ui.

simon-lanf avatar simon-lanf commented on July 28, 2024

Please fix this, it is also affecting almost every testing frameworks. You can't use Selenium, Cypress and stuff like that.

from amplify-ui.

ericclemmons avatar ericclemmons commented on July 28, 2024

There's a lot of research in this thread to look into, thanks for that!

In the meantime, @simon-lanf can you provide more info on you how Cypress isn't working? We have Cypress tests using commands like the following successfully:

cy.get(selectors.usernameInput).type(COGNITO_SIGN_IN_USERNAME);
cy.get(selectors.signInPasswordInput).type(COGNITO_SIGN_IN_PASSWORD);
cy.get(selectors.signInSignInButton).contains('Sign In').click();
``

from amplify-ui.

ryan-mars avatar ryan-mars commented on July 28, 2024

This makes @aws-amplify/ui-react a no-go for us. Password managers are a healthy security pattern.

Happy to contribute but I'm not sure where to start.

from amplify-ui.

ryan-mars avatar ryan-mars commented on July 28, 2024

@BrnoPCmaniak does this change work? ---> fdobrovolny/amplify-js@fe91c7e

from amplify-ui.

Purii avatar Purii commented on July 28, 2024

Slots might help fixing the issue: https://stenciljs.com/docs/templating-jsx#slots
That way the Input field is not "hidden" in the shadow dom.

Unfortunately I can't spare the time to try it out at the moment..

from amplify-ui.

richiewebgate avatar richiewebgate commented on July 28, 2024

I will unsubscribe now, because this will never be fixed.

from amplify-ui.

wlee221 avatar wlee221 commented on July 28, 2024

Hello all, we apologize about the lack of response here. As pointed out, this is due to the use of shadow DOM. We will mark this as a bug.

We found that disabling the shadow DOM with current UI components is problematic, because slots (which we use to allow customization) do not work well with shadow DOM disabled. In particular, conditional rendering (ionic-team/stencil#848), default rendering (ionic-team/stencil#2054), and nested slots (ionic-team/stencil#399) do not work, which are patterns we use throughout the library.

For that reason, we are currently working on rewriting our ui-components to not use Web Components / shadow DOM. This will make the form elements accessible to browsers and enable password managers to work. We will hold a public RFC and create a public repo rewrite soon to solicit feedbacks. We will let you know when we publish the RFC.

from amplify-ui.

wlee221 avatar wlee221 commented on July 28, 2024

@cburkins, you would have to eject out of withAuthenticator to use this workaround:
EDIT: see comment below -- we can have the HOC to include this workaround by default.

from amplify-ui.

wlee221 avatar wlee221 commented on July 28, 2024

Hi all, I've opened aws-amplify/amplify-js#8243, which will include this workaround to withAuthenticator by default. We'll get this reviewed and merged this week.

I've also published the patch to ui-preview npm tag for testing purposes. Please note that you can use this tag to unblock yourself and try this out. Let us know if you catch any unexpected behaviors. Thank you!

yarn add aws-amplify@ui-preview @aws-amplify/ui-react@ui-preview

from amplify-ui.

cburkins avatar cburkins commented on July 28, 2024

@wlee221 This is great ! Thanks for the consideration.

from amplify-ui.

cburkins avatar cburkins commented on July 28, 2024

@wlee221 Thanks ! Tested with native Chrome autofill, and seems to work ! Am I correct in saying that this won't currently work within 3rd party password managers like LastPass ? Doesn't seem like it works.....

from amplify-ui.

wlee221 avatar wlee221 commented on July 28, 2024

Right, this workaround is intended for browser password managers.

Support for third party password manager really depends on their implementation and whether they can transverse through the top level shadow DOM or not. For instance 1password supports shadow DOM (discussion) while LastPass does not (forum).

from amplify-ui.

ClaudioVR avatar ClaudioVR commented on July 28, 2024

@wlee221 Regarding what you said: "you shouldn't need to update any code to use this workaround"...

does this also stand for Vue.js?

from amplify-ui.

fosrias avatar fosrias commented on July 28, 2024

while we work on the rewrite.

@wlee221 Just to be crystal clear in this thread, is the re-write being done to accommodate a broad variety of password managers or not? Thanks.

from amplify-ui.

sammartinez avatar sammartinez commented on July 28, 2024

aws-amplify/amplify-js#8289

from amplify-ui.

tommulkins avatar tommulkins commented on July 28, 2024

Just checking in to see if there's an update on this. It appears the Amplify Admin UI suffers from the same problem. At least AWS is dog-fooding their own product πŸ˜†

from amplify-ui.

ericclemmons avatar ericclemmons commented on July 28, 2024

@tommulkins It could be for a different reason! Can you open an issue here so they can improve that separately?

https://github.com/aws-amplify/amplify-adminui

Thanks!

from amplify-ui.

thetrevdev avatar thetrevdev commented on July 28, 2024

Is there a rough ETA for a fix that would include Safari?

from amplify-ui.

patti-berchi avatar patti-berchi commented on July 28, 2024

@wlee221 I have tried the solution above and wrapped AmplifyAuthenticator with AmplifyAuthContainer but I get the following error:

Element type is invalid: expected a string (for built-in components) or a class/function (for composite components) but got: undefined. You likely forgot to export your component from the file it's defined in, or you might have mixed up default and named imports. Check the render method of AuthStateApp.

I have installed the last unstable version ([email protected]) and I'm importing AmplifyAuthenticator from "aws-amplify" + wrapping as follows

            <AmplifyAuthContainer>
              <AmplifyAuthenticator hideToast>
                <AmplifySignUp
                  slot="sign-up" formFields={[...]}  />
                <AmplifyConfirmSignUp
                  slot="confirm-sign-up" >
               </AmplifyConfirmSignUp>
              </AmplifyAuthenticator>
            </AmplifyAuthContainer>

Any ideas?

from amplify-ui.

wlee221 avatar wlee221 commented on July 28, 2024

@patti-berchi, I was able to get AmplifyAuthContainer working on latest React -- codesandbox

Do you still experience this issue? If so, can you open a new issue with more context? I'll be happy to take a look at it!

from amplify-ui.

patti-berchi avatar patti-berchi commented on July 28, 2024

@wlee221 Fixed! after hours of tweaking and trying different things just uninstalling and reinstalling the modules again seems to have solved it. Thanks though!

from amplify-ui.

gornypiotr avatar gornypiotr commented on July 28, 2024

This is great news! Is there any roadmap so we could see the timeline and plan accordingly?

from amplify-ui.

ericclemmons avatar ericclemmons commented on July 28, 2024

@strugman We're aiming to have the preview available within a few weeks, so late September πŸ™

from amplify-ui.

andre347 avatar andre347 commented on July 28, 2024

Hi @strugman , we have added support for password managers to our new Authenticator, which is currently under the @next tag.

Link to our documentation - https://ui.docs.amplify.aws/ui/components/authenticator?platform=react

And a link to a small test checking for support - https://github.com/aws-amplify/amplify-ui/blob/main/packages/e2e/features/ui/components/authenticator/sign-up-with-username.feature#L30-L33

If you'd like, give it a try and see if the new version works better for you!

Thanks! This is a massive improvement because lots of my users reported issues with password managers such as LastPass. However, could you provide some more code snippets that show how to use this with the 'AmplifyAuthenticator' (which I think is now replaced with the 'Authenticator') and how to use the 'AmplifySignIn' and 'AmplifySignUp' components/slots. I used those in my React project to add in custom fields. My user sign up with email but also create a username. But I don't know how to add this field to the new 'Authenticator' component. This component only seems to expose the 'initialState' and 'loginMechanisms'.

from amplify-ui.

eddiekeller avatar eddiekeller commented on July 28, 2024

Yep so the new authenticator is still under our @next tag, which basically means it is in a "developer preview" of sorts. Things like the use case you are mentioning are still to come.

But, I will be closing this issue now as password manager support (the original topic of the ticket) is included in the new version. @andre347 Please open a new ticket for us detailing the use case you are talking about. As soon as that is added (and it will be added), we will update you promptly!

from amplify-ui.

markcarroll avatar markcarroll commented on July 28, 2024

Glad this is close to resolution. @eddiekeller is there a tracking issue for the move from @next to full release that we can follow?

from amplify-ui.

tswacast avatar tswacast commented on July 28, 2024

Apologies for adding a comment to a closed thread, but am I correct in my understanding that the current @next version of @aws-amplify/ui-react should work with 3rd party password managers? I have a minimal react project where I implemented a basic workflow wrapping the App component in AmplifyProvider and then exporting default App using the withAuthenticator HOC. LastPass will autocomplete the username and password, but when I submit the form (sign in, sign up, or change password), LastPass does not detect and prompt me to save the username/password. The Firefox password manager is working properly, but LastPass doesn't ever prompt to save. I can include my code if helpful, or open a new thread, but for now I just want to make sure that with the current @next version that it should be working.

from amplify-ui.

aaronfay avatar aaronfay commented on July 28, 2024

For anyone coming into this thread struggling with Selenium, Cypress, or Ghost Inspector (et al) and trying to automate a sign in using these components, I've found the following workaround. The above <amplify-auth-container> suggestion adds a hidden form to the page which can be used to interact with the actual form:

Assign your username

Use this selector:

[name="username"]

Execute JavaScript to trigger the proper form change

document.querySelector('[name="username"]').dispatchEvent(new Event('input'))

Assign password

Use this selector:

[name="password"]

Execute JavaScript again to update the proper form

document.querySelector('[name="password"]').dispatchEvent(new Event('input'))

Click the submit button

This proved to be a little more complex, one more JavaScript is necessary:

document.querySelector('amplify-authenticator')
  .shadowRoot.querySelector('amplify-sign-in')
  .shadowRoot.querySelector('amplify-form-section')
  .querySelector('.button').click()

Hope that helps anyone else struggling with test automation.

from amplify-ui.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    πŸ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. πŸ“ŠπŸ“ˆπŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❀️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.