openwisp / openwisp-radius Goto Github PK

1. We need to update all the dependencies.
2. Add support for django 2.2
3. Add support / tests for python 3.6 & 3.7

Add support for throttling to the endpoints using openwisp_utils.api.apps.ApiConfig.

The following ones, should have throttling set to None by default:

• authorize
• postauth
• accounting

These two should have a strict throttling policy:

• obtain_auth_token
• validate_auth_token

A little bit less strict:

• create_phone_token
• validate_phone_token

More relaxed policy: all the rest.

The build on the current master is broken, there's some issue that has appeared after changes in openwisp-users.

A unit test should check that the API view of the batch user creation operation assigns created users to the right organization.

Some of them seem to fail as of now.

I think we can find a way to simplify the REST API endpoints consumed by freeradius so that the need of having one freeradius instance for each organization is avoided, this would make it also easier to create new organizations.

I think that we can use the RadiusToken model, when a new session starts and the RadiusToken instance is created (or updated if already exists) and it should carry also the information of the organization.

We can also add an "is_expired" boolean to indicate whether this token can be used for authentication or not.

There can be at max one RadiusToken instance per user. When it already exists it will be just updated.

The REST API endpoints consumed by freeradius will have to do extra queries to find the radius token associated to the current request and will figure out the organization based on that information.

We should also cache the RadiusToken with the username as part of the key, so that everytime we need to do the lookup we can look in the cache first.

This should work!

How to reproduce the problem:

1. set DJANGO_FREERADIUS_EDITABLE_ACCOUNTING = True in your local_settings.py (or settings.py if you like quick & dirty hacks)
2. ensure you have at least 2 organizations created in the system
3. create a few fake accounting records for both organizations
4. now open http://localhost:8000/api/v1/accounting/?uuid=<uuid-of-org>&token=<token-of-org> substituting and with the uuid and token of one of the two orgs

We expect to be able to see only accounting records of that org, but we see all of them.

How to fix:

• write a failing test case that replicates what you just did manually in the automated test suite (Test Driven Development is the best approach when fixing bugs)
• now edit AccountingView, probably defining a custom get_queryset method which filters by organization id would do, you can get the organization id from self.request.auth (read the code of TokenAuthentication and see also get_queryset on DRF docs)

• #89 (review)
• #89 (review)

Depends on openwisp/openwisp-utils#98.

Use the decorators mentioned in the issue linked above to silence the output of noisy tests.

The README of this project needs some love ;-)

PS: this will need to be updated too https://github.com/openwisp/openwisp-radius/blob/master/CONTRIBUTING.rst

It seems to me that the OrganizationRadiusSettings object (and relative secret token) is not being created for the default organizaiton.

Write a test which fails if the default org does not have a related OrganizationRadiusSettings created.

Let the test fail, then add the logic to create it in the add_default_organization function (which is called during the first DB migration).

Reserved for GCI

This issue can be done only once openwisp/openwisp-controller#44 is completed.

Remove openwisp_radius.admin.AlwaysHasChangedForm logic in favour of AlwaysHasChangedMixin in openwisp-utils (instead of inheriting from object, inherith from AlwaysHasChangedMixin and remove the logic of the class).

This is done to avoid duplication with other modules that are implementing similar logic.

Hi everybody,
we are planning to implement a new version in Django of the OpenWISP User Management System
using these 3 modules: Django-freeradius, openwisp-users, openwisp-radius

As we know, openwisp-users is the django app to manage operators of the web application with a profile and an Organization;
openwisp-radius is the django app to manage multi organizations for radius database and
Django-freeradius is the django app to manage directly freeradius database.

openwisp-users should implement user registration and verification via multiple methods such as mobile phone call, credit card check and saml2 assertions.

openwisp-users should also implements global and per openwisp-user stats

Now we want to start to implement the phone call check, so we are asking
guidelines to integrate our code.

Thank you in advance
Regards

Ilaria

SQL query fails if valid_until is null/blank.
We just have to decide to modify radcheck SQL query or force valid_until to be not null.

Depends on openwisp/openwisp-users#122.

Once swappable models are available in openwisp-users, we should update openwisp-radius to make sure it imports the models of openwisp-radius using the swappable methods and we should provide a documentation that explains how to extend the user model.

We should make sure that when the user model is extended, the new fields are automatically included in its API.

Add two endpoints:

• one for users who forgot their password and need to reset it
• one for users which are authenticated and need to change their password

