From 2929b648bc98625c9e76d8c8e0fae2c64ee7c376 Mon Sep 17 00:00:00 2001 From: Daniel Biegler Date: Wed, 20 Nov 2024 23:23:10 +0100 Subject: [PATCH 01/11] feat(core): Refactor how permissions get serialized for sessions into using a new strategy --- .../api/resolvers/base/base-auth.resolver.ts | 9 +++-- ...annel-role-permission-resolver-strategy.ts | 39 +++++++++++++++++++ ...fault-role-permission-resolver-strategy.ts | 13 +++++++ .../auth/role-permission-resolver-strategy.ts | 10 +++++ packages/core/src/config/config.module.ts | 3 +- packages/core/src/config/default-config.ts | 6 ++- packages/core/src/config/vendure-config.ts | 2 + .../src/entity/channel-role/channel-role.ts | 34 ++++++++++++++++ .../core/src/entity/custom-entity-fields.ts | 1 + packages/core/src/entity/entities.ts | 2 + .../request-context.service.ts | 5 ++- .../utils/get-user-channels-permissions.ts | 1 + .../service/services/channel-role.service.ts | 16 ++++++++ .../core/src/service/services/role.service.ts | 7 +--- .../src/service/services/session.service.ts | 22 ++++++----- packages/dev-server/dev-config.ts | 13 +++---- 16 files changed, 154 insertions(+), 29 deletions(-) create mode 100644 packages/core/src/config/auth/channel-role-permission-resolver-strategy.ts create mode 100644 packages/core/src/config/auth/default-role-permission-resolver-strategy.ts create mode 100644 packages/core/src/config/auth/role-permission-resolver-strategy.ts create mode 100644 packages/core/src/entity/channel-role/channel-role.ts create mode 100644 packages/core/src/service/services/channel-role.service.ts diff --git a/packages/core/src/api/resolvers/base/base-auth.resolver.ts b/packages/core/src/api/resolvers/base/base-auth.resolver.ts index edbdc94bf0..5213c42b1a 100644 --- a/packages/core/src/api/resolvers/base/base-auth.resolver.ts +++ b/packages/core/src/api/resolvers/base/base-auth.resolver.ts @@ -1,6 +1,6 @@ import { - AuthenticationResult as ShopAuthenticationResult, PasswordValidationError, + AuthenticationResult as ShopAuthenticationResult, } from '@vendure/common/lib/generated-shop-types'; import { AuthenticationResult as AdminAuthenticationResult, @@ -22,7 +22,6 @@ import { NATIVE_AUTH_STRATEGY_NAME } from '../../../config/auth/native-authentic import { ConfigService } from '../../../config/config.service'; import { LogLevel } from '../../../config/logger/vendure-logger'; import { User } from '../../../entity/user/user.entity'; -import { getUserChannelsPermissions } from '../../../service/helpers/utils/get-user-channels-permissions'; import { AdministratorService } from '../../../service/services/administrator.service'; import { AuthService } from '../../../service/services/auth.service'; import { UserService } from '../../../service/services/user.service'; @@ -143,11 +142,13 @@ export class BaseAuthResolver { /** * Exposes a subset of the User properties which we want to expose to the public API. */ - protected publiclyAccessibleUser(user: User): CurrentUser { + protected async publiclyAccessibleUser(user: User): Promise { return { id: user.id, identifier: user.identifier, - channels: getUserChannelsPermissions(user) as CurrentUserChannel[], + channels: (await this.configService.authOptions.rolePermissionResolverStrategy.resolvePermissions( + user, + )) as CurrentUserChannel[], }; } } diff --git a/packages/core/src/config/auth/channel-role-permission-resolver-strategy.ts b/packages/core/src/config/auth/channel-role-permission-resolver-strategy.ts new file mode 100644 index 0000000000..c219528bb6 --- /dev/null +++ b/packages/core/src/config/auth/channel-role-permission-resolver-strategy.ts @@ -0,0 +1,39 @@ +import { Injector } from '../../common'; +import { TransactionalConnection } from '../../connection'; +import { User } from '../../entity'; +import { ChannelRole } from '../../entity/channel-role/channel-role'; +import { UserChannelPermissions } from '../../service/helpers/utils/get-user-channels-permissions'; + +import { RolePermissionResolverStrategy } from './role-permission-resolver-strategy'; + +export class ChannelRolePermissionResolverStrategy implements RolePermissionResolverStrategy { + private connection: TransactionalConnection; + + async init(injector: Injector) { + this.connection = injector.get(TransactionalConnection); + } + + async resolvePermissions(user: User): Promise { + console.log('---- BEGIN RESOLVE'); + const channelRoleEntries = await this.connection.rawConnection.getRepository(ChannelRole).find({ + where: { user: { id: user.id } }, + relations: ['user', 'channel', 'role'], + }); + console.log('---- RESOLVE -- ENTRIES:', JSON.stringify(channelRoleEntries)); + + const channelRolePermissions = new Array(channelRoleEntries.length); + for (let i = 0; i < channelRoleEntries.length; i++) { + channelRolePermissions[i] = { + id: channelRoleEntries[i].channel.id, + token: channelRoleEntries[i].channel.token, + code: channelRoleEntries[i].channel.code, + permissions: channelRoleEntries[i].role.permissions, + }; + } + channelRoleEntries.sort((a, b) => (a.id < b.id ? -1 : 1)); + console.log('---- RESOLVE -- OUTPUT:', channelRolePermissions); + + console.log('---- END RESOLVE'); + return channelRolePermissions; + } +} diff --git a/packages/core/src/config/auth/default-role-permission-resolver-strategy.ts b/packages/core/src/config/auth/default-role-permission-resolver-strategy.ts new file mode 100644 index 0000000000..ef1cba3f4d --- /dev/null +++ b/packages/core/src/config/auth/default-role-permission-resolver-strategy.ts @@ -0,0 +1,13 @@ +import { User } from '../../entity'; +import { + getChannelPermissions, + UserChannelPermissions, +} from '../../service/helpers/utils/get-user-channels-permissions'; + +import { RolePermissionResolverStrategy } from './role-permission-resolver-strategy'; + +export class DefaultRolePermissionResolverStrategy implements RolePermissionResolverStrategy { + async resolvePermissions(user: User): Promise { + return getChannelPermissions(user.roles); + } +} diff --git a/packages/core/src/config/auth/role-permission-resolver-strategy.ts b/packages/core/src/config/auth/role-permission-resolver-strategy.ts new file mode 100644 index 0000000000..44636df234 --- /dev/null +++ b/packages/core/src/config/auth/role-permission-resolver-strategy.ts @@ -0,0 +1,10 @@ +import { InjectableStrategy } from '../../common'; +import { User } from '../../entity'; +import { UserChannelPermissions } from '../../service/helpers/utils/get-user-channels-permissions'; + +/** + * @description TODO + */ +export interface RolePermissionResolverStrategy extends InjectableStrategy { + resolvePermissions(user: User): Promise; +} diff --git a/packages/core/src/config/config.module.ts b/packages/core/src/config/config.module.ts index 26b9662268..d36e79ec7a 100644 --- a/packages/core/src/config/config.module.ts +++ b/packages/core/src/config/config.module.ts @@ -1,6 +1,5 @@ import { Module, OnApplicationBootstrap, OnApplicationShutdown } from '@nestjs/common'; import { ModuleRef } from '@nestjs/core'; -import { notNullOrUndefined } from '@vendure/common/lib/shared-utils'; import { ConfigurableOperationDef } from '../common/configurable-operation'; import { Injector } from '../common/injector'; @@ -83,6 +82,7 @@ export class ConfigModule implements OnApplicationBootstrap, OnApplicationShutdo sessionCacheStrategy, passwordHashingStrategy, passwordValidationStrategy, + rolePermissionResolverStrategy, } = this.configService.authOptions; const { taxZoneStrategy, taxLineCalculationStrategy } = this.configService.taxOptions; const { jobQueueStrategy, jobBufferStorageStrategy } = this.configService.jobQueueOptions; @@ -117,6 +117,7 @@ export class ConfigModule implements OnApplicationBootstrap, OnApplicationShutdo sessionCacheStrategy, passwordHashingStrategy, passwordValidationStrategy, + rolePermissionResolverStrategy, assetNamingStrategy, assetPreviewStrategy, assetStorageStrategy, diff --git a/packages/core/src/config/default-config.ts b/packages/core/src/config/default-config.ts index dfd9fe3e38..ebb083d45b 100644 --- a/packages/core/src/config/default-config.ts +++ b/packages/core/src/config/default-config.ts @@ -1,9 +1,9 @@ import { LanguageCode } from '@vendure/common/lib/generated-types'; import { DEFAULT_AUTH_TOKEN_HEADER_KEY, + DEFAULT_CHANNEL_TOKEN_KEY, SUPER_ADMIN_USER_IDENTIFIER, SUPER_ADMIN_USER_PASSWORD, - DEFAULT_CHANNEL_TOKEN_KEY, } from '@vendure/common/lib/shared-constants'; import { randomBytes } from 'crypto'; @@ -17,6 +17,7 @@ import { NoAssetPreviewStrategy } from './asset-preview-strategy/no-asset-previe import { NoAssetStorageStrategy } from './asset-storage-strategy/no-asset-storage-strategy'; import { BcryptPasswordHashingStrategy } from './auth/bcrypt-password-hashing-strategy'; import { DefaultPasswordValidationStrategy } from './auth/default-password-validation-strategy'; +import { DefaultRolePermissionResolverStrategy } from './auth/default-role-permission-resolver-strategy'; import { NativeAuthenticationStrategy } from './auth/native-authentication-strategy'; import { defaultCollectionFilters } from './catalog/default-collection-filters'; import { DefaultProductVariantPriceCalculationStrategy } from './catalog/default-product-variant-price-calculation-strategy'; @@ -109,6 +110,9 @@ export const defaultConfig: RuntimeVendureConfig = { customPermissions: [], passwordHashingStrategy: new BcryptPasswordHashingStrategy(), passwordValidationStrategy: new DefaultPasswordValidationStrategy({ minLength: 4 }), + rolePermissionResolverStrategy: new DefaultRolePermissionResolverStrategy(), + // TODO: remove once the weird type mismatch from dev-config gets fixed + // rolePermissionResolverStrategy: new ChannelRolePermissionResolverStrategy(), }, catalogOptions: { collectionFilters: defaultCollectionFilters, diff --git a/packages/core/src/config/vendure-config.ts b/packages/core/src/config/vendure-config.ts index 017dfce4c5..0a13f538f3 100644 --- a/packages/core/src/config/vendure-config.ts +++ b/packages/core/src/config/vendure-config.ts @@ -17,6 +17,7 @@ import { AssetStorageStrategy } from './asset-storage-strategy/asset-storage-str import { AuthenticationStrategy } from './auth/authentication-strategy'; import { PasswordHashingStrategy } from './auth/password-hashing-strategy'; import { PasswordValidationStrategy } from './auth/password-validation-strategy'; +import { RolePermissionResolverStrategy } from './auth/role-permission-resolver-strategy'; import { CollectionFilter } from './catalog/collection-filter'; import { ProductVariantPriceCalculationStrategy } from './catalog/product-variant-price-calculation-strategy'; import { ProductVariantPriceSelectionStrategy } from './catalog/product-variant-price-selection-strategy'; @@ -473,6 +474,7 @@ export interface AuthOptions { * @default DefaultPasswordValidationStrategy */ passwordValidationStrategy?: PasswordValidationStrategy; + rolePermissionResolverStrategy?: RolePermissionResolverStrategy; } /** diff --git a/packages/core/src/entity/channel-role/channel-role.ts b/packages/core/src/entity/channel-role/channel-role.ts new file mode 100644 index 0000000000..db10d10ced --- /dev/null +++ b/packages/core/src/entity/channel-role/channel-role.ts @@ -0,0 +1,34 @@ +import { DeepPartial } from '@vendure/common/lib/shared-types'; +import { Column, Entity, ManyToOne } from 'typeorm'; + +import { HasCustomFields } from '../../config'; +import { VendureEntity } from '../base/base.entity'; +import { Channel } from '../channel/channel.entity'; +import { CustomChannelRoleFields } from '../custom-entity-fields'; +import { Role } from '../role/role.entity'; +import { User } from '../user/user.entity'; + +/** + * @description + * TODO + * + * @docsCategory entities + */ +@Entity() +export class ChannelRole extends VendureEntity implements HasCustomFields { + constructor(input?: DeepPartial) { + super(input); + } + + @Column(type => CustomChannelRoleFields) + customFields: CustomChannelRoleFields; + + @ManyToOne(type => User) + user: User; + + @ManyToOne(type => Channel) + channel: Channel; + + @ManyToOne(type => Role) + role: Role; +} diff --git a/packages/core/src/entity/custom-entity-fields.ts b/packages/core/src/entity/custom-entity-fields.ts index 298368199e..80aeb9b1eb 100644 --- a/packages/core/src/entity/custom-entity-fields.ts +++ b/packages/core/src/entity/custom-entity-fields.ts @@ -35,5 +35,6 @@ export class CustomShippingMethodFieldsTranslation {} export class CustomStockLocationFields {} export class CustomTaxCategoryFields {} export class CustomTaxRateFields {} +export class CustomChannelRoleFields {} export class CustomUserFields {} export class CustomZoneFields {} diff --git a/packages/core/src/entity/entities.ts b/packages/core/src/entity/entities.ts index 54e1c245f3..55e903c89e 100644 --- a/packages/core/src/entity/entities.ts +++ b/packages/core/src/entity/entities.ts @@ -5,6 +5,7 @@ import { AuthenticationMethod } from './authentication-method/authentication-met import { ExternalAuthenticationMethod } from './authentication-method/external-authentication-method.entity'; import { NativeAuthenticationMethod } from './authentication-method/native-authentication-method.entity'; import { Channel } from './channel/channel.entity'; +import { ChannelRole } from './channel-role/channel-role'; import { CollectionAsset } from './collection/collection-asset.entity'; import { CollectionTranslation } from './collection/collection-translation.entity'; import { Collection } from './collection/collection.entity'; @@ -143,6 +144,7 @@ export const coreEntitiesMap = { TaxCategory, TaxRate, User, + ChannelRole, Seller, Zone, }; diff --git a/packages/core/src/service/helpers/request-context/request-context.service.ts b/packages/core/src/service/helpers/request-context/request-context.service.ts index f87c5216a2..a3b357870d 100644 --- a/packages/core/src/service/helpers/request-context/request-context.service.ts +++ b/packages/core/src/service/helpers/request-context/request-context.service.ts @@ -14,7 +14,6 @@ import { CachedSession, CachedSessionUser } from '../../../config/session-cache/ import { Channel } from '../../../entity/channel/channel.entity'; import { User } from '../../../entity/user/user.entity'; import { ChannelService } from '../../services/channel.service'; -import { getUserChannelsPermissions } from '../utils/get-user-channels-permissions'; /** * @description @@ -58,7 +57,9 @@ export class RequestContextService { } let session: CachedSession | undefined; if (user) { - const channelPermissions = user.roles ? getUserChannelsPermissions(user) : []; + const channelPermissions = user.roles + ? await this.configService.authOptions.rolePermissionResolverStrategy.resolvePermissions(user) + : []; session = { user: { id: user.id, diff --git a/packages/core/src/service/helpers/utils/get-user-channels-permissions.ts b/packages/core/src/service/helpers/utils/get-user-channels-permissions.ts index 748719f8e8..c8727fa170 100644 --- a/packages/core/src/service/helpers/utils/get-user-channels-permissions.ts +++ b/packages/core/src/service/helpers/utils/get-user-channels-permissions.ts @@ -14,6 +14,7 @@ export interface UserChannelPermissions { /** * Returns an array of Channels and permissions on those Channels for the given User. + * @deprecated See `RolePermissionResolverStrategy` */ export function getUserChannelsPermissions(user: User): UserChannelPermissions[] { return getChannelPermissions(user.roles); diff --git a/packages/core/src/service/services/channel-role.service.ts b/packages/core/src/service/services/channel-role.service.ts new file mode 100644 index 0000000000..a52068a600 --- /dev/null +++ b/packages/core/src/service/services/channel-role.service.ts @@ -0,0 +1,16 @@ +import { Injectable } from '@nestjs/common'; + +import { TransactionalConnection } from '../../connection'; + +/** + * @description + * Contains methods relating to {@link ChannelRole} entities. + * + * @todo TODO + * + * @docsCategory services + */ +@Injectable() +export class ChannelRoleService { + constructor(private connection: TransactionalConnection) {} +} diff --git a/packages/core/src/service/services/role.service.ts b/packages/core/src/service/services/role.service.ts index f551de8bdb..f88841ce5b 100644 --- a/packages/core/src/service/services/role.service.ts +++ b/packages/core/src/service/services/role.service.ts @@ -35,10 +35,7 @@ import { User } from '../../entity/user/user.entity'; import { EventBus } from '../../event-bus'; import { RoleEvent } from '../../event-bus/events/role-event'; import { ListQueryBuilder } from '../helpers/list-query-builder/list-query-builder'; -import { - getChannelPermissions, - getUserChannelsPermissions, -} from '../helpers/utils/get-user-channels-permissions'; +import { getChannelPermissions } from '../helpers/utils/get-user-channels-permissions'; import { patchEntity } from '../helpers/utils/patch-entity'; import { ChannelService } from './channel.service'; @@ -222,7 +219,7 @@ export class RoleService { const user = await this.connection.getEntityOrThrow(ctx, User, activeUserId, { relations: ['roles', 'roles.channels'], }); - return getUserChannelsPermissions(user); + return this.configService.authOptions.rolePermissionResolverStrategy.resolvePermissions(user); }, ); diff --git a/packages/core/src/service/services/session.service.ts b/packages/core/src/service/services/session.service.ts index 13c4568329..1c9f1c16f7 100644 --- a/packages/core/src/service/services/session.service.ts +++ b/packages/core/src/service/services/session.service.ts @@ -5,6 +5,7 @@ import ms from 'ms'; import { EntitySubscriberInterface, InsertEvent, RemoveEvent, UpdateEvent } from 'typeorm'; import { RequestContext } from '../../api/common/request-context'; +import { RolePermissionResolverStrategy } from '../../config/auth/role-permission-resolver-strategy'; import { ConfigService } from '../../config/config.service'; import { CachedSession, SessionCacheStrategy } from '../../config/session-cache/session-cache-strategy'; import { TransactionalConnection } from '../../connection/transactional-connection'; @@ -15,7 +16,6 @@ import { AnonymousSession } from '../../entity/session/anonymous-session.entity' import { AuthenticatedSession } from '../../entity/session/authenticated-session.entity'; import { Session } from '../../entity/session/session.entity'; import { User } from '../../entity/user/user.entity'; -import { getUserChannelsPermissions } from '../helpers/utils/get-user-channels-permissions'; import { OrderService } from './order.service'; @@ -28,6 +28,7 @@ import { OrderService } from './order.service'; @Injectable() export class SessionService implements EntitySubscriberInterface { private sessionCacheStrategy: SessionCacheStrategy; + private rolePermissionResolverStrategy: RolePermissionResolverStrategy; private readonly sessionDurationInMs: number; private readonly sessionCacheTimeoutMs = 50; @@ -37,6 +38,7 @@ export class SessionService implements EntitySubscriberInterface { private orderService: OrderService, ) { this.sessionCacheStrategy = this.configService.authOptions.sessionCacheStrategy; + this.rolePermissionResolverStrategy = this.configService.authOptions.rolePermissionResolverStrategy; const { sessionDuration } = this.configService.authOptions; this.sessionDurationInMs = @@ -101,7 +103,9 @@ export class SessionService implements EntitySubscriberInterface { invalidated: false, }), ); - await this.withTimeout(this.sessionCacheStrategy.set(this.serializeSession(authenticatedSession))); + await this.withTimeout( + this.sessionCacheStrategy.set(await this.serializeSession(authenticatedSession)), + ); return authenticatedSession; } @@ -119,7 +123,7 @@ export class SessionService implements EntitySubscriberInterface { }); // save the new session const newSession = await this.connection.rawConnection.getRepository(AnonymousSession).save(session); - const serializedSession = this.serializeSession(newSession); + const serializedSession = await this.serializeSession(newSession); await this.withTimeout(this.sessionCacheStrategy.set(serializedSession)); return serializedSession; } @@ -135,7 +139,7 @@ export class SessionService implements EntitySubscriberInterface { if (!serializedSession || stale || expired) { const session = await this.findSessionByToken(sessionToken); if (session) { - serializedSession = this.serializeSession(session); + serializedSession = await this.serializeSession(session); await this.withTimeout(this.sessionCacheStrategy.set(serializedSession)); return serializedSession; } else { @@ -149,7 +153,7 @@ export class SessionService implements EntitySubscriberInterface { * @description * Serializes a {@link Session} instance into a simplified plain object suitable for caching. */ - serializeSession(session: AuthenticatedSession | AnonymousSession): CachedSession { + async serializeSession(session: AuthenticatedSession | AnonymousSession): Promise { const expiry = Math.floor(new Date().getTime() / 1000) + this.configService.authOptions.sessionCacheTTL; const serializedSession: CachedSession = { @@ -167,7 +171,7 @@ export class SessionService implements EntitySubscriberInterface { id: user.id, identifier: user.identifier, verified: user.verified, - channelPermissions: getUserChannelsPermissions(user), + channelPermissions: await this.rolePermissionResolverStrategy.resolvePermissions(user), }; } return serializedSession; @@ -222,7 +226,7 @@ export class SessionService implements EntitySubscriberInterface { if (session) { session.activeOrder = order; await this.connection.getRepository(ctx, Session).save(session, { reload: false }); - const updatedSerializedSession = this.serializeSession(session); + const updatedSerializedSession = await this.serializeSession(session); await this.withTimeout(this.sessionCacheStrategy.set(updatedSerializedSession)); return updatedSerializedSession; } @@ -242,7 +246,7 @@ export class SessionService implements EntitySubscriberInterface { if (session) { session.activeOrder = null; await this.connection.getRepository(ctx, Session).save(session); - const updatedSerializedSession = this.serializeSession(session); + const updatedSerializedSession = await this.serializeSession(session); await this.configService.authOptions.sessionCacheStrategy.set(updatedSerializedSession); return updatedSerializedSession; } @@ -262,7 +266,7 @@ export class SessionService implements EntitySubscriberInterface { if (session) { session.activeChannel = channel; await this.connection.rawConnection.getRepository(Session).save(session, { reload: false }); - const updatedSerializedSession = this.serializeSession(session); + const updatedSerializedSession = await this.serializeSession(session); await this.withTimeout(this.sessionCacheStrategy.set(updatedSerializedSession)); return updatedSerializedSession; } diff --git a/packages/dev-server/dev-config.ts b/packages/dev-server/dev-config.ts index 68824177de..4bd3436b9b 100644 --- a/packages/dev-server/dev-config.ts +++ b/packages/dev-server/dev-config.ts @@ -9,20 +9,14 @@ import { DefaultSearchPlugin, dummyPaymentHandler, FacetValue, - LanguageCode, LogLevel, VendureConfig, } from '@vendure/core'; -import { ElasticsearchPlugin } from '@vendure/elasticsearch-plugin'; import { defaultEmailHandlers, EmailPlugin, FileBasedTemplateLoader } from '@vendure/email-plugin'; -import { BullMQJobQueuePlugin } from '@vendure/job-queue-plugin/package/bullmq'; import 'dotenv/config'; -import { compileUiExtensions } from '@vendure/ui-devkit/compiler'; import path from 'path'; import { DataSourceOptions } from 'typeorm'; -import { MultivendorPlugin } from './example-plugins/multivendor-plugin/multivendor.plugin'; - /** * Config settings used during development */ @@ -52,6 +46,10 @@ export const devConfig: VendureConfig = { cookieOptions: { secret: 'abc', }, + // TODO THIS DOESNT WORK? TYPE MISMATCH?? + // but it works in default config... huhhhh?? why + // For now if you wanna test, change the default-config.ts + // rolePermissionResolverStrategy: new ChannelRolePermissionResolverStrategy(), }, dbConnectionOptions: { synchronize: false, @@ -79,7 +77,8 @@ export const devConfig: VendureConfig = { }, ], }, - logger: new DefaultLogger({ level: LogLevel.Info }), + // TODO remove if this feature is closer to merging + logger: new DefaultLogger({ level: LogLevel.Debug }), importExportOptions: { importAssetsDir: path.join(__dirname, 'import-assets'), }, From c3b269d17673535fcd97d27916e0c81e64f1bca6 Mon Sep 17 00:00:00 2001 From: Daniel Biegler Date: Thu, 21 Nov 2024 11:30:28 +0100 Subject: [PATCH 02/11] fix(core): Add deletion cascades for channel-roles --- packages/core/src/entity/channel-role/channel-role.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/core/src/entity/channel-role/channel-role.ts b/packages/core/src/entity/channel-role/channel-role.ts index db10d10ced..ae155030ce 100644 --- a/packages/core/src/entity/channel-role/channel-role.ts +++ b/packages/core/src/entity/channel-role/channel-role.ts @@ -23,12 +23,12 @@ export class ChannelRole extends VendureEntity implements HasCustomFields { @Column(type => CustomChannelRoleFields) customFields: CustomChannelRoleFields; - @ManyToOne(type => User) + @ManyToOne(type => User, { onDelete: 'CASCADE' }) user: User; - @ManyToOne(type => Channel) + @ManyToOne(type => Channel, { onDelete: 'CASCADE' }) channel: Channel; - @ManyToOne(type => Role) + @ManyToOne(type => Role, { onDelete: 'CASCADE' }) role: Role; } From 1f8438ca9182723de7f3a708e143a4eb9d38e35a Mon Sep 17 00:00:00 2001 From: Daniel Biegler Date: Thu, 21 Nov 2024 11:38:49 +0100 Subject: [PATCH 03/11] fix(core): Lets not make it a private field just for one use --- packages/core/src/service/services/session.service.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/core/src/service/services/session.service.ts b/packages/core/src/service/services/session.service.ts index 1c9f1c16f7..dbefa88c57 100644 --- a/packages/core/src/service/services/session.service.ts +++ b/packages/core/src/service/services/session.service.ts @@ -5,7 +5,6 @@ import ms from 'ms'; import { EntitySubscriberInterface, InsertEvent, RemoveEvent, UpdateEvent } from 'typeorm'; import { RequestContext } from '../../api/common/request-context'; -import { RolePermissionResolverStrategy } from '../../config/auth/role-permission-resolver-strategy'; import { ConfigService } from '../../config/config.service'; import { CachedSession, SessionCacheStrategy } from '../../config/session-cache/session-cache-strategy'; import { TransactionalConnection } from '../../connection/transactional-connection'; @@ -28,7 +27,6 @@ import { OrderService } from './order.service'; @Injectable() export class SessionService implements EntitySubscriberInterface { private sessionCacheStrategy: SessionCacheStrategy; - private rolePermissionResolverStrategy: RolePermissionResolverStrategy; private readonly sessionDurationInMs: number; private readonly sessionCacheTimeoutMs = 50; @@ -38,7 +36,6 @@ export class SessionService implements EntitySubscriberInterface { private orderService: OrderService, ) { this.sessionCacheStrategy = this.configService.authOptions.sessionCacheStrategy; - this.rolePermissionResolverStrategy = this.configService.authOptions.rolePermissionResolverStrategy; const { sessionDuration } = this.configService.authOptions; this.sessionDurationInMs = @@ -171,7 +168,10 @@ export class SessionService implements EntitySubscriberInterface { id: user.id, identifier: user.identifier, verified: user.verified, - channelPermissions: await this.rolePermissionResolverStrategy.resolvePermissions(user), + channelPermissions: + await this.configService.authOptions.rolePermissionResolverStrategy.resolvePermissions( + user, + ), }; } return serializedSession; From 2ca5f290f8b6db64e86b5061636ca6651497cf50 Mon Sep 17 00:00:00 2001 From: Daniel Biegler Date: Thu, 21 Nov 2024 11:57:32 +0100 Subject: [PATCH 04/11] fix(core): Fix type mismatch by exporting strategies, add to dev config, edit comments --- packages/core/src/config/default-config.ts | 2 -- packages/core/src/config/index.ts | 3 +++ packages/core/src/config/vendure-config.ts | 6 ++++++ packages/dev-server/dev-config.ts | 7 +++---- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/packages/core/src/config/default-config.ts b/packages/core/src/config/default-config.ts index ebb083d45b..654a570d3d 100644 --- a/packages/core/src/config/default-config.ts +++ b/packages/core/src/config/default-config.ts @@ -111,8 +111,6 @@ export const defaultConfig: RuntimeVendureConfig = { passwordHashingStrategy: new BcryptPasswordHashingStrategy(), passwordValidationStrategy: new DefaultPasswordValidationStrategy({ minLength: 4 }), rolePermissionResolverStrategy: new DefaultRolePermissionResolverStrategy(), - // TODO: remove once the weird type mismatch from dev-config gets fixed - // rolePermissionResolverStrategy: new ChannelRolePermissionResolverStrategy(), }, catalogOptions: { collectionFilters: defaultCollectionFilters, diff --git a/packages/core/src/config/index.ts b/packages/core/src/config/index.ts index 52d797a280..0430c99fc4 100644 --- a/packages/core/src/config/index.ts +++ b/packages/core/src/config/index.ts @@ -9,6 +9,9 @@ export * from './auth/default-password-validation-strategy'; export * from './auth/native-authentication-strategy'; export * from './auth/password-hashing-strategy'; export * from './auth/password-validation-strategy'; +export * from './auth/role-permission-resolver-strategy'; +export * from './auth/channel-role-permission-resolver-strategy'; +export * from './auth/default-role-permission-resolver-strategy'; export * from './catalog/collection-filter'; export * from './catalog/default-collection-filters'; export * from './catalog/default-product-variant-price-selection-strategy'; diff --git a/packages/core/src/config/vendure-config.ts b/packages/core/src/config/vendure-config.ts index 0a13f538f3..fddc649032 100644 --- a/packages/core/src/config/vendure-config.ts +++ b/packages/core/src/config/vendure-config.ts @@ -474,6 +474,12 @@ export interface AuthOptions { * @default DefaultPasswordValidationStrategy */ passwordValidationStrategy?: PasswordValidationStrategy; + /** + * @todo TODO + * @description TODO By default, it uses the {@link DefaultRolePermissionResolverStrategy}, which TODO + * @since TODO + * @default DefaultRolePermissionResolverStrategy + */ rolePermissionResolverStrategy?: RolePermissionResolverStrategy; } diff --git a/packages/dev-server/dev-config.ts b/packages/dev-server/dev-config.ts index 4bd3436b9b..a7c1caf674 100644 --- a/packages/dev-server/dev-config.ts +++ b/packages/dev-server/dev-config.ts @@ -4,6 +4,7 @@ import { AssetServerPlugin } from '@vendure/asset-server-plugin'; import { ADMIN_API_PATH, API_PORT, SHOP_API_PATH } from '@vendure/common/lib/shared-constants'; import { Asset, + ChannelRolePermissionResolverStrategy, DefaultJobQueuePlugin, DefaultLogger, DefaultSearchPlugin, @@ -46,10 +47,8 @@ export const devConfig: VendureConfig = { cookieOptions: { secret: 'abc', }, - // TODO THIS DOESNT WORK? TYPE MISMATCH?? - // but it works in default config... huhhhh?? why - // For now if you wanna test, change the default-config.ts - // rolePermissionResolverStrategy: new ChannelRolePermissionResolverStrategy(), + // TODO remove once PR is ready to merge :) + rolePermissionResolverStrategy: new ChannelRolePermissionResolverStrategy(), }, dbConnectionOptions: { synchronize: false, From 30326ba44045d22457f5ffeebd3d0cde773de2a0 Mon Sep 17 00:00:00 2001 From: Daniel Biegler Date: Thu, 21 Nov 2024 13:13:37 +0100 Subject: [PATCH 05/11] fix(core): Rename channel-role file & update imports, rm logs, add wip comments --- .../auth/channel-role-permission-resolver-strategy.ts | 6 +----- .../{channel-role.ts => channel-role.entity.ts} | 0 packages/core/src/entity/entities.ts | 4 ++-- packages/core/src/service/services/role.service.ts | 8 +++++++- 4 files changed, 10 insertions(+), 8 deletions(-) rename packages/core/src/entity/channel-role/{channel-role.ts => channel-role.entity.ts} (100%) diff --git a/packages/core/src/config/auth/channel-role-permission-resolver-strategy.ts b/packages/core/src/config/auth/channel-role-permission-resolver-strategy.ts index c219528bb6..3dac443ed6 100644 --- a/packages/core/src/config/auth/channel-role-permission-resolver-strategy.ts +++ b/packages/core/src/config/auth/channel-role-permission-resolver-strategy.ts @@ -1,7 +1,7 @@ import { Injector } from '../../common'; import { TransactionalConnection } from '../../connection'; import { User } from '../../entity'; -import { ChannelRole } from '../../entity/channel-role/channel-role'; +import { ChannelRole } from '../../entity/channel-role/channel-role.entity'; import { UserChannelPermissions } from '../../service/helpers/utils/get-user-channels-permissions'; import { RolePermissionResolverStrategy } from './role-permission-resolver-strategy'; @@ -14,12 +14,10 @@ export class ChannelRolePermissionResolverStrategy implements RolePermissionReso } async resolvePermissions(user: User): Promise { - console.log('---- BEGIN RESOLVE'); const channelRoleEntries = await this.connection.rawConnection.getRepository(ChannelRole).find({ where: { user: { id: user.id } }, relations: ['user', 'channel', 'role'], }); - console.log('---- RESOLVE -- ENTRIES:', JSON.stringify(channelRoleEntries)); const channelRolePermissions = new Array(channelRoleEntries.length); for (let i = 0; i < channelRoleEntries.length; i++) { @@ -31,9 +29,7 @@ export class ChannelRolePermissionResolverStrategy implements RolePermissionReso }; } channelRoleEntries.sort((a, b) => (a.id < b.id ? -1 : 1)); - console.log('---- RESOLVE -- OUTPUT:', channelRolePermissions); - console.log('---- END RESOLVE'); return channelRolePermissions; } } diff --git a/packages/core/src/entity/channel-role/channel-role.ts b/packages/core/src/entity/channel-role/channel-role.entity.ts similarity index 100% rename from packages/core/src/entity/channel-role/channel-role.ts rename to packages/core/src/entity/channel-role/channel-role.entity.ts diff --git a/packages/core/src/entity/entities.ts b/packages/core/src/entity/entities.ts index 55e903c89e..4f3f6ec666 100644 --- a/packages/core/src/entity/entities.ts +++ b/packages/core/src/entity/entities.ts @@ -5,7 +5,7 @@ import { AuthenticationMethod } from './authentication-method/authentication-met import { ExternalAuthenticationMethod } from './authentication-method/external-authentication-method.entity'; import { NativeAuthenticationMethod } from './authentication-method/native-authentication-method.entity'; import { Channel } from './channel/channel.entity'; -import { ChannelRole } from './channel-role/channel-role'; +import { ChannelRole } from './channel-role/channel-role.entity'; import { CollectionAsset } from './collection/collection-asset.entity'; import { CollectionTranslation } from './collection/collection-translation.entity'; import { Collection } from './collection/collection.entity'; @@ -84,6 +84,7 @@ export const coreEntitiesMap = { AuthenticationMethod, Cancellation, Channel, + ChannelRole, Collection, CollectionAsset, CollectionTranslation, @@ -144,7 +145,6 @@ export const coreEntitiesMap = { TaxCategory, TaxRate, User, - ChannelRole, Seller, Zone, }; diff --git a/packages/core/src/service/services/role.service.ts b/packages/core/src/service/services/role.service.ts index f88841ce5b..d6458f0789 100644 --- a/packages/core/src/service/services/role.service.ts +++ b/packages/core/src/service/services/role.service.ts @@ -274,6 +274,9 @@ export class RoleService { if (targetChannels) { updatedRole.channels = targetChannels; } + + // TODO need to update the channel-role rows here if the strategy demands it + await this.connection.getRepository(ctx, Role).save(updatedRole, { reload: false }); await this.eventBus.publish(new RoleEvent(ctx, role, 'updated', input)); return await assertFound(this.findOne(ctx, role.id)); @@ -433,8 +436,11 @@ export class RoleService { code: input.code, description: input.description, permissions: unique([Permission.Authenticated, ...input.permissions]), + channels, }); - role.channels = channels; + + // TODO need to create channel-role rows here if the strategy demands it + return this.connection.getRepository(ctx, Role).save(role); } From f70a352c26fb77e00b87479686330d4f6a1ff9e5 Mon Sep 17 00:00:00 2001 From: Daniel Biegler Date: Thu, 21 Nov 2024 13:36:22 +0100 Subject: [PATCH 06/11] fix(core): Remove type overwrite, typescript is smart enough to understand the overlap --- packages/core/src/api/resolvers/base/base-auth.resolver.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/core/src/api/resolvers/base/base-auth.resolver.ts b/packages/core/src/api/resolvers/base/base-auth.resolver.ts index 5213c42b1a..0ed34cb704 100644 --- a/packages/core/src/api/resolvers/base/base-auth.resolver.ts +++ b/packages/core/src/api/resolvers/base/base-auth.resolver.ts @@ -5,7 +5,6 @@ import { import { AuthenticationResult as AdminAuthenticationResult, CurrentUser, - CurrentUserChannel, MutationAuthenticateArgs, MutationLoginArgs, Success, @@ -146,9 +145,8 @@ export class BaseAuthResolver { return { id: user.id, identifier: user.identifier, - channels: (await this.configService.authOptions.rolePermissionResolverStrategy.resolvePermissions( - user, - )) as CurrentUserChannel[], + channels: + await this.configService.authOptions.rolePermissionResolverStrategy.resolvePermissions(user), }; } } From d8120cea175aa1a04292f64f3ffdae0a128641a5 Mon Sep 17 00:00:00 2001 From: Daniel Biegler Date: Thu, 21 Nov 2024 23:48:24 +0100 Subject: [PATCH 07/11] fix(core): Add unique constraint - doesnt make sense to have multiple same rows --- packages/core/src/entity/channel-role/channel-role.entity.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/core/src/entity/channel-role/channel-role.entity.ts b/packages/core/src/entity/channel-role/channel-role.entity.ts index ae155030ce..56c2fdd30b 100644 --- a/packages/core/src/entity/channel-role/channel-role.entity.ts +++ b/packages/core/src/entity/channel-role/channel-role.entity.ts @@ -1,5 +1,5 @@ import { DeepPartial } from '@vendure/common/lib/shared-types'; -import { Column, Entity, ManyToOne } from 'typeorm'; +import { Column, Entity, ManyToOne, Unique } from 'typeorm'; import { HasCustomFields } from '../../config'; import { VendureEntity } from '../base/base.entity'; @@ -15,6 +15,7 @@ import { User } from '../user/user.entity'; * @docsCategory entities */ @Entity() +@Unique('UNIQUE_CHANNEL_ROLE_PER_USER', ['user', 'channel', 'role']) export class ChannelRole extends VendureEntity implements HasCustomFields { constructor(input?: DeepPartial) { super(input); From b32642b78165cc3a0d4468ef5ca6857174ee406a Mon Sep 17 00:00:00 2001 From: Daniel Biegler Date: Tue, 26 Nov 2024 13:30:46 +0100 Subject: [PATCH 08/11] fix(core): You can now use the UI with the new `ChannelRolePermissionResolverStrategy` We need UI for the selection of channels but for the POC it will simply assign the default channel. Also moved emitting of events to the end of admin-update function so that a failure from updating custom field relations doesnt lead to wrong behavior of event handlers. --- ...annel-role-permission-resolver-strategy.ts | 62 ++++++++++++++++++- ...fault-role-permission-resolver-strategy.ts | 27 +++++++- .../auth/role-permission-resolver-strategy.ts | 14 +++++ .../service/services/administrator.service.ts | 53 +++++++++++----- 4 files changed, 136 insertions(+), 20 deletions(-) diff --git a/packages/core/src/config/auth/channel-role-permission-resolver-strategy.ts b/packages/core/src/config/auth/channel-role-permission-resolver-strategy.ts index 3dac443ed6..cadac1fad4 100644 --- a/packages/core/src/config/auth/channel-role-permission-resolver-strategy.ts +++ b/packages/core/src/config/auth/channel-role-permission-resolver-strategy.ts @@ -1,6 +1,10 @@ -import { Injector } from '../../common'; +import { DEFAULT_CHANNEL_CODE } from '@vendure/common/lib/shared-constants'; +import { ID } from '@vendure/common/lib/shared-types'; + +import { RequestContext } from '../../api'; +import { EntityNotFoundError, Injector } from '../../common'; import { TransactionalConnection } from '../../connection'; -import { User } from '../../entity'; +import { Channel, Role, User } from '../../entity'; import { ChannelRole } from '../../entity/channel-role/channel-role.entity'; import { UserChannelPermissions } from '../../service/helpers/utils/get-user-channels-permissions'; @@ -8,9 +12,63 @@ import { RolePermissionResolverStrategy } from './role-permission-resolver-strat export class ChannelRolePermissionResolverStrategy implements RolePermissionResolverStrategy { private connection: TransactionalConnection; + private userService: import('../../service/services/user.service').UserService; async init(injector: Injector) { this.connection = injector.get(TransactionalConnection); + this.userService = injector.get((await import('../../service/services/user.service.js')).UserService); + } + + /** + * @description TODO + */ + async persistUserAndTheirRoles(ctx: RequestContext, user: User, roleIds: ID[]): Promise { + const roles = + // Empty array is important because that would return every row when used in the query + roleIds.length === 0 + ? [] + : await this.connection.getRepository(ctx, Role).findBy(roleIds.map(id => ({ id }))); + + // TODO we are relying here on the `roles` relation existing, it could be missing if you query + // user entries without supplying the relations argument, for example: + // this happens when you create a new user via `.save()` because the default reloading doesnt fetch relations + // Q: Should we simply refetch inside her to be more fault tolerant? Could be fixed on the outside too + const currentUser = user.roles ? user : await this.userService.getUserById(ctx, user.id); + if (!currentUser) throw new EntityNotFoundError('User', user.id); + + const rolesAdded = roles.filter(role => !currentUser.roles.some(userRole => userRole.id === role.id)); + const rolesRemoved = currentUser.roles.filter(role => roleIds.indexOf(role.id) === -1); + + // Copy so as to not mutate the original user object when setting roles + const userCopy = new User({ ...currentUser, roles }); + // Lets keep the roles on the user eventhough this strategy technically doesnt need them there + // This makes it possible to switch back to the default strategy without breaking anything + const newUser = await this.connection.getRepository(ctx, User).save(userCopy); + + if (rolesAdded.length > 0) { + // TODO these would come from the new `channelIds` argument from the UI + // For now as proof of concept, lets just always assign the default channel + // Test the permissions by manually creating rows for your channels + const channels = await this.connection + .getRepository(ctx, Channel) + .findBy([{ code: DEFAULT_CHANNEL_CODE }]); + + const newChannelRoleEntries = channels.flatMap(channel => + rolesAdded.map(role => new ChannelRole({ user: newUser, channel, role })), + ); + + await this.connection + .getRepository(ctx, ChannelRole) + .save(newChannelRoleEntries, { reload: false }); + } + + // TODO could be reduced to one query + // potentially improve later once we're happy with the `persistUserAndTheirRoles` level of abstraction + for (const role of rolesRemoved) { + await this.connection + .getRepository(ctx, ChannelRole) + .delete({ role: { id: role.id }, user: { id: user.id } }); + } } async resolvePermissions(user: User): Promise { diff --git a/packages/core/src/config/auth/default-role-permission-resolver-strategy.ts b/packages/core/src/config/auth/default-role-permission-resolver-strategy.ts index ef1cba3f4d..215455817b 100644 --- a/packages/core/src/config/auth/default-role-permission-resolver-strategy.ts +++ b/packages/core/src/config/auth/default-role-permission-resolver-strategy.ts @@ -1,4 +1,9 @@ -import { User } from '../../entity'; +import { ID } from '@vendure/common/lib/shared-types'; + +import { RequestContext } from '../../api'; +import { Injector } from '../../common'; +import { TransactionalConnection } from '../../connection'; +import { Role, User } from '../../entity'; import { getChannelPermissions, UserChannelPermissions, @@ -7,6 +12,26 @@ import { import { RolePermissionResolverStrategy } from './role-permission-resolver-strategy'; export class DefaultRolePermissionResolverStrategy implements RolePermissionResolverStrategy { + private connection: TransactionalConnection; + + async init(injector: Injector) { + this.connection = injector.get(TransactionalConnection); + } + + /** + * @description TODO + */ + async persistUserAndTheirRoles(ctx: RequestContext, user: User, roleIds: ID[]): Promise { + const roles = + // Empty array is important because that would return every row when used in the query + roleIds.length === 0 + ? [] + : await this.connection.getRepository(ctx, Role).findBy(roleIds.map(id => ({ id }))); + // Copy so as to not mutate the original user object when setting roles + const userCopy = new User({ ...user, roles }); + await this.connection.getRepository(ctx, User).save(userCopy, { reload: false }); + } + async resolvePermissions(user: User): Promise { return getChannelPermissions(user.roles); } diff --git a/packages/core/src/config/auth/role-permission-resolver-strategy.ts b/packages/core/src/config/auth/role-permission-resolver-strategy.ts index 44636df234..22991b2b1b 100644 --- a/packages/core/src/config/auth/role-permission-resolver-strategy.ts +++ b/packages/core/src/config/auth/role-permission-resolver-strategy.ts @@ -1,3 +1,6 @@ +import { ID } from '@vendure/common/lib/shared-types'; + +import { RequestContext } from '../../api'; import { InjectableStrategy } from '../../common'; import { User } from '../../entity'; import { UserChannelPermissions } from '../../service/helpers/utils/get-user-channels-permissions'; @@ -6,5 +9,16 @@ import { UserChannelPermissions } from '../../service/helpers/utils/get-user-cha * @description TODO */ export interface RolePermissionResolverStrategy extends InjectableStrategy { + /** + * TODO + */ + persistUserAndTheirRoles( + ctx: RequestContext, + user: User, + /* channelIds: ID[], */ roleIds: ID[], + ): Promise; + /** + * @param user User for which you want to retrieve permissions + */ resolvePermissions(user: User): Promise; } diff --git a/packages/core/src/service/services/administrator.service.ts b/packages/core/src/service/services/administrator.service.ts index 5e084d2ffa..fb5425af8e 100644 --- a/packages/core/src/service/services/administrator.service.ts +++ b/packages/core/src/service/services/administrator.service.ts @@ -132,15 +132,22 @@ export class AdministratorService { let createdAdministrator = await this.connection .getRepository(ctx, Administrator) .save(administrator); - for (const roleId of input.roleIds) { - createdAdministrator = await this.assignRole(ctx, createdAdministrator.id, roleId); - } + + await this.configService.authOptions.rolePermissionResolverStrategy.persistUserAndTheirRoles( + ctx, + createdAdministrator.user, + /* TODO input.channels */ + input.roleIds, + ); + await this.customFieldRelationService.updateRelations( ctx, Administrator, input, createdAdministrator, ); + + createdAdministrator = await assertFound(this.findOne(ctx, createdAdministrator.id)); await this.eventBus.publish(new AdministratorEvent(ctx, createdAdministrator, 'created', input)); return createdAdministrator; } @@ -180,29 +187,41 @@ export class AdministratorService { throw new InternalServerError('error.superadmin-must-have-superadmin-role'); } } - const removeIds = administrator.user.roles - .map(role => role.id) - .filter(roleId => (input.roleIds as ID[]).indexOf(roleId) === -1); - const addIds = (input.roleIds as ID[]).filter( - roleId => !administrator.user.roles.some(role => role.id === roleId), + await this.configService.authOptions.rolePermissionResolverStrategy.persistUserAndTheirRoles( + ctx, + administrator.user, + /* TODO input.channels */ + input.roleIds, ); - - administrator.user.roles = []; - await this.connection.getRepository(ctx, User).save(administrator.user, { reload: false }); - for (const roleId of input.roleIds) { - updatedAdministrator = await this.assignRole(ctx, administrator.id, roleId); - } - await this.eventBus.publish(new RoleChangeEvent(ctx, administrator, addIds, 'assigned')); - await this.eventBus.publish(new RoleChangeEvent(ctx, administrator, removeIds, 'removed')); } + + updatedAdministrator = await assertFound(this.findOne(ctx, administrator.id)); + await this.customFieldRelationService.updateRelations( ctx, Administrator, input, updatedAdministrator, ); - await this.eventBus.publish(new AdministratorEvent(ctx, administrator, 'updated', input)); + + if (input.roleIds) { + const roleIdsAdded = (input.roleIds as ID[]).filter( + roleId => !administrator.user.roles.some(role => role.id === roleId), + ); + + const roleIdsRemoved = administrator.user.roles + .map(role => role.id) + .filter(roleId => (input.roleIds as ID[]).indexOf(roleId) === -1); + + await this.eventBus.publish( + new RoleChangeEvent(ctx, updatedAdministrator, roleIdsAdded, 'assigned'), + ); + await this.eventBus.publish( + new RoleChangeEvent(ctx, updatedAdministrator, roleIdsRemoved, 'removed'), + ); + } + await this.eventBus.publish(new AdministratorEvent(ctx, updatedAdministrator, 'updated', input)); return updatedAdministrator; } From 9fb95b923566cda2e8003f77b50184d4f59c54b0 Mon Sep 17 00:00:00 2001 From: Daniel Biegler Date: Tue, 26 Nov 2024 14:13:12 +0100 Subject: [PATCH 09/11] fix(core): Fix test where administrator service needs to throw for invalid role id --- .../auth/channel-role-permission-resolver-strategy.ts | 4 ++++ .../auth/default-role-permission-resolver-strategy.ts | 6 +++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/core/src/config/auth/channel-role-permission-resolver-strategy.ts b/packages/core/src/config/auth/channel-role-permission-resolver-strategy.ts index cadac1fad4..7c69080043 100644 --- a/packages/core/src/config/auth/channel-role-permission-resolver-strategy.ts +++ b/packages/core/src/config/auth/channel-role-permission-resolver-strategy.ts @@ -28,6 +28,10 @@ export class ChannelRolePermissionResolverStrategy implements RolePermissionReso roleIds.length === 0 ? [] : await this.connection.getRepository(ctx, Role).findBy(roleIds.map(id => ({ id }))); + for (const roleId of roleIds) { + const foundRole = roles.find(role => role.id === roleId); + if (!foundRole) throw new EntityNotFoundError('Role', roleId); + } // TODO we are relying here on the `roles` relation existing, it could be missing if you query // user entries without supplying the relations argument, for example: diff --git a/packages/core/src/config/auth/default-role-permission-resolver-strategy.ts b/packages/core/src/config/auth/default-role-permission-resolver-strategy.ts index 215455817b..10734baea2 100644 --- a/packages/core/src/config/auth/default-role-permission-resolver-strategy.ts +++ b/packages/core/src/config/auth/default-role-permission-resolver-strategy.ts @@ -1,7 +1,7 @@ import { ID } from '@vendure/common/lib/shared-types'; import { RequestContext } from '../../api'; -import { Injector } from '../../common'; +import { EntityNotFoundError, Injector } from '../../common'; import { TransactionalConnection } from '../../connection'; import { Role, User } from '../../entity'; import { @@ -27,6 +27,10 @@ export class DefaultRolePermissionResolverStrategy implements RolePermissionReso roleIds.length === 0 ? [] : await this.connection.getRepository(ctx, Role).findBy(roleIds.map(id => ({ id }))); + for (const roleId of roleIds) { + const foundRole = roles.find(role => role.id === roleId); + if (!foundRole) throw new EntityNotFoundError('Role', roleId); + } // Copy so as to not mutate the original user object when setting roles const userCopy = new User({ ...user, roles }); await this.connection.getRepository(ctx, User).save(userCopy, { reload: false }); From a5a3b661dca0b57fb3af90ba405a432a33010c01 Mon Sep 17 00:00:00 2001 From: Daniel Biegler Date: Sun, 1 Dec 2024 13:58:44 +0100 Subject: [PATCH 10/11] fix(core): Remove obsolete comment --- packages/core/src/service/services/role.service.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/core/src/service/services/role.service.ts b/packages/core/src/service/services/role.service.ts index 24ea21ffbe..2857c33163 100644 --- a/packages/core/src/service/services/role.service.ts +++ b/packages/core/src/service/services/role.service.ts @@ -275,8 +275,6 @@ export class RoleService { role.channels = targetChannels; } - // TODO need to update the channel-role rows here if the strategy demands it - await this.connection.getRepository(ctx, Role).save(role, { reload: false }); const updatedRole = await assertFound(this.findOne(ctx, role.id)); await this.eventBus.publish(new RoleEvent(ctx, updatedRole, 'updated', input)); From 8262d7fc6698764c3f846d4b2bed992cd83d1efb Mon Sep 17 00:00:00 2001 From: Daniel Biegler Date: Sun, 1 Dec 2024 14:04:18 +0100 Subject: [PATCH 11/11] fix(core): Remove obsolete comment --- packages/core/src/service/services/role.service.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/core/src/service/services/role.service.ts b/packages/core/src/service/services/role.service.ts index 2857c33163..59ec6e4568 100644 --- a/packages/core/src/service/services/role.service.ts +++ b/packages/core/src/service/services/role.service.ts @@ -438,8 +438,6 @@ export class RoleService { channels, }); - // TODO need to create channel-role rows here if the strategy demands it - return this.connection.getRepository(ctx, Role).save(role); }