Shayokh144 / SurveyApp

0 stars 0 forks source link

[Improvement] Refresh token logic concerns #11

Closed minhnimble closed 2 years ago

minhnimble commented 2 years ago

Right now I notice that you apply the refresh token logic at each time the application triggers to load the survey list. With this approach, there is an issue that it doesn't depend on when the server returns an authentication expired error (401). Therefore, I am curious on why you choose to implement the refresh token logic like this approach? And how would you solve the issue when user alters the device's time to make the token check always passes but the token is actually expired (server will return 401 issue here) when the user is still active on the current session?

At the same time, if we need to apply the token refresh logic for multiple places in other Modules and for other endpoints later on, this approach will create duplicated token refresh logic code in every Module and endpoint, which affects the scalability in the future. Do you have any solution for this problem? https://github.com/Shayokh144/SurveyApp/blob/c90b970ecda49fcdcc5471f5ad70cf3c055c97c2/SurveyApp/SurveyApp/src/survey/presenter/SurveyPresenter.swift#L36-L50

Shayokh144 commented 2 years ago

You have addressed a great issue. This is the first time I have implemented oauth 2 and didn't think this way. I think to solve this issue I have to rely on authentication expired error (401). Whenever the app calls an api and find the authentication expired error, it should request for refresh token. After getting refresh token and new access token it can call the previous api. Please let me if there is other better approach.

minhnimble commented 2 years ago

Your solution sounds like a good summary of the approach for handling the problem. Just a side note, you should pay close attention to scalability and re-usability of the solution when you actually implement it. 🙏

Shayokh144 commented 2 years ago

The logic is added with a commit