Guardian permissions #45

sharma-sagar · 2017-06-07T13:32:53Z

Guardian Permission Implementation.
This commit is on top of sprint_1 (user management and subscriber management).

Also:
Added user management where user wasn't able to see any same level user to block/unblock or delete.
Added Role in subscriber management.

Please review
Thanks

This reverts commit 3103133.

Simplify management of CCM repo on GitHub.

…ellularManager

This reverts commit 06c22aa.

…tyCellularManager

…arManager into aricent_dev

This reverts commit 3b362c1.

This reverts commit b08541b.

…nto aricent_dev

Fix: Cloud Admin was available to Network Admin and Cloud Admin Fix: Username replaced with Email

Fix for user role permissions Code added to ajax request to get permission depending on role

facebook-github-bot · 2017-06-07T13:33:00Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks!

If you are contributing on behalf of someone else (eg your employer): the individual CLA is not sufficient - use https://developers.facebook.com/opensource/cla?type=company instead. Contact [email protected] if you have any questions.

kkroo

Permission checks should be against a particular network instance as apposed to get_all_permissions for a user. Also I am not sure why we are rendering a permission denied message in the templates. This should be handled by the view to ensure it applies to both GET/POST. Some questions about permission_required selection for user management

kkroo · 2017-06-07T16:22:18Z

cloud/endagaweb/forms/dashboard_forms.py

+            self.helper.form_action = '/dashboard/user/management/blocking'
+        else:
+            self.helper.form_action = '/dashboard/user/management/delete'
+        # self.helper.form_class = 'form-vertical'


kkroo · 2017-06-07T16:35:32Z

cloud/endagaweb/templates/dashboard/activity.html

@@ -26,7 +26,7 @@
    <h3>Network Activity</h3>
  </div>
 </div>
-
+{% if 'endagaweb.view_usage' in user_permission %}


this check should be against the network object

kkroo · 2017-06-07T16:37:03Z

cloud/endagaweb/templates/dashboard/activity.html

@@ -146,6 +146,7 @@
    </div>
 </div>

+{% if 'endagaweb.download_usage' in user_permission %}


for these permissions to work with guardian they need to be a part of the Network model and you can then use Guardian to check this permission against this object

kkroo · 2017-06-07T16:37:28Z

cloud/endagaweb/templates/dashboard/index.html

@@ -34,6 +34,7 @@
  </div>
 </div>

+{% if 'endagaweb.view_graph' in user_permission %}


this permission is global to all networks per user

kkroo · 2017-06-07T16:39:03Z

cloud/endagaweb/templates/dashboard/layout.html

-    <link href="/static/css/dashboard.css" rel="stylesheet">
-    <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/4.0.3/css/font-awesome.min.css">
+    <link href="/static/css/dashboard.css" rel="stylesheet"/>
+    <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/4.0.3/css/font-awesome.min.css"/>


if you use the CDN delete the file from static

accidental change

kkroo · 2017-06-07T16:52:02Z

cloud/endagaweb/templates/dashboard/user_management/add.html

+                        <div class="input-group col-xs-8">
+                            <select name='network' id="network" class="form-control" multiple="multiple" size="5"
+                                    required="required">
+                                {% for network in networks %}


this may need to be thought over since if I don't have a particular permission on a network, I shouldnt be able to grant it to someone else

kkroo · 2017-06-07T16:52:53Z

cloud/endagaweb/templates/dashboard/user_management/block-unblock.html

+                    </body>
+                </div>
+            </form>
+            {% else %}


post? the check needs to happen in the view

kkroo · 2017-06-07T16:55:54Z

cloud/endagaweb/views/dashboard.py

