search

uppsaladatavetare / foobar-api

The backend of the FooBar kiosk and inventory system.
MIT License
12 stars 6 forks source link

Added new web token authorization for FooCards #23

Closed flaeppe closed 7 years ago

flaeppe commented 7 years ago

Leave som input on expiration time and encryption algorithm, any opinions are very welcome.

Also if we instead should import its configuration, i.e. set the expiration time and or algorithm "globally" from a settings file or similar.

codecov[bot] commented 7 years ago

Codecov Report

Merging #23 into develop will increase coverage by 1.85%. The diff coverage is 99.38%.

@@             Coverage Diff             @@
##           develop      #23      +/-   ##
===========================================
+ Coverage    86.88%   88.73%   +1.85%     
===========================================
  Files           71       76       +5     
  Lines         2218     2565     +347     
  Branches       115      131      +16     
===========================================
+ Hits          1927     2276     +349     
+ Misses         275      274       -1     
+ Partials        16       15       -1
Impacted Files Coverage Δ
src/authtoken/views.py 100% <100%> (ø)
src/authtoken/test.py 100% <100%> (ø)
src/foobar/settings/base.py 100% <100%> (ø) :white_check_mark:
src/foobar/tests/factories.py 100% <100%> (ø) :white_check_mark:
src/authtoken/serializers.py 100% <100%> (ø)
src/authtoken/authentication.py 85.24% <97.36%> (+31.91%) :white_check_mark:
src/wallet/api.py 100% <ø> (ø) :white_check_mark:
src/foobar/wallet/api.py 100% <ø> (ø) :white_check_mark:
src/foobar/tests/test_api.py 100% <ø> (ø) :white_check_mark:
src/wallet/tests/test_api.py 100% <ø> (ø) :white_check_mark:
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a663d96...752be01. Read the comment docs.

kjagiello commented 7 years ago

I want to propose that we extend the scope of this PR and aim to implement all the other things that are needed in order to be able to start using the card tokens. I think that the next step should be reorganisation of the permissions. Currently there are no permissions that can be assigned to a card token, making them completely unusable. I think this is a good chance to rethink the entire permission system and implement something more flexible. I'm open for suggestions here.

flaeppe commented 7 years ago

Maybe we should change the destination of this merge into a new feature branch then?

If the task list grows a bit more, maybe it's relevant for more people to work on it?

flaeppe commented 7 years ago

However, there is at least one more thing left before we should do a merge.

kjagiello commented 7 years ago

Sounds like a plan!