diff --git a/app/models/awareness_session.rb b/app/models/awareness_session.rb index faadb7e0ba9..602888182b1 100644 --- a/app/models/awareness_session.rb +++ b/app/models/awareness_session.rb @@ -148,8 +148,13 @@ class AwarenessSession # rubocop:disable Gitlab/NamespacedClass end def users_with_last_activity - user_ids, last_activities = user_ids_with_last_activity.transpose - users = User.where(id: user_ids) + # where in (x, y, [...z]) is a set and does not maintain any order, we need to + # make sure to establish a stable order for both, the pairs returned from + # redis and the ActiveRecord query. Using IDs in ascending order. + user_ids, last_activities = user_ids_with_last_activity + .sort_by(&:first) + .transpose + users = User.where(id: user_ids).order(id: :asc) users.zip(last_activities) end diff --git a/config/feature_flags/development/batch_load_environment_last_deployment_group.yml b/config/feature_flags/development/batch_load_environment_last_deployment_group.yml index d6de45eacdf..4d35b638fbc 100644 --- a/config/feature_flags/development/batch_load_environment_last_deployment_group.yml +++ b/config/feature_flags/development/batch_load_environment_last_deployment_group.yml @@ -5,4 +5,4 @@ rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/363023 milestone: '15.1' type: development group: group::release -default_enabled: false +default_enabled: true diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index 7827ad8f168..6ad8b02bfea 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -39,7 +39,7 @@ Doorkeeper.configure do # Reuse access token for the same resource owner within an application (disabled by default) # Rationale: https://github.com/doorkeeper-gem/doorkeeper/issues/383 - reuse_access_token + # reuse_access_token # Issue access tokens with refresh token (disabled by default) use_refresh_token diff --git a/doc/ci/pipelines/merged_results_pipelines.md b/doc/ci/pipelines/merged_results_pipelines.md index 9661d2f5263..777871a7c5f 100644 --- a/doc/ci/pipelines/merged_results_pipelines.md +++ b/doc/ci/pipelines/merged_results_pipelines.md @@ -6,7 +6,8 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Merged results pipelines **(PREMIUM)** -> [Renamed](https://gitlab.com/gitlab-org/gitlab/-/issues/351192) from `pipelines for merged results` to `merged results pipelines` in GitLab 14.8. +> - [Renamed](https://gitlab.com/gitlab-org/gitlab/-/issues/351192) from `pipelines for merged results` to `merged results pipelines` in GitLab 14.8. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/91849) in GitLab 15.1, merged results pipelines also run on [Draft merge requests](../../user/project/merge_requests/drafts.md). A *merged results pipeline* is a type of [merge request pipeline](merge_request_pipelines.md). It is a pipeline that runs against the results of the source and target branches merged together. @@ -19,10 +20,7 @@ The pipeline runs against the target branch as it exists at the moment you run t Over time, while you're working in the source branch, the target branch might change. Any time you want to be sure the merged results are accurate, you should re-run the pipeline. -Merged results pipelines can't run when: - -- The target branch has changes that conflict with the changes in the source branch. -- The merge request is a [**Draft** merge request](../../user/project/merge_requests/drafts.md). +Merged results pipelines can't run when the target branch has changes that conflict with the changes in the source branch. In these cases, the pipeline runs as a [merge request pipeline](merge_request_pipelines.md) and [is labeled as `merge request`](merge_request_pipelines.md#types-of-merge-request-pipelines). diff --git a/doc/user/group/saml_sso/scim_setup.md b/doc/user/group/saml_sso/scim_setup.md index 6bf7d792114..04aa99e08af 100644 --- a/doc/user/group/saml_sso/scim_setup.md +++ b/doc/user/group/saml_sso/scim_setup.md @@ -261,6 +261,18 @@ Changing the SAML or SCIM configuration or provider can cause the following prob | SAML and SCIM identity mismatch. | First [verify that the user's SAML NameId matches the SCIM externalId](#how-do-i-verify-users-saml-nameid-matches-the-scim-externalid) and then [update or fix the mismatched SCIM externalId and SAML NameId](#update-or-fix-mismatched-scim-externalid-and-saml-nameid). | | SCIM identity mismatch between GitLab and the identity provider SCIM app. | You can confirm whether you're hitting the error because of your SCIM identity mismatch between your SCIM app and GitLab.com by using [SCIM API](../../../api/scim.md#update-a-single-scim-provisioned-user) which shows up in the `id` key and compares it with the user `externalId` in the SCIM app. You can use the same [SCIM API](../../../api/scim.md#update-a-single-scim-provisioned-user) to update the SCIM `id` for the user on GitLab.com. | +### Search Rails logs for SCIM requests + +GitLab.com administrators can search for SCIM requests in the `api_json.log` using the `pubsub-rails-inf-gprd-*` index in [Kibana](https://about.gitlab.com/handbook/support/workflows/kibana.html#using-kibana). Use the following filters based on the [SCIM API](../../../api/scim.md): + +- `json.path`: `/scim/v2/groups/` +- `json.params.value`: `` + +In a relevant log entry, the `json.params.value` shows the values of SCIM parameters GitLab receives. These values can be used to verify if SCIM parameters configured in an +identity provider's SCIM app are communicated to GitLab as intended. For example, we can use these values as a definitive source on why an account was provisioned with a certain +set of details. This information can help where an account was SCIM provisioned with details that appear to be incongruent with what might have been configured within an identity +provider's SCIM app. + ### Azure #### How do I verify my SCIM configuration is correct? diff --git a/spec/factories/oauth_access_tokens.rb b/spec/factories/oauth_access_tokens.rb index 8d1075dacbb..8fd8aef9b49 100644 --- a/spec/factories/oauth_access_tokens.rb +++ b/spec/factories/oauth_access_tokens.rb @@ -5,6 +5,7 @@ FactoryBot.define do resource_owner application token { Doorkeeper::OAuth::Helpers::UniqueToken.generate } + refresh_token { Doorkeeper::OAuth::Helpers::UniqueToken.generate } scopes { application.scopes } end end diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index e09016b2b2b..3ccc3a17862 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -2477,6 +2477,9 @@ RSpec.describe Gitlab::Database::MigrationHelpers do describe '#backfill_iids' do include MigrationsHelpers + let_it_be(:issue_base_type_enum) { 0 } + let_it_be(:issue_type) { table(:work_item_types).find_by(base_type: issue_base_type_enum) } + let(:issue_class) do Class.new(ActiveRecord::Base) do include AtomicInternalId @@ -2490,6 +2493,8 @@ RSpec.describe Gitlab::Database::MigrationHelpers do scope: :project, init: ->(s, _scope) { s&.project&.issues&.maximum(:iid) }, presence: false + + before_validation -> { self.work_item_type_id = ::WorkItems::Type.default_issue_type.id } end end @@ -2515,7 +2520,7 @@ RSpec.describe Gitlab::Database::MigrationHelpers do it 'generates iids properly for models created after the migration when iids are backfilled' do project = setup - issue_a = issues.create!(project_id: project.id) + issue_a = issues.create!(project_id: project.id, work_item_type_id: issue_type.id) model.backfill_iids('issues') @@ -2528,14 +2533,14 @@ RSpec.describe Gitlab::Database::MigrationHelpers do it 'generates iids properly for models created after the migration across multiple projects' do project_a = setup project_b = setup - issues.create!(project_id: project_a.id) - issues.create!(project_id: project_b.id) - issues.create!(project_id: project_b.id) + issues.create!(project_id: project_a.id, work_item_type_id: issue_type.id) + issues.create!(project_id: project_b.id, work_item_type_id: issue_type.id) + issues.create!(project_id: project_b.id, work_item_type_id: issue_type.id) model.backfill_iids('issues') - issue_a = issue_class.create!(project_id: project_a.id) - issue_b = issue_class.create!(project_id: project_b.id) + issue_a = issue_class.create!(project_id: project_a.id, work_item_type_id: issue_type.id) + issue_b = issue_class.create!(project_id: project_b.id, work_item_type_id: issue_type.id) expect(issue_a.iid).to eq(2) expect(issue_b.iid).to eq(3) @@ -2545,7 +2550,7 @@ RSpec.describe Gitlab::Database::MigrationHelpers do it 'generates an iid' do project_a = setup project_b = setup - issue_a = issues.create!(project_id: project_a.id) + issue_a = issues.create!(project_id: project_a.id, work_item_type_id: issue_type.id) model.backfill_iids('issues') @@ -2559,8 +2564,8 @@ RSpec.describe Gitlab::Database::MigrationHelpers do context 'when a row already has an iid set in the database' do it 'backfills iids' do project = setup - issue_a = issues.create!(project_id: project.id, iid: 1) - issue_b = issues.create!(project_id: project.id, iid: 2) + issue_a = issues.create!(project_id: project.id, work_item_type_id: issue_type.id, iid: 1) + issue_b = issues.create!(project_id: project.id, work_item_type_id: issue_type.id, iid: 2) model.backfill_iids('issues') @@ -2571,9 +2576,9 @@ RSpec.describe Gitlab::Database::MigrationHelpers do it 'backfills for multiple projects' do project_a = setup project_b = setup - issue_a = issues.create!(project_id: project_a.id, iid: 1) - issue_b = issues.create!(project_id: project_b.id, iid: 1) - issue_c = issues.create!(project_id: project_a.id, iid: 2) + issue_a = issues.create!(project_id: project_a.id, work_item_type_id: issue_type.id, iid: 1) + issue_b = issues.create!(project_id: project_b.id, work_item_type_id: issue_type.id, iid: 1) + issue_c = issues.create!(project_id: project_a.id, work_item_type_id: issue_type.id, iid: 2) model.backfill_iids('issues') diff --git a/spec/lib/gitlab/markdown_cache/active_record/extension_spec.rb b/spec/lib/gitlab/markdown_cache/active_record/extension_spec.rb index d22bef5bda9..81910773dfa 100644 --- a/spec/lib/gitlab/markdown_cache/active_record/extension_spec.rb +++ b/spec/lib/gitlab/markdown_cache/active_record/extension_spec.rb @@ -11,6 +11,8 @@ RSpec.describe Gitlab::MarkdownCache::ActiveRecord::Extension do attribute :author attribute :project + + before_validation -> { self.work_item_type_id = ::WorkItems::Type.default_issue_type.id } end end diff --git a/spec/models/awareness_session_spec.rb b/spec/models/awareness_session_spec.rb index f1369514610..4dace7cb8de 100644 --- a/spec/models/awareness_session_spec.rb +++ b/spec/models/awareness_session_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe AwarenessSession do subject { AwarenessSession.for(session_id) } - let(:user) { create(:user) } + let!(:user) { create(:user) } let(:session_id) { 1 } after do @@ -13,6 +13,8 @@ RSpec.describe AwarenessSession do end describe "when a user joins a session" do + let(:user2) { create(:user) } + let(:presence_ttl) { 15.minutes } it "changes number of session members" do @@ -31,6 +33,26 @@ RSpec.describe AwarenessSession do end end + it "maintains user ID and last_activity pairs" do + now = Time.zone.now + + travel_to now - 1.minute do + subject.join(user2) + end + + travel_to now do + subject.join(user) + end + + session_users = subject.users_with_last_activity + + expect(session_users[0].first.id).to eql(user.id) + expect(session_users[0].last.to_i).to eql(now.to_i) + + expect(session_users[1].first.id).to eql(user2.id) + expect(session_users[1].last.to_i).to eql((now - 1.minute).to_i) + end + it "reports user as present" do freeze_time do subject.join(user) diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb index a00129b3fdf..19b9a1519eb 100644 --- a/spec/models/concerns/cache_markdown_field_spec.rb +++ b/spec/models/concerns/cache_markdown_field_spec.rb @@ -9,6 +9,8 @@ RSpec.describe CacheMarkdownField, :clean_gitlab_redis_cache do include CacheMarkdownField cache_markdown_field :title, pipeline: :single_line cache_markdown_field :description + + before_validation -> { self.work_item_type_id = ::WorkItems::Type.default_issue_type.id } end end diff --git a/spec/models/concerns/pg_full_text_searchable_spec.rb b/spec/models/concerns/pg_full_text_searchable_spec.rb index 84209999ab2..55e3caf3c4c 100644 --- a/spec/models/concerns/pg_full_text_searchable_spec.rb +++ b/spec/models/concerns/pg_full_text_searchable_spec.rb @@ -14,6 +14,8 @@ RSpec.describe PgFullTextSearchable do belongs_to :project has_one :search_data, class_name: 'Issues::SearchData' + before_validation -> { self.work_item_type_id = ::WorkItems::Type.default_issue_type.id } + def persist_pg_full_text_search_vector(search_vector) Issues::SearchData.upsert({ project_id: project_id, issue_id: id, search_vector: search_vector }, unique_by: %i(project_id issue_id)) end @@ -185,6 +187,8 @@ RSpec.describe PgFullTextSearchable do belongs_to :project has_one :search_data, class_name: 'Issues::SearchData' + before_validation -> { self.work_item_type_id = ::WorkItems::Type.default_issue_type.id } + def self.name 'Issue' end diff --git a/spec/requests/jira_authorizations_spec.rb b/spec/requests/jira_authorizations_spec.rb index b43d36e94f4..f67288b286b 100644 --- a/spec/requests/jira_authorizations_spec.rb +++ b/spec/requests/jira_authorizations_spec.rb @@ -27,14 +27,16 @@ RSpec.describe 'Jira authorization requests' do redirect_uri: redirect_uri }) oauth_response = json_response + oauth_response_access_token, scope, token_type = oauth_response.values_at('access_token', 'scope', 'token_type') post '/login/oauth/access_token', params: post_data.merge({ code: generate_access_grant.token }) jira_response = response.body + jira_response_access_token = Rack::Utils.parse_nested_query(jira_response)['access_token'] - access_token, scope, token_type = oauth_response.values_at('access_token', 'scope', 'token_type') - expect(jira_response).to eq("access_token=#{access_token}&scope=#{scope}&token_type=#{token_type}") + expect(jira_response).to include("scope=#{scope}&token_type=#{token_type}") + expect(oauth_response_access_token).not_to eql(jira_response_access_token) end context 'when authorization fails' do diff --git a/spec/requests/oauth_tokens_spec.rb b/spec/requests/oauth_tokens_spec.rb index 30659a5b896..180341fc85d 100644 --- a/spec/requests/oauth_tokens_spec.rb +++ b/spec/requests/oauth_tokens_spec.rb @@ -5,44 +5,92 @@ require 'spec_helper' RSpec.describe 'OAuth Tokens requests' do let(:user) { create :user } let(:application) { create :oauth_application, scopes: 'api' } + let(:grant_type) { 'authorization_code' } + let(:refresh_token) { nil } def request_access_token(user) post '/oauth/token', params: { - grant_type: 'authorization_code', + grant_type: grant_type, code: generate_access_grant(user).token, redirect_uri: application.redirect_uri, client_id: application.uid, - client_secret: application.secret + client_secret: application.secret, + refresh_token: refresh_token + } end def generate_access_grant(user) - create :oauth_access_grant, application: application, resource_owner_id: user.id + create(:oauth_access_grant, application: application, resource_owner_id: user.id) end context 'when there is already a token for the application' do - let!(:existing_token) { create :oauth_access_token, application: application, resource_owner_id: user.id } + let!(:existing_token) { create(:oauth_access_token, application: application, resource_owner_id: user.id) } - context 'and the request is done by the resource owner' do - it 'reuses and returns the stored token' do + shared_examples 'issues a new token' do + it 'issues a new token' do expect do request_access_token(user) - end.not_to change { Doorkeeper::AccessToken.count } + end.to change { Doorkeeper::AccessToken.count }.from(1).to(2) - expect(json_response['access_token']).to eq existing_token.token + expect(json_response['access_token']).not_to eq existing_token.token + expect(json_response['refresh_token']).not_to eq existing_token.refresh_token end end - context 'and the request is done by a different user' do - let(:other_user) { create :user } + shared_examples 'revokes previous token' do + it 'revokes previous token' do + expect { request_access_token(user) }.to( + change { existing_token.reload.revoked_at }.from(nil)) + end + end - it 'generates and returns a different token for a different owner' do - expect do - request_access_token(other_user) - end.to change { Doorkeeper::AccessToken.count }.by(1) + context 'and the request is done by the resource owner' do + context 'with authorization code grant type' do + include_examples 'issues a new token' - expect(json_response['access_token']).not_to be_nil + it 'does not revoke previous token' do + request_access_token(user) + + expect(existing_token.reload.revoked_at).to be_nil + end + end + + context 'with refresh token grant type' do + let(:grant_type) { 'refresh_token' } + let(:refresh_token) { existing_token.refresh_token } + + include_examples 'issues a new token' + include_examples 'revokes previous token' + + context 'expired refresh token' do + let!(:existing_token) do + create(:oauth_access_token, application: application, + resource_owner_id: user.id, + created_at: 10.minutes.ago, + expires_in: 5) + end + + include_examples 'issues a new token' + include_examples 'revokes previous token' + end + + context 'revoked refresh token' do + let!(:existing_token) do + create(:oauth_access_token, application: application, + resource_owner_id: user.id, + created_at: 2.hours.ago, + revoked_at: 1.hour.ago, + expires_in: 5) + end + + it 'does not issue a new token' do + request_access_token(user) + + expect(json_response['error']).to eq('invalid_grant') + end + end end end end diff --git a/workhorse/go.mod b/workhorse/go.mod index 4afb47fb82a..283a2878c42 100644 --- a/workhorse/go.mod +++ b/workhorse/go.mod @@ -4,7 +4,7 @@ go 1.17 require ( github.com/Azure/azure-storage-blob-go v0.14.0 - github.com/BurntSushi/toml v0.3.1 + github.com/BurntSushi/toml v1.1.0 github.com/FZambia/sentinel v1.1.0 github.com/alecthomas/chroma/v2 v2.2.0 github.com/aws/aws-sdk-go v1.43.31 diff --git a/workhorse/go.sum b/workhorse/go.sum index 063d287b61c..50b11b945d3 100644 --- a/workhorse/go.sum +++ b/workhorse/go.sum @@ -143,8 +143,9 @@ github.com/Azure/go-autorest/logger v0.2.1 h1:IG7i4p/mDa2Ce4TRyAO8IHnVhAVF3RFU+Z github.com/Azure/go-autorest/logger v0.2.1/go.mod h1:T9E3cAhj2VqvPOtCYAvby9aBXkZmbF5NWuPV8+WeEW8= github.com/Azure/go-autorest/tracing v0.6.0 h1:TYi4+3m5t6K48TGI9AUdb+IzbnSxvnvUMfuitfgcfuo= github.com/Azure/go-autorest/tracing v0.6.0/go.mod h1:+vhtPC754Xsa23ID7GlGsrdKBpUA79WCAKPPZVC2DeU= -github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= +github.com/BurntSushi/toml v1.1.0 h1:ksErzDEI1khOiGPgpwuI7x2ebx/uXQNw7xJpn9Eq1+I= +github.com/BurntSushi/toml v1.1.0/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ= github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo= github.com/CloudyKit/fastprinter v0.0.0-20170127035650-74b38d55f37a/go.mod h1:EFZQ978U7x8IRnstaskI3IysnWY5Ao3QgZUKOXlsAdw= github.com/CloudyKit/fastprinter v0.0.0-20200109182630-33d98a066a53/go.mod h1:+3IMCy2vIlbG1XG/0ggNQv0SvxCAIpPM5b1nCz56Xno=