Add user access functionality for KAS
What does this MR do and why?
Implements #381561 (closed) and #381566 (closed). Implements, behind a feature flag:
-
authorize_proxy_user
API endpoint that looks up a user by session ID and returns the metadata for KAS - Setting the KAS cookie. Currently at the end of every request. Should limit it to relevant controllers.
The agent MR is here: gitlab-org/cluster-integration/gitlab-agent!841 (merged)
Screenshots or screen recordings
How to set up and validate locally
You have to be familiar with KAS and agentk to setup this up:
- Enable the feature flag
Feature.enable(:kas_user_access)
- Setup your GitLab (GDK) with KAS and a working agentk from gitlab-org/cluster-integration/gitlab-agent!841 (merged)
- Register that agent in a project and enable the ff for that project, too:
Feature.enable(:kas_user_access_project, Project.find(<project-id>))
- Browse to the agent overview page
- Open up the browser dev console and run something like this to query Kube API via KAS and agentk (make sure that the KAS address is correct for your setup):
fetch('https://kas.gdk.test:3443/-/k8s-proxy/api/v1/namespaces', {credentials: 'include', headers: {'X-Csrf-TOKEN': document.head.querySelector('meta[name="csrf-token"]').content, 'GitLab-Agent-Id': '1'}}).then((response) => response.json()).then((data) => console.log(data));
- Verify that the response is a
NamespaceList
Kube API response
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
assigned to @hfyngvason
mentioned in issue #381566 (closed)
- Resolved by Hordur Freyr Yngvason
- Resolved by Timo Furrer
6 Warnings This merge request is quite big (778 lines changed), please consider splitting it into multiple merge requests. 28751ad6: The commit subject may not be longer than 72 characters. For more information, take a look at our Commit message guidelines. eaab6e3f: 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. 9d229393: 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. featureaddition and featureenhancement merge requests normally have a documentation change. Consider adding a documentation update or confirming the documentation plan with the Technical Writer counterpart.
For more information, see:
- The Handbook page on merge request types.
- The definition of done documentation.
Do not add new controller specs. We are moving from controller specs to
request specs (and/or feature specs). Please add request specs under
/spec/requests
and/or/ee/spec/requests
instead.See &5076 for information.
1 Message CHANGELOG missing: If you want to create a changelog entry for GitLab FOSS, add the
Changelog
trailer to the commit message you want to add to the changelog.If you want to create a changelog entry for GitLab EE, also add the
EE: true
trailer to your commit message.If this merge request doesn't need a CHANGELOG entry, feel free to ignore this message.
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 Harsimar Sandhu (
@harsimarsandhu
) (UTC+5.5, 10.5 hours ahead of@hfyngvason
)Jessie Young (
@jessieay
) (UTC-8, 3 hours behind@hfyngvason
)Application Security Reviewer review is optional for Application Security Greg Alfaro (
@truegreg
) (UTC-7, 2 hours behind@hfyngvason
)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-review
job that generated this comment.Generated by
Danger
- Resolved by 🤖 GitLab Bot 🤖
Proper labels assigned to this merge request. Please ignore me.
@hfyngvason - 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 🤖
Allure report
allure-report-publisher
generated test report!e2e-package-and-test:
test report for dd0f5277expand test summary
+-----------------------------------------------------------------------+ | suites summary | +------------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +------------------+--------+--------+---------+-------+-------+--------+ | Manage | 132 | 0 | 6 | 4 | 138 | ❗ | | Create | 292 | 0 | 40 | 20 | 332 | ❗ | | Plan | 118 | 2 | 0 | 2 | 120 | ❌ | | Govern | 84 | 0 | 2 | 0 | 86 | ✅ | | Verify | 98 | 0 | 4 | 0 | 102 | ✅ | | Analytics | 4 | 0 | 0 | 0 | 4 | ✅ | | Release | 12 | 0 | 0 | 0 | 12 | ✅ | | Monitor | 10 | 0 | 0 | 0 | 10 | ✅ | | ModelOps | 0 | 0 | 2 | 0 | 2 | ➖ | | Secure | 14 | 0 | 2 | 0 | 16 | ✅ | | Fulfillment | 4 | 0 | 46 | 0 | 50 | ✅ | | Package | 0 | 0 | 6 | 0 | 6 | ➖ | | Growth | 0 | 0 | 4 | 0 | 4 | ➖ | | Framework sanity | 0 | 0 | 2 | 0 | 2 | ➖ | | Configure | 0 | 0 | 6 | 0 | 6 | ➖ | +------------------+--------+--------+---------+-------+-------+--------+ | Total | 768 | 2 | 120 | 26 | 890 | ❌ | +------------------+--------+--------+---------+-------+-------+--------+
added 2444 commits
-
747cf465...b837a1ea - 2443 commits from branch
master
- f8c51514 - Add KAS user authorization behind a feature flag
-
747cf465...b837a1ea - 2443 commits from branch
added 1340 commits
-
f8c51514...e13e3269 - 1339 commits from branch
master
- 55cfcf5e - Add KAS user authorization behind a feature flag
-
f8c51514...e13e3269 - 1339 commits from branch
added 3 commits
- Resolved by Hordur Freyr Yngvason
- Resolved by Hordur Freyr Yngvason
- Resolved by Hordur Freyr Yngvason
- Resolved by Hordur Freyr Yngvason
- Resolved by João Alexandre Cunha
- Resolved by Timo Furrer
- Resolved by Timo Furrer
- Resolved by Hordur Freyr Yngvason
- Resolved by Hordur Freyr Yngvason
- Resolved by Hordur Freyr Yngvason
- Resolved by Hordur Freyr Yngvason
added 2 commits
- Resolved by Hordur Freyr Yngvason
- Resolved by Hordur Freyr Yngvason
- Resolved by Hordur Freyr Yngvason
- Resolved by Hordur Freyr Yngvason
removed review request for @Alexand
- Resolved by Hordur Freyr Yngvason
mentioned in issue Alexand/growth-and-development#1 (closed)
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@0507d6c1
added 4586 commits
-
394880ca...cbc29ecc - 4572 commits from branch
master
- 0940d233 - Add KAS user authorization behind a feature flag
- 7d653a96 - Encrypt / decrypt cookie
- bd6e967e - Add initital tests
- b41661b3 - Fix rubocop offenses
- 51c57eb5 - Use grape for param validation
- 2b5677be - Remove redundant validation line
- 2a5d2be0 - Add missing comma, improve description
- 03a60f65 - Add missing feature_category on endpoint
- 95ef7f12 - Use safe navigation for session id
- 83d5c945 - Make ff disabled by default; bump milestone
- eea56bd5 - Fix grape value type (should be String, not Symbol)
- 78027245 - Use secure_compare and disallow blank csrf_token
- d61b929c - Add introduced_by_url
- 4f5f2094 - Add extra flag for actor-based rollout
Toggle commit list-
394880ca...cbc29ecc - 4572 commits from branch
added 1 commit
- 893c9c45 - Add KAS user authorization behind a feature flag
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@097f2bd0
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@82fab2b5
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@c525161f
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@6fc9c39d
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@d21b617c
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@02c17c27
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@b24193c1
added 566 commits
-
8e36cf7c...fc02ebb9 - 560 commits from branch
master
- 1d0c8142 - Add KAS user authorization behind a feature flag
- d6738398 - Fix next where return was expected
- 305cd24e - Rely on existing 404 wrappers for #find
- 9c6ab06e - Add feature category to spec
- b470ae36 - Fix and optimize spec
- 21af89d2 - Update tests; add EE service tests
Toggle commit list-
8e36cf7c...fc02ebb9 - 560 commits from branch
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@5aa65632
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@59ebfd9e
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@bd66f5ba
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@427c501c
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@02bb69d1
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@65f608eb
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@75c5b932
added 2 commits
- Resolved by Hordur Freyr Yngvason
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@c14338b0
added 2 commits
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@f625f445
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@cdfa16fa
added 1 commit
- febfac02 - Encode cookie data to JSON prior to encryption
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@0513a2a5
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@8e8bce36
added 1 commit
- 177b0a33 - Add KAS user authorization behind a feature flag
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@90a35b9a
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@5895c428
added 409 commits
-
841304d5...f7e25ab2 - 406 commits from branch
master
- 1ea54cf3 - Add KAS user authorization behind a feature flag
- eaf6804a - Validate session before loading agent
- b1ef2478 - Add unit tests for cookie data method
Toggle commit list-
841304d5...f7e25ab2 - 406 commits from branch
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@9c166d84
added 1 commit
- 0d3631e4 - Only read up to `AUTHORIZED_ENTITY_LIMIT` entries
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@2fcc380c
requested review from @Alexand
- Resolved by João Alexandre Cunha
- Resolved by João Alexandre Cunha
mentioned in issue #389261
- Resolved by Hordur Freyr Yngvason
added typefeature label
added featureaddition label
mentioned in issue #389430 (closed)
- Resolved by Tiger Watson
- Resolved by Hordur Freyr Yngvason
@tigerwnz
, thanks for approving this merge request.This is the first time the merge request is approved. To ensure full test coverage, a new pipeline will be started shortly.
For more info, please refer to the following links:
added pipeline:mr-approved label
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@396b40bf
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@95ad03d5
added 1 commit
- 2d80d545 - Add tests for ApplicationController#set_kas_cookie
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@15b878f2
- Resolved by Timo Furrer
@gitlab-com/gl-security/appsec Could I please have a review of these changes?
In summary, it adds a cookie
_gitlab_kas
intended to be read by KAS, containing the users's encrypted public session id. KAS then passes this back to Rails to authorize proxy requests coming from the GitLab frontend.This MR contains the entire Rails-side MVC. For the KAS side, you can see gitlab-org/cluster-integration/gitlab-agent!841 (merged).
- Resolved by João Alexandre Cunha
added 1 commit
- b2eb1110 - Only call #success and #forbidden in #execute
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@b43767e8
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@12e69b99
- Resolved by João Alexandre Cunha
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@3937f288
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@3ee88a1e
mentioned in issue #381561 (closed)
removed review request for @Alexand
added 1 commit
- 4dffb059 - Use properly masked and encoded token in tests
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@47a6f06b
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@91198df3
added 1 commit
- ea869eeb - Add KAS user authorization behind a feature flag
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@35998153
added 2960 commits
-
ea869eeb...174cf1c9 - 2959 commits from branch
master
- 91f89c95 - Add KAS user authorization behind a feature flag
-
ea869eeb...174cf1c9 - 2959 commits from branch
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@af49098a
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@dea09122
mentioned in issue #390767 (closed)
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@77b46bf9
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@78bc13e9
added 1 commit
- b8ec8b6a - Add KasCookie to the agents controller for testing
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@32d93ee0
assigned to @timofurrer and unassigned @hfyngvason
- Resolved by Timo Furrer
- Resolved by Timo Furrer
added 2040 commits
-
068dd72a...db75ef75 - 2034 commits from branch
master
- e1136890 - Add KAS user authorization behind a feature flag
- 056cc46e - Add rollout issues for feature flags
- 5f51e23a - Add KAS domain to content security policy
- b19b4bdc - Remove from ApplicationController; extract concern
- 2aacc3c6 - Add KasCookie to the agents controller for testing
- a4a9b34c - WIP: add spec for config loader
Toggle commit list-
068dd72a...db75ef75 - 2034 commits from branch
changed milestone to %15.10
added workflowin dev label
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@829243df
added 2 commits
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@4f8eda21
added 336 commits
-
639176a1...48bc3616 - 329 commits from branch
master
- 00e989bc - Add KAS user authorization behind a feature flag
- bcec6549 - Add rollout issues for feature flags
- 4ea1fbc1 - Add KAS domain to content security policy
- ba78cbcf - Remove from ApplicationController; extract concern
- 5ae5a6ac - Add KasCookie to the agents controller for testing
- 2d5ebe22 - Move KAS User Access FFs to 15.10
- 0e0c9f6d - Add spec for config loader
Toggle commit list-
639176a1...48bc3616 - 329 commits from branch
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@895bfc33
mentioned in epic &9859 (closed)
- Resolved by Lin Jen-Shin
@hfyngvason @tigerwnz any ideas why the pipeline fails? It's from this JH Validation and it fails during image creation with:
#34 ERROR: failed to push registry.gitlab.com/gitlab-org-sandbox/gitlab-jh-validation/gitlab-jh-qa:269f6f5d3d0c8bc90d002d6564b91ea0208ce466: server message: insufficient_scope: authorization failed
mentioned in merge request gitlab-org/cluster-integration/gitlab-agent!841 (merged)
marked the checklist item I have evaluated the MR acceptance checklist for this MR. as completed
added 295 commits
-
0e0c9f6d...4b8a049b - 287 commits from branch
master
- 681b799c - Add KAS user authorization behind a feature flag
- e6db35f7 - Add rollout issues for feature flags
- 71bacf3e - Add KAS domain to content security policy
- 6b3f316d - Remove from ApplicationController; extract concern
- b0d80b4a - Add KasCookie to the agents controller for testing
- 81a9e578 - Move KAS User Access FFs to 15.10
- 5b882511 - Add spec for config loader
- 1cdd5889 - Respond with unauthorized instead of not found for invalid sessions in kubernetes proxy
Toggle commit list-
0e0c9f6d...4b8a049b - 287 commits from branch
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@ce9457e1
mentioned in issue #393002 (closed)
added workflowready for review label and removed workflowin dev label
requested review from @rutgerwessels
- Resolved by Timo Furrer
- Resolved by Timo Furrer
- Resolved by Timo Furrer
- Resolved by Timo Furrer
- Resolved by Timo Furrer
- Resolved by Timo Furrer
It looks impressive
I have some suggestions for test coverage but in general looks good!
removed review request for @rutgerwessels
mentioned in issue #393180
- Resolved by Thong Kuah
Tested and working for me @timofurrer! Looks like we now have some conflicts to fix, then I think we should be ready to move to maintainer review.
added 398 commits
-
fd247230...01eca377 - 388 commits from branch
master
- 4c865bea - Add KAS user authorization behind a feature flag
- 69e355e5 - Add rollout issues for feature flags
- ba7fe21d - Add KAS domain to content security policy
- b471f540 - Remove from ApplicationController; extract concern
- f8c35a87 - Add KasCookie to the agents controller for testing
- dadd8fc9 - Move KAS User Access FFs to 15.10
- 624e8135 - Add spec for config loader
- fd63ba24 - Respond with unauthorized instead of not found for invalid sessions in kubernetes proxy
- 3c3579f4 - Add more test coverage
- 3602f153 - Rename `authorizations` to `access_as` in KAS k8s API
Toggle commit list-
fd247230...01eca377 - 388 commits from branch
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@42e25e81
mentioned in issue #393336 (closed)
added workflowin review label and removed workflowready for review label
requested review from @tkuah
- Resolved by Timo Furrer
- Resolved by Thong Kuah
- Resolved by Timo Furrer
- Resolved by Timo Furrer
- Resolved by Nick Malcolm
- Resolved by Thong Kuah
- Resolved by Thong Kuah
- Resolved by Thong Kuah
- Resolved by Timo Furrer
- Resolved by Timo Furrer
- Resolved by Timo Furrer
- Resolved by Thong Kuah
@timofurrer Thanks for this - overall it is easy to understand.
I started with some questions on a higher level, focusing on authentication, and authorization.
Next round, I will look more into functionality
- Resolved by Thong Kuah
added 1279 commits
-
3602f153...4832d535 - 1266 commits from branch
master
- 4832d535...7846e11d - 3 earlier commits
- 61d17191 - Remove from ApplicationController; extract concern
- 73028335 - Add KasCookie to the agents controller for testing
- bf970fa9 - Move KAS User Access FFs to 15.10
- eac2fcc0 - Add spec for config loader
- b299c9fe - Respond with unauthorized instead of not found for invalid sessions in kubernetes proxy
- 28211b7f - Add more test coverage
- d2e738c3 - Rename `authorizations` to `access_as` in KAS k8s API
- 8e6a944e - Make KAS cookie key a constant
- bc99c3fa - Reference N+1 issue for KAS project and group access
- 1d50fbf6 - Use GitLab HTTPS config for KAS cookie secure setting
Toggle commit list-
3602f153...4832d535 - 1266 commits from branch
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@d80b9b87
added 1 commit
- 664cebbe - Use GitLab HTTPS config for KAS cookie secure setting
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@e03cd9c0
- Resolved by Timo Furrer
Thanks @tkuah and @timofurrer! I've responded to the questions directed at me.
added 1 commit
- 2caffb13 - Use explicit default cookie cipher for KAS cookie encryptor
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@637b982f
added 280 commits
-
2caffb13...6a3125f3 - 265 commits from branch
master
- 6a3125f3...b58d6d68 - 5 earlier commits
- 071b2280 - Move KAS User Access FFs to 15.10
- 391af3ba - Add spec for config loader
- 28751ad6 - Respond with unauthorized instead of not found for invalid sessions in kubernetes proxy
- c95ce396 - Add more test coverage
- eeceb7fa - Rename `authorizations` to `access_as` in KAS k8s API
- e25fa254 - Make KAS cookie key a constant
- 7c69c838 - Reference N+1 issue for KAS project and group access
- 20a82727 - Use GitLab HTTPS config for KAS cookie secure setting
- 9e008b5d - Use explicit default cookie cipher for KAS cookie encryptor
- c283340e - Use Warden session serializer instead of session dig
Toggle commit list-
2caffb13...6a3125f3 - 265 commits from branch
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@d2f4c1c7
- Resolved by Timo Furrer
- Resolved by Timo Furrer
- Resolved by Thong Kuah
- Resolved by Thong Kuah
- Resolved by Thong Kuah
Thanks @timofurrer @hfyngvason I have looked into the functionality, and specs as well. Just minor comments from me.
I haven't had time to try our manual validation locally but I trust the screenshots above !
removed review request for @tkuah
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@e45d3f28
mentioned in issue #395131 (closed)
added 1 commit
- 6372f8fc - Memoize authorized projects and groups calls
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@bcbaa874
requested review from @tkuah
mentioned in issue #395498 (closed)
mentioned in commit gitlab-org-sandbox/gitlab-jh-validation@27f2376d
- Resolved by Thong Kuah
mentioned in issue #395499
enabled an automatic merge when the pipeline for 27f2376d succeeds
- Resolved by Timo Furrer
Great work @timofurrer @hfyngvason ! Thanks for the care you have taken.
Setting MWPS
mentioned in commit dce8db7f
added workflowstaging-canary label and removed workflowin review label
mentioned in merge request !114073 (merged)
added workflowcanary label and removed workflowstaging-canary label
added workflowstaging label and removed workflowcanary label
added workflowproduction label and removed workflowstaging label
mentioned in commit b9b19a1d
mentioned in merge request !115226 (merged)
added releasedcandidate label
added releasedpublished label and removed releasedcandidate label
mentioned in issue gitlab-org/opstrace/opstrace#2241 (closed)
added groupenvironments label and removed groupconfigure [DEPRECATED] label