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

Run Kitodo Script commands via Active MQ #6013

Merged
merged 13 commits into from
Aug 23, 2024
Merged

Conversation

matthias-ronge
Copy link
Collaborator

@matthias-ronge matthias-ronge commented Mar 25, 2024

This development allows to run Kitodo Script commands using the Active MQ interface.

For example, to export processes, you send a Map Message with the content:

id        := "ID string used with Active MQ logging. Must be present, but is meaningless"
script    := "action:export exportImages:true"
processes := 4

processes can be a single integer, or string representing an integer, or a list of either, or string with several integers separated by non-digit character(s).

Resolves #5980

This development is funded by the Municipality of The Hague—Construction Office.

@matthias-ronge matthias-ronge marked this pull request as ready for review March 26, 2024 10:45
Copy link
Member

@solth solth left a comment

Choose a reason for hiding this comment

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

Looks good (apart from some small suggestions), but lacks a test. Please provide a test for this new functionality and update the documentation in the wiki.

Copy link
Member

@solth solth left a comment

Choose a reason for hiding this comment

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

Thank you for incorporating the change requests.

@solth solth requested a review from henning-gerhardt August 9, 2024 10:24
Copy link
Collaborator

@henning-gerhardt henning-gerhardt left a comment

Choose a reason for hiding this comment

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

Changes looks good but I can not test the changes by real samples but the tests should cover the introduced new processor. After merging the documentation https://github.com/kitodo/kitodo-production/wiki/Developer_3.x-Active-MQ should / must be updated for the new ActiveMQ processor so anyone can create the correct message queue messages.

@henning-gerhardt
Copy link
Collaborator

A hopefully important information and maybe discussion decision:

It could be a dangerous to enable feature through the configuration option as any KitodoScript can be executed for an unlimited list of process ids. You only needed access to the used ActiveMQ server and send to this queue a message. There is no restriction like in the UI where you need at least a login and proper rights to execute KitodoScripts for amount of or all projects and their processes.

As everyone how can send ActiveMQ messages to that queue can execute the "hidden" KitodoScript action deleteProcess with a unlimited number of process ids and with this an anonymous person can delete all the data. Until now it was not necessary to introduce a security concept on external accessible APIs like in the UI but with introducing this possibility it is needed. With the current not really existing security concept you can only enable this feature including dangerous KitodoScripts or you can not use this feature at all. Deleting this KitodoScript deleteProcess is a bad option as is could be really needed if f.e. 300 - 400 wrong created newspaper issues must be deleted. It is the only way to do this without deleting process by process in the UI which take a very long time (even deleting a lot of processes through the KitodoScript needs a lot of time but deleting one by one takes "years" of manual deleting).

At least a note be applied to the part inside kitodo_config.properties which hints to the dangerous part of this feature. A security concept for using the application from outside - until now only for ActiveMQ but needed for future developments like HTTP based REST APIs - is needed and even the development of this security access system (may re-use existing concepts and implementations instead of re-do all this again with all possible bugs and security holes).

@matthias-ronge
Copy link
Collaborator Author

I can't really imagine that it happens by accident, that a Kitodo script is parameterized like that. Yes, and maybe it's even intended to delete processes, who knows. On our systems, only defined ports are accessible, so no one can activate anything other than localhost, not anything else connect to Active MQ. Of course, we can add an additional explanation to the kitodo_config.properties file, what do you think:

# You can provide a queue from which messages are read to run a Kitodo Script
# •-------------------------------------------------------------------------•
# | CAUTION: This can be used to use an unknown Kitodo script with the name |
# | "action:deleteProcesses", and this could result in data destruction!    |
# •-------------------------------------------------------------------------•
#activeMQ.kitodoScript.queue=KitodoProduction.KitodoScript.Queue

@henning-gerhardt
Copy link
Collaborator

The text looks good but I don't know if we really want to hint to the dangerous script call.

If you run everything on one host / virtual machine / real system and access everything only by localhost the risk is lower to to them who run this parts on different hosts / virtual machines / real systems as they need this splitting.

@matthias-ronge
Copy link
Collaborator Author

I take your concerns seriously and added a whitelist to the configuration so that only explicitly permitted commands can be started remotely. I think this should relieve all worries.

@henning-gerhardt
Copy link
Collaborator

I take your concerns seriously and added a whitelist to the configuration so that only explicitly permitted commands can be started remotely. I think this should relieve all worries.

Thanks. Such an allow list is a simple way to archive this - better then nothing at all.

@solth solth merged commit d838f8e into kitodo:master Aug 23, 2024
5 checks passed
@henning-gerhardt
Copy link
Collaborator

@matthias-ronge @solth The ActiveMQ developer documenation is not yet updated for this new feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API to trigger actions, e.g., DMS Export
3 participants