From a70cd368af3f90c12ad3d0dfb35ab75b98d5e619 Mon Sep 17 00:00:00 2001
From: eleith <eleith@users.noreply.github.com>
Date: Mon, 29 Jul 2024 14:38:47 -0700
Subject: [PATCH] fix(server): use fqdn for og:image meta tag value (#11082)

* attempt to use fqdn for og:image

opengraph image specifies that the url contains http or https, thus
implying a fqdn.

this change uses the external domain from the server config to attempt
to make the og:image have both the existing path to the thumbnail along
with the desired domain

if the server setting is empty, the old behavior will persist

please note, some og implementations do work with relative paths, so not
all og image checkers may still pass, but not all implementations have
this fallback and thus will not find the image otherwise

* tests and ssr for og:image value as fqdn

* formatting

* fix test

* formatting

* formatting

* fix tests

getConfig was requiring authentication. using already initiated global stores instead

* load config in shared link service itself

* join host and pathname/params safely

* use origin instead of host for full domain string

also fixes lint and address the imageURL type which is optional

* chore: clean up

---------

Co-authored-by: eleith <eleith@lemon.localdomain>
Co-authored-by: eleith <online-github@eleith.com>
Co-authored-by: Jason Rasmussen <jason@rasm.me>
---
 e2e/src/api/specs/shared-link.e2e-spec.ts       |  7 +++++++
 server/src/services/shared-link.service.spec.ts | 14 +++++++++++---
 server/src/services/shared-link.service.ts      | 17 ++++++++++++++---
 .../[[photos=photos]]/[[assetId=id]]/+page.ts   | 12 ++++++------
 web/src/routes/+layout.svelte                   | 16 +++++++++++++---
 5 files changed, 51 insertions(+), 15 deletions(-)

diff --git a/e2e/src/api/specs/shared-link.e2e-spec.ts b/e2e/src/api/specs/shared-link.e2e-spec.ts
index 3448b2c5f2..e62d18b72d 100644
--- a/e2e/src/api/specs/shared-link.e2e-spec.ts
+++ b/e2e/src/api/specs/shared-link.e2e-spec.ts
@@ -112,6 +112,13 @@ describe('/shared-links', () => {
       expect(resp.header['content-type']).toContain('text/html');
       expect(resp.text).toContain(`<meta name="description" content="1 shared photos & videos" />`);
     });
+
+    it('should have fqdn og:image meta tag for shared asset', async () => {
+      const resp = await request(shareUrl).get(`/${linkWithAssets.key}`);
+      expect(resp.status).toBe(200);
+      expect(resp.header['content-type']).toContain('text/html');
+      expect(resp.text).toContain(`<meta property="og:image" content="http://`);
+    });
   });
 
   describe('GET /shared-links', () => {
diff --git a/server/src/services/shared-link.service.spec.ts b/server/src/services/shared-link.service.spec.ts
index a5a24cfd7b..f0b42b0153 100644
--- a/server/src/services/shared-link.service.spec.ts
+++ b/server/src/services/shared-link.service.spec.ts
@@ -1,9 +1,12 @@
 import { BadRequestException, ForbiddenException, UnauthorizedException } from '@nestjs/common';
 import _ from 'lodash';
+import { DEFAULT_EXTERNAL_DOMAIN } from 'src/constants';
 import { AssetIdErrorReason } from 'src/dtos/asset-ids.response.dto';
 import { SharedLinkType } from 'src/entities/shared-link.entity';
 import { ICryptoRepository } from 'src/interfaces/crypto.interface';
+import { ILoggerRepository } from 'src/interfaces/logger.interface';
 import { ISharedLinkRepository } from 'src/interfaces/shared-link.interface';
+import { ISystemMetadataRepository } from 'src/interfaces/system-metadata.interface';
 import { SharedLinkService } from 'src/services/shared-link.service';
 import { albumStub } from 'test/fixtures/album.stub';
 import { assetStub } from 'test/fixtures/asset.stub';
@@ -11,7 +14,9 @@ import { authStub } from 'test/fixtures/auth.stub';
 import { sharedLinkResponseStub, sharedLinkStub } from 'test/fixtures/shared-link.stub';
 import { IAccessRepositoryMock, newAccessRepositoryMock } from 'test/repositories/access.repository.mock';
 import { newCryptoRepositoryMock } from 'test/repositories/crypto.repository.mock';
+import { newLoggerRepositoryMock } from 'test/repositories/logger.repository.mock';
 import { newSharedLinkRepositoryMock } from 'test/repositories/shared-link.repository.mock';
+import { newSystemMetadataRepositoryMock } from 'test/repositories/system-metadata.repository.mock';
 import { Mocked } from 'vitest';
 
 describe(SharedLinkService.name, () => {
@@ -19,13 +24,17 @@ describe(SharedLinkService.name, () => {
   let accessMock: IAccessRepositoryMock;
   let cryptoMock: Mocked<ICryptoRepository>;
   let shareMock: Mocked<ISharedLinkRepository>;
+  let systemMock: Mocked<ISystemMetadataRepository>;
+  let logMock: Mocked<ILoggerRepository>;
 
   beforeEach(() => {
     accessMock = newAccessRepositoryMock();
     cryptoMock = newCryptoRepositoryMock();
     shareMock = newSharedLinkRepositoryMock();
+    systemMock = newSystemMetadataRepositoryMock();
+    logMock = newLoggerRepositoryMock();
 
-    sut = new SharedLinkService(accessMock, cryptoMock, shareMock);
+    sut = new SharedLinkService(accessMock, cryptoMock, logMock, shareMock, systemMock);
   });
 
   it('should work', () => {
@@ -300,8 +309,7 @@ describe(SharedLinkService.name, () => {
       shareMock.get.mockResolvedValue(sharedLinkStub.individual);
       await expect(sut.getMetadataTags(authStub.adminSharedLink)).resolves.toEqual({
         description: '1 shared photos & videos',
-        imageUrl:
-          '/api/assets/asset-id/thumbnail?key=LCtkaJX4R1O_9D-2lq0STzsPryoL1UdAbyb6Sna1xxmQCSuqU2J1ZUsqt6GR-yGm1s0',
+        imageUrl: `${DEFAULT_EXTERNAL_DOMAIN}/api/assets/asset-id/thumbnail?key=LCtkaJX4R1O_9D-2lq0STzsPryoL1UdAbyb6Sna1xxmQCSuqU2J1ZUsqt6GR-yGm1s0`,
         title: 'Public Share',
       });
       expect(shareMock.get).toHaveBeenCalled();
diff --git a/server/src/services/shared-link.service.ts b/server/src/services/shared-link.service.ts
index 50ddba65cf..d1caf55e16 100644
--- a/server/src/services/shared-link.service.ts
+++ b/server/src/services/shared-link.service.ts
@@ -1,5 +1,7 @@
 import { BadRequestException, ForbiddenException, Inject, Injectable, UnauthorizedException } from '@nestjs/common';
+import { DEFAULT_EXTERNAL_DOMAIN } from 'src/constants';
 import { AccessCore, Permission } from 'src/cores/access.core';
+import { SystemConfigCore } from 'src/cores/system-config.core';
 import { AssetIdErrorReason, AssetIdsResponseDto } from 'src/dtos/asset-ids.response.dto';
 import { AssetIdsDto } from 'src/dtos/asset.dto';
 import { AuthDto } from 'src/dtos/auth.dto';
@@ -15,19 +17,26 @@ import { AssetEntity } from 'src/entities/asset.entity';
 import { SharedLinkEntity, SharedLinkType } from 'src/entities/shared-link.entity';
 import { IAccessRepository } from 'src/interfaces/access.interface';
 import { ICryptoRepository } from 'src/interfaces/crypto.interface';
+import { ILoggerRepository } from 'src/interfaces/logger.interface';
 import { ISharedLinkRepository } from 'src/interfaces/shared-link.interface';
+import { ISystemMetadataRepository } from 'src/interfaces/system-metadata.interface';
 import { OpenGraphTags } from 'src/utils/misc';
 
 @Injectable()
 export class SharedLinkService {
   private access: AccessCore;
+  private configCore: SystemConfigCore;
 
   constructor(
     @Inject(IAccessRepository) accessRepository: IAccessRepository,
     @Inject(ICryptoRepository) private cryptoRepository: ICryptoRepository,
+    @Inject(ILoggerRepository) private logger: ILoggerRepository,
     @Inject(ISharedLinkRepository) private repository: ISharedLinkRepository,
+    @Inject(ISystemMetadataRepository) systemMetadataRepository: ISystemMetadataRepository,
   ) {
+    this.logger.setContext(SharedLinkService.name);
     this.access = AccessCore.create(accessRepository);
+    this.configCore = SystemConfigCore.create(systemMetadataRepository, this.logger);
   }
 
   getAll(auth: AuthDto): Promise<SharedLinkResponseDto[]> {
@@ -183,16 +192,18 @@ export class SharedLinkService {
       return null;
     }
 
+    const config = await this.configCore.getConfig({ withCache: true });
     const sharedLink = await this.findOrFail(auth.sharedLink.userId, auth.sharedLink.id);
     const assetId = sharedLink.album?.albumThumbnailAssetId || sharedLink.assets[0]?.id;
     const assetCount = sharedLink.assets.length > 0 ? sharedLink.assets.length : sharedLink.album?.assets.length || 0;
+    const imagePath = assetId
+      ? `/api/assets/${assetId}/thumbnail?key=${sharedLink.key.toString('base64url')}`
+      : '/feature-panel.png';
 
     return {
       title: sharedLink.album ? sharedLink.album.albumName : 'Public Share',
       description: sharedLink.description || `${assetCount} shared photos & videos`,
-      imageUrl: assetId
-        ? `/api/assets/${assetId}/thumbnail?key=${sharedLink.key.toString('base64url')}`
-        : '/feature-panel.png',
+      imageUrl: new URL(imagePath, config.server.externalDomain || DEFAULT_EXTERNAL_DOMAIN).href,
     };
   }
 
diff --git a/web/src/routes/(user)/share/[key]/[[photos=photos]]/[[assetId=id]]/+page.ts b/web/src/routes/(user)/share/[key]/[[photos=photos]]/[[assetId=id]]/+page.ts
index b19b18a8da..66fc3552c7 100644
--- a/web/src/routes/(user)/share/[key]/[[photos=photos]]/[[assetId=id]]/+page.ts
+++ b/web/src/routes/(user)/share/[key]/[[photos=photos]]/[[assetId=id]]/+page.ts
@@ -8,28 +8,28 @@ import type { PageLoad } from './$types';
 export const load = (async ({ params }) => {
   const { key } = params;
   await authenticate({ public: true });
-  const asset = await getAssetInfoFromParam(params);
+
+  const $t = await getFormatter();
 
   try {
-    const sharedLink = await getMySharedLink({ key });
+    const [sharedLink, asset] = await Promise.all([getMySharedLink({ key }), getAssetInfoFromParam(params)]);
     setSharedLink(sharedLink);
     const assetCount = sharedLink.assets.length;
     const assetId = sharedLink.album?.albumThumbnailAssetId || sharedLink.assets[0]?.id;
-    const $t = await getFormatter();
+    const assetPath = assetId ? getAssetThumbnailUrl(assetId) : '/feature-panel.png';
 
     return {
       sharedLink,
+      sharedLinkKey: key,
       asset,
-      key,
       meta: {
         title: sharedLink.album ? sharedLink.album.albumName : $t('public_share'),
         description: sharedLink.description || $t('shared_photos_and_videos_count', { values: { assetCount } }),
-        imageUrl: assetId ? getAssetThumbnailUrl(assetId) : '/feature-panel.png',
+        imageUrl: assetPath,
       },
     };
   } catch (error) {
     if (isHttpError(error) && error.data.message === 'Invalid password') {
-      const $t = await getFormatter();
       return {
         passwordRequired: true,
         sharedLinkKey: key,
diff --git a/web/src/routes/+layout.svelte b/web/src/routes/+layout.svelte
index a6661f88cd..d086129d7f 100644
--- a/web/src/routes/+layout.svelte
+++ b/web/src/routes/+layout.svelte
@@ -10,7 +10,7 @@
   import VersionAnnouncementBox from '$lib/components/shared-components/version-announcement-box.svelte';
   import { Theme } from '$lib/constants';
   import { colorTheme, handleToggleTheme, type ThemeSetting } from '$lib/stores/preferences.store';
-  import { loadConfig } from '$lib/stores/server-config.store';
+  import { loadConfig, serverConfig } from '$lib/stores/server-config.store';
   import { user } from '$lib/stores/user.store';
   import { closeWebsocketConnection, openWebsocketConnection } from '$lib/stores/websocket';
   import { setKey } from '$lib/utils';
@@ -95,13 +95,23 @@
     <meta property="og:type" content="website" />
     <meta property="og:title" content={$page.data.meta.title} />
     <meta property="og:description" content={$page.data.meta.description} />
-    <meta property="og:image" content={$page.data.meta.imageUrl} />
+    {#if $page.data.meta.imageUrl}
+      <meta
+        property="og:image"
+        content={new URL($page.data.meta.imageUrl, $serverConfig.externalDomain || window.location.origin).href}
+      />
+    {/if}
 
     <!-- Twitter Meta Tags -->
     <meta name="twitter:card" content="summary_large_image" />
     <meta name="twitter:title" content={$page.data.meta.title} />
     <meta name="twitter:description" content={$page.data.meta.description} />
-    <meta name="twitter:image" content={$page.data.meta.imageUrl} />
+    {#if $page.data.meta.imageUrl}
+      <meta
+        name="twitter:image"
+        content={new URL($page.data.meta.imageUrl, $serverConfig.externalDomain || window.location.origin).href}
+      />
+    {/if}
   {/if}
 </svelte:head>