This instance will be upgraded to Heptapod 0.31.0 (final) on 2022-05-24 at 14:00 UTC+2 (a few minutes of down time)

Commit 98e666ab authored by Alex Lossent's avatar Alex Lossent
Browse files

Improve invalidation of stored service password if the endpoint URL is changed

Password can now be specified at the same time as the new URL, and the service
template admin pages now work.
parent 616d42aceaf8
......@@ -39,7 +39,13 @@ def service
end
def application_services_params
params.permit(:id,
application_services_params = params.permit(:id,
service: Projects::ServicesController::ALLOWED_PARAMS)
if application_services_params[:service].is_a?(Hash)
Projects::ServicesController::FILTER_BLANK_PARAMS.each do |param|
application_services_params[:service].delete(param) if application_services_params[:service][param].blank?
end
end
application_services_params
end
end
......@@ -9,6 +9,10 @@ class Projects::ServicesController < Projects::ApplicationController
:note_events, :send_from_committer_email, :disable_diffs, :external_wiki_url,
:notify, :color,
:server_host, :server_port, :default_irc_uri, :enable_ssl_verification]
# Parameters to ignore if no value is specified
FILTER_BLANK_PARAMS = [:password]
# Authorize
before_action :authorize_admin_project!
before_action :service, only: [:edit, :update, :test]
......@@ -59,7 +63,9 @@ def service
def service_params
service_params = params.require(:service).permit(ALLOWED_PARAMS)
service_params.delete("password") if service_params["password"].blank?
FILTER_BLANK_PARAMS.each do |param|
service_params.delete(param) if service_params[param].blank?
end
service_params
end
end
......@@ -48,7 +48,7 @@ def compose_service_hook
end
def reset_password
if prop_updated?(:bamboo_url)
if bamboo_url_changed? && !password_touched?
self.password = nil
end
end
......
......@@ -45,7 +45,7 @@ def compose_service_hook
end
def reset_password
if prop_updated?(:teamcity_url)
if teamcity_url_changed? && !password_touched?
self.password = nil
end
end
......
......@@ -33,6 +33,8 @@ class Service < ActiveRecord::Base
after_initialize :initialize_properties
after_commit :reset_updated_properties
belongs_to :project
has_one :service_hook
......@@ -103,6 +105,7 @@ def can_test?
# Provide convenient accessor methods
# for each serialized property.
# Also keep track of updated properties in a similar way as ActiveModel::Dirty
def self.prop_accessor(*args)
args.each do |arg|
class_eval %{
......@@ -111,21 +114,39 @@ def #{arg}
end
def #{arg}=(value)
updated_properties['#{arg}'] = #{arg} unless #{arg}_changed?
self.properties['#{arg}'] = value
end
def #{arg}_changed?
#{arg}_touched? && #{arg} != #{arg}_was
end
def #{arg}_touched?
updated_properties.include?('#{arg}')
end
def #{arg}_was
updated_properties['#{arg}']
end
}
end
end
# ActiveRecord does not provide a mechanism to track changes in serialized keys.
# This is why we need to perform extra query to do it mannually.
def prop_updated?(prop_name)
relation_name = self.type.underscore
previous_value = project.send(relation_name).send(prop_name)
return false if previous_value.nil?
previous_value != send(prop_name)
# Returns a hash of the properties that have been assigned a new value since last save,
# indicating their original values (attr => original value).
# ActiveRecord does not provide a mechanism to track changes in serialized keys,
# so we need a specific implementation for service properties.
# This allows to track changes to properties set with the accessor methods,
# but not direct manipulation of properties hash.
def updated_properties
@updated_properties ||= ActiveSupport::HashWithIndifferentAccess.new
end
def reset_updated_properties
@updated_properties = nil
end
def async_execute(data)
return unless supported_events.include?(data[:object_kind])
......
......@@ -30,27 +30,65 @@
let(:user) { create(:user) }
let(:project) { create(:project) }
before do
@bamboo_service = BambooService.create(
project: create(:project),
properties: {
bamboo_url: 'http://gitlab.com',
username: 'mic',
password: "password"
}
)
end
context "when a password was previously set" do
before do
@bamboo_service = BambooService.create(
project: create(:project),
properties: {
bamboo_url: 'http://gitlab.com',
username: 'mic',
password: "password"
}
)
end
it "reset password if url changed" do
@bamboo_service.bamboo_url = 'http://gitlab1.com'
@bamboo_service.save
expect(@bamboo_service.password).to be_nil
end
it "does not reset password if username changed" do
@bamboo_service.username = "some_name"
@bamboo_service.save
expect(@bamboo_service.password).to eq("password")
end
it "does not reset password if new url is set together with password, even if it's the same password" do
@bamboo_service.bamboo_url = 'http://gitlab_edited.com'
@bamboo_service.password = 'password'
@bamboo_service.save
expect(@bamboo_service.password).to eq("password")
expect(@bamboo_service.bamboo_url).to eq("http://gitlab_edited.com")
end
it "reset password if url changed" do
@bamboo_service.bamboo_url = 'http://gitlab1.com'
@bamboo_service.save
expect(@bamboo_service.password).to be_nil
it "should reset password if url changed, even if setter called multiple times" do
@bamboo_service.bamboo_url = 'http://gitlab1.com'
@bamboo_service.bamboo_url = 'http://gitlab1.com'
@bamboo_service.save
expect(@bamboo_service.password).to be_nil
end
end
context "when no password was previously set" do
before do
@bamboo_service = BambooService.create(
project: create(:project),
properties: {
bamboo_url: 'http://gitlab.com',
username: 'mic'
}
)
end
it "saves password if new url is set together with password" do
@bamboo_service.bamboo_url = 'http://gitlab_edited.com'
@bamboo_service.password = 'password'
@bamboo_service.save
expect(@bamboo_service.password).to eq("password")
expect(@bamboo_service.bamboo_url).to eq("http://gitlab_edited.com")
end
it "does not reset password if username changed" do
@bamboo_service.username = "some_name"
@bamboo_service.save
expect(@bamboo_service.password).to eq("password")
end
end
end
......@@ -30,27 +30,64 @@
let(:user) { create(:user) }
let(:project) { create(:project) }
before do
@teamcity_service = TeamcityService.create(
project: create(:project),
properties: {
teamcity_url: 'http://gitlab.com',
username: 'mic',
password: "password"
}
)
end
context "when a password was previously set" do
before do
@teamcity_service = TeamcityService.create(
project: create(:project),
properties: {
teamcity_url: 'http://gitlab.com',
username: 'mic',
password: "password"
}
)
end
it "reset password if url changed" do
@teamcity_service.teamcity_url = 'http://gitlab1.com'
@teamcity_service.save
expect(@teamcity_service.password).to be_nil
end
it "does not reset password if username changed" do
@teamcity_service.username = "some_name"
@teamcity_service.save
expect(@teamcity_service.password).to eq("password")
end
it "does not reset password if new url is set together with password, even if it's the same password" do
@teamcity_service.teamcity_url = 'http://gitlab_edited.com'
@teamcity_service.password = 'password'
@teamcity_service.save
expect(@teamcity_service.password).to eq("password")
expect(@teamcity_service.teamcity_url).to eq("http://gitlab_edited.com")
end
it "reset password if url changed" do
@teamcity_service.teamcity_url = 'http://gitlab1.com'
@teamcity_service.save
expect(@teamcity_service.password).to be_nil
it "should reset password if url changed, even if setter called multiple times" do
@teamcity_service.teamcity_url = 'http://gitlab1.com'
@teamcity_service.teamcity_url = 'http://gitlab1.com'
@teamcity_service.save
expect(@teamcity_service.password).to be_nil
end
end
context "when no password was previously set" do
before do
@teamcity_service = TeamcityService.create(
project: create(:project),
properties: {
teamcity_url: 'http://gitlab.com',
username: 'mic'
}
)
end
it "does not reset password if username changed" do
@teamcity_service.username = "some_name"
@teamcity_service.save
expect(@teamcity_service.password).to eq("password")
it "saves password if new url is set together with password" do
@teamcity_service.teamcity_url = 'http://gitlab_edited.com'
@teamcity_service.password = 'password'
@teamcity_service.save
expect(@teamcity_service.password).to eq("password")
expect(@teamcity_service.teamcity_url).to eq("http://gitlab_edited.com")
end
end
end
end
......@@ -104,7 +104,7 @@
end
end
describe "#prop_updated?" do
describe "{property}_changed?" do
let(:service) do
BambooService.create(
project: create(:project),
......@@ -116,14 +116,112 @@
)
end
it "returns false" do
it "returns false when the property has not been assigned a new value" do
service.username = "key_changed"
expect(service.prop_updated?(:bamboo_url)).to be_falsy
expect(service.bamboo_url_changed?).to be_falsy
end
it "returns true" do
service.bamboo_url = "http://other.com"
expect(service.prop_updated?(:bamboo_url)).to be_truthy
it "returns true when the property has been assigned a different value" do
service.bamboo_url = "http://example.com"
expect(service.bamboo_url_changed?).to be_truthy
end
it "returns true when the property has been assigned a different value twice" do
service.bamboo_url = "http://example.com"
service.bamboo_url = "http://example.com"
expect(service.bamboo_url_changed?).to be_truthy
end
it "returns false when the property has been re-assigned the same value" do
service.bamboo_url = 'http://gitlab.com'
expect(service.bamboo_url_changed?).to be_falsy
end
it "returns false when the property has been assigned a new value then saved" do
service.bamboo_url = 'http://example.com'
service.save
expect(service.bamboo_url_changed?).to be_falsy
end
end
describe "{property}_touched?" do
let(:service) do
BambooService.create(
project: create(:project),
properties: {
bamboo_url: 'http://gitlab.com',
username: 'mic',
password: "password"
}
)
end
it "returns false when the property has not been assigned a new value" do
service.username = "key_changed"
expect(service.bamboo_url_touched?).to be_falsy
end
it "returns true when the property has been assigned a different value" do
service.bamboo_url = "http://example.com"
expect(service.bamboo_url_touched?).to be_truthy
end
it "returns true when the property has been assigned a different value twice" do
service.bamboo_url = "http://example.com"
service.bamboo_url = "http://example.com"
expect(service.bamboo_url_touched?).to be_truthy
end
it "returns true when the property has been re-assigned the same value" do
service.bamboo_url = 'http://gitlab.com'
expect(service.bamboo_url_touched?).to be_truthy
end
it "returns false when the property has been assigned a new value then saved" do
service.bamboo_url = 'http://example.com'
service.save
expect(service.bamboo_url_changed?).to be_falsy
end
end
describe "{property}_was" do
let(:service) do
BambooService.create(
project: create(:project),
properties: {
bamboo_url: 'http://gitlab.com',
username: 'mic',
password: "password"
}
)
end
it "returns nil when the property has not been assigned a new value" do
service.username = "key_changed"
expect(service.bamboo_url_was).to be_nil
end
it "returns the previous value when the property has been assigned a different value" do
service.bamboo_url = "http://example.com"
expect(service.bamboo_url_was).to eq('http://gitlab.com')
end
it "returns initial value when the property has been re-assigned the same value" do
service.bamboo_url = 'http://gitlab.com'
expect(service.bamboo_url_was).to eq('http://gitlab.com')
end
it "returns initial value when the property has been assigned multiple values" do
service.bamboo_url = "http://example.com"
service.bamboo_url = "http://example2.com"
expect(service.bamboo_url_was).to eq('http://gitlab.com')
end
it "returns nil when the property has been assigned a new value then saved" do
service.bamboo_url = 'http://example.com'
service.save
expect(service.bamboo_url_was).to be_nil
end
end
end
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