Skip to content

Various fixes #6

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Nov 11, 2019
Merged

Various fixes #6

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e189bbe
fix: "Cannot read property 'hits' of undefined" in Project Service
maxceem Nov 1, 2019
b960306
fix: member role validation on create and update
maxceem Nov 4, 2019
141e6da
fix: create productCategory validation
maxceem Nov 4, 2019
436b900
fix: resource names match ones in "project-service"
maxceem Nov 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions src/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ const RESOURCES = {
PROJECT: 'project',
PROJECT_TEMPLATE: 'project.template',
PROJECT_TYPE: 'project.type',
PROJECT_MEMBER: 'project.member',
PROJECT_MEMBER_INVITE: 'project.member.invite',
ORG_CONFIG: 'project.orgConfig',
FORM_VERSION: 'project.form.version',
FORM_REVISION: 'project.form.revision',
Expand All @@ -25,14 +27,12 @@ const RESOURCES = {
PLAN_CONFIG_REVISION: 'project.planConfig.revision',
PRODUCT_TEMPLATE: 'product.template',
PRODUCT_CATEGORY: 'product.category',
ATTACHMENT: 'attachment',
PHASE: 'phase',
PROJECT_MEMBER: 'project.member',
PHASE_PRODUCT: 'phase.product',
PHASE: 'project.phase',
PHASE_PRODUCT: 'project.phase.product',
TIMELINE: 'timeline',
MILESTONE: 'milestone',
MILESTONE_TEMPLATE: 'milestone.template',
PROJECT_MEMBER_INVITE: 'project.member.invite'
ATTACHMENT: 'attachment'
}

const TIMELINE_REFERENCES = {
Expand All @@ -56,7 +56,11 @@ const PROJECT_MEMBER_ROLE = {
OBSERVER: 'observer',
CUSTOMER: 'customer',
COPILOT: 'copilot',
ACCOUNT_MANAGER: 'account_manager'
ACCOUNT_MANAGER: 'account_manager',
PROGRAM_MANAGER: 'program_manager',
ACCOUNT_EXECUTIVE: 'account_executive',
SOLUTION_ARCHITECT: 'solution_architect',
PROJECT_MANAGER: 'project_manager'
}

const MILESTONE_TEMPLATE_REFERENCES = {
Expand Down
18 changes: 9 additions & 9 deletions src/services/ProcessorServiceMilestoneTemplate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,17 +68,17 @@ function createSchema () {
async function create (message) {
// handle ES Update
async function updateDocPromise (doc) {
const milestoneTemplate = _.isArray(doc._source.milestoneTemplate) ? doc._source.milestoneTemplate : []
const milestoneTemplates = _.isArray(doc._source.milestoneTemplates) ? doc._source.milestoneTemplates : []

const existingMilestoneTemplateIndex = _.findIndex(milestoneTemplate, p => p.id === message.id)// if milestone template does not exists already
const existingMilestoneTemplateIndex = _.findIndex(milestoneTemplates, p => p.id === message.id)// if milestone template does not exists already
if (existingMilestoneTemplateIndex === -1) {
milestoneTemplate.push(message)
milestoneTemplates.push(message)
} else { // if milestone template already exists, ideally we should never land here, but code handles the buggy indexing
// replaces the old inconsistent index where previously milestone template was not removed from the index but deleted
// from the database
milestoneTemplate.splice(existingMilestoneTemplateIndex, 1, message)
milestoneTemplates.splice(existingMilestoneTemplateIndex, 1, message)
}
return _.assign(doc._source, { milestoneTemplate })
return _.assign(doc._source, { milestoneTemplates })
}

await helper.updateMetadadaESPromise(updateDocPromise)
Expand All @@ -97,14 +97,14 @@ create.schema = {
async function update (message) {
// handle ES Update
async function updateDocPromise (doc) {
const milestoneTemplate = _.map(doc._source.milestoneTemplate, (single) => {
const milestoneTemplates = _.map(doc._source.milestoneTemplates, (single) => {
if (single.id === message.id) {
return _.assign(single, message)
}
return single
})

return _.assign(doc._source, { milestoneTemplate })
return _.assign(doc._source, { milestoneTemplates })
}

await helper.updateMetadadaESPromise(updateDocPromise)
Expand All @@ -123,8 +123,8 @@ update.schema = {
async function deleteMessage (message) {
// handle ES Update
async function updateDocPromise (doc) {
const milestoneTemplate = _.filter(doc._source.milestoneTemplate, single => single.id !== message.id)
return _.assign(doc._source, { milestoneTemplate })
const milestoneTemplates = _.filter(doc._source.milestoneTemplates, single => single.id !== message.id)
return _.assign(doc._source, { milestoneTemplates })
}

await helper.updateMetadadaESPromise(updateDocPromise)
Expand Down
1 change: 0 additions & 1 deletion src/services/ProcessorServiceProductCategory.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ async function create (message) {

create.schema = {
message: createSchema()
.xor('form', 'template')
}

/**
Expand Down
8 changes: 4 additions & 4 deletions src/services/ProcessorServiceProjectMember.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,9 @@ function createIdSchema () {
function updateSchema () {
return createIdSchema().keys({
isPrimary: Joi.boolean(),
role: Joi.any().valid(PROJECT_MEMBER_ROLE.CUSTOMER, PROJECT_MEMBER_ROLE.MANAGER,
PROJECT_MEMBER_ROLE.ACCOUNT_MANAGER, PROJECT_MEMBER_ROLE.COPILOT, PROJECT_MEMBER_ROLE.OBSERVER).required()
// unlike in Project Service endpoints, in this processor we should let create members with any roles
// because Project Service may create members with any roles implicitly, when accepting invitations
role: Joi.string().valid(_.values(PROJECT_MEMBER_ROLE)).required()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maxceem can it create any security loop hole? I mean if some one is able to raise the bus event with data which is not allowed by the project service, then if we don't have checks here, it might get through. However, on second thoughts, I think sending events to bus is not possible without authentication so we should be safe. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion, this place doesn't bring an additional security loop. If someone happens to be able to send arbitrary messages to the Bus Api or Kafka, he could do much bigger harm than adding a member: removing or updating projects or any other arbitrary data which we store in ES. Also, he can still update or remove members.

We may still think if there is any chance someone could gain access to the Bus Api or Kafka because if so, it gives almost full control over the data we have inside ES.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.

})
}

Expand All @@ -38,8 +39,7 @@ function updateSchema () {
*/
function createSchema () {
return createIdSchema().keys({
role: Joi.any()
.valid(PROJECT_MEMBER_ROLE.MANAGER, PROJECT_MEMBER_ROLE.ACCOUNT_MANAGER, PROJECT_MEMBER_ROLE.COPILOT)
role: Joi.string().valid(_.values(PROJECT_MEMBER_ROLE))
})
}

Expand Down
10 changes: 5 additions & 5 deletions test/e2e/processor.metadata.index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,23 +66,23 @@ describe('TC Milestone Template Topic Tests', () => {
it('create milestone template message', async () => {
await ProcessorService.create(milestoneTemplateCreatedMessage)
const data = await testHelper.getMetadataESData(metadataId)
testHelper.expectObj(_.find(data.milestoneTemplate, { id: milestoneTemplateId }),
testHelper.expectObj(_.find(data.milestoneTemplates, { id: milestoneTemplateId }),
milestoneTemplateCreatedMessage.payload,
_.keys(_.omit(milestoneTemplateCreatedMessage.payload, ['resource'])))
})

it('create milestone template message - already exists', async () => {
await ProcessorService.create(milestoneTemplateCreatedMessage)
const data = await testHelper.getMetadataESData(metadataId)
testHelper.expectObj(_.find(data.milestoneTemplate, { id: milestoneTemplateId }),
testHelper.expectObj(_.find(data.milestoneTemplates, { id: milestoneTemplateId }),
milestoneTemplateCreatedMessage.payload,
_.keys(_.omit(milestoneTemplateCreatedMessage.payload, ['resource'])))
})

it('update milestone template message', async () => {
await ProcessorService.update(milestoneTemplateUpdatedMessage)
const data = await testHelper.getMetadataESData(metadataId)
testHelper.expectObj(_.find(data.milestoneTemplate, { id: milestoneTemplateId }),
testHelper.expectObj(_.find(data.milestoneTemplates, { id: milestoneTemplateId }),
milestoneTemplateUpdatedMessage.payload,
_.keys(_.omit(milestoneTemplateUpdatedMessage.payload, ['resource'])))
})
Expand All @@ -92,13 +92,13 @@ describe('TC Milestone Template Topic Tests', () => {
message.payload.id = notFoundId
await ProcessorService.update(message)
const data = await testHelper.getMetadataESData(metadataId)
expect(_.find(data.milestoneTemplate, { id: notFoundId })).to.be.an('undefined')
expect(_.find(data.milestoneTemplates, { id: notFoundId })).to.be.an('undefined')
})

it('delete milestone template message', async () => {
await ProcessorService.deleteMessage(milestoneTemplateDeletedMessage)
const data = await testHelper.getMetadataESData(metadataId)
expect(_.find(data.milestoneTemplate, { id: milestoneTemplateId })).to.be.an('undefined')
expect(_.find(data.milestoneTemplates, { id: milestoneTemplateId })).to.be.an('undefined')
})
})

Expand Down