fix/guest user dialog

cypress/e2e/share.spec.js

@@ -116,22 +116,16 @@ describe('Open test.md in viewer', function () {
 			})
 			.then(() => {
 				cy.getEditor().should('be.visible')
-				cy.getContent()
-					.should('contain', 'Hello world')
-					.find('h2')
-					.should('contain', 'Hello world')
-
-				cy.intercept({
-					method: 'POST',
-					url: '**/apps/text/public/session/*/session',
-				}).as('updateSession')
+				cy.getContent().find('h2').should('contain', 'Hello world')
 				cy.get('button.avatar-list').click()
+				cy.get('.session-menu button').click()
 				cy.get('.guest-name-dialog input[type="text"]').type(
 					'someone{enter}',
 				)
-				cy.wait('@updateSession')
-					.its('response.body.guestName')
-					.should('eq', 'someone')
+				cy.get('.session-menu .session-label.guest').should(
+					'contain',
+					'someone',
+				)
 			})
 	})
 

lib/Service/SessionService.php

@@ -71,10 +71,9 @@ public function __construct(
 	public function initSession(int $documentId, ?string $guestName = null): Session {
 		$session = new Session();
 		$session->setDocumentId($documentId);
-		$userName = $this->userId ?? $guestName;
 		$session->setUserId($this->userId);
 		$session->setToken($this->secureRandom->generate(64));
-		$session->setColor($this->getColorForGuestName($guestName ?? ''));
+		$session->setColor($this->getColor());
 		if ($this->userId === null) {
 			$session->setGuestName($guestName ?? '');
 		}
@@ -223,9 +222,19 @@ public function updateSessionAwareness(Session $session, string $message): Sessi
 		return $this->sessionMapper->update($session);
 	}
 
+	private function getColor(string $guestName = ''): string {
+		if ($this->userId === null) {
+			return $this->getColorForGuestName($guestName);
+		}
+		$name = $this->userManager->getDisplayName($this->userId) ?? $this->userId;
+		$color = $this->avatarManager->getAvatar($this->userId)->avatarBackgroundColor($name);
+		return $color->name();
+	}
+
 	private function getColorForGuestName(string $guestName = ''): string {
-		$guestName = $this->userId !== null ? $guestName : '';
-		$uniqueGuestId = !empty($guestName) ? $guestName : $this->secureRandom->generate(12);
+		$uniqueGuestId = !empty($guestName)
+			? $guestName . '(guest)' // make it harder to impersonate users.
+			: $this->secureRandom->generate(12);
 		$color = $this->avatarManager->getGuestAvatar($uniqueGuestId)->avatarBackgroundColor($uniqueGuestId);
 		return $color->name();
 	}

src/apis/connect.ts

@@ -6,7 +6,7 @@
 import axios from '@nextcloud/axios'
 import { generateUrl } from '@nextcloud/router'
 import type { Connection } from '../composables/useConnection.js'
-import type { Document, Session } from '../services/SyncService.js'
+import type { Document, GuestSession, Session } from '../services/SyncService.js'
 
 export interface OpenParams {
 	fileId?: number
@@ -58,7 +58,7 @@ export async function open(
 export async function update(
 	guestName: string,
 	connection: Connection,
-): Promise<Session> {
+): Promise<GuestSession> {
 	if (!connection.shareToken) {
 		throw new Error('Cannot set guest name without a share token!')
 	}

src/components/Editor.vue

@@ -31,7 +31,6 @@
 							<Status
 								:document="document"
 								:dirty="dirty"
-								:sessions="filteredSessions"
 								:sync-error="syncError"
 								:has-connection-issue="requireReconnect" />
 						</ReadonlyBar>
@@ -48,7 +47,6 @@
 						<Status
 							:document="document"
 							:dirty="dirty"
-							:sessions="filteredSessions"
 							:sync-error="syncError"
 							:has-connection-issue="requireReconnect" />
 						<slot name="header" />
@@ -85,7 +83,7 @@ import { emit, subscribe, unsubscribe } from '@nextcloud/event-bus'
 import { File } from '@nextcloud/files'
 import { Collaboration } from '@tiptap/extension-collaboration'
 import { useElementSize } from '@vueuse/core'
-import Vue, { defineComponent, ref, set, shallowRef, watch } from 'vue'
+import { defineComponent, ref, shallowRef, watch } from 'vue'
 import { Doc } from 'yjs'
 import Autofocus from '../extensions/Autofocus.js'
 
@@ -94,7 +92,6 @@ import { provideEditorFlags } from '../composables/useEditorFlags.ts'
 import { ATTACHMENT_RESOLVER, FILE, IS_MOBILE } from './Editor.provider.ts'
 import ReadonlyBar from './Menu/ReadonlyBar.vue'
 
-import { t } from '@nextcloud/l10n'
 import { generateRemoteUrl } from '@nextcloud/router'
 import { Awareness } from 'y-protocols/awareness.js'
 import { provideConnection } from '../composables/useConnection.ts'
@@ -177,7 +174,6 @@ export default defineComponent({
 	props: {
 		richWorkspace: {
 			type: Boolean,
-			require: false,
 			default: false,
 		},
 		initialSession: {
@@ -258,7 +254,7 @@ export default defineComponent({
 		const { applyEditorWidth } = provideEditorWidth()
 		applyEditorWidth()
 
-		const { setEditable } = useEditorMethods(editor)
+		const { setEditable, updateUser } = useEditorMethods(editor)
 
 		const serialize = isRichEditor
 			? () =>
@@ -278,6 +274,7 @@ export default defineComponent({
 
 		return {
 			awareness,
+			connection,
 			editor,
 			el,
 			hasConnectionIssue,
@@ -292,8 +289,8 @@ export default defineComponent({
 			setEditable,
 			syncProvider,
 			syncService,
+			updateUser,
 			width,
-			connection,
 			ydoc,
 		}
 	},
@@ -303,12 +300,8 @@ export default defineComponent({
 			IDLE_TIMEOUT,
 
 			document: null,
-			sessions: [],
-			currentSession: null,
 			fileNode: null,
 
-			filteredSessions: {},
-
 			idle: false,
 			dirty: false,
 			contentLoaded: false,
@@ -342,7 +335,7 @@ export default defineComponent({
 				: '/'
 		},
 		displayed() {
-			return (this.currentSession && this.active) || this.syncError
+			return (this.connection && this.active) || this.syncError
 		},
 		showLoadingSkeleton() {
 			return (!this.contentLoaded || !this.displayed) && !this.syncError
@@ -493,72 +486,24 @@ export default defineComponent({
 			this.idle = false
 		},
 
-		updateSessions(sessions) {
-			this.sessions = sessions.sort((a, b) => b.lastContact - a.lastContact)
-
-			// Make sure we get our own session updated
-			// This should ideally be part of a global store where we can have that updated on the actual name change for guests
-			const currentUpdatedSession = this.sessions.find(
-				(session) => session.id === this.currentSession.id,
-			)
-			set(this, 'currentSession', currentUpdatedSession)
-
-			const currentSessionIds = this.sessions.map((session) => session.userId)
-			const currentGuestIds = this.sessions.map((session) => session.guestId)
-
-			const removedSessions = Object.keys(this.filteredSessions).filter(
-				(sessionId) =>
-					!currentSessionIds.includes(sessionId)
-					&& !currentGuestIds.includes(sessionId),
-			)
-
-			for (const index in removedSessions) {
-				Vue.delete(this.filteredSessions, removedSessions[index])
-			}
-			for (const index in this.sessions) {
-				const session = this.sessions[index]
-				const sessionKey = session.displayName ? session.userId : session.id
-				if (this.filteredSessions[sessionKey]) {
-					// update timestamp if relevant
-					if (
-						this.filteredSessions[sessionKey].lastContact
-						< session.lastContact
-					) {
-						set(
-							this.filteredSessions[sessionKey],
-							'lastContact',
-							session.lastContact,
-						)
-					}
-				} else {
-					set(this.filteredSessions, sessionKey, session)
-				}
-				if (session.id === this.currentSession.id) {
-					set(this.filteredSessions[sessionKey], 'isCurrent', true)
-				}
-			}
-		},
-
 		onOpened({ document, session, content, documentState, readOnly }) {
-			this.currentSession = session
 			this.document = document
 			this.readOnly = readOnly
 			this.editMode = !readOnly && !this.openReadOnlyEnabled
 			this.hasConnectionIssue = false
 
 			this.setEditable(this.editMode)
-			localStorage.setItem('nick', this.currentSession.guestName)
 			this.$attachmentResolver = new AttachmentResolver({
-				session: this.currentSession,
+				session,
 				user: getCurrentUser(),
 				shareToken: this.shareToken,
 				currentDirectory: this.currentDirectory,
 			})
-			if (this.currentSession?.userId && this.relativePath?.length) {
+			if (session.userId && this.relativePath?.length) {
 				const node = new File({
 					id: this.fileId,
 					source: generateRemoteUrl(
-						`dav/files/${this.currentSession.userId}${this.relativePath}`,
+						`dav/files/${session.userId}${this.relativePath}`,
 					),
 					mime: this.mime,
 				})
@@ -577,18 +522,10 @@ export default defineComponent({
 					})
 				}
 			})
-			const user = {
-				name: session?.userId
-					? session.displayName
-					: session?.guestName || t('text', 'Guest'),
-				color: session?.color,
-				clientId: this.ydoc.clientID,
-			}
-			this.editor.commands.updateUser(user)
+			this.updateUser(session)
 		},
 
-		onChange({ document, sessions }) {
-			this.updateSessions.bind(this)(sessions)
+		onChange({ document }) {
 			this.document = document
 
 			this.syncError = null

src/components/Editor/AvatarWrapper.vue

@@ -7,51 +7,48 @@
 	<div class="avatar-wrapper" :style="sessionAvatarStyle">
 		<NcAvatar
 			v-if="session.userId"
-			:user="session.userId ? session.userId : session.guestName"
-			:is-guest="session.userId === null"
+			:user="session.userId"
+			:is-guest="false"
 			:disable-menu="true"
 			hide-status
-			:disable-tooltip="true"
-			:size="size" />
+			:disable-tooltip="true" />
 		<div v-else class="avatar" :style="sessionBackgroundStyle">
-			{{ guestInitial }}
+			<template v-if="session.guestName">
+				{{ guestInitial }}
+			</template>
+			<AccountOutlineIcon v-else />
 		</div>
 	</div>
 </template>
 
 <script>
 import NcAvatar from '@nextcloud/vue/components/NcAvatar'
+import AccountOutlineIcon from 'vue-material-design-icons/AccountOutline.vue'
+
 export default {
 	name: 'AvatarWrapper',
 	components: {
 		NcAvatar,
+		AccountOutlineIcon,
 	},
 	props: {
 		session: {
 			type: Object,
 			required: true,
 		},
-		size: {
-			type: Number,
-			default: () => 32,
-		},
 	},
 	computed: {
 		sessionAvatarStyle() {
 			return {
 				...this.sessionBackgroundStyle,
 				'border-color': this.session.color,
-				'border-width': '2px',
-				'border-style': 'solid',
-				'--size': this.size + 'px',
-				'--font-size': this.size / 2 + 'px',
 			}
 		},
 		sessionBackgroundStyle() {
 			return {
 				'background-color': this.session.userId
 					? this.session.color + ' !important'
-					: '#b9b9b9',
+					: 'var(--color-background-dark)',
 			}
 		},
 		guestInitial() {
@@ -64,18 +61,26 @@ export default {
 </script>
 
 <style lang="scss" scoped>
+.avatar-wrapper {
+	overflow: hidden;
+}
+
 .avatar,
 .avatar-wrapper {
+	--size: var(--default-clickable-area);
 	border-radius: 50%;
 	width: var(--size);
 	height: var(--size);
 	text-align: center;
-	color: #ffffff;
+	color: var(--color-text-maxcontrast);
 	line-height: var(--size);
-	font-size: var(--font-size);
+	font-size: calc(var(--size) / 2);
 	font-weight: normal;
 	display: flex;
 	justify-content: center;
 	align-items: center;
+	border-width: 2px;
+	border-style: solid;
+	box-sizing: border-box;
 }
 </style>

src/components/Editor/DocumentStatus.vue

@@ -41,7 +41,7 @@ export default {
 	props: {
 		idle: {
 			type: Boolean,
-			require: true,
+			required: true,
 		},
 		lock: {
 			type: Object,
@@ -53,7 +53,7 @@ export default {
 		},
 		hasConnectionIssue: {
 			type: Boolean,
-			require: true,
+			required: true,
 		},
 	},