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

fix 2497: suspend account #2702

Closed
wants to merge 29 commits into from
Closed

Conversation

ddhuy
Copy link

@ddhuy ddhuy commented Dec 6, 2017

We can now suspend an account, all of its DIDs are prevented to call. Sub-accounts cannot log-in also.

@gsaslis
Copy link
Contributor

gsaslis commented Dec 7, 2017

Thanks for the PR @ddhuy !

We would still need some tests around this to be sure we're not breaking anything critical with these changes.

Would you please include the following test cases:

  • No REST API request is allowed for a SUSPENDED account. Account cannot login, create calls, search DID etc.
  • All resources (apps, clients, DIDs etc) are suspended and cannot be used. Clients cannot be registered, DID numbers return 403
  • All sub-accounts of the account and their resources are also suspended and cannot be used.
  • The Account, Sub-Accounts and all related resources still exist / are not removed from db.
  • A suspended account can move to either CLOSED state or to ACTIVE state again.
  • Moving to ACTIVE state means that Account and sub-accounts are active and can be used again, same for all related resources.
  • Moving to CLOSED state means that Account, sub-accounts and related resources are closed forever and will be never used again. External resources like, purchased DID number will be released (so no more costs) but everything else (account, sub-account, resources) will remain in the DB for audit purposes.

Looking forward to reviewing once these are in place!

Copy link
Contributor

@gsaslis gsaslis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need some regression tests against this

@ddhuy
Copy link
Author

ddhuy commented Jan 17, 2018

@gsaslis I have added regression tests as your suggestion. Please help verify them again.

Moving to CLOSED test is not added because we have issue with closing account and I already created a new issue for this. So this test will be added according to the fix of it

@ddhuy ddhuy closed this Apr 7, 2018
@ddhuy
Copy link
Author

ddhuy commented Apr 7, 2018

This PR is so far out of date with master. So I create a new PR 2904 one.

@gsaslis
Copy link
Contributor

gsaslis commented Apr 25, 2018

thanks @ddhuy !

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

Successfully merging this pull request may close these issues.

2 participants