yugabyte / yugabyte-db

YugabyteDB - the cloud native distributed SQL database for mission-critical applications.
https://www.yugabyte.com
Other
8.92k stars 1.06k forks source link

[Platform] UI: Enforce configured password policy #8152

Open anmalysh-yb opened 3 years ago

anmalysh-yb commented 3 years ago

Configurable password policy was implemented here on backend side: https://github.com/yugabyte/yugabyte-db/issues/7939 However, on UI side it's still hardcoded. Need to get configured password policy from backend and apply it on UI side. Password policy can be retrieved from CustomerConfigController as CustomerConfig object with type==PASSWORD_POLICY.

rajmaddy89 commented 3 years ago

@streddy-yb or @iSignal : Can you please assign this issue to my name

anmalysh-yb commented 3 years ago

Actually, we have at least 3 issues with CustomerConfigController:

  1. It have different methods to create and update config. Not transient for the client.
  2. It has no method to get configured values OR default. Without that - UI code will need to have defaults hardcoded.
  3. It’s raw json - the only way to find out the exact object which need to be passed to the API - is to dig into backend code.

I’d say that at least #2 should be addressed on the backend side These are common issues for all types of customer config - so probably there should be some common fix.

rajmaddy89 commented 3 years ago

@anmalysh-yb : I am Raj and I just started working on this issue, the below is my plan for this story, do you have any thoughts on the steps:

  1. Create a method in PasswordPolicyService.java to return(expose) PolicyPasswordFormData
  2. Create a new endpoint in CustomerConfigController.javawhich should call the above function
  3. The above end point should return a plain old Java Object which will contain the policy information of password
  4. Now client side REGISTER function in register.js should call the above API which in turn will give us the policy and we will show validation based on that
rajmaddy89 commented 3 years ago

PR: https://github.com/yugabyte/yugabyte-db/pull/9210

rajmaddy89 commented 3 years ago

Can this issue be closed now ?

anmalysh-yb commented 3 years ago

I took a quick look into code review. It seems like UI code is still using old regexp for password policy check:

https://github.com/yugabyte/yugabyte-db/pull/9210/files#diff-62db5af8d2a399fe2d5ebe276571f87df6d5c5c962b02265556aee399ef1a541L47

Basically you just changed the error message.

Also, One more place which applies password policy on UI is UserProfileForm.js - which is not covered in your PR.

One more note - your getPasswordPolicyData() method in PasswordPolicyService seems weird. It should get policy, configured for customer first (so at least it should get customer ID as an input). And only if it's missing - should use default values from config object. See checkPasswordPolicy method in the same class for details on how effective password policy is calculated - you can actually separate effective policy calculation to reuse in both these methods.

I'd fix these items first - as currently UI is still checking for hardcoded policy.

@rajmaddy89

rajmaddy89 commented 3 years ago

Sure, thanks for reviewing it. I will do the changes this week, I have a question, how does a configured policy looks like, the reason I am asking is when a user REGISTERS in the Registration page or User ProfileForm Page, they are not still customers at that point, they will be assigned CUSTOMER ID only after they finish registration. So my question is how will new users have PASSWORD POLICY.

Regards, Rajagopalan Madhavan

On Tue, Jul 20, 2021 at 5:14 AM anmalysh-yb @.***> wrote:

I took a quick look into code review. It seems like UI code is still using old regexp for password policy check:

https://github.com/yugabyte/yugabyte-db/pull/9210/files#diff-62db5af8d2a399fe2d5ebe276571f87df6d5c5c962b02265556aee399ef1a541L47

Basically you just changed the error message.

Also, One more place which applies password policy on UI is UserProfileForm.js - which is not covered in your PR.

One more note - your getPasswordPolicyData() method in PasswordPolicyService seems weird. It should get policy, configured for customer first (so at least it should get customer ID as an input). And only if it's missing - should use default values from config object. See checkPasswordPolicy method in the same class for details on how effective password policy is calculated - you can actually separate effective policy calculation to reuse in both these methods.

I'd fix these items first - as currently UI is still checking for hardcoded policy.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/yugabyte/yugabyte-db/issues/8152#issuecomment-883235212, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACNAVTRHWX7GVEJIWSJ4EF3TYU46NANCNFSM43PH3Y4Q .

anmalysh-yb commented 3 years ago

So, while customer is not yet registered - it can't have custom password policy configured - so default one is used at this point. We should use customer-specific policy only when customer is already created. You can create customer policy via the following API (no UI for that yet) once customer is registered:

POST /api/v1/customers//configs

with the body like that:

{
  "name": "password policy",
  "type": "PASSWORD_POLICY",
  "data": {
     "minLength": 8,
     "minUppercase": 1,
     "minLowercase": 1,
     "minDigits": 1,
     "minSpecialCharacters": 1
  }
}

Once it's created you'll get config with uuid set, and you can control it using this uuid with the following API calls:

GET /api/v1/customers//configs - list all configs PUT /api/v1/customers//configs/ - update config DELETE /api/v1/customers//configs/ - delete config

Don't forget to include api token header like this:

X-AUTH-YW-API-TOKEN:

You can generate a token on customer profile page.

rajmaddy89 commented 3 years ago

@anmalysh-yb anmalysh-yb For the scope of this story: I have done all the above changes, I will push it shortly

  1. including creating configs for new registered users,
  2. creating an endpoint to get configured password for a user,
  3. validating the client side with policy data,
  4. creating redux actions for the same.
  5. Fix the password policy config function in PasswordPolicyService
  6. Ensured UserProfileForm is also handled with password policy check

I don't understand below comment of yours, can you please elaborate

It seems like UI code is still using old regexp for password policy check:

https://github.com/yugabyte/yugabyte-db/pull/9210/files#diff-62db5af8d2a399fe2d5ebe276571f87df6d5c5c962b02265556aee399ef1a541L47

What do you expect the regex function to do as it just checks it just includes alpha numeric and @ symbol. Not sure what change I need to do there

However can you please let me know if List, Update and Delete Configs should be done in this issue or a separate issue as this was not mentioned in the issue above

anmalysh-yb commented 3 years ago

Really sorry for late response @rajmaddy89. Missed this issue somehow. On the regex comment - I meant that UI should actually check password to comply server side password policy, not something hardcoded on the UI. If I configure it to be 5 characters of any type - current UI checks will not allow me to set such a policy as regexp will raise validation error.

As for list/update/delete password policy UI - I guess it should be a separate issue to implement that.