Create read_admin_subscription custom admin ability
What does this MR do and why?
Related to #507961 (closed)
This MR adds the custom ability for a non-admin user to read the instance subscription details in the admin dashboard.
References
Please include cross links to any resources that are relevant to this MR. This will give reviewers and future readers helpful context to give an efficient review of the changes introduced.
MR acceptance checklist
Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.
Screenshots or screen recordings
How to set up and validate locally
Numbered steps to set up and validate the change are strongly suggested.
- Enable the feature flag.
Feature.enable(:custom_ability_read_admin_subscription)
- Create the admin role.
admin_member_role = MemberRole.create(name: 'View subscription', description: 'View subscription', read_admin_subscription: true)
- Assign the member role to a user
Users::UserMemberRole.create(member_role: admin_member_role, user: user)
- Log in with the user account and navigate to
/admin/subscription
. You should see the subscription details but not have any access to other admin functionality.
Merge request reports
Activity
changed milestone to %17.9
assigned to @imand3r
added pipelinetier-1 label
added backend label
6 Warnings 563231d6: The commit subject may not be longer than 72 characters. For more information, take a look at our Commit message guidelines. 35058792: The commit subject must contain at least 3 words. For more information, take a look at our Commit message guidelines. 35058792: Commits that change 30 or more lines across at least 3 files should describe these changes in the commit body. For more information, take a look at our Commit message guidelines. 41fa04a1: Commits that change 30 or more lines across at least 3 files should describe these changes in the commit body. For more information, take a look at our Commit message guidelines. 5216e8f0: Commits that change 30 or more lines across at least 3 files should describe these changes in the commit body. For more information, take a look at our Commit message guidelines. The master pipeline status page reported failures in If these jobs fail in your merge request with the same errors, then they are not caused by your changes.
Please check for any on-going incidents in the incident issue tracker or in the#master-broken
Slack channel.2 Messages CHANGELOG missing: If this merge request needs a changelog entry, add the
Changelog
trailer to the commit message you want to add to the changelog.If this merge request doesn't need a CHANGELOG entry, feel free to ignore this message.
This merge request adds or changes documentation files and requires Technical Writing review. The review should happen before merge, but can be post-merge if the merge request is time sensitive. Documentation review
The following files require a review from a technical writer:
-
doc/api/graphql/reference/index.md
(Link to current live version)
The review does not need to block merging this merge request. See the:
-
Metadata for the
*.md
files that you've changed. The first few lines of each*.md
file identify the stage and group most closely associated with your docs change. - The Technical Writer assigned for that stage and group.
- Documentation workflows for information on when to assign a merge request for review.
Reviewer roulette
Category Reviewer Maintainer backend @ddieulivol
(UTC+1, 9 hours ahead of author)
@jpcyiza
(UTC+1, 9 hours ahead of author)
groupauthorization Reviewer review is optional for groupauthorization @alexbuijs
(UTC+1, 9 hours ahead of author)
groupauthentication Reviewer review is optional for groupauthentication @dblessing
(UTC-6, 2 hours ahead of author)
Please refer to documentation page for guidance on how you can benefit from the Reviewer Roulette, or use the GitLab Review Workload Dashboard to find other available reviewers.
If needed, you can retry the
danger-review
job that generated this comment.Generated by
DangerEdited by ****-
- Resolved by 🤖 GitLab Bot 🤖
Proper labels assigned to this merge request. Please ignore me.
@imand3r
- please see the following guidance and update this merge request.1 Error Please add typebug typefeature, or typemaintenance label to this merge request. Edited by 🤖 GitLab Bot 🤖
added featureaddition label
added typefeature label
added 1 commit
- 2d8cddec - Create read_admin_subscription custom admin ability
added documentation label
added 1 commit
- d97e7df6 - Create read_admin_subscription custom admin ability
Generated bygitlab_quality-test_tooling
.
Slow tests detected in this merge request. These slow tests might be related to this merge request's changes.Click to expand
Job File Name Duration Expected duration #8885746088 ee/spec/features/boards/sidebar_spec.rb#L196
Issue Boards when drawer is disabled epic when the issue is not associated with an epic displays None
for value of epic51.54 s < 50.13 s #8886277971 ee/spec/features/boards/sidebar_spec.rb#L196
Issue Boards when drawer is disabled epic when the issue is not associated with an epic displays None
for value of epic52.91 s < 50.13 s #8887075168 ee/spec/features/boards/sidebar_spec.rb#L196
Issue Boards when drawer is disabled epic when the issue is not associated with an epic displays None
for value of epic52.01 s < 50.13 s #8913608763 ee/spec/features/boards/sidebar_spec.rb#L196
Issue Boards when drawer is disabled epic when the issue is not associated with an epic displays None
for value of epic52.49 s < 50.13 s #8915191491 ee/spec/features/boards/sidebar_spec.rb#L196
Issue Boards when drawer is disabled epic when the issue is not associated with an epic displays None
for value of epic58.36 s < 50.13 s #8937344541 ee/spec/features/boards/sidebar_spec.rb#L196
Issue Boards when drawer is disabled epic when the issue is not associated with an epic displays None
for value of epic63.56 s < 50.13 s #8940249064 spec/lib/release_highlights/validator_spec.rb#L82
ReleaseHighlights::Validator when validating all files they should have no errors 64.56 s < 27.12 s #8940249165 spec/lib/release_highlights/validator_spec.rb#L82
ReleaseHighlights::Validator when validating all files they should have no errors 74.0 s < 27.12 s #8946722851 spec/lib/release_highlights/validator_spec.rb#L82
ReleaseHighlights::Validator when validating all files they should have no errors 73.49 s < 27.12 s #8946722988 spec/lib/release_highlights/validator_spec.rb#L82
ReleaseHighlights::Validator when validating all files they should have no errors 76.94 s < 27.12 s #8947495073 spec/lib/release_highlights/validator_spec.rb#L82
ReleaseHighlights::Validator when validating all files they should have no errors 68.25 s < 27.12 s #8947494988 spec/lib/release_highlights/validator_spec.rb#L82
ReleaseHighlights::Validator when validating all files they should have no errors 71.62 s < 27.12 s #8948250916 spec/lib/release_highlights/validator_spec.rb#L82
ReleaseHighlights::Validator when validating all files they should have no errors 69.87 s < 27.12 s #8948251063 spec/lib/release_highlights/validator_spec.rb#L82
ReleaseHighlights::Validator when validating all files they should have no errors 71.99 s < 27.12 s #8965255964 spec/lib/release_highlights/validator_spec.rb#L82
ReleaseHighlights::Validator when validating all files they should have no errors 77.82 s < 27.12 s #8965256127 spec/lib/release_highlights/validator_spec.rb#L82
ReleaseHighlights::Validator when validating all files they should have no errors 73.88 s < 27.12 s #8976382914 spec/lib/release_highlights/validator_spec.rb#L82
ReleaseHighlights::Validator when validating all files they should have no errors 70.8 s < 27.12 s #8976383606 spec/lib/release_highlights/validator_spec.rb#L82
ReleaseHighlights::Validator when validating all files they should have no errors 76.29 s < 27.12 s #8977449067 spec/lib/release_highlights/validator_spec.rb#L82
ReleaseHighlights::Validator when validating all files they should have no errors 63.18 s < 27.12 s #8977448960 spec/lib/release_highlights/validator_spec.rb#L82
ReleaseHighlights::Validator when validating all files they should have no errors 69.44 s < 27.12 s #8990362990 spec/lib/release_highlights/validator_spec.rb#L82
ReleaseHighlights::Validator when validating all files they should have no errors 70.49 s < 27.12 s #8990362885 spec/lib/release_highlights/validator_spec.rb#L82
ReleaseHighlights::Validator when validating all files they should have no errors 71.04 s < 27.12 s #8998583077 spec/lib/release_highlights/validator_spec.rb#L82
ReleaseHighlights::Validator when validating all files they should have no errors 65.82 s < 27.12 s #8998583396 spec/lib/release_highlights/validator_spec.rb#L82
ReleaseHighlights::Validator when validating all files they should have no errors 69.67 s < 27.12 s Edited by ****added rspec:slow test detected label
added 1 commit
- 336f3651 - Create read_admin_subscription custom admin ability
added 1 commit
- b8181a18 - Create read_admin_subscription custom admin ability
added 1 commit
- 20d94b19 - Create read_admin_subscription custom admin ability
added feature flag label
mentioned in issue #507961 (closed)
- Resolved by Eugie Limpin
requested review from @eugielimpin
removed workflowready for development label
2 2 3 3 # EE:Self Managed 4 4 class Admin::SubscriptionsController < Admin::ApplicationController 5 extend ::Gitlab::Utils::Override 6 5 7 respond_to :html 6 8 7 9 feature_category :plan_provisioning 8 10 urgency :low 11 12 private 13 14 override :authenticate_admin! 15 def authenticate_admin! changed this line in version 10 of the diff
mentioned in merge request !177514 (merged)
- Resolved by Eugie Limpin
- Resolved by Eugie Limpin
- Resolved by Eugie Limpin
- Resolved by Ian Anderson
- Resolved by Eugie Limpin
- Resolved by Eugie Limpin
- Resolved by Eugie Limpin
Nice work @imand3r! This is looking good.
I left mostly non-blocking feedback for you
Let me know what you think!Edited by Eugie Limpin
requested review from @eugielimpin
- Resolved by Eugie Limpin
LGTM!
added pipeline:mr-approved label
added pipelinetier-3 pipeline:run-e2e-omnibus-once labels and removed pipelinetier-1 label
Before you set this MR to auto-merge
This merge request will progress on pipeline tiers until it reaches the last tier: pipelinetier-3. We will trigger a new pipeline for each transition to a higher tier.
Before you set this MR to auto-merge, please check the following:
- You are the last maintainer of this merge request
- The latest pipeline for this merge request is pipelinetier-3 (You can find which tier it is in the pipeline name)
- This pipeline is recent enough (created in the last 8 hours)
If all the criteria above apply, please set auto-merge for this merge request.
See pipeline tiers and merging a merge request for more details.
requested review from @SamWord
E2E Test Result Summary
allure-report-publisher
generated test report!e2e-test-on-cng:
test report for 6228b59bexpand test summary
+------------------------------------------------------------------+ | suites summary | +-------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +-------------+--------+--------+---------+-------+-------+--------+ | Plan | 86 | 0 | 8 | 0 | 94 | ✅ | | Create | 143 | 0 | 19 | 0 | 162 | ✅ | | Data Stores | 33 | 0 | 10 | 0 | 43 | ✅ | | Govern | 84 | 0 | 10 | 0 | 94 | ✅ | | Monitor | 8 | 0 | 12 | 0 | 20 | ✅ | | Package | 29 | 0 | 15 | 0 | 44 | ✅ | | Verify | 53 | 0 | 19 | 0 | 72 | ✅ | | Release | 5 | 0 | 1 | 0 | 6 | ✅ | | Secure | 2 | 0 | 5 | 0 | 7 | ✅ | | Growth | 0 | 0 | 2 | 0 | 2 | ➖ | | Manage | 1 | 0 | 9 | 0 | 10 | ✅ | | Fulfillment | 2 | 0 | 7 | 0 | 9 | ✅ | | Analytics | 2 | 0 | 0 | 0 | 2 | ✅ | | ModelOps | 0 | 0 | 1 | 0 | 1 | ➖ | | Ai-powered | 0 | 0 | 2 | 0 | 2 | ➖ | | Configure | 0 | 0 | 3 | 0 | 3 | ➖ | +-------------+--------+--------+---------+-------+-------+--------+ | Total | 448 | 0 | 123 | 0 | 571 | ✅ | +-------------+--------+--------+---------+-------+-------+--------+
e2e-test-on-gdk:
test report for 6228b59bexpand test summary
+------------------------------------------------------------------+ | suites summary | +-------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +-------------+--------+--------+---------+-------+-------+--------+ | Plan | 164 | 0 | 16 | 0 | 180 | ✅ | | Create | 276 | 0 | 40 | 0 | 316 | ✅ | | Data Stores | 66 | 0 | 20 | 0 | 86 | ✅ | | Monitor | 16 | 0 | 24 | 0 | 40 | ✅ | | Verify | 104 | 0 | 40 | 2 | 144 | ✅ | | Package | 48 | 0 | 28 | 0 | 76 | ✅ | | Growth | 0 | 0 | 4 | 0 | 4 | ➖ | | Govern | 158 | 0 | 26 | 0 | 184 | ✅ | | Configure | 0 | 0 | 6 | 0 | 6 | ➖ | | Fulfillment | 4 | 0 | 14 | 0 | 18 | ✅ | | Analytics | 4 | 0 | 0 | 0 | 4 | ✅ | | Manage | 2 | 0 | 18 | 0 | 20 | ✅ | | Secure | 8 | 0 | 6 | 0 | 14 | ✅ | | Release | 10 | 0 | 2 | 0 | 12 | ✅ | | ModelOps | 0 | 0 | 2 | 0 | 2 | ➖ | | Ai-powered | 0 | 0 | 4 | 0 | 4 | ➖ | +-------------+--------+--------+---------+-------+-------+--------+ | Total | 860 | 0 | 250 | 2 | 1110 | ✅ | +-------------+--------+--------+---------+-------+-------+--------+
Edited by ****reset approvals from @eugielimpin by pushing to the branch
added 1 commit
- 6e64213f - Add ability to override authorization when enforcing admin authentication
@tachyons-gitlab are you available to review changes to
app/controllers/concerns/enforces_admin_authentication.rb
for groupauthenticationrequested review from @tachyons-gitlab
added 1 commit
- 5b1245b5 - Add ability to override authorization when enforcing admin authentication
mentioned in task #514142 (closed)
- Resolved by Ian Anderson
mentioned in commit 12c6d88c
mentioned in issue #508782 (closed)
mentioned in commit fdb33d7d
- Resolved by Sam Word
- Resolved by Sam Word
- Resolved by Ian Anderson
@imand3r , just one small suggestion from me!
added 2150 commits
-
fef8d524...3f8ec3d5 - 2145 commits from branch
master
- 48e0372f - Create read_admin_subscription custom admin ability
- 4c624371 - Add feature flag
- daaed204 - Apply suggestions
- bc553686 - Add ability to override authorization when enforcing admin authentication
- 4767a403 - Apply 1 suggestion(s) to 1 file(s)
Toggle commit list-
fef8d524...3f8ec3d5 - 2145 commits from branch
reset approvals from @tachyons-gitlab by pushing to the branch
@tachyons-gitlab Do you mind re-approving? I had to resolve a merge conflict which reset your approval.
added 214 commits
-
9bb47548...5fdd0a5d - 209 commits from branch
master
- 5216e8f0 - Create read_admin_subscription custom admin ability
- 41fa04a1 - Add feature flag
- 35058792 - Apply suggestions
- 563231d6 - Add ability to override authorization when enforcing admin authentication
- 6228b59b - Apply 1 suggestion(s) to 1 file(s)
Toggle commit list-
9bb47548...5fdd0a5d - 209 commits from branch
reset approvals from @SamWord by pushing to the branch
requested review from @eugielimpin
- Resolved by Eugie Limpin
@eugielimpin Can you take a final look as well after the rebases?
requested review from @tachyons-gitlab
requested review from @SamWord
mentioned in commit 2fa8a880
started a merge train
mentioned in commit da673cff
added workflowstaging-canary label
added workflowcanary label and removed workflowstaging-canary label
added workflowstaging label and removed workflowcanary label
added workflowproduction label and removed workflowstaging label
mentioned in merge request !179794 (merged)
added workflowpost-deploy-db-staging label and removed workflowproduction label
added workflowpost-deploy-db-production label and removed workflowpost-deploy-db-staging label