]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-5963 Automatically reactivate user when using a disabled login
authorJulien Lancelot <julien.lancelot@sonarsource.com>
Thu, 18 Dec 2014 16:11:19 +0000 (17:11 +0100)
committerJulien Lancelot <julien.lancelot@sonarsource.com>
Thu, 18 Dec 2014 17:05:07 +0000 (18:05 +0100)
server/sonar-server/src/main/java/org/sonar/server/user/DefaultUserService.java
server/sonar-server/src/test/java/org/sonar/server/user/DefaultUserServiceTest.java
server/sonar-web/src/main/js/application.js
server/sonar-web/src/main/webapp/WEB-INF/app/controllers/api/users_controller.rb
server/sonar-web/src/main/webapp/WEB-INF/app/controllers/users_controller.rb
server/sonar-web/src/main/webapp/WEB-INF/app/views/users/_create_form.html.erb
server/sonar-web/src/main/webapp/WEB-INF/app/views/users/_edit_form.html.erb
server/sonar-web/src/main/webapp/WEB-INF/app/views/users/_reactivate_form.html.erb [deleted file]

index df1cf9691c7ad3eae32d579f6466bd11428427d8..7de63a1792089b28de1f864df744faf00309452a 100644 (file)
@@ -82,7 +82,7 @@ public class DefaultUserService implements RubyUserService {
     userService.index();
   }
 
-  public void create(Map<String, Object> params) {
+  public boolean create(Map<String, Object> 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<String, Object> 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);
   }
 
index 4d87662e4e9ccf6bbf51edb2583a012538ac893d..b8d17b96fa65d44f00c05f91773d33354de4e42b 100644 (file)
@@ -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<UpdateUser> 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<String, Object> params = newHashMap();
+    params.put("login", "john");
+    params.put("name", "John");
+    service.update(params);
+
+    ArgumentCaptor<UpdateUser> 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<String, Object> params = newHashMap();
+    params.put("login", "john");
+    params.put("email", "john@email.com");
+    service.update(params);
+
+    ArgumentCaptor<UpdateUser> 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<String, Object> params = newHashMap();
+    params.put("login", "john");
+    params.put("scm_accounts", newArrayList("jn"));
+    service.update(params);
+
+    ArgumentCaptor<UpdateUser> 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<String, Object> params = newHashMap();
+    params.put("login", "john");
+    params.put("password", "1234");
+    params.put("password_confirmation", "1234");
+    service.update(params);
+
+    ArgumentCaptor<UpdateUser> 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
index 629f45e7612420a9bfb7d5b7807dd38374613e1b..60b31242d864ffd05d5f8418d70c91fcb0707202 100644 (file)
@@ -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('<div id="modal" class="ui-widget-overlay ui-front"></div>').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("<div/>").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('<div class="error">').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('<div class="error">').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;
   });
index 7d8faa991256a4e646b6c0f7a8f015d9acb3a393..11353ded147d160b39ecf02f7f420966db89a200 100644 (file)
@@ -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<<default_group if default_group
-    user
-  end
-
 end
index 6b7bc5504b57e596bb7afd1df873bc99981a088a..948aa8bcbbf44e2825c56469dd3ea011c0fa1b91 100644 (file)
@@ -27,35 +27,15 @@ class UsersController < ApplicationController
   def create
     return unless request.post?
     cookies.delete :auth_token
-    @errors = []
-    user = User.find_by_login(params[:user][:login])
-    if user && !user.active
-      if user.update_attributes(params[:user])
-        # case user: exist,inactive,no errors when update BUT TO REACTIVATE
-        @user = user
-        user.errors.full_messages.each { |msg| @errors<<msg }
-        render :partial => '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<<msg }
-        render :partial => '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<<msg }
-        render :partial => '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("<br/>\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
 
