Skip to content

Commit

Permalink
refactor(auth): remove refresh token expiration handling
Browse files Browse the repository at this point in the history
- Remove handling of refresh token expiration as Casdoor never returns
`refresh_expires_in`[^1].
- Treat refresh tokens as always valid as they are renewed with each
access token refresh[^2].
- Return `401 Unauthorized` instead of `500 Internal Server Error` when
JWT parsing fails.

[^1]: https://github.com/casdoor/casdoor/blob/6f1f93725e77c8288aa0ac1c0d996feff906ddcf/object/token_oauth.go#L383-L390
[^2]: https://github.com/casdoor/casdoor/blob/6f1f93725e77c8288aa0ac1c0d996feff906ddcf/object/token_oauth.go#L351-L376

Signed-off-by: Aofei Sheng <aofei@aofeisheng.com>
  • Loading branch information
aofei committed Oct 18, 2024
1 parent 0050905 commit c97ec8f
Show file tree
Hide file tree
Showing 8 changed files with 56 additions and 67 deletions.
2 changes: 1 addition & 1 deletion spx-backend/internal/controller/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func ensureUser(ctx context.Context, expectedUserID int64) (*model.User, error)
func (ctrl *Controller) UserFromToken(ctx context.Context, token string) (*model.User, error) {
claims, err := ctrl.casdoorClient.ParseJwtToken(token)
if err != nil {
return nil, fmt.Errorf("ctrl.casdoorClient.ParseJwtToken failed: %w", err)
return nil, fmt.Errorf("ctrl.casdoorClient.ParseJwtToken failed: %w: %w", ErrUnauthorized, err)
}
mUser, err := model.FirstOrCreateUser(ctx, ctrl.db, claims.Name)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion spx-backend/internal/controller/user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ func TestControllerUserFromToken(t *testing.T) {

_, err := ctrl.UserFromToken(context.Background(), "invalid-token")
require.Error(t, err)
assert.EqualError(t, err, "ctrl.casdoorClient.ParseJwtToken failed: token contains an invalid number of segments")
assert.EqualError(t, err, "ctrl.casdoorClient.ParseJwtToken failed: unauthorized: token contains an invalid number of segments")
})
}

Expand Down
2 changes: 1 addition & 1 deletion spx-gui/src/components/community/user/FollowButton.vue
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const props = defineProps<{
name: string
}>()
const followable = computed(() => props.name !== useUserStore().userInfo?.name)
const followable = computed(() => props.name !== useUserStore().userInfo()?.name)
const following = ref<boolean | null>(null)
watch(
Expand Down
13 changes: 8 additions & 5 deletions spx-gui/src/components/navbar/NavbarProfile.vue
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
<template>
<div v-if="!userStore.userInfo" class="sign-in">
<div v-if="!userInfo" class="sign-in">
<UIButton type="secondary" :disabled="!isOnline" @click="userStore.initiateSignIn()">{{
$t({ en: 'Sign in', zh: '登录' })
}}</UIButton>
</div>
<UIDropdown v-else placement="bottom-end" :offset="{ x: -4, y: 8 }">
<template #trigger>
<div class="avatar">
<img class="avatar-img" :src="userStore.userInfo.avatar" />
<img class="avatar-img" :src="userInfo.avatar" />
</div>
</template>
<UIMenu class="user-menu">
<UIMenuGroup>
<UIMenuItem :interactive="false">
{{ userStore.userInfo.displayName || userStore.userInfo.name }}
{{ userInfo.displayName || userInfo.name }}
</UIMenuItem>
</UIMenuGroup>
<UIMenuGroup>
Expand All @@ -39,17 +39,20 @@ import { useNetwork } from '@/utils/network'
import { getUserPageRoute } from '@/router'
import { useUserStore } from '@/stores'
import { UIButton, UIDropdown, UIMenu, UIMenuGroup, UIMenuItem } from '@/components/ui'
import { computed } from 'vue'
const userStore = useUserStore()
const { isOnline } = useNetwork()
const router = useRouter()
const userInfo = computed(() => userStore.userInfo())
function handleUserPage() {
router.push(getUserPageRoute(userStore.userInfo!.name))
router.push(getUserPageRoute(userInfo.value!.name))
}
function handleProjects() {
router.push(getUserPageRoute(userStore.userInfo!.name, 'projects'))
router.push(getUserPageRoute(userInfo.value!.name, 'projects'))
}
</script>

Expand Down
7 changes: 5 additions & 2 deletions spx-gui/src/components/project/ProjectCreateModal.vue
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ import defaultSpritePng from '@/assets/default-sprite.png'
import defaultBackdropImg from '@/assets/default-backdrop.png'
import { Backdrop } from '@/models/backdrop'
import { Project } from '@/models/project'
import { computed } from 'vue'
const props = defineProps<{
visible: boolean
Expand All @@ -71,6 +72,8 @@ const emit = defineEmits<{
const { t } = useI18n()
const userStore = useUserStore()
const userInfo = computed(() => userStore.userInfo())
const form = useForm({
name: ['', validateName]
})
Expand Down Expand Up @@ -134,8 +137,8 @@ async function validateName(name: string): Promise<FormValidationResult> {
})
// check naming conflict
if (userStore.userInfo == null) throw new Error('login required')
const username = userStore.userInfo.name
if (userInfo.value == null) throw new Error('login required')
const username = userInfo.value.name
const existedProject = await getProject(username, name).catch((e) => {
if (e instanceof ApiException && e.code === ApiExceptionCode.errorNotFound) return null
throw e
Expand Down
16 changes: 9 additions & 7 deletions spx-gui/src/pages/community/home.vue
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<template>
<CenteredWrapper class="main">
<ProjectsSection
v-if="userStore.isSignedIn"
v-if="userStore.isSignedIn()"
context="home"
:link-to="myProjectsRoute"
:query-ret="myProjects"
Expand Down Expand Up @@ -84,7 +84,7 @@
/>
</ProjectsSection>
<ProjectsSection
v-if="userStore.isSignedIn"
v-if="userStore.isSignedIn()"
context="home"
:link-to="followingCreatedRoute"
:query-ret="followingCreatedProjects"
Expand Down Expand Up @@ -128,14 +128,16 @@ const numInRow = 5 // at most 5 projects in a row, depending on the screen width
const userStore = useUserStore()
const userInfo = computed(() => userStore.userInfo())
const myProjectsRoute = computed(() => {
if (userStore.userInfo == null) return ''
return getUserPageRoute(userStore.userInfo.name, 'projects')
if (userInfo.value == null) return ''
return getUserPageRoute(userInfo.value.name, 'projects')
})
const myProjects = useQuery(
async () => {
if (userStore.userInfo == null) return []
if (userInfo.value == null) return []
const { data: projects } = await listProject({
pageIndex: 1,
pageSize: numInRow,
Expand All @@ -162,13 +164,13 @@ const communityRemixingProjects = useQuery(
)
const followingCreatedRoute = computed(() => {
if (userStore.userInfo == null) return ''
if (userInfo.value == null) return ''
return getExploreRoute(ExploreOrder.FollowingCreated)
})
const followingCreatedProjects = useQuery(
async () => {
if (userStore.userInfo == null) return []
if (userInfo.value == null) return []
return exploreProjects({ order: ExploreOrder.FollowingCreated, count: numInRow })
},
{ en: 'Failed to load projects', zh: '加载失败' }
Expand Down
18 changes: 8 additions & 10 deletions spx-gui/src/pages/editor/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,20 @@
<header class="editor-header">
<EditorNavbar :project="project" />
</header>
<main v-if="userStore.userInfo" class="editor-main">
<main v-if="userInfo" class="editor-main">
<UILoading v-if="isLoading" />
<UIError v-else-if="error != null" :retry="refetch">
{{ $t(error.userMessage) }}
</UIError>
<EditorContextProvider
v-else-if="project != null"
:project="project"
:user-info="userStore.userInfo"
>
<EditorContextProvider v-else-if="project != null" :project="project" :user-info="userInfo">
<ProjectEditor />
</EditorContextProvider>
</main>
</section>
</template>

<script setup lang="ts">
import { watchEffect, watch, onMounted, onUnmounted } from 'vue'
import { watchEffect, watch, onMounted, onUnmounted, computed } from 'vue'
import { useRouter } from 'vue-router'
import { useUserStore } from '@/stores'
import { AutoSaveMode, Project } from '@/models/project'
Expand All @@ -45,11 +41,13 @@ const userStore = useUserStore()
watchEffect(() => {
// This will be called on mount and whenever userStore changes,
// which are the cases when userStore.signOut() is called
if (!userStore.isSignedIn) {
if (!userStore.isSignedIn()) {
userStore.initiateSignIn()
}
})
const userInfo = computed(() => userStore.userInfo())
const router = useRouter()
const withConfirm = useConfirmDialog()
Expand Down Expand Up @@ -113,9 +111,9 @@ const {
refetch
} = useQuery(
() => {
// We need to read `userStore.userInfo.name` & `projectName.value` synchronously,
// We need to read `userInfo.value?.name` & `projectName.value` synchronously,
// so their change will drive `useQuery` to re-fetch
return loadProject(userStore.userInfo?.name, props.projectName)
return loadProject(userInfo.value?.name, props.projectName)
},
{ en: 'Failed to load project', zh: '加载项目失败' }
)
Expand Down
63 changes: 23 additions & 40 deletions spx-gui/src/stores/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,8 @@ export interface UserInfo {

interface TokenResponse {
access_token: string
refresh_token: string
expires_in: number
refresh_expires_in: number
refresh_token: string
}

const casdoorAuthRedirectPath = '/callback'
Expand All @@ -28,35 +27,9 @@ const tokenExpiryDelta = 60 * 1000 // 1 minute in milliseconds
export const useUserStore = defineStore('spx-user', {
state: () => ({
accessToken: null as string | null,
refreshToken: null as string | null,

// timestamp in milliseconds, null if never expires
accessTokenExpiresAt: null as number | null,
refreshTokenExpiresAt: null as number | null
accessTokenExpiresAt: null as number | null, // timestamp in milliseconds, null if never expires
refreshToken: null as string | null
}),
getters: {
isAccessTokenValid(): boolean {
return !!(
this.accessToken &&
(this.accessTokenExpiresAt === null ||
this.accessTokenExpiresAt - tokenExpiryDelta > Date.now())
)
},
isRefreshTokenValid(): boolean {
return !!(
this.refreshToken &&
(this.refreshTokenExpiresAt === null ||
this.refreshTokenExpiresAt - tokenExpiryDelta > Date.now())
)
},
isSignedIn(): boolean {
return this.isAccessTokenValid || this.isRefreshTokenValid
},
userInfo(): UserInfo | null {
if (!this.isSignedIn) return null
return jwtDecode<UserInfo>(this.accessToken!)
}
},
actions: {
initiateSignIn(
returnTo: string = window.location.pathname + window.location.search + window.location.hash
Expand All @@ -74,16 +47,15 @@ export const useUserStore = defineStore('spx-user', {
},
signOut() {
this.accessToken = null
this.refreshToken = null
this.accessTokenExpiresAt = null
this.refreshTokenExpiresAt = null
this.refreshToken = null
},
async ensureAccessToken(): Promise<string | null> {
if (this.isAccessTokenValid) return this.accessToken
if (this.isAccessTokenValid()) return this.accessToken

if (this.isRefreshTokenValid) {
if (this.refreshToken != null) {
try {
const resp = await casdoorSdk.refreshAccessToken(this.refreshToken!)
const resp = await casdoorSdk.refreshAccessToken(this.refreshToken)
this.handleTokenResponse(resp)
} catch (e) {
console.error('failed to refresh access token', e)
Expand All @@ -93,19 +65,30 @@ export const useUserStore = defineStore('spx-user', {
// Due to casdoor-js-sdk's lack of error handling, we must check if the access token is valid after calling
// `casdoorSdk.refreshAccessToken`. The token might still be invalid if, e.g., the server has already revoked
// the refresh token.
if (this.isAccessTokenValid) return this.accessToken
if (this.isAccessTokenValid()) return this.accessToken
}

this.signOut()
return null
},
handleTokenResponse(resp: TokenResponse) {
this.accessToken = resp.access_token
this.refreshToken = resp.refresh_token
this.accessTokenExpiresAt = resp.expires_in ? Date.now() + resp.expires_in * 1000 : null
this.refreshTokenExpiresAt = resp.refresh_expires_in
? Date.now() + resp.refresh_expires_in * 1000
: null
this.refreshToken = resp.refresh_token
},
isAccessTokenValid(): boolean {
return !!(
this.accessToken &&
(this.accessTokenExpiresAt === null ||
this.accessTokenExpiresAt - tokenExpiryDelta > Date.now())
)
},
isSignedIn(): boolean {
return this.isAccessTokenValid()
},
userInfo(): UserInfo | null {
if (!this.isSignedIn()) return null
return jwtDecode<UserInfo>(this.accessToken!)
}
},
persist: true
Expand Down

0 comments on commit c97ec8f

Please sign in to comment.