From 1f304e809e6ed76ff47bad2b0b491748cddc1ac3 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Thu, 18 Dec 2014 17:11:19 +0100 Subject: [PATCH] SONAR-5963 Automatically reactivate user when using a disabled login --- .../sonar/server/user/DefaultUserService.java | 24 +++-- .../server/user/DefaultUserServiceTest.java | 66 +++++++++++++ server/sonar-web/src/main/js/application.js | 90 +++++++++--------- .../app/controllers/api/users_controller.rb | 95 +------------------ .../app/controllers/users_controller.rb | 68 ++----------- .../app/views/users/_create_form.html.erb | 33 +++---- .../app/views/users/_edit_form.html.erb | 8 +- .../app/views/users/_reactivate_form.html.erb | 24 ----- 8 files changed, 152 insertions(+), 256 deletions(-) delete mode 100644 server/sonar-web/src/main/webapp/WEB-INF/app/views/users/_reactivate_form.html.erb diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/DefaultUserService.java b/server/sonar-server/src/main/java/org/sonar/server/user/DefaultUserService.java index df1cf9691c7..7de63a17920 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/user/DefaultUserService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/user/DefaultUserService.java @@ -82,7 +82,7 @@ public class DefaultUserService implements RubyUserService { userService.index(); } - public void create(Map params) { + public boolean create(Map params) { NewUser newUser = NewUser.create() .setLogin((String) params.get("login")) .setName((String) params.get("name")) @@ -90,16 +90,24 @@ public class DefaultUserService implements RubyUserService { .setScmAccounts((RubyUtils.toStrings(params.get("scm_accounts")))) .setPassword((String) params.get("password")) .setPasswordConfirmation((String) params.get("password_confirmation")); - userService.create(newUser); + return userService.create(newUser); } public void update(Map params) { - UpdateUser updateUser = UpdateUser.create((String) params.get("login")) - .setName((String) params.get("name")) - .setEmail((String) params.get("email")) - .setScmAccounts((RubyUtils.toStrings(params.get("scm_accounts")))) - .setPassword((String) params.get("password")) - .setPasswordConfirmation((String) params.get("password_confirmation")); + UpdateUser updateUser = UpdateUser.create((String) params.get("login")); + if (params.containsKey("name")) { + updateUser.setName((String) params.get("name")); + } + if (params.containsKey("email")) { + updateUser.setEmail((String) params.get("email")); + } + if (params.containsKey("scm_accounts")) { + updateUser.setScmAccounts((RubyUtils.toStrings(params.get("scm_accounts")))); + } + if (params.containsKey("password")) { + updateUser.setPassword((String) params.get("password")); + updateUser.setPasswordConfirmation((String) params.get("password_confirmation")); + } userService.update(updateUser); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/DefaultUserServiceTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/DefaultUserServiceTest.java index 4d87662e4e9..b8d17b96fa6 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/user/DefaultUserServiceTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/user/DefaultUserServiceTest.java @@ -154,6 +154,9 @@ public class DefaultUserServiceTest { params.put("name", "John"); params.put("email", "john@email.com"); params.put("scm_accounts", newArrayList("jn")); + params.put("password", "1234"); + params.put("password_confirmation", "1234"); + service.update(params); ArgumentCaptor userCaptor = ArgumentCaptor.forClass(UpdateUser.class); @@ -162,6 +165,69 @@ public class DefaultUserServiceTest { assertThat(userCaptor.getValue().name()).isEqualTo("John"); assertThat(userCaptor.getValue().email()).isEqualTo("john@email.com"); assertThat(userCaptor.getValue().scmAccounts()).containsOnly("jn"); + assertThat(userCaptor.getValue().password()).isEqualTo("1234"); + assertThat(userCaptor.getValue().passwordConfirmation()).isEqualTo("1234"); + } + + @Test + public void update_only_name() throws Exception { + Map params = newHashMap(); + params.put("login", "john"); + params.put("name", "John"); + service.update(params); + + ArgumentCaptor userCaptor = ArgumentCaptor.forClass(UpdateUser.class); + verify(userService).update(userCaptor.capture()); + assertThat(userCaptor.getValue().isNameChanged()).isTrue(); + assertThat(userCaptor.getValue().isEmailChanged()).isFalse(); + assertThat(userCaptor.getValue().isScmAccountsChanged()).isFalse(); + assertThat(userCaptor.getValue().isPasswordChanged()).isFalse(); + } + + @Test + public void update_only_email() throws Exception { + Map params = newHashMap(); + params.put("login", "john"); + params.put("email", "john@email.com"); + service.update(params); + + ArgumentCaptor userCaptor = ArgumentCaptor.forClass(UpdateUser.class); + verify(userService).update(userCaptor.capture()); + assertThat(userCaptor.getValue().isNameChanged()).isFalse(); + assertThat(userCaptor.getValue().isEmailChanged()).isTrue(); + assertThat(userCaptor.getValue().isScmAccountsChanged()).isFalse(); + assertThat(userCaptor.getValue().isPasswordChanged()).isFalse(); + } + + @Test + public void update_only_scm_accounts() throws Exception { + Map params = newHashMap(); + params.put("login", "john"); + params.put("scm_accounts", newArrayList("jn")); + service.update(params); + + ArgumentCaptor userCaptor = ArgumentCaptor.forClass(UpdateUser.class); + verify(userService).update(userCaptor.capture()); + assertThat(userCaptor.getValue().isNameChanged()).isFalse(); + assertThat(userCaptor.getValue().isEmailChanged()).isFalse(); + assertThat(userCaptor.getValue().isScmAccountsChanged()).isTrue(); + assertThat(userCaptor.getValue().isPasswordChanged()).isFalse(); + } + + @Test + public void update_only_password() throws Exception { + Map params = newHashMap(); + params.put("login", "john"); + params.put("password", "1234"); + params.put("password_confirmation", "1234"); + service.update(params); + + ArgumentCaptor userCaptor = ArgumentCaptor.forClass(UpdateUser.class); + verify(userService).update(userCaptor.capture()); + assertThat(userCaptor.getValue().isNameChanged()).isFalse(); + assertThat(userCaptor.getValue().isEmailChanged()).isFalse(); + assertThat(userCaptor.getValue().isScmAccountsChanged()).isFalse(); + assertThat(userCaptor.getValue().isPasswordChanged()).isTrue(); } @Test diff --git a/server/sonar-web/src/main/js/application.js b/server/sonar-web/src/main/js/application.js index 629f45e7612..60b31242d86 100644 --- a/server/sonar-web/src/main/js/application.js +++ b/server/sonar-web/src/main/js/application.js @@ -90,7 +90,7 @@ Treemap.prototype.load = function () { $j.ajax({ type: 'GET', url: baseUrl + '/treemap/index?html_id=' + this.id + '&size_metric=' + this.sizeMetric + - '&color_metric=' + this.colorMetric + '&resource=' + context.rid, + '&color_metric=' + this.colorMetric + '&resource=' + context.rid, dataType: 'html', success: function (data) { if (data.length > 1) { @@ -133,12 +133,12 @@ Treemap.prototype.initNodes = function () { return false; }); $j(this).on('click', function () { - var source = $j(this); - var rid = source.attr('rid'); - var context = new TreemapContext(rid, source.text()); - self.breadcrumb.push(context); - self.load(); - } + var source = $j(this); + var rid = source.attr('rid'); + var context = new TreemapContext(rid, source.text()); + self.breadcrumb.push(context); + self.load(); + } ); }); }; @@ -149,35 +149,35 @@ function openModalWindow(url, options) { if (!$dialog.length) { $dialog = $j('').appendTo('body'); } - $j.get(url,function (html) { + $j.get(url, function (html) { $dialog.removeClass('ui-widget-overlay'); $dialog.html(html); $dialog - .dialog({ - dialogClass: 'no-close', - width: width, - draggable: false, - autoOpen: false, - modal: true, - minHeight: 50, - resizable: false, - title: null, - close: function () { - $j('#modal').remove(); - } - }); + .dialog({ + dialogClass: 'no-close', + width: width, + draggable: false, + autoOpen: false, + modal: true, + minHeight: 50, + resizable: false, + title: null, + close: function () { + $j('#modal').remove(); + } + }); $dialog.dialog('open'); }).fail(function () { - alert('Server error. Please contact your administrator.'); - }).always(function () { - $dialog.removeClass('ui-widget-overlay'); - }); + alert('Server error. Please contact your administrator.'); + }).always(function () { + $dialog.removeClass('ui-widget-overlay'); + }); return false; } (function ($j) { $j.fn.extend({ - openModal: function() { + openModal: function () { return this.each(function () { var obj = $j(this); var url = obj.attr('modal-url') || obj.attr('href'); @@ -215,7 +215,7 @@ function openModalWindow(url, options) { // Re activate submit button $j('input[type=submit]', obj).removeAttr('disabled'); errorElt.show(); - errorElt.html(xhr.responseText); + errorElt.html($j("
").html(xhr.responseText).text()); } else { // otherwise replace modal window by the returned text $j('#modal').html(xhr.responseText); @@ -246,17 +246,17 @@ function supportsHTML5Storage() { function openAccordionItem(url) { return $j.ajax({ - url: url - }).fail(function (jqXHR, textStatus) { - var error = 'Server error. Please contact your administrator. The status of the error is : ' + - jqXHR.status + ', textStatus is : ' + textStatus; - console.log(error); - $j('#accordion-panel').append($j('
').append(error)); - }).done(function (html) { - var panel = $j('#accordion-panel'); - panel.html(html); - panel.scrollIntoView(false); - }); + url: url + }).fail(function (jqXHR, textStatus) { + var error = 'Server error. Please contact your administrator. The status of the error is : ' + + jqXHR.status + ', textStatus is : ' + textStatus; + console.log(error); + $j('#accordion-panel').append($j('
').append(error)); + }).done(function (html) { + var panel = $j('#accordion-panel'); + panel.html(html); + panel.scrollIntoView(false); + }); } @@ -311,12 +311,12 @@ function showDropdownMenuOnElement(elt) { //******************* HANDLING OF DROPDOWN MENUS [END] ******************* // function openPopup(url, popupId) { - window.open(url,popupId,'height=800,width=900,scrollbars=1,resizable=1'); + window.open(url, popupId, 'height=800,width=900,scrollbars=1,resizable=1'); return false; } -jQuery(function() { +jQuery(function () { // Initialize top search jQuery('#searchInput').topSearch({ @@ -327,7 +327,7 @@ jQuery(function() { // Process login link in order to add the anchor - jQuery('#login-link').on('click', function(e) { + jQuery('#login-link').on('click', function (e) { e.preventDefault(); var href = jQuery(this).prop('href'), hash = window.location.hash; @@ -339,11 +339,11 @@ jQuery(function() { // Define global shortcuts - key('s', function() { + key('s', function () { jQuery('#searchInput').focus().on('keydown', function (e) { - if (e.keyCode === 27) { - jQuery('#searchInput').blur(); - } + if (e.keyCode === 27) { + jQuery('#searchInput').blur(); + } }); return false; }); diff --git a/server/sonar-web/src/main/webapp/WEB-INF/app/controllers/api/users_controller.rb b/server/sonar-web/src/main/webapp/WEB-INF/app/controllers/api/users_controller.rb index 7d8faa99125..11353ded147 100644 --- a/server/sonar-web/src/main/webapp/WEB-INF/app/controllers/api/users_controller.rb +++ b/server/sonar-web/src/main/webapp/WEB-INF/app/controllers/api/users_controller.rb @@ -33,8 +33,8 @@ class Api::UsersController < Api::ApiController select2_format=(params[:f]=='s2') if select2_format hash = { - :more => false, - :results => users.map { |user| {:id => user.login, :text => "#{user.name} (#{user.login})"} } + :more => false, + :results => users.map { |user| {:id => user.login, :text => "#{user.name} (#{user.login})"} } } else hash = {:users => users.map { |user| User.to_hash(user) }} @@ -46,86 +46,6 @@ class Api::UsersController < Api::ApiController end end - # - # POST /api/users/create - # - # -- Mandatory parameters - # 'login' is the user identifier - # 'name' is the user display name - # 'password' is the user password - # 'password_confirmation' is the confirmed user password - # - # -- Optional parameters - # 'email' is the user email - # - # -- Example - # curl -X POST -v -u admin:admin 'http://localhost:9000/api/users/create?login=user&name=user_name&password=user_pw&password_confirmation=user_pw' - # - # since SonarQube 3.7 - # SonarQube 3.7.1 update : name is now mandatory - # - def create - verify_post_request - access_denied unless has_role?(:admin) - require_parameters :login, :name, :password, :password_confirmation - - user = User.find_by_login(params[:login]) - - if user && user.active - render_bad_request('An active user with this login already exists') - else - if user - user.reactivate!(java_facade.getSettings().getString('sonar.defaultGroup')) - user.notify_creation_handlers - else - user = prepare_user - user.save! - user.notify_creation_handlers - end - Internal.users_api.index() - hash = user.to_hash - respond_to do |format| - format.json { render :json => jsonp(hash) } - format.xml { render :xml => hash.to_xml(:skip_types => true, :root => 'users') } - end - end - end - - # - # POST /api/users/update - # - # -- Mandatory parameters - # 'login' is the user identifier - # - # -- Optional parameters - # 'name' is the user display name - # 'email' is the user email - # - # -- Example - # curl -X POST -v -u admin:admin 'http://localhost:9000/api/users/update?login=user&email=new_email' - # - # since 3.7 - # - def update - verify_post_request - access_denied unless has_role?(:admin) - require_parameters :login - - user = User.find_active_by_login(params[:login]) - - if user.nil? - render_bad_request("Could not find user with login #{params[:login]}") - elsif user.update_attributes!(params) - Internal.users_api.index() - hash = user.to_hash - respond_to do |format| - format.json { render :json => jsonp(hash) } - format.xml { render :xml => hash.to_xml(:skip_types => true, :root => 'users') } - end - end - end - - # # POST /api/users/deactivate # @@ -150,15 +70,4 @@ class Api::UsersController < Api::ApiController end end - - private - - def prepare_user - user = User.new(params) - default_group_name=java_facade.getSettings().getString('sonar.defaultGroup') - default_group=Group.find_by_name(default_group_name) - user.groups< 'users/reactivate_form', :status => 400 - else - # case user: exist,inactive, WITH ERRORS when update - @user = user - @user.id = nil - user.errors.full_messages.each { |msg| @errors< 'users/create_form', :status => 400 - end - else - user=prepare_user - if user.save - # case user: don't exist, no errors when create - Internal.users_api.index() - user.notify_creation_handlers + + call_backend do + isUserReactivated = Internal.users_api.create(params[:user]) + if !isUserReactivated flash[:notice] = 'User is created.' - render :text => 'ok', :status => 200 else - # case user: don't exist, WITH ERRORS when create - @user = user - user.errors.full_messages.each { |msg| @errors< 'users/create_form', :status => 400 + flash[:notice] = Api::Utils.message('user.reactivated', :params => params[:user][:login]) end + render :text => 'ok', :status => 200 end end @@ -92,15 +72,6 @@ class UsersController < ApplicationController render :partial => 'users/create_form' end - def reactivate_form - if params[:id] - @user = User.find(params[:id]) - else - @user = User.new - end - render :partial => 'users/reactivate_form' - end - def new render :action => 'new', :layout => 'nonav' end @@ -132,33 +103,10 @@ class UsersController < ApplicationController end def update - user = User.find(params[:id]) - @user = user - @errors = [] - if user.login!=params[:user][:login] - @errors = 'Login can not be changed.' - render :partial => 'users/edit_form', :status => 400 - elsif user.update_attributes(params[:user]) - Internal.users_api.index() + call_backend do + Internal.users_api.update(params[:user]) flash[:notice] = 'User was successfully updated.' render :text => 'ok', :status => 200 - else - @errors = user.errors.full_messages.join("
\n") - render :partial => 'users/edit_form', :status => 400 - end - end - - def reactivate - user = User.find_by_login(params[:user][:login]) - if user - user.reactivate!(java_facade.getSettings().getString('sonar.defaultGroup')) - Internal.users_api.index() - user.notify_creation_handlers - flash[:notice] = 'User was successfully reactivated.' - render :text => 'ok', :status => 200 - else - flash[:error] = "A user with login #{params[:user][:login]} does not exist." - render :text => 'login unknown', :status => 200 end end diff --git a/server/sonar-web/src/main/webapp/WEB-INF/app/views/users/_create_form.html.erb b/server/sonar-web/src/main/webapp/WEB-INF/app/views/users/_create_form.html.erb index e1caad17b23..140d52a7d91 100644 --- a/server/sonar-web/src/main/webapp/WEB-INF/app/views/users/_create_form.html.erb +++ b/server/sonar-web/src/main/webapp/WEB-INF/app/views/users/_create_form.html.erb @@ -1,16 +1,10 @@ -<% form_for :user, @user, :url => { :id => @user.id, :action => 'create' }, :html => { :id =>'user_create_form', :method => @user.id.nil? ? :post : :put} do |f| %> +<% form_for :user, @user, :url => {:id => @user.id, :action => 'create'}, :html => {:id => 'user_create_form', :method => @user.id.nil? ? :post : :put} do |f| %>

Add new user

- <% if @errors - @errors.each do |error| - %> -

<%= h error -%>

- <% end - end - %> +
<% if @user.id %> <%= @user.login %> @@ -20,19 +14,20 @@ <% end %>
- - <%= f.text_field :name, :size => 30, :maxLength => 200 %>
+ + <%= f.text_field :name, :size => 30, :maxLength => 200 %>
- - <%= f.text_field :email, :size => 30, :maxLength => 100 %>
-
<%= f.password_field :password, :size => 30, :maxLength => 50, :autocomplete => 'off' %>
-
<%= f.password_field :password_confirmation, :size => 30, :maxLength => 50, :autocomplete => 'off' %>
- + + <%= f.text_field :email, :size => 30, :maxLength => 100 %>
+
+ <%= f.password_field :password, :size => 30, :maxLength => 50, :autocomplete => 'off' %>
+
<%= f.password_field :password_confirmation, :size => 30, :maxLength => 50, :autocomplete => 'off' %>
+
+
+ <%= submit_tag 'Create' %> + <%= link_to 'Cancel', {:controller => 'users', :action => 'index'}, {:class => 'action'} %>
-
- <%= submit_tag 'Create' %> - <%= link_to 'Cancel', { :controller => 'users', :action => 'index'}, { :class => 'action' } %>
-
<% end %> diff --git a/server/sonar-web/src/main/webapp/WEB-INF/app/views/users/_edit_form.html.erb b/server/sonar-web/src/main/webapp/WEB-INF/app/views/users/_edit_form.html.erb index d2d963d086b..54144e89ac8 100644 --- a/server/sonar-web/src/main/webapp/WEB-INF/app/views/users/_edit_form.html.erb +++ b/server/sonar-web/src/main/webapp/WEB-INF/app/views/users/_edit_form.html.erb @@ -4,13 +4,7 @@

Edit user: <%= h @user.login -%>

- <% if @errors - @errors.each do |error| - %> -

<%= h error -%>

- <% end - end - %> +
<% if @user.id %> <%= f.hidden_field :login %> diff --git a/server/sonar-web/src/main/webapp/WEB-INF/app/views/users/_reactivate_form.html.erb b/server/sonar-web/src/main/webapp/WEB-INF/app/views/users/_reactivate_form.html.erb deleted file mode 100644 index 15ddfd9f6ce..00000000000 --- a/server/sonar-web/src/main/webapp/WEB-INF/app/views/users/_reactivate_form.html.erb +++ /dev/null @@ -1,24 +0,0 @@ -<% form_for :user, @user, :url => { :id => @user.id, :action => 'reactivate' }, :html => { :id =>'user_form', :method => @user.id.nil? ? :post : :put} do |f| %> -
-
-

Reactivate user?

-
-
- <%= f.hidden_field :login %> - -

- A user with login "<%= h @user.login -%>" already exists in the database but is deactivated.
-
- Do you really want to reactivate this user? -

-
-
- <%= submit_tag 'Reactivate user' %> - <%= link_to 'Cancel', { :controller => 'users', :action => 'index'}, { :class => 'action' } %>
-
-
- - -<% end %> -- 2.39.5