In-app awareness of Registration Features - Group access by IP address
What does this MR do and why?
Related to #341949 (closed)
To increase discoverability for the features that can be enabled with Registration features, we'll be presenting in-app messages/hints. In this case, for setting allowed IP addresses.
Criteria to show the CTA
The offer shouldn't be presented to licensed instances because they already have the paid features available, so the callout (to enable features they already have) doesn't make sense. The decision on whether to show the offer is on the true/false of whether Service Ping is enabled, not on whether Registration Features is enabled.
| Instance Type | License Type | Service Ping Setting | Expected Behavior |
|---|---|---|---|
| CE | Unlicensed | Off | Link to docs |
| EE | Unlicensed | Off | Link to admin setting |
| EE | Licensed | Off | Do not present offer |
| CE | Unlicensed | On | Do not present offer |
| EE | Unlicensed | On | Do not present offer |
| EE | Licensed | On | Do not present offer |
Screenshots or screen recordings
Enabled feature for reference
When Service Ping is disabled
The CTA is always hidden if Service Ping is enabled.
| Before (hidden) | After (CE) | After (EE) |
|---|---|---|
 |
 |
 |
How to set up and validate locally
- Optionally restart
gdkas FOSS (FOSS_ONLY=1 gdk restart) to test like a CE instance. - Go to any group’s Settings > General page. and expand the Permissions, LFS, 2FA section.
- Scroll to the Allow access to the following IP addresses section.
- Toggle a local license/subscription and/or service ping/registration features.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Merge request reports
Activity
changed milestone to %14.5
assigned to @agarciatesares
added 1 commit
- d3d41b95 - Registration features info for ip restriction
Reviewer roulette
Changes that require review have been detected!
Please refer to the table below for assigning reviewers and maintainers suggested by Danger in the specified category:
Category Reviewer Maintainer backend Valery Sizov ( @vsizov) (UTC+1, 4 hours ahead of@agarciatesares)Michael Kozono ( @mkozono) (UTC-8, 5 hours behind@agarciatesares)frontend Justin Ho ( @justin_ho) (UTC+7, 10 hours ahead of@agarciatesares)Ezekiel Kigbo ( @ekigbo) (UTC+11, 14 hours ahead of@agarciatesares)To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.
To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.
Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.
If needed, you can retry the
danger-reviewjob that generated this comment.Generated by
removed workflowin dev label
mentioned in commit b2ba9261
added 1 commit
- b2ba9261 - Registration features info for ip restriction
Allure report
allure-report-publishergenerated test report for a360a735!review-qa-smoke:
review-qa-reliable:- Resolved by Rajendra Kadam
@rkadam3 Hello! Could you give a preliminary look at the conditional content I added? At first, I created a helper that verified if the user was an admin and enabled the usage ping (similar to &6822 (comment 690515750)). Still, in practice, the features are already being guarded by their corresponding criteria, including the registration features (that implies that the usage ping is enabled), so I'm using that as a conditional to show the message. Then I'm checking the user license.
I tested this manually, and I believe it's working correctly, but please let me know if you see any issues. I'm adding feature specs to check the conditional content next.
/cc @nicolasdular, if you have any full-stack-powered comments about this.
- Last reply by Rajendra Kadam
requested review from @rkadam3
mentioned in commit 1819b616
added 75 commits
-
b2ba9261...daa5ac42 - 74 commits from branch
master - 1819b616 - Registration features info for ip restriction
-
b2ba9261...daa5ac42 - 74 commits from branch
changed milestone to %14.6
mentioned in commit cc84b05a
added 1727 commits
-
1819b616...c8d7cf7a - 1726 commits from branch
master - cc84b05a - Registration features info for ip restriction
-
1819b616...c8d7cf7a - 1726 commits from branch
removed review request for @rkadam3
mentioned in commit 0bc2c474
added 183 commits
-
cc84b05a...eefe6ffc - 181 commits from branch
master - 0bc2c474 - Registration features info for ip restriction
- a6fddbb1 - Update IP restrictions specs
-
cc84b05a...eefe6ffc - 181 commits from branch
- A deleted user
added backend label
mentioned in commit ca6100f7
added 199 commits
-
a6fddbb1...68bca4de - 197 commits from branch
master - ca6100f7 - Registration features info for ip restriction
- ddaf9de5 - Update IP restrictions specs
-
a6fddbb1...68bca4de - 197 commits from branch
requested review from @rkadam3
removed review request for @rkadam3
marked the checklist item I have evaluated the MR acceptance checklist for this MR. as completed
mentioned in commit 17561163
added 114 commits
-
ddaf9de5...9bb39fdb - 112 commits from branch
master - 17561163 - Registration features info for ip restriction
- 39be02e1 - Update IP restrictions specs
-
ddaf9de5...9bb39fdb - 112 commits from branch
- Resolved by Axel García
- Last reply by Axel García
requested review from @himkp
- Resolved by Axel García
Howdy @marc_shaw
- Last reply by Marc Shaw
requested review from @marc_shaw
mentioned in commit 7e5ca33b
added 61 commits
-
39be02e1...b6df7992 - 58 commits from branch
master - 7e5ca33b - Registration features info for ip restriction
- 6e29ebdb - Update IP restrictions specs
- 52443c7c - Update registration features prompt copy
Toggle commit list-
39be02e1...b6df7992 - 58 commits from branch
requested review from @fneill
@amandarueda, thanks for approving this merge request.This is the first time the merge request is approved. To ensure full test coverage, a new pipeline has been started.
For more info, please refer to the following links:
mentioned in commit d9710fdd
added 47 commits
-
52443c7c...55a15031 - 44 commits from branch
master - d9710fdd - Registration features info for ip restriction
- e7f960d0 - Update IP restrictions specs
- 1ef0d7cd - Update registration features prompt copy
Toggle commit list-
52443c7c...55a15031 - 44 commits from branch
mentioned in commit 204ab76a
added 9 commits
-
1ef0d7cd...e8ed8549 - 6 commits from branch
master - 204ab76a - Registration features info for ip restriction
- 5494b31b - Update IP restrictions specs
- 84af6d5f - Update registration features prompt copy
Toggle commit list-
1ef0d7cd...e8ed8549 - 6 commits from branch
removed review request for @fneill
removed review request for @himkp
added pipeline:run-all-rspec pipeline:run-as-if-foss labels
mentioned in commit 7aaf085b
added 804 commits
-
84af6d5f...2ba756c4 - 801 commits from branch
master - 7aaf085b - Registration features info for ip restriction
- 7b630a80 - Update IP restrictions specs
- 11642852 - Update registration features prompt copy
Toggle commit list-
84af6d5f...2ba756c4 - 801 commits from branch
added 1 commit
- fc6e2b93 - Pass license as parameter for RF shared view
mentioned in commit 27ca5754
added 20 commits
-
fc6e2b93...7cf3bb60 - 16 commits from branch
master - 27ca5754 - Registration features info for ip restriction
- 5175aa76 - Update IP restrictions specs
- ed2f7365 - Update registration features prompt copy
- f279d236 - Pass license as parameter for RF shared view
Toggle commit list-
fc6e2b93...7cf3bb60 - 16 commits from branch
requested review from @psimyn
couple of minor suggestions @agarciatesares - rest looks super
removed review request for @psimyn
mentioned in merge request !74042 (merged)
mentioned in commit 341b61f7
added 1111 commits
-
f279d236...f7963d40 - 1107 commits from branch
master - 341b61f7 - Registration features info for ip restriction
- dac58697 - Update IP restrictions specs
- 1665135c - Update registration features prompt copy
- 2f70fd17 - Pass license as parameter for RF shared view
Toggle commit list-
f279d236...f7963d40 - 1107 commits from branch
mentioned in commit b5756212
added 138 commits
-
2f70fd17...8ab9f4b3 - 134 commits from branch
master - b5756212 - Registration features info for ip restriction
- d6ecc1ed - Update IP restrictions specs
- 83be0880 - Update registration features prompt copy
- 76e1b851 - Pass license as parameter for RF shared view
Toggle commit list-
2f70fd17...8ab9f4b3 - 134 commits from branch
mentioned in issue gitlab-com/www-gitlab-com#9408 (closed)
mentioned in commit 395f96e5
1 - return if !ip_restriction_feature_available?(group) || group.parent_id.present? 2 1 - hidden_input_id = 'group_ip_restriction_ranges' 3 2 - label_id = "#{hidden_input_id}_label" 4 3 5 4 .form-group 6 5 %label{ id: label_id } 7 6 = _('Allow access to the following IP addresses') 8 .js-ip-restriction{ data: { hidden_input_id: hidden_input_id, label_id: label_id } } 9 = f.hidden_field :ip_restriction_ranges, id: hidden_input_id 10 .form-text.text-muted 11 - learn_more_link = link_to(_('Learn more'), help_page_path('user/group/index.md', anchor: 'restrict-group-access-by-ip-address')) 12 = _('Only users from the specified IP address ranges are able to reach this group, including all subgroups, projects, and Git repositories.') 13 %br 14 = _('Multiple IP address ranges are supported.') 15 = html_escape(_('%{learn_more_link}.')) % { learn_more_link: learn_more_link } 7 - if ip_restriction_feature_available?(group) && !group.parent_id.present? changed this line in version 16 of the diff
added 389 commits
-
76e1b851...f42bdfa9 - 384 commits from branch
master - 395f96e5 - Registration features info for ip restriction
- 68721ca6 - Update IP restrictions specs
- 7921ca29 - Update registration features prompt copy
- 20ace00f - Pass license as parameter for RF shared view
- 65441614 - Move IP restriction logic to CE
Toggle commit list-
76e1b851...f42bdfa9 - 384 commits from branch
mentioned in issue #348475 (closed)
mentioned in commit 1cbbb89e
added 172 commits
-
c8b898c4...72f23cf7 - 166 commits from branch
master - 1cbbb89e - Registration features info for ip restriction
- 406465cd - Update IP restrictions specs
- 6c648972 - Update registration features prompt copy
- 06835752 - Pass license as parameter for RF shared view
- 3df81106 - Move IP restriction form group to CE
- 6aa52272 - Allow IP restriction helper on CE
Toggle commit list-
c8b898c4...72f23cf7 - 166 commits from branch
added pipeline:skip-undercoverage label
added 2 commits
requested review from @rkadam3
removed review request for @rkadam3
mentioned in commit ceeffe01
added 222 commits
Toggle commit listadded 2 commits
Fixed the conflicts after !73222 (merged)
- Resolved by Axel García
- Last reply by Axel García
assigned to @rkadam3
mentioned in issue #341949 (closed)
unassigned @agarciatesares
changed milestone to %14.7
assigned to @agarciatesares and unassigned @rkadam3
removed pipeline:skip-undercoverage label
added 1086 commits
-
19009169...f3e83a99 - 1080 commits from branch
master - b842d437 - Registration features info for ip restriction
- 7ca4dfb9 - Update IP restrictions specs
- c22d08d6 - Pass license as parameter for RF shared view
- 504ae27b - Move IP restriction form group to CE
- 6555c460 - Allow IP restriction helper on CE
- 20b448f7 - Add IP restriction CTA
Toggle commit list-
19009169...f3e83a99 - 1080 commits from branch
mentioned in commit b842d437
removed review request for @marc_shaw
mentioned in commit 2787b620
added 389 commits
Toggle commit list- Resolved by Mikołaj Wawrzyniak
Hello @marc_shaw
- Last reply by Mikołaj Wawrzyniak
requested review from @marc_shaw
added 2 commits
mentioned in commit 413040ca
added 176 commits
-
8965c0fe...c6db9521 - 170 commits from branch
master - 413040ca - Registration features info for ip restriction
- 0c378900 - Update IP restrictions specs
- 14d1dc2d - Pass license as parameter for RF shared view
- 13820d06 - Move IP restriction form group to CE
- 6410b258 - Add IP restriction CTA
- 8fe0cb1a - Specs for IP restriction RF CTA
Toggle commit list-
8965c0fe...c6db9521 - 170 commits from branch
mentioned in commit df84aed0
added 232 commits
-
8fe0cb1a...e3d7c020 - 224 commits from branch
master - df84aed0 - Registration features info for ip restriction
- 2a3a6d00 - Update IP restrictions specs
- 2a5dadb2 - Pass license as parameter for RF shared view
- 71480a3f - Move IP restriction form group to CE
- 1686377e - Add IP restriction CTA
- e6a87281 - Specs for IP restriction RF CTA
- 1beaab90 - Add shared examples for RF CTA
- 49c2a470 - Update RF CTA related tests
Toggle commit list-
8fe0cb1a...e3d7c020 - 224 commits from branch
requested review from @mikolaj_wawrzyniak and removed review request for @marc_shaw
mentioned in merge request !77355 (merged)
enabled an automatic merge when the pipeline for 7587935f succeeds
mentioned in commit 83cc2c5f
mentioned in commit 70dad3a4
mentioned in merge request !78115 (merged)
added workflowstaging-canary label
added workflowstaging label and removed workflowstaging-canary label
added workflowcanary label and removed workflowstaging label
added workflowproduction label and removed workflowcanary label
mentioned in commit 235d52df
added releasedcandidate label
added releasedpublished label and removed releasedcandidate label
mentioned in merge request kubitus-project/kubitus-installer!562 (merged)
mentioned in issue #364871
