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

Support account role update #1168

Closed
otsakir opened this Issue Jun 9, 2016 · 7 comments

Comments

Projects
None yet
4 participants
@otsakir
Contributor

otsakir commented Jun 9, 2016

Account role update is not supported. The existing policy used in account creation is the following:

  • Administrators can explicitly set the roles of the sub-accounts they create.
  • non-administrators create sub-accounts of their own role.

The same policy can be adapted to account modification:

  • Administrators can explicitly update the role of a sub-account arbitrarily.
  • non-administrator cannot change the role of their sub-accounts at all.
  • Administrators can change their own role arbitrarily too. That may imply privilege loss and a warning can be displayed in the UI in such a case.

The patch includes patching AccountsEndpoint, AdminUI account details form and the AccountsEndpointTest in the testsuite (check role is updated).

@otsakir otsakir added this to the 7.8.0 milestone Jun 9, 2016

@otsakir otsakir added the Help Wanted label Jun 9, 2016

@otsakir otsakir changed the title from Support account roles update to Support account role update Jun 9, 2016

muhammadbilal19 added a commit to muhammadbilal19/Restcomm-Connect that referenced this issue Jun 17, 2016

Add support for account role update only for administrator on AdminUI
Child user now only can see Developer role in drop down RestComm#1168

muhammadbilal19 added a commit to muhammadbilal19/Restcomm-Connect that referenced this issue Jun 17, 2016

Add role update functionality in AccountsEndpoint
Add testcase for role update in AccountsEndpointTest RestComm#1168
@muhammadbilal19

This comment has been minimized.

Collaborator

muhammadbilal19 commented Jun 17, 2016

@otsakir i stated making some changes for this issue. I need to clarify few things

  • if Current login user Say Default Admin (admin@company.com) want to update own role do we need to allow ? In that case do system need to force this user to login again?
  • According to the rule non-administrator can only create sub accounts of their own types but in that scenario their is a possibility that Default Admin may have few administrator sub accounts

@gvagenas gvagenas modified the milestones: 7.8.0, 7.9.0 Jul 4, 2016

@muhammadbilal19

This comment has been minimized.

Collaborator

muhammadbilal19 commented Jul 5, 2016

@otsakir can you please comment on my previous comment.

@otsakir

This comment has been minimized.

Contributor

otsakir commented Jul 5, 2016

@muhammadbilal19, appologies for the delay. Your comment fell through the cracks :-(.

  • Indeed, it seems that allowing an admin to change his own role is not really necessary, i agree.
  • The Default Admin may have administrator sub-accounts. I think that's ok. Default Admin should take care when someone is made admin (not sure if i got your second bullet here).
  • In general, allowing an account changing his own role seems like a useless feature.

Thinking over it, it seems to me that the only case of role modification that is really needed is for an Admin to be allowed to change the roles of his sub-accounts arbitrarily.

@muhammadbilal19 wdyt?
@gvagenas, @deruelle , any thoughts on this ?

@deruelle

This comment has been minimized.

Member

deruelle commented Jul 6, 2016

As a general rule, there is some documentation on accounts and sub accounts at http://documentation.telestax.com/connect/api/Restcomm%20-%20Multi-tenancy%20and%20Managing%20Sub-Accounts.html#restcomm-rest-api. If there is not enough information. Please consider contributing changes to it at https://github.com/RestComm/Restcomm-Connect/blob/master/restcomm/restcomm.docs/sources-asciidoc/src/main/asciidoc/api/Restcomm%20-%20Multi-tenancy%20and%20Managing%20Sub-Accounts.adoc

  1. "allowing an admin to change his own role is not really necessary" i agree.
  2. "The Default Admin may have administrator sub-accounts." I think that's ok too
  3. "In general, allowing an account changing his own role seems like a useless feature." Agreed.
@muhammadbilal19

This comment has been minimized.

Collaborator

muhammadbilal19 commented Jul 12, 2016

Thanks @otsakir and @deruelle for your comments and apologies for the late reply.

I also agree that the only thing which is required is to allow ADMIN to update its sub accounts. I will create a pull request for this issue after doing above changes.

@deruelle

This comment has been minimized.

Member

deruelle commented Jul 19, 2016

Thanks @muhammadbilal19 . Please refer to the Pull request here once done so it can be reviewed

muhammadbilal19 added a commit to muhammadbilal19/Restcomm-Connect that referenced this issue Jul 20, 2016

Add support for account role update only for administrator on AdminUI
Child user now only can see Developer role in drop down RestComm#1168

muhammadbilal19 added a commit to muhammadbilal19/Restcomm-Connect that referenced this issue Jul 20, 2016

Add role update functionality in AccountsEndpoint
Add testcase for role update in AccountsEndpointTest RestComm#1168

muhammadbilal19 added a commit to muhammadbilal19/Restcomm-Connect that referenced this issue Jul 20, 2016

@muhammadbilal19

This comment has been minimized.

Collaborator

muhammadbilal19 commented Jul 20, 2016

@deruelle Pull request created for this issue #1294

otsakir added a commit that referenced this issue Jul 26, 2016

Enforced acount role update rules as described in #1168. Other minor …
…fixes too included.

- AccountsEndpoint allows role updates only for administators.
- Supplied the respective automated tests.
- Minor fixes in AdminUI.

Refers #1168

@otsakir otsakir closed this Jul 26, 2016

maria-farooq added a commit to maria-farooq/Restcomm-Connect that referenced this issue Jul 29, 2016

Merge branch 'master' into issue-1154
* master: (30 commits)
  Prevented non-admins from changing other sub-accounts.
  Enforced acount role update rules as described in RestComm#1168. Other minor fixes too included. - AccountsEndpoint allows role updates only for administators. - Supplied the respective automated tests. - Minor fixes in AdminUI.
  change the update option of current login user RestComm#1168
  Add role update functionality in AccountsEndpoint Add testcase for role update in AccountsEndpointTest RestComm#1168
  Add support for account role update only for administrator on AdminUI Child user now only can see Developer role in drop down RestComm#1168
  Update Issue RestComm#1285
  Issue RestComm#1285
  Fix for issue RestComm#1285
  fix css
  USSD configuration for RestComm. Issue RestComm#1288
  Updated links for Docker and general Restcomm Connect documentation
  Updated Restcomm-SipServlets to build 740
  Fixes for peer to peer calls that has Friendly Name different than the Login. This close RestComm#1278
  Fixes for issue RestComm#1275
  Fix for DB update script
  Update for UssdPull test cases
  Properly handle Reject with reason "rejected". This close RestComm#1275
  Properly link the upgrade scripts for 7.8.0
  Fix crontab for monitoring bug.
  Fixing documentation for Attaching a SIP number to an application
  ...

Conflicts Resolved in following classes:
	restcomm/restcomm.interpreter/src/main/java/org/mobicents/servlet/restcomm/interpreter/BaseVoiceInterpreter.java
	restcomm/restcomm.interpreter/src/main/java/org/mobicents/servlet/restcomm/interpreter/SubVoiceInterpreter.java
	restcomm/restcomm.interpreter/src/main/java/org/mobicents/servlet/restcomm/interpreter/VoiceInterpreter.java
RestComm#1104
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment