Skip to content

Resolve "Support restricting group access by multiple IP subnets"

CE PORT: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/31959

What does this MR do?

For #12873 (closed)

Currently, for the group IP restriction feature, only 1 IP subnet can be provided.

This MR adds the ability to provide comma separated subnets for group IP restrictions.

This is how the change looks (the input box now accepts comma separated IP subnets)

Screenshot_2019-08-14_at_5.26.09_PM

Our long term goal is to move the IP restriction UI to something like this:

Screenshot_2019-08-07_at_2.51.26_PM

but right now, due to low bandwidth in FE, we were not able to get FE help for this change.

Hence, we had to make do with the existing form, which only has a single text input, but at the same time, move the association from a has_one :ip_restriction to a has_many :ip_restrictions.

In order to do this, I've made use of virtual attributes (a getter and setter named ip_restriction_ranges in the Group model), that creates and updates multiples ip_restriction records for us, while still allowing us to maintain the single text input in the form.

The benefits of virtual attribute I see are:

  1. The surface area of the change is minimum: the change affects only the form, and not any area associated with the working of the IP Restriction feature.

  2. When we move to the new UI as described above:

    2.1 we will already have has_many :ip_restrictions in place (no db migration required from has_one)

    2.2 we can get rid of the getter, setter and the form and it won't affect the working of the IP restriction feature in any manner.

Does this MR meet the acceptance criteria?

Conformity

Performance and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Manoj M J

Merge request reports