django-rest-auth may already provide these endpoints, in that case we can mostly reuse their base classes as we do for the registration API endpoint.

Reserved for GCI

Now that openwisp/openwisp-users#40 is ready we should enable operators to manage the following objects:

• accounting
• post auth logs

While all the remaining objects of openwisp-radius should be read-only (will only work for django >= 2.1).

Administrators instead will be able to manage everything.

Similar PRs done in other modules:

• openwisp/openwisp-users#40
• openwisp/openwisp-controller#57

Error:

(openwisp-radius) $ pip install -r requirements.txt --upgrade
 
ERROR: Could not find a version that satisfies the requirement django-freeradius<0.2.0 (from -r requirements.txt (line 1)) (from versions: 0.1a0)
ERROR: No matching distribution found for django-freeradius<0.2.0 (from -r requirements.txt (line 1))

Even the travis on openwisp-radius doesn't use requirements.txt, which I think it should before installing anything from github/master!

The BatchView of the API should be usable only by admin/staff users.

The organization should be part of the URL, eg: /api/v1/<org-slug>/radius/batch/.

It should use the user authentication from openwisp-users (not the other authentication designed to be used by freeradius).
It should use the django permissions to reject requests coming from users which do not have permissions to manipulate batch objects, it should allow superusers though.

Test will have to be changed. API docs need to be updated.

Prepare the docs for this: openwisp/openwisp-docs#107
Do this before publishing the docs on readthedocs.org.

Users created with batch operations from the admin must be created as members of the same organization which was used to create the batch operation object, at the moment they get no organization.

It seems that this is being done for the API but the feature has to be unit tested.

Move tests.py and mixins.py to new test/ dir to clearly indicate that logic in both files is used in automated tests.

Use openwisp-users 0.3 as minimum version.

Use openwisp_users.authentication.BearerAuthentication instead of rest_framework.authentication.TokenAuthentication.

Update documentation to mention this.

This app will have to be upgraded to support this change: https://github.com/openwisp/openwisp-wifi-login-pages

At the moment it is necessary to change the configuration of freeradius to send the organization, but this doesn't scale well when using multiple organizations.

We can make openwisp-radius automatically figure out for which organization the request is coming from by doing something similar to what openwisp-controller does.

Here is a rough estimate of the things that need to be changed/added:

