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

APDS-61 - [BE] add API to list and delete permanent tokens #1

Merged
merged 14 commits into from
Aug 1, 2017

Conversation

poxip
Copy link

@poxip poxip commented Jul 21, 2017

This also closes APDS-60.

In this PR I had to drop support for DRF 3.1 and 3.2, because their code is incompatible with Django >= 1.10 (see: encode/django-rest-framework#3252 - it has been fixed in 3.3). I can't do anything about it to make it backwards compatible.

@poxip poxip requested review from remik, jacoor and pkrzyzaniak July 21, 2017 12:00
def get_queryset(self):
return self.queryset.filter(user=self.request.user)

def get(self, *args, **kwargs):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this and below if you have mixins?

Copy link
Author

@poxip poxip Jul 21, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mixins used here only add the "destroy" and "list" methods

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We used mixins so router should discover it. Check: http://www.django-rest-framework.org/api-guide/viewsets/

@@ -492,3 +495,38 @@ def test_refresh_jwt_after_refresh_expiration(self):
def tearDown(self):
# Restore original settings
api_settings.JWT_ALLOW_REFRESH = DEFAULTS['JWT_ALLOW_REFRESH']


class DeviceViewTests(TokenTestCase):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add devices for not your user to make sure we are not showing it and can't delete.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in the CR commit.

tests/urls.py Outdated
@@ -28,6 +28,8 @@ def post(self, request):
url(r'^auth-token/$', views.obtain_jwt_token),
url(r'^auth-token-refresh/$', views.refresh_jwt_token),
url(r'^auth-token-verify/$', views.verify_jwt_token),
url(r'^devices/(?P<pk>[\w]+)$', views.device_delete_view),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use router.

@poxip
Copy link
Author

poxip commented Jul 21, 2017

The tests are okay (see details), Travis does not provide Python3.3 somehow (it works in the original repo though). We do not have to drop support for DRF 3.0, 3.1, 3.2 because those versions are only supposed to work with Django 1.8 and 1.9.

@poxip poxip changed the title APDS-61 - add API to list and delete permanent tokens APDS-61 - [BE] add API to list and delete permanent tokens Jul 24, 2017
user = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.CASCADE)
name = models.CharField(_('Device name'), max_length=255)
details = models.CharField(_('Device details'), max_length=255, blank=True)
last_request_datetime = models.DateTimeField(auto_now=True)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing empty line at the end of file.
Pylint and pep8 will complain.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GitHub does not show the last empty line as far as I remember. The flake8 task in tox did not complain, see: https://travis-ci.org/ArabellaTech/django-rest-framework-jwt/jobs/256993179

class DeviceSerializer(serializers.ModelSerializer):
class Meta:
model = Device
fields = ['permanent_token', 'jwt_secret', 'created', 'name', 'details', 'last_request_datetime']
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WHat?! you return JWT secret to the world?
http://getblimp.github.io/django-rest-framework-jwt/#jwt_secret_key Please check documentation what JWT secret is.

self.assertEqual(response.status_code, 200)
self.assertEqual(len(response.data), 1)
self.assertEqual(set(response.data[0].keys()), {
'permanent_token', 'jwt_secret', 'created', 'name', 'details', 'last_request_datetime'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, do not make any field secret public. After all, there is a reason why they call it secret.
http://getblimp.github.io/django-rest-framework-jwt/#jwt_secret_key

Change jwt_secret to UUUIDField with auto value, add Device.id and remove vulnerable data from Device serializer
* APDS-65 - [BE] modify login API to generate permanent token

* Remove TODO

* Remove python select from travis config

* Use user agent

* Remove Python 3.3 from travis

* CR

* Correct

* APDS-64 - [BE] add API to logout device (#6)

* APDS-64 - [BE] remove Device on logout

* Corrects

* Replace doublequotes with singlequotes

* Remove duplicated lines

* Remove python setting from Travis config

* Use user agent in logout view

* Fix tests

* Remove Python 3.3 from travis

* CR

* CR

* APDS-63, APDS-62 - [BE] add API to refresh token using permanent_token (#4)

* APDS-63 - [BE] disallow passing permanent_token header in views other than device refresh

This requires APDS-62 to be finished

* APDS-62 - [BE] add API to refresh token using permanent_token

* Replace doublequotes with singlequotes

* Update with changes from APDS-62

* Remove python setting from .travis.yml

* Correct tests

* Auto logout when permanent token has expired

* Remove Python 3.3 from travis

* CR

* Remove addons from .travis.yml
@poxip poxip merged commit 73b247b into master Aug 1, 2017
@poxip poxip deleted the feature/APDS-61-add-api branch August 1, 2017 12:21
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.

3 participants