This issue has been kept as a record of the discussion.
Click to show original description
Problem
The presence of project integrations (like Jira, Jenkins, Datadog) as abstractions to transfer data to other platforms (or more minor functionality), by nature, is concerning security and compliance teams of high-compliance customers.
Customer feedback: "being able to disable by default and then pick what to re-enable would be ideal, and that behaviour wouldn't break in-use integrations so we could close the barn door and then be selective and allow security review on what integrations are enabled."
Proposed solution
Allow instance Admins to explicitly disable/enable chosen integrations for the instance from UI.
Radio button with two mutually exclusive options:
Allow all integrations
Allow only integrations on this allowlist
When the option "Allow only integrations on this allowlist" we show a list of all integrations with checkboxes, default to un-checked.
By default, all instances will have "Allow all integrations" option enabled.
If the Admin had chosen the "Allow only integrations on this allowlist" option, and had not enabled any integrations, we interpret that as the entire feature of integrations is disabled on the instance and remove the Settings > Integrations link from the sidenav on group and project levels, and 404 the integrations settings page.
If the Admin had chosen the "Allow only integrations on this allowlist" option, and had enabled at least one integration, we keep the Settings > Integrations link in the sidenav on group and project levels and grey out integrations not enabled by the Admin in UI on group and project level and show a tooltip with a message along the lines of "This integration has been disabled by your Administrator. Contact them to use it." for disabled integrations.
Attempting to edit a disabled integration would result in a 404.
When integrations are removed from the allow list, we will not set their active properties to false. But they must no longer be triggered. This means if the integration was to be later added to the allow list, these integrations would be active immediately.
In case Admin attempts to disable integration that is currently in use, a modal with a warning that this integration is in use and disabling it would break existing workflows should pop up.
This issue was automatically tagged with the label groupproject management by TanukiStan, a machine learning classification model, with a probability of 0.97.
If this label is incorrect, please tag this issue with the correct group label as well as automation:ml wrong to help TanukiStan learn from its mistakes.
Authors who do not have permission to update labels can use the @gitlab-bot label ~"group::<correct group name>" command, or leave the issue to be triaged by group leaders initially assigned by TanukiStan.
This message was generated automatically.
You're welcome to improve it.
To set expectations, GitLab product managers or team members can't make any promise if they will proceed with this.
However, we believe everyone can contribute,
and welcome you to work on this proposed change, feature or bug fix.
There is a bias for action,
so you don't need to wait. Try and spin up that merge request yourself.
If you need help doing so, we're always open to mentor you
to drive this change.
Hey Luke - that sounds like a very broad brush to solve a specific problem; our implementation does use network-level control to break some behaviours via blackholing specific paths and limiting traffic. The problem here is more that integrations create a simple path to a solution that a path isn't desirable, such as emitting metrics to third party for a customer that doesn't want this to be simplifed, as well as UI/UX - breaking things at a network level creates user friction. Our implementation to solve that is patching css to hide things we've broken, but that's a break glass scenario we try to avoid :)
@connelldave Thank you for clarifying! I have a feeling we had an issue for this topic already, but I haven't been able to find it after a couple of attempts. So let's keep this feature issue and if we find there is an issue for it later we can mark one of them as duplicates of the other.
Perhaps GitLab's cascading settings feature could be utilised for this, cascading from instance through to group:
The cascading settings framework allows groups to essentially inherit settings values from ancestors (parent group on up the group hierarchy) and from instance-level application settings. The framework also allows settings values to be enforced on groups lower in the hierarchy.
One thing to consider is how to handle the scenario when a new integration is added, and how that would interact with this feature. Perhaps the instances/groups can signal they want to review new integrations before deciding if they should be enabled or disabled.
I think that would be a helpful first step - it's the kind of sledgehammer approach compliance and security teams would appreciate, but users less so. For example the "external wiki" integration doesn't really fall into the same catagory as the Datadog integration.
Disabling all integrations entirely would be a helpful emergency button, but being able to disable by default and then pick what to re-enable would be ideal, and that behaviour wouldn't break in-use integrations so we could close the barn door and then be selective and allow security review on what integrations are enabled.
I see that as very important as Gitlab builds more integrations too - today's state might be compliant and reviewed, but how do I promise stakeholders that won't change as new integrations with third parties are released? I realise that's the nature of an evolving product, but it creates tension for high-compliance users not being able to pick and choose.
I understand that it is important to have as many integrations as possible, but in some cases this would represent a security issue.
For example, any integration that could take information to the outside would be considered an issue.
Another point is that giving the group owner the possibility of reactivating/resetting the integration (which was once disabled at the instance level) makes the issue even more critical.
So the request would be: Having the possibility of using some integrations and not others, removing them completely from the list.
@fzimmer@cbalane do you know if this is something we could set up specifically for dedicated?
Currently, even if the IP address is not allowed or blocked, the custom integration feature mentioned here overrides the rules in Switchboard.
I think this issue mentions a couple of different approaches but the most straightforward one seems to be to allow group / instance admins to explicitly disable some integrations types.
When that happens, these integrations will be added to the disabled_integrations list and will stop appearing in it (this is mostly backend btw). As an added improvement, we can consider hiding the "Integrations" menu (and not allowing access to /integrations pages) if there are no integrations available.
We have similar functionality in Category:Importers where users can choose which importers they want to enable / disable (the only difference is that all importers are disabled by default and need to be manually enabled):
Let's agree on the approach and scope (do we need it in both instance / group settings) of this issue first then we can add weights.
Let's wait for Luke and others to chip in first before finalizing the issue.
New integrations should be instance-wide disabled by default. SM instance Admins would need to review first and enable it if it should be used.
@m_frankiewicz Are we sure we want to implement this? IMO it sounds a bit too restrictive and makes using integrations much harder for most users (they need to enable them one-by-one before being able to use one). We also don't even enforce this sort of rule for webhooks which are technically a sort of generic integration and also send lots of data.
Disabled integrations should not appear on the list of available integrations
I just thought of an alternative solution for this. We can consider greying out (like a disabled button) and showing a tooltip along the lines of "This integration has been disabled by your Administrator. Contact them to use it.". This will make it clear to users that there exists such an integration but they can't use it (yet). If not, it might be difficult to know what the available integrations are, aka to differentiate between "there is no such integration" and "that integration is not allowed".
In case Admin wants to disable integration that is currently in use, they should receive a warning
This makes sense in terms of UX but I have a feeling it will be a pretty complicated thing to implement. Let's see what are the most high value things we want to implement and then consider improvements later.
Are we sure we want to implement this? IMO it sounds a bit too restrictive and makes using integrations much harder for most users (they need to enable them one-by-one before being able to use one). We also don't even enforce this sort of rule for webhooks which are technically a sort of generic integration and also send lots of data.
I hear what you say. Hmm, would it be possible to have a sort of meta setting that would allow admins how they want to have it - new integrations on or off by default? I'm trying to be mindful of this #437656 (comment 1728302501)
I just thought of an alternative solution for this. We can consider greying out (like a disabled button) and showing a tooltip along the lines of "This integration has been disabled by your Administrator. Contact them to use it.". This will make it clear to users that there exists such an integration but they can't use it (yet).
Yes, absolutely, thank you so much!!
In case Admin wants to disable integration that is currently in use, they should receive a warning
This makes sense in terms of UX but I have a feeling it will be a pretty complicated thing to implement. Let's see what are the most high value things we want to implement and then consider improvements later.
Ok, sounds good. The most valuable thing here is for the Admin to enable/disable integrations, and what you described above about grading out disabled integrations should happen.
If warning for the Admin is difficult to show relative to integration being used or not, we should show warning saying that disabling integrations in use will break them and we cannot guarantee that if enabled again they would work with no problem. Something along those lines.
@m_frankiewicz@justin_ho From a backend perspective, it might be worth while doing a quick spike into understanding the effort. Ideally, doing this through cascading settings could bring a lot of value to GitLab.com customers also who could use this as group-owners. However, I'm very uncertain about how much complexity this would add to the feature. There are a number of methods on the integration models that are used to determine what integrations are available, and how easy it is to successfully insert settings checks within them is something that would be good to understand further.
Sometimes I've gone to refactor those methods and found changing them isn't easy, as there can be odd cascading effects.
It sounds like we'd also want to be able to list integrations that are unavailable within the index context in order to grey them out, but in all other contexts the integration would be "hidden" and be considered to not exist at all within the app. This part of the requirement is also something that could be good to quickly understand through a spike.
I think a weight 1 spike into those two points could give us a sense of feasibility from the backend perspective.
Sounds like a plan. Let's create a separate spike issue for the backend and we can come back here once we have more info. Either way, I think most of the complexity will be in the backend, frontend should be mostly straightforward.
Thank you both @.luke and @justin_ho. Spike sounds good. Let's do it after we've completed (or are very close to and there's little left to do) user mapping.
New integrations should be instance-wide disabled by default. SM instance Admins would need to review first and enable it if it should be used.
@m_frankiewicz There may be some instances (like GitLab.com) that wouldn't like this overhead. What do you think of presenting admins with these radio button options:
Enable all integrations
Enable only integrations on this allowlist:
(UI to build allow list of integrations)
By default instances would have "Enable all integrations" enabled. They continue as-normal with no workflow changes for them.
Instances that have chosen "Enable only integrations on this allowlist" (copy to be improved!) would need to add any new integrations to the allow list to enable them.
By default instances would have "Enable all integrations" enabled. They continue as-normal with no workflow changes for them.
Makes sense.
Instances that have chosen "Enable only integrations on this allowlist" (copy to be improved!) would need to add any new integrations to the allow list to enable them.
How could this allow list look like?
Would it be list of all possible integrations with checkboxes, like the import sources? - That would make for UI for Admin to see all available integrations and enable/disable them for the whole instance. Such list would be rather long, so I wonder how that would affect the Admin page layout. We could show it only when Admin checks "Enable only integrations on this allowlist" and not when the "Enable all integrations". @justin_ho WDYT?
@m_frankiewicz Yes we can show / hide the list selectively. I also think in Luke's example UI, we should be using radio buttons (which means users can only choose 1 of the 2 options) and not checkboxes.
As for the list of integrations, we have a few options for UI. We can make a long list of integration names with checkboxes (I think I prefer this option) or a dropdown and allow users to select from it.
We should disallow access to /integrations pages if there are no integrations available.
@m_frankiewicz Do you imagine this could be an alert message that displays, or would we remove the Integrations link from the side-nav as well as return a 404 for the route?
@.luke sorry, I think I made mistake here. We cannot do this and this
In UI on group and project level: grey out (like a disabled button) and show a tooltip along the lines of "This integration has been disabled by your Administrator. Contact them to use it." for disabled integrations. This will make it clear to users that there exists such an integration but they can't use it (yet).
I think we should not remove menu items or remove integrations from the list completely. We can still do the 404 if they try to go to an integration not on the allowlist.
And yes let's go with the greyed out / disabled option with tooltip.
@m_frankiewicz@justin_ho We could do it at the same time, if the instance had chosen the "allow list of integrations" option, and had not enabled any integrations, we can interpret that as the entire feature of integrations is disabled on the instance. There are other features that allow people to disable them, removing them from the sidenav. For example, the wiki. This would be in keeping with that.
If there was any allow listed integrations we wouldn't consider the entire feature of integrations to be disabled, and would do the greyed out UI instead.
There are other features that allow people to disable them, removing them from the sidenav. For example, the wiki. This would be in keeping with that.
If there's a pattern of doing it like that, we can follow it. Let's make sure this is documented well though, so people are not surprised they can't see integrations as all.
@.luke in a duplicate issue, there's comment with customer's question:
Additionally, they asked for "the possibility to do a such configuration through configuration file like gitlab.rb." which makes sense for even more strict security needs.
I.e., the integrations can't be re-activated during runtime, and would need even higher privileges (system-level "root" access to the GitLab servers) to be toggled.
@m_frankiewicz My understanding from this comment about gitlab.yml which I understand is the same/has a relationship with gitlab.rb is that we're strongly discouraged from adding new settings there, and instead are strongly encouraged to do so through application settings. I feel like these settings are similar in their concern with the Outbound Network Filtering settings which are also configurable through the admin application settings.
Why interested: For security purpose, some integration are not compliant with cyber security policy.
Problem they are trying to solve: The fact that even if the integration is disabled at instance level it could be set up unintentionally at group or project level.
Current solution for this problem: n/a
Impact to the customer of not having this: It could be an issue with security topics including standards requirements.
!169687 (closed) - there are some videos in the description that demonstrate the features.
The cascading settings feature is very well suited to this problem, and the feature allows a number of flexible arrangements:
What it allows us to do
Instance admins can "lock" the cascading settings, in which case the admin can:
Enforce only certain integrations to be allowed across the instance
Enforce that no integrations are allowed across the instance
Or instance admins can leave the settings unlocked, allowing group owners to set these rules for their groups, subgroups and projects. In this configuration, group owners can themselves "lock" the settings to prevent any sub-groups from being able to set their own rules. This configuration would be the default and would be a useful feature for GitLab.com.
What it doesn't allow us to do out of the box
Specify a certain subset of integrations to be allowed across instance, but allow group owners to further reduce that subset only but not allow more than the subset.
If customers required that feature it could be new bespoke work.
What we need before the FE can start
We need to know where we should put these settings in the:
Admin settings UI
Group settings UI
For example, should it be:
Incorporated into the existing Settings > integrations pages as a new "tab"?
Incorporated into some other settings sections
Iterations:
We could first deliver what the specific customers we're aware of need now in a first iteration, and add value in further iterations.
First iteration
This iteration would enable the cascading settings feature. Weight 5.
Modal warning when integrations of that type are active and are being disabled by admin, or group owner.
Greyed out integrations with tooltips when not available. I found this was not simple to do on the backend because it requires punching a "hole" through a number of methods which determine if an integration is available or not. The backend code is not in great shape, and this kind of work makes the backend code less maintainable. It may require a backstep first to refactor the backend code to allow this kind of logic. To be honest though, I'm also unsure how good this idea is, as I imagine in most situations people using the UI will not be interested in seeing the greyed-out integrations - especially on repeat visits, but likely even on the first visit. I strongly suspect it will be experienced by most people as a nuisance that makes it harder for people to do the work that they came to the page to do.
Instead of the grey-out integrations idea a simple to implement idea could be to use some simple text near the top of the page when some integrations have been disallowed. For example "some integrations have been disallowed by [admin/group owner]". It could involve a link. This would be simple to implement, have a lighter touch, and be less obtrusive when using the UI.
Let me know your thoughts, and I'll then finalize the issues.
Instance admins can "lock" the cascading settings, in which case the admin can:
Enforce only certain integrations to be allowed across the instance
Enforce that no integrations are allowed across the instance
This is what we need first. Group owners cannot override Admin settings.
This:
Specify a certain subset of integrations to be allowed across instance, but allow group owners to further reduce that subset only but not allow more than the subset.
We don't need this now.
allowing group owners to set these rules for their groups, subgroups and projects. In this configuration, group owners can themselves "lock" the settings to prevent any sub-groups from being able to set their own rules. This configuration would be the default and would be a useful feature for GitLab.com.
That could be nice, but I think what we need first is the admin only feature. For now we won't give the group owners similar functionality. This should reduce the scope of the issue, BE and FE changes, right?
Where we should put the settings in UI?
The instance level integration management we have now is described here and the UI is available in Admin UI under Settings > Integrations. This is about default vs custom setting for integrations.
What we want to do in this feature is something different, so I wonder if we should not add this new Admin settings to Settings - General, like import settings are done. The UI could be as described:
Radio button with two mutually exclusive options:
Allow all integrations
Allow only integrations on this allowlist
When the option "Allow only integrations on this allowlist" we show a list of all integrations with checkboxes, default to un-checked. (Personally I don't like the box for typing integrations names.)
By itself looks ok, but within the page looks confusing, to me. I'll reach out for UX help, maybe we can get some eyes here.
Modal warning when integrations of that type are active and are being disabled by admin
If we cannot get it in this iteration, we need to add to docs and text in UI a warning.
Instead of the grey-out integrations idea a simple to implement idea could be to use some simple text near the top of the page when some integrations have been disallowed. For example "some integrations have been disallowed by [admin/group owner]". It could involve a link. This would be simple to implement, have a lighter touch, and be less obtrusive when using the UI.
Sounds good to me, thanks for raising up disadvantages and obstacles to implementing greying out. I prefer simple solution too.
@ashrafkhamis there will be important work here to do for TW, so heads up.
@justin_ho could you review and see if there are outstanding questions from your side? WDYT about putting new setting to Settings - General, like import settings are done?
@m_frankiewicz I think adding this to Settings > General would be the easiest and most straightforward to implement. It's also pretty standard in terms of UX as a lot of integrations-related settings are there as well.
As for the overall feature, I don't have any concerns / questions. Overall looks pretty good and @.luke has done an awesome work in his research.
Thanks @justin_ho, that's good to hear. Let's wait for @.luke reply, but I think we can start working on this from tomorrow (FE part). I'll need to adjust planning for milestone for BE. I understand that for FE it would be possible to squeeze it in current milestone, or somethings needs to be removed as well?
I've created an epic for the work &15666 and made this issue we're currently talking to each other on part of that epic, retitling it as the "original discussion/planning issue" and porting the description to the epic's description. I hope that rearrangement makes sense - it's hard to know how to convert an issue to an epic without disrupting everything. I've stubbed some of the issues in the epic and will continue to write them up this week.
I wonder if Add note to top of integrations settings pages ... (#500613) with a general message would be easy enough to do in this milestone as well or not. Perhaps a very generic message like "If you don't see an integration on this list you'd expect to see, it might have been disabled by admin, speak to them" show at all times would do.
@.luke Weight 2 for FE sounds good to me. I've actually already started and have some basic UI, so will just branch out of your changes and continue work from there.
@m_frankiewicz From a backend perspective most of the feature would have been implemented after supporting the admin-only feature. @justin_ho What weight would you estimate supporting group-level setting would be, i.e., surfacing the settings on the group admin page?
@.luke Assuming we have this in place for instances, it should be a pretty small one to add for groups as frontend is pretty simple. So let's say frontend-weight1 for that upcoming issue.
@jmandell@justin_ho@m_frankiewicz We could introduce the setting initially as a non-cascading setting, regular instance admin-only setting.
I suggest this just because there is an extra overhead in GitLab checking the cascading setting if we're not going to use it initially . If there's no feature benefit, it would be much more performant to check regular admin application settings only initially.
Update, Justin's branch is currently working with regular settings components, so I think it's a sensible idea to do this as regular settings first, and save cascading settings for later with group-level support.
I'll restructure the epic to remove all group-level support for a later issue.
Hi @khornergit Just came cross this. It is interesting info for us when we think of the controls for compliance framework in the future. Could be a setting to be overwritten by compliance framework.
@jmandell Change the settings is not something we will directly take into compliance area for now, but I think this is aligned with our future planning.
I talked with @khornergit , after re-org, the enforcement will still be part of policy group. This could be interesting @g.hickman Something policy could have in the future overwrite settings as part of compliance enforcement.
Customer feedback: "being able to disable by default and then pick what to re-enable would be ideal, and that behaviour wouldn't break in-use integrations so we could close the barn door and then be selective and allow security review on what integrations are enabled."
Hi @m_frankiewicz, I am the designer from compliance, this feedback is interesting for us. Could you share more? Such as:
What is security review they mean?
The customer is following certain compliance standard?
Beside the setting, do you know whether they want the evidence of what is allowed/disallowed for auditing?
Luke Duncalfechanged title from Ability to disable project integrations for high compliance requirement customers to Discussion/planning: Ability to disable project integrations for high compliance requirement customers
changed title from Ability to disable project integrations for high compliance requirement customers to Discussion/planning: Ability to disable project integrations for high compliance requirement customers
Luke Duncalfechanged the descriptionCompare with previous version
changed the description
Luke Duncalfechanged title from Discussion/planning: Ability to disable project integrations for high compliance requirement customers to Original discussion/planning issue: Ability to disable project integrations for high compliance requirement customers
changed title from Discussion/planning: Ability to disable project integrations for high compliance requirement customers to Original discussion/planning issue: Ability to disable project integrations for high compliance requirement customers
Why interested: "We operate in a high governance and compliance-led way. Generally there's a process to be followed for adopting new tooling (especially for SaaS services) to ensure data isn't put at risk, to minimise duplication across the estate."
Problem they are trying to solve: Control access to the Internet
Impact to the customer of not having this:
"A lot of extra work for the GitLab team having to have conversations with internal security architects etc about how we're going to control the perceived risks of unconstrained egress from GitLab Dedicated to the Internet. The ingress side is nicely locked down behind Private Link, excepting things like OIDC which can be enabled only if needed, so TBH it comes across like a bit of a product limitation that egress isn't similarly controlled."
"Our organisation has to accept some risks we'd prefer to not accept. Though even if you magically implemented the feature tomorrow, we potentially might still be accepting some risks because locking out SaaS integrations is only part of the picture. What our security people would really like is a mechanism by which all egress traffic that's been allowed still gets passed back via our firewalls for inspection. Example: it might be we whitelist something like Slack, that's better than uncontrolled egress for everything, but there's still some residual risk something might get passed over that Slack connection we wouldn't want. But that type of inspection's outside the scope of what we're talking about here, and arguably a self-hosted instance would be the correct product choice for that feature."