From 7aff75bbd90279c59ba0e9710609cc8d2329f72d Mon Sep 17 00:00:00 2001 From: Roman Nastyuk Date: Fri, 3 Oct 2025 15:42:03 +0300 Subject: [PATCH 1/3] fix(ang-925): fixed addons security issues --- .../project-addons.component.html | 2 +- .../settings-addons.component.html | 4 +-- .../addon-card-list.component.html | 2 +- .../addon-card-list.component.spec.ts | 2 +- .../addon-card-list.component.ts | 2 +- .../addon-card/addon-card.component.html | 4 ++- .../addons/addon-card/addon-card.component.ts | 31 +++++++++++++++++-- 7 files changed, 38 insertions(+), 9 deletions(-) diff --git a/src/app/features/project/project-addons/project-addons.component.html b/src/app/features/project/project-addons/project-addons.component.html index b61ef5235..bf105fc8f 100644 --- a/src/app/features/project/project-addons/project-addons.component.html +++ b/src/app/features/project/project-addons/project-addons.component.html @@ -40,7 +40,7 @@ @if (!isAllAddonsTabLoading()) { - + } @else {
diff --git a/src/app/features/settings/settings-addons/settings-addons.component.html b/src/app/features/settings/settings-addons/settings-addons.component.html index 6e347e7b4..f75878339 100644 --- a/src/app/features/settings/settings-addons/settings-addons.component.html +++ b/src/app/features/settings/settings-addons/settings-addons.component.html @@ -43,7 +43,7 @@ @if (!isAddonsLoading()) { - + } @else {
@@ -62,7 +62,7 @@ /> @if (!isAuthorizedAddonsLoading()) { - + } @else {
diff --git a/src/app/shared/components/addons/addon-card-list/addon-card-list.component.html b/src/app/shared/components/addons/addon-card-list/addon-card-list.component.html index ff6826ec2..6c46e5d4e 100644 --- a/src/app/shared/components/addons/addon-card-list/addon-card-list.component.html +++ b/src/app/shared/components/addons/addon-card-list/addon-card-list.component.html @@ -2,7 +2,7 @@ @if (cards().length) { @for (card of cards(); track $index) {
- +
} } @else { diff --git a/src/app/shared/components/addons/addon-card-list/addon-card-list.component.spec.ts b/src/app/shared/components/addons/addon-card-list/addon-card-list.component.spec.ts index 8b64c144d..a0a08d1ea 100644 --- a/src/app/shared/components/addons/addon-card-list/addon-card-list.component.spec.ts +++ b/src/app/shared/components/addons/addon-card-list/addon-card-list.component.spec.ts @@ -28,6 +28,6 @@ describe('AddonCardListComponent', () => { }); it('should have default false value', () => { - expect(component.showDangerButton()).toBe(false); + expect(component.isConnected()).toBe(false); }); }); diff --git a/src/app/shared/components/addons/addon-card-list/addon-card-list.component.ts b/src/app/shared/components/addons/addon-card-list/addon-card-list.component.ts index 757788b99..c24567b46 100644 --- a/src/app/shared/components/addons/addon-card-list/addon-card-list.component.ts +++ b/src/app/shared/components/addons/addon-card-list/addon-card-list.component.ts @@ -14,5 +14,5 @@ import { AddonCardComponent } from '../addon-card/addon-card.component'; }) export class AddonCardListComponent { cards = input<(AddonModel | AuthorizedAccountModel | ConfiguredAddonModel | AddonCardModel)[]>([]); - showDangerButton = input(false); + isConnected = input(false); } diff --git a/src/app/shared/components/addons/addon-card/addon-card.component.html b/src/app/shared/components/addons/addon-card/addon-card.component.html index 3536dbd6c..51fd2c4ac 100644 --- a/src/app/shared/components/addons/addon-card/addon-card.component.html +++ b/src/app/shared/components/addons/addon-card/addon-card.component.html @@ -14,7 +14,7 @@

{{ actualAddon()?.displayName }}

