From 59a0b9c40d1422c1595572cc2c01550a0c4cc64a Mon Sep 17 00:00:00 2001
From: kolaente <k@knt.li>
Date: Sat, 14 Jun 2025 15:12:41 +0200
Subject: [PATCH] feat(auth): require auth to fetch avatars (#930)

---
 .vscode/settings.json                         |  2 +-
 .../cypress/e2e/sharing/linkShare.spec.ts     |  2 +
 frontend/src/components/misc/User.vue         | 13 +++++--
 .../components/tasks/partials/Comments.vue    | 30 ++++++++++++---
 frontend/src/models/user.ts                   | 37 ++++++++++++++++++-
 frontend/src/services/abstractService.ts      | 11 ++++++
 frontend/src/stores/auth.ts                   | 24 +++++-------
 pkg/models/link_sharing.go                    |  8 ++--
 pkg/models/link_sharing_test.go               | 33 +++++++++++++++++
 pkg/models/task_collection_test.go            |  9 +++--
 pkg/routes/routes.go                          |  6 +--
 pkg/user/error.go                             | 23 ++++++++++++
 pkg/user/user_create.go                       |  9 +++++
 pkg/user/user_test.go                         | 26 +++++++++++++
 pkg/user/validator.go                         |  8 +++-
 15 files changed, 204 insertions(+), 37 deletions(-)

diff --git a/.vscode/settings.json b/.vscode/settings.json
index 8a613b1b2..bc43bd7b3 100644
--- a/.vscode/settings.json
+++ b/.vscode/settings.json
@@ -8,7 +8,7 @@
     },
     "eslint.format.enable": true,
     "[javascript]": {
-        "editor.defaultFormatter": "dbaeumer.vscode-eslint"
+        "editor.defaultFormatter": "vscode.typescript-language-features"
     },
     "[typescript]": {
         "editor.defaultFormatter": "vscode.typescript-language-features"
diff --git a/frontend/cypress/e2e/sharing/linkShare.spec.ts b/frontend/cypress/e2e/sharing/linkShare.spec.ts
index 12bb9ed28..37b5b8cc3 100644
--- a/frontend/cypress/e2e/sharing/linkShare.spec.ts
+++ b/frontend/cypress/e2e/sharing/linkShare.spec.ts
@@ -1,8 +1,10 @@
 import {LinkShareFactory} from '../../factories/link_sharing'
 import {TaskFactory} from '../../factories/task'
+import {UserFactory} from '../../factories/user'
 import {createProjects} from '../project/prepareProjects'
 
 function prepareLinkShare() {
+	UserFactory.create()
 	const projects = createProjects()
 	const tasks = TaskFactory.create(10, {
 		project_id: projects[0].id,
diff --git a/frontend/src/components/misc/User.vue b/frontend/src/components/misc/User.vue
index 4f985fbfc..9dab04df8 100644
--- a/frontend/src/components/misc/User.vue
+++ b/frontend/src/components/misc/User.vue
@@ -6,7 +6,7 @@
 		<img
 			v-tooltip="displayName"
 			:height="avatarSize"
-			:src="getAvatarUrl(user, avatarSize)"
+			:src="avatarSrc"
 			:width="avatarSize"
 			:alt="'Avatar of ' + displayName"
 			class="avatar"
@@ -19,9 +19,9 @@
 </template>
 
 <script lang="ts" setup>
-import {computed} from 'vue'
+import {computed, ref, watch} from 'vue'
 
-import {getAvatarUrl, getDisplayName} from '@/models/user'
+import {fetchAvatarBlobUrl, getDisplayName} from '@/models/user'
 import type {IUser} from '@/modelTypes/IUser'
 
 const props = withDefaults(defineProps<{
@@ -36,6 +36,13 @@ const props = withDefaults(defineProps<{
 })
 
 const displayName = computed(() => getDisplayName(props.user))
+const avatarSrc = ref('')
+
+async function loadAvatar() {
+	avatarSrc.value = await fetchAvatarBlobUrl(props.user, props.avatarSize)
+}
+
+watch(() => [props.user, props.avatarSize], loadAvatar, { immediate: true })
 </script>
 
 <style lang="scss" scoped>
diff --git a/frontend/src/components/tasks/partials/Comments.vue b/frontend/src/components/tasks/partials/Comments.vue
index 116c7e5d8..b49514093 100644
--- a/frontend/src/components/tasks/partials/Comments.vue
+++ b/frontend/src/components/tasks/partials/Comments.vue
@@ -29,7 +29,7 @@
 			>
 				<figure class="media-left is-hidden-mobile">
 					<img
-						:src="getAvatarUrl(c.author, 48)"
+						:src="avatarFor(c.author, 48)"
 						alt=""
 						class="image is-avatar"
 						height="48"
@@ -42,7 +42,7 @@
 				<div class="media-content">
 					<div class="comment-info">
 						<img
-							:src="getAvatarUrl(c.author, 20)"
+							:src="avatarFor(c.author, 20)"
 							alt=""
 							class="image is-avatar d-print-none"
 							height="20"
@@ -222,11 +222,12 @@ import type {ITask} from '@/modelTypes/ITask'
 import {uploadFile} from '@/helpers/attachments'
 import {success} from '@/message'
 import {formatDateLong, formatDateSince} from '@/helpers/time/formatDate'
-import {getAvatarUrl, getDisplayName} from '@/models/user'
+import {fetchAvatarBlobUrl, getDisplayName} from '@/models/user'
+import type {IUser} from '@/modelTypes/IUser'
 import {useConfigStore} from '@/stores/config'
 import {useAuthStore} from '@/stores/auth'
 import Reactions from '@/components/input/Reactions.vue'
-import { useCopyToClipboard } from '@/composables/useCopyToClipboard'
+import {useCopyToClipboard} from '@/composables/useCopyToClipboard'
 
 const props = withDefaults(defineProps<{
 	taskId: number,
@@ -255,7 +256,26 @@ const newCommentText = ref('')
 const saved = ref<ITask['id'] | null>(null)
 const saving = ref<ITask['id'] | null>(null)
 
-const userAvatar = computed(() => getAvatarUrl(authStore.info, 48))
+const userAvatar = ref('')
+const avatarCache = reactive(new Map<string, string>())
+
+function avatarFor(u: IUser, size: number) {
+	const key = `${u.id}-${size}`
+	const cached = avatarCache.get(key)
+	if (!cached) {
+		fetchAvatarBlobUrl(u, size).then(url => avatarCache.set(key, url))
+	}
+
+	return avatarCache.get(key) || ''
+}
+
+watch(() => authStore.info, async (nu) => {
+	if (!nu) {
+		return
+	}
+	userAvatar.value = await fetchAvatarBlobUrl(nu, 48)
+}, {immediate: true})
+
 const currentUserId = computed(() => authStore.info.id)
 const enabled = computed(() => configStore.taskCommentsEnabled)
 const actions = computed(() => {
diff --git a/frontend/src/models/user.ts b/frontend/src/models/user.ts
index f6398d55f..8fb418b15 100644
--- a/frontend/src/models/user.ts
+++ b/frontend/src/models/user.ts
@@ -3,9 +3,42 @@ import UserSettingsModel from '@/models/userSettings'
 
 import { AUTH_TYPES, type IUser, type AuthType } from '@/modelTypes/IUser'
 import type { IUserSettings } from '@/modelTypes/IUserSettings'
+import AvatarService from '@/services/avatar'
 
-export function getAvatarUrl(user: IUser, size = 50) {
-	return `${window.API_URL}/avatar/${user.username}?size=${size}`
+const avatarService = new AvatarService()
+const avatarCache = new Map<string, string>()
+const pendingRequests = new Map<string, Promise<string>>()
+
+export async function fetchAvatarBlobUrl(user: IUser, size = 50) {
+	if (!user || !user.username) {
+		return ''
+	}
+	const key = `${user.username}-${size}`
+	
+	// Return cached URL if available
+	if (avatarCache.has(key)) {
+		return avatarCache.get(key) as string
+	}
+	
+	// If there's already a pending request for this avatar, wait for it
+	if (pendingRequests.has(key)) {
+		return await pendingRequests.get(key) as string
+	}
+	
+	// Create a new request
+	const requestPromise = avatarService.getBlobUrl(`/avatar/${user.username}?size=${size}`)
+		.then(url => {
+			avatarCache.set(key, url)
+			pendingRequests.delete(key)
+			return url
+		})
+		.catch(error => {
+			pendingRequests.delete(key)
+			throw error
+		})
+	
+	pendingRequests.set(key, requestPromise)
+	return await requestPromise
 }
 
 export function getDisplayName(user: IUser) {
diff --git a/frontend/src/services/abstractService.ts b/frontend/src/services/abstractService.ts
index 5881e3a72..391305b82 100644
--- a/frontend/src/services/abstractService.ts
+++ b/frontend/src/services/abstractService.ts
@@ -324,6 +324,17 @@ export default abstract class AbstractService<Model extends IAbstract = IAbstrac
 			responseType: 'blob',
 			data,
 		})
+		
+		// Handle SVG blobs specially - convert to data URL for better browser compatibility
+		if (response.data.type === 'image/svg+xml') {
+			return new Promise((resolve, reject) => {
+				const reader = new FileReader()
+				reader.onload = () => resolve(reader.result as string)
+				reader.onerror = reject
+				reader.readAsDataURL(response.data)
+			})
+		}
+		
 		return window.URL.createObjectURL(new Blob([response.data]))
 	}
 
diff --git a/frontend/src/stores/auth.ts b/frontend/src/stores/auth.ts
index adea233b0..c423859e0 100644
--- a/frontend/src/stores/auth.ts
+++ b/frontend/src/stores/auth.ts
@@ -4,7 +4,7 @@ import {acceptHMRUpdate, defineStore} from 'pinia'
 import {AuthenticatedHTTPFactory, HTTPFactory} from '@/helpers/fetcher'
 import {getBrowserLanguage, i18n, setLanguage} from '@/i18n'
 import {objectToSnakeCase} from '@/helpers/case'
-import UserModel, {getAvatarUrl, getDisplayName} from '@/models/user'
+import UserModel, {getDisplayName, fetchAvatarBlobUrl} from '@/models/user'
 import UserSettingsService from '@/services/userSettings'
 import {getToken, refreshToken, removeToken, saveToken} from '@/helpers/auth'
 import {setModuleLoading} from '@/stores/helper'
@@ -66,7 +66,6 @@ export const useAuthStore = defineStore('auth', () => {
 	const configStore = useConfigStore()
 	
 	const authenticated = ref(false)
-	const isLinkShareAuth = ref(false)
 	const needsTotpPasscode = ref(false)
 	
 	const info = ref<IUser | null>(null)
@@ -92,7 +91,8 @@ export const useAuthStore = defineStore('auth', () => {
 	})
 
 	const userDisplayName = computed(() => info.value ? getDisplayName(info.value) : undefined)
-
+	
+	const isLinkShareAuth = computed(() => info.value?.type === AUTH_TYPES.LINK_SHARE)
 
 	function setIsLoading(newIsLoading: boolean) {
 		isLoading.value = newIsLoading 
@@ -104,14 +104,12 @@ export const useAuthStore = defineStore('auth', () => {
 
 	function setUser(newUser: IUser | null, saveSettings = true) {
 		info.value = newUser
-		if (newUser !== null) {
+		if (newUser !== null && !isLinkShareAuth.value) {
 			reloadAvatar()
 
 			if (saveSettings && newUser.settings) {
 				loadSettings(newUser.settings)
 			}
-
-			isLinkShareAuth.value = newUser.id < 0
 		}
 	}
 
@@ -141,17 +139,16 @@ export const useAuthStore = defineStore('auth', () => {
 		authenticated.value = newAuthenticated
 	}
 
-	function setIsLinkShareAuth(newIsLinkShareAuth: boolean) {
-		isLinkShareAuth.value = newIsLinkShareAuth
-	}
 
 	function setNeedsTotpPasscode(newNeedsTotpPasscode: boolean) {
 		needsTotpPasscode.value = newNeedsTotpPasscode
 	}
 
-	function reloadAvatar() {
-		if (!info.value) return
-		avatarUrl.value = `${getAvatarUrl(info.value)}&=${new Date().valueOf()}`
+	async function reloadAvatar() {
+		if (!info.value || !info.value.username) {
+			return
+		}
+		avatarUrl.value = await fetchAvatarBlobUrl(info.value, 40)
 	}
 
 	function updateLastUserRefresh() {
@@ -442,7 +439,6 @@ export const useAuthStore = defineStore('auth', () => {
 	return {
 		// state
 		authenticated: readonly(authenticated),
-		isLinkShareAuth: readonly(isLinkShareAuth),
 		needsTotpPasscode: readonly(needsTotpPasscode),
 
 		info: readonly(info),
@@ -454,6 +450,7 @@ export const useAuthStore = defineStore('auth', () => {
 		authUser,
 		authLinkShare,
 		userDisplayName,
+		isLinkShareAuth,
 
 		isLoading: readonly(isLoading),
 		setIsLoading,
@@ -464,7 +461,6 @@ export const useAuthStore = defineStore('auth', () => {
 		setUser,
 		setUserSettings,
 		setAuthenticated,
-		setIsLinkShareAuth,
 		setNeedsTotpPasscode,
 
 		reloadAvatar,
diff --git a/pkg/models/link_sharing.go b/pkg/models/link_sharing.go
index f43da932f..796bd715e 100644
--- a/pkg/models/link_sharing.go
+++ b/pkg/models/link_sharing.go
@@ -18,14 +18,14 @@ package models
 
 import (
 	"errors"
+	"strconv"
 	"time"
 
 	"code.vikunja.io/api/pkg/db"
-
 	"code.vikunja.io/api/pkg/user"
 	"code.vikunja.io/api/pkg/utils"
-
 	"code.vikunja.io/api/pkg/web"
+
 	"github.com/golang-jwt/jwt/v5"
 	"golang.org/x/crypto/bcrypt"
 	"xorm.io/builder"
@@ -110,10 +110,12 @@ func (share *LinkSharing) toUser() *user.User {
 		suffix = " (" + suffix + ")"
 	}
 
+	username := "link-share-" + strconv.FormatInt(share.ID, 10)
+
 	return &user.User{
 		ID:       share.getUserID(),
 		Name:     share.Name + suffix,
-		Username: share.Name,
+		Username: username,
 		Created:  share.Created,
 		Updated:  share.Updated,
 	}
diff --git a/pkg/models/link_sharing_test.go b/pkg/models/link_sharing_test.go
index a62b350e8..2804a4e0a 100644
--- a/pkg/models/link_sharing_test.go
+++ b/pkg/models/link_sharing_test.go
@@ -18,6 +18,7 @@ package models
 
 import (
 	"testing"
+	"time"
 
 	"code.vikunja.io/api/pkg/db"
 	"code.vikunja.io/api/pkg/user"
@@ -155,3 +156,35 @@ func TestLinkSharing_ReadOne(t *testing.T) {
 		assert.Empty(t, share.Password)
 	})
 }
+
+func TestLinkSharing_toUser(t *testing.T) {
+	t.Run("empty name", func(t *testing.T) {
+		share := &LinkSharing{
+			ID:      1,
+			Name:    "",
+			Created: time.Now(),
+			Updated: time.Now(),
+		}
+
+		user := share.toUser()
+
+		assert.Equal(t, "link-share-1", user.Username)
+		assert.Equal(t, "Link Share", user.Name)
+		assert.Equal(t, int64(-1), user.ID)
+	})
+
+	t.Run("name provided", func(t *testing.T) {
+		share := &LinkSharing{
+			ID:      2,
+			Name:    "My Test Share",
+			Created: time.Now(),
+			Updated: time.Now(),
+		}
+
+		user := share.toUser()
+
+		assert.Equal(t, "link-share-2", user.Username)
+		assert.Equal(t, "My Test Share (Link Share)", user.Name)
+		assert.Equal(t, int64(-2), user.ID)
+	})
+}
diff --git a/pkg/models/task_collection_test.go b/pkg/models/task_collection_test.go
index ba5585feb..5f2ecc4d8 100644
--- a/pkg/models/task_collection_test.go
+++ b/pkg/models/task_collection_test.go
@@ -69,10 +69,11 @@ func TestTaskCollection_ReadAll(t *testing.T) {
 		Updated:                      testUpdatedTime,
 	}
 	linkShareUser2 := &user.User{
-		ID:      -2,
-		Name:    "Link Share",
-		Created: testCreatedTime,
-		Updated: testUpdatedTime,
+		ID:       -2,
+		Name:     "Link Share",
+		Username: "link-share-2",
+		Created:  testCreatedTime,
+		Updated:  testUpdatedTime,
 	}
 
 	loc := config.GetTimeZone()
diff --git a/pkg/routes/routes.go b/pkg/routes/routes.go
index ea2ae1440..da802bad9 100644
--- a/pkg/routes/routes.go
+++ b/pkg/routes/routes.go
@@ -269,9 +269,6 @@ func registerAPIRoutes(a *echo.Group) {
 	// Info endpoint
 	n.GET("/info", apiv1.Info)
 
-	// Avatar endpoint
-	n.GET("/avatar/:username", apiv1.GetAvatar)
-
 	// Link share auth
 	if config.ServiceEnableLinkSharing.GetBool() {
 		ur.POST("/shares/:share/auth", apiv1.AuthenticateLinkShare)
@@ -290,6 +287,9 @@ func registerAPIRoutes(a *echo.Group) {
 	a.POST("/token/test", apiv1.CheckToken)
 	a.GET("/routes", models.GetAvailableAPIRoutesForToken)
 
+	// Avatar endpoint
+	a.GET("/avatar/:username", apiv1.GetAvatar)
+
 	// User stuff
 	u := a.Group("/user")
 
diff --git a/pkg/user/error.go b/pkg/user/error.go
index 42174a7c5..707c51fe9 100644
--- a/pkg/user/error.go
+++ b/pkg/user/error.go
@@ -609,3 +609,26 @@ const ErrorCodeInvalidTimezone = 1025
 func (err ErrInvalidTimezone) HTTPError() web.HTTPError {
 	return web.HTTPError{HTTPCode: http.StatusBadRequest, Code: ErrorCodeInvalidTimezone, Message: fmt.Sprintf("The timezone '%s' is invalid. Please select a valid timezone from the list.", err.Name)}
 }
+
+// ErrUsernameReserved represents a "UsernameReserved" kind of error.
+type ErrUsernameReserved struct {
+	Username string
+}
+
+// IsErrUsernameReserved checks if an error is a ErrUsernameReserved.
+func IsErrUsernameReserved(err error) bool {
+	_, ok := err.(ErrUsernameReserved)
+	return ok
+}
+
+func (err ErrUsernameReserved) Error() string {
+	return fmt.Sprintf("Username is reserved [Username: %s]", err.Username)
+}
+
+// ErrorCodeUsernameReserved holds the unique world-error code of this error
+const ErrorCodeUsernameReserved = 1026
+
+// HTTPError holds the http error description
+func (err ErrUsernameReserved) HTTPError() web.HTTPError {
+	return web.HTTPError{HTTPCode: http.StatusBadRequest, Code: ErrorCodeUsernameReserved, Message: "This username is reserved and cannot be used."}
+}
diff --git a/pkg/user/user_create.go b/pkg/user/user_create.go
index ddc9e8a4e..d25e0e975 100644
--- a/pkg/user/user_create.go
+++ b/pkg/user/user_create.go
@@ -17,6 +17,7 @@
 package user
 
 import (
+	"regexp"
 	"strings"
 
 	"code.vikunja.io/api/pkg/config"
@@ -143,6 +144,14 @@ func checkIfUserIsValid(user *User) error {
 		}
 	}
 
+	// Check if username matches the reserved link-share pattern
+	linkSharePattern := regexp.MustCompile(`^link-share-\d+$`)
+	if linkSharePattern.MatchString(user.Username) {
+		return ErrUsernameReserved{
+			Username: user.Username,
+		}
+	}
+
 	return nil
 }
 
diff --git a/pkg/user/user_test.go b/pkg/user/user_test.go
index 7ccaffe9d..e4661a28d 100644
--- a/pkg/user/user_test.go
+++ b/pkg/user/user_test.go
@@ -147,6 +147,32 @@ func TestCreateUser(t *testing.T) {
 		require.Error(t, err)
 		assert.True(t, IsErrUsernameMustNotContainSpaces(err))
 	})
+	t.Run("reserved link-share username", func(t *testing.T) {
+		db.LoadAndAssertFixtures(t)
+		s := db.NewSession()
+		defer s.Close()
+
+		_, err := CreateUser(s, &User{
+			Username: "link-share-123",
+			Password: "12345678",
+			Email:    "user2@example.com",
+		})
+		require.Error(t, err)
+		assert.True(t, IsErrUsernameReserved(err))
+	})
+	t.Run("reserved link-share username with single digit", func(t *testing.T) {
+		db.LoadAndAssertFixtures(t)
+		s := db.NewSession()
+		defer s.Close()
+
+		_, err := CreateUser(s, &User{
+			Username: "link-share-1",
+			Password: "12345678",
+			Email:    "user3@example.com",
+		})
+		require.Error(t, err)
+		assert.True(t, IsErrUsernameReserved(err))
+	})
 }
 
 func TestGetUser(t *testing.T) {
diff --git a/pkg/user/validator.go b/pkg/user/validator.go
index 508b95c44..67aaedae5 100644
--- a/pkg/user/validator.go
+++ b/pkg/user/validator.go
@@ -17,6 +17,7 @@
 package user
 
 import (
+	"regexp"
 	"strings"
 
 	"code.vikunja.io/api/pkg/i18n"
@@ -26,10 +27,11 @@ import (
 
 func init() {
 	govalidator.TagMap["username"] = func(i string) bool {
-		// To avoid making this overly complicated, we only check three things:
+		// To avoid making this overly complicated, we only check a few things:
 		// 1. No Spaces
 		// 2. Should not look like an url
 		// 3. Should not contain , (because then it will be impossible to search for)
+		// 4. Should not start with link-share-[NUMBER] (reserved for link sharing system)
 		if govalidator.HasWhitespace(i) {
 			return false
 		}
@@ -42,7 +44,9 @@ func init() {
 			return false
 		}
 
-		return true
+		// Check if username matches the reserved link-share pattern
+		linkSharePattern := regexp.MustCompile(`^link-share-\d+$`)
+		return !linkSharePattern.MatchString(i)
 	}
 
 	govalidator.TagMap["bcrypt_password"] = func(str string) bool {