@@ -122,7 +151,9 @@ def dashboard_view(request):
    network_has_activity = UsageEvent.objects.filter(
        network=network).exists()
    context = {
-        'networks': get_objects_for_user(request.user, 'view_network', klass=Network),
+        'user_permission': request.user.get_all_permissions(),


the user_permission should contain the context of the selected network

kkroo · 2017-06-07T17:03:37Z

cloud/endagaweb/views/towers.py



-class TowerList(drf_views.APIView):
+class TowerList(PermissionRequiredMixin, drf_views.APIView):


should check against the network instance:

If a get_object() method is defined either manually or by including another mixin (for example SingleObjectMixin) or self.object is defined then the permission will be tested against that specific instance, alternatively you can specify get_permission_object() method if self.object or get_object() does not return the object against you want to test permission

http://django-guardian.readthedocs.io/en/stable/api/guardian.mixins.html#guardian.mixins.PermissionRequiredMixin

kkroo · 2017-06-07T17:09:08Z

cloud/endagaweb/views/dashboard.py

+
+
+class UserBlockUnblock(ProtectedView):
+    permission_required = 'endagaweb.view_user'


should this be the block_user permission

9muir · 2017-06-15T14:50:25Z

Just a process point: this PR is 'on top' of the original Sprint 1 PR #33, do you intend to close the original one and instead have us focus our attention on this one, or address them separately? At this point I recommend the former, as we don't want to merge the permission changes without Guardian incorporated.

Once you have addressed @kkroo's comments we can review this again. Just one additional minor request that will eliminate numerous lint errors that get flagged by our internal build system: please make sure EVERY file ends with a newline, but that the last line of the file does not contain any other whitespace. And remove any code that is currently commented out.

Thanks

…features

facebook-github-bot · 2017-06-27T14:03:01Z

@sagarsharma-aricent updated the pull request - view changes

…tyCellularManager into guardian_features

facebook-github-bot · 2017-06-30T10:27:52Z

@sagarsharma-aricent updated the pull request - view changes

facebook-github-bot · 2017-06-30T11:48:16Z

@sagarsharma-aricent updated the pull request - view changes

kkroo

very close! minor changes requested. thanks for all the hard work

kkroo · 2017-07-07T17:13:38Z

cloud/endagaweb/templates/dashboard/subscriber_management/subscribers.html

+  $.each(imsi_list, function(key, val){
+    imsi_val[i++]=val.value;
+  });
+  console.log(imsi_val)


kkroo · 2017-07-07T17:14:02Z

cloud/endagaweb/templates/dashboard/subscriber_management/subscribers.html

+        console.log("done");
+        window.location = "/dashboard/subscriber_management/subscriber";
+    }).fail(function(response) {
+        console.log(response);


kkroo · 2017-07-07T17:16:21Z

cloud/endagaweb/templates/dashboard/user_management/add.html

+                        </div>
+                    </div><!--Email Ends-->
+
+                    <!--Password-->


maybe we don't require a password? password reset is sent anyhow. what about existing users?

Password is kept temporarily for testing purpose.
If we mention existing user in email field , an edit option will be shown just below email for editing the user.
Please find attached snapshots

kkroo · 2017-07-07T17:17:16Z

cloud/endagaweb/templates/dashboard/user_management/add.html

+                                <div class="input-group-addon">
+                                    <i class="glyphicon glyphicon-signal"></i>
+                                </div>
+                                <input type="hidden" name="network_id" id="network_id" value="{{ network.id }}"/>


lets get rid of this? there is only one option here can be implicit

kkroo · 2017-07-07T17:19:38Z

cloud/endagaweb/templates/dashboard/user_management/add.html

+                    $.ajax({
+                        url: "/dashboard/user/management/checkuser?email=" + $('#email').val(),
+                        //beforeSend: function(xhr) {
+                        // xhr.setRequestHeader("X-CSRFToken", '{{ csrf_token }}');


CSRF should be required. I think it gets switched on for POST by default so you can change the request type for this endpoint

its a GET request

kkroo · 2017-07-07T17:22:19Z

cloud/endagaweb/templates/dashboard/user_management/edit.html

+                        </div>
+                    </div><!--Email Ends-->
+
+                    <!--Network-->


this is implicit in the current selected network

updated now

kkroo · 2017-07-07T17:24:30Z

cloud/endagaweb/templates/dashboard/user_management/popup.html

@@ -0,0 +1,259 @@
+{% extends "dashboard/layout.html" %}


I'm not sure why this file was added

Wrongly added, now removed

kkroo · 2017-07-07T17:35:41Z

cloud/endagaweb/views/dashboard.py

+        network = user_profile.network
+        user = User.objects.get(id=user_profile.user_id)
+        available_permissions = get_perms(request.user, network)
+        print network.id


kkroo · 2017-07-07T17:36:50Z

cloud/endagaweb/views/dashboard.py

+                user_profile = UserProfile.objects.create(user=user)
+                user_network = Network.objects.get(id=networks)
+                # Add the permissions
+                for permission_id in permissions:


needs validation that its only from the list of available permissions

added check to validate assigned permission against available permissions

kkroo · 2017-07-07T17:37:11Z

cloud/endagaweb/views/dashboard.py

+
+    def _send_reset_link(self, request):
+        return password_reset(request,
+                              # email_template_name='dashboard/user_management/reset_email.html',


facebook-github-bot · 2017-07-13T10:25:49Z

@sagarsharma-aricent updated the pull request - view changes

sharma-sagar · 2017-07-13T10:33:57Z

All the changes are incorporated as requested.
Thanks
Sagar Sharma

facebook-github-bot · 2017-07-18T07:28:54Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

facebook-github-bot · 2017-08-03T14:43:36Z

@sharma-sagar updated the pull request - view changes

sharma-sagar · 2017-08-04T05:21:00Z

Hello,
Please review 3ed0bf7 on top of this with all changes incorporated and added redesign which includes
This include:

Add/Edit/Block/Delete User
Assign permission on multiple networks
Guardian permissions
Attached are the snapshots for user management

this commit is on top of this pr ( #45 )
closing #75 as this commit is same.

facebook-github-bot · 2017-08-04T05:26:47Z

@sharma-sagar updated the pull request - view changes

facebook-github-bot · 2017-08-10T08:45:27Z

@sharma-sagar updated the pull request - view changes

facebook-github-bot · 2017-08-10T17:07:43Z

@kkroo has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

piyush and others added 30 commits April 5, 2017 15:23

Updated for ForceFullyDeregister Tower

f49b300

generalised code for deregister tower

fd0a013

Updated for ForceFullyDeregister Tower

7945ccf

generalised code for deregister tower

3103133

Revert "generalised code for deregister tower"

0322cc2

This reverts commit 3103133.

updated review comments

01f2662

updated No Data tower status check

8f410e3

Add config file for browser extension

87cc459

Simplify management of CCM repo on GitHub.

Add comment, remove trailing comma

130200f

Merge branch 'aricent_dev' of https://github.com/piyush-kr/CommunityC…

1b1982e

…ellularManager

local working

06c22aa

Merge remote-tracking branch 'origin/master' into aricent_dev

dac1590

Revert "local working"

3b362c1

This reverts commit 06c22aa.

Merge branch 'master' of https://github.com/facebookincubator/Communi…

25da6ed

…tyCellularManager

Merge branch 'master' of https://github.com/piyush-kr/CommunityCellul…

6f63fbf

…arManager into aricent_dev

Revert "Revert "local working""

d18b26e

This reverts commit 3b362c1.

Revert "make sms_utilities py3 friendly [part 2 of more]"

63384a6

This reverts commit b08541b.

Merge https://github.com/facebookincubator/CommunityCellularManager i…

90dbd42

…nto aricent_dev

Added User Management with Subscriber Profiling

7f10343

env setting

36ca3e7

role permission implementation

04cd8db

migration

f59bb3d

role permission model changes

5861ae4

Fix: User permission for user management.

9335fb3

Fix: Cloud Admin was available to Network Admin and Cloud Admin Fix: Username replaced with Email

Adding reset templates

5e027cf

Merging code for user management

a50fd02

Fix for permission selection

37a328d

Fix for user role permissions Code added to ajax request to get permission depending on role

new permission assignment UI

9cb19d7

new permission assignment UI

84aaf0e

new permission assignment UI

32a0d16

facebook-github-bot added the GH Review: review-needed label Jun 7, 2017

kkroo suggested changes Jun 7, 2017

View reviewed changes

9muir mentioned this pull request Jun 17, 2017

CCM Enhancement sprint1 -updated #33

Open

Merge remote-tracking branch 'remotes/upstream/master' into guardian_…

750dbdb

…features

sharma-sagar added 2 commits June 30, 2017 15:42

Merge branch 'master' of https://github.com/facebookincubator/Communi…

e789c64

…tyCellularManager into guardian_features

Guardian Permission with User Management (sprint 1)

213a282

Disable Network in Add

b302fd8

kkroo suggested changes Jul 7, 2017

View reviewed changes

Incorporated changes for PR-45

cab4227

sharma-sagar changed the title ~~Guardian permissions (**do not merge)~~ Guardian permissions Jul 13, 2017

facebook-github-bot added the CLA Signed label Jul 18, 2017

This was referenced Jul 28, 2017

User Management Redesign__on_top_of_Guardian features piyushabad88/CommunityCellularManager#3

Closed

Sprint_5: User Management Redesign__on_top_of_Guardian features #75

Closed

Guardian Redesign and Edit User

3ed0bf7

Merge branch 'master' into guardian_features

771efad

Fixed typo for tower add

04ed325

facebook-github-bot added the Import Started label Aug 10, 2017



		class TowerList(drf_views.APIView):
		class TowerList(PermissionRequiredMixin, drf_views.APIView):



		class UserBlockUnblock(ProtectedView):
		permission_required = 'endagaweb.view_user'

Guardian permissions #45

Are you sure you want to change the base?

Guardian permissions #45

Conversation

sharma-sagar commented Jun 7, 2017

facebook-github-bot commented Jun 7, 2017

kkroo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

9muir commented Jun 15, 2017

facebook-github-bot commented Jun 27, 2017

facebook-github-bot commented Jun 30, 2017

facebook-github-bot commented Jun 30, 2017

kkroo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sharma-sagar Jul 13, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

facebook-github-bot commented Jul 13, 2017

sharma-sagar commented Jul 13, 2017

facebook-github-bot commented Jul 18, 2017

facebook-github-bot commented Aug 3, 2017

sharma-sagar commented Aug 4, 2017 • edited Loading

facebook-github-bot commented Aug 4, 2017

facebook-github-bot commented Aug 10, 2017

facebook-github-bot commented Aug 10, 2017

sharma-sagar Jul 13, 2017 •

edited

Loading

sharma-sagar commented Aug 4, 2017 •

edited

Loading