Skip to content
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.

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

Remove ftp functionality #2064

Merged
merged 20 commits into from
Nov 25, 2024

Conversation

ScottDormand96
Copy link
Collaborator

@ScottDormand96 ScottDormand96 commented Oct 9, 2024

https://eaflood.atlassian.net/browse/IWTF-4278

Remove ftp functionality as package we use in fulfilment and pocl jobs (ssh2-sftp-client) has critical vulnerability

https://eaflood.atlassian.net/browse/IWTF-4278

Remove ftp functionality as package we use in fulfilment and pocl jobs (ssh2-sftp-client) has critical vulnerability
@ScottDormand96 ScottDormand96 added the enhancement New feature or request label Oct 9, 2024
Copy link
Member

@irisfaraway irisfaraway left a comment

Choose a reason for hiding this comment

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

Now that's the kind of line count diff I like to see, hahaha.

One suggested change to do with the DD process.

Comment on lines 51 to 56
const dynamicsRecord = await salesApi.getTransactionFile(filename)
if (!dynamicsRecord || !DYNAMICS_IMPORT_STAGE.isAlreadyProcessed(dynamicsRecord.status.description)) {
await storeS3Metadata(file.ETag, filesize(file.Size), filename, file.Key, moment(new Date(file.LastModified)))
} else {
console.log(`${file.Key} is already processed, skipping`)
}
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if we want to delete this and the storeS3Metadata function – I think this can still be used for other file uploads as well and we don't want to blow up the direct debit process. However if we do keep it then it might make sense to move that function out of a file called ftp-to-s3.js anyway.

Copy link
Collaborator

@jaucourt jaucourt left a comment

Choose a reason for hiding this comment

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

LGTM, but let's wait until Iris is back and can re-review than the s3 changes have been made to her satisfaction

Copy link
Collaborator

@jaucourt jaucourt left a comment

Choose a reason for hiding this comment

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

One little thing to change

'3des-cbc',
'cast128-cbc'
]

class Config {
_db
_ftp
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this is needed anymore either, is it?

Copy link

sonarqubecloud bot commented Nov 8, 2024

@ScottDormand96 ScottDormand96 merged commit 42f1d84 into develop Nov 25, 2024
3 checks passed
@ScottDormand96 ScottDormand96 deleted the feature/iwtf-4278-remove-ftp-functionality branch November 25, 2024 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants