mirror of
https://github.com/opf/openproject.git
synced 2026-06-14 03:30:14 +00:00
Merge pull request #22691 from opf/bug/73707-user-does-not-meet-password-requirements-during-jira-import
[#73707] User does not meet password requirements during Jira Import.
This commit is contained in:
@@ -30,6 +30,8 @@
|
||||
|
||||
module Import
|
||||
class JiraImport < ApplicationRecord
|
||||
include Import::JiraOpenProjectReferenceCreation
|
||||
|
||||
self.table_name = "jira_imports"
|
||||
|
||||
belongs_to :jira, class_name: "Import::Jira"
|
||||
@@ -81,5 +83,93 @@ module Import
|
||||
Import::JiraUser.where(jira_import_id: id).destroy_all
|
||||
end
|
||||
# rubocop:enable Metrics/AbcSize
|
||||
|
||||
def import_users
|
||||
Import::JiraUser.where(jira_import_id: id).find_each do |jira_user|
|
||||
import_user(jira_user)
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
# rubocop:disable Metrics/PerceivedComplexity
|
||||
# rubocop:disable Metrics/AbcSize
|
||||
def import_user(jira_user)
|
||||
call = Users::CreateService
|
||||
.new(user: User.system, contract_class: EmptyContract)
|
||||
.call(jira_user.to_op_attributes)
|
||||
|
||||
call.on_success do |_result|
|
||||
create_reference!(
|
||||
op_leg: call.result,
|
||||
jira_leg: jira_user,
|
||||
jira_import: self,
|
||||
uses_existing: false
|
||||
)
|
||||
end
|
||||
call.on_failure do |_result|
|
||||
if call.errors.find { |error| error.type == :taken }.present?
|
||||
user = jira_user.try_to_find_existing_op_users.first
|
||||
if user.present?
|
||||
create_reference!(
|
||||
op_leg: user,
|
||||
jira_leg: jira_user,
|
||||
jira_import: self,
|
||||
uses_existing: true
|
||||
)
|
||||
else
|
||||
raise "Existing User is expected to be found, because there was an email " \
|
||||
"or login collision. See attributes: #{jira_user.to_op_attributes}"
|
||||
end
|
||||
else
|
||||
raise call.message
|
||||
end
|
||||
end
|
||||
|
||||
jira_user_groups = jira_user.payload["groups"]["items"].pluck("name")
|
||||
|
||||
jira_user_groups.each do |group_name|
|
||||
call = Groups::CreateService
|
||||
.new(user: User.system, contract_class: EmptyContract)
|
||||
.call(name: group_name)
|
||||
call.on_success do |result|
|
||||
group = result.result
|
||||
create_reference!(
|
||||
op_leg: group,
|
||||
jira_leg: nil,
|
||||
jira_import: self,
|
||||
uses_existing: false
|
||||
)
|
||||
end
|
||||
call.on_failure do |_result|
|
||||
if call.errors.find { |error| error.type == :taken }.present?
|
||||
group = Group.where(name: group_name).first
|
||||
if group.present?
|
||||
create_reference!(
|
||||
op_leg: group,
|
||||
jira_leg: nil,
|
||||
jira_import: self,
|
||||
uses_existing: true
|
||||
)
|
||||
else
|
||||
raise "Existing Group is expected to be found. Group name: #{group_name}"
|
||||
end
|
||||
else
|
||||
raise call.message
|
||||
end
|
||||
end
|
||||
member_id = Import::JiraOpenProjectReference.where(
|
||||
jira_import_id: id,
|
||||
jira_entity_id: jira_user.id,
|
||||
jira_entity_class: jira_user.class.to_s
|
||||
).pick(:op_entity_id)
|
||||
group = Group.find_by!(name: group_name)
|
||||
Groups::AddUsersService
|
||||
.new(group, current_user: User.system)
|
||||
.call(ids: [member_id], send_notifications: false)
|
||||
end
|
||||
end
|
||||
# rubocop:enable Metrics/PerceivedComplexity
|
||||
# rubocop:enable Metrics/AbcSize
|
||||
end
|
||||
end
|
||||
|
||||
@@ -43,7 +43,7 @@ module Import
|
||||
firstname, lastname = split_display_name(payload["displayName"])
|
||||
{
|
||||
login: payload["name"],
|
||||
password: SecureRandom.uuid,
|
||||
password: OpenProject::Passwords::Generator.random_password,
|
||||
firstname:,
|
||||
lastname:,
|
||||
mail: payload["emailAddress"],
|
||||
|
||||
@@ -39,7 +39,7 @@ module Import
|
||||
fetch_and_save_users_data(jira_import)
|
||||
|
||||
Journal::NotificationConfiguration.with(false) do
|
||||
import_users(jira_import)
|
||||
jira_import.import_users
|
||||
Import::JiraImportProjectsJob.perform_now(jira_import_id)
|
||||
end
|
||||
|
||||
@@ -163,91 +163,5 @@ module Import
|
||||
raise "Error fetching user data for user key #{jira_user_key}: #{e.message}"
|
||||
end
|
||||
end
|
||||
|
||||
def import_users(jira_import)
|
||||
Import::JiraUser.where(jira_import_id: jira_import.id).find_each do |jira_user|
|
||||
import_user(jira_user, jira_import)
|
||||
end
|
||||
end
|
||||
|
||||
# rubocop:disable Metrics/PerceivedComplexity
|
||||
# rubocop:disable Metrics/AbcSize
|
||||
def import_user(jira_user, jira_import)
|
||||
call = Users::CreateService
|
||||
.new(user: User.system, contract_class: EmptyContract)
|
||||
.call(jira_user.to_op_attributes)
|
||||
|
||||
call.on_success do |_result|
|
||||
create_reference!(
|
||||
op_leg: call.result,
|
||||
jira_leg: jira_user,
|
||||
jira_import:,
|
||||
uses_existing: false
|
||||
)
|
||||
end
|
||||
call.on_failure do |_result|
|
||||
if call.errors.find { |error| error.type == :taken }.present?
|
||||
user = jira_user.try_to_find_existing_op_users.first
|
||||
if user.present?
|
||||
create_reference!(
|
||||
op_leg: user,
|
||||
jira_leg: jira_user,
|
||||
jira_import:,
|
||||
uses_existing: true
|
||||
)
|
||||
else
|
||||
raise "Existing User is expected to be found, because there was an email " \
|
||||
"or login collision. See attributes: #{jira_user.to_op_attributes}"
|
||||
end
|
||||
else
|
||||
raise call.message
|
||||
end
|
||||
end
|
||||
|
||||
jira_user_groups = jira_user.payload["groups"]["items"].pluck("name")
|
||||
|
||||
jira_user_groups.each do |group_name|
|
||||
call = Groups::CreateService
|
||||
.new(user: User.system, contract_class: EmptyContract)
|
||||
.call(name: group_name)
|
||||
call.on_success do |result|
|
||||
group = result.result
|
||||
create_reference!(
|
||||
op_leg: group,
|
||||
jira_leg: nil,
|
||||
jira_import:,
|
||||
uses_existing: false
|
||||
)
|
||||
end
|
||||
call.on_failure do |_result|
|
||||
if call.errors.find { |error| error.type == :taken }.present?
|
||||
group = Group.where(name: group_name).first
|
||||
if group.present?
|
||||
create_reference!(
|
||||
op_leg: group,
|
||||
jira_leg: nil,
|
||||
jira_import:,
|
||||
uses_existing: true
|
||||
)
|
||||
else
|
||||
raise "Existing Group is expected to be found. Group name: #{group_name}"
|
||||
end
|
||||
else
|
||||
raise call.message
|
||||
end
|
||||
end
|
||||
member_id = Import::JiraOpenProjectReference.where(
|
||||
jira_import_id: jira_import.id,
|
||||
jira_entity_id: jira_user.id,
|
||||
jira_entity_class: jira_user.class.to_s
|
||||
).pick(:op_entity_id)
|
||||
group = Group.find_by!(name: group_name)
|
||||
Groups::AddUsersService
|
||||
.new(group, current_user: User.system)
|
||||
.call(ids: [member_id], send_notifications: false)
|
||||
end
|
||||
end
|
||||
# rubocop:enable Metrics/PerceivedComplexity
|
||||
# rubocop:enable Metrics/AbcSize
|
||||
end
|
||||
end
|
||||
|
||||
@@ -32,7 +32,8 @@ require "spec_helper"
|
||||
|
||||
RSpec.describe Import::JiraImport do
|
||||
let(:jira) { create(:jira) }
|
||||
let(:author) { create(:user) }
|
||||
let(:author_password) { OpenProject::Passwords::Generator.random_password }
|
||||
let(:author) { create(:user, password: author_password, password_confirmation: author_password) }
|
||||
|
||||
subject(:jira_import) { create(:jira_import, jira:, author:) }
|
||||
|
||||
@@ -132,4 +133,332 @@ RSpec.describe Import::JiraImport do
|
||||
expect(Import::JiraField.exists?(other_field.id)).to be true
|
||||
end
|
||||
end
|
||||
|
||||
describe "#import_users", with_settings: {
|
||||
password_active_rules: %w(lowercase uppercase numeric special),
|
||||
password_min_length: 4,
|
||||
password_min_adhered_rules: 4
|
||||
} do
|
||||
def jira_user_payload(name:, display_name:, email:, groups: [], key: "JIRAUSER10000", active: true)
|
||||
{
|
||||
"key" => key,
|
||||
"name" => name,
|
||||
"self" => "https://jira-dc.openproject.org/rest/api/2/user?username=#{name}",
|
||||
"active" => active,
|
||||
"expand" => "groups,applicationRoles",
|
||||
"groups" => {
|
||||
"size" => groups.size,
|
||||
"items" => groups.map { |g| { "name" => g, "self" => "https://jira-dc.openproject.org/rest/api/2/group?groupname=#{g}" } }
|
||||
},
|
||||
"locale" => "en_US",
|
||||
"deleted" => false,
|
||||
"timeZone" => "Europe/Berlin",
|
||||
"avatarUrls" => {
|
||||
"16x16" => "https://www.gravatar.com/avatar/abc?d=mm&s=16",
|
||||
"24x24" => "https://www.gravatar.com/avatar/abc?d=mm&s=24",
|
||||
"32x32" => "https://www.gravatar.com/avatar/abc?d=mm&s=32",
|
||||
"48x48" => "https://www.gravatar.com/avatar/abc?d=mm&s=48"
|
||||
},
|
||||
"displayName" => display_name,
|
||||
"emailAddress" => email,
|
||||
"lastLoginTime" => "2026-03-26T08:49:31+0000",
|
||||
"applicationRoles" => { "size" => 1, "items" => [] }
|
||||
}
|
||||
end
|
||||
|
||||
let(:email) { "jdoe@example.com" }
|
||||
let(:existing_user_password) { OpenProject::Passwords::Generator.random_password }
|
||||
|
||||
# creates system user proactively. so, next coming User.count change cases don't count
|
||||
# this one as created during #import_users call.
|
||||
before { User.system }
|
||||
|
||||
context "when importing a new user without groups" do
|
||||
let!(:jira_user) do
|
||||
create(:jira_user,
|
||||
jira:,
|
||||
jira_import:,
|
||||
payload: jira_user_payload(
|
||||
key: "JIRAUSER10100",
|
||||
name: "jdoe@example.com",
|
||||
display_name: "John Doe",
|
||||
email:,
|
||||
groups: []
|
||||
))
|
||||
end
|
||||
|
||||
it "creates a new OpenProject user" do
|
||||
expect { jira_import.import_users }.to change(User, :count).by(1)
|
||||
end
|
||||
|
||||
it "creates the user with correct attributes" do
|
||||
jira_import.import_users
|
||||
|
||||
user = User.find_by(login: email)
|
||||
expect(user).to have_attributes(
|
||||
firstname: "John",
|
||||
lastname: "Doe",
|
||||
mail: email,
|
||||
status: "locked"
|
||||
)
|
||||
end
|
||||
|
||||
it "creates a reference between Jira user and OpenProject user" do
|
||||
expect { jira_import.import_users }.to change(Import::JiraOpenProjectReference, :count).by(1)
|
||||
|
||||
reference = Import::JiraOpenProjectReference.last
|
||||
expect(reference).to have_attributes(
|
||||
jira_entity_id: jira_user.id.to_s,
|
||||
jira_entity_class: "Import::JiraUser",
|
||||
op_entity_class: "User",
|
||||
uses_existing: false
|
||||
)
|
||||
end
|
||||
end
|
||||
|
||||
context "when importing a user that already exists by email" do
|
||||
let!(:existing_user) do
|
||||
create(:user,
|
||||
mail: email,
|
||||
password: existing_user_password,
|
||||
password_confirmation: existing_user_password,
|
||||
login: "login")
|
||||
end
|
||||
let!(:jira_user) do
|
||||
create(:jira_user,
|
||||
jira:,
|
||||
jira_import:,
|
||||
payload: jira_user_payload(
|
||||
key: "JIRAUSER10101",
|
||||
name: "jdoe@example.com",
|
||||
display_name: "John Doe",
|
||||
email:,
|
||||
groups: []
|
||||
))
|
||||
end
|
||||
|
||||
it "does not create a new user" do
|
||||
expect { jira_import.import_users }.not_to change(User, :count)
|
||||
end
|
||||
|
||||
it "creates a reference to the existing user with uses_existing flag" do
|
||||
jira_import.import_users
|
||||
|
||||
reference = Import::JiraOpenProjectReference.find_by(jira_entity_id: jira_user.id)
|
||||
expect(reference).to have_attributes(
|
||||
jira_entity_id: jira_user.id.to_s,
|
||||
jira_entity_class: "Import::JiraUser",
|
||||
op_entity_id: existing_user.id.to_s,
|
||||
op_entity_class: "User",
|
||||
uses_existing: true
|
||||
)
|
||||
end
|
||||
end
|
||||
|
||||
context "when importing a user that already exists by login" do
|
||||
let(:login) { "login" }
|
||||
let!(:existing_user) do
|
||||
create(:user,
|
||||
mail: "other@example.com",
|
||||
password_confirmation: existing_user_password,
|
||||
password: existing_user_password,
|
||||
login:)
|
||||
end
|
||||
let!(:jira_user) do
|
||||
create(:jira_user,
|
||||
jira:,
|
||||
jira_import:,
|
||||
payload: jira_user_payload(
|
||||
key: "JIRAUSER10102",
|
||||
name: login,
|
||||
display_name: "John Doe",
|
||||
email:,
|
||||
groups: []
|
||||
))
|
||||
end
|
||||
|
||||
it "does not create a new user" do
|
||||
expect { jira_import.import_users }.not_to change(User, :count)
|
||||
end
|
||||
|
||||
it "creates a reference to the existing user with uses_existing flag" do
|
||||
jira_import.import_users
|
||||
|
||||
reference = Import::JiraOpenProjectReference.find_by(jira_entity_id: jira_user.id)
|
||||
expect(reference).to have_attributes(
|
||||
jira_entity_id: jira_user.id.to_s,
|
||||
jira_entity_class: "Import::JiraUser",
|
||||
op_entity_id: existing_user.id.to_s,
|
||||
op_entity_class: "User",
|
||||
uses_existing: true
|
||||
)
|
||||
end
|
||||
end
|
||||
|
||||
context "when importing a user with groups" do
|
||||
let!(:jira_user) do
|
||||
create(:jira_user,
|
||||
jira:,
|
||||
jira_import:,
|
||||
payload: jira_user_payload(
|
||||
key: "JIRAUSER10103",
|
||||
name: "j.roth@openproject.com",
|
||||
display_name: "Judith Roth",
|
||||
email: "j.roth@openproject.com",
|
||||
groups: ["jira-administrators", "jira-software-users"]
|
||||
))
|
||||
end
|
||||
|
||||
it "creates the groups" do
|
||||
expect { jira_import.import_users }.to change(Group, :count).by(2)
|
||||
|
||||
expect(Group.exists?(name: "jira-administrators")).to be true
|
||||
expect(Group.exists?(name: "jira-software-users")).to be true
|
||||
end
|
||||
|
||||
it "adds the user to the groups" do
|
||||
jira_import.import_users
|
||||
|
||||
user = User.find_by(login: "j.roth@openproject.com")
|
||||
expect(user.groups.pluck(:name)).to contain_exactly("jira-administrators", "jira-software-users")
|
||||
end
|
||||
|
||||
it "creates references for the groups" do
|
||||
jira_import.import_users
|
||||
|
||||
group_references = Import::JiraOpenProjectReference.where(op_entity_class: "Group")
|
||||
expect(group_references.count).to eq(2)
|
||||
expect(group_references.pluck(:uses_existing)).to all(be false)
|
||||
end
|
||||
end
|
||||
|
||||
context "when importing a user with an existing group" do
|
||||
let!(:existing_group) { create(:group, name: "jira-administrators") }
|
||||
let!(:jira_user) do
|
||||
create(:jira_user,
|
||||
jira:,
|
||||
jira_import:,
|
||||
payload: jira_user_payload(
|
||||
key: "JIRAUSER10104",
|
||||
name: "j.roth@openproject.com",
|
||||
display_name: "Judith Roth",
|
||||
email: "j.roth@openproject.com",
|
||||
groups: ["jira-administrators"]
|
||||
))
|
||||
end
|
||||
|
||||
it "does not create a duplicate group" do
|
||||
expect { jira_import.import_users }.not_to change(Group, :count)
|
||||
end
|
||||
|
||||
it "adds the user to the existing group" do
|
||||
jira_import.import_users
|
||||
|
||||
user = User.find_by(login: "j.roth@openproject.com")
|
||||
expect(user.groups).to include(existing_group)
|
||||
end
|
||||
|
||||
it "creates a reference with uses_existing flag for the group" do
|
||||
jira_import.import_users
|
||||
|
||||
group_reference = Import::JiraOpenProjectReference.find_by(
|
||||
op_entity_class: "Group",
|
||||
op_entity_id: existing_group.id
|
||||
)
|
||||
expect(group_reference.uses_existing).to be true
|
||||
end
|
||||
end
|
||||
|
||||
context "when importing multiple users" do
|
||||
let!(:jira_user1) do
|
||||
create(:jira_user,
|
||||
jira:,
|
||||
jira_import:,
|
||||
payload: jira_user_payload(
|
||||
key: "JIRAUSER10105",
|
||||
name: "jdoe@example.com",
|
||||
display_name: "John Doe",
|
||||
email: "jdoe@example.com",
|
||||
groups: ["jira-software-users"]
|
||||
))
|
||||
end
|
||||
let!(:jira_user2) do
|
||||
create(:jira_user,
|
||||
jira:,
|
||||
jira_import:,
|
||||
payload: jira_user_payload(
|
||||
key: "JIRAUSER10106",
|
||||
name: "jsmith@example.com",
|
||||
display_name: "Jane Smith",
|
||||
email: "jsmith@example.com",
|
||||
groups: ["jira-software-users"]
|
||||
))
|
||||
end
|
||||
|
||||
it "creates all users" do
|
||||
expect { jira_import.import_users }.to change(User, :count).by(2)
|
||||
end
|
||||
|
||||
it "creates the shared group only once" do
|
||||
expect { jira_import.import_users }.to change(Group, :count).by(1)
|
||||
end
|
||||
|
||||
it "adds both users to the shared group" do
|
||||
jira_import.import_users
|
||||
|
||||
group = Group.find_by(name: "jira-software-users")
|
||||
expect(group.users.pluck(:login)).to contain_exactly("jdoe@example.com", "jsmith@example.com")
|
||||
end
|
||||
end
|
||||
|
||||
context "when user has a single-word display name" do
|
||||
let!(:jira_user) do
|
||||
create(:jira_user,
|
||||
jira:,
|
||||
jira_import:,
|
||||
payload: jira_user_payload(
|
||||
key: "JIRAUSER10108",
|
||||
name: "admin@example.com",
|
||||
display_name: "Administrator",
|
||||
email: "admin@example.com",
|
||||
groups: []
|
||||
))
|
||||
end
|
||||
|
||||
it "uses the name for both firstname and lastname" do
|
||||
jira_import.import_users
|
||||
|
||||
user = User.find_by(login: "admin@example.com")
|
||||
expect(user).to have_attributes(
|
||||
firstname: "Administrator",
|
||||
lastname: "Administrator"
|
||||
)
|
||||
end
|
||||
end
|
||||
|
||||
context "when user has a multi-part display name" do
|
||||
let!(:jira_user) do
|
||||
create(:jira_user,
|
||||
jira:,
|
||||
jira_import:,
|
||||
payload: jira_user_payload(
|
||||
key: "JIRAUSER10109",
|
||||
name: "jvd@example.com",
|
||||
display_name: "Jean Van Der Berg",
|
||||
email: "jvd@example.com",
|
||||
groups: []
|
||||
))
|
||||
end
|
||||
|
||||
it "uses all but last word as firstname and last word as lastname" do
|
||||
jira_import.import_users
|
||||
|
||||
user = User.find_by(login: "jvd@example.com")
|
||||
expect(user).to have_attributes(
|
||||
firstname: "Jean Van Der",
|
||||
lastname: "Berg"
|
||||
)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user