- @if (showDangerButton()) { + @if (isConnected()) { {{ actualAddon()?.displayName
diff --git a/src/app/shared/components/addons/addon-card/addon-card.component.ts b/src/app/shared/components/addons/addon-card/addon-card.component.ts index 2f72405b7..d37eb4f86 100644 --- a/src/app/shared/components/addons/addon-card/addon-card.component.ts +++ b/src/app/shared/components/addons/addon-card/addon-card.component.ts @@ -25,7 +25,7 @@ export class AddonCardComponent { private readonly actions = createDispatchMap({ deleteAuthorizedAddon: DeleteAuthorizedAddon }); readonly card = input(null); - readonly showDangerButton = input(false); + readonly isConnected = input(false); readonly actualAddon = computed(() => { const actualCard = this.card(); @@ -50,8 +50,35 @@ export class AddonCardComponent { return isConfiguredAddon(actualCard); }); + readonly canConfigure = computed(() => { + const isConfigured = this.isConfiguredAddon(); + + if (!isConfigured) return true; + + const addon = this.card(); + + if (!addon) return true; + + if ('configuredAddon' in addon && addon.configuredAddon) { + return addon.configuredAddon.currentUserIsOwner; + } + + if ('currentUserIsOwner' in addon) { + return addon.currentUserIsOwner; + } + + return true; + }); + readonly buttonLabel = computed(() => { - return this.isConfiguredAddon() ? 'settings.addons.form.buttons.configure' : 'settings.addons.form.buttons.connect'; + const isConfigured = this.isConfiguredAddon(); + const isConnected = this.isConnected(); + + if (isConfigured) { + return 'settings.addons.form.buttons.configure'; + } + + return isConnected ? 'settings.addons.form.buttons.reconnect' : 'settings.addons.form.buttons.connect'; }); onConnectAddon(): void { From 2d1d689d010afdc1cb6ec369219af4b1e2fbd69b Mon Sep 17 00:00:00 2001 From: Roman Nastyuk Date: Fri, 3 Oct 2025 16:31:09 +0300 Subject: [PATCH 2/3] fix(ang-925): fixed permissions issue for addons --- .../configure-addon.component.html | 14 ++++++++------ .../configure-addon/configure-addon.component.ts | 1 + .../project-addons/project-addons.component.html | 12 ++++++++++-- .../project-addons/project-addons.component.ts | 2 ++ .../settings-addons.component.html | 4 ++-- .../addon-card-list.component.html | 2 +- .../addon-card-list/addon-card-list.component.ts | 1 + .../addons/addon-card/addon-card.component.ts | 16 +++------------- 8 files changed, 28 insertions(+), 24 deletions(-) diff --git a/src/app/features/project/project-addons/components/configure-addon/configure-addon.component.html b/src/app/features/project/project-addons/components/configure-addon/configure-addon.component.html index 977fdcf37..196a27a5e 100644 --- a/src/app/features/project/project-addons/components/configure-addon/configure-addon.component.html +++ b/src/app/features/project/project-addons/components/configure-addon/configure-addon.component.html @@ -24,12 +24,14 @@

- + @if (canRename()) { + + } (null); readonly isGoogleDrive = computed(() => this.storageAddon()?.wbKey === 'googledrive'); + readonly canRename = computed(() => this.addon()?.currentUserIsOwner ?? false); isEditMode = signal(false); selectedStorageItemId = signal(''); diff --git a/src/app/features/project/project-addons/project-addons.component.html b/src/app/features/project/project-addons/project-addons.component.html index bf105fc8f..a318e1f44 100644 --- a/src/app/features/project/project-addons/project-addons.component.html +++ b/src/app/features/project/project-addons/project-addons.component.html @@ -40,7 +40,11 @@
@if (!isAllAddonsTabLoading()) { - + } @else {
@@ -58,7 +62,11 @@ /> @if (!isConfiguredAddonsLoading()) { - + } @else {
diff --git a/src/app/features/project/project-addons/project-addons.component.ts b/src/app/features/project/project-addons/project-addons.component.ts index 1c836492d..afef047ac 100644 --- a/src/app/features/project/project-addons/project-addons.component.ts +++ b/src/app/features/project/project-addons/project-addons.component.ts @@ -41,6 +41,7 @@ import { GetLinkAddons, GetStorageAddons, } from '@shared/stores/addons'; +import { CurrentResourceSelectors } from '@shared/stores/current-resource'; @Component({ selector: 'osf-project-addons', @@ -75,6 +76,7 @@ export class ProjectAddonsComponent implements OnInit { selectedTab = signal(this.defaultTabValue); currentUser = select(UserSelectors.getCurrentUser); + hasAdminAccess = select(CurrentResourceSelectors.hasAdminAccess); addonsResourceReference = select(AddonsSelectors.getAddonsResourceReference); addonsUserReference = select(AddonsSelectors.getAddonsUserReference); storageAddons = select(AddonsSelectors.getStorageAddons); diff --git a/src/app/features/settings/settings-addons/settings-addons.component.html b/src/app/features/settings/settings-addons/settings-addons.component.html index f75878339..fe79a2592 100644 --- a/src/app/features/settings/settings-addons/settings-addons.component.html +++ b/src/app/features/settings/settings-addons/settings-addons.component.html @@ -43,7 +43,7 @@ @if (!isAddonsLoading()) { - + } @else {
@@ -62,7 +62,7 @@ /> @if (!isAuthorizedAddonsLoading()) { - + } @else {
diff --git a/src/app/shared/components/addons/addon-card-list/addon-card-list.component.html b/src/app/shared/components/addons/addon-card-list/addon-card-list.component.html index 6c46e5d4e..a9b81d562 100644 --- a/src/app/shared/components/addons/addon-card-list/addon-card-list.component.html +++ b/src/app/shared/components/addons/addon-card-list/addon-card-list.component.html @@ -2,7 +2,7 @@ @if (cards().length) { @for (card of cards(); track $index) {
- +
} } @else { diff --git a/src/app/shared/components/addons/addon-card-list/addon-card-list.component.ts b/src/app/shared/components/addons/addon-card-list/addon-card-list.component.ts index c24567b46..546b49fd2 100644 --- a/src/app/shared/components/addons/addon-card-list/addon-card-list.component.ts +++ b/src/app/shared/components/addons/addon-card-list/addon-card-list.component.ts @@ -15,4 +15,5 @@ import { AddonCardComponent } from '../addon-card/addon-card.component'; export class AddonCardListComponent { cards = input<(AddonModel | AuthorizedAccountModel | ConfiguredAddonModel | AddonCardModel)[]>([]); isConnected = input(false); + hasAdminAccess = input(false); } diff --git a/src/app/shared/components/addons/addon-card/addon-card.component.ts b/src/app/shared/components/addons/addon-card/addon-card.component.ts index d37eb4f86..44aaf058c 100644 --- a/src/app/shared/components/addons/addon-card/addon-card.component.ts +++ b/src/app/shared/components/addons/addon-card/addon-card.component.ts @@ -26,6 +26,7 @@ export class AddonCardComponent { readonly card = input(null); readonly isConnected = input(false); + readonly hasAdminAccess = input(false); readonly actualAddon = computed(() => { const actualCard = this.card(); @@ -52,22 +53,11 @@ export class AddonCardComponent { readonly canConfigure = computed(() => { const isConfigured = this.isConfiguredAddon(); + const hasAdmin = this.hasAdminAccess(); if (!isConfigured) return true; - const addon = this.card(); - - if (!addon) return true; - - if ('configuredAddon' in addon && addon.configuredAddon) { - return addon.configuredAddon.currentUserIsOwner; - } - - if ('currentUserIsOwner' in addon) { - return addon.currentUserIsOwner; - } - - return true; + return hasAdmin; }); readonly buttonLabel = computed(() => { From eab4da666c96d111e6d3a3db5a165cca232ef2ce Mon Sep 17 00:00:00 2001 From: Roman Nastyuk Date: Fri, 3 Oct 2025 17:06:22 +0300 Subject: [PATCH 3/3] fix(ang-923/924): fixed citation addons connection issue --- .../settings-addons.component.ts | 4 +- .../addons/addon-card/addon-card.component.ts | 12 +++++- src/app/shared/enums/operation-names.enum.ts | 2 + .../services/addons/addon-form.service.ts | 10 ++++- .../addon-operation-invocation.service.ts | 37 ++++++++++++++++--- 5 files changed, 55 insertions(+), 10 deletions(-) diff --git a/src/app/features/settings/settings-addons/settings-addons.component.ts b/src/app/features/settings/settings-addons/settings-addons.component.ts index da34cd0fc..da2a68440 100644 --- a/src/app/features/settings/settings-addons/settings-addons.component.ts +++ b/src/app/features/settings/settings-addons/settings-addons.component.ts @@ -161,11 +161,13 @@ export class SettingsAddonsComponent { readonly filteredAddonCards = computed(() => { const searchValue = this.searchValue().toLowerCase(); - return this.currentAddonsState().filter( + const filteredAddons = this.currentAddonsState().filter( (card) => card.externalServiceName.toLowerCase().includes(searchValue) || card.displayName.toLowerCase().includes(searchValue) ); + + return sortAddonCardsAlphabetically(filteredAddons); }); onCategoryChange(value: Primitive): void { diff --git a/src/app/shared/components/addons/addon-card/addon-card.component.ts b/src/app/shared/components/addons/addon-card/addon-card.component.ts index 44aaf058c..7c1ef612d 100644 --- a/src/app/shared/components/addons/addon-card/addon-card.component.ts +++ b/src/app/shared/components/addons/addon-card/addon-card.component.ts @@ -57,7 +57,17 @@ export class AddonCardComponent { if (!isConfigured) return true; - return hasAdmin; + const addon = this.card(); + if (!addon) return true; + + let isOwner = false; + if ('configuredAddon' in addon && addon.configuredAddon) { + isOwner = addon.configuredAddon.currentUserIsOwner; + } else if ('currentUserIsOwner' in addon) { + isOwner = addon.currentUserIsOwner; + } + + return hasAdmin || isOwner; }); readonly buttonLabel = computed(() => { diff --git a/src/app/shared/enums/operation-names.enum.ts b/src/app/shared/enums/operation-names.enum.ts index db58bb61b..ab725cfc8 100644 --- a/src/app/shared/enums/operation-names.enum.ts +++ b/src/app/shared/enums/operation-names.enum.ts @@ -1,6 +1,8 @@ export enum OperationNames { LIST_ROOT_ITEMS = 'list_root_items', LIST_CHILD_ITEMS = 'list_child_items', + LIST_ROOT_COLLECTIONS = 'list_root_collections', + LIST_COLLECTION_ITEMS = 'list_collection_items', GET_ITEM_INFO = 'get_item_info', HAS_REVISIONS = 'has_revisions', } diff --git a/src/app/shared/services/addons/addon-form.service.ts b/src/app/shared/services/addons/addon-form.service.ts index 33a534f1c..9f164b626 100644 --- a/src/app/shared/services/addons/addon-form.service.ts +++ b/src/app/shared/services/addons/addon-form.service.ts @@ -113,6 +113,12 @@ export class AddonFormService { : (addon as AddonModel).id; } + private getOperationNames(addonTypeString: string): string[] { + return addonTypeString === AddonType.CITATION + ? ['list_collection_items', 'list_root_collections', 'get_item_info'] + : ['list_child_items', 'list_root_items', 'get_item_info']; + } + generateConfiguredAddonCreatePayload( addon: AddonModel | AuthorizedAccountModel, selectedAccount: AuthorizedAccountModel, @@ -132,7 +138,7 @@ export class AddonFormService { display_name: displayName, ...(addonTypeString !== AddonType.LINK && { root_folder: selectedStorageItemId }), connected_capabilities: ['UPDATE', 'ACCESS'], - connected_operation_names: ['list_child_items', 'list_root_items', 'get_item_info'], + connected_operation_names: this.getOperationNames(addonTypeString), external_service_name: addon.externalServiceName, ...(resourceType && { resource_type: resourceType }), ...(addonTypeString === AddonType.LINK && { target_id: selectedStorageItemId }), @@ -180,7 +186,7 @@ export class AddonFormService { authorized_resource_uri: resourceUri, display_name: displayName, connected_capabilities: ['UPDATE', 'ACCESS'], - connected_operation_names: ['list_child_items', 'list_root_items', 'get_item_info'], + connected_operation_names: this.getOperationNames(addonTypeString), external_service_name: addon.externalServiceName, ...(resourceType && { resource_type: resourceType }), ...(addonTypeString !== AddonType.LINK && { root_folder: selectedStorageItemId }), diff --git a/src/app/shared/services/addons/addon-operation-invocation.service.ts b/src/app/shared/services/addons/addon-operation-invocation.service.ts index c2fc0bec9..15526dce9 100644 --- a/src/app/shared/services/addons/addon-operation-invocation.service.ts +++ b/src/app/shared/services/addons/addon-operation-invocation.service.ts @@ -1,25 +1,43 @@ import { Injectable } from '@angular/core'; import { OperationNames } from '@osf/shared/enums'; +import { isCitationAddon } from '@osf/shared/helpers'; import { AuthorizedAccountModel, ConfiguredAddonModel, OperationInvocationRequestJsonApi } from '@shared/models'; @Injectable({ providedIn: 'root', }) export class AddonOperationInvocationService { + private getAddonSpecificOperationName( + operationName: OperationNames, + addon: AuthorizedAccountModel | ConfiguredAddonModel + ): OperationNames { + if (!isCitationAddon(addon)) { + return operationName; + } + + const operationMap: Record = { + [OperationNames.LIST_ROOT_ITEMS]: OperationNames.LIST_ROOT_COLLECTIONS, + [OperationNames.LIST_CHILD_ITEMS]: OperationNames.LIST_COLLECTION_ITEMS, + }; + + return operationMap[operationName] ?? operationName; + } + createInitialOperationInvocationPayload( operationName: OperationNames, selectedAccount: AuthorizedAccountModel, itemId?: string ): OperationInvocationRequestJsonApi { - const operationKwargs = this.getOperationKwargs(operationName, itemId); + const addonSpecificOperationName = this.getAddonSpecificOperationName(operationName, selectedAccount); + const operationKwargs = this.getOperationKwargs(addonSpecificOperationName, itemId); return { data: { type: 'addon-operation-invocations', attributes: { invocation_status: null, - operation_name: operationName, + operation_name: addonSpecificOperationName, operation_kwargs: operationKwargs, operation_result: {}, created: null, @@ -42,14 +60,15 @@ export class AddonOperationInvocationService { operationName: OperationNames, itemId: string ): OperationInvocationRequestJsonApi { - const operationKwargs = this.getOperationKwargs(operationName, itemId); + const addonSpecificOperationName = this.getAddonSpecificOperationName(operationName, addon); + const operationKwargs = this.getOperationKwargs(addonSpecificOperationName, itemId); return { data: { type: 'addon-operation-invocations', attributes: { invocation_status: null, - operation_name: operationName, + operation_name: addonSpecificOperationName, operation_kwargs: operationKwargs, operation_result: {}, created: null, @@ -68,13 +87,19 @@ export class AddonOperationInvocationService { } private getOperationKwargs(operationName: OperationNames, itemId?: string): Record { - if (!itemId || operationName === OperationNames.LIST_ROOT_ITEMS) { + const isRootOperation = + operationName === OperationNames.LIST_ROOT_ITEMS || operationName === OperationNames.LIST_ROOT_COLLECTIONS; + + if (!itemId || isRootOperation) { return {}; } + const isChildOperation = + operationName === OperationNames.LIST_CHILD_ITEMS || operationName === OperationNames.LIST_COLLECTION_ITEMS; + return { item_id: itemId, - ...(operationName === OperationNames.LIST_CHILD_ITEMS && { item_type: 'FOLDER' }), + ...(isChildOperation && { item_type: 'FOLDER' }), }; } }