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-85 - [BE] move permanent token authentication to a separate library #1

Merged
merged 19 commits into from
Aug 4, 2017

Conversation

poxip
Copy link
Contributor

@poxip poxip commented Aug 3, 2017

No description provided.

@poxip poxip requested review from remik and pkrzyzaniak August 3, 2017 09:45
@coveralls
Copy link

coveralls commented Aug 3, 2017

Coverage Status

Changes Unknown when pulling 091143f on feature/APDS-85 into ** on master**.

@coveralls
Copy link

coveralls commented Aug 3, 2017

Coverage Status

Changes Unknown when pulling f5d3231 on feature/APDS-85 into ** on master**.

@coveralls
Copy link

coveralls commented Aug 3, 2017

Coverage Status

Changes Unknown when pulling b0a4c73 on feature/APDS-85 into ** on master**.

@coveralls
Copy link

coveralls commented Aug 3, 2017

Coverage Status

Changes Unknown when pulling b0a4c73 on feature/APDS-85 into ** on master**.

@coveralls
Copy link

coveralls commented Aug 3, 2017

Coverage Status

Changes Unknown when pulling 59da092 on feature/APDS-85 into ** on master**.

@coveralls
Copy link

coveralls commented Aug 3, 2017

Coverage Status

Changes Unknown when pulling e259db6 on feature/APDS-85 into ** on master**.

5 similar comments
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling e259db6 on feature/APDS-85 into ** on master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling e259db6 on feature/APDS-85 into ** on master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling e259db6 on feature/APDS-85 into ** on master**.

@coveralls
Copy link

coveralls commented Aug 3, 2017

Coverage Status

Changes Unknown when pulling e259db6 on feature/APDS-85 into ** on master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling e259db6 on feature/APDS-85 into ** on master**.

@coveralls
Copy link

coveralls commented Aug 3, 2017

Coverage Status

Changes Unknown when pulling 298ad91 on feature/APDS-85 into ** on master**.

@coveralls
Copy link

coveralls commented Aug 3, 2017

Coverage Status

Changes Unknown when pulling 298ad91 on feature/APDS-85 into ** on master**.

@coveralls
Copy link

coveralls commented Aug 3, 2017

Coverage Status

Changes Unknown when pulling 3d19d8f on feature/APDS-85 into ** on master**.

@coveralls
Copy link

coveralls commented Aug 3, 2017

Coverage Status

Changes Unknown when pulling 3d19d8f on feature/APDS-85 into ** on master**.

Things that came out during implementing the library in demo api
@coveralls
Copy link

coveralls commented Aug 3, 2017

Coverage Status

Changes Unknown when pulling f086e21 on feature/APDS-85 into ** on master**.

@coveralls
Copy link

coveralls commented Aug 4, 2017

Coverage Status

Changes Unknown when pulling bd58aa5 on feature/APDS-85 into ** on master**.

Copy link
Contributor

@remik remik left a comment

Choose a reason for hiding this comment

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

Minor comment.

@@ -0,0 +1,2 @@
# Change Log
All notable changes to this project will be documented in this file.
Copy link
Contributor

Choose a reason for hiding this comment

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

Add 0.1 with initial version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I will change it to 1.0.0 (as we talked about it, we will use semver).

@coveralls
Copy link

coveralls commented Aug 4, 2017

Coverage Status

Changes Unknown when pulling 1e88eb7 on feature/APDS-85 into ** on master**.

Copy link

@pkrzyzaniak pkrzyzaniak left a comment

Choose a reason for hiding this comment

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

Two things for the future:

  • there should be a way to see all logged in devices (as in google for example): so you can see which devices are authorized
  • there should be a way to logout all devices / single device, not necessarily the one user is operating at the moment.

{
"token": "ads344fdgfd5454yJ0eAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJ1c2VynRlYW1AYXJhYmVsLmxh",
"permanent_token": "gfd5454yJ0eAiOiJKV1QiLCJhbGciOiJ",
"device_id": 1

Choose a reason for hiding this comment

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

why the need for device_id? permanent_token would not be enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was agreed with @jacoor in the previous pull requests (in https://github.com/ArabellaTech/django-rest-framework-jwt), permanent_token is too fragile.

README.rst Outdated
``REST_FRAMEWORK`` configuration in **settings.py**


**PermitHeaders middleware**

Choose a reason for hiding this comment

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

PermittedHeaders would be a better name I think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, why not, will change.

@poxip
Copy link
Contributor Author

poxip commented Aug 4, 2017

@pkrzyzaniak Both of those things are implemented. You can list all user's devices (DeviceViewSet), and user can logout each device they are logged in.

@poxip poxip merged commit 04b5619 into master Aug 4, 2017
@poxip poxip deleted the feature/APDS-85 branch August 4, 2017 13:18
@coveralls
Copy link

coveralls commented Aug 4, 2017

Coverage Status

Changes Unknown when pulling 10f1647 on feature/APDS-85 into ** on master**.

7 similar comments
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 10f1647 on feature/APDS-85 into ** on master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 10f1647 on feature/APDS-85 into ** on master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 10f1647 on feature/APDS-85 into ** on master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 10f1647 on feature/APDS-85 into ** on master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 10f1647 on feature/APDS-85 into ** on master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 10f1647 on feature/APDS-85 into ** on master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 10f1647 on feature/APDS-85 into ** on master**.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants