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

Improve input attributes for password managers / accessibility #2396

Closed
7 of 12 tasks
woylie opened this issue Apr 14, 2022 · 11 comments
Closed
7 of 12 tasks

Improve input attributes for password managers / accessibility #2396

woylie opened this issue Apr 14, 2022 · 11 comments
Labels
feat New feature or request. stale Feedback from one or more authors is required to proceed.

Comments

@woylie
Copy link
Contributor

woylie commented Apr 14, 2022

Preflight checklist

Describe your problem

I'd like to add some attributes to help password managers understand Kratos forms better, and improve accessibility in general.

Describe your ideal solution

  • add autocomplete attribute to password inputs (new-password or current-password depending on the form) improvement: add autocomplete attributes #2523
  • add additional autocomplete attributes to email and name fields etc. (might need option in identity schema to set those) improvement: add autocomplete attributes #2523
  • add minlength and maxlength attributes to password inputs (when setting a new password)
  • add passwordrules attribute to password inputs (when setting a new password)
  • add id attributes to all input nodes, so that we can easily set the for attributes on <label> without generating IDs in the UI
  • ensure the selfservice UI applications set the aria-describedby attribute on inputs when there are input messages

References

Workarounds or alternatives

none

Version

v0.9.0-alpha.3

Additional Context

No response

@woylie woylie added the feat New feature or request. label Apr 14, 2022
@vinckr
Copy link
Member

vinckr commented Apr 14, 2022

This would be a feature request for https://github.com/ory/kratos-selfservice-ui-node, right?

@woylie
Copy link
Contributor Author

woylie commented Apr 14, 2022

No, that would be a feature request for Kratos (except maybe the last point - haven't thought about whether Kratos could make this easier as well yet). These attributes should be generated by Kratos and returned in the UI node attributes.

@aeneasr
Copy link
Member

aeneasr commented Apr 14, 2022

Thank you! I think the autocomplete is a really good idea. Regarding the other requests, I'm not too sure. The idea for this is really that you have a custom UI which implements whatever layout you want to have. For example, min/max-length are something we tests against and setting those as a requirement in the form field would prevent some tests from passing.

For IDs, while I think that the idea is solid, we can't really impose what ID structure you have in your UI. It might be conflicting what we generate and what you have in your UI already! So I'm a bit hesitant to dictate this.

Additionally, many of these HTML elements do not work like that on mobile apps or other clients which need to render forms.

Basically, we want to provide the bare minimum of info required to render the form successfully. All cosmetics around that are up to the implementor.

Not sure if that makes sense to you?

@woylie
Copy link
Contributor Author

woylie commented Apr 14, 2022

The minlength and maxlength attributes depend on the password policy. minlength would have to be the same value as min_password_length value in the configuration. For Bcrypt, we have a maximum password length of 72. So the maxlength would only be set if the hashing algorithm is Bcrypt. Since the values for these attributes depend on the configuration, I think it makes sense if Kratos returns them. Otherwise one might change the configuration at some point and forget to update the UI accordingly.

As for passwordrules: Since we don't restrict the password composition, the value would only depend on the min and max length of the password (minlength: 8; maxlength: 72; allowed: ascii-printable;). So if minlength and maxlength are returned by the API as described, it doesn't have to additionally return passwordrules. I retract the request :)

As for IDs: If they are consistently prefixed, there shouldn't be any conflicts (e.g. ory_kr_field_{input_name} or similar).

Basically, we want to provide the bare minimum of info required to render the form successfully. All cosmetics around that are up to the implementor.

That makes sense, but I also would argue that usability and accessibility are not cosmetics, and I think we should always make it easy to do the right thing.

@woylie
Copy link
Contributor Author

woylie commented Apr 14, 2022

Actually, having said that, in general, Kratos should help to enforce best practices. While it's easy to build the passwordrules attribute on your own (if the API returns minlength and maxlength), adding it to the API response would make it immediately visible to the implementer, thereby promoting it and in the end increase the number of websites that don't annoy me with bad password forms :)

Even if you don't want to add the attribute to the API response, at the very least we should add it to the example UI application to set a good example.

@woylie
Copy link
Contributor Author

woylie commented Apr 14, 2022

And another argument for adding it to the response would be that Kratos already knows what kind of flow its dealing with and can easily add it depending on the context, while my UI application just renders the UI nodes generically and would need additional logic to only add it depending on the flow. Not that this logic is difficult, but... I like things to be nice...

@aeneasr
Copy link
Member

aeneasr commented Apr 15, 2022

Thank you, you convinced me. That approach makes total sense.

One thing I thought of is - should we provide these default values for more then just plain HTML? I'm thinking about iOS and Android schemes here. But I'm not really sure whether this actually works - Cordova, React Native, Expo, I think they all use their own way of doing this (but I'm not 100% certain). But it would be great to be able to support the different client devices out of the box, to support an even wider range of use cases.

Do you have experience with this?

@woylie
Copy link
Contributor Author

woylie commented Apr 15, 2022

I don't know much about Cordova, React Native, and Expo. I would stick to plain HTML, otherwise you'll potentially add more and more attributes in the future, and then the next tool comes along. If you're rendering the UI nodes with anything other than standard HTML, you can easily convert a minlength integer into whatever attribute you need.

@aeneasr
Copy link
Member

aeneasr commented Apr 17, 2022

True, that makes sense!

@winston0410
Copy link

+1 for this, it would be great if the API would return minlength based on the configuration found here:

https://www.ory.sh/docs/kratos/guides/password-policy

@github-actions
Copy link

Hello contributors!

I am marking this issue as stale as it has not received any engagement from the community or maintainers for a year. That does not imply that the issue has no merit! If you feel strongly about this issue

  • open a PR referencing and resolving the issue;
  • leave a comment on it and discuss ideas on how you could contribute towards resolving it;
  • leave a comment and describe in detail why this issue is critical for your use case;
  • open a new issue with updated details and a plan for resolving the issue.

Throughout its lifetime, Ory has received over 10.000 issues and PRs. To sustain that growth, we need to prioritize and focus on issues that are important to the community. A good indication of importance, and thus priority, is activity on a topic.

Unfortunately, burnout has become a topic of concern amongst open-source projects.

It can lead to severe personal and health issues as well as opening catastrophic attack vectors.

The motivation for this automation is to help prioritize issues in the backlog and not ignore, reject, or belittle anyone.

If this issue was marked as stale erroneously you can exempt it by adding the backlog label, assigning someone, or setting a milestone for it.

Thank you for your understanding and to anyone who participated in the conversation! And as written above, please do participate in the conversation if this topic is important to you!

Thank you 🙏✌️

@github-actions github-actions bot added the stale Feedback from one or more authors is required to proceed. label Jun 19, 2023
peturgeorgievv pushed a commit to senteca/kratos-fork that referenced this issue Jun 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature or request. stale Feedback from one or more authors is required to proceed.
Projects
None yet
Development

No branches or pull requests

4 participants