search

SolutionGuidance / psm

Welcome to the Medicare/Medicaid Provider Enrollment Screening Portal
http://projectpsm.org/
Other
26 stars 20 forks source link

Fix #33. Add user account suspend/reinstate UI. #943

Closed frankduncan closed 6 years ago

frankduncan commented 6 years ago

Issue #33: Complete or remove the ability to suspend user accounts

This is the System UI, as well as login check, only. There is probably a lot more around account management that we could do, but I believe the spirit of #33 was that we should at least do something with the field in the DB and the related dao functions. So I think we should hold off on messaging, better error handling, etc until more requirements come, and log issues for those.

One large note is that I found no easy way to force a logout of someone who is suspended while already logged in. That would involve a DB call on every single page view to check the cms_user table, and add that to the top level spring security interceptors, as the system currently works. My view was that was overkill for this step of this feature.

Test via logging in as a system user. Suspend an account. Ensure you can no longer log in as that account. Log back in as a system user, reinstate it. Log back in as other account.

jasonaowen commented 6 years ago

Two things related to this occurred to me today: we should probably send an email notification on suspension/reinstatement, and we should disable "forgot password" resets for suspended accounts. Since the "is suspended?" check happens before the "is correct password?" check, and we don't have a suspension-specific error, the result is fairly confusing for users. Maybe going through the forgot password workflow would result in an email notification to the effect of "Your password was not changed because your account is suspended."

Those could probably happen later, but if so we should avoid closing the issue. (Or, open follow-up issues.)

frankduncan commented 6 years ago

we should probably send an email notification on suspension/reinstatement

I can open an issue, since there's some design discussion here. I can make a cogent case for either side of whether users should be alerted that they've been suspended/reinstated.

we should disable "forgot password" resets for suspended accounts

This depends on whether we want a different workflow for suspended accounts, and whether we want that information to be presented to the user. If you think so, I'll open an issue for it.

frankduncan commented 6 years ago

Thanks @jasonaowen ! Ready for re-review and discussion :)

jasonaowen commented 6 years ago

we should probably send an email notification on suspension/reinstatement

I can open an issue, since there's some design discussion here. I can make a cogent case for either side of whether users should be alerted that they've been suspended/reinstated.

we should disable "forgot password" resets for suspended accounts

This depends on whether we want a different workflow for suspended accounts, and whether we want that information to be presented to the user. If you think so, I'll open an issue for it.

You're right, those are both design questions, not implementation questions! Would you mind opening (an) issue(s)?