Add Demonstrating Proof of Possession (DPoP) for Personal Access Tokens
What does this MR do and why?
This MR initiates the implementation of sender-constrained access tokens designed to minimise the risk of token leaks. I am putting this feature behind a feature flag.
Related to Sender constraining personal access tokens (#425130).
How to set up and validate locally
Currently this feature works only for RSA keys. Support for other algorithms will be added later.
For frontend:
- Checkout this branch locally.
- Run
bin/rails db:migrate
- In rails console, enable the feature flag:
Feature.enable(:dpop_authentication, User.first)
- Login as root.
- Go to Settings > Access tokens > Toggle the DPoP option.
- Confirm it persists in the database
User.first.dpop_enabled
and also on the frontend on refreshing the page.
For backend:
- Build
glab
from this branch. - Ensure DPoP is enabled following the steps above.
- Ensure you have an SSH key-pair setup with the public key uploaded to your user account. Ensure that the key type is saved as "Signing" or "Authentication and Signing".
- Using
glab
generate a DPoP header:bin/glab auth dpop-gen --pat "glpat-PAT" --private-key ~.ssh/id_rsa
- Use the generated header to make an HTTP API request:
curl http://localhost:3000/api/v4/projects --header "Private-Token: glpat-PAT" --header "DPoP: <GLAB OUTPUT HERE>"
- Confirm valid response is received. Confirm that the request fails without a valid DPoP header.
Merge request reports
Activity
assigned to @ameyadarshan
- Resolved by 🤖 GitLab Bot 🤖
Proper labels assigned to this merge request. Please ignore me.
@ameyadarshan - 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 🤖
- A deleted user
- Resolved by Ameya Darshan
5 Warnings This merge request is quite big (887 lines changed), please consider splitting it into multiple merge requests. 11bf173d: The commit subject must contain at least 3 words. For more information, take a look at our Commit message guidelines. 44443f2c: 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. Migration Timestamp Out of Date
The following migrations have timestamps that are over three weeks old:- db/migrate/20240523115831_add_dpop_enabled_to_user_preferences.rb
Please double check the timestamps and update them if possible. Why does this matter?
This merge request contains lines with testid selectors. Please ensure e2e:package-and-test
job is run.3 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 files that require a review from the Database team. This merge request adds or changes documentation files. A review from the Technical Writing team before you merge is recommended. Reviews can happen after you merge. This merge request requires a database review. To make sure these changes are reviewed, take the following steps:
- Ensure the merge request has database and databasereview pending labels. If the merge request modifies database files, Danger will do this for you.
- Prepare your MR for database review according to the docs.
- Assign and mention the database reviewer suggested by Reviewer Roulette.
The following files require a review from the Database team:
db/migrate/20240523115831_add_dpop_enabled_to_user_preferences.rb
db/schema_migrations/20240523115831
db/structure.sql
Documentation review
The following files require a review from a technical writer:
-
doc/user/profile/personal_access_tokens.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.
testid
selectorsThe following changed lines in this MR contain
testid
selectors:app/views/user_settings/personal_access_tokens/index.html.haml
+ = gitlab_ui_form_for @user, url: toggle_dpop_user_settings_personal_access_tokens_path, method: :put, html: { data: { testid: 'dpop-form' } } do |f|
If the
e2e:package-and-test
job in theqa
stage has run automatically, please ensure the tests are passing. If the job has not run, please start themanual:e2e-test-pipeline-generate
job in theprepare
stage and ensure the tests infollow-up:e2e:package-and-test-ee
pipeline are passing.For the list of known failures please refer to the latest pipeline triage issue.
If your changes are under a feature flag, please check our Testing with feature flags documentation for instructions.
Reviewer roulette
Category Reviewer Maintainer backend @jmontal
(UTC-6, 11.5 hours behind author)
@vshushlin
(UTC+2, 3.5 hours behind author)
database @fcatteau
(UTC+2, 3.5 hours behind author)
@praba.m7n
(UTC+5.5, same timezone as author)
frontend @leetickett-gitlab
(UTC+1, 4.5 hours behind author)
@deepika.guliani
(UTC+5.5, same timezone as author)
groupauthentication Reviewer review is optional for groupauthentication @ifarkas
(UTC+2, 3.5 hours behind author)
Please check reviewer's status!
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
Dangeradded devopsgovern sectionsec labels
E2E Test Result Summary
allure-report-publisher
generated test report!e2e-test-on-gdk:
test report for 11bf173dexpand test summary
+------------------------------------------------------------------+ | suites summary | +-------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +-------------+--------+--------+---------+-------+-------+--------+ | Govern | 128 | 0 | 2 | 0 | 130 | ✅ | | Create | 121 | 0 | 10 | 1 | 131 | ✅ | | Verify | 31 | 0 | 1 | 0 | 32 | ✅ | | Package | 19 | 0 | 12 | 0 | 31 | ✅ | | Plan | 54 | 0 | 2 | 0 | 56 | ✅ | | Data Stores | 31 | 0 | 0 | 0 | 31 | ✅ | | Release | 5 | 0 | 0 | 0 | 5 | ✅ | | Monitor | 8 | 0 | 0 | 0 | 8 | ✅ | | Analytics | 1 | 0 | 1 | 0 | 2 | ✅ | | Manage | 0 | 0 | 1 | 0 | 1 | ➖ | +-------------+--------+--------+---------+-------+-------+--------+ | Total | 398 | 0 | 29 | 1 | 427 | ✅ | +-------------+--------+--------+---------+-------+-------+--------+
e2e-package-and-test:
test report for 11bf173dexpand test summary
+------------------------------------------------------------------+ | suites summary | +-------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +-------------+--------+--------+---------+-------+-------+--------+ | Govern | 180 | 0 | 18 | 0 | 198 | ✅ | | Plan | 44 | 0 | 4 | 0 | 48 | ✅ | | Package | 6 | 0 | 8 | 0 | 14 | ✅ | | Verify | 10 | 0 | 0 | 0 | 10 | ✅ | | Data Stores | 22 | 0 | 0 | 0 | 22 | ✅ | | Create | 46 | 0 | 6 | 0 | 52 | ✅ | | Release | 2 | 0 | 0 | 0 | 2 | ✅ | | Monitor | 8 | 0 | 0 | 0 | 8 | ✅ | +-------------+--------+--------+---------+-------+-------+--------+ | Total | 318 | 0 | 36 | 0 | 354 | ✅ | +-------------+--------+--------+---------+-------+-------+--------+
added typefeature label
added 14 commits
- 8c3eff3e...c8de1787 - 4 earlier commits
- f488ccd1 - Add more HAML specs
- 9e5ebf11 - Fix HAML spec
- 2e699d9b - Fix help_page_path error
- fe0bf295 - Fix rubocop offenses and add gitlab.pot
- 6ae7f984 - Add doc placeholder
- 75c71cf4 - Add backend validation
- 809f19ec - Add JWK validation
- b51cdf60 - Add errors, spec and spec placeholders
- 93310bb5 - Add specs for errors
- 5e0e104e - Fix error specs
Toggle commit listadded 16 commits
- 5e0e104e...a902ab5d - 6 earlier commits
- 0ffc7d27 - Add more HAML specs
- 1cdb4614 - Fix HAML spec
- 6dae6c37 - Fix help_page_path error
- 4f8abcc7 - Fix rubocop offenses and add gitlab.pot
- f909286d - Add doc placeholder
- 33a3422f - Add backend validation
- 54494f5b - Add JWK validation
- 8b517a9d - Add errors, spec and spec placeholders
- 919f0405 - Add specs for errors
- 7a48fded - Fix error specs
Toggle commit listadded 1 commit
- 00d9812f - Fix rubocop offenses and add JWT expiry with spec
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 #6544434696 spec/features/profile_spec.rb#L68
Profile account page allows resetting of feed token 61.99 s < 50.13 s #6544434696 spec/features/profile_spec.rb#L88
Profile account page allows resetting of incoming email token 124.57 s < 50.13 s #6544434696 spec/features/profile_spec.rb#L45
Profile account page when I delete my account shows invalid password flash message 61.96 s < 50.13 s #6544434696 spec/features/profile_spec.rb#L57
Profile account page when I delete my account does not show delete button when user owns a group 62.04 s < 50.13 s #6544434154 spec/features/profile_spec.rb#L68
Profile account page allows resetting of feed token 62.42 s < 50.13 s #6544434154 spec/features/profile_spec.rb#L88
Profile account page allows resetting of incoming email token 62.54 s < 50.13 s #6544434154 spec/features/profile_spec.rb#L45
Profile account page when I delete my account shows invalid password flash message 62.25 s < 50.13 s #6544434154 spec/features/profile_spec.rb#L57
Profile account page when I delete my account does not show delete button when user owns a group 62.24 s < 50.13 s #6675178351 ee/spec/models/epic_spec.rb#L917
Epic relative positioning there is a parent behaves like a class that supports relative positioning .move_nulls_to_end can move many nulls 45.99 s < 45.4 s #6675178319 ee/spec/models/epic_spec.rb#L917
Epic relative positioning there is a parent behaves like a class that supports relative positioning .move_nulls_to_end can move many nulls 54.62 s < 45.4 s #6686945175 ee/spec/models/epic_spec.rb#L917
Epic relative positioning there is a parent behaves like a class that supports relative positioning .move_nulls_to_end can move many nulls 49.3 s < 45.4 s #6717709293 spec/features/user_settings/personal_access_tokens_spec.rb#L92
User Settings > Personal Access Tokens inactive tokens allows revocation of an active token 62.22 s < 50.13 s #6717709293 spec/features/user_settings/personal_access_tokens_spec.rb#L107
User Settings > Personal Access Tokens inactive tokens when revocation fails displays an error message 124.79 s < 50.13 s #6717709568 spec/features/user_settings/personal_access_tokens_spec.rb#L92
User Settings > Personal Access Tokens inactive tokens allows revocation of an active token 62.67 s < 50.13 s #6717709568 spec/features/user_settings/personal_access_tokens_spec.rb#L107
User Settings > Personal Access Tokens inactive tokens when revocation fails displays an error message 125.73 s < 50.13 s #6717709105 spec/features/user_settings/personal_access_tokens_spec.rb#L92
User Settings > Personal Access Tokens inactive tokens allows revocation of an active token 62.46 s < 50.13 s #6717709105 spec/features/user_settings/personal_access_tokens_spec.rb#L107
User Settings > Personal Access Tokens inactive tokens when revocation fails displays an error message 63.15 s < 50.13 s #6717709822 spec/features/user_settings/personal_access_tokens_spec.rb#L92
User Settings > Personal Access Tokens inactive tokens allows revocation of an active token 125.39 s < 50.13 s #6717709822 spec/features/user_settings/personal_access_tokens_spec.rb#L107
User Settings > Personal Access Tokens inactive tokens when revocation fails displays an error message 125.05 s < 50.13 s #6732124017 ee/spec/models/epic_spec.rb#L917
Epic relative positioning there is a parent behaves like a class that supports relative positioning .move_nulls_to_end can move many nulls 47.27 s < 45.4 s #6732123906 ee/spec/models/epic_spec.rb#L917
Epic relative positioning there is a parent behaves like a class that supports relative positioning .move_nulls_to_end can move many nulls 49.77 s < 45.4 s #6860828747 ee/spec/models/epic_spec.rb#L917
Epic relative positioning there is a parent behaves like a class that supports relative positioning .move_nulls_to_end can move many nulls 45.86 s < 45.4 s #6869504814 ee/spec/models/epic_spec.rb#L917
Epic relative positioning there is a parent behaves like a class that supports relative positioning .move_nulls_to_end can move many nulls 45.58 s < 45.4 s #6900553323 ee/spec/models/epic_spec.rb#L917
Epic relative positioning there is a parent behaves like a class that supports relative positioning .move_nulls_to_end can move many nulls 46.25 s < 45.4 s #6902408334 ee/spec/models/epic_spec.rb#L917
Epic relative positioning there is a parent behaves like a class that supports relative positioning .move_nulls_to_end can move many nulls 47.65 s < 45.4 s #6922146132 ee/spec/models/epic_spec.rb#L917
Epic relative positioning there is a parent behaves like a class that supports relative positioning .move_nulls_to_end can move many nulls 47.17 s < 45.4 s #6923373138 ee/spec/models/epic_spec.rb#L924
Epic relative positioning there is a parent behaves like a class that supports relative positioning .move_nulls_to_end can move many nulls 48.43 s < 45.4 s - A deleted user
added rspec:slow test detected label