-
Notifications
You must be signed in to change notification settings - Fork 12
Conversation
❌ Deploy Preview for reearth-web failed. 🔨 Explore the source changes: a913b27 🔍 Inspect the deploy log: https://app.netlify.com/sites/reearth-web/deploys/616d167235ee360007a6adf3 |
Codecov Report
@@ Coverage Diff @@
## main #86 +/- ##
=======================================
Coverage 53.53% 53.53%
=======================================
Files 48 48
Lines 891 891
Branches 119 119
=======================================
Hits 477 477
Misses 364 364
Partials 50 50 |
Update TextBox styles.
src/components/molecules/Settings/Account/PasswordModal/index.tsx
Outdated
Show resolved
Hide resolved
src/components/molecules/Settings/Account/PasswordModal/index.tsx
Outdated
Show resolved
Hide resolved
Move regex code out of function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It turns out that this PR actually contains a problem that we need to think about.
- Why do you limit passwords to between 5 and 25 characters? It is usually safe to not limit the maximum number of characters unless it is very long.
- Is the password policy in sync with the backend implementation and the OAuth provider's settings? If you don't have such a restriction in the backend or the provider, then trying to restrict it only in the frontend will result in a restriction that doesn't make much sense from a security perspective.
- Don't forget that users can also change the password from the login page provided by the OAuth provider. What if a different password policy is applied there?
Re:Earth is OSS, so we never know who will use it and where will be used it, and even what OAuth2 provider will be used. Considering the possibility that various password policies may be required, it would be desirable to be able to change the password policy in, for example, reearth_config.json.
src/components/molecules/Settings/Account/PasswordModal/index.tsx
Outdated
Show resolved
Hide resolved
Regenerate.
Additional implementation: to make password policy changeable by
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your implementation. 👍
I've left a couple of comments. Plz, check them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems good! 👍
Overview
The passwordModal for changing your password merely disables the submit button unless new password and confirmation password match. This adds validation and feedback to make sure users are using secure passwords.
What I've done
Please check
Memo