Commit 04e5ab35 authored by Pavel Shutsin's avatar Pavel Shutsin

Inherit user external status while creating project bots

Project bots should be external if created by external user

Changelog: security
parent 9251f6b9
...@@ -16,6 +16,8 @@ module ResourceAccessTokens ...@@ -16,6 +16,8 @@ module ResourceAccessTokens
return error(user.errors.full_messages.to_sentence) unless user.persisted? return error(user.errors.full_messages.to_sentence) unless user.persisted?
user.update!(external: true) if current_user.external?
access_level = params[:access_level] || Gitlab::Access::MAINTAINER access_level = params[:access_level] || Gitlab::Access::MAINTAINER
member = create_membership(resource, user, access_level) member = create_membership(resource, user, access_level)
......
# frozen_string_literal: true
class UpdateExternalProjectBots < ActiveRecord::Migration[6.1]
include Gitlab::Database::MigrationHelpers
class User < ActiveRecord::Base
self.table_name = 'users'
end
disable_ddl_transaction!
TMP_INDEX_NAME = 'tmp_idx_update_external_project_bots'
def up
add_concurrent_index('users', 'id', name: TMP_INDEX_NAME, where: 'external = true')
ids = ActiveRecord::Base.connection
.execute("SELECT u.id FROM users u JOIN users u2 on u2.id = u.created_by_id WHERE u.user_type = 6 AND u2.external = true")
.map { |result| result['id'] }
ids.each_slice(10) do |group|
UpdateExternalProjectBots::User.where(id: group).update_all(external: true)
end
remove_concurrent_index_by_name('users', TMP_INDEX_NAME)
end
def down
remove_concurrent_index_by_name('users', TMP_INDEX_NAME) if index_exists_by_name?('users', TMP_INDEX_NAME)
# This migration is irreversible
end
end
f6f5e081672fb42adde980fa12f696f5d8fd11921ee52c1472b3d745bb11a5ff
\ No newline at end of file
# frozen_string_literal: true
require 'spec_helper'
require_migration!('update_external_project_bots')
RSpec.describe UpdateExternalProjectBots, :migration do
def create_user(**extra_options)
defaults = { projects_limit: 0, email: "#{extra_options[:username]}@example.com" }
table(:users).create!(defaults.merge(extra_options))
end
it 'sets bot users as external if were created by external users' do
internal_user = create_user(username: 'foo')
external_user = create_user(username: 'bar', external: true)
internal_project_bot = create_user(username: 'foo2', user_type: 6, created_by_id: internal_user.id, external: false)
external_project_bot = create_user(username: 'bar2', user_type: 6, created_by_id: external_user.id, external: false)
migrate!
expect(table(:users).find(internal_project_bot.id).external).to eq false
expect(table(:users).find(external_project_bot.id).external).to eq true
end
end
...@@ -110,6 +110,18 @@ RSpec.describe ResourceAccessTokens::CreateService do ...@@ -110,6 +110,18 @@ RSpec.describe ResourceAccessTokens::CreateService do
expect(resource.members.developers.map(&:user_id)).to include(bot_user.id) expect(resource.members.developers.map(&:user_id)).to include(bot_user.id)
end end
end end
context 'when user is external' do
let(:user) { create(:user, :external) }
before do
project.add_maintainer(user)
end
it 'creates resource bot user with external status' do
expect(subject.payload[:access_token].user.external).to eq true
end
end
end end
context 'personal access token' do context 'personal access token' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment