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

Restcomm 2497: implement suspend account #2904

Open
wants to merge 7,348 commits into
base: master
Choose a base branch
from

Conversation

ddhuy
Copy link

@ddhuy ddhuy commented Apr 7, 2018

What this PR does / why we need it:
Currently we can only update an account to Closed state, which means all related resources (DIDs, Clients etc) will be removed and Account will be inactive.
From this state there is no way to get back to Active.

For this reason we need to implement the Suspended state which means, Account and account's resources (DIDs, Clients etc) cannot operate but are not deleted. At any time a Suspended account can move to either Active or Closed state

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #2497

Special notes for your reviewer:
Following regression tests are added as @gsaslis recommend:

  • 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.

jaimecasero and others added 30 commits February 20, 2018 11:21
fixes restcom1757 allowing to override LINK/UNLINK methods
This refer to #RESTCOMM-1636
…er and if exists, set the destOrganization from the number discovered otherwise, fail.

This close #RESTCOMM-1782
Don't fail fast when destOrganization is null. First check for a number…
…to the Accounts Endpoint

This close #RESTCOMM-1784
RESTCOMM-1784 SecurityFilter patch to allow requests from `UNINITIALIZED` accounts …
MSB-326: Documentation: Multiprovider extension: Fix broken link
Profiles API doc: Add additional examples. Add index link
Yorgos Saslis and others added 24 commits March 29, 2018 13:01
corrected typo
Jira 1738 Dial Timeout does not cancel task when Callee is busy/Unava…
This refer to RESTCOMM-1932
Improved log statements and test case to reproduce issue
* master:
  Minor fix for log statement This refer to RESTCOMM-1932
  Improved log statements and test case to reproduce issue This refer to RESTCOMM-1932
  inherit MockMediaGateway to reduce dullpicated code
  corrected typo
  Jira 1738 Dial Timeout does not cancel task when Callee is busy/Unavailable
  Added agpl license
  Applying directory name standard to core module.
  Fix retrieving profile over http with https configured
  Update console to match Restcomm look & feel
  Implement new console sign in page
  fixed cipher suite property name
  Added inbound sms SDR event
and reworked acknowledgements section
Support for SBC in front of Restcomm Pull Request Review
* master: (23 commits)
  RESTCOMM-2000: check WebOlympusDomain before modifying olympus.xml
  added testing categories
  Added JProfiler to acknowledgements
  Fix Olympus configuration.
  Fix.
  RESTCOM-1935: Add extensionHeaders for Call authenticate
  Configure Web Olympus Domain.
  Added extra logging statement to check the callState at the end of a dial fork call flow
  Fix conf Script.
  Update RestComm#2741 Fixing tests compilation failing after latest master merge and fix from aziz on multiprovider extension for specific rule for calls between Olympus and PSTN
  Update RestComm#2741 making sure when using useSbc that we don't enable webRTC in SDP between RC and SBC. SBC will be responsible for DTLS termination and media interactions with WebRTC Clients
  Enable RCUSESBC configuration option.
  Enable USESBC configuration option.
  Fix issue where the SBC route is pushed twice on the INVITE
  Fixing failing tests, adding some for Dial with SBC and fixed registration removal in case of SBC being used
  Update RestComm#2741 Fixing wrong boolean assignment on disable SDP Patching and handling default ports
  Update RestComm#2741 Fixing wrong values in restcomm.xml from previous commit
  Update RestComm#2741 with support for disabling of NAT patching when an SBC is used in front of RC + test case for it. It fixes also broken tests in UserAgentManager
  #MSB-352 Handle Route Header for MultiProvider config. Add handling for B2BUA Motivation: - Push routes defined in MultiProvider outbound-voice config.
  Fixes.
  ...
RESTCOMM-1032 : Client Password Proposal 1a
Also fix the file formatting for events.js
@ddhuy ddhuy mentioned this pull request Apr 7, 2018
@ddhuy
Copy link
Author

ddhuy commented Apr 7, 2018

Hi @gsaslis ,
In current design, each time we update account status, we also update status of its sub-accounts. Please check this piece of code switchAccountStatusTree

I have one concern here, what if the account is suspended then activated again so the suspended/inactive/uninitialized sub-accounts will be activated too.
33177183-532e11ce-d094-11e7-8c34-7d8c963a4bd0

This could be our new expectation: update sub-account's status regarding to the parent account's status, right? Do you have any idea for this situation?

@gsaslis
Copy link
Contributor

gsaslis commented Apr 25, 2018

@ddhuy thanks for the new PR. This is much clearer.

with regards to your particular question, I will say the same thing I had replied back then: #2497 (comment)

Plz do feel free to let me know if that's not clear or you need any further clarification there.

@ddhuy
Copy link
Author

ddhuy commented Apr 26, 2018

@gsaslis thank you for your comment. I understand your point. But with current implementation in our source code, the sub-account status is updated according to parent account status. Please have look at this function switchAccountTree. This function is called whenever we update one account status.
I already tried to test this behavior, and observed the same problem.

@gvagenas gvagenas force-pushed the master branch 2 times, most recently from 9469191 to c67f142 Compare January 13, 2022 15:49
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.

Implement new Account State Suspended
9 participants