index e1caad17b23c54d305c3551eb66e575ad577e807..140d52a7d91b5dc38b7ba527c2731a7f1d2ef12a 100644 (file)
@@ -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| %>
   <fieldset>
     <div class="modal-head">
       <h2>Add new user</h2>
     </div>
     <div class="modal-body">
-      <% if @errors
-           @errors.each do |error|
-      %>
-          <p class="error"><%= h error -%></p>
-      <% end
-         end
-      %>
+      <div class="modal-error"/>
       <div class="modal-field"><label for="user[login]">Login <em class="mandatory">*</em></label>
         <% if @user.id %>
           <%= @user.login %>
         <% end %>
       </div>
       <div class="modal-field">
-          <label for="user[]">Name <em class="mandatory">*</em></label>
-          <%= f.text_field :name, :size => 30, :maxLength => 200 %></div>
+        <label for="user[]">Name <em class="mandatory">*</em></label>
+        <%= f.text_field :name, :size => 30, :maxLength => 200 %></div>
       <div class="modal-field">
-          <label for="user[]">Email</label>
-          <%= f.text_field :email, :size => 30, :maxLength => 100 %></div>
-      <div class="modal-field"><label for="user[password]">Password <em class="mandatory">*</em></label><%= f.password_field :password, :size => 30, :maxLength => 50, :autocomplete => 'off' %></div>
-      <div class="modal-field"><label for="user[password_confirmation]">Confirm password <em class="mandatory">*</em></label><%= f.password_field :password_confirmation, :size => 30, :maxLength => 50, :autocomplete => 'off' %></div>
-
+        <label for="user[]">Email</label>
+        <%= f.text_field :email, :size => 30, :maxLength => 100 %></div>
+      <div class="modal-field">
+        <label for="user[password]">Password <em class="mandatory">*</em></label><%= f.password_field :password, :size => 30, :maxLength => 50, :autocomplete => 'off' %></div>
+      <div class="modal-field"><label for="user[password_confirmation]">Confirm password
+        <em class="mandatory">*</em></label><%= f.password_field :password_confirmation, :size => 30, :maxLength => 50, :autocomplete => 'off' %></div>
+    </div>
+    <div class="modal-foot">
+      <%= submit_tag 'Create' %>
+      <%= link_to 'Cancel', {:controller => 'users', :action => 'index'}, {:class => 'action'} %><br/>
     </div>
-      <div class="modal-foot">
-          <%= submit_tag 'Create' %>
-          <%= link_to 'Cancel', { :controller => 'users', :action => 'index'}, { :class => 'action' } %><br/>
-      </div>
   </fieldset>
 <% end %>
 
index d2d963d086b59b1c9276469139f5eb752739bc1e..54144e89ac87d99b38cbb3a02d14fb207370b74d 100644 (file)
@@ -4,13 +4,7 @@
           <h2>Edit user: <%= h @user.login -%></h2>
       </div>
       <div class="modal-body">
-        <% if @errors
-             @errors.each do |error|
-        %>
-            <p class="error"><%= h error -%></p>
-          <% end
-             end
-          %>
+        <div class="modal-error"></div>
         <div class="modal-field">
           <% 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 (file)
index 15ddfd9..0000000
+++ /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| %>
-  <fieldset>
-    <div class="modal-head">
-        <h2>Reactivate user?</h2>
-    </div>
-    <div class="modal-body">
-      <%= f.hidden_field :login %>
-
-              <p class="error">
-                  A user with login "<%= h @user.login -%>" already exists in the database but is deactivated.<br/>
-                  <br/>
-                  Do you really want to reactivate this user?
-              </p>
-    </div>
-    <div class="modal-foot">
-           <%= submit_tag 'Reactivate user' %>
-           <%= link_to 'Cancel', { :controller => 'users', :action => 'index'}, { :class => 'action' } %><br/>
-    </div>
-  </fieldset>
-
-  <script>
-    $j("#user_form").modalForm();
-  </script>
-<% end %>