ory / kratos

The most scalable and customizable identity server on the market. Replace your Homegrown, Auth0, Okta, Firebase with better UX and DX. Has all the tablestakes: Passkeys, Social Sign In, Multi-Factor Auth, SMS, SAML, TOTP, and more. Written in Go, cloud native, headless, API-first. Available as a service on Ory Network and for self-hosters.
https://www.ory.sh/?utm_source=github&utm_medium=banner&utm_campaign=kratos
Apache License 2.0
11.19k stars 959 forks source link

Improve input attributes for password managers / accessibility #2396

Closed woylie closed 1 year ago

woylie commented 2 years ago

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

References

Workarounds or alternatives

none

Version

v0.9.0-alpha.3

Additional Context

No response

vinckr commented 2 years ago

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

woylie commented 2 years ago

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

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

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

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

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

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

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

True, that makes sense!

winston0410 commented 2 years ago

+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[bot] commented 1 year ago

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

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 🙏✌️