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

Deactivated user: Redirect to homepage with flash error when trying to access /admin #9338

Closed
drummer83 opened this issue Jun 21, 2022 · 6 comments · Fixed by #9351
Closed
Assignees
Labels
bug-s5 We can live with it... Few users are impacted.

Comments

@drummer83
Copy link
Contributor

drummer83 commented Jun 21, 2022

Description

When a deactivated user tries to access an admin page, he/she is redirected to the homepage with the login modal open. Trying to log in there redirects to the homepage without login modal - which feels like the user has successfully logged in, but he/she actually hasn't. He/she can access shops but the checkout flow is broken: Clicking checkout in the cart always takes the user back to the shop with empty cart.
Peek 2022-06-21 19-20

Expected Behavior

When a deactivated user is trying to access an admin page, he/she should be redirected to the homepage with the flash error message as shown below.
grafik

Actual Behaviour

The user is redirected to the homepage with login modal but login doesn't work and checkout flow is broken.

Steps to Reproduce

  1. As super admin deactivate a user.
  2. As deactivated user try to access /admin --> login modal appears
  3. Try to login --> homepage opens
  4. Try to checkout --> not working

Animated Gif/Screenshot

See above.

Workaround

Severity

bug-s5: we can live with it, only a few users impacted

Your Environment

Possible Fix

See #9154 for implementation of the deactivation of a user.

@drummer83 drummer83 added the bug-s5 We can live with it... Few users are impacted. label Jun 21, 2022
@vsmay98
Copy link

vsmay98 commented Jun 23, 2022

Hi @drummer83
Can I work on this issue?

@drummer83
Copy link
Contributor Author

Sure, @vsmay98 and welcome!
I have assigned it to you! Don't hesitate to contact us here or on our Slack in case you get stuck!
Looking forward to your contributions! 😊

@vsmay98
Copy link

vsmay98 commented Jun 24, 2022

Hi @drummer83
Do we only need to redirect the user to homepage only when they try to access /admin page? I was thinking of to add redirect to homepage whenever the disabled user tries to access any page.

We have this method to sign out the disabled user and show flash message. So after user sign out we can redirect them to homepage with the flash message.

  def check_disabled_user
    return unless current_spree_user.disabled

    flash[:success] = nil
    flash.now[:error] = I18n.t("devise.failure.disabled")
    sign_out current_spree_user
  end

@RachL
Copy link
Contributor

RachL commented Jun 24, 2022

Hello @vsmay98 yes it's cleaner if we do it for any page rather than just the admin 👍 You can go ahead :)

@vsmay98
Copy link

vsmay98 commented Jun 24, 2022

Hello @RachL @drummer83

I have raised a PR #9351, Please review it.

@drummer83
Copy link
Contributor Author

Thanks for your PR, @vsmay98! It is now in code review by our developers. This may take a few days or so and will probably not happen during the weekend. There seems to be a conflict with another PR #9341. We will get back to you if we need to.
In the meantime feel free to pick another issue and let us know!

Have fun! And thanks again! 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-s5 We can live with it... Few users are impacted.
Projects
None yet
3 participants