• a model similar to to OrganizationConfigSettings (eg: OrganizationRadiusSettings) of openwisp-controller should be added, this model must store the token for the freeradius API and a boolean value that indicates whether the freeradius module is active for that organization (some organization may not use it and hence it doesn't make sense to keep it enabled, it also reduces the attack surface)
• TokenAuthentication needs to be extended and instead of checking the settings it should query the database, should look for the uuid and token of the organization, if not found should return 403
• since freeradius may send many requests to the API, performing the same DB query each time for authorization would slow the system down, so we should use caching to avoid this
• the cache needs to be invalidated (partially or totally, depends on how we decide to implement it) each time OrganizationRadiusSettings emits the signal of post_save

This change may require changes in django-freeradius too.

• Merge django-freeradius into openwisp-radius
• Migrate the django settings of each module so that the naming is consistent. (eg: NETJSONCONFIG_[SETTING_NAME] -> OPENWISP_CONTROLLER_[SETTING_NAME])
• Keep backward compatibility for setting names but log a warning to encourage the users to upgrade the name of the setting to the new name.
• Implement as easy as possible extensibility of Django modules.
• Add sample_app for testing extensibility with some changes.
• Update documentation

There are very limited attributes as of now for the RadiusCheck model. We could have additional attributes or add a way to use those additional attributes. Like Simultaneous-Use etc. We'll have to research a bit more about this and then proceed. This option exists for the RadiusGroupCheck as of now.

def test_radius_group_delete_default_by_non_superuser(self):
        user = User.objects.get(username='admin')
        user.is_superuser = False
        user.save()
        for permission in Permission.objects.all():
            user.user_permissions.add(permission)
        rg = self.radius_group_model.objects
        default = rg.get(default=True)
        url_name = 'admin:{0}_radiusgroup_delete'.format(self.app_name)
        delete_url = reverse(url_name, args=[default.pk])
        response = self.client.get(delete_url)
        self.assertEqual(rg.filter(default=True).count(), 1)
        self.assertEqual(response.status_code, 403)

This test fails if the user deleting RadiusGroup is from diffrent organisation. Instead of returning Forbidden it redirects to admin homepage.

How to reproduce?
Create a new user (say dummy), assign him staff user (not superuser) and assign all permissions to it. Instead of using default organization, create a new organization (say dummyorg) and add that organization to this user.
Now, copy primary key of default radiusgroup of default organisation and go to this link after signing in from this dummy account. Link : 127.0.0.1:8000/admin/openwisp_radius/radiusgroup//delete
Instead of getting a 403 Forbidden page, you'll be redirected to admin homepage, with a message suggesting that this object might already be deleted.

URLs are a bit messy now, it's better to rename and reorganize them before the 0.1 release.

It may be necessary to change some URLs in django-freeradius too.

Connecting signals in this way is not as good as doing it in the ready method of the AppConfig.

This commit in django-freeradius makes that line unnecessary and we can remove it.

Enable CORS in django rest framework for cross-domain ajax requests from the frontend Apps

Same as openwisp/django-freeradius#41.

It would be cool to make it possible to reuse this migration here:
https://github.com/openwisp/django-freeradius/blob/master/django_freeradius/migrations/0001_initial_squashed.py

Enable API docs (Swagger/OpenAPI).

Improve the documentation to mention this, show a screenshot, and mention the browsable API as well, as we did in openwisp-firmware-upgrader (use a similar structure).

Add support for django 3.0 and update dependencies.

Drop support for python 3.5, add 3.6 and 3.7 to the travis build.

It may be possible and useful to implement the session limits defined in group checks directly in the REST API without the need of configuring the sqlcounter in freeradius.

The advantages of doing so could be:

• configuration of freeradius would result simpler
• we could make the API view extensible and customizable, opening up to scenarios that now are not even thinkable with the freeradius sqlcounter module
• it would support multi-tenancy out of the box

This is just a proposal for the future, I'll leave it here in case somebody is interested.

How to reproduce the problem:

1. open http://localhost:8000/api/v1/accounting/?uuid=<uuid-of-org>&token=<token-of-org>
2. click on "Filters" in the upper right corner
3. notice the last row: [invalid name]:

How do we fix that? Could you check if it's happening also in django-freeradius? Does it have to be fixed in django-freeradius to be fixed here as well?

As of now only the very basic multitenancy features are available. This thread could be used a discussion thread to see what all are needed.

Reserved for GCI

Help us to to get the test coverage of openwisp-radius to 100%.
To inspect the test coverage, run these commands:

coverage run --source=openwisp_radius runtests.py
coverage html

The last command will create an HTML report which you can open in the browser and inspect in order to find which lines of codes need to be executed in the test to raise the test coverage.

Ask your mentors if you have any doubt.

There are some problems with the radius profile implementation:

• The data migration to create default radius profiles needs to be added. It would be cool to reuse the one of django-freeradius: https://github.com/openwisp/django-freeradius/blob/master/django_freeradius/migrations/0002_create_default_radius_profiles.py
• Moreover, we also need to edit the RadiusProfile model so it can be shared across organizations, the default profiles must be available to all organizations.
• Last but not least, the RadiusUserProfile does not need the organization field.

Calling ./runtests.py fails:

./runtests.py --keepdb
Using existing test database for alias 'default'...
System check identified no issues (0 silenced).
E
======================================================================
ERROR: openwisp_radius.tests (unittest.loader._FailedTest)
----------------------------------------------------------------------
ImportError: Failed to import test module: openwisp_radius.tests
Traceback (most recent call last):
  File "/usr/lib/python3.5/unittest/loader.py", line 428, in _find_test_path
    module = self._get_module_from_name(name)
  File "/usr/lib/python3.5/unittest/loader.py", line 369, in _get_module_from_name
    __import__(name)
  File "/home/nemesis/Code/openwisp-radius/openwisp_radius/tests.py", line 7, in <module>
    from django_freeradius.tests import FileMixin
ImportError: cannot import name 'FileMixin'

The freeradius configuration documentation page assumes the configuration is being done to set up for a captive portal wifi setup, other configurations like WPA2 Enterprise are implemented differently so we need to make this more explicit.

Currently, openwisp-radius doesn't have any documentation of it's own,
luckily the documentation for openwisp-radius is going to be similar to the documentation of django-freeradius: https://github.com/openwisp/django-freeradius

For this task, you need to:

1. Read about multi-tenancy, the difference between django-freeradius and openwisp-radius is multi-tenancy.
2. Create a similar appropriate documentation for openwisp-radius.
3. Submit the pull request.