vikunja
/
frontend
Archived
6
39
Fork 49
Code Issues 1 Pull Requests 4 Projects Releases 26 Activity

feat: fix calculation of token invalidation #3077

Merged
konrad merged 1 commits from dpschen/frontend:feature/fix-calculation-of-token-invalidation into main 2023-02-09 21:45:19 +00:00
Conversation 1 Commits 1 Files Changed 1 +6 -3
1 changed files with 6 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
src/composables/useRenewTokenOnFocus.ts
View File

@ -3,7 +3,7 @@ import {useRouter} from 'vue-router'
import {useEventListener} from '@vueuse/core'
import {useAuthStore} from '@/stores/auth'
import {MILLISECONDS_A_HOUR, SECONDS_A_HOUR} from '@/constants/date'
import {MILLISECONDS_A_SECOND, SECONDS_A_HOUR} from '@/constants/date'
const SECONDS_TOKEN_VALID = 60 * SECONDS_A_HOUR
@ -24,11 +24,14 @@ export function useRenewTokenOnFocus() {
return
}
const expiresIn = (userInfo.value !== null ? userInfo.value.exp : 0) - new Date().valueOf() / MILLISECONDS_A_HOUR
const nowInSeconds = new Date().getTime() / MILLISECONDS_A_SECOND
konrad marked this conversation as resolved
dpschen commented 2023-02-08 14:40:14 +00:00
Review
Copy Link

MILLISECONDS_A_HOUR is wrong. It was originally the value 1000 which is MILLISECONDS_A_HOUR.

`MILLISECONDS_A_HOUR` is wrong. It was originally the value `1000` which is `MILLISECONDS_A_HOUR`.
konrad commented 2023-02-08 17:42:37 +00:00
Review
Copy Link

Why do we need that constant anyway if we're only dealing with milliseconds -> seconds here?

Why do we need that constant anyway if we're only dealing with milliseconds -> seconds here?
dpschen commented 2023-02-08 23:24:52 +00:00
Review
Copy Link

1000 wouldn't explain why we divide through it.
The constant has the explanation in its name. but the real reason for me is mostly to be consequent. One should probably not touch the code if one doesn't know the metric 'milli'.

`1000` wouldn't explain why we divide through it. The constant has the explanation in its name. but the real reason for me is mostly to be consequent. One should probably not touch the code if one doesn't know the metric 'milli'.
konrad commented 2023-02-09 21:44:43 +00:00
Review
Copy Link

That makes sense.

That makes sense.
👍 1
const expiresIn = userInfo.value !== null
dpschen marked this conversation as resolved
dpschen commented 2023-02-08 14:44:33 +00:00
Review
Copy Link

I tried to make this part easier to understand.

I tried to make this part easier to understand.
👍 1
? userInfo.value.exp - nowInSeconds
: 0
// If the token expiry is negative, it is already expired and we have no choice but to redirect
// the user to the login page
if (expiresIn < 0) {
if (expiresIn <= 0) {
dpschen marked this conversation as resolved
dpschen commented 2023-02-08 14:44:17 +00:00
Review
Copy Link

I'm unsure about this. Even though very unlikely I understood this as we allow tokens that are only valid another second. Would probably be best to have some buffer here.

I'm unsure about this. Even though very unlikely I understood this as we allow tokens that are only valid another second. Would probably be best to have some buffer here.
konrad commented 2023-02-08 17:43:14 +00:00
Review
Copy Link

I think it does not matter much to have that buffer but it's fine to leave it in.

I think it does not matter much to have that buffer but it's fine to leave it in.
👍 1
await authStore.checkAuth()
await router.push({name: 'user.login'})
return