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

Fixed JWT expired token being logged as unhandled error #5603

Merged
merged 1 commit into from
May 7, 2020

Conversation

NyanKiyoshi
Copy link
Member

Closes #5601.

  • Properly handle valid errors such as JSONWebTokenExpired coming from invalid JWT payload;
  • Allow to toggle JWT verification whether DEBUG is enable or not (useful for testing each mode);
  • Added the default value of JWT_EXPIRATION_DELTA as it allows the users to customize it if needed or at least know how long a token stays valid.

Impact

  • New migrations
  • New/Updated API fields or mutations
  • Deprecated API fields or mutations
  • Removed API types, fields, or mutations

Pull Request Checklist

  • Privileged queries and mutations are guarded by proper permission checks
  • Database queries are optimized and the number of queries is constant
  • Database migration files are up to date
  • The changes are tested
  • GraphQL schema and type definitions are up to date
  • Changes are mentioned in the changelog

@NyanKiyoshi NyanKiyoshi added bug graphql Issues related to the GraphQL API labels May 6, 2020
@NyanKiyoshi NyanKiyoshi self-assigned this May 6, 2020
@db-queries
Copy link

db-queries bot commented May 6, 2020

Here is the report for b84a113 (NyanKiyoshi/saleor @ jwt/handle-expired)
Base comparison is 1043f23.

No differences were found. (click me)

# api.benchmark account
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  delete staff members                                       	         32	         32	              0
  query staff user                                           	         21	         21	              4
  staff create                                               	         24	         24	              5
  staff update groups and permissions                        	         36	         36	              6

# api.benchmark category
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  category view                                              	         18	         18	              1

# api.benchmark checkout mutations
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  add billing address to checkout                            	         52	         52	             26
  add shipping to checkout                                   	         40	         40	             12
  checkout email update                                      	         29	         29	             13
  checkout payment charge                                    	         23	         23	              5
  checkout shipping address update                           	         35	         35	              8
  checkout voucher code                                      	         59	         59	             31
  complete checkout                                          	         74	         74	             19
  create checkout                                            	        140	        140	             75
  update checkout lines                                      	        101	        101	             50

# api.benchmark collection
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  collection view                                            	         17	         17	              0

# api.benchmark homepage
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  featured products list                                     	         14	         14	              0
  retrieve main menu                                         	          8	          8	              0
  retrieve product list                                      	          5	          5	              0
  retrieve secondary menu                                    	          8	          8	              0
  retrieve shop                                              	          2	          2	              0
  user checkout details                                      	         52	         52	             26

# api.benchmark order
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  user order details                                         	         17	         17	              2

# api.benchmark permission group
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  permission group create                                    	         21	         21	              3
  permission group delete                                    	         22	         22	              4
  permission group query                                     	          8	          8	              0
  permission group update                                    	         36	         36	              2
  permission group update remove users with manage staff     	         31	         31	              4

# api.benchmark product
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  product details                                            	         19	         19	              2
  retrieve product attributes                                	          7	          7	              0

# api.benchmark variant
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  product variant bulk create                                	         48	         48	              2
  retrieve variant list                                      	         23	         23	              6

# api.benchmark variant stocks
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  product variants stocks create                             	         23	         23	              5
  product variants stocks delete                             	         20	         20	              5
  product variants stocks update                             	         28	         28	              5

# api product sorting attributes
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  sort product not having attribute data                     	         20	         20	              0

@codecov
Copy link

codecov bot commented May 6, 2020

Codecov Report

Merging #5603 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #5603   +/-   ##
=======================================
  Coverage   91.68%   91.68%           
=======================================
  Files         304      304           
  Lines       19843    19843           
  Branches     1847     1847           
=======================================
  Hits        18194    18194           
  Misses       1215     1215           
  Partials      434      434           
Impacted Files Coverage Δ
saleor/graphql/views.py 85.20% <100.00%> (ø)

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 1043f23...b84a113. Read the comment docs.

saleor/settings.py Outdated Show resolved Hide resolved
@maarcingebala maarcingebala merged commit 2a01638 into saleor:master May 7, 2020
@JrmyDev
Copy link

JrmyDev commented May 7, 2020

Why not add time expiration as an environment parameter ??

@NyanKiyoshi NyanKiyoshi deleted the jwt/handle-expired branch May 7, 2020 09:58
@NyanKiyoshi
Copy link
Member Author

@JrmyDev what do you mean by optional?

@JrmyDev
Copy link

JrmyDev commented May 7, 2020

@NyanKiyoshi : I mean we could set JWT_EXPIRATION_DELTA as an environment variable instead of editing settings.py

@NyanKiyoshi
Copy link
Member Author

I don't really expect users to want to edit that value. Unless there is an actual use case?

@JrmyDev
Copy link

JrmyDev commented May 7, 2020

Isn't this parameter the session duration ??

@NyanKiyoshi
Copy link
Member Author

The time before the token must be refreshed.

@JrmyDev
Copy link

JrmyDev commented May 7, 2020

Ok, sorry i misunderstood...
Is there a parameter for session duration?

@NyanKiyoshi
Copy link
Member Author

No, as long the client refreshes the token, it's all good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug graphql Issues related to the GraphQL API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unhandled exception -- JSONWebTokenExpired: Signature has expired
3 participants