diff --git a/e2e/src/responses.ts b/e2e/src/responses.ts index 958548435..3d7971d6f 100644 --- a/e2e/src/responses.ts +++ b/e2e/src/responses.ts @@ -43,10 +43,10 @@ export const errorDto = { message: 'Invalid share key', correlationId: expect.any(String), }, - invalidSharePassword: { + passwordRequired: { error: 'Unauthorized', statusCode: 401, - message: 'Invalid password', + message: 'Password required', correlationId: expect.any(String), }, badRequest: (message: any = null) => ({ diff --git a/e2e/src/specs/server/api/shared-link.e2e-spec.ts b/e2e/src/specs/server/api/shared-link.e2e-spec.ts index 8c15a14da..80232beb7 100644 --- a/e2e/src/specs/server/api/shared-link.e2e-spec.ts +++ b/e2e/src/specs/server/api/shared-link.e2e-spec.ts @@ -239,7 +239,7 @@ describe('/shared-links', () => { const { status, body } = await request(app).get('/shared-links/me').query({ key: linkWithPassword.key }); expect(status).toBe(401); - expect(body).toEqual(errorDto.invalidSharePassword); + expect(body).toEqual(errorDto.passwordRequired); }); it('should get data for correct password protected link', async () => { diff --git a/mobile/openapi/README.md b/mobile/openapi/README.md index 702870c85..4ebe5c7c6 100644 Binary files a/mobile/openapi/README.md and b/mobile/openapi/README.md differ diff --git a/mobile/openapi/lib/api.dart b/mobile/openapi/lib/api.dart index 90e426b54..f10490e09 100644 Binary files a/mobile/openapi/lib/api.dart and b/mobile/openapi/lib/api.dart differ diff --git a/mobile/openapi/lib/api/shared_links_api.dart b/mobile/openapi/lib/api/shared_links_api.dart index 7f11db76d..37eeffcf4 100644 Binary files a/mobile/openapi/lib/api/shared_links_api.dart and b/mobile/openapi/lib/api/shared_links_api.dart differ diff --git a/mobile/openapi/lib/api_client.dart b/mobile/openapi/lib/api_client.dart index 7f5cd50ed..470f3aec2 100644 Binary files a/mobile/openapi/lib/api_client.dart and b/mobile/openapi/lib/api_client.dart differ diff --git a/mobile/openapi/lib/model/shared_link_login_dto.dart b/mobile/openapi/lib/model/shared_link_login_dto.dart new file mode 100644 index 000000000..1ab1bc934 Binary files /dev/null and b/mobile/openapi/lib/model/shared_link_login_dto.dart differ diff --git a/open-api/immich-openapi-specs.json b/open-api/immich-openapi-specs.json index d9743e3d4..13d6ba7e5 100644 --- a/open-api/immich-openapi-specs.json +++ b/open-api/immich-openapi-specs.json @@ -11239,6 +11239,78 @@ "x-immich-state": "Stable" } }, + "/shared-links/login": { + "post": { + "description": "Login to a password protected shared link", + "operationId": "sharedLinkLogin", + "parameters": [ + { + "name": "key", + "required": false, + "in": "query", + "schema": { + "type": "string" + } + }, + { + "name": "slug", + "required": false, + "in": "query", + "schema": { + "type": "string" + } + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/SharedLinkLoginDto" + } + } + }, + "required": true + }, + "responses": { + "201": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/SharedLinkResponseDto" + } + } + }, + "description": "" + } + }, + "security": [ + { + "bearer": [] + }, + { + "cookie": [] + }, + { + "api_key": [] + } + ], + "summary": "Shared link login", + "tags": [ + "Shared links" + ], + "x-immich-history": [ + { + "version": "v2.6.0", + "state": "Added" + }, + { + "version": "v2.6.0", + "state": "Beta" + } + ], + "x-immich-state": "Beta" + } + }, "/shared-links/me": { "get": { "description": "Retrieve the current shared link associated with authentication method.", @@ -21686,6 +21758,19 @@ }, "type": "object" }, + "SharedLinkLoginDto": { + "properties": { + "password": { + "description": "Shared link password", + "example": "password", + "type": "string" + } + }, + "required": [ + "password" + ], + "type": "object" + }, "SharedLinkResponseDto": { "properties": { "album": { @@ -21744,9 +21829,25 @@ "type": "string" }, "token": { + "deprecated": true, "description": "Access token", "nullable": true, - "type": "string" + "type": "string", + "x-immich-history": [ + { + "version": "v1", + "state": "Added" + }, + { + "version": "v2", + "state": "Stable" + }, + { + "version": "v2.6.0", + "state": "Deprecated" + } + ], + "x-immich-state": "Deprecated" }, "type": { "allOf": [ diff --git a/open-api/typescript-sdk/src/fetch-client.ts b/open-api/typescript-sdk/src/fetch-client.ts index 28a8bc495..59a25d58b 100644 --- a/open-api/typescript-sdk/src/fetch-client.ts +++ b/open-api/typescript-sdk/src/fetch-client.ts @@ -2287,6 +2287,10 @@ export type SharedLinkCreateDto = { /** Shared link type */ "type": SharedLinkType; }; +export type SharedLinkLoginDto = { + /** Shared link password */ + password: string; +}; export type SharedLinkEditDto = { /** Allow downloads */ allowDownload?: boolean; @@ -5861,6 +5865,26 @@ export function createSharedLink({ sharedLinkCreateDto }: { body: sharedLinkCreateDto }))); } +/** + * Shared link login + */ +export function sharedLinkLogin({ key, slug, sharedLinkLoginDto }: { + key?: string; + slug?: string; + sharedLinkLoginDto: SharedLinkLoginDto; +}, opts?: Oazapfts.RequestOpts) { + return oazapfts.ok(oazapfts.fetchJson<{ + status: 201; + data: SharedLinkResponseDto; + }>(`/shared-links/login${QS.query(QS.explode({ + key, + slug + }))}`, oazapfts.json({ + ...opts, + method: "POST", + body: sharedLinkLoginDto + }))); +} /** * Retrieve current shared link */ diff --git a/server/src/controllers/shared-link.controller.ts b/server/src/controllers/shared-link.controller.ts index 8875127a2..1f91409e8 100644 --- a/server/src/controllers/shared-link.controller.ts +++ b/server/src/controllers/shared-link.controller.ts @@ -22,21 +22,39 @@ import { AuthDto } from 'src/dtos/auth.dto'; import { SharedLinkCreateDto, SharedLinkEditDto, + SharedLinkLoginDto, SharedLinkPasswordDto, SharedLinkResponseDto, SharedLinkSearchDto, } from 'src/dtos/shared-link.dto'; import { ApiTag, ImmichCookie, Permission } from 'src/enum'; import { Auth, Authenticated, GetLoginDetails } from 'src/middleware/auth.guard'; +import { LoggingRepository } from 'src/repositories/logging.repository'; import { LoginDetails } from 'src/services/auth.service'; import { SharedLinkService } from 'src/services/shared-link.service'; import { respondWithCookie } from 'src/utils/response'; import { UUIDParamDto } from 'src/validation'; +const getAuthTokens = (cookies: Record | undefined) => { + return cookies?.[ImmichCookie.SharedLinkToken]?.split(',') || []; +}; + +const merge = (cookies: Record | undefined, token: string) => { + const authTokens = getAuthTokens(cookies); + if (!authTokens.includes(token)) { + authTokens.push(token); + } + + return authTokens.join(','); +}; + @ApiTags(ApiTag.SharedLinks) @Controller('shared-links') export class SharedLinkController { - constructor(private service: SharedLinkService) {} + constructor( + private service: SharedLinkService, + private logger: LoggingRepository, + ) {} @Get() @Authenticated({ permission: Permission.SharedLinkRead }) @@ -49,6 +67,28 @@ export class SharedLinkController { return this.service.getAll(auth, dto); } + @Post('login') + @Authenticated({ sharedLink: true }) + @Endpoint({ + summary: 'Shared link login', + description: 'Login to a password protected shared link', + history: new HistoryBuilder().added('v2.6.0').beta('v2.6.0'), + }) + async sharedLinkLogin( + @Auth() auth: AuthDto, + @Body() dto: SharedLinkLoginDto, + @Req() req: Request, + @Res({ passthrough: true }) res: Response, + @GetLoginDetails() loginDetails: LoginDetails, + ): Promise { + const { sharedLink, token } = await this.service.login(auth, dto); + + return respondWithCookie(res, sharedLink, { + isSecure: loginDetails.isSecure, + values: [{ key: ImmichCookie.SharedLinkToken, value: merge(req.cookies, token) }], + }); + } + @Get('me') @Authenticated({ sharedLink: true }) @Endpoint({ @@ -59,19 +99,19 @@ export class SharedLinkController { async getMySharedLink( @Auth() auth: AuthDto, @Query() dto: SharedLinkPasswordDto, - @Req() request: Request, + @Req() req: Request, @Res({ passthrough: true }) res: Response, @GetLoginDetails() loginDetails: LoginDetails, ): Promise { - const sharedLinkToken = request.cookies?.[ImmichCookie.SharedLinkToken]; - if (sharedLinkToken) { - dto.token = sharedLinkToken; + if (dto.password) { + this.logger.deprecate( + 'Passing shared link password via query parameters is deprecated and will be removed in the next major release. Please use POST /shared-links/login instead.', + ); + + return this.sharedLinkLogin(auth, { password: dto.password }, req, res, loginDetails); } - const body = await this.service.getMine(auth, dto); - return respondWithCookie(res, body, { - isSecure: loginDetails.isSecure, - values: body.token ? [{ key: ImmichCookie.SharedLinkToken, value: body.token }] : [], - }); + + return this.service.getMine(auth, getAuthTokens(req.cookies)); } @Get(':id') diff --git a/server/src/dtos/shared-link.dto.ts b/server/src/dtos/shared-link.dto.ts index 1465f6895..b2ecc70a3 100644 --- a/server/src/dtos/shared-link.dto.ts +++ b/server/src/dtos/shared-link.dto.ts @@ -1,11 +1,11 @@ import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; import { IsString } from 'class-validator'; import { SharedLink } from 'src/database'; -import { HistoryBuilder } from 'src/decorators'; +import { HistoryBuilder, Property } from 'src/decorators'; import { AlbumResponseDto, mapAlbumWithoutAssets } from 'src/dtos/album.dto'; import { AssetResponseDto, mapAsset } from 'src/dtos/asset-response.dto'; import { SharedLinkType } from 'src/enum'; -import { Optional, ValidateBoolean, ValidateDate, ValidateEnum, ValidateUUID } from 'src/validation'; +import { Optional, ValidateBoolean, ValidateDate, ValidateEnum, ValidateString, ValidateUUID } from 'src/validation'; export class SharedLinkSearchDto { @ValidateUUID({ optional: true, description: 'Filter by album ID' }) @@ -94,6 +94,11 @@ export class SharedLinkEditDto { changeExpiryTime?: boolean; } +export class SharedLinkLoginDto { + @ValidateString({ description: 'Shared link password', example: 'password' }) + password!: string; +} + export class SharedLinkPasswordDto { @ApiPropertyOptional({ example: 'password', description: 'Link password' }) @IsString() @@ -112,7 +117,10 @@ export class SharedLinkResponseDto { description!: string | null; @ApiProperty({ description: 'Has password' }) password!: string | null; - @ApiPropertyOptional({ description: 'Access token' }) + @Property({ + description: 'Access token', + history: new HistoryBuilder().added('v1').stable('v2').deprecated('v2.6.0'), + }) token?: string | null; @ApiProperty({ description: 'Owner user ID' }) userId!: string; diff --git a/server/src/services/shared-link.service.spec.ts b/server/src/services/shared-link.service.spec.ts index d4f11f37a..5ad145af2 100644 --- a/server/src/services/shared-link.service.spec.ts +++ b/server/src/services/shared-link.service.spec.ts @@ -35,14 +35,14 @@ describe(SharedLinkService.name, () => { describe('getMine', () => { it('should only work for a public user', async () => { - await expect(sut.getMine(authStub.admin, {})).rejects.toBeInstanceOf(ForbiddenException); + await expect(sut.getMine(authStub.admin, [])).rejects.toBeInstanceOf(ForbiddenException); expect(mocks.sharedLink.get).not.toHaveBeenCalled(); }); it('should return the shared link for the public user', async () => { const authDto = authStub.adminSharedLink; mocks.sharedLink.get.mockResolvedValue(sharedLinkStub.valid); - await expect(sut.getMine(authDto, {})).resolves.toEqual(sharedLinkResponseStub.valid); + await expect(sut.getMine(authDto, [])).resolves.toEqual(sharedLinkResponseStub.valid); expect(mocks.sharedLink.get).toHaveBeenCalledWith(authDto.user.id, authDto.sharedLink?.id); }); @@ -55,21 +55,22 @@ describe(SharedLinkService.name, () => { }, }); mocks.sharedLink.get.mockResolvedValue(sharedLinkStub.readonlyNoExif); - const response = await sut.getMine(authDto, {}); + const response = await sut.getMine(authDto, []); expect(response.assets[0]).toMatchObject({ hasMetadata: false }); expect(mocks.sharedLink.get).toHaveBeenCalledWith(authDto.user.id, authDto.sharedLink?.id); }); - it('should throw an error for an invalid password protected shared link', async () => { + it('should throw an error for a request without a shared link auth token', async () => { const authDto = authStub.adminSharedLink; mocks.sharedLink.get.mockResolvedValue(sharedLinkStub.passwordRequired); - await expect(sut.getMine(authDto, {})).rejects.toBeInstanceOf(UnauthorizedException); + await expect(sut.getMine(authDto, [])).rejects.toBeInstanceOf(UnauthorizedException); expect(mocks.sharedLink.get).toHaveBeenCalledWith(authDto.user.id, authDto.sharedLink?.id); }); - it('should allow a correct password on a password protected shared link', async () => { + it('should accept a valid shared link auth token', async () => { mocks.sharedLink.get.mockResolvedValue({ ...sharedLinkStub.individual, password: '123' }); - await expect(sut.getMine(authStub.adminSharedLink, { password: '123' })).resolves.toBeDefined(); + mocks.crypto.hashSha256.mockReturnValue('hashed-auth-token'); + await expect(sut.getMine(authStub.adminSharedLink, ['hashed-auth-token'])).resolves.toBeDefined(); expect(mocks.sharedLink.get).toHaveBeenCalledWith( authStub.adminSharedLink.user.id, authStub.adminSharedLink.sharedLink?.id, diff --git a/server/src/services/shared-link.service.ts b/server/src/services/shared-link.service.ts index 144059808..e321e4990 100644 --- a/server/src/services/shared-link.service.ts +++ b/server/src/services/shared-link.service.ts @@ -1,6 +1,5 @@ import { BadRequestException, ForbiddenException, Injectable, UnauthorizedException } from '@nestjs/common'; import { PostgresError } from 'postgres'; -import { SharedLink } from 'src/database'; import { AssetIdErrorReason, AssetIdsResponseDto } from 'src/dtos/asset-ids.response.dto'; import { AssetIdsDto } from 'src/dtos/asset.dto'; import { AuthDto } from 'src/dtos/auth.dto'; @@ -8,7 +7,7 @@ import { mapSharedLink, SharedLinkCreateDto, SharedLinkEditDto, - SharedLinkPasswordDto, + SharedLinkLoginDto, SharedLinkResponseDto, SharedLinkSearchDto, } from 'src/dtos/shared-link.dto'; @@ -24,18 +23,41 @@ export class SharedLinkService extends BaseService { .then((links) => links.map((link) => mapSharedLink(link, { stripAssetMetadata: false }))); } - async getMine(auth: AuthDto, dto: SharedLinkPasswordDto): Promise { + async login(auth: AuthDto, dto: SharedLinkLoginDto) { if (!auth.sharedLink) { throw new ForbiddenException(); } const sharedLink = await this.findOrFail(auth.user.id, auth.sharedLink.id); - const response = mapSharedLink(sharedLink, { stripAssetMetadata: !sharedLink.showExif }); - if (sharedLink.password) { - response.token = this.validateAndRefreshToken(sharedLink, dto); + const { id, password } = sharedLink; + + if (!password) { + throw new BadRequestException('Shared link is not password protected'); } - return response; + if (password !== dto.password) { + throw new UnauthorizedException('Invalid password'); + } + + return { + sharedLink: mapSharedLink(sharedLink, { stripAssetMetadata: !sharedLink.showExif }), + token: this.asToken({ id, password }), + }; + } + + async getMine(auth: AuthDto, authTokens: string[]) { + if (!auth.sharedLink) { + throw new ForbiddenException(); + } + + const sharedLink = await this.findOrFail(auth.user.id, auth.sharedLink.id); + const { id, password } = sharedLink; + + if (password && !authTokens.includes(this.asToken({ id, password }))) { + throw new UnauthorizedException('Password required'); + } + + return mapSharedLink(sharedLink, { stripAssetMetadata: !sharedLink.showExif }); } async get(auth: AuthDto, id: string): Promise { @@ -213,16 +235,7 @@ export class SharedLinkService extends BaseService { }; } - private validateAndRefreshToken(sharedLink: SharedLink, dto: SharedLinkPasswordDto): string { - const token = this.cryptoRepository.hashSha256(`${sharedLink.id}-${sharedLink.password}`); - const sharedLinkTokens = dto.token?.split(',') || []; - if (sharedLink.password !== dto.password && !sharedLinkTokens.includes(token)) { - throw new UnauthorizedException('Invalid password'); - } - - if (!sharedLinkTokens.includes(token)) { - sharedLinkTokens.push(token); - } - return sharedLinkTokens.join(','); + private asToken(sharedLink: { id: string; password: string }) { + return this.cryptoRepository.hashSha256(`${sharedLink.id}-${sharedLink.password}`); } } diff --git a/server/test/medium/specs/services/shared-link.service.spec.ts b/server/test/medium/specs/services/shared-link.service.spec.ts index acc51374d..a43d0de9b 100644 --- a/server/test/medium/specs/services/shared-link.service.spec.ts +++ b/server/test/medium/specs/services/shared-link.service.spec.ts @@ -90,7 +90,7 @@ describe(SharedLinkService.name, () => { assetIds: assets.map(({ asset }) => asset.id), }); - await expect(sut.getMine({ user, sharedLink }, {})).resolves.toMatchObject({ + await expect(sut.getMine({ user, sharedLink }, [])).resolves.toMatchObject({ assets: assets.map(({ asset }) => expect.objectContaining({ id: asset.id })), }); }); @@ -114,7 +114,7 @@ describe(SharedLinkService.name, () => { assetIds: [asset.id], }); - await expect(sut.getMine({ user, sharedLink }, {})).resolves.toMatchObject({ + await expect(sut.getMine({ user, sharedLink }, [])).resolves.toMatchObject({ assets: [expect.objectContaining({ id: asset.id })], }); @@ -122,6 +122,6 @@ describe(SharedLinkService.name, () => { assetIds: [asset.id], }); - await expect(sut.getMine({ user, sharedLink }, {})).resolves.toHaveProperty('assets', []); + await expect(sut.getMine({ user, sharedLink }, [])).resolves.toHaveProperty('assets', []); }); }); diff --git a/web/src/lib/components/pages/SharedLinkPage.svelte b/web/src/lib/components/pages/SharedLinkPage.svelte index d30c3ce34..9965be231 100644 --- a/web/src/lib/components/pages/SharedLinkPage.svelte +++ b/web/src/lib/components/pages/SharedLinkPage.svelte @@ -8,7 +8,7 @@ import { setSharedLink } from '$lib/utils'; import { handleError } from '$lib/utils/handle-error'; import { navigate } from '$lib/utils/navigation'; - import { getMySharedLink, SharedLinkType, type AssetResponseDto, type SharedLinkResponseDto } from '@immich/sdk'; + import { sharedLinkLogin, SharedLinkType, type AssetResponseDto, type SharedLinkResponseDto } from '@immich/sdk'; import { Button, Logo, PasswordInput } from '@immich/ui'; import { tick } from 'svelte'; import { t } from 'svelte-i18n'; @@ -39,7 +39,7 @@ const handlePasswordSubmit = async () => { try { - sharedLink = await getMySharedLink({ password, key, slug }); + sharedLink = await sharedLinkLogin({ key, slug, sharedLinkLoginDto: { password } }); setSharedLink(sharedLink); passwordRequired = false; title = (sharedLink.album ? sharedLink.album.albumName : $t('public_share')) + ' - Immich'; diff --git a/web/src/lib/utils/shared-links.ts b/web/src/lib/utils/shared-links.ts index e1bad6bf3..423eda310 100644 --- a/web/src/lib/utils/shared-links.ts +++ b/web/src/lib/utils/shared-links.ts @@ -49,7 +49,7 @@ export const loadSharedLink = async ({ }, }; } catch (error) { - if (isHttpError(error) && error.data.message === 'Invalid password') { + if (isHttpError(error) && error.data.message === 'Password required') { return { ...common, passwordRequired: true,