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

Add relation between import configurations and clients #6124

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

solth
Copy link
Member

@solth solth commented Jul 12, 2024

Since import configurations can contain sensitive data like FTP credentials, they shouldn't be freely shared between clients. This pull request adds a relation between clients and import configurations similar to that between clients and projects, so that import configurations are explicitely mapped to specific lists of clients and users can only see and use those import configurations that are mapped to at least one of their assigned clients.

(since XSLT mapping files normally do not contain such sensitive data and are - like rulesets - available to all users via the file system anyway, a relation between clients and mapping files does not seem necessary and therefore has not been added)

The mapping between import configurations and clients can be edited via a new third tab named "Mandanten" ("Clients") on the import configuration edit page:

Bildschirmfoto 2024-07-12 um 14 05 53

The SQL migration file in this pull request that adds the corresponding cross table also assigns all existing import configurations to all existing clients by default to maintain the current status quo. An admin can then make use of the new functionality instroduced in this pull request and restrict access to specific configurations using the import configuration edit formular shown above.

Fixes #6059

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.

Code changes look good, but I have 2 functional questions:

  1. It is possible to de-assign all clients from an import configuration. This import configuration becomes invisible but is still existing. Without manipulating the database table it is not possible anymore to interact with this hidden import configuration. In my opinion at least one client should be assigned to an import configuration.

  2. If you have at least two clients Client_A and Client_B. You logged in in Client_A and de-assign an import configuration from Client_A so this import configuration should only assigned to Client_B, the import configuration is still visible while logged in in Client_A (even after re-log in). This behaviour is different to all the other client-depending relations like projects, process templates, workflows, ... Should this be the case?

@solth
Copy link
Member Author

solth commented Jul 16, 2024

@henning-gerhardt thanks a lot for the remarks!

  1. It is possible to de-assign all clients from an import configuration. This import configuration becomes invisible but is still existing. Without manipulating the database table it is not possible anymore to interact with this hidden import configuration. In my opinion at least one client should be assigned to an import configuration.

Thanks for pointing this out. Indeed, it would become difficult to re-assign an import configuration that is not assigned to any client anymore. I see two ways to approach this:

  • either add a special permission/authority that allows users to see all import configurations, no matter to which client(s) they are assigned. This way an "System admin" could be defined that can configure import configurations system wide without any client restrictions
  • or add some frontend logic that disables an item in the client list when it is the last selected item, to prevent the situation you described, where an import configuration isn't assigned to any client anymore. This approach would have to take into account that a user might only be assigned to some clients (not all of them) and an import configuration might still be assigned to a client to which the user is not assigned; this could also be validated when saving an import configuration.

What would be the better solution? Or do you have an alternative idea how to resolve this issue?

  1. If you have at least two clients Client_A and Client_B. You logged in in Client_A and de-assign an import configuration from Client_A so this import configuration should only assigned to Client_B, the import configuration is still visible while logged in in Client_A (even after re-log in). This behaviour is different to all the other client-depending relations like projects, process templates, workflows, ... Should this be the case?

This indeed sounds like a bug, e.g. wasn't intended. I will try to find the cause for this issue and resolve it. I got this wrong. I programmed it deliberately like this, but didn't realize the other object types you mentioned - project, template etc. - behaved differently. I guess if we can solve the first point you raised it shouldn't be a problem to only show those import configurations that are assigned to the current client and bring the functionality more in line with the rest of the system.

@henning-gerhardt
Copy link
Collaborator

I would go for the first option (special authority to "see" all available clients) even this behaviour is different then an other places like projects, process templates, ... but I think that sharing import configurations is more common than sharing projects, process templates, ... between clients.

@henning-gerhardt
Copy link
Collaborator

After a discussion between @solth and @henning-gerhardt we come to the following solution:

  • adding a new permission/authority to see all clients inside the import configuration independent of which your current client is
  • adding a custon validator which ensures that at least one client is assigned to an import configuration

@solth
Copy link
Member Author

solth commented Jul 16, 2024

@henning-gerhardt I updated the pull request as discussed: a new special permission was added that allows assigning import configurations to clients globally. The corresponding and new third "Clients" tab in the "Import configuration edit" page is now only rendered when the user has this new permission. On this third tab all clients are displayed and offered for assignement (not just those assigned to the current user/admin). Additionally, I added a new validator that ensures at least one client is assigned when saving the import configuration.

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 and new feature is working as defined.

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.

Import configurations not filtered by client
2 participants