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

Prevent demo users from changing the password and the email #35

Closed
1 task
Tracked by #33
ck3g opened this issue Sep 22, 2020 · 8 comments · Fixed by #81
Closed
1 task
Tracked by #33

Prevent demo users from changing the password and the email #35

ck3g opened this issue Sep 22, 2020 · 8 comments · Fixed by #81

Comments

@ck3g
Copy link
Owner

ck3g commented Sep 22, 2020

Contributes to #33

Demo account should not have the ability to change the password and the email

Replace Edit Profile form /users/edit with the static text:

Email: <demo user Email>
Password: demo1234


The /users/edit page should behave differently for demo users.

When the logged-in user (or current_user) is a demo_user? the edit profile should not let to update the email and password for that user. It can be done by making the form read-only, or by replacing the form with the text information.

When the logged-in user is a regular user (demo_user? == false), the form should work as it works now. The user should be able to update the email and password.

Dependencies

@m8051
Copy link
Contributor

m8051 commented Sep 25, 2020

Hi @ck3g , I think I could work on this ... is it ok?

@ck3g
Copy link
Owner Author

ck3g commented Sep 25, 2020

Hey @m8051 absolutely

@m8051
Copy link
Contributor

m8051 commented Sep 26, 2020

@ck3g I have been thinking about it and I believe we could disable the inputs in the /users/edit form along with the submit button, instead of replacing them with texts.

My idea is to implement a very basic boolean method in the ApplicationController that checks if @user.email is 'user@example.com'.

  def demo_user?
    true if @user.email == 'user@example.com'
  end

Then disable the inputs and submit button in the edit.html.haml

%h2= t(:profile)
= simple_form_for(resource, as: resource_name, url: registration_path(resource_name), html: { method: :put, class: "form-horizontal" }, defaults: { disabled: (demo_user?) }) do |f|
  = f.error_notification
  .form-inputs
    = f.input :email, required: true, autofocus: true
    = f.input :password, autocomplete: "off", hint: t("devise.registrations.leave_it_blank_if_dont_want_to_change"), required: false
    = f.input :password_confirmation, required: false
    = f.input :current_password, hint: t("devise.registrations.need_current_password_to_confirm"), required: true
  .form-actions
    = f.button :submit, t(:update), class: "btn btn-primary", disabled: (demo_user?)

Would you be happy with these changes? I can see an issue hardcoding the address. Maybe we can use the first method, which would return the first record created through seeds instead.

Or better to stick to the original plan?

I would like to know your thoughts on how you would like implement it ...

Thank you.

@ck3g
Copy link
Owner Author

ck3g commented Sep 26, 2020

@m8051

When we add a new column with the boolean value here #34 we will get the demo_user? method for free.
But in the meantime, we can introduce the method as you're suggesting, to make the form work. You can check against the following email for now: demo@homebugh.info.

We can disable fields as well, that's absolutely fine. The problem with the disabled fields is that anybody can inspect the page and change their values to not disabled and submit the form. That's why (in the separate issue) we need to prevent updates for demo users as well.

To summarize. The solution you're suggestion is fine. Feel free to proceed.

@m8051
Copy link
Contributor

m8051 commented Sep 26, 2020

Oh I did not notice issue #34 will give us the demo_user? for free, brilliant

You are right anyone could manipulate the form and submit it by inspecting the source code.

Was your original idea based on checking the demo_user value and render two forms ?

Thanks @ck3g

@ck3g
Copy link
Owner Author

ck3g commented Sep 26, 2020

Was your original idea based on checking the demo_user value and render two forms ?

Not sure I understand the question.

The main idea here is to prevent changing credentials for a demo user. On the /users/edit page we need to check for current_user.demo_user? and alter the view when it's true. We can replace it with text, or disable the form to make it read-only.

So the page will behave normally for all of the users, allowing them to change their password or email. But it will be a read-only form (or text view) for the demo users.

Let me know if that makes sense.

@m8051
Copy link
Contributor

m8051 commented Sep 26, 2020

Yes makes a lot of sense, I will create the migration myself based on #34 and implement what we discussed above ^^

@ck3g
Copy link
Owner Author

ck3g commented Sep 27, 2020

I will create the migration myself

I think @phunq-0851 wanted to work on the migration. For your changes you can define a temporary method in the User model

def demo_user?
  email == "demo@homebugh.info"
end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants