[feature] REST API for main controller features #379 #386

ManishShah120 · 2021-02-17T13:04:07Z

Closes #379

ManishShah120 · 2021-02-17T18:10:12Z

@atb00ker The tests cases are failing because of this

    from . import views as api_views
  File "/home/runner/work/openwisp-controller/openwisp-controller/openwisp_controller/api/views.py", line 12, in <module>
    from openwisp_users.api.mixins import FilterByOrganizationManaged
ModuleNotFoundError: No module named 'openwisp_users.api.mixins'

I tested it by creating a new env but while installing all the dependencies and requirements I found that the problem arises from the openwisp-users module all the files from this https://github.com/openwisp/openwisp-users/tree/master/openwisp_users/api folder gets downloaded except the mixins.py file. I created that file manually and all the test cases are passing.

Is there any way to fix this or am I missing or doing something wrong. 🤔 💭

atb00ker · 2021-02-17T19:44:48Z

Is there any way to fix this or am I missing or doing something wrong.

I have not looked into the problem in detail, but could you please use openwisp-user from github, maybe that change is not released.
Something like: pip install -U https://github.com/openwisp/openwisp-users/archive/master.zip

Read more: https://stackoverflow.com/a/24811490/6410464

nemesifier

The tests cases are failing because of this
    from . import views as api_views
  File "/home/runner/work/openwisp-controller/openwisp-controller/openwisp_controller/api/views.py", line 12, in <module>
    from openwisp_users.api.mixins import FilterByOrganizationManaged
ModuleNotFoundError: No module named 'openwisp_users.api.mixins'
I tested it by creating a new env but while installing all the dependencies and requirements I found that the problem arises from the openwisp-users module all the files from this https://github.com/openwisp/openwisp-users/tree/master/openwisp_users/api folder gets downloaded except the mixins.py file. I created that file manually and all the test cases are passing.

Is there any way to fix this or am I missing or doing something wrong.

Rebase on the latest master to pull the changes just merged from #374.

The solution to the problem lies in these lines:
7b78fa0#diff-4d7c51b1efe9043e44439a949dfd92e5827321b34082903477fd04876edb7552L8-R9

For reference see PEP 508.

nemesifier

The PR is too basic, not much logic is implemented.

Please read again the issue description and follow the instructions closely!

If there's something that is not clear about the issue description, quote each line which is not clear and ask specific questions about what is not clear.

nemesifier · 2021-02-18T00:48:37Z

openwisp_controller/api/urls.py

+urlpatterns = [
+    path(
+        'controller/',
+        include(


is this nested include really needed or is it the best practice to do this? I've never seen it.

Since more than one app's API's are present in this module I tried to keep the URL in the main urls.py as api/v1/ so if I don't use nesting here then each time I write path I will be required to add controller to it, so proceeded with this approach.

Maybe, you could move these URLs in a separate file and get urls from the function there like done in openwisp-radius urls.

Sure, I will do it in this manner.

@atb00ker this is done.

nemesifier · 2021-02-18T00:51:05Z

openwisp_controller/api/serializers.py

+class DeviceSerializer(ValidatedModelSerializer):
+    class Meta:
+        model = Device
+        fields = '__all__'


all the templates will surely need to contain more logic

coveralls · 2021-02-23T18:33:05Z

Coverage decreased (-0.3%) to 98.625% when pulling 28f1622 on ManishShah120:issues/379-REST_API-for-main-controller into a91e9e7 on openwisp:master.

tests/openwisp2/urls.py

atb00ker · 2021-02-28T10:31:47Z

openwisp_controller/api/urls.py

+                path(
+                    'device/<str:pk>/', api_views.device_detail, name='device_detail',


Quick Q: shouldn't the details pages be read-only and we should have separate edit pages.

Currently, I went to http://127.0.0.1:8000/api/v1/controller/device/<pk>/

and it gave me details and the form to edit the device! 😄

Yes, @atb00ker I had a discussion about this with Federico, to have separate endpoints for GET/PATCH/PUT of the device detail and other endpoints but he suggested me to keep it in this way. 😄

Thanks for the info, I found the comment: #379 (comment)

I highly recommend you guys to read this book: https://www.amazon.com/RESTful-Web-Services-Leonard-Richardson/dp/0596529260
Having separate endpoints for editing a resource does not follow the RESTful API design principles.
A resource can be created (POST /), retrieved (GET /<pk>), updated (PUT /<pk>), destroyed (DELETE /<pk>).
This is the standard across pretty much all modern REST APIs.

Thanks for the recommendation, I'll check it out! 😄

atb00ker · 2021-02-28T10:33:03Z

Looks like it's going in the correct direction! 😄

ManishShah120 · 2021-03-03T17:54:10Z

@nemesisdesign , @atb00ker , @pandafy
Can I please get inputs or suggestions regarding this?

I am having a tough time solving this error for the past few days and not able to proceed further, While trying to make a PUT/PATCH request to this http://127.0.0.1:8000/api/v1/controller/device/12eef6c9-6c82-4f43-8f90-0fffaa72d74a/ endpoint I am getting this error. Since the device serializer contains a nested serializer which contains a many-to-many fields[templates]. This is what which is causing this error, while I try to assign/remove templates of any device.

I have tried this approach https://stackoverflow.com/questions/50015204/direct-assignment-to-the-forward-side-of-a-many-to-many-set-is-prohibited-use-e but didn't seem to work.

Also tried overriding the .put and .update() methods by iterating over all the selected templates but still no. What am I doing wrong or missing or should try a different View or serializer.

nemesifier

@ManishShah120 I must see your attempted implementation in order to correct it. Can you point out what lines of codes are you talking about specifically that are not working out?

While we get this sorted out, can you start working on adding tests for the other endpoints which are working and also work on the README documentation?

atb00ker · 2021-03-06T20:42:23Z

@ManishShah120

I was able to solve the problem stackoverflow with the link that you send:

class ConfigSerializer(ValidatedModelSerializer):
    context = serializers.JSONField()
    config = serializers.JSONField()
    # templates = serializers.PrimaryKeyRelatedField(queryset=Template.objects.all(), many=True) # This is working

    def validate(self, data):
        config_templates = Template.objects.filter(name__in=data.pop('templates'))
        instance = self.instance or self.Meta.model(**data)
        instance.templates.add(*config_templates.values_list('pk', flat=True))
        instance.full_clean()
        return data

    class Meta(BaseMeta):
        model = Config
        fields = '__all__'

However, this will not work because you will need to create a new .update() in the view to accommodate the nested object, are you stuck with that or were you stuck with writing the custom .validate() function for template?

ManishShah120 · 2021-03-07T05:58:22Z

@ManishShah120

I was able to solve the problem stackoverflow with the link that you send:
class ConfigSerializer(ValidatedModelSerializer):
    context = serializers.JSONField()
    config = serializers.JSONField()
    # templates = serializers.PrimaryKeyRelatedField(queryset=Template.objects.all(), many=True) # This is working

    def validate(self, data):
        config_templates = Template.objects.filter(name__in=data.pop('templates'))
        instance = self.instance or self.Meta.model(**data)
        instance.templates.add(*config_templates.values_list('pk', flat=True))
        instance.full_clean()
        return data

    class Meta(BaseMeta):
        model = Config
        fields = '__all__'
However, this will not work because you will need to create a new .update() in the view to accommodate the nested object, are you stuck with that or were you stuck with writing the custom .validate() function for template?

Yes @atb00ker I was stuck with the .update method but then the validation was causing the error, Now I would like to put some more effort to see if it works, since the custom validate has solved the issue of validation. Also is the new FOREIGN KEY constraint failed error raised due to the extra Device option present?

atb00ker · 2021-03-07T16:20:29Z

FOREIGN KEY constraint failed

Provide a config with some basic data. (eg: {'general': {'hostname': 'config'}})

(You'll need to check in validation and ensure user gets a more meaningful error when they provide an empty config.)

ManishShah120 · 2021-03-16T13:03:17Z

@atb00ker, @nemesisdesign , I have added the .create and .update method, but still, the direct assignment error exists, When I try to create a device the device gets created without nesting the data of the config relation, but when I perform the same request by popping out the templates field the device is getting created successfully with the configurations, and the defaults and required templates assign to it. Could you please look into it?

And after the device being created, I am able to add more template to any configuration but not able to remove it, But when I try to perform all the operations like assign/unassign of templates, changing the order, everything works fine with the /device/config/<config: id> endpoint.

atb00ker · 2021-03-16T18:13:19Z

I have added the .create and .update method

Where? I don't see those functions, can you please push that code, it's hard to tell the error without seeing the code.

And after the device being created, I am able to add more template to any configuration but not able to remove it

I think a better way to explain the problem would be to follow the format:

Steps to repro
expected result by following steps
actual result by following steps

Is the last problem solved and you've moved ahead of it, or is it the same?

atb00ker · 2021-03-16T18:15:35Z

I have added the .create and .update method

Where? I don't see those functions, can you please push that code, it's hard to tell the error without seeing the code.

And after the device being created, I am able to add more template to any configuration but not able to remove it

I think a better way to explain the problem would be to follow the format:

Steps to repro
expected result by following steps
actual result by following steps

Is the last problem solved and you've moved ahead of it, or is it the same?

ManishShah120 · 2021-03-16T13:04:40Z

openwisp_controller/api/serializers.py

+    def create(self, validated_data):
+        config_data = validated_data.pop('config')
+        config_data.pop('templates')
+        device = Device.objects.create(**validated_data)
+        Config.objects.create(device=device, **config_data)
+        return device


Here after removing the templatesfield,I am able to create a device, with its configuration, but we should not pop this field, because while creating a device user may want to assign any number of templates.

@atb00ker here is the .create method.

Got it, I'll have to look into why do you need to pop the templates.
If I remember correctly, some UTs are saving device without poping template; I would start looking there.

I'll look into it as well! (Probably weekend!)

I just checked it out, looks like this is fixed.
Please update here if done.

openwisp_controller/api/serializers.py

ManishShah120 · 2021-03-16T13:08:13Z

openwisp_controller/api/urls.py

+                    'device-config/<str:pk>/',
+                    api_views.device_config,
+                    name='device_config',


Here, I have added an extra endpoint which works fine for all operation we want from the configurations associated with any device we want to perform, but the problems arises when we try to perform operations on the nested models and serializers.

nemesifier

Great! I did some basic testing again and it's working as expected.

I found other minor details to improve and I realized something is missing: extensibility (running a modified version of the API when the SAMPLE_APP=1 environment variable is set).

See the work done by @atb00ker here:

I think we need to:

add sample_config.api.views
ensure API urls are imported here: https://github.com/openwisp/openwisp-controller/blob/a91e9e76cc0d640f504380bb465f9508f2ff7981/tests/openwisp2/urls.py
import api tests here: https://github.com/openwisp/openwisp-controller/blob/a91e9e76cc0d640f504380bb465f9508f2ff7981/tests/openwisp2/sample_config/tests.py
update the docs in a consistent way with what we have now, linking the REST API in the sample app

@atb00ker please double check this and let me know if I forgot anything regarding extensibility.

See also my comments below.

nemesifier · 2021-05-08T00:00:17Z

openwisp_controller/config/tests/test_api.py

+        data['vpn'] = vpn1.id
+        response = self.client.post(path, data, content_type='application/json')
+        validation_msg = "To select a VPN, set the template type to 'VPN-client'"
+        self.assertIn(validation_msg, response.data['vpn'])


it's always a good idea to assert for the status code as well, eg: self.assertEqual(r.status_code, 400)

nemesifier · 2021-05-08T00:01:46Z

openwisp_controller/config/tests/test_api.py

+        data = self._get_template_data.copy()
+        data['type'] = 'vpn'
+        data['vpn'] = vpn1.id
+        data['organization'] = org1.pk


if the vpn is shared, usually the template is shared as well

Yes, but aren't we testing this for the operator and operator can't create shared templates, other than that admin gets all the the vpn by default, so how are we gonna test for the shared objects. 🤔

nemesifier · 2021-05-08T00:08:28Z

openwisp_controller/config/tests/test_api.py

+        r = self.client.get(path, {'format': 'api'})
+        self.assertContains(r, 'shared_ca</option>')
+        self.assertContains(r, 'shared_cert</option>')
+


I don't see a test which ensures shared templates can be assigned to a device, was that already added before? If not, can you please add it?

Yes, It is contained in this:- https://github.com/ManishShah120/openwisp-controller/blob/4eacd3cca7e06e8a9f84153ef966bb0601d765c1/openwisp_controller/config/tests/test_api.py#L203-L215

nemesifier · 2021-05-08T00:29:25Z

openwisp_controller/config/utils.py

@@ -199,3 +201,49 @@ def get_default_templates_queryset(
    if backend:
        queryset = queryset.filter(backend=backend)
    return queryset
+
+
+def get_api_urls(api_views):


why is this function being added here? I think it should go in openwisp_controller.config.api.urls.

BTW the reasons we use functions is to allow to extende the API views and URLs but I don't see anything done on that front but I think we should.

Okay, will move it to openwisp_controller.config.api.urls

- Moved url function to api dir - Added response status code check in tests

atb00ker

Hi, right now, I am not doing code review just the functional review.

Legends:
[SU] - Tested with Superuser
[O1A] - Tested with Org1 Administrator
[O1O] - Tested with Org1 Operator
Status:Pass - Behaves like admin UI or deviation documented
Status:Fail - Does not behave like admin UI
Status:NTD - Need to discuss

atb00ker · 2021-05-08T15:38:46Z

openwisp_controller/config/api/urls.py

+    if getattr(settings, 'OPENWISP_CONTROLLER_API', True):
+        return [
+            path(
+                'controller/template/',


SN /api/v1/controller/template/ Status

1 [SU] Can view all templates Pass

2 [O1A] can view only org1 templates (not even shared org) Pass

3 [O1O] can view no templates Pass

4 [SU] VPN-Client hidden for Generic Template NTD

5 [SU][O1A] Field validation error on field Fail

6 [SU][O1A] Allows creating template without config Fail

7 [SU][O1A][O1O] Allows creating template with config Pass

8 [SU][O1A][O1O] Allow creating VPN template Pass

9 [SU][O1A][O1O] Allow creating VPN template without openvpn NTD

10 [O1O] Access denied to org1 operator user NTD

11 [SU] Unique template name ensured Pass

12 [O1A] Trying to create template for another org gives object not found error Pass

SN. 4:

@nemesisdesign I think this is an acceptable deviation, can you please confirm.

SN. 5:

@ManishShah120 Can we show the error on config instead of __all__? it would be useful for people making apps using the API.

SN. 9

@nemesisdesign is this okay?

SN. 10
@nemesisdesign Shouldn't we deny access to the page if creating and viewing is not allowed? Same for admin UI. Why is access even allowed?

SN /api/v1/controller/template/ Status
1 [SU] Can view all templates Pass
2 [O1A] can view only org1 templates (not even shared org) Pass
3 [O1O] can view no templates Pass
4 [SU] VPN-Client hidden for Generic Template NTD
5 [SU][O1A] Field validation error on field Fail
6 [SU][O1A] Allows creating template without config Fail
7 [SU][O1A][O1O] Allows creating template with config Pass
8 [SU][O1A][O1O] Allow creating VPN template Pass
9 [SU][O1A][O1O] Allow creating VPN template without openvpn NTD
10 [O1O] Access denied to org1 operator user NTD
11 [SU] Unique template name ensured Pass
12 [O1A] Trying to create template for another org gives object not found error Pass
SN. 4:

@nemesisdesign I think this is an acceptable deviation, can you please confirm.

Yes it's acceptable because it's an API and not a frontend.

SN. 5:

@ManishShah120 Can we show the error on config instead of __all__? it would be useful for people making apps using the API.

If possible this would be a good improvement.

SN. 9

@nemesisdesign is this okay?

It does look good to me. Why it shouldn't? I don't understand your point.

SN. 10
@nemesisdesign Shouldn't we deny access to the page if creating and viewing is not allowed? Same for admin UI. Why is access even allowed?

Right, good catch!
DRF doesn't implement the view permission by default:
https://www.django-rest-framework.org/api-guide/permissions/#djangomodelpermissions

We have to do it. Please @ManishShah120 can you add a DjangoModelPermissions class which inherits from DjangoModelPermissions and adds the view permission as indicated in the DRF docs?
Then we should use this new permission class instead of the default one.

@atb00ker regarding the admin view, what do you refer to exactly?

@nemesisdesign is this okay?

It does look good to me. Why it shouldn't? I don't understand your point.

The VPN servers created from UI always have a openvpn section in configuration, here we are able to make it without that! 😄

@atb00ker SN5, SN6 is resolved.

@atb00ker regarding SN10, I think you missed one more failing case which is foralong with this [O1A] administrator but not manager i.e., (is_admin for org1 not set to True)

Copying @nemesisdesign 's response on Gitter:

@ManishShah120 @atb00ker I am not sure I understand what we're discussing, can you be more explicit and provide some user stories as examples? I am not following you. As a general rule, for now, I'd simplify everything: only organization managers (having is_admin=True) or superusers can use the API non superusers have limited permissions, according to the permissions defined in django So if user is not manager of the org, it can't do anything via the API right now. If user is superuser, can do anything. If user is not superuser and is manager of an org, it can do what their django permissions allow them to do.

atb00ker

More functional checks, we have some failures, please check them. Please test all the cases and ensure they are passing before the next functional review, this will help speed up the process! 😄

I have tested VPN & Config, I expect similar failures in device create page, can you please check that as well before I do functional check of the entire thing again? :-)

one more problem I've noticed in multiple places is that when there are multiple errors in the form, it reports only one or two of them, no all. I think you are returning when you encounter an error, can we return after we have collected all the errors? 😄
After sucessful creation of device/template/vpn, can we clean the form?
After succesful deletion, it does not redirect me to list page.

P.S: Please wait for Fed to check the 3 points above.

atb00ker · 2021-05-08T17:53:38Z

openwisp_controller/config/api/urls.py

+                name='api_template_list',
+            ),
+            path(
+                'controller/template/<str:pk>/',


controller/template/str:pk/ Status

1 [SU] Can view any templates Pass

2 [O1A] Can view only org1 templates (not even shared org) Pass

3 [O1O] Can view no templates Pass

4 [SU][O1A] Correct data of the template shown Pass

5 [SU][O1A][O1O] Can only delete templates if they have correct privilege Pass

atb00ker · 2021-05-08T18:06:27Z

openwisp_controller/config/api/urls.py

+                name='api_template_detail',
+            ),
+            path(
+                'controller/template/<str:pk>/configuration/',


controller/template/str:pk/configuration Status

1 [SU][O1A][O1O] Can download any configuration Pass

2 [SU][O1A] Correct template config downloaded Pass

atb00ker · 2021-05-08T19:14:55Z

openwisp_controller/config/api/urls.py

+                api_views.download_template_config,
+                name='api_download_template_config',
+            ),
+            path('controller/vpn/', api_views.vpn_list, name='api_vpn_list',),


controller/vpn/ Status Fail Reason

1 [SU] Can view any VPN Pass

2 [O1A] Can view only org1 VPN (not even shared org) Pass

3 [O1O] Cannot view any VPN Pass

4 [SU][O1A] Field validation error on field Fail { "config" : ["This field cannot be blank."], "all" :["Invalid configuration triggered by "#/", validator says:\n\n'openvpn' is a required property"] }

5 [SU] Only valid config schema accepted Pass

6 [SU][O1A] Can create VPN Pass

7 [SU][O1A] Cannot create VPN Pass

8 [SU][O1A] Unique name per org check Pass

9 [SU][O1A] Appropriate CA/Certs visible Fail O1A does not see shared CA/Certs

10 [01A] Trying to create device with another org's id gives object not found error Pass

I'm afraid "the invalid configuration triggered by ... " is defined at model level and we have to leave as is for now.

S9, is working fine for me.

atb00ker · 2021-05-08T19:19:21Z

openwisp_controller/config/api/urls.py

+            ),
+            path('controller/vpn/', api_views.vpn_list, name='api_vpn_list',),
+            path(
+                'controller/vpn/<str:pk>/', api_views.vpn_detail, name='api_vpn_detail',


SN controller/vpn/str:pk/ Status

1 [SU] Can view any vpn Pass

2 [O1A] Can view only org1 vpns (not even shared org) Pass

3 [O1O] Cannot view any vpn Pass

4 [SU][O1A] Correct data of the vpn shown Pass

5 [SU][O1A][O1O] Can only delete templates if they have correct privilege Pass

atb00ker · 2021-05-08T19:26:53Z

openwisp_controller/config/api/urls.py

+                'controller/vpn/<str:pk>/', api_views.vpn_detail, name='api_vpn_detail',
+            ),
+            path(
+                'controller/vpn/<str:pk>/configuration/',


controller/vpn/str:pk/configuration Status

1 [SU] Can download any configuration Pass

2 [SU][O1A] Correct template config downloaded Pass

nemesifier · 2021-05-09T23:37:03Z

More functional checks, we have some failures, please check them. Please test all the cases and ensure they are passing before the next functional review, this will help speed up the process!

I have tested VPN & Config, I expect similar failures in device create page, can you please check that as well before I do functional check of the entire thing again? :-)

one more problem I've noticed in multiple places is that when there are multiple errors in the form, it reports only one or two of them, no all. I think you are returning when you encounter an error, can we return after we have collected all the errors? smile

Can you point out the parts of the code you refer to? We need to be precise with our requests, we have to ensure there's no ambiguity.

After sucessful creation of device/template/vpn, can we clean the form?

After succesful deletion, it does not redirect me to list page.

Isn't this the standard behavior of DRF?
If it's the standard DRF behvaior we can leave it.

ManishShah120 · 2021-05-10T15:26:55Z

Hi, @nemesisdesign @atb00ker all the tests are passing locally, I am not able to figure out the actual reason for build failing here.

atb00ker · 2021-05-10T18:28:39Z

Can you point out the parts of the code you refer to? We need to be precise with our requests, we have to ensure there's no ambiguity.

There were too many cases of it that I didn't document them, after @ManishShah120 spends sometime at it, if I still find a case, I'll document it! 😄

P.S: I think I am going to start a document for the functional testing -- to list all TC and failures, might be useful in the future as well! 😄

atb00ker · 2021-05-10T18:29:19Z

all the tests are passing locally, I am not able to figure out the actual reason for build failing here.

@ManishShah120 Have you tried SAMPLE_APP=1 ./runtests.py as well? 😄

P.S: also Rebase! 😄

nemesifier

all the tests are passing locally, I am not able to figure out the actual reason for build failing here.

@ManishShah120 Have you tried SAMPLE_APP=1 ./runtests.py as well?

P.S: also Rebase!

👍

nemesifier · 2021-05-10T21:05:47Z

Can you point out the parts of the code you refer to? We need to be precise with our requests, we have to ensure there's no ambiguity.

There were too many cases of it that I didn't document them, after @ManishShah120 spends sometime at it, if I still find a case, I'll document it!

P.S: I think I am going to start a document for the functional testing -- to list all TC and failures, might be useful in the future as well!

Sounds good, thank you Ajay.

ManishShah120 · 2021-05-11T06:01:17Z

There were too many cases of it that I didn't document them, after @ManishShah120 spends sometime at it, if I still find a case, I'll document it!

Yes, I am trying to tackle each one of them.

ManishShah120 · 2021-05-11T18:54:06Z

Hi, @atb00ker @nemesisdesign I tried to override the DjangoModelPermissions in this way:-

class CustomDjangoModelPermission(DjangoModelPermissions):
    def __init__(self):
        self.perms_map = copy.deepcopy(self.perms_map)
        self.perms_map['GET'] = ['%(app_label)s.view_%(model_name)s']

Base DjangoModelPermission:- https://github.com/encode/django-rest-framework/blob/3875d3284e73ed4d8e36c07d9b70c1b22c9d5998/rest_framework/permissions.py#L162

But with the above code all the administrator and operator gets effected, whereas only the page should be obstructed for org_users.
I also suggest we should do something with the model permissions in migrations. As it gets the same permissions as in the admin panel.

Else If I am missing something, please suggest. 😄

other than this all the issue from @atb00ker review is resolved.

nemesifier

Ok @ManishShah120, you've done great work in this PR and I think this is now ready for usage, even though there's some minor details missing for which I have created issues:

[change] REST API: implement view permissions #454 (depends on [feature] Add class to implement view permissions in DRF openwisp-users#249)

@atb00ker let me know if you find other issues and let's create more follow up issues if needed.

We will surely find things to improve in the following weeks while testing, so @ManishShah120 stay tuned and please follow up and help us with the issues mentioned above, all those are needed for a proper implementation of a REST API in @openwisp 😊 .

ManishShah120 marked this pull request as draft February 17, 2021 13:16

nemesifier requested changes Feb 18, 2021

View reviewed changes

ManishShah120 force-pushed the issues/379-REST_API-for-main-controller branch from 85edce7 to 5760b39 Compare February 23, 2021 18:25

nemesifier assigned ManishShah120 Feb 24, 2021

ManishShah120 force-pushed the issues/379-REST_API-for-main-controller branch 2 times, most recently from 4feda2a to a1888ff Compare February 26, 2021 18:03

atb00ker reviewed Feb 28, 2021

View reviewed changes

tests/openwisp2/urls.py Show resolved Hide resolved

atb00ker reviewed Feb 28, 2021

View reviewed changes

ManishShah120 force-pushed the issues/379-REST_API-for-main-controller branch 2 times, most recently from 806d3c6 to fab61ba Compare February 28, 2021 16:44

ManishShah120 force-pushed the issues/379-REST_API-for-main-controller branch from fab61ba to c5a72ce Compare March 3, 2021 17:32

nemesifier reviewed Mar 6, 2021

View reviewed changes

ManishShah120 force-pushed the issues/379-REST_API-for-main-controller branch 2 times, most recently from e8b1d70 to 88a2c48 Compare March 16, 2021 12:53

devkapilbansal force-pushed the issues/379-REST_API-for-main-controller branch 2 times, most recently from 0d351ce to 88a2c48 Compare March 16, 2021 17:49

ManishShah120 commented Mar 16, 2021

View reviewed changes

ManishShah120 force-pushed the issues/379-REST_API-for-main-controller branch from 99cc1ea to 4eacd3c Compare May 7, 2021 21:56

nemesifier requested changes May 8, 2021

View reviewed changes

[api] Minor improvements

3eed885

- Moved url function to api dir - Added response status code check in tests

atb00ker reviewed May 8, 2021

View reviewed changes

[api] Added Extensibility feat for the config api

15405eb

[chores] Fix field name while validation in template serializer

7e1d9ca

nemesifier reviewed May 10, 2021

View reviewed changes

[tests] Fixed build failing issue

c742290

nemesifier mentioned this pull request May 12, 2021

[change] REST API: implement view permissions #454

Closed

nemesifier previously approved these changes May 12, 2021

View reviewed changes

[chores] Point screenshots to master

28f1622

nemesifier dismissed their stale review via 28f1622 May 12, 2021 03:44

nemesifier approved these changes May 12, 2021

View reviewed changes

nemesifier merged commit 9108474 into openwisp:master May 12, 2021

ManishShah120 mentioned this pull request May 14, 2021

[feature] Add class to implement view permissions in DRF openwisp/openwisp-users#249

Closed

nemesifier mentioned this pull request Jun 13, 2021

[controller/feature] Add DeviceGroup model #203

Closed

8 tasks

pandafy mentioned this pull request Jun 15, 2021

[feature] Added device groups #203 #492

Merged

8 tasks

		path(
		'device/<str:pk>/', api_views.device_detail, name='device_detail',

SN	/api/v1/controller/template/	Status
1	[SU] Can view all templates	Pass
2	[O1A] can view only org1 templates (not even shared org)	Pass
3	[O1O] can view no templates	Pass
4	[SU] VPN-Client hidden for Generic Template	NTD
5	[SU][O1A] Field validation error on field	Fail
6	[SU][O1A] Allows creating template without config	Fail
7	[SU][O1A][O1O] Allows creating template with config	Pass
8	[SU][O1A][O1O] Allow creating VPN template	Pass
9	[SU][O1A][O1O] Allow creating VPN template without openvpn	NTD
10	[O1O] Access denied to org1 operator user	NTD
11	[SU] Unique template name ensured	Pass
12	[O1A] Trying to create template for another org gives object not found error	Pass

	controller/template/str:pk/	Status
1	[SU] Can view any templates	Pass
2	[O1A] Can view only org1 templates (not even shared org)	Pass
3	[O1O] Can view no templates	Pass
4	[SU][O1A] Correct data of the template shown	Pass
5	[SU][O1A][O1O] Can only delete templates if they have correct privilege	Pass

	controller/template/str:pk/configuration	Status
1	[SU][O1A][O1O] Can download any configuration	Pass
2	[SU][O1A] Correct template config downloaded	Pass

	controller/vpn/	Status	Fail Reason
1	[SU] Can view any VPN	Pass
2	[O1A] Can view only org1 VPN (not even shared org)	Pass
3	[O1O] Cannot view any VPN	Pass
4	[SU][O1A] Field validation error on field	Fail	{ "config" : ["This field cannot be blank."], "all" :["Invalid configuration triggered by "#/", validator says:\n\n'openvpn' is a required property"] }
5	[SU] Only valid config schema accepted	Pass
6	[SU][O1A] Can create VPN	Pass
7	[SU][O1A] Cannot create VPN	Pass
8	[SU][O1A] Unique name per org check	Pass
9	[SU][O1A] Appropriate CA/Certs visible	Fail	O1A does not see shared CA/Certs
10	[01A] Trying to create device with another org's id gives object not found error	Pass

SN	controller/vpn/str:pk/	Status
1	[SU] Can view any vpn	Pass
2	[O1A] Can view only org1 vpns (not even shared org)	Pass
3	[O1O] Cannot view any vpn	Pass
4	[SU][O1A] Correct data of the vpn shown	Pass
5	[SU][O1A][O1O] Can only delete templates if they have correct privilege	Pass

	controller/vpn/str:pk/configuration	Status
1	[SU] Can download any configuration	Pass
2	[SU][O1A] Correct template config downloaded	Pass

[feature] REST API for main controller features #379 #386

[feature] REST API for main controller features #379 #386

Conversation

ManishShah120 commented Feb 17, 2021

ManishShah120 commented Feb 17, 2021

atb00ker commented Feb 17, 2021

nemesifier left a comment

Choose a reason for hiding this comment

nemesifier left a comment • 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

Choose a reason for hiding this comment

coveralls commented Feb 23, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nemesifier Mar 2, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

atb00ker commented Feb 28, 2021

ManishShah120 commented Mar 3, 2021

nemesifier left a comment

Choose a reason for hiding this comment

atb00ker commented Mar 6, 2021 • edited Loading

ManishShah120 commented Mar 7, 2021 • edited Loading

atb00ker commented Mar 7, 2021 • edited Loading

ManishShah120 commented Mar 16, 2021

atb00ker commented Mar 16, 2021

atb00ker commented Mar 16, 2021

ManishShah120 Mar 16, 2021 • 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

nemesifier 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

ManishShah120 May 8, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

atb00ker left a comment

Choose a reason for hiding this comment

atb00ker May 8, 2021 • edited Loading

Choose a reason for hiding this comment

nemesifier May 9, 2021 • 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

atb00ker 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

nemesifier commented May 9, 2021

ManishShah120 commented May 10, 2021

atb00ker commented May 10, 2021

atb00ker commented May 10, 2021 • edited Loading

nemesifier left a comment

Choose a reason for hiding this comment

nemesifier commented May 10, 2021

ManishShah120 commented May 11, 2021 • edited Loading

ManishShah120 commented May 11, 2021 • edited Loading

nemesifier left a comment

Choose a reason for hiding this comment

nemesifier left a comment •

edited

Loading

coveralls commented Feb 23, 2021 •

edited

Loading

nemesifier Mar 2, 2021 •

edited

Loading

atb00ker commented Mar 6, 2021 •

edited

Loading

ManishShah120 commented Mar 7, 2021 •

edited

Loading

atb00ker commented Mar 7, 2021 •

edited

Loading

ManishShah120 Mar 16, 2021 •

edited

Loading

ManishShah120 May 8, 2021 •

edited

Loading

atb00ker May 8, 2021 •

edited

Loading

nemesifier May 9, 2021 •

edited

Loading

atb00ker commented May 10, 2021 •

edited

Loading

ManishShah120 commented May 11, 2021 •

edited

Loading

ManishShah120 commented May 11, 2021 •

edited

Loading