diff --git a/packages/server/modules/accessrequests/graph/resolvers/index.ts b/packages/server/modules/accessrequests/graph/resolvers/index.ts index 35c2fe463..ef23446f5 100644 --- a/packages/server/modules/accessrequests/graph/resolvers/index.ts +++ b/packages/server/modules/accessrequests/graph/resolvers/index.ts @@ -17,15 +17,26 @@ import { requestProjectAccessFactory, requestStreamAccessFactory } from '@/modules/accessrequests/services/stream' +import { saveActivityFactory } from '@/modules/activitystream/repositories' +import { + addStreamInviteAcceptedActivityFactory, + addStreamPermissionsAddedActivityFactory +} from '@/modules/activitystream/services/streamActivity' import { Resolvers } from '@/modules/core/graph/generated/graphql' import { mapStreamRoleToValue } from '@/modules/core/helpers/graphTypes' import { Roles } from '@/modules/core/helpers/mainConstants' -import { getStreamFactory } from '@/modules/core/repositories/streams' import { - addOrUpdateStreamCollaborator, - validateStreamAccess -} from '@/modules/core/services/streams/streamAccessService' + getStreamFactory, + grantStreamPermissionsFactory +} from '@/modules/core/repositories/streams' +import { getUser } from '@/modules/core/repositories/users' +import { + addOrUpdateStreamCollaboratorFactory, + validateStreamAccessFactory +} from '@/modules/core/services/streams/access' +import { authorizeResolver } from '@/modules/shared' import { LogicError } from '@/modules/shared/errors' +import { publish } from '@/modules/shared/utils/subscriptions' const getStream = getStreamFactory({ db }) const getUserProjectAccessRequest = getUserProjectAccessRequestFactory({ @@ -55,6 +66,24 @@ const getPendingStreamRequests = getPendingStreamRequestsFactory({ getPendingProjectRequests }) +const saveActivity = saveActivityFactory({ db }) +const validateStreamAccess = validateStreamAccessFactory({ + authorizeResolver +}) +const addOrUpdateStreamCollaborator = addOrUpdateStreamCollaboratorFactory({ + validateStreamAccess, + getUser, + grantStreamPermissions: grantStreamPermissionsFactory({ db }), + addStreamInviteAcceptedActivity: addStreamInviteAcceptedActivityFactory({ + saveActivity, + publish + }), + addStreamPermissionsAddedActivity: addStreamPermissionsAddedActivityFactory({ + saveActivity, + publish + }) +}) + const processPendingStreamRequest = processPendingStreamRequestFactory({ getPendingAccessRequest: getPendingAccessRequestFactory({ db }), validateStreamAccess, diff --git a/packages/server/modules/accessrequests/services/stream.ts b/packages/server/modules/accessrequests/services/stream.ts index d0dcb835b..10c9a3759 100644 --- a/packages/server/modules/accessrequests/services/stream.ts +++ b/packages/server/modules/accessrequests/services/stream.ts @@ -13,10 +13,6 @@ import { import { StreamInvalidAccessError } from '@/modules/core/errors/stream' import { TokenResourceIdentifier } from '@/modules/core/domain/tokens/types' import { Roles, StreamRoles } from '@/modules/core/helpers/mainConstants' -import { - addOrUpdateStreamCollaborator, - validateStreamAccess -} from '@/modules/core/services/streams/streamAccessService' import { ensureError } from '@/modules/shared/helpers/errorHelper' import { MaybeNullOrUndefined, @@ -34,7 +30,11 @@ import { GetUserStreamAccessRequest, RequestProjectAccess } from '@/modules/accessrequests/domain/operations' -import { GetStream } from '@/modules/core/domain/streams/operations' +import { + AddOrUpdateStreamCollaborator, + GetStream, + ValidateStreamAccess +} from '@/modules/core/domain/streams/operations' function buildStreamAccessRequestGraphQLReturn( record: ServerAccessRequestRecord @@ -165,8 +165,8 @@ export const getPendingStreamRequestsFactory = export const processPendingStreamRequestFactory = (deps: { getPendingAccessRequest: GetPendingAccessRequest - validateStreamAccess: typeof validateStreamAccess - addOrUpdateStreamCollaborator: typeof addOrUpdateStreamCollaborator + validateStreamAccess: ValidateStreamAccess + addOrUpdateStreamCollaborator: AddOrUpdateStreamCollaborator deleteRequestById: DeleteRequestById accessRequestsEmitter: (typeof AccessRequestsEmitter)['emit'] }) => diff --git a/packages/server/modules/accessrequests/tests/projectAccessRequests.spec.ts b/packages/server/modules/accessrequests/tests/projectAccessRequests.spec.ts index 225b5641d..0371b7ebd 100644 --- a/packages/server/modules/accessrequests/tests/projectAccessRequests.spec.ts +++ b/packages/server/modules/accessrequests/tests/projectAccessRequests.spec.ts @@ -12,6 +12,12 @@ import { requestProjectAccessFactory } from '@/modules/accessrequests/services/stream' import { ActionTypes } from '@/modules/activitystream/helpers/types' +import { saveActivityFactory } from '@/modules/activitystream/repositories' +import { + addStreamInviteAcceptedActivityFactory, + addStreamPermissionsAddedActivityFactory, + addStreamPermissionsRevokedActivityFactory +} from '@/modules/activitystream/services/streamActivity' import { ServerAccessRequests, StreamActivity, @@ -23,13 +29,20 @@ import { mapStreamRoleToValue } from '@/modules/core/helpers/graphTypes' import { Roles } from '@/modules/core/helpers/mainConstants' import { getStreamCollaboratorsFactory, - getStreamFactory + getStreamFactory, + grantStreamPermissionsFactory, + revokeStreamPermissionsFactory } from '@/modules/core/repositories/streams' +import { getUser } from '@/modules/core/repositories/users' import { - addOrUpdateStreamCollaborator, - removeStreamCollaborator -} from '@/modules/core/services/streams/streamAccessService' + addOrUpdateStreamCollaboratorFactory, + isStreamCollaboratorFactory, + removeStreamCollaboratorFactory, + validateStreamAccessFactory +} from '@/modules/core/services/streams/access' import { NotificationType } from '@/modules/notifications/helpers/types' +import { authorizeResolver } from '@/modules/shared' +import { publish } from '@/modules/shared/utils/subscriptions' import { BasicTestUser, createTestUsers } from '@/test/authHelper' import { CreateProjectAccessRequestDocument, @@ -63,6 +76,34 @@ const requestProjectAccess = requestProjectAccessFactory({ createNewRequest: createNewRequestFactory({ db }), accessRequestsEmitter: AccessRequestsEmitter.emit }) +const saveActivity = saveActivityFactory({ db }) +const validateStreamAccess = validateStreamAccessFactory({ authorizeResolver }) +const isStreamCollaborator = isStreamCollaboratorFactory({ + getStream +}) +const removeStreamCollaborator = removeStreamCollaboratorFactory({ + validateStreamAccess, + isStreamCollaborator, + revokeStreamPermissions: revokeStreamPermissionsFactory({ db }), + addStreamPermissionsRevokedActivity: addStreamPermissionsRevokedActivityFactory({ + saveActivity, + publish + }) +}) + +const addOrUpdateStreamCollaborator = addOrUpdateStreamCollaboratorFactory({ + validateStreamAccess, + getUser, + grantStreamPermissions: grantStreamPermissionsFactory({ db }), + addStreamInviteAcceptedActivity: addStreamInviteAcceptedActivityFactory({ + saveActivity, + publish + }), + addStreamPermissionsAddedActivity: addStreamPermissionsAddedActivityFactory({ + saveActivity, + publish + }) +}) const isNotCollaboratorError = (e: unknown) => e instanceof StreamAccessUpdateError && diff --git a/packages/server/modules/accessrequests/tests/streamAccessRequests.spec.ts b/packages/server/modules/accessrequests/tests/streamAccessRequests.spec.ts index 60a7d829e..43962701a 100644 --- a/packages/server/modules/accessrequests/tests/streamAccessRequests.spec.ts +++ b/packages/server/modules/accessrequests/tests/streamAccessRequests.spec.ts @@ -14,6 +14,12 @@ import { requestStreamAccessFactory } from '@/modules/accessrequests/services/stream' import { ActionTypes } from '@/modules/activitystream/helpers/types' +import { saveActivityFactory } from '@/modules/activitystream/repositories' +import { + addStreamInviteAcceptedActivityFactory, + addStreamPermissionsAddedActivityFactory, + addStreamPermissionsRevokedActivityFactory +} from '@/modules/activitystream/services/streamActivity' import { ServerAccessRequests, StreamActivity, @@ -25,13 +31,20 @@ import { mapStreamRoleToValue } from '@/modules/core/helpers/graphTypes' import { Roles } from '@/modules/core/helpers/mainConstants' import { getStreamCollaboratorsFactory, - getStreamFactory + getStreamFactory, + grantStreamPermissionsFactory, + revokeStreamPermissionsFactory } from '@/modules/core/repositories/streams' +import { getUser } from '@/modules/core/repositories/users' import { - addOrUpdateStreamCollaborator, - removeStreamCollaborator -} from '@/modules/core/services/streams/streamAccessService' + addOrUpdateStreamCollaboratorFactory, + isStreamCollaboratorFactory, + removeStreamCollaboratorFactory, + validateStreamAccessFactory +} from '@/modules/core/services/streams/access' import { NotificationType } from '@/modules/notifications/helpers/types' +import { authorizeResolver } from '@/modules/shared' +import { publish } from '@/modules/shared/utils/subscriptions' import { BasicTestUser, createTestUsers } from '@/test/authHelper' import { createStreamAccessRequest, @@ -67,6 +80,34 @@ const requestStreamAccess = requestStreamAccessFactory({ accessRequestsEmitter: AccessRequestsEmitter.emit }) }) +const saveActivity = saveActivityFactory({ db }) +const validateStreamAccess = validateStreamAccessFactory({ authorizeResolver }) +const isStreamCollaborator = isStreamCollaboratorFactory({ + getStream +}) +const removeStreamCollaborator = removeStreamCollaboratorFactory({ + validateStreamAccess, + isStreamCollaborator, + revokeStreamPermissions: revokeStreamPermissionsFactory({ db }), + addStreamPermissionsRevokedActivity: addStreamPermissionsRevokedActivityFactory({ + saveActivity, + publish + }) +}) + +const addOrUpdateStreamCollaborator = addOrUpdateStreamCollaboratorFactory({ + validateStreamAccess, + getUser, + grantStreamPermissions: grantStreamPermissionsFactory({ db }), + addStreamInviteAcceptedActivity: addStreamInviteAcceptedActivityFactory({ + saveActivity, + publish + }), + addStreamPermissionsAddedActivity: addStreamPermissionsAddedActivityFactory({ + saveActivity, + publish + }) +}) const isNotCollaboratorError = (e: unknown) => e instanceof StreamAccessUpdateError && diff --git a/packages/server/modules/activitystream/tests/activity.spec.js b/packages/server/modules/activitystream/tests/activity.spec.js index d7760ad88..9e0c7425d 100644 --- a/packages/server/modules/activitystream/tests/activity.spec.js +++ b/packages/server/modules/activitystream/tests/activity.spec.js @@ -7,14 +7,42 @@ const { createObject } = require('../../core/services/objects') const { beforeEachContext, initializeTestServer } = require('@/test/hooks') const { noErrors } = require('@/test/helpers') -const { - addOrUpdateStreamCollaborator -} = require('@/modules/core/services/streams/streamAccessService') + const { Roles, Scopes } = require('@speckle/shared') -const { getUserActivityFactory } = require('@/modules/activitystream/repositories') +const { + getUserActivityFactory, + saveActivityFactory +} = require('@/modules/activitystream/repositories') const { db } = require('@/db/knex') +const { + validateStreamAccessFactory, + addOrUpdateStreamCollaboratorFactory +} = require('@/modules/core/services/streams/access') +const { authorizeResolver } = require('@/modules/shared') +const { getUser } = require('@/modules/core/repositories/users') +const { grantStreamPermissionsFactory } = require('@/modules/core/repositories/streams') +const { + addStreamInviteAcceptedActivityFactory, + addStreamPermissionsAddedActivityFactory +} = require('@/modules/activitystream/services/streamActivity') +const { publish } = require('@/modules/shared/utils/subscriptions') const getUserActivity = getUserActivityFactory({ db }) +const saveActivity = saveActivityFactory({ db }) +const validateStreamAccess = validateStreamAccessFactory({ authorizeResolver }) +const addOrUpdateStreamCollaborator = addOrUpdateStreamCollaboratorFactory({ + validateStreamAccess, + getUser, + grantStreamPermissions: grantStreamPermissionsFactory({ db }), + addStreamInviteAcceptedActivity: addStreamInviteAcceptedActivityFactory({ + saveActivity, + publish + }), + addStreamPermissionsAddedActivity: addStreamPermissionsAddedActivityFactory({ + saveActivity, + publish + }) +}) let sendRequest diff --git a/packages/server/modules/automate/graph/resolvers/automate.ts b/packages/server/modules/automate/graph/resolvers/automate.ts index ad986d0bd..198fad66b 100644 --- a/packages/server/modules/automate/graph/resolvers/automate.ts +++ b/packages/server/modules/automate/graph/resolvers/automate.ts @@ -55,7 +55,6 @@ import { import { getGenericRedis } from '@/modules/core/index' import { getUser } from '@/modules/core/repositories/users' import { createAutomation as clientCreateAutomation } from '@/modules/automate/clients/executionEngine' -import { validateStreamAccess } from '@/modules/core/services/streams/streamAccessService' import { Automate, Roles, isNullOrUndefined, isNonNullable } from '@speckle/shared' import { getFeatureFlags, getServerOrigin } from '@/modules/shared/helpers/envHelper' import { @@ -110,6 +109,7 @@ import { AutomationsEmitter } from '@/modules/automate/events/automations' import { AutomateRunsEmitter } from '@/modules/automate/events/runs' import { createAppToken } from '@/modules/core/services/tokens' import { getCommitFactory } from '@/modules/core/repositories/commits' +import { validateStreamAccessFactory } from '@/modules/core/services/streams/access' const { FF_AUTOMATE_MODULE_ENABLED } = getFeatureFlags() @@ -136,6 +136,7 @@ const getAutomationRunsItems = getAutomationRunsItemsFactory({ db }) const getProjectAutomationsItems = getProjectAutomationsItemsFactory({ db }) const getProjectAutomationsTotalCount = getProjectAutomationsTotalCountFactory({ db }) const getBranchLatestCommits = getBranchLatestCommitsFactory({ db }) +const validateStreamAccess = validateStreamAccessFactory({ authorizeResolver }) export = (FF_AUTOMATE_MODULE_ENABLED ? { diff --git a/packages/server/modules/automate/services/automationManagement.ts b/packages/server/modules/automate/services/automationManagement.ts index 1b8c105f2..70e977dbf 100644 --- a/packages/server/modules/automate/services/automationManagement.ts +++ b/packages/server/modules/automate/services/automationManagement.ts @@ -11,7 +11,6 @@ import { getFunctionRelease, getFunctionReleases } from '@/modules/automate/clients/executionEngine' -import { validateStreamAccess } from '@/modules/core/services/streams/streamAccessService' import { Automate, Roles, removeNullOrUndefinedKeys } from '@speckle/shared' import { AuthCodePayloadAction } from '@/modules/automate/services/authCode' import { @@ -56,13 +55,14 @@ import { UpdateAutomation } from '@/modules/automate/domain/operations' import { GetBranchesByIds } from '@/modules/core/domain/branches/operations' +import { ValidateStreamAccess } from '@/modules/core/domain/streams/operations' export type CreateAutomationDeps = { createAuthCode: CreateStoredAuthCode automateCreateAutomation: typeof clientCreateAutomation storeAutomation: StoreAutomation storeAutomationToken: StoreAutomationToken - validateStreamAccess: typeof validateStreamAccess + validateStreamAccess: ValidateStreamAccess automationsEventsEmit: AutomationsEventsEmit } @@ -141,7 +141,7 @@ export type CreateTestAutomationDeps = { getFunction: typeof getFunction storeAutomation: StoreAutomation storeAutomationRevision: StoreAutomationRevision - validateStreamAccess: typeof validateStreamAccess + validateStreamAccess: ValidateStreamAccess automationsEventsEmit: AutomationsEventsEmit } @@ -246,7 +246,7 @@ export const createTestAutomationFactory = export type ValidateAndUpdateAutomationDeps = { getAutomation: GetAutomation updateAutomation: UpdateAutomation - validateStreamAccess: typeof validateStreamAccess + validateStreamAccess: ValidateStreamAccess automationsEventsEmit: AutomationsEventsEmit } @@ -395,7 +395,7 @@ export type CreateAutomationRevisionDeps = { getEncryptionKeyPair: GetEncryptionKeyPair getFunctionInputDecryptor: FunctionInputDecryptor getFunctionReleases: typeof getFunctionReleases - validateStreamAccess: typeof validateStreamAccess + validateStreamAccess: ValidateStreamAccess automationsEventsEmit: AutomationsEventsEmit } & ValidateNewTriggerDefinitionsDeps & ValidateNewRevisionFunctionsDeps diff --git a/packages/server/modules/automate/services/trigger.ts b/packages/server/modules/automate/services/trigger.ts index bc18b14de..66a9bbe2e 100644 --- a/packages/server/modules/automate/services/trigger.ts +++ b/packages/server/modules/automate/services/trigger.ts @@ -24,7 +24,6 @@ import { type TriggeredAutomationFunctionRun } from '@/modules/automate/clients/executionEngine' import { TriggerAutomationError } from '@/modules/automate/errors/runs' -import { validateStreamAccess } from '@/modules/core/services/streams/streamAccessService' import { ContextResourceAccessRules } from '@/modules/core/helpers/token' import { TokenResourceIdentifierType } from '@/modules/core/graph/generated/graphql' import { automateLogger } from '@/logging/logging' @@ -48,6 +47,7 @@ import { } from '@/modules/automate/domain/operations' import { GetBranchLatestCommits } from '@/modules/core/domain/branches/operations' import { GetCommit } from '@/modules/core/domain/commits/operations' +import { ValidateStreamAccess } from '@/modules/core/domain/streams/operations' export type OnModelVersionCreateDeps = { getAutomation: GetAutomation @@ -467,7 +467,7 @@ export type ManuallyTriggerAutomationDeps = { getAutomation: GetAutomation getBranchLatestCommits: GetBranchLatestCommits triggerFunction: TriggerAutomationRevisionRun - validateStreamAccess: typeof validateStreamAccess + validateStreamAccess: ValidateStreamAccess } export const manuallyTriggerAutomationFactory = @@ -542,7 +542,7 @@ export type CreateTestAutomationRunDeps = { getLatestAutomationRevision: GetLatestAutomationRevision getFullAutomationRevisionMetadata: GetFullAutomationRevisionMetadata upsertAutomationRun: UpsertAutomationRun - validateStreamAccess: typeof validateStreamAccess + validateStreamAccess: ValidateStreamAccess getBranchLatestCommits: GetBranchLatestCommits } & CreateAutomationRunDataDeps & ComposeTriggerDataDeps diff --git a/packages/server/modules/automate/tests/automations.spec.ts b/packages/server/modules/automate/tests/automations.spec.ts index f826fe360..d57669cb2 100644 --- a/packages/server/modules/automate/tests/automations.spec.ts +++ b/packages/server/modules/automate/tests/automations.spec.ts @@ -15,10 +15,6 @@ import { getGenericRedis } from '@/modules/core' import { ProjectAutomationRevisionCreateInput } from '@/modules/core/graph/generated/graphql' import { BranchRecord } from '@/modules/core/helpers/types' import { getLatestStreamBranchFactory } from '@/modules/core/repositories/branches' -import { - addOrUpdateStreamCollaborator, - validateStreamAccess -} from '@/modules/core/services/streams/streamAccessService' import { expectToThrow } from '@/test/assertionHelper' import { BasicTestUser, createTestUsers } from '@/test/authHelper' import { @@ -47,6 +43,19 @@ import { times } from 'lodash' import { getFeatureFlags } from '@/modules/shared/helpers/envHelper' import { db } from '@/db/knex' import { AutomationsEmitter } from '@/modules/automate/events/automations' +import { + addOrUpdateStreamCollaboratorFactory, + validateStreamAccessFactory +} from '@/modules/core/services/streams/access' +import { authorizeResolver } from '@/modules/shared' +import { getUser } from '@/modules/core/repositories/users' +import { grantStreamPermissionsFactory } from '@/modules/core/repositories/streams' +import { + addStreamInviteAcceptedActivityFactory, + addStreamPermissionsAddedActivityFactory +} from '@/modules/activitystream/services/streamActivity' +import { saveActivityFactory } from '@/modules/activitystream/repositories' +import { publish } from '@/modules/shared/utils/subscriptions' /** * TODO: Extra test ideas @@ -56,6 +65,22 @@ import { AutomationsEmitter } from '@/modules/automate/events/automations' const { FF_AUTOMATE_MODULE_ENABLED } = getFeatureFlags() +const saveActivity = saveActivityFactory({ db }) +const validateStreamAccess = validateStreamAccessFactory({ authorizeResolver }) +const addOrUpdateStreamCollaborator = addOrUpdateStreamCollaboratorFactory({ + validateStreamAccess, + getUser, + grantStreamPermissions: grantStreamPermissionsFactory({ db }), + addStreamInviteAcceptedActivity: addStreamInviteAcceptedActivityFactory({ + saveActivity, + publish + }), + addStreamPermissionsAddedActivity: addStreamPermissionsAddedActivityFactory({ + saveActivity, + publish + }) +}) + const buildAutomationUpdate = () => { const getAutomation = getAutomationFactory({ db }) const updateDbAutomation = updateAutomationFactory({ db }) diff --git a/packages/server/modules/automate/tests/trigger.spec.ts b/packages/server/modules/automate/tests/trigger.spec.ts index e64bbae4c..f4a2231bc 100644 --- a/packages/server/modules/automate/tests/trigger.spec.ts +++ b/packages/server/modules/automate/tests/trigger.spec.ts @@ -76,8 +76,9 @@ import { mapGqlStatusToDbStatus } from '@/modules/automate/utils/automateFunctio import { db } from '@/db/knex' import { AutomateRunsEmitter } from '@/modules/automate/events/runs' import { createAppToken } from '@/modules/core/services/tokens' -import { validateStreamAccess } from '@/modules/core/services/streams/streamAccessService' import { getCommitFactory } from '@/modules/core/repositories/commits' +import { validateStreamAccessFactory } from '@/modules/core/services/streams/access' +import { authorizeResolver } from '@/modules/shared' const { FF_AUTOMATE_MODULE_ENABLED } = getFeatureFlags() @@ -99,6 +100,7 @@ const updateAutomationRevision = updateAutomationRevisionFactory({ db }) const updateAutomationRun = updateAutomationRunFactory({ db }) const getBranchLatestCommits = getBranchLatestCommitsFactory({ db }) const getCommit = getCommitFactory({ db }) +const validateStreamAccess = validateStreamAccessFactory({ authorizeResolver }) ;(FF_AUTOMATE_MODULE_ENABLED ? describe : describe.skip)( 'Automate triggers @automate', diff --git a/packages/server/modules/comments/tests/comments.graph.spec.js b/packages/server/modules/comments/tests/comments.graph.spec.js index 3c0c38191..59b97cec5 100644 --- a/packages/server/modules/comments/tests/comments.graph.spec.js +++ b/packages/server/modules/comments/tests/comments.graph.spec.js @@ -4,7 +4,6 @@ const crs = require('crypto-random-string') const { buildApolloServer } = require('@/app') const { beforeEachContext } = require('@/test/hooks') const { Roles } = require('@/modules/core/helpers/mainConstants') -const { grantPermissionsStream } = require('@/modules/core/services/streams') const { createUser } = require('@/modules/core/services/users') const { gql } = require('graphql-tag') const { createObject } = require('@/modules/core/services/objects') @@ -52,7 +51,8 @@ const { markCommitStreamUpdated, getStreamFactory, createStreamFactory, - updateStreamFactory + updateStreamFactory, + grantStreamPermissionsFactory } = require('@/modules/core/repositories/streams') const { VersionsEmitter } = require('@/modules/core/events/versionsEmitter') const { @@ -159,6 +159,7 @@ const createStream = legacyCreateStreamFactory({ const updateStream = legacyUpdateStreamFactory({ updateStream: updateStreamFactory({ db }) }) +const grantPermissionsStream = grantStreamPermissionsFactory({ db }) function buildCommentInputFromString(textString) { return convertBasicStringToDocument(textString) diff --git a/packages/server/modules/core/domain/streams/operations.ts b/packages/server/modules/core/domain/streams/operations.ts index b875c2c00..66852ced5 100644 --- a/packages/server/modules/core/domain/streams/operations.ts +++ b/packages/server/modules/core/domain/streams/operations.ts @@ -8,8 +8,11 @@ import { TokenResourceIdentifier } from '@/modules/core/domain/tokens/types' import { ProjectCreateInput, ProjectUpdateInput, + ProjectUpdateRoleInput, StreamCreateInput, - StreamUpdateInput + StreamRevokePermissionInput, + StreamUpdateInput, + StreamUpdatePermissionInput } from '@/modules/core/graph/generated/graphql' import { ContextResourceAccessRules } from '@/modules/core/helpers/token' import { MaybeNullOrUndefined, Nullable, Optional, StreamRoles } from '@speckle/shared' @@ -62,6 +65,22 @@ export type UpdateStreamRecord = ( update: StreamUpdateInput | ProjectUpdateInput ) => Promise> +export type RevokeStreamPermissions = (params: { + streamId: string + userId: string +}) => Promise> + +export type GrantStreamPermissions = ( + params: { + streamId: string + userId: string + role: StreamRoles + }, + options?: { + trackProjectUpdate?: boolean + } +) => Promise> + export type CreateStream = ( params: (StreamCreateInput | ProjectCreateInput) & { ownerId: string @@ -94,3 +113,44 @@ export type UpdateStream = ( export type LegacyUpdateStream = ( update: StreamUpdateInput ) => Promise> + +export type PermissionUpdateInput = + | StreamUpdatePermissionInput + | StreamRevokePermissionInput + | ProjectUpdateRoleInput + +export type UpdateStreamRole = ( + update: PermissionUpdateInput, + updaterId: string, + updaterResourceAccessRules: MaybeNullOrUndefined +) => Promise + +export type IsStreamCollaborator = ( + userId: string, + streamId: string +) => Promise + +export type ValidateStreamAccess = ( + userId: MaybeNullOrUndefined, + streamId: string, + expectedRole?: string | undefined, + userResourceAccessLimits?: MaybeNullOrUndefined +) => Promise + +export type AddOrUpdateStreamCollaborator = ( + streamId: string, + userId: string, + role: string, + addedById: string, + adderResourceAccessRules?: MaybeNullOrUndefined, + options?: Partial<{ + fromInvite: boolean + }> +) => Promise + +export type RemoveStreamCollaborator = ( + streamId: string, + userId: string, + removedById: string, + removerResourceAccessRules?: MaybeNullOrUndefined +) => Promise diff --git a/packages/server/modules/core/graph/resolvers/commits.js b/packages/server/modules/core/graph/resolvers/commits.js index 424e1e261..24b706f4a 100644 --- a/packages/server/modules/core/graph/resolvers/commits.js +++ b/packages/server/modules/core/graph/resolvers/commits.js @@ -32,9 +32,6 @@ const { batchDeleteCommits, batchMoveCommitsFactory } = require('@/modules/core/services/commit/batchCommitActions') -const { - validateStreamAccess -} = require('@/modules/core/services/streams/streamAccessService') const { StreamInvalidAccessError } = require('@/modules/core/errors/stream') const { Roles } = require('@speckle/shared') const { toProjectIdWhitelist } = require('@/modules/core/helpers/token') @@ -75,6 +72,9 @@ const { } = require('@/modules/activitystream/services/commitActivity') const { VersionsEmitter } = require('@/modules/core/events/versionsEmitter') const { getObjectFactory } = require('@/modules/core/repositories/objects') +const { + validateStreamAccessFactory +} = require('@/modules/core/services/streams/access') // subscription events const COMMIT_CREATED = CommitPubsubEvents.CommitCreated @@ -138,6 +138,7 @@ const batchMoveCommits = batchMoveCommitsFactory({ moveCommitsToBranch: moveCommitsToBranchFactory({ db }), addCommitMovedActivity }) +const validateStreamAccess = validateStreamAccessFactory({ authorizeResolver }) /** * @param {boolean} publicOnly diff --git a/packages/server/modules/core/graph/resolvers/projects.ts b/packages/server/modules/core/graph/resolvers/projects.ts index 3e93bd0ab..766fc31ed 100644 --- a/packages/server/modules/core/graph/resolvers/projects.ts +++ b/packages/server/modules/core/graph/resolvers/projects.ts @@ -3,6 +3,9 @@ import { saveActivityFactory } from '@/modules/activitystream/repositories' import { addStreamCreatedActivityFactory, addStreamDeletedActivity, + addStreamInviteAcceptedActivityFactory, + addStreamPermissionsAddedActivityFactory, + addStreamPermissionsRevokedActivityFactory, addStreamUpdatedActivity } from '@/modules/activitystream/services/streamActivity' import { RateLimitError } from '@/modules/core/errors/ratelimit' @@ -25,21 +28,28 @@ import { getStreamCollaboratorsFactory, createStreamFactory, deleteStreamFactory, - updateStreamFactory + updateStreamFactory, + revokeStreamPermissionsFactory, + grantStreamPermissionsFactory } from '@/modules/core/repositories/streams' -import { getUsers } from '@/modules/core/repositories/users' +import { getUser, getUsers } from '@/modules/core/repositories/users' import { getRateLimitResult, isRateLimitBreached } from '@/modules/core/services/ratelimiter' +import { + addOrUpdateStreamCollaboratorFactory, + isStreamCollaboratorFactory, + removeStreamCollaboratorFactory, + validateStreamAccessFactory +} from '@/modules/core/services/streams/access' import { createStreamReturnRecordFactory, deleteStreamAndNotifyFactory, updateStreamAndNotifyFactory, - updateStreamRoleAndNotify + updateStreamRoleAndNotifyFactory } from '@/modules/core/services/streams/management' import { createOnboardingStream } from '@/modules/core/services/streams/onboarding' -import { removeStreamCollaborator } from '@/modules/core/services/streams/streamAccessService' import { deleteAllResourceInvitesFactory, findUserByTargetFactory, @@ -60,6 +70,7 @@ import { } from '@/modules/shared/utils/subscriptions' import { has } from 'lodash' +const saveActivity = saveActivityFactory({ db }) const getStream = getStreamFactory({ db }) const getStreamCollaborators = getStreamCollaboratorsFactory({ db }) const createStreamReturnRecord = createStreamReturnRecordFactory({ @@ -84,7 +95,7 @@ const createStreamReturnRecord = createStreamReturnRecordFactory({ createStream: createStreamFactory({ db }), createBranch: createBranchFactory({ db }), addStreamCreatedActivity: addStreamCreatedActivityFactory({ - saveActivity: saveActivityFactory({ db }), + saveActivity, publish }), projectsEventsEmitter: ProjectsEmitter.emit @@ -101,6 +112,36 @@ const updateStreamAndNotify = updateStreamAndNotifyFactory({ updateStream: updateStreamFactory({ db }), addStreamUpdatedActivity }) +const validateStreamAccess = validateStreamAccessFactory({ authorizeResolver }) +const isStreamCollaborator = isStreamCollaboratorFactory({ + getStream +}) +const removeStreamCollaborator = removeStreamCollaboratorFactory({ + validateStreamAccess, + isStreamCollaborator, + revokeStreamPermissions: revokeStreamPermissionsFactory({ db }), + addStreamPermissionsRevokedActivity: addStreamPermissionsRevokedActivityFactory({ + saveActivity, + publish + }) +}) +const updateStreamRoleAndNotify = updateStreamRoleAndNotifyFactory({ + isStreamCollaborator, + addOrUpdateStreamCollaborator: addOrUpdateStreamCollaboratorFactory({ + validateStreamAccess, + getUser, + grantStreamPermissions: grantStreamPermissionsFactory({ db }), + addStreamInviteAcceptedActivity: addStreamInviteAcceptedActivityFactory({ + saveActivity, + publish + }), + addStreamPermissionsAddedActivity: addStreamPermissionsAddedActivityFactory({ + saveActivity, + publish + }) + }), + removeStreamCollaborator +}) export = { Query: { diff --git a/packages/server/modules/core/graph/resolvers/streams.ts b/packages/server/modules/core/graph/resolvers/streams.ts index 1a6a1bef3..4b1ae9891 100644 --- a/packages/server/modules/core/graph/resolvers/streams.ts +++ b/packages/server/modules/core/graph/resolvers/streams.ts @@ -22,7 +22,6 @@ import { inviteUsersToProjectFactory } from '@/modules/serverinvites/services/projectInviteManagement' import { removePrivateFields } from '@/modules/core/helpers/userHelper' -import { removeStreamCollaborator } from '@/modules/core/services/streams/streamAccessService' import { getDiscoverableStreams } from '@/modules/core/services/streams/discoverableStreams' import { get } from 'lodash' import { @@ -31,13 +30,15 @@ import { getStreamFactory, createStreamFactory, deleteStreamFactory, - updateStreamFactory + updateStreamFactory, + revokeStreamPermissionsFactory, + grantStreamPermissionsFactory } from '@/modules/core/repositories/streams' import { createStreamReturnRecordFactory, deleteStreamAndNotifyFactory, updateStreamAndNotifyFactory, - updateStreamRoleAndNotify + updateStreamRoleAndNotifyFactory } from '@/modules/core/services/streams/management' import { adminOverrideEnabled } from '@/modules/shared/helpers/envHelper' import { Roles, Scopes } from '@speckle/shared' @@ -59,7 +60,7 @@ import { } from '@/modules/serverinvites/repositories/serverInvites' import db from '@/db/knex' import { getInvitationTargetUsersFactory } from '@/modules/serverinvites/services/retrieval' -import { getUsers } from '@/modules/core/repositories/users' +import { getUser, getUsers } from '@/modules/core/repositories/users' import { BadRequestError } from '@/modules/shared/errors' import { createAndSendInviteFactory } from '@/modules/serverinvites/services/creation' import { collectAndValidateCoreTargetsFactory } from '@/modules/serverinvites/services/coreResourceCollection' @@ -69,11 +70,21 @@ import { createBranchFactory } from '@/modules/core/repositories/branches' import { addStreamCreatedActivityFactory, addStreamDeletedActivity, + addStreamInviteAcceptedActivityFactory, + addStreamPermissionsAddedActivityFactory, + addStreamPermissionsRevokedActivityFactory, addStreamUpdatedActivity } from '@/modules/activitystream/services/streamActivity' import { saveActivityFactory } from '@/modules/activitystream/repositories' import { ProjectsEmitter } from '@/modules/core/events/projectsEmitter' +import { + addOrUpdateStreamCollaboratorFactory, + isStreamCollaboratorFactory, + removeStreamCollaboratorFactory, + validateStreamAccessFactory +} from '@/modules/core/services/streams/access' +const saveActivity = saveActivityFactory({ db }) const getStream = getStreamFactory({ db }) const createStreamReturnRecord = createStreamReturnRecordFactory({ inviteUsersToProject: inviteUsersToProjectFactory({ @@ -97,7 +108,7 @@ const createStreamReturnRecord = createStreamReturnRecordFactory({ createStream: createStreamFactory({ db }), createBranch: createBranchFactory({ db }), addStreamCreatedActivity: addStreamCreatedActivityFactory({ - saveActivity: saveActivityFactory({ db }), + saveActivity, publish }), projectsEventsEmitter: ProjectsEmitter.emit @@ -114,6 +125,36 @@ const updateStreamAndNotify = updateStreamAndNotifyFactory({ updateStream: updateStreamFactory({ db }), addStreamUpdatedActivity }) +const validateStreamAccess = validateStreamAccessFactory({ authorizeResolver }) +const isStreamCollaborator = isStreamCollaboratorFactory({ + getStream +}) +const removeStreamCollaborator = removeStreamCollaboratorFactory({ + validateStreamAccess, + isStreamCollaborator, + revokeStreamPermissions: revokeStreamPermissionsFactory({ db }), + addStreamPermissionsRevokedActivity: addStreamPermissionsRevokedActivityFactory({ + saveActivity, + publish + }) +}) +const updateStreamRoleAndNotify = updateStreamRoleAndNotifyFactory({ + isStreamCollaborator, + addOrUpdateStreamCollaborator: addOrUpdateStreamCollaboratorFactory({ + validateStreamAccess, + getUser, + grantStreamPermissions: grantStreamPermissionsFactory({ db }), + addStreamInviteAcceptedActivity: addStreamInviteAcceptedActivityFactory({ + saveActivity, + publish + }), + addStreamPermissionsAddedActivity: addStreamPermissionsAddedActivityFactory({ + saveActivity, + publish + }) + }), + removeStreamCollaborator +}) const getUserStreamsCore = async ( forOtherUser: boolean, diff --git a/packages/server/modules/core/repositories/streams.ts b/packages/server/modules/core/repositories/streams.ts index 0f7aaf96d..9e975afb7 100644 --- a/packages/server/modules/core/repositories/streams.ts +++ b/packages/server/modules/core/repositories/streams.ts @@ -58,7 +58,7 @@ import { } from '@/modules/core/errors/stream' import { metaHelpers } from '@/modules/core/helpers/meta' import { removePrivateFields } from '@/modules/core/helpers/userHelper' -import { db, db as defaultKnexInstance } from '@/db/knex' +import { db } from '@/db/knex' import { DeleteProjectRole, GetProject, @@ -79,7 +79,9 @@ import { GetStreamCollaborators, GetStreams, DeleteStreamRecords, - UpdateStreamRecord + UpdateStreamRecord, + RevokeStreamPermissions, + GrantStreamPermissions } from '@/modules/core/domain/streams/operations' export type { StreamWithOptionalRole, StreamWithCommitId } @@ -1038,168 +1040,161 @@ export async function markCommitStreamUpdated(commitId: string) { return updates > 0 } -// TODO: Replace all calls to `grantStreamPermissions` with this export const upsertProjectRoleFactory = ({ db }: { db: Knex }): UpsertProjectRole => async ( { projectId, userId, role }, { trackProjectUpdate } = { trackProjectUpdate: true } ) => { - return await grantStreamPermissions( + const res = await grantStreamPermissionsFactory({ db })( { streamId: projectId, userId, role }, - db, { trackProjectUpdate } ) + return res! // TODO: stream theoretically can be optional, return type needs fixing } -/** - * (Moved from services/streams.js, could be refactored into something better) - * @deprecated use return value from `grantStreamPermissionsFactory` - */ -export async function grantStreamPermissions( - params: { - streamId: string - userId: string - role: StreamRoles - }, - db: Knex = defaultKnexInstance, - options: { trackProjectUpdate?: boolean } = { trackProjectUpdate: true } -) { - const { streamId, userId, role } = params +export const grantStreamPermissionsFactory = + (deps: { db: Knex }): GrantStreamPermissions => + async ( + params: { + streamId: string + userId: string + role: StreamRoles + }, + options: { trackProjectUpdate?: boolean } = { trackProjectUpdate: true } + ) => { + const { streamId, userId, role } = params - // assert we are not removing last admin from project - const existingRole = await tables - .streamAcl(db) - .where({ - [StreamAcl.col.resourceId]: streamId, - [StreamAcl.col.userId]: userId - }) - .first() - - if (existingRole?.role === Roles.Stream.Owner && role !== Roles.Stream.Owner) { - const [countObj] = await tables - .streamAcl(db) + // assert we are not removing last admin from project + const existingRole = await tables + .streamAcl(deps.db) .where({ - resourceId: streamId, - role: Roles.Stream.Owner - }) - .count() - if (parseInt(countObj.count as string) === 1) - throw new StreamAccessUpdateError('Could not revoke permissions for last admin', { - info: { streamId, userId } + [StreamAcl.col.resourceId]: streamId, + [StreamAcl.col.userId]: userId }) + .first() + + if (existingRole?.role === Roles.Stream.Owner && role !== Roles.Stream.Owner) { + const [countObj] = await tables + .streamAcl(deps.db) + .where({ + resourceId: streamId, + role: Roles.Stream.Owner + }) + .count() + if (parseInt(countObj.count as string) === 1) + throw new StreamAccessUpdateError( + 'Could not revoke permissions for last admin', + { + info: { streamId, userId } + } + ) + } + + // upserts the existing role (sets a new one!) + const query = + tables + .streamAcl(deps.db) + .insert({ userId, resourceId: streamId, role }) + .toString() + + ' on conflict on constraint stream_acl_pkey do update set role=excluded.role' + + await deps.db.raw(query) + + const streamsQuery = tables.streams(deps.db) + if (options.trackProjectUpdate) { + // update stream updated at + streamsQuery.update({ updatedAt: knex.fn.now() }, '*') + } + + const streams = await streamsQuery.where({ id: streamId }) + return streams[0] as StreamRecord } - // upserts the existing role (sets a new one!) - const query = - tables.streamAcl(db).insert({ userId, resourceId: streamId, role }).toString() + - ' on conflict on constraint stream_acl_pkey do update set role=excluded.role' - - await knex.raw(query) - - const streamsQuery = tables.streams(db) - if (options.trackProjectUpdate) { - // update stream updated at - streamsQuery.update({ updatedAt: knex.fn.now() }, '*') - } - - const streams = await streamsQuery.where({ id: streamId }) - return streams[0] as StreamRecord -} - -// TODO: Replace all calls of `revokeStreamPermissions` with this export const deleteProjectRoleFactory = ({ db }: { db: Knex }): DeleteProjectRole => async ({ projectId, userId }) => { - return await revokeStreamPermissions( - { - streamId: projectId, - userId - }, - db - ) - } - -/** - * (Moved from services/streams.js, could be refactored into something better) - * @deprecated Use return value from `revokeStreamPermissionsFactory` - */ -export async function revokeStreamPermissions( - params: { - streamId: string - userId: string - }, - db: Knex = defaultKnexInstance -) { - const { streamId, userId } = params - - const existingPermission = await tables - .streamAcl(db) - .where({ - [StreamAcl.col.resourceId]: streamId, - [StreamAcl.col.userId]: userId + return await revokeStreamPermissionsFactory({ db })({ + streamId: projectId, + userId }) - .first() - if (!existingPermission) { - // User already doesn't have permissions - return await tables - .streams(db) - .where({ [Streams.col.id]: streamId }) - .first() } - const [streamAclEntriesCount] = await tables - .streamAcl(db) - .where({ resourceId: streamId }) - .count<{ count: string }[]>() +export const revokeStreamPermissionsFactory = + (deps: { db: Knex }): RevokeStreamPermissions => + async (params: { streamId: string; userId: string }) => { + const { streamId, userId } = params - if (parseInt(streamAclEntriesCount.count) === 1) - throw new StreamAccessUpdateError( - 'Stream has only one ownership link left - cannot revoke permissions.', - { info: { streamId, userId } } - ) - - const aclEntry = existingPermission - if (aclEntry?.role === Roles.Stream.Owner) { - const [countObj] = await tables - .streamAcl(db) + const existingPermission = await tables + .streamAcl(deps.db) .where({ - resourceId: streamId, - role: Roles.Stream.Owner + [StreamAcl.col.resourceId]: streamId, + [StreamAcl.col.userId]: userId }) - .count() - if (parseInt(countObj.count as string) === 1) - throw new StreamAccessUpdateError('Could not revoke permissions for last admin', { - info: { streamId, userId } - }) - else { - await tables.streamAcl(db).where({ resourceId: streamId, userId }).del() + .first() + if (!existingPermission) { + // User already doesn't have permissions + return await tables + .streams(deps.db) + .where({ [Streams.col.id]: streamId }) + .first() } - } else { - const delCount = await tables - .streamAcl(db) - .where({ resourceId: streamId, userId }) - .del() - if (delCount === 0) - throw new StreamAccessUpdateError('Could not revoke permissions for user', { - info: { streamId, userId } - }) + const [streamAclEntriesCount] = await tables + .streamAcl(deps.db) + .where({ resourceId: streamId }) + .count<{ count: string }[]>() + + if (parseInt(streamAclEntriesCount.count) === 1) + throw new StreamAccessUpdateError( + 'Stream has only one ownership link left - cannot revoke permissions.', + { info: { streamId, userId } } + ) + + const aclEntry = existingPermission + if (aclEntry?.role === Roles.Stream.Owner) { + const [countObj] = await tables + .streamAcl(deps.db) + .where({ + resourceId: streamId, + role: Roles.Stream.Owner + }) + .count() + if (parseInt(countObj.count as string) === 1) + throw new StreamAccessUpdateError( + 'Could not revoke permissions for last admin', + { + info: { streamId, userId } + } + ) + else { + await tables.streamAcl(deps.db).where({ resourceId: streamId, userId }).del() + } + } else { + const delCount = await tables + .streamAcl(deps.db) + .where({ resourceId: streamId, userId }) + .del() + + if (delCount === 0) + throw new StreamAccessUpdateError('Could not revoke permissions for user', { + info: { streamId, userId } + }) + } + + // update stream updated at + const [stream] = await tables + .streams(deps.db) + .where({ id: streamId }) + .update({ updatedAt: knex.fn.now() }, '*') + + return stream } - // update stream updated at - const [stream] = await tables - .streams(db) - .where({ id: streamId }) - .update({ updatedAt: knex.fn.now() }, '*') - - return stream as StreamRecord -} - /** * Mark stream as the onboarding base stream from which user onboarding streams will be cloned */ diff --git a/packages/server/modules/core/services/streams.js b/packages/server/modules/core/services/streams.js index aba3798d1..728a80fc5 100644 --- a/packages/server/modules/core/services/streams.js +++ b/packages/server/modules/core/services/streams.js @@ -6,8 +6,6 @@ const { getFavoritedStreamsCount, setStreamFavorited, canUserFavoriteStream, - revokeStreamPermissions, - grantStreamPermissions, getStreamFactory } = require('@/modules/core/repositories/streams') const { UnauthorizedError, InvalidArgumentError } = require('@/modules/shared/errors') @@ -26,20 +24,6 @@ const { module.exports = { setStreamFavorited, - /** - * @deprecated Use repository method directly - */ - async grantPermissionsStream({ streamId, userId, role }) { - return await grantStreamPermissions({ streamId, userId, role }) - }, - - /** - * @deprecated Use repository method directly - */ - async revokePermissionsStream({ streamId, userId }) { - return await revokeStreamPermissions({ streamId, userId }) - }, - /** * @param {Object} p * @param {string | Date | null} [p.cursor] diff --git a/packages/server/modules/core/services/streams/access.ts b/packages/server/modules/core/services/streams/access.ts new file mode 100644 index 000000000..6279469c9 --- /dev/null +++ b/packages/server/modules/core/services/streams/access.ts @@ -0,0 +1,212 @@ +import { + addStreamInviteAcceptedActivityFactory, + addStreamPermissionsAddedActivityFactory, + addStreamPermissionsRevokedActivityFactory +} from '@/modules/activitystream/services/streamActivity' +import { + AddOrUpdateStreamCollaborator, + GetStream, + GrantStreamPermissions, + IsStreamCollaborator, + RemoveStreamCollaborator, + RevokeStreamPermissions, + ValidateStreamAccess +} from '@/modules/core/domain/streams/operations' +import { + StreamAccessUpdateError, + StreamInvalidAccessError +} from '@/modules/core/errors/stream' +import { StreamRecord } from '@/modules/core/helpers/types' +import { getUser } from '@/modules/core/repositories/users' +import { AuthorizeResolver } from '@/modules/shared/domain/operations' +import { BadRequestError, ForbiddenError, LogicError } from '@/modules/shared/errors' +import { ensureError, Roles, StreamRoles } from '@speckle/shared' + +/** + * Check if user is a stream collaborator + */ +export const isStreamCollaboratorFactory = + (deps: { getStream: GetStream }): IsStreamCollaborator => + async (userId, streamId) => { + const stream = await deps.getStream({ streamId, userId }) + return !!stream?.role + } + +/** + * Validate that the user has the required permission level (or one above it) for the specified stream. + * + * Note: The access check can sometimes succeed even if the user being tested is a guest, e.g. + * if the stream is public and we're only looking for stream:reviewer or up. If you want to check + * that the target user is an actual collaborator, use isStreamCollaborator instead. + */ +export const validateStreamAccessFactory = + (deps: { authorizeResolver: AuthorizeResolver }): ValidateStreamAccess => + async (userId, streamId, expectedRole, userResourceAccessLimits) => { + expectedRole = expectedRole || Roles.Stream.Reviewer + + const streamRoles = Object.values(Roles.Stream) + if (!streamRoles.includes(expectedRole as StreamRoles)) { + throw new LogicError('Unexpected stream role') + } + + userId = userId || null + + try { + await deps.authorizeResolver( + userId, + streamId, + expectedRole as StreamRoles, + userResourceAccessLimits + ) + } catch (e) { + if ( + e instanceof ForbiddenError || + /^resource of type streams .* not found$/i.test(ensureError(e).message) + ) { + throw new StreamInvalidAccessError( + 'User does not have required access to stream', + { + // cause: e, // We don't want to show the real cause to the user + info: { + userId, + streamId, + expectedRole + } + } + ) + } else { + throw e + } + } + + return true + } + +/** + * Remove collaborator from stream + * @param {string} streamId + * @param {string} userId ID of user that should be removed + * @param {string} removedById ID of user that is doing the removing + */ +export const removeStreamCollaboratorFactory = + (deps: { + validateStreamAccess: ValidateStreamAccess + isStreamCollaborator: IsStreamCollaborator + revokeStreamPermissions: RevokeStreamPermissions + addStreamPermissionsRevokedActivity: ReturnType< + typeof addStreamPermissionsRevokedActivityFactory + > + }): RemoveStreamCollaborator => + async (streamId, userId, removedById, removerResourceAccessRules) => { + if (userId !== removedById) { + // User must be a stream owner to remove others + await deps.validateStreamAccess( + removedById, + streamId, + Roles.Stream.Owner, + removerResourceAccessRules + ) + } else { + // User must have any kind of role to remove himself + const isCollaborator = await deps.isStreamCollaborator(userId, streamId) + if (!isCollaborator) { + throw new StreamAccessUpdateError('User is not a stream collaborator') + } + } + + const stream = await deps.revokeStreamPermissions({ streamId, userId }) + if (!stream) { + throw new LogicError('Stream not found') + } + + await deps.addStreamPermissionsRevokedActivity({ + streamId, + activityUserId: removedById, + removedUserId: userId, + stream + }) + + return stream + } + +/** + * Add a new collaborator to the stream or update their access level + * + * Optional parameters: + * - fromInvite: Set to true, if user is being added as a result of accepting an invitation + * @param {string} streamId + * @param {string} userId ID of user who is being added + * @param {string} role + * @param {string} addedById ID of user who is adding the new collaborator + */ +export const addOrUpdateStreamCollaboratorFactory = + (deps: { + validateStreamAccess: ValidateStreamAccess + getUser: typeof getUser + grantStreamPermissions: GrantStreamPermissions + addStreamInviteAcceptedActivity: ReturnType< + typeof addStreamInviteAcceptedActivityFactory + > + addStreamPermissionsAddedActivity: ReturnType< + typeof addStreamPermissionsAddedActivityFactory + > + }): AddOrUpdateStreamCollaborator => + async ( + streamId, + userId, + role, + addedById, + adderResourceAccessRules, + { fromInvite } = {} + ) => { + const validRoles = Object.values(Roles.Stream) as string[] + if (!validRoles.includes(role)) { + throw new LogicError('Unexpected stream role') + } + + if (userId === addedById) { + throw new StreamInvalidAccessError( + 'User cannot change their own stream access level' + ) + } + + await deps.validateStreamAccess( + addedById, + streamId, + Roles.Stream.Owner, + adderResourceAccessRules + ) + + // make sure server guests cannot be stream owners + if (role === Roles.Stream.Owner) { + const user = await deps.getUser(userId, { withRole: true }) + if (user?.role === Roles.Server.Guest) + throw new BadRequestError('Server guests cannot own streams') + } + + const stream = (await deps.grantStreamPermissions({ + streamId, + userId, + role: role as StreamRoles + })) as StreamRecord // validateStreamAccess already checked that it exists + + if (fromInvite) { + await deps.addStreamInviteAcceptedActivity({ + streamId, + inviterId: addedById, + inviteTargetId: userId, + role: role as StreamRoles, + stream + }) + } else { + await deps.addStreamPermissionsAddedActivity({ + streamId, + activityUserId: addedById, + targetUserId: userId, + role: role as StreamRoles, + stream + }) + } + + return stream + } diff --git a/packages/server/modules/core/services/streams/management.ts b/packages/server/modules/core/services/streams/management.ts index 2f1c723ee..d327d400a 100644 --- a/packages/server/modules/core/services/streams/management.ts +++ b/packages/server/modules/core/services/streams/management.ts @@ -10,8 +10,7 @@ import { ProjectUpdateRoleInput, StreamCreateInput, StreamRevokePermissionInput, - StreamUpdateInput, - StreamUpdatePermissionInput + StreamUpdateInput } from '@/modules/core/graph/generated/graphql' import { StreamRecord } from '@/modules/core/helpers/types' import { @@ -20,11 +19,6 @@ import { } from '@/modules/core/errors/stream' import { isProjectCreateInput } from '@/modules/core/helpers/stream' import { has } from 'lodash' -import { - addOrUpdateStreamCollaborator, - isStreamCollaborator, - removeStreamCollaborator -} from '@/modules/core/services/streams/streamAccessService' import { ContextResourceAccessRules, isNewResourceAllowed @@ -40,15 +34,20 @@ import { import { inviteUsersToProjectFactory } from '@/modules/serverinvites/services/projectInviteManagement' import { ProjectInviteResourceType } from '@/modules/serverinvites/domain/constants' import { + AddOrUpdateStreamCollaborator, CreateStream, DeleteStream, DeleteStreamRecords, GetStream, + IsStreamCollaborator, LegacyCreateStream, LegacyUpdateStream, + PermissionUpdateInput, + RemoveStreamCollaborator, StoreStream, UpdateStream, - UpdateStreamRecord + UpdateStreamRecord, + UpdateStreamRole } from '@/modules/core/domain/streams/operations' import { StoreBranch } from '@/modules/core/domain/branches/operations' import { AuthorizeResolver } from '@/modules/shared/domain/operations' @@ -236,11 +235,6 @@ export const legacyUpdateStreamFactory = return updatedStream?.id || null } -type PermissionUpdateInput = - | StreamUpdatePermissionInput - | StreamRevokePermissionInput - | ProjectUpdateRoleInput - const isProjectUpdateRoleInput = ( i: PermissionUpdateInput ): i is ProjectUpdateRoleInput => has(i, 'projectId') @@ -248,53 +242,59 @@ const isStreamRevokePermissionInput = ( i: PermissionUpdateInput ): i is StreamRevokePermissionInput => has(i, 'streamId') && !has(i, 'role') -export async function updateStreamRoleAndNotify( - update: PermissionUpdateInput, - updaterId: string, - updaterResourceAccessRules: MaybeNullOrUndefined -) { - const smallestStreamRole = Roles.Stream.Reviewer - const params = { - streamId: isProjectUpdateRoleInput(update) ? update.projectId : update.streamId, - userId: update.userId, - role: - isStreamRevokePermissionInput(update) || !update.role - ? null - : update.role.toLowerCase() || smallestStreamRole - } - - if (params.role) { - // Updating role - if (!(Object.values(Roles.Stream) as string[]).includes(params.role)) { - throw new StreamUpdateError('Invalid role specified', { - info: { params } - }) +export const updateStreamRoleAndNotifyFactory = + (deps: { + isStreamCollaborator: IsStreamCollaborator + addOrUpdateStreamCollaborator: AddOrUpdateStreamCollaborator + removeStreamCollaborator: RemoveStreamCollaborator + }): UpdateStreamRole => + async ( + update: PermissionUpdateInput, + updaterId: string, + updaterResourceAccessRules: MaybeNullOrUndefined + ) => { + const smallestStreamRole = Roles.Stream.Reviewer + const params = { + streamId: isProjectUpdateRoleInput(update) ? update.projectId : update.streamId, + userId: update.userId, + role: + isStreamRevokePermissionInput(update) || !update.role + ? null + : update.role.toLowerCase() || smallestStreamRole } - // We only allow changing roles, not adding access - for that the user must use stream invites - const isCollaboratorAlready = await isStreamCollaborator( - params.userId, - params.streamId - ) - if (!isCollaboratorAlready) { - throw new StreamUpdateError( - "Cannot grant permissions to users who aren't collaborators already - invite the user to the stream first" + if (params.role) { + // Updating role + if (!(Object.values(Roles.Stream) as string[]).includes(params.role)) { + throw new StreamUpdateError('Invalid role specified', { + info: { params } + }) + } + + // We only allow changing roles, not adding access - for that the user must use stream invites + const isCollaboratorAlready = await deps.isStreamCollaborator( + params.userId, + params.streamId + ) + if (!isCollaboratorAlready) { + throw new StreamUpdateError( + "Cannot grant permissions to users who aren't collaborators already - invite the user to the stream first" + ) + } + + return await deps.addOrUpdateStreamCollaborator( + params.streamId, + params.userId, + params.role, + updaterId, + updaterResourceAccessRules + ) + } else { + return await deps.removeStreamCollaborator( + params.streamId, + params.userId, + updaterId, + updaterResourceAccessRules ) } - - return await addOrUpdateStreamCollaborator( - params.streamId, - params.userId, - params.role, - updaterId, - updaterResourceAccessRules - ) - } else { - return await removeStreamCollaborator( - params.streamId, - params.userId, - updaterId, - updaterResourceAccessRules - ) } -} diff --git a/packages/server/modules/core/services/streams/streamAccessService.js b/packages/server/modules/core/services/streams/streamAccessService.js deleted file mode 100644 index 14955ffa8..000000000 --- a/packages/server/modules/core/services/streams/streamAccessService.js +++ /dev/null @@ -1,224 +0,0 @@ -const { authorizeResolver } = require(`@/modules/shared`) - -const { Roles } = require('@/modules/core/helpers/mainConstants') -const { - LogicError, - ForbiddenError, - BadRequestError -} = require('@/modules/shared/errors') -const { - StreamInvalidAccessError, - StreamAccessUpdateError -} = require('@/modules/core/errors/stream') -const { - addStreamPermissionsRevokedActivityFactory, - addStreamInviteAcceptedActivityFactory, - addStreamPermissionsAddedActivityFactory -} = require('@/modules/activitystream/services/streamActivity') -const { - revokeStreamPermissions, - grantStreamPermissions, - getStreamFactory -} = require('@/modules/core/repositories/streams') - -const { ServerAcl } = require('@/modules/core/dbSchema') -const { ensureError } = require('@speckle/shared') -const { publish } = require('@/modules/shared/utils/subscriptions') -const { saveActivityFactory } = require('@/modules/activitystream/repositories') -const { db } = require('@/db/knex') - -/** - * Check if user is a stream collaborator - * @param {string} userId - * @param {string} streamId - * @returns - */ -async function isStreamCollaborator(userId, streamId) { - const getStream = getStreamFactory({ db }) - const stream = await getStream({ streamId, userId }) - return !!stream.role -} - -/** - * Validate that the user has the required permission level (or one above it) for the specified stream. - * - * Note: The access check can sometimes succeed even if the user being tested is a guest, e.g. - * if the stream is public and we're only looking for stream:reviewer or up. If you want to check - * that the target user is an actual collaborator, use isStreamCollaborator instead. - * @param {string} [userId] If falsy, will throw for non-public streams - * @param {string} streamId - * @param {string} [expectedRole] Defaults to reviewer - * @param {import('@/modules/core/domain/tokens/types').TokenResourceIdentifier[] | undefined | null} [userResourceAccessLimits] - * @returns {Promise} - */ -async function validateStreamAccess( - userId, - streamId, - expectedRole, - userResourceAccessLimits -) { - expectedRole = expectedRole || Roles.Stream.Reviewer - - const streamRoles = Object.values(Roles.Stream) - if (!streamRoles.includes(expectedRole)) { - throw new LogicError('Unexpected stream role') - } - - userId = userId || null - - try { - await authorizeResolver(userId, streamId, expectedRole, userResourceAccessLimits) - } catch (e) { - if ( - e instanceof ForbiddenError || - /^resource of type streams .* not found$/i.test(ensureError(e).message) - ) { - throw new StreamInvalidAccessError( - 'User does not have required access to stream', - { - // cause: e, // We don't want to show the real cause to the user - info: { - userId, - streamId, - expectedRole - } - } - ) - } else { - throw e - } - } - - return true -} - -/** - * Remove collaborator from stream - * @param {string} streamId - * @param {string} userId ID of user that should be removed - * @param {string} removedById ID of user that is doing the removing - * @param {import('@/modules/core/domain/tokens/types').TokenResourceIdentifier[] | undefined | null} [removerResourceAccessRules] Resource access rules (if any) for the user doing the removing - */ -async function removeStreamCollaborator( - streamId, - userId, - removedById, - removerResourceAccessRules -) { - if (userId !== removedById) { - // User must be a stream owner to remove others - await validateStreamAccess( - removedById, - streamId, - Roles.Stream.Owner, - removerResourceAccessRules - ) - } else { - // User must have any kind of role to remove himself - const isCollaborator = await isStreamCollaborator(userId, streamId) - if (!isCollaborator) { - throw new StreamAccessUpdateError('User is not a stream collaborator') - } - } - - const stream = await revokeStreamPermissions({ streamId, userId }) - - await addStreamPermissionsRevokedActivityFactory({ - publish, - saveActivity: saveActivityFactory({ db }) - })({ - streamId, - activityUserId: removedById, - removedUserId: userId, - stream - }) - - return stream -} - -/** - * Add a new collaborator to the stream or update their access level - * - * Optional parameters: - * - fromInvite: Set to true, if user is being added as a result of accepting an invitation - * @param {string} streamId - * @param {string} userId ID of user who is being added - * @param {string} role - * @param {string} addedById ID of user who is adding the new collaborator - * @param {import('@/modules/core/domain/tokens/types').TokenResourceIdentifier[] | undefined | null} [adderResourceAccessRules] Resource access rules (if any) for the user doing the adding - * @param {{ - * fromInvite?: boolean, - * }} param4 - */ -async function addOrUpdateStreamCollaborator( - streamId, - userId, - role, - addedById, - adderResourceAccessRules, - { fromInvite } = {} -) { - const validRoles = Object.values(Roles.Stream) - if (!validRoles.includes(role)) { - throw new LogicError('Unexpected stream role') - } - - if (userId === addedById) { - throw new StreamInvalidAccessError( - 'User cannot change their own stream access level' - ) - } - - await validateStreamAccess( - addedById, - streamId, - Roles.Stream.Owner, - adderResourceAccessRules - ) - - // make sure server guests cannot be stream owners - if (role === Roles.Stream.Owner) { - const userServerRole = await ServerAcl.knex().where({ userId }).first() - if (userServerRole.role === Roles.Server.Guest) - throw new BadRequestError('Server guests cannot own streams') - } - - const stream = await grantStreamPermissions({ - streamId, - userId, - role - }) - - if (fromInvite) { - await addStreamInviteAcceptedActivityFactory({ - saveActivity: saveActivityFactory({ db }), - publish - })({ - streamId, - inviterId: addedById, - inviteTargetId: userId, - role, - stream - }) - } else { - await addStreamPermissionsAddedActivityFactory({ - saveActivity: saveActivityFactory({ db }), - publish - })({ - streamId, - activityUserId: addedById, - targetUserId: userId, - role, - stream - }) - } - - return stream -} - -module.exports = { - validateStreamAccess, - removeStreamCollaborator, - addOrUpdateStreamCollaborator, - isStreamCollaborator -} diff --git a/packages/server/modules/core/tests/batchCommits.spec.ts b/packages/server/modules/core/tests/batchCommits.spec.ts index 98098b268..652490794 100644 --- a/packages/server/modules/core/tests/batchCommits.spec.ts +++ b/packages/server/modules/core/tests/batchCommits.spec.ts @@ -1,10 +1,22 @@ import { buildApolloServer } from '@/app' import { db } from '@/db/knex' +import { saveActivityFactory } from '@/modules/activitystream/repositories' +import { + addStreamInviteAcceptedActivityFactory, + addStreamPermissionsAddedActivityFactory +} from '@/modules/activitystream/services/streamActivity' import { Commits, Streams, Users } from '@/modules/core/dbSchema' import { Roles } from '@/modules/core/helpers/mainConstants' import { createBranchFactory } from '@/modules/core/repositories/branches' import { getCommitsFactory } from '@/modules/core/repositories/commits' -import { addOrUpdateStreamCollaborator } from '@/modules/core/services/streams/streamAccessService' +import { grantStreamPermissionsFactory } from '@/modules/core/repositories/streams' +import { getUser } from '@/modules/core/repositories/users' +import { + addOrUpdateStreamCollaboratorFactory, + validateStreamAccessFactory +} from '@/modules/core/services/streams/access' +import { authorizeResolver } from '@/modules/shared' +import { publish } from '@/modules/shared/utils/subscriptions' import { BasicTestUser, createTestUsers } from '@/test/authHelper' import { deleteCommits, moveCommits } from '@/test/graphql/commits' import { @@ -26,6 +38,21 @@ enum BatchActionType { const createBranch = createBranchFactory({ db }) const getCommits = getCommitsFactory({ db }) +const saveActivity = saveActivityFactory({ db }) +const validateStreamAccess = validateStreamAccessFactory({ authorizeResolver }) +const addOrUpdateStreamCollaborator = addOrUpdateStreamCollaboratorFactory({ + validateStreamAccess, + getUser, + grantStreamPermissions: grantStreamPermissionsFactory({ db }), + addStreamInviteAcceptedActivity: addStreamInviteAcceptedActivityFactory({ + saveActivity, + publish + }), + addStreamPermissionsAddedActivity: addStreamPermissionsAddedActivityFactory({ + saveActivity, + publish + }) +}) const cleanup = async () => { await truncateTables([Streams.name, Users.name, Commits.name]) diff --git a/packages/server/modules/core/tests/commitsGraphql.spec.ts b/packages/server/modules/core/tests/commitsGraphql.spec.ts index 0ea67d498..a6fae0163 100644 --- a/packages/server/modules/core/tests/commitsGraphql.spec.ts +++ b/packages/server/modules/core/tests/commitsGraphql.spec.ts @@ -1,8 +1,21 @@ import { buildApolloServer } from '@/app' +import { db } from '@/db/knex' +import { saveActivityFactory } from '@/modules/activitystream/repositories' +import { + addStreamInviteAcceptedActivityFactory, + addStreamPermissionsAddedActivityFactory +} from '@/modules/activitystream/services/streamActivity' import { Commits, Streams, Users } from '@/modules/core/dbSchema' import { Roles } from '@/modules/core/helpers/mainConstants' -import { addOrUpdateStreamCollaborator } from '@/modules/core/services/streams/streamAccessService' +import { grantStreamPermissionsFactory } from '@/modules/core/repositories/streams' +import { getUser } from '@/modules/core/repositories/users' +import { + addOrUpdateStreamCollaboratorFactory, + validateStreamAccessFactory +} from '@/modules/core/services/streams/access' +import { authorizeResolver } from '@/modules/shared' import { Nullable } from '@/modules/shared/helpers/typeHelper' +import { publish } from '@/modules/shared/utils/subscriptions' import { BasicTestUser, createTestUsers } from '@/test/authHelper' import { readOtherUsersCommits, readOwnCommits } from '@/test/graphql/commits' import { createAuthedTestContext, ServerAndContext } from '@/test/graphqlHelper' @@ -11,6 +24,22 @@ import { createTestCommit } from '@/test/speckle-helpers/commitHelper' import { BasicTestStream, createTestStreams } from '@/test/speckle-helpers/streamHelper' import { expect } from 'chai' +const saveActivity = saveActivityFactory({ db }) +const validateStreamAccess = validateStreamAccessFactory({ authorizeResolver }) +const addOrUpdateStreamCollaborator = addOrUpdateStreamCollaboratorFactory({ + validateStreamAccess, + getUser, + grantStreamPermissions: grantStreamPermissionsFactory({ db }), + addStreamInviteAcceptedActivity: addStreamInviteAcceptedActivityFactory({ + saveActivity, + publish + }), + addStreamPermissionsAddedActivity: addStreamPermissionsAddedActivityFactory({ + saveActivity, + publish + }) +}) + describe('Commits (GraphQL)', () => { const readableUserCommitsCount = 15 let readablePublicUserCommitsCount = 0 diff --git a/packages/server/modules/core/tests/graph.spec.js b/packages/server/modules/core/tests/graph.spec.js index d198b9eba..cd65ad3c8 100644 --- a/packages/server/modules/core/tests/graph.spec.js +++ b/packages/server/modules/core/tests/graph.spec.js @@ -11,12 +11,59 @@ const { changeUserRole } = require('@/modules/core/services/users') const { createPersonalAccessToken } = require('../services/tokens') -const { - addOrUpdateStreamCollaborator, - removeStreamCollaborator -} = require('@/modules/core/services/streams/streamAccessService') const { Roles, Scopes } = require('@speckle/shared') const cryptoRandomString = require('crypto-random-string') +const { saveActivityFactory } = require('@/modules/activitystream/repositories') +const { db } = require('@/db/knex') +const { + validateStreamAccessFactory, + isStreamCollaboratorFactory, + removeStreamCollaboratorFactory, + addOrUpdateStreamCollaboratorFactory +} = require('@/modules/core/services/streams/access') +const { authorizeResolver } = require('@/modules/shared') +const { + getStreamFactory, + revokeStreamPermissionsFactory, + grantStreamPermissionsFactory +} = require('@/modules/core/repositories/streams') +const { + addStreamPermissionsRevokedActivityFactory, + addStreamInviteAcceptedActivityFactory, + addStreamPermissionsAddedActivityFactory +} = require('@/modules/activitystream/services/streamActivity') +const { publish } = require('@/modules/shared/utils/subscriptions') +const { getUser } = require('@/modules/core/repositories/users') + +const getStream = getStreamFactory({ db }) +const saveActivity = saveActivityFactory({ db }) +const validateStreamAccess = validateStreamAccessFactory({ authorizeResolver }) +const isStreamCollaborator = isStreamCollaboratorFactory({ + getStream +}) +const removeStreamCollaborator = removeStreamCollaboratorFactory({ + validateStreamAccess, + isStreamCollaborator, + revokeStreamPermissions: revokeStreamPermissionsFactory({ db }), + addStreamPermissionsRevokedActivity: addStreamPermissionsRevokedActivityFactory({ + saveActivity, + publish + }) +}) + +const addOrUpdateStreamCollaborator = addOrUpdateStreamCollaboratorFactory({ + validateStreamAccess, + getUser, + grantStreamPermissions: grantStreamPermissionsFactory({ db }), + addStreamInviteAcceptedActivity: addStreamInviteAcceptedActivityFactory({ + saveActivity, + publish + }), + addStreamPermissionsAddedActivity: addStreamPermissionsAddedActivityFactory({ + saveActivity, + publish + }) +}) let app let server diff --git a/packages/server/modules/core/tests/graphSubs.spec.js b/packages/server/modules/core/tests/graphSubs.spec.js index b9f1a4c76..9e157631f 100644 --- a/packages/server/modules/core/tests/graphSubs.spec.js +++ b/packages/server/modules/core/tests/graphSubs.spec.js @@ -13,11 +13,39 @@ const { beforeEachContext } = require(`@/test/hooks`) const { sleep, noErrors } = require('@/test/helpers') const { packageRoot } = require('@/bootstrap') -const { - addOrUpdateStreamCollaborator -} = require('@/modules/core/services/streams/streamAccessService') const { Roles, Scopes } = require('@speckle/shared') const { getFreeServerPort } = require('@/test/serverHelper') +const { saveActivityFactory } = require('@/modules/activitystream/repositories') +const { db } = require('@/db/knex') +const { + validateStreamAccessFactory, + addOrUpdateStreamCollaboratorFactory +} = require('@/modules/core/services/streams/access') +const { authorizeResolver } = require('@/modules/shared') +const { getUser } = require('@/modules/core/repositories/users') +const { grantStreamPermissionsFactory } = require('@/modules/core/repositories/streams') +const { + addStreamInviteAcceptedActivityFactory, + addStreamPermissionsAddedActivityFactory +} = require('@/modules/activitystream/services/streamActivity') +const { publish } = require('@/modules/shared/utils/subscriptions') + +const saveActivity = saveActivityFactory({ db }) +const validateStreamAccess = validateStreamAccessFactory({ authorizeResolver }) + +const addOrUpdateStreamCollaborator = addOrUpdateStreamCollaboratorFactory({ + validateStreamAccess, + getUser, + grantStreamPermissions: grantStreamPermissionsFactory({ db }), + addStreamInviteAcceptedActivity: addStreamInviteAcceptedActivityFactory({ + saveActivity, + publish + }), + addStreamPermissionsAddedActivity: addStreamPermissionsAddedActivityFactory({ + saveActivity, + publish + }) +}) let addr let wsAddr diff --git a/packages/server/modules/core/tests/streams.spec.ts b/packages/server/modules/core/tests/streams.spec.ts index efe3e0394..7620a093a 100644 --- a/packages/server/modules/core/tests/streams.spec.ts +++ b/packages/server/modules/core/tests/streams.spec.ts @@ -1,13 +1,9 @@ import { expect } from 'chai' -import { getStreamUsers, grantPermissionsStream } from '@/modules/core/services/streams' +import { getStreamUsers } from '@/modules/core/services/streams' import { createObject } from '@/modules/core/services/objects' import { beforeEachContext, truncateTables } from '@/test/hooks' -import { - addOrUpdateStreamCollaborator, - isStreamCollaborator -} from '@/modules/core/services/streams/streamAccessService' import { Roles } from '@/modules/core/helpers/mainConstants' import { getLimitedUserStreams, @@ -25,9 +21,10 @@ import { createStreamFactory, deleteStreamFactory, getStreamFactory, + grantStreamPermissionsFactory, markBranchStreamUpdated, markCommitStreamUpdated, - revokeStreamPermissions, + revokeStreamPermissionsFactory, updateStreamFactory } from '@/modules/core/repositories/streams' import { has, times } from 'lodash' @@ -84,11 +81,21 @@ import { import { collectAndValidateCoreTargetsFactory } from '@/modules/serverinvites/services/coreResourceCollection' import { buildCoreInviteEmailContentsFactory } from '@/modules/serverinvites/services/coreEmailContents' import { getEventBus } from '@/modules/shared/services/eventBus' -import { getUsers } from '@/modules/core/repositories/users' +import { getUser, getUsers } from '@/modules/core/repositories/users' import { ProjectsEmitter } from '@/modules/core/events/projectsEmitter' -import { addStreamCreatedActivityFactory } from '@/modules/activitystream/services/streamActivity' +import { + addStreamCreatedActivityFactory, + addStreamInviteAcceptedActivityFactory, + addStreamPermissionsAddedActivityFactory +} from '@/modules/activitystream/services/streamActivity' import { saveActivityFactory } from '@/modules/activitystream/repositories' import { publish } from '@/modules/shared/utils/subscriptions' +import { + addOrUpdateStreamCollaboratorFactory, + isStreamCollaboratorFactory, + validateStreamAccessFactory +} from '@/modules/core/services/streams/access' +import { authorizeResolver } from '@/modules/shared' const getStream = getStreamFactory({ db }) const getStreamBranchByName = getStreamBranchByNameFactory({ db }) @@ -156,6 +163,29 @@ const updateStream = legacyUpdateStreamFactory({ updateStream: updateStreamFactory({ db }) }) +const revokeStreamPermissions = revokeStreamPermissionsFactory({ db }) +const saveActivity = saveActivityFactory({ db }) +const validateStreamAccess = validateStreamAccessFactory({ + authorizeResolver +}) +const addOrUpdateStreamCollaborator = addOrUpdateStreamCollaboratorFactory({ + validateStreamAccess, + getUser, + grantStreamPermissions: grantStreamPermissionsFactory({ db }), + addStreamInviteAcceptedActivity: addStreamInviteAcceptedActivityFactory({ + saveActivity, + publish + }), + addStreamPermissionsAddedActivity: addStreamPermissionsAddedActivityFactory({ + saveActivity, + publish + }) +}) +const isStreamCollaborator = isStreamCollaboratorFactory({ + getStream +}) +const grantPermissionsStream = grantStreamPermissionsFactory({ db }) + describe('Streams @core-streams', () => { const userOne: BasicTestUser = { name: 'Dimitrie Stefanescu', diff --git a/packages/server/modules/core/tests/users.spec.js b/packages/server/modules/core/tests/users.spec.js index e1d2f150f..29e30e411 100644 --- a/packages/server/modules/core/tests/users.spec.js +++ b/packages/server/modules/core/tests/users.spec.js @@ -21,7 +21,6 @@ const { validateToken, getUserTokens } = require('../services/tokens') -const { grantPermissionsStream } = require('../services/streams') const { getBranchesByStreamId } = require('../services/branches') @@ -51,7 +50,8 @@ const { const { markCommitStreamUpdated, getStreamFactory, - createStreamFactory + createStreamFactory, + grantStreamPermissionsFactory } = require('@/modules/core/repositories/streams') const { VersionsEmitter } = require('@/modules/core/events/versionsEmitter') const { @@ -140,6 +140,7 @@ const createStream = legacyCreateStreamFactory({ projectsEventsEmitter: ProjectsEmitter.emit }) }) +const grantPermissionsStream = grantStreamPermissionsFactory({ db }) describe('Actors & Tokens @user-services', () => { const myTestActor = { diff --git a/packages/server/modules/serverinvites/graph/resolvers/serverInvites.ts b/packages/server/modules/serverinvites/graph/resolvers/serverInvites.ts index 683617ab3..cd9c0d1a5 100644 --- a/packages/server/modules/serverinvites/graph/resolvers/serverInvites.ts +++ b/packages/server/modules/serverinvites/graph/resolvers/serverInvites.ts @@ -57,8 +57,11 @@ import { processFinalizedProjectInviteFactory, validateProjectInviteBeforeFinalizationFactory } from '@/modules/serverinvites/services/coreFinalization' -import { addStreamInviteDeclinedActivityFactory } from '@/modules/activitystream/services/streamActivity' -import { addOrUpdateStreamCollaborator } from '@/modules/core/services/streams/streamAccessService' +import { + addStreamInviteAcceptedActivityFactory, + addStreamInviteDeclinedActivityFactory, + addStreamPermissionsAddedActivityFactory +} from '@/modules/activitystream/services/streamActivity' import { createUserEmailFactory, ensureNoPrimaryEmailForUserFactory, @@ -72,8 +75,31 @@ import { renderEmail } from '@/modules/emails/services/emailRendering' import { sendEmail } from '@/modules/emails/services/sending' import { publish } from '@/modules/shared/utils/subscriptions' import { saveActivityFactory } from '@/modules/activitystream/repositories' -import { getStreamFactory } from '@/modules/core/repositories/streams' +import { + getStreamFactory, + grantStreamPermissionsFactory +} from '@/modules/core/repositories/streams' +import { + addOrUpdateStreamCollaboratorFactory, + validateStreamAccessFactory +} from '@/modules/core/services/streams/access' +const saveActivity = saveActivityFactory({ db }) +const validateStreamAccess = validateStreamAccessFactory({ authorizeResolver }) + +const addOrUpdateStreamCollaborator = addOrUpdateStreamCollaboratorFactory({ + validateStreamAccess, + getUser, + grantStreamPermissions: grantStreamPermissionsFactory({ db }), + addStreamInviteAcceptedActivity: addStreamInviteAcceptedActivityFactory({ + saveActivity, + publish + }), + addStreamPermissionsAddedActivity: addStreamPermissionsAddedActivityFactory({ + saveActivity, + publish + }) +}) const getStream = getStreamFactory({ db }) const requestNewEmailVerification = requestNewEmailVerificationFactory({ findEmail: findEmailFactory({ db }), diff --git a/packages/server/modules/serverinvites/services/coreFinalization.ts b/packages/server/modules/serverinvites/services/coreFinalization.ts index 67f2c7dd9..80fde78f3 100644 --- a/packages/server/modules/serverinvites/services/coreFinalization.ts +++ b/packages/server/modules/serverinvites/services/coreFinalization.ts @@ -1,8 +1,10 @@ import { AddStreamInviteDeclinedActivity } from '@/modules/activitystream/domain/operations' -import { GetStream } from '@/modules/core/domain/streams/operations' +import { + AddOrUpdateStreamCollaborator, + GetStream +} from '@/modules/core/domain/streams/operations' import { StreamInvalidAccessError } from '@/modules/core/errors/stream' import { isResourceAllowed } from '@/modules/core/helpers/token' -import { addOrUpdateStreamCollaborator } from '@/modules/core/services/streams/streamAccessService' import { ProjectInviteResourceType } from '@/modules/serverinvites/domain/constants' import { InviteFinalizingError } from '@/modules/serverinvites/errors' import { @@ -76,7 +78,7 @@ export const validateProjectInviteBeforeFinalizationFactory = type ProcessFinalizedProjectInviteFactoryDeps = { getProject: GetStream addInviteDeclinedActivity: AddStreamInviteDeclinedActivity - addProjectRole: typeof addOrUpdateStreamCollaborator + addProjectRole: AddOrUpdateStreamCollaborator } export const processFinalizedProjectInviteFactory = diff --git a/packages/server/modules/serverinvites/tests/invites.spec.ts b/packages/server/modules/serverinvites/tests/invites.spec.ts index fb59efea8..cab20436c 100644 --- a/packages/server/modules/serverinvites/tests/invites.spec.ts +++ b/packages/server/modules/serverinvites/tests/invites.spec.ts @@ -40,9 +40,9 @@ import { StreamInviteCreateInput, UseStreamInviteDocument } from '@/test/graphql/generated/graphql' -import { grantStreamPermissions } from '@/modules/core/repositories/streams' import { ServerInviteRecord } from '@/modules/serverinvites/domain/types' import { reduce } from 'lodash' +import { grantStreamPermissionsFactory } from '@/modules/core/repositories/streams' async function cleanup() { await truncateTables([ServerInvites.name, Streams.name, Users.name]) @@ -50,6 +50,7 @@ async function cleanup() { const findInvite = findInviteFactory({ db }) const createInviteDirectly = createStreamInviteDirectly +const grantStreamPermissions = grantStreamPermissionsFactory({ db }) const mailerMock = EmailSendingServiceMock diff --git a/packages/server/modules/webhooks/tests/webhooks.spec.js b/packages/server/modules/webhooks/tests/webhooks.spec.js index b7919e239..9da6f1323 100644 --- a/packages/server/modules/webhooks/tests/webhooks.spec.js +++ b/packages/server/modules/webhooks/tests/webhooks.spec.js @@ -10,7 +10,6 @@ const { const { noErrors } = require('@/test/helpers') const { createPersonalAccessToken } = require('../../core/services/tokens') const { createUser } = require('../../core/services/users') -const { grantPermissionsStream } = require('../../core/services/streams') const { Scopes, Roles } = require('@speckle/shared') const { createWebhookConfigFactory, @@ -34,7 +33,8 @@ const { getServerInfo } = require('@/modules/core/services/generic') const { getUser, getUsers } = require('@/modules/core/repositories/users') const { getStreamFactory, - createStreamFactory + createStreamFactory, + grantStreamPermissionsFactory } = require('@/modules/core/repositories/streams') const { legacyCreateStreamFactory, @@ -100,6 +100,7 @@ const createStream = legacyCreateStreamFactory({ projectsEventsEmitter: ProjectsEmitter.emit }) }) +const grantPermissionsStream = grantStreamPermissionsFactory({ db }) describe('Webhooks @webhooks', () => { const getWebhook = getWebhookByIdFactory({ db }) diff --git a/packages/server/modules/workspaces/graph/resolvers/workspaces.ts b/packages/server/modules/workspaces/graph/resolvers/workspaces.ts index 43f88176d..3e349704e 100644 --- a/packages/server/modules/workspaces/graph/resolvers/workspaces.ts +++ b/packages/server/modules/workspaces/graph/resolvers/workspaces.ts @@ -10,7 +10,9 @@ import { upsertProjectRoleFactory, getRolesByUserIdFactory, getStreamFactory, - deleteStreamFactory + deleteStreamFactory, + revokeStreamPermissionsFactory, + grantStreamPermissionsFactory } from '@/modules/core/repositories/streams' import { getUser, getUsers } from '@/modules/core/repositories/users' import { getStreams } from '@/modules/core/services/streams' @@ -130,11 +132,24 @@ import { isUserWorkspaceDomainPolicyCompliantFactory } from '@/modules/workspaces/services/domains' import { getServerInfo } from '@/modules/core/services/generic' -import { updateStreamRoleAndNotify } from '@/modules/core/services/streams/management' import { deleteOldAndInsertNewVerificationFactory } from '@/modules/emails/repositories' import { renderEmail } from '@/modules/emails/services/emailRendering' import { sendEmail } from '@/modules/emails/services/sending' import { parseDefaultProjectRole } from '@/modules/workspaces/domain/logic' +import { saveActivityFactory } from '@/modules/activitystream/repositories' +import { + addOrUpdateStreamCollaboratorFactory, + isStreamCollaboratorFactory, + removeStreamCollaboratorFactory, + validateStreamAccessFactory +} from '@/modules/core/services/streams/access' +import { + addStreamInviteAcceptedActivityFactory, + addStreamPermissionsAddedActivityFactory, + addStreamPermissionsRevokedActivityFactory +} from '@/modules/activitystream/services/streamActivity' +import { publish } from '@/modules/shared/utils/subscriptions' +import { updateStreamRoleAndNotifyFactory } from '@/modules/core/services/streams/management' const getStream = getStreamFactory({ db }) const requestNewEmailVerification = requestNewEmailVerificationFactory({ @@ -185,6 +200,37 @@ const buildCreateAndSendWorkspaceInvite = () => }) }) const deleteStream = deleteStreamFactory({ db }) +const saveActivity = saveActivityFactory({ db }) +const validateStreamAccess = validateStreamAccessFactory({ authorizeResolver }) +const isStreamCollaborator = isStreamCollaboratorFactory({ + getStream +}) +const removeStreamCollaborator = removeStreamCollaboratorFactory({ + validateStreamAccess, + isStreamCollaborator, + revokeStreamPermissions: revokeStreamPermissionsFactory({ db }), + addStreamPermissionsRevokedActivity: addStreamPermissionsRevokedActivityFactory({ + saveActivity, + publish + }) +}) +const updateStreamRoleAndNotify = updateStreamRoleAndNotifyFactory({ + isStreamCollaborator, + addOrUpdateStreamCollaborator: addOrUpdateStreamCollaboratorFactory({ + validateStreamAccess, + getUser, + grantStreamPermissions: grantStreamPermissionsFactory({ db }), + addStreamInviteAcceptedActivity: addStreamInviteAcceptedActivityFactory({ + saveActivity, + publish + }), + addStreamPermissionsAddedActivity: addStreamPermissionsAddedActivityFactory({ + saveActivity, + publish + }) + }), + removeStreamCollaborator +}) const { FF_WORKSPACES_MODULE_ENABLED } = getFeatureFlags() diff --git a/packages/server/modules/workspaces/tests/integration/invites.graph.spec.ts b/packages/server/modules/workspaces/tests/integration/invites.graph.spec.ts index 19fed22b2..e74635d24 100644 --- a/packages/server/modules/workspaces/tests/integration/invites.graph.spec.ts +++ b/packages/server/modules/workspaces/tests/integration/invites.graph.spec.ts @@ -74,11 +74,25 @@ import { } from '@/modules/core/repositories/userEmails' import { markUserEmailAsVerifiedFactory } from '@/modules/core/services/users/emailVerification' import { createRandomPassword } from '@/modules/core/helpers/testHelpers' -import { addOrUpdateStreamCollaborator } from '@/modules/core/services/streams/streamAccessService' import { WorkspaceProtectedError } from '@/modules/workspaces/errors/workspace' import { ForbiddenError } from '@/modules/shared/errors' import cryptoRandomString from 'crypto-random-string' -import { getStreamFactory } from '@/modules/core/repositories/streams' +import { + getStreamFactory, + grantStreamPermissionsFactory +} from '@/modules/core/repositories/streams' +import { saveActivityFactory } from '@/modules/activitystream/repositories' +import { + addOrUpdateStreamCollaboratorFactory, + validateStreamAccessFactory +} from '@/modules/core/services/streams/access' +import { authorizeResolver } from '@/modules/shared' +import { getUser } from '@/modules/core/repositories/users' +import { + addStreamInviteAcceptedActivityFactory, + addStreamPermissionsAddedActivityFactory +} from '@/modules/activitystream/services/streamActivity' +import { publish } from '@/modules/shared/utils/subscriptions' enum InviteByTarget { Email = 'email', @@ -88,6 +102,22 @@ enum InviteByTarget { type TestGraphQLOperations = ReturnType const getStream = getStreamFactory({ db }) +const saveActivity = saveActivityFactory({ db }) +const validateStreamAccess = validateStreamAccessFactory({ authorizeResolver }) + +const addOrUpdateStreamCollaborator = addOrUpdateStreamCollaboratorFactory({ + validateStreamAccess, + getUser, + grantStreamPermissions: grantStreamPermissionsFactory({ db }), + addStreamInviteAcceptedActivity: addStreamInviteAcceptedActivityFactory({ + saveActivity, + publish + }), + addStreamPermissionsAddedActivity: addStreamPermissionsAddedActivityFactory({ + saveActivity, + publish + }) +}) const buildGraphqlOperations = (deps: { apollo: TestApolloServer }) => { const { apollo } = deps diff --git a/packages/server/modules/workspaces/tests/integration/projects.graph.spec.ts b/packages/server/modules/workspaces/tests/integration/projects.graph.spec.ts index d842783ac..0e9aba2b8 100644 --- a/packages/server/modules/workspaces/tests/integration/projects.graph.spec.ts +++ b/packages/server/modules/workspaces/tests/integration/projects.graph.spec.ts @@ -1,5 +1,6 @@ +import { db } from '@/db/knex' import { AllScopes } from '@/modules/core/helpers/mainConstants' -import { grantStreamPermissions } from '@/modules/core/repositories/streams' +import { grantStreamPermissionsFactory } from '@/modules/core/repositories/streams' import { BasicTestWorkspace, createTestWorkspace @@ -27,6 +28,8 @@ import { Roles } from '@speckle/shared' import { expect } from 'chai' import cryptoRandomString from 'crypto-random-string' +const grantStreamPermissions = grantStreamPermissionsFactory({ db }) + describe('Workspace project GQL CRUD', () => { let apollo: TestApolloServer diff --git a/packages/server/modules/workspaces/tests/integration/repositories.spec.ts b/packages/server/modules/workspaces/tests/integration/repositories.spec.ts index a05d68848..9ef797ef8 100644 --- a/packages/server/modules/workspaces/tests/integration/repositories.spec.ts +++ b/packages/server/modules/workspaces/tests/integration/repositories.spec.ts @@ -38,7 +38,7 @@ import { import { truncateTables } from '@/test/hooks' import { createTestStream } from '@/test/speckle-helpers/streamHelper' import { - grantStreamPermissions, + grantStreamPermissionsFactory, upsertProjectRoleFactory } from '@/modules/core/repositories/streams' import { omit } from 'lodash' @@ -58,6 +58,7 @@ const createUserEmail = createUserEmailFactory({ db }) const updateUserEmail = updateUserEmailFactory({ db }) const getUserDiscoverableWorkspaces = getUserDiscoverableWorkspacesFactory({ db }) const upsertProjectRole = upsertProjectRoleFactory({ db }) +const grantStreamPermissions = grantStreamPermissionsFactory({ db }) const createAndStoreTestUser = async (): Promise => { const testId = cryptoRandomString({ length: 6 }) diff --git a/packages/server/modules/workspaces/tests/integration/roles.graph.spec.ts b/packages/server/modules/workspaces/tests/integration/roles.graph.spec.ts index 8bdc06c1d..e149b9650 100644 --- a/packages/server/modules/workspaces/tests/integration/roles.graph.spec.ts +++ b/packages/server/modules/workspaces/tests/integration/roles.graph.spec.ts @@ -1,5 +1,6 @@ +import { db } from '@/db/knex' import { AllScopes } from '@/modules/core/helpers/mainConstants' -import { grantStreamPermissions } from '@/modules/core/repositories/streams' +import { grantStreamPermissionsFactory } from '@/modules/core/repositories/streams' import { assignToWorkspace, BasicTestWorkspace, @@ -29,6 +30,8 @@ import { expect } from 'chai' import cryptoRandomString from 'crypto-random-string' import { isUndefined } from 'lodash' +const grantStreamPermissions = grantStreamPermissionsFactory({ db }) + describe('Workspaces Roles GQL', () => { let apollo: TestApolloServer diff --git a/packages/server/modules/workspaces/tests/integration/workspaces.graph.spec.ts b/packages/server/modules/workspaces/tests/integration/workspaces.graph.spec.ts index 9f74cb597..50795fa2c 100644 --- a/packages/server/modules/workspaces/tests/integration/workspaces.graph.spec.ts +++ b/packages/server/modules/workspaces/tests/integration/workspaces.graph.spec.ts @@ -49,8 +49,10 @@ import { createRandomString } from '@/modules/core/helpers/testHelpers' import { getBranchesByStreamId } from '@/modules/core/services/branches' -import { grantStreamPermissions } from '@/modules/core/repositories/streams' import { getWorkspaceFactory } from '@/modules/workspaces/repositories/workspaces' +import { grantStreamPermissionsFactory } from '@/modules/core/repositories/streams' + +const grantStreamPermissions = grantStreamPermissionsFactory({ db }) const createProjectWithVersions = ({ apollo }: { apollo: TestApolloServer }) => diff --git a/packages/server/test/speckle-helpers/automationHelper.ts b/packages/server/test/speckle-helpers/automationHelper.ts index 294b28e98..81334c311 100644 --- a/packages/server/test/speckle-helpers/automationHelper.ts +++ b/packages/server/test/speckle-helpers/automationHelper.ts @@ -40,13 +40,15 @@ import { import { buildDecryptor } from '@/modules/shared/utils/libsodium' import { db } from '@/db/knex' import { AutomationsEmitter } from '@/modules/automate/events/automations' -import { validateStreamAccess } from '@/modules/core/services/streams/streamAccessService' +import { validateStreamAccessFactory } from '@/modules/core/services/streams/access' +import { authorizeResolver } from '@/modules/shared' const storeAutomation = storeAutomationFactory({ db }) const storeAutomationToken = storeAutomationTokenFactory({ db }) const storeAutomationRevision = storeAutomationRevisionFactory({ db }) const getAutomation = getAutomationFactory({ db }) const getLatestStreamBranch = getLatestStreamBranchFactory({ db }) +const validateStreamAccess = validateStreamAccessFactory({ authorizeResolver }) export const generateFunctionId = () => cryptoRandomString({ length: 10 }) export const generateFunctionReleaseId = () => cryptoRandomString({ length: 10 }) diff --git a/packages/server/test/speckle-helpers/streamHelper.ts b/packages/server/test/speckle-helpers/streamHelper.ts index a21eca173..86a6d7fe2 100644 --- a/packages/server/test/speckle-helpers/streamHelper.ts +++ b/packages/server/test/speckle-helpers/streamHelper.ts @@ -1,20 +1,28 @@ import { db } from '@/db/knex' import { saveActivityFactory } from '@/modules/activitystream/repositories' -import { addStreamCreatedActivityFactory } from '@/modules/activitystream/services/streamActivity' +import { + addStreamCreatedActivityFactory, + addStreamPermissionsRevokedActivityFactory +} from '@/modules/activitystream/services/streamActivity' import { StreamAcl } from '@/modules/core/dbSchema' import { ProjectsEmitter } from '@/modules/core/events/projectsEmitter' import { StreamAclRecord, StreamRecord } from '@/modules/core/helpers/types' import { createBranchFactory } from '@/modules/core/repositories/branches' import { createStreamFactory, - getStreamFactory + getStreamFactory, + revokeStreamPermissionsFactory } from '@/modules/core/repositories/streams' import { getUsers } from '@/modules/core/repositories/users' +import { + isStreamCollaboratorFactory, + removeStreamCollaboratorFactory, + validateStreamAccessFactory +} from '@/modules/core/services/streams/access' import { createStreamReturnRecordFactory, legacyCreateStreamFactory } from '@/modules/core/services/streams/management' -import { removeStreamCollaborator } from '@/modules/core/services/streams/streamAccessService' import { findUserByTargetFactory, insertInviteAndDeleteOldFactory @@ -23,6 +31,7 @@ import { buildCoreInviteEmailContentsFactory } from '@/modules/serverinvites/ser import { collectAndValidateCoreTargetsFactory } from '@/modules/serverinvites/services/coreResourceCollection' import { createAndSendInviteFactory } from '@/modules/serverinvites/services/creation' import { inviteUsersToProjectFactory } from '@/modules/serverinvites/services/projectInviteManagement' +import { authorizeResolver } from '@/modules/shared' import { Nullable } from '@/modules/shared/helpers/typeHelper' import { getEventBus } from '@/modules/shared/services/eventBus' import { publish } from '@/modules/shared/utils/subscriptions' @@ -62,6 +71,21 @@ const createStream = legacyCreateStreamFactory({ }) }) +const saveActivity = saveActivityFactory({ db }) +const validateStreamAccess = validateStreamAccessFactory({ authorizeResolver }) +const isStreamCollaborator = isStreamCollaboratorFactory({ + getStream +}) +const removeStreamCollaborator = removeStreamCollaboratorFactory({ + validateStreamAccess, + isStreamCollaborator, + revokeStreamPermissions: revokeStreamPermissionsFactory({ db }), + addStreamPermissionsRevokedActivity: addStreamPermissionsRevokedActivityFactory({ + saveActivity, + publish + }) +}) + export type BasicTestStream = { name: string isPublic: boolean