From 44443f2c684913330826724f27115a4b377ac0c6 Mon Sep 17 00:00:00 2001 From: Ameya Darshan Date: Fri, 1 Mar 2024 06:42:01 -0800 Subject: [PATCH 1/3] Add DPoP functionality to Personal Access Tokens --- app/controllers/graphql_controller.rb | 36 ++ .../personal_access_tokens_controller.rb | 26 ++ app/models/user.rb | 1 + app/models/user_preference.rb | 1 + .../auth/dpop_authentication_service.rb | 161 +++++++ .../personal_access_tokens/index.html.haml | 15 + .../beta/dpop_authentication.yml | 9 + config/routes/user_settings.rb | 1 + ...31_add_dpop_enabled_to_user_preferences.rb | 31 ++ db/schema_migrations/20240523115831 | 1 + db/structure.sql | 1 + doc/user/profile/personal_access_tokens.md | 37 ++ ee/spec/requests/api/epics_spec.rb | 1 + lib/api/api_guard.rb | 8 +- lib/gitlab/auth/auth_finders.rb | 17 +- locale/gitlab.pot | 15 + .../personal_access_tokens_controller_spec.rb | 25 + spec/models/user_preference_spec.rb | 14 + spec/requests/api/ci/jobs_spec.rb | 2 +- .../api/graphql/projects/projects_spec.rb | 2 +- spec/requests/api/project_import_spec.rb | 2 +- .../auth/dpop_authentication_service_spec.rb | 431 ++++++++++++++++++ .../index.html.haml_spec.rb | 52 +++ 23 files changed, 884 insertions(+), 5 deletions(-) create mode 100644 app/services/auth/dpop_authentication_service.rb create mode 100644 config/feature_flags/beta/dpop_authentication.yml create mode 100644 db/migrate/20240523115831_add_dpop_enabled_to_user_preferences.rb create mode 100644 db/schema_migrations/20240523115831 create mode 100644 spec/services/auth/dpop_authentication_service_spec.rb create mode 100644 spec/views/user_settings/personal_access_tokens/index.html.haml_spec.rb diff --git a/app/controllers/graphql_controller.rb b/app/controllers/graphql_controller.rb index a86afda9a4ed37..c2ea3b504ea827 100644 --- a/app/controllers/graphql_controller.rb +++ b/app/controllers/graphql_controller.rb @@ -8,6 +8,8 @@ class GraphqlController < ApplicationController # Header can be passed by tests to disable SQL query limits. DISABLE_SQL_QUERY_LIMIT_HEADER = 'HTTP_X_GITLAB_DISABLE_SQL_QUERY_LIMIT' + PRIVATE_TOKEN_HEADER = 'HTTP_PRIVATE_TOKEN' + PRIVATE_TOKEN_PARAM = :private_token # Max size of the query text in characters MAX_QUERY_SIZE = 10_000 @@ -29,6 +31,8 @@ class GraphqlController < ApplicationController # around in GraphiQL. protect_from_forgery with: :null_session, only: :execute + before_action(only: [:execute]) { check_dpop } + # must come first: current_user is set up here before_action(only: [:execute]) do if Feature.enabled? :graphql_minimal_auth_methods @@ -88,6 +92,12 @@ def execute end end + rescue_from Gitlab::Auth::DpopValidationError do |exception| + log_exception(exception) + + render_error(exception.message, status: :unauthorized) + end + rescue_from Gitlab::Auth::TooManyIps do |exception| log_exception(exception) @@ -130,6 +140,7 @@ def feature_category def authenticate_graphql user = request_authenticator.find_user_from_web_access_token(:api, scopes: authorization_scopes) user ||= request_authenticator.find_user_from_personal_access_token_for_api_or_git + sessionless_sign_in(user) if user rescue Gitlab::Auth::AuthenticationError nil @@ -140,6 +151,31 @@ def authorization_scopes [:api, :read_api] end + def check_dpop + ::Auth::DpopAuthenticationService.new(current_user, get_dpop_token_from_request, + get_personal_access_token).execute + end + + def get_dpop_token_from_request + request.headers['dpop'].presence + end + + def get_personal_access_token + token = + request.params[PRIVATE_TOKEN_PARAM].presence || + request.env[PRIVATE_TOKEN_HEADER].presence || + parsed_oauth_token + return unless token + + # || raise(UnauthorizedError) is not needed as authenticate_sessionless_user!(:api) + # will raise the error + PersonalAccessToken.find_by_token(token) + end + + def parsed_oauth_token + Doorkeeper::OAuth::Token.from_request(request, *Doorkeeper.configuration.access_token_methods) + end + def permitted_multiplex_params params.permit(_json: [:query, :operationName, { variables: {} }]) end diff --git a/app/controllers/user_settings/personal_access_tokens_controller.rb b/app/controllers/user_settings/personal_access_tokens_controller.rb index de1a33647fa38b..9b10528239fc31 100644 --- a/app/controllers/user_settings/personal_access_tokens_controller.rb +++ b/app/controllers/user_settings/personal_access_tokens_controller.rb @@ -9,6 +9,7 @@ class PersonalAccessTokensController < ApplicationController before_action :check_personal_access_tokens_enabled prepend_before_action(only: [:index]) { authenticate_sessionless_user!(:ics) } + before_action :user def index set_index_vars @@ -60,8 +61,29 @@ def revoke redirect_to user_settings_personal_access_tokens_path end + def toggle_dpop + unless Feature.enabled?(:dpop_authentication, user, type: :beta) + redirect_to user_settings_personal_access_tokens_path + return + end + + result = UserPreferences::UpdateService.new(@user, dpop_params).execute + + if result.success? + flash[:notice] = _('DPoP preference updated.') + else + flash[:warning] = _('Unable to update DPoP preference.') + end + + redirect_to user_settings_personal_access_tokens_path + end + private + def dpop_params + params.require(:user).permit(:dpop_enabled) + end + def finder(options = {}) PersonalAccessTokensFinder.new({ user: current_user, impersonation: false }.merge(options)) end @@ -70,6 +92,10 @@ def personal_access_token_params params.require(:personal_access_token).permit(:name, :expires_at, scopes: []) end + def user + @user = current_user + end + def set_index_vars @scopes = Gitlab::Auth.available_scopes_for(current_user) @active_access_tokens = active_access_tokens diff --git a/app/models/user.rb b/app/models/user.rb index b7ea304fc4d9aa..c0db1e8d58e3c1 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -402,6 +402,7 @@ def update_tracked_fields!(request) delegate :notes_filter_for, :set_notes_filter, :first_day_of_week, :first_day_of_week=, + :dpop_enabled, :timezone, :timezone=, :time_display_relative, :time_display_relative=, :time_display_format, :time_display_format=, diff --git a/app/models/user_preference.rb b/app/models/user_preference.rb index 73162ba6e84340..762242716e086d 100644 --- a/app/models/user_preference.rb +++ b/app/models/user_preference.rb @@ -40,6 +40,7 @@ class UserPreference < MainClusterwide::ApplicationRecord attribute :project_shortcut_buttons, default: true attribute :keyboard_shortcuts_enabled, default: true attribute :use_web_ide_extension_marketplace, default: false + attribute :dpop_enabled, default: false enum visibility_pipeline_id_type: { id: 0, iid: 1 } enum extensions_marketplace_opt_in_status: Enums::WebIde::ExtensionsMarketplaceOptInStatus.statuses diff --git a/app/services/auth/dpop_authentication_service.rb b/app/services/auth/dpop_authentication_service.rb new file mode 100644 index 00000000000000..41652648bd8b1c --- /dev/null +++ b/app/services/auth/dpop_authentication_service.rb @@ -0,0 +1,161 @@ +# frozen_string_literal: true + +# rubocop:disable Gitlab/BoundedContexts -- following the same structure as other services + +module Auth + class DpopAuthenticationService < BaseService + include Gitlab::Utils::StrongMemoize + + SUPPORTED_ALGOS = { + 'ssh-ed25519' => 'ED25519', + 'ecdsa-sha2-nistp256' => 'ES256', + 'ssh-rsa' => 'RS512' + }.freeze + + def initialize(user, dpop, pat) + @user = user + @dpop = dpop + @pat = pat + end + + def execute + return unless @user + return unless dpop_enabled? + return unless @pat + + validate_dpop_header_count + + key = find_key_from_dpop_proof + validate_dpop_proof!(key) + end + + private + + def dpop_enabled? + Feature.enabled?(:dpop_authentication, @user, type: :beta) && @user.dpop_enabled + end + + def find_key_from_dpop_proof + header = get_header_from_token + validate_header!(header) + + key = @user.keys.signing.find_by_fingerprint_sha256(header["kid"]&.delete_prefix("SHA256:"))&.key + raise Gitlab::Auth::DpopValidationError, "No matching key found" unless key + + # remove when support for ED25519 is added + # this checks if somehow the matching key is not RSA + # even if the alg in header says it is + validate_key_algo!(key) + + key + end + + def get_header_from_token + decoded_token = JWT.decode(@dpop, '', false) + decoded_token[1] + rescue JWT::DecodeError => decode_error + raise Gitlab::Auth::DpopValidationError, "Malformed JWT, unable to decode. #{decode_error.message}" + end + + def validate_header!(header) + raise Gitlab::Auth::DpopValidationError, "No kid in JWT, unable to fetch key" if header["kid"].nil? + + algo = header['alg'] + + # remove when support for ED25519 is added + # this checks for 'alg' in the header and exits early + unless algo.casecmp('RS512') == 0 + raise Gitlab::Auth::DpopValidationError, + "Currently only RSA keys are supported" + end + + return if header["kid"].split(":").length == 2 + + raise Gitlab::Auth::DpopValidationError, "Malformed fingerprint value in kid" + end + + def validate_key_algo!(key) + algo = key.split(" ")[0] + + return if algo.casecmp('ssh-rsa') == 0 + + raise Gitlab::Auth::DpopValidationError, "Currently only RSA keys are supported" + end + + def validate_dpop_proof!(key) + selected_algorithm = get_dpop_algorithm(key) + + raise Gitlab::Auth::DpopValidationError, "None algorithm is not supported" if selected_algorithm.nil? + + public_key = public_key(key) + decoded_token = decode_token(public_key, selected_algorithm) + + validate_decoded_token!(decoded_token) + validate_key_type!(public_key, decoded_token) + end + + def public_key(key) + SSHData::PublicKey.parse_openssh(key).openssl + rescue SSHData::DecodeError => decode_error + raise Gitlab::Auth::DpopValidationError, "Unable to parse public key. #{decode_error.message}" + end + + def decode_token(public_key, selected_algorithm) + JWT.decode(@dpop, public_key, true, + { required_claims: %w[exp ath iat], algorithm: selected_algorithm, verify_iat: true }) + rescue JWT::ExpiredSignature + raise Gitlab::Auth::DpopValidationError, "Signature expired" + rescue JWT::InvalidIatError + raise Gitlab::Auth::DpopValidationError, "Invalid IAT value" + rescue JWT::MissingRequiredClaim => decode_error + raise Gitlab::Auth::DpopValidationError, decode_error.message + rescue StandardError => decode_error + raise Gitlab::Auth::DpopValidationError, "Unable to decode JWT. #{decode_error.message}" + end + + def get_dpop_algorithm(key) + SUPPORTED_ALGOS.each do |key_algorithm, jwt_algorithm| + return jwt_algorithm if key.start_with?(key_algorithm) + end + + nil + end + + def validate_decoded_token!(decoded_token) + raise Gitlab::Auth::DpopValidationError, "Unable to decode JWT" if decoded_token.nil? + raise Gitlab::Auth::DpopValidationError, "Invalid typ value in JWT" unless decoded_token[1]["typ"] == 'dpop+jwt' + + validate_ath!(decoded_token) + end + + def validate_ath!(decoded_token) + return if decoded_token[0]["ath"] == @pat.token_digest + + raise Gitlab::Auth::DpopValidationError, "Incorrect access token hash in JWT" + end + + def validate_key_type!(public_key, decoded_token) + jwk = decoded_token[1]["jwk"] + raise Gitlab::Auth::DpopValidationError, "Failed to parse JWK" if jwk.nil? + + kty = jwk["kty"] + raise Gitlab::Auth::DpopValidationError, "No key type in JWK" if kty.nil? + + raise Gitlab::Auth::DpopValidationError, "Unknown algorithm in JWK" unless kty == "RSA" + return if public_key.to_s == OpenSSL::PKey.read(JWT::JWK::RSA.import(jwk).public_key.to_pem).to_s + + raise Gitlab::Auth::DpopValidationError, "Failed to parse JWK" + end + + def validate_dpop_header_count + # check if there are 2 dpop headers && + # check if there are 2 JWTs in dpop header value + raise Gitlab::Auth::DpopValidationError, "No DPoP header in request" unless @dpop + + return if @dpop.split(",").count == 1 && @dpop.split(" ").count == 1 + + raise Gitlab::Auth::DpopValidationError, "Only 1 DPoP header is allowed in request" + end + end +end +# rubocop:enable Gitlab/BoundedContexts diff --git a/app/views/user_settings/personal_access_tokens/index.html.haml b/app/views/user_settings/personal_access_tokens/index.html.haml index dc87407adc7d26..d5da55aa062e86 100644 --- a/app/views/user_settings/personal_access_tokens/index.html.haml +++ b/app/views/user_settings/personal_access_tokens/index.html.haml @@ -39,4 +39,19 @@ #js-access-token-table-app{ data: { access_token_type: type, access_token_type_plural: type_plural, initial_active_access_tokens: @active_access_tokens.to_json } } +- if Feature.enabled?(:dpop_authentication, @user, type: :beta) + = gitlab_ui_form_for @user, url: toggle_dpop_user_settings_personal_access_tokens_path, method: :put, html: { data: { testid: 'dpop-form' } } do |f| + .settings-section.js-dpop-form.js-search-settings-section#dpop-toggle + .settings-sticky-header + .settings-sticky-header-inner + %h4.gl-my-0 + = s_('AccessTokens|Use Demonstrating Proof of Possession (DPoP)') + %p.gl-text-secondary + = s_('AccessTokens|Choose whether you want to enforce Demonstrating Proof of Possession (DPoP) with the usage of Personal Access Tokens on the REST and GraphQL APIs.') + = succeed '.' do + = link_to _('Learn more'), help_page_path('user/profile/personal_access_tokens', anchor: 'enable-dpop-for-your-personal-access-tokens'), target: '_blank', rel: 'noopener noreferrer' + .form-group + = f.gitlab_ui_checkbox_component :dpop_enabled, s_('AccessTokens|Enable DPoP'), checkbox_options: { checked: @user.dpop_enabled } + = f.submit _('Save changes'), pajamas_button: true + #js-tokens-app{ data: { tokens_data: tokens_app_data } } diff --git a/config/feature_flags/beta/dpop_authentication.yml b/config/feature_flags/beta/dpop_authentication.yml new file mode 100644 index 00000000000000..426b4ab7a4f3c2 --- /dev/null +++ b/config/feature_flags/beta/dpop_authentication.yml @@ -0,0 +1,9 @@ +--- +name: dpop_authentication +feature_issue_url: +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/148175 +rollout_issue_url: https://gitlab.com/gitlab-com/gl-infra/production/-/issues/17713 +milestone: '17.0' +group: group::authentication +type: beta +default_enabled: false diff --git a/config/routes/user_settings.rb b/config/routes/user_settings.rb index fbd85aabcbdf27..8ca788d1d0cb05 100644 --- a/config/routes/user_settings.rb +++ b/config/routes/user_settings.rb @@ -14,6 +14,7 @@ end end resources :personal_access_tokens, only: [:index, :create] do + put :toggle_dpop, on: :collection member do put :revoke end diff --git a/db/migrate/20240523115831_add_dpop_enabled_to_user_preferences.rb b/db/migrate/20240523115831_add_dpop_enabled_to_user_preferences.rb new file mode 100644 index 00000000000000..d77c70362e5ed7 --- /dev/null +++ b/db/migrate/20240523115831_add_dpop_enabled_to_user_preferences.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddDpopEnabledToUserPreferences < Gitlab::Database::Migration[2.2] + # When using the methods "add_concurrent_index" or "remove_concurrent_index" + # you must disable the use of transactions + # as these methods can not run in an existing transaction. + # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure + # that either of them is the _only_ method called in the migration, + # any other changes should go in a separate migration. + # This ensures that upon failure _only_ the index creation or removing fails + # and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + # disable_ddl_transaction! + # + # Configure the `gitlab_schema` to perform data manipulation (DML). + # Visit: https://docs.gitlab.com/ee/development/database/migrations_for_multiple_databases.html + # restrict_gitlab_migration gitlab_schema: :gitlab_main + + # Add dependent 'batched_background_migrations.queued_migration_version' values. + # DEPENDENT_BATCHED_BACKGROUND_MIGRATIONS = [] + milestone '16.11' + + def change + add_column :user_preferences, :dpop_enabled, :boolean, default: false, null: false + end +end diff --git a/db/schema_migrations/20240523115831 b/db/schema_migrations/20240523115831 new file mode 100644 index 00000000000000..282516c0dcee43 --- /dev/null +++ b/db/schema_migrations/20240523115831 @@ -0,0 +1 @@ +76192fcb6c3d975f95976994023053cbcf245ea75c8ea7fed387995ec4da410c \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index accdd587c470a2..68634bf5afb41a 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -17534,6 +17534,7 @@ CREATE TABLE user_preferences ( early_access_program_participant boolean DEFAULT false NOT NULL, early_access_program_tracking boolean DEFAULT false NOT NULL, extensions_marketplace_opt_in_status smallint DEFAULT 0 NOT NULL, + dpop_enabled boolean DEFAULT false, CONSTRAINT check_1d670edc68 CHECK ((time_display_relative IS NOT NULL)), CONSTRAINT check_89bf269f41 CHECK ((char_length(diffs_deletion_color) <= 7)), CONSTRAINT check_b22446f91a CHECK ((render_whitespace_in_code IS NOT NULL)), diff --git a/doc/user/profile/personal_access_tokens.md b/doc/user/profile/personal_access_tokens.md index c831f3411a0920..d5b0e24dec40eb 100644 --- a/doc/user/profile/personal_access_tokens.md +++ b/doc/user/profile/personal_access_tokens.md @@ -216,6 +216,43 @@ Prerequisites: You can now create personal access tokens for a service account user with no expiry date. +## Enable DPoP for your personal access tokens + +DETAILS: +**Tier:** Free, Premium, Ultimate +**Offering:** Self-managed + +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/148175) in GitLab 17.0 [with a flag](../../administration/feature_flags.md) named `dpop_authentication`. Disabled by default. + +FLAG: +The availability of this feature is controlled by a feature flag. +For more information, see the history. +This feature is available for testing, but not ready for production use. + +Demonstrating Proof of Possession (DPoP) enhances the security of your personal access tokens, +and minimizes the effects of unintended token leaks. When you enable this feature on your +account, all REST and GraphQL API requests containing a PAT must also provide a signed DPoP header. Creating a +signed DPoP header requires your corresponding private SSH key. + +NOTE: +If you enable this feature, all REST and GraphQL API requests without a valid DPoP header fail. + +Prerequisites: + +- You must have [added at least one public SSH key](../ssh.md#add-an-ssh-key-to-your-gitlab-account) + to your account, with the **Usage type** of **Signing**, or **Authentication & Signing**. + +To require DPoP on all calls to the REST and GraphQL APIs: + +1. On the left sidebar, select your avatar. +1. Select **Edit profile**. +1. On the left sidebar, select **Access Tokens**. +1. Go to the **Use Demonstrating Proof of Possession** section, and select **Enable DPoP**. +1. Select **Save changes**. + +To learn more about DPoP headers, see the blueprint +[Sender Constraining Personal Access Tokens](https://gitlab.com/gitlab-com/gl-security/appsec/security-feature-blueprints/-/tree/main/sender_constraining_access_tokens). + ## Create a personal access token programmatically DETAILS: diff --git a/ee/spec/requests/api/epics_spec.rb b/ee/spec/requests/api/epics_spec.rb index 9b8afa91f2adb1..cb9d6a7bc19ee5 100644 --- a/ee/spec/requests/api/epics_spec.rb +++ b/ee/spec/requests/api/epics_spec.rb @@ -681,6 +681,7 @@ context 'when the request is correct' do before do + allow(Gitlab::QueryLimiting::Transaction).to receive(:threshold).and_return(102) post api(url, user), params: params end diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index 7033856a42e09d..66c9a4cf8b8d56 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -147,7 +147,8 @@ def install_error_responders(base) Gitlab::Auth::ExpiredError, Gitlab::Auth::RevokedError, Gitlab::Auth::ImpersonationDisabled, - Gitlab::Auth::InsufficientScopeError] + Gitlab::Auth::InsufficientScopeError, + Gitlab::Auth::DpopValidationError] base.__send__(:rescue_from, *error_classes, oauth2_bearer_token_error_handler) # rubocop:disable GitlabSecurity/PublicSend end @@ -186,6 +187,11 @@ def oauth2_bearer_token_error_handler :insufficient_scope, Rack::OAuth2::Server::Resource::ErrorMethods::DEFAULT_DESCRIPTION[:insufficient_scope], { scope: e.scopes }) + + when Gitlab::Auth::DpopValidationError + Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new( + :dpop_error, + e) end status, headers, body = response.finish diff --git a/lib/gitlab/auth/auth_finders.rb b/lib/gitlab/auth/auth_finders.rb index f753db5076b70e..41e7ad43e66c5e 100644 --- a/lib/gitlab/auth/auth_finders.rb +++ b/lib/gitlab/auth/auth_finders.rb @@ -10,6 +10,12 @@ module Auth ImpersonationDisabled = Class.new(AuthenticationError) UnauthorizedError = Class.new(AuthenticationError) + class DpopValidationError < AuthenticationError + def initialize(msg) + super("DPoP validation error: #{msg}") + end + end + class InsufficientScopeError < AuthenticationError attr_reader :scopes @@ -202,6 +208,10 @@ def authentication_token_present? parsed_oauth_token.present? end + def get_dpop_token_from_request + current_request.headers['dpop'].presence + end + private def find_user_from_job_bearer_token @@ -252,7 +262,12 @@ def find_personal_access_token return unless token # Expiration, revocation and scopes are verified in `validate_access_token!` - PersonalAccessToken.find_by_token(token.to_s) || raise(UnauthorizedError) + personal_access_token = PersonalAccessToken.find_by_token(token.to_s) || raise(UnauthorizedError) + + ::Auth::DpopAuthenticationService.new(personal_access_token.user, get_dpop_token_from_request, + personal_access_token).execute + + personal_access_token end def find_oauth_access_token diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 736ba58b3eedec..140badcdb0ea8f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2684,6 +2684,9 @@ msgstr "" msgid "AccessTokens|Are you sure? Any issue email addresses currently in use will stop working." msgstr "" +msgid "AccessTokens|Choose whether you want to enforce Demonstrating Proof of Possession (DPoP) with the usage of Personal Access Tokens on the REST and GraphQL APIs." +msgstr "" + msgid "AccessTokens|Copy feed token" msgstr "" @@ -2699,6 +2702,9 @@ msgstr "" msgid "AccessTokens|Created" msgstr "" +msgid "AccessTokens|Enable DPoP" +msgstr "" + msgid "AccessTokens|Feed token" msgstr "" @@ -2747,6 +2753,9 @@ msgstr "" msgid "AccessTokens|Token name" msgstr "" +msgid "AccessTokens|Use Demonstrating Proof of Possession (DPoP)" +msgstr "" + msgid "AccessTokens|You can also use personal access tokens to authenticate against Git over HTTP." msgstr "" @@ -16353,6 +16362,9 @@ msgstr "" msgid "DORA4Metrics|Total projects (%{count}) with DORA metrics" msgstr "" +msgid "DPoP preference updated." +msgstr "" + msgid "DSN" msgstr "" @@ -55427,6 +55439,9 @@ msgstr "" msgid "Unable to suggest a path. Please refresh and try again." msgstr "" +msgid "Unable to update DPoP preference." +msgstr "" + msgid "Unable to update label prioritization at this time" msgstr "" diff --git a/spec/controllers/user_settings/personal_access_tokens_controller_spec.rb b/spec/controllers/user_settings/personal_access_tokens_controller_spec.rb index 0487c1f2e2f23f..e33d013afda139 100644 --- a/spec/controllers/user_settings/personal_access_tokens_controller_spec.rb +++ b/spec/controllers/user_settings/personal_access_tokens_controller_spec.rb @@ -63,6 +63,31 @@ def created_token it_behaves_like 'GET access tokens are paginated and ordered' end + describe '#toggle_dpop' do + it "enables dpop" do + put :toggle_dpop, params: { user: { dpop_enabled: "1" } } + expect(access_token_user.dpop_enabled).to eq(true) + end + + it "disables dpop" do + put :toggle_dpop, params: { user: { dpop_enabled: "0" } } + expect(access_token_user.dpop_enabled).to eq(false) + end + + context "when feature flag is disabled" do + before do + stub_feature_flags(dpop_authentication: false) + end + + it "redirects to controller" do + put :toggle_dpop, params: { user: { dpop_enabled: "1" } } + + expect(response).to redirect_to(user_settings_personal_access_tokens_path) + expect(access_token_user.dpop_enabled).to eq(false) + end + end + end + describe '#index' do let!(:active_personal_access_token) { create(:personal_access_token, user: access_token_user) } diff --git a/spec/models/user_preference_spec.rb b/spec/models/user_preference_spec.rb index d90958aa95acc8..5f76010da319fe 100644 --- a/spec/models/user_preference_spec.rb +++ b/spec/models/user_preference_spec.rb @@ -221,6 +221,20 @@ end end + describe '#dpop_enabled' do + it 'is set to false by default' do + pref = described_class.new + + expect(pref.dpop_enabled).to eq(false) + end + + it 'returns assigned value' do + pref = described_class.new(dpop_enabled: true) + + expect(pref.dpop_enabled).to eq(true) + end + end + describe '#time_display_relative' do it 'is set to true by default' do pref = described_class.new diff --git a/spec/requests/api/ci/jobs_spec.rb b/spec/requests/api/ci/jobs_spec.rb index adf822e3cc5113..c51428e4023638 100644 --- a/spec/requests/api/ci/jobs_spec.rb +++ b/spec/requests/api/ci/jobs_spec.rb @@ -823,7 +823,7 @@ def go let(:job) { create(:ci_build, :canceled, pipeline: pipeline) } before do - allow(Gitlab::QueryLimiting::Transaction).to receive(:threshold).and_return(103) + allow(Gitlab::QueryLimiting::Transaction).to receive(:threshold).and_return(104) post api("/projects/#{project.id}/jobs/#{job.id}/retry", api_user) end diff --git a/spec/requests/api/graphql/projects/projects_spec.rb b/spec/requests/api/graphql/projects/projects_spec.rb index 9dcf4020ed063d..3916c0bee5c96c 100644 --- a/spec/requests/api/graphql/projects/projects_spec.rb +++ b/spec/requests/api/graphql/projects/projects_spec.rb @@ -50,7 +50,7 @@ # https://gitlab.com/gitlab-org/gitlab/-/issues/442164 expect do post_graphql(query, current_user: current_user) - end.not_to exceed_all_query_limit(control).with_threshold(17) + end.not_to exceed_all_query_limit(control).with_threshold(18) end it 'returns the expected projects' do diff --git a/spec/requests/api/project_import_spec.rb b/spec/requests/api/project_import_spec.rb index 9bc4be936955f4..a86e77319d4163 100644 --- a/spec/requests/api/project_import_spec.rb +++ b/spec/requests/api/project_import_spec.rb @@ -64,7 +64,7 @@ it 'executes a limited number of queries', :use_clean_rails_redis_caching do control = ActiveRecord::QueryRecorder.new { perform_archive_upload } - expect(control.count).to be <= 111 + expect(control.count).to be <= 113 end it 'schedules an import using a namespace' do diff --git a/spec/services/auth/dpop_authentication_service_spec.rb b/spec/services/auth/dpop_authentication_service_spec.rb new file mode 100644 index 00000000000000..5f2bffb52ac8ec --- /dev/null +++ b/spec/services/auth/dpop_authentication_service_spec.rb @@ -0,0 +1,431 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Auth::DpopAuthenticationService, feature_category: :system_access do + # Create the feed_token and static_object_token for the user + let_it_be(:user, freeze: true) { create(:user).tap(&:feed_token).tap(&:static_object_token) } + let(:keys) { user.keys.signing } + let_it_be(:personal_access_token, freeze: true) { create(:personal_access_token, user: user) } + + let(:ssh_public_key) do + "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQC1ZgRbeixURy9/HxU5r5O3Xobnw1bmQx3dyFMkRLMFCy" \ + "8aVkBvMw6CAc+81miOv+Sg/CZA2DKBAiEz0YwgPlD32o0q/OR5JdFAMH7e5IObm/4wr8dqm4JDE6eZ6f" \ + "eO+0tFwlrPnV8oiymw4SXeJLJf0n9f7HhH7xJJdWOOQZ2Ku/KMuNdf0aWYhbywUFWN4k5JtwCdEBxZYM" \ + "NqRYv28i76j3rTm7hBMyor7B2+3lPfeUQpTJkW1UBwUDAYeKZAl6HgZPE9DmcaDSRVViLErp00/iaQSs" \ + "MxlDSvhaOWARxFgFVNX7iV6S2MNaxah2CrPOyhBA2f2QSdQIQRB/NYjZivROMKaHmaflSB7VVUWyKWPR" \ + "QfSHbCgbnLpjYyoG7W+mRZ47i9+DeQLsBcIXjTJF/XfcV0iotFxPXiQk5h42+Oi+4YdGWerNl3JY15Dj" \ + "6QG1Q5UD8J026H4/P/2mPytFTb3iA9jmWEveF6nMC9RVNmqEep51/ZsL1zfenX3vr9Nl0= example@gitlab.com" + end + + let(:private_key) do + "-----BEGIN RSA PRIVATE KEY-----" \ + "\nMIIG5AIBAAKCAYEAtWYEW3osVEcvfx8VOa+Tt16G58NW5kMd3chTJESzBQsvGlZA" \ + "\nbzMOggHPvNZojr/koPwmQNgygQIhM9GMID5Q99qNKvzkeSXRQDB+3uSDm5v+MK/H" \ + "\napuCQxOnmen3jvtLRcJaz51fKIspsOEl3iSyX9J/X+x4R+8SSXVjjkGdirvyjLjX" \ + "\nX9GlmIW8sFBVjeJOSbcAnRAcWWDDakWL9vIu+o9605u4QTMqK+wdvt5T33lEKUyZ" \ + "\nFtVAcFAwGHimQJeh4GTxPQ5nGg0kVVYixK6dNP4mkErDMZQ0r4WjlgEcRYBVTV+4" \ + "\nlektjDWsWodgqzzsoQQNn9kEnUCEEQfzWI2Yr0TjCmh5mn5Uge1VVFsilj0UH0h2" \ + "\nwoG5y6Y2MqBu1vpkWeO4vfg3kC7AXCF40yRf133FdIqLRcT14kJOYeNvjovuGHRl" \ + "\nnqzZdyWNeQ4+kBtUOVA/CdNuh+Pz/9pj8rRU294gPY5lhL3hepzAvUVTZqhHqedf" \ + "\n2bC9c33p1976/TZdAgMBAAECggGBAKkx5o6Mfhx96Udg7qNHqTg36wzxnnRX1duv" \ + "\nph0GFxR1QhIGsUMHFFke52zzb8L2KYIerm99OF4sZlu28ESC23LTXyjhiRmWtH5y" \ + "\nvWOZMUhLT+SJkC9XrUBzbLibClVK/wKqLZnI56EhbFmXJ4L0J4xJApWuMuKlkyEB" \ + "\nZUKi4RcuByZKoli1awfAdibeR253zx3im6fkBw02vA67n7lOW5NJkP8fF9V4q7Uc" \ + "\nHwKQzRp8OZ9r2r75WYlowfORVUCaLMc0PN9AvduKOY7EAKULfyeFaPWFkY8UV6Ec" \ + "\n2JQAJ0MkJqxek5Hwi0+st9QGgUbg9z2yDEsOcJuYL1Se8DoagMqjmY66NNGYHe+D" \ + "\nwL6kw68JYxcQ5fQqyavCECPx8ayzVGbsCSMfaPDscSDO8gyZPnmbjDxyAiPpaPWk" \ + "\nXnInnKLAQHtVe9zCrgfpdXHqjvcnl/xxNcnKKzsASsdTFFJ2GOcGBatgIdQ0nVkg" \ + "\nuJbtvHwcK9qt1cux0dZnPNzuqNVBAQKBwQDsQy/JUIYdi8jxImUevetwRvB0tfZA" \ + "\nnHaIpHwUoX3fzwtVXD/14HbgB9LPG9aQDZ64Gr7HTvCY4tStSTIVoTiNOWc3JbjR" \ + "\nzN/FLykAseUCfZPI/EJolrgoVFq49C1O5zb2LwBhHWrA9RuQuvGCEHcTmyinm7pG" \ + "\n6CgqKrZ23FYnn4rj3pb1rv12pcPtYwIV9jksAaYOeGBFJ4GgDnL8XIdJYMdIkIeb" \ + "\niJAKLAOJEqYmqedVJcU49h6LFF0mXz6/eZUCgcEAxI18WnTjC66ZTTqcitohBVMi" \ + "\nbc4P1AeP08WspQSkpgnUvOzKIwyCzuwNEeBgmznD6iVFV86+tW2wAY3Ap2wcs9Vk" \ + "\nOqL+YYPtQDQSHfco7E9bNp7E/30w6t4WJfBg5X7AmCMUkVoyW2ol7aybGjgDWuB6" \ + "\nMQPczCgqfoLJk47tZu+5baZtqH7E36r7/Nf1rVgiksei4uX5O3wP+eNYTBoKwvlB" \ + "\n1XDLoidoXXO2tKhCd3j/x2XnLUcT3ha3H/H1P2epAoHAfCr3U1shkSek7K4B7P0t" \ + "\nXm258+ypxd01Iq0nlQQmjlhXAX6hEszsTONvtG9R/ZVa5DESMNdY9VDJK2U7kEiR" \ + "\n2w7fIwmNL533wL7/UqEr1XpAEDIbiLIliPSEVY3mvgAgT5P2JBP8xfpLiW3mfU+/" \ + "\n9SrnW+cpKBjc+wRFrwQvt1VO/mE+f1J/XTrTVNBjCT3FYE5hgltbZRzVMFRHtD/A" \ + "\nzhyxv35N9rz3zpDBLuoBLnK+5G4cT8px1PBX4FHQPXtdAoHANIkQvOjTKvMvHJpW" \ + "\n7zIgc1jmMe1LA8RFqDgEzlKwY4TrLNgpqzaT3BTx5V5Q1AyblgECSNcE2F+KFNA7" \ + "\nt0RJY7Pcx2N7lLr7dha05PeEI62OVsoXI6blpVFZICjg7VZ0yfVOcQ9nuFFl8+IX" \ + "\nzuk71FV9s44xvQvbV9dDY8JnKAVZTbqXQtsnahU8pzdd/kg5bXwYyIbpmAGwD325" \ + "\nwxWO3NBczV0JwLzBw4DDTARRR7e6viQ5pzuBTvJJXiuA/sKJAoHBAMu1EauYlaDZ" \ + "\nHFEzPUuII8nVuD+FrslJFmbArudzEJcq4byLRlLiBhELKTY2QNIYcmHKzD7rAkuW" \ + "\nLFEEU4FQ8fYJ9JDZjjOJmwmCzpBKRFRraIrcbmUWHvPPqcaG59dRi9BX0+ZVtjS3" \ + "\nH+ud5zpAjLvpRVVuxKtg38GPo9+F99iUHTqM1zBouhSpTqWQebR+Dln6HjMFqRwL" \ + "\ny1Y0tD9WVuVwFMEfkENQzOEJxVHwQpsxBRQ5snustS/HmrF5SIZyeg==" \ + "\n-----END RSA PRIVATE KEY-----" + end + + let(:pkey) { OpenSSL::PKey::RSA.new(private_key) } + let(:fingerprint) { create_fingerprint(keys.find_by(key: ssh_public_key).key) } + let(:public_key) do + { + kty: "RSA", + n: Base64.urlsafe_encode64(pkey.n.to_s(2), padding: false), + e: Base64.urlsafe_encode64(pkey.e.to_s(2), padding: false) + } + end + + let(:dpop_token) do + create_dpop_proof("RS512", "dpop+jwt", fingerprint, public_key, pkey, + ath: generate_ath(personal_access_token)) + end + + before do + user.keys.create!( + title: "Sample key #{user.id}", + key: ssh_public_key + ) + + user.user_preference.update!(dpop_enabled: true) + end + + def generate_ath(pat) + pat.token_digest + end + + def create_dpop_proof(alg, typ, kid, public_key, private_key, htu: '', htm: '', ath: nil, iat: Time.now.to_i, exp: Time.now.to_i + (4 * 3600)) # rubocop:disable Metrics/ParameterLists -- all params needed for edge cases + headers = create_headers(alg, typ, public_key, kid) + + jti = SecureRandom.uuid + + payload = create_payload( + htu: htu, htm: htm, ath: ath, iat: iat, jti: jti, exp: exp) + + JWT.encode(payload, private_key, alg, headers) + end + + def create_headers(alg, typ, public_key, kid) + if kid == "" + { + alg: alg, + typ: typ, + jwk: public_key + } + else + { + alg: alg, + typ: typ, + jwk: public_key, + kid: kid + } + end + end + + def create_payload(htu:, htm:, ath: nil, iat: Time.now.to_i, jti: SecureRandom.uuid, exp: Time.now.to_i + (4 * 3600)) + if exp == "" + { + htu: htu, + htm: htm, + ath: ath, + iat: iat, + jti: jti + } + else + { + htu: htu, + htm: htm, + ath: ath, + iat: iat, + jti: jti, + exp: exp + } + end + end + + def create_fingerprint(key) + Gitlab::SSHPublicKey.new(key).fingerprint_sha256 + end + + describe '#dpop_enabled?' do + it 'returns false' do + user.user_preference.update!(dpop_enabled: false) + + service = described_class.new(user, nil, nil) + expect(service.send(:dpop_enabled?)).to be_falsey + end + end + + describe '#find_key_from_dpop_proof?' do + it 'raises no error' do + user.user_preference.update!(dpop_enabled: false) + + service = described_class.new(user, dpop_token, personal_access_token) + expect(service.send(:find_key_from_dpop_proof)).to eq(ssh_public_key) + end + end + + describe '#get_header_from_token' do + it 'raises error for malformed JWT token' do + service = described_class.new(user, "malformed_token", personal_access_token) + expect do + service.send(:get_header_from_token) + end.to raise_error(Gitlab::Auth::DpopValidationError, + "DPoP validation error: Malformed JWT, unable to decode. Not enough or too many segments") + end + end + + describe '#validate_header!' do + it 'raises error when no kid in JWK' do + service = described_class.new(user, dpop_token, personal_access_token) + + expect do + service.send(:validate_header!, { "kid" => nil, "alg" => 'RS512' }) + end.to raise_error(Gitlab::Auth::DpopValidationError, + "DPoP validation error: No kid in JWT, unable to fetch key") + end + + # Remove this after support for ED25519 is added + it 'raises error when no kid in JWK' do + service = described_class.new(user, dpop_token, personal_access_token) + + expect do + service.send(:validate_header!, { "kid" => '123', "alg" => 'ED25519' }) + end.to raise_error(Gitlab::Auth::DpopValidationError, + "DPoP validation error: Currently only RSA keys are supported") + end + end + + describe 'validate_key_algo!' do + it 'raises error for unsupported algorithm' do + service = described_class.new(user, dpop_token, personal_access_token) + + expect do + service.send(:validate_key_algo!, "ssh-ed25519 publickey") + end.to raise_error(Gitlab::Auth::DpopValidationError, + "DPoP validation error: Currently only RSA keys are supported") + end + end + + describe 'validate_dpop_proof!' do + it 'raises error for None algorithm' do + service = described_class.new(user, dpop_token, personal_access_token) + allow(service).to receive(:get_dpop_algorithm).and_return(nil) + expect do + service.send(:validate_dpop_proof!, "ssh-rsa publickey") + end.to raise_error(Gitlab::Auth::DpopValidationError, + "DPoP validation error: None algorithm is not supported") + end + end + + describe '#public_key' do + it 'raises error for invalid public key' do + service = described_class.new(user, nil, nil) + + expect do + service.send(:public_key, "ssh-rsa publickey") + end.to raise_error(Gitlab::Auth::DpopValidationError, + "DPoP validation error: Unable to parse public key. bad data format") + end + end + + describe '#decode_token' do + it 'raises error for invalid dpop token' do + service = described_class.new(user, 'malformed_token', personal_access_token) + + parsed_public_key = service.send(:public_key, ssh_public_key) + + expect do + service.send(:decode_token, parsed_public_key, 'RS512') + end.to raise_error(Gitlab::Auth::DpopValidationError, + "DPoP validation error: Unable to decode JWT. Not enough or too many segments") + end + end + + describe '#get_dpop_algorithm' do + it 'returns parsed algorithm' do + service = described_class.new(user, nil, nil) + + expect(service.send(:get_dpop_algorithm, "ssh-rsa publickey")).to eq("RS512") + end + end + + describe '#validate_decoded_token!' do + it 'returns nil for no token' do + service = described_class.new(user, nil, nil) + + expect do + service.send(:validate_decoded_token!, nil) + end.to raise_error(Gitlab::Auth::DpopValidationError, "DPoP validation error: Unable to decode JWT") + end + end + + describe '#validate_ath!' do + it 'returns when ath matches access token' do + service = described_class.new(user, nil, personal_access_token) + + expect(service.send(:validate_ath!, [{ 'ath' => personal_access_token.token_digest }])).to be_nil + end + end + + describe '#validate_key_type!' do + it 'raises error for missing JWK' do + service = described_class.new(user, nil, nil) + + expect do + service.send(:validate_key_type!, nil, [nil, {}]) + end.to raise_error(Gitlab::Auth::DpopValidationError, "DPoP validation error: Failed to parse JWK") + end + end + + describe '#validate_dpop_header_count' do + it 'raises error for empty dpop header' do + service = described_class.new(user, nil, nil) + + expect do + service.send(:validate_dpop_header_count) + end.to raise_error(Gitlab::Auth::DpopValidationError, "DPoP validation error: No DPoP header in request") + end + end + + describe '#execute' do + context 'when dpop is disabled' do + it 'returns nil' do + user.user_preference.update!(dpop_enabled: false) + + service = described_class.new(user, dpop_token, personal_access_token) + allow(service).to receive(:dpop_enabled?).and_return(false) + expect(service.execute).to be_nil + end + end + + context 'when there is no pat' do + it 'returns nil' do + service = described_class.new(user, dpop_token, nil) + expect(service.execute).to be_nil + end + end + + context 'when there is no dpop token' do + it 'returns nil' do + service = described_class.new(user, nil, personal_access_token) + expect do + service.execute + end.to raise_error(Gitlab::Auth::DpopValidationError, "DPoP validation error: No DPoP header in request") + end + end + + context 'with invalid dpop header' do + it 'raises error for more than 1 dpop header' do + expect do + described_class.new(user, "#{dpop_token},#{dpop_token}", + personal_access_token).execute + end.to raise_error(Gitlab::Auth::DpopValidationError, + "DPoP validation error: Only 1 DPoP header is allowed in request") + end + + it 'raises error for empty dpop header' do + expect do + described_class.new(user, nil, + personal_access_token).execute + end.to raise_error(Gitlab::Auth::DpopValidationError, + "DPoP validation error: No DPoP header in request") + end + + it 'raises error when no matching key found' do + fingerprint = "SHA256:DiKyMUXG3A4RmLVsgMDQEt0XrxFgYOdvqo/Penwl2Ug" + dpop_token = create_dpop_proof("RS512", "dpop+jwt", fingerprint, public_key, pkey, + ath: generate_ath(personal_access_token), exp: Time.now.to_i - (4 * 3600)) + + expect do + described_class.new(user, dpop_token, + personal_access_token).execute + end.to raise_error(Gitlab::Auth::DpopValidationError, "DPoP validation error: No matching key found") + end + + it 'raises error for invalid typ' do + dpop_token = create_dpop_proof("RS512", "dpop+jwt+invalid", fingerprint, public_key, pkey, + ath: generate_ath(personal_access_token)) + + expect do + described_class.new(user, dpop_token, + personal_access_token).execute + end.to raise_error(Gitlab::Auth::DpopValidationError, "DPoP validation error: Invalid typ value in JWT") + end + + it 'raises error for invalid jwt' do + expect do + described_class.new(user, "malformed_token", + personal_access_token).execute + end.to raise_error(Gitlab::Auth::DpopValidationError) + end + + it 'raises error for invalid ath' do + dpop_token = create_dpop_proof("RS512", "dpop+jwt", fingerprint, public_key, pkey, + ath: 'invalid_access_token_hash') + + expect do + described_class.new(user, dpop_token, + personal_access_token).execute + end.to raise_error(Gitlab::Auth::DpopValidationError, + "DPoP validation error: Incorrect access token hash in JWT") + end + + it 'raises error for invalid jwk' do + public_key = { + kty: "RSA", + n: "invalid", + e: Base64.urlsafe_encode64(pkey.e.to_s(2), padding: false) + } + + dpop_token = create_dpop_proof("RS512", "dpop+jwt", fingerprint, public_key, pkey, + ath: generate_ath(personal_access_token)) + + expect do + described_class.new(user, dpop_token, + personal_access_token).execute + end.to raise_error(Gitlab::Auth::DpopValidationError, "DPoP validation error: Failed to parse JWK") + end + + it 'raises error for expired token' do + dpop_token = create_dpop_proof("RS512", "dpop+jwt", fingerprint, public_key, pkey, + ath: generate_ath(personal_access_token), exp: Time.now.to_i - (4 * 3600)) + + expect do + described_class.new(user, dpop_token, + personal_access_token).execute + end.to raise_error(Gitlab::Auth::DpopValidationError, "DPoP validation error: Signature expired") + end + + it 'raises error for missing claims' do + dpop_token = create_dpop_proof("RS512", "dpop+jwt", fingerprint, public_key, pkey, + ath: generate_ath(personal_access_token), exp: "") + + expect do + described_class.new(user, dpop_token, + personal_access_token).execute + end.to raise_error(Gitlab::Auth::DpopValidationError, "DPoP validation error: Missing required claim exp") + end + + it 'raises error for no kid' do + fingerprint = "" + + dpop_token = create_dpop_proof("RS512", "dpop+jwt", fingerprint, public_key, pkey, + ath: generate_ath(personal_access_token), exp: Time.now.to_i - (4 * 3600)) + + expect do + described_class.new(user, dpop_token, + personal_access_token).execute + end.to raise_error(Gitlab::Auth::DpopValidationError, + "DPoP validation error: No kid in JWT, unable to fetch key") + end + + it 'raises error for malformed kid' do + fingerprint = "SHA256:DiKyMUXG3A4RmLVsgMDQEt0XrxFgYOdvqo/Penwl2Ug:ABCD" + + dpop_token = create_dpop_proof("RS512", "dpop+jwt", fingerprint, public_key, pkey, + ath: generate_ath(personal_access_token), exp: Time.now.to_i - (4 * 3600)) + + expect do + described_class.new(user, dpop_token, + personal_access_token).execute + end.to raise_error(Gitlab::Auth::DpopValidationError, + "DPoP validation error: Malformed fingerprint value in kid") + end + end + end +end diff --git a/spec/views/user_settings/personal_access_tokens/index.html.haml_spec.rb b/spec/views/user_settings/personal_access_tokens/index.html.haml_spec.rb new file mode 100644 index 00000000000000..461a359796b315 --- /dev/null +++ b/spec/views/user_settings/personal_access_tokens/index.html.haml_spec.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'user_settings/personal_access_tokens/index.html.haml', feature_category: :system_access do + # rubocop:disable RSpec/FactoryBot/AvoidCreate -- we need these objects to be persisted + let(:user) { create(:user) } + let(:personal_access_token) { create(:personal_access_token, user: user) } + # rubocop:enable RSpec/FactoryBot/AvoidCreate + + before do + assign(:user, user) + sign_in(user) + allow(view).to receive(:current_user).and_return(user) + + assign(:active_access_tokens, ::PersonalAccessTokenSerializer.new.represent([personal_access_token])) + assign(:personal_access_token, personal_access_token) + assign(:scopes, [:api, :read_api]) + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(dpop_authentication: false) + end + + it 'does not show dpop options' do + render + + expect(rendered).not_to have_selector('div', id: 'dpop-form') + end + end + + context 'when feature flag is enabled' do + before do + stub_feature_flags(dpop_authentication: true) + end + + it 'shows ticked checkbox for dpop' do + user.user_preference.update!(dpop_enabled: true) + render + + expect(rendered).to have_checked_field('user[dpop_enabled]', class: 'custom-control-input') + end + + it 'shows unticked checkbox for dpop' do + user.user_preference.update!(dpop_enabled: false) + render + + expect(rendered).not_to have_checked_field('user[dpop_enabled]', class: 'custom-control-input') + end + end +end -- GitLab From 4389d8550b37f9754b469fba15a621b75bb0930f Mon Sep 17 00:00:00 2001 From: Ameya Darshan Date: Tue, 4 Jun 2024 20:49:50 +0530 Subject: [PATCH 2/3] Replace pat retrieval method --- app/controllers/graphql_controller.rb | 27 +++---------------- .../auth/dpop_authentication_service.rb | 2 +- lib/gitlab/auth/auth_finders.rb | 23 ++++++++++++++-- 3 files changed, 25 insertions(+), 27 deletions(-) diff --git a/app/controllers/graphql_controller.rb b/app/controllers/graphql_controller.rb index c2ea3b504ea827..f0dbb2650ae69f 100644 --- a/app/controllers/graphql_controller.rb +++ b/app/controllers/graphql_controller.rb @@ -31,8 +31,6 @@ class GraphqlController < ApplicationController # around in GraphiQL. protect_from_forgery with: :null_session, only: :execute - before_action(only: [:execute]) { check_dpop } - # must come first: current_user is set up here before_action(only: [:execute]) do if Feature.enabled? :graphql_minimal_auth_methods @@ -43,6 +41,7 @@ class GraphqlController < ApplicationController end before_action :authorize_access_api! + before_action(only: [:execute]) { check_dpop } before_action :set_user_last_activity before_action :track_vs_code_usage before_action :track_jetbrains_usage @@ -152,28 +151,8 @@ def authorization_scopes end def check_dpop - ::Auth::DpopAuthenticationService.new(current_user, get_dpop_token_from_request, - get_personal_access_token).execute - end - - def get_dpop_token_from_request - request.headers['dpop'].presence - end - - def get_personal_access_token - token = - request.params[PRIVATE_TOKEN_PARAM].presence || - request.env[PRIVATE_TOKEN_HEADER].presence || - parsed_oauth_token - return unless token - - # || raise(UnauthorizedError) is not needed as authenticate_sessionless_user!(:api) - # will raise the error - PersonalAccessToken.find_by_token(token) - end - - def parsed_oauth_token - Doorkeeper::OAuth::Token.from_request(request, *Doorkeeper.configuration.access_token_methods) + ::Auth::DpopAuthenticationService.new(current_user, request_authenticator.get_dpop_token_from_request, + request_authenticator.get_personal_access_token).execute end def permitted_multiplex_params diff --git a/app/services/auth/dpop_authentication_service.rb b/app/services/auth/dpop_authentication_service.rb index 41652648bd8b1c..82f0107a4bff6d 100644 --- a/app/services/auth/dpop_authentication_service.rb +++ b/app/services/auth/dpop_authentication_service.rb @@ -129,7 +129,7 @@ def validate_decoded_token!(decoded_token) end def validate_ath!(decoded_token) - return if decoded_token[0]["ath"] == @pat.token_digest + return if decoded_token[0]["ath"] == Base64.urlsafe_encode64(Digest::SHA256.digest(@pat), padding: false) raise Gitlab::Auth::DpopValidationError, "Incorrect access token hash in JWT" end diff --git a/lib/gitlab/auth/auth_finders.rb b/lib/gitlab/auth/auth_finders.rb index 41e7ad43e66c5e..9492798c1b47a5 100644 --- a/lib/gitlab/auth/auth_finders.rb +++ b/lib/gitlab/auth/auth_finders.rb @@ -208,6 +208,17 @@ def authentication_token_present? parsed_oauth_token.present? end + def get_personal_access_token + token = + current_request.params[PRIVATE_TOKEN_PARAM].presence || + current_request.env[PRIVATE_TOKEN_HEADER].presence || + parsed_oauth_token + + return unless token + + token + end + def get_dpop_token_from_request current_request.headers['dpop'].presence end @@ -264,8 +275,12 @@ def find_personal_access_token # Expiration, revocation and scopes are verified in `validate_access_token!` personal_access_token = PersonalAccessToken.find_by_token(token.to_s) || raise(UnauthorizedError) - ::Auth::DpopAuthenticationService.new(personal_access_token.user, get_dpop_token_from_request, - personal_access_token).execute + # authenticate_graphql in GraphqlController already calls find_personal_access_token + # via authenticate_sessionless_user! + unless graphql_request? + ::Auth::DpopAuthenticationService.new(personal_access_token.user, get_dpop_token_from_request, + get_personal_access_token).execute + end personal_access_token end @@ -421,6 +436,10 @@ def api_request? current_request.path.starts_with?(Gitlab::Utils.append_path(Gitlab.config.gitlab.relative_url_root, '/api/')) end + def graphql_request? + current_request.path == Gitlab::Routing.url_helpers.api_graphql_path + end + def git_request? Gitlab::PathRegex.repository_git_route_regex.match?(current_request.path) end -- GitLab From 11bf173df874a5369d62052941c4723e2a2411db Mon Sep 17 00:00:00 2001 From: Ameya Darshan Date: Fri, 5 Jul 2024 13:54:45 +0530 Subject: [PATCH 3/3] Fix specs --- .../auth/dpop_authentication_service.rb | 2 - .../auth/dpop_authentication_service_spec.rb | 48 ++++++++++--------- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/app/services/auth/dpop_authentication_service.rb b/app/services/auth/dpop_authentication_service.rb index 82f0107a4bff6d..874f74de3f87ef 100644 --- a/app/services/auth/dpop_authentication_service.rb +++ b/app/services/auth/dpop_authentication_service.rb @@ -7,8 +7,6 @@ class DpopAuthenticationService < BaseService include Gitlab::Utils::StrongMemoize SUPPORTED_ALGOS = { - 'ssh-ed25519' => 'ED25519', - 'ecdsa-sha2-nistp256' => 'ES256', 'ssh-rsa' => 'RS512' }.freeze diff --git a/spec/services/auth/dpop_authentication_service_spec.rb b/spec/services/auth/dpop_authentication_service_spec.rb index 5f2bffb52ac8ec..6ca6baae29ac94 100644 --- a/spec/services/auth/dpop_authentication_service_spec.rb +++ b/spec/services/auth/dpop_authentication_service_spec.rb @@ -85,7 +85,7 @@ end def generate_ath(pat) - pat.token_digest + Base64.urlsafe_encode64(Digest::SHA256.digest(pat.token), padding: false) end def create_dpop_proof(alg, typ, kid, public_key, private_key, htu: '', htm: '', ath: nil, iat: Time.now.to_i, exp: Time.now.to_i + (4 * 3600)) # rubocop:disable Metrics/ParameterLists -- all params needed for edge cases @@ -154,14 +154,14 @@ def create_fingerprint(key) it 'raises no error' do user.user_preference.update!(dpop_enabled: false) - service = described_class.new(user, dpop_token, personal_access_token) + service = described_class.new(user, dpop_token, personal_access_token.token) expect(service.send(:find_key_from_dpop_proof)).to eq(ssh_public_key) end end describe '#get_header_from_token' do it 'raises error for malformed JWT token' do - service = described_class.new(user, "malformed_token", personal_access_token) + service = described_class.new(user, "malformed_token", personal_access_token.token) expect do service.send(:get_header_from_token) end.to raise_error(Gitlab::Auth::DpopValidationError, @@ -171,7 +171,7 @@ def create_fingerprint(key) describe '#validate_header!' do it 'raises error when no kid in JWK' do - service = described_class.new(user, dpop_token, personal_access_token) + service = described_class.new(user, dpop_token, personal_access_token.token) expect do service.send(:validate_header!, { "kid" => nil, "alg" => 'RS512' }) @@ -181,7 +181,7 @@ def create_fingerprint(key) # Remove this after support for ED25519 is added it 'raises error when no kid in JWK' do - service = described_class.new(user, dpop_token, personal_access_token) + service = described_class.new(user, dpop_token, personal_access_token.token) expect do service.send(:validate_header!, { "kid" => '123', "alg" => 'ED25519' }) @@ -192,7 +192,7 @@ def create_fingerprint(key) describe 'validate_key_algo!' do it 'raises error for unsupported algorithm' do - service = described_class.new(user, dpop_token, personal_access_token) + service = described_class.new(user, dpop_token, personal_access_token.token) expect do service.send(:validate_key_algo!, "ssh-ed25519 publickey") @@ -203,7 +203,7 @@ def create_fingerprint(key) describe 'validate_dpop_proof!' do it 'raises error for None algorithm' do - service = described_class.new(user, dpop_token, personal_access_token) + service = described_class.new(user, dpop_token, personal_access_token.token) allow(service).to receive(:get_dpop_algorithm).and_return(nil) expect do service.send(:validate_dpop_proof!, "ssh-rsa publickey") @@ -225,7 +225,7 @@ def create_fingerprint(key) describe '#decode_token' do it 'raises error for invalid dpop token' do - service = described_class.new(user, 'malformed_token', personal_access_token) + service = described_class.new(user, 'malformed_token', personal_access_token.token) parsed_public_key = service.send(:public_key, ssh_public_key) @@ -256,9 +256,11 @@ def create_fingerprint(key) describe '#validate_ath!' do it 'returns when ath matches access token' do - service = described_class.new(user, nil, personal_access_token) + service = described_class.new(user, nil, personal_access_token.token) - expect(service.send(:validate_ath!, [{ 'ath' => personal_access_token.token_digest }])).to be_nil + expect(service.send(:validate_ath!, + [{ 'ath' => Base64.urlsafe_encode64(Digest::SHA256.digest(personal_access_token.token), + padding: false) }])).to be_nil end end @@ -287,7 +289,7 @@ def create_fingerprint(key) it 'returns nil' do user.user_preference.update!(dpop_enabled: false) - service = described_class.new(user, dpop_token, personal_access_token) + service = described_class.new(user, dpop_token, personal_access_token.token) allow(service).to receive(:dpop_enabled?).and_return(false) expect(service.execute).to be_nil end @@ -302,7 +304,7 @@ def create_fingerprint(key) context 'when there is no dpop token' do it 'returns nil' do - service = described_class.new(user, nil, personal_access_token) + service = described_class.new(user, nil, personal_access_token.token) expect do service.execute end.to raise_error(Gitlab::Auth::DpopValidationError, "DPoP validation error: No DPoP header in request") @@ -313,7 +315,7 @@ def create_fingerprint(key) it 'raises error for more than 1 dpop header' do expect do described_class.new(user, "#{dpop_token},#{dpop_token}", - personal_access_token).execute + personal_access_token.token).execute end.to raise_error(Gitlab::Auth::DpopValidationError, "DPoP validation error: Only 1 DPoP header is allowed in request") end @@ -321,7 +323,7 @@ def create_fingerprint(key) it 'raises error for empty dpop header' do expect do described_class.new(user, nil, - personal_access_token).execute + personal_access_token.token).execute end.to raise_error(Gitlab::Auth::DpopValidationError, "DPoP validation error: No DPoP header in request") end @@ -333,7 +335,7 @@ def create_fingerprint(key) expect do described_class.new(user, dpop_token, - personal_access_token).execute + personal_access_token.token).execute end.to raise_error(Gitlab::Auth::DpopValidationError, "DPoP validation error: No matching key found") end @@ -343,14 +345,14 @@ def create_fingerprint(key) expect do described_class.new(user, dpop_token, - personal_access_token).execute + personal_access_token.token).execute end.to raise_error(Gitlab::Auth::DpopValidationError, "DPoP validation error: Invalid typ value in JWT") end it 'raises error for invalid jwt' do expect do described_class.new(user, "malformed_token", - personal_access_token).execute + personal_access_token.token).execute end.to raise_error(Gitlab::Auth::DpopValidationError) end @@ -360,7 +362,7 @@ def create_fingerprint(key) expect do described_class.new(user, dpop_token, - personal_access_token).execute + personal_access_token.token).execute end.to raise_error(Gitlab::Auth::DpopValidationError, "DPoP validation error: Incorrect access token hash in JWT") end @@ -377,7 +379,7 @@ def create_fingerprint(key) expect do described_class.new(user, dpop_token, - personal_access_token).execute + personal_access_token.token).execute end.to raise_error(Gitlab::Auth::DpopValidationError, "DPoP validation error: Failed to parse JWK") end @@ -387,7 +389,7 @@ def create_fingerprint(key) expect do described_class.new(user, dpop_token, - personal_access_token).execute + personal_access_token.token).execute end.to raise_error(Gitlab::Auth::DpopValidationError, "DPoP validation error: Signature expired") end @@ -397,7 +399,7 @@ def create_fingerprint(key) expect do described_class.new(user, dpop_token, - personal_access_token).execute + personal_access_token.token).execute end.to raise_error(Gitlab::Auth::DpopValidationError, "DPoP validation error: Missing required claim exp") end @@ -409,7 +411,7 @@ def create_fingerprint(key) expect do described_class.new(user, dpop_token, - personal_access_token).execute + personal_access_token.token).execute end.to raise_error(Gitlab::Auth::DpopValidationError, "DPoP validation error: No kid in JWT, unable to fetch key") end @@ -422,7 +424,7 @@ def create_fingerprint(key) expect do described_class.new(user, dpop_token, - personal_access_token).execute + personal_access_token.token).execute end.to raise_error(Gitlab::Auth::DpopValidationError, "DPoP validation error: Malformed fingerprint value in kid") end -- GitLab