feat: Implement JWT refresh token for authorization (#2230) #2349
Conversation
|
@iamareebjamal do take a look for refreshtokenservice |
|
@iamareebjamal one more thing to update the values of refresh token and access token throughout the application we need to have authholder dependency inside the tokenautheticator class |
| .baseUrl(BuildConfig.DEFAULT_BASE_URL) | ||
| .build() | ||
| val api: AuthApi = retrofit.create(AuthApi::class.java) | ||
| } |
There was a problem hiding this comment.
Make it like other classes with DI, and manually inject
There was a problem hiding this comment.
Manually inject this class into the TokenAutheticator?
There was a problem hiding this comment.
@iamareebjamal take a look and i also couldn't figure out a method to logout user in case if the refresh token is expired
There was a problem hiding this comment.
Oh yes right 😅
aggarwalpulkit596
force-pushed
the
2230
branch
from
1c4eca0 to
67c2390
Compare
|
Travis is failing |
| .build() | ||
| } | ||
| } else { | ||
| response.request |
There was a problem hiding this comment.
Should i delete the token here itself ? This is the case if the existing refresh token is not valid anymore
aggarwalpulkit596
force-pushed
the
2230
branch
3 times, most recently
from
52a6cf2 to
3f25fc2
Compare
aggarwalpulkit596
force-pushed
the
2230
branch
from
3f25fc2 to
71a1d31
Compare
|
there is a failed dependency test @iamareebjamal can you please look |
|
Which means it'll crash on runtime. Run the app and debug |
|
Okay means crashing for a specific case 😅 can you please tell how to manually expire jwt token |
|
You can't. What's the usecase |
|
Then how do i check because refreshtokenservice dependancy is failing |
|
You have to break the circular dependency by manually injecting |
|
@iamareebjamal sorry for the delay will finalize this today |
|
@iamareebjamal i've completed this but is there any way I can manually check whether it is working or not? |
|
Also what it TTL for the jwt token |
|
24 hours 30 minutes if using refresh tokens |
|
@iamareebjamal can I take this up? |
|
Yes |
Fixes #2230