12995: Don't let admins lose admin privs by linking a non-admin account.

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz@veritasgenetics.com>

diff --git a/apps/workbench/app/views/users/link_account.html.erb b/apps/workbench/app/views/users/link_account.html.erb
index 84a1e137abe0559ebae070b2020b9fae6385c4d5..a0fb41b2b6af60defb8d9c8070a4bce2edcc3922 100644 (file)
--- a/apps/workbench/app/views/users/link_account.html.erb
+++ b/apps/workbench/app/views/users/link_account.html.erb
@@ -9,32 +9,30 @@
       <% if params[:direction] == "in" %>
       var user_a = "<b>"+sessionStorage.getItem('link_account_email')+"</b> ("+sessionStorage.getItem('link_account_uuid')+")";
       var user_b = "<b><%= Thread.current[:user].email %></b> (<%= Thread.current[:user].uuid%>)";
-      var user_a_is_active = sessionStorage.getItem('link_account_is_active');
-      var user_a_is_admin = sessionStorage.getItem('link_account_is_admin');
-      var user_b_is_admin = <%=if Thread.current[:user].is_admin then "true" else "false"%>;
+      var user_a_is_active = (sessionStorage.getItem('link_account_is_active') == "true");
+      var user_a_is_admin = (sessionStorage.getItem('link_account_is_admin') == "true");
+      var user_b_is_admin = <%=if Thread.current[:user].is_admin then "true" else "false" end %>;
       <% else %>
       var user_a = "<b><%= Thread.current[:user].email %></b> (<%= Thread.current[:user].uuid%>)";
       var user_b = "<b>"+sessionStorage.getItem('link_account_email')+"</b> ("+sessionStorage.getItem('link_account_uuid')+")";
       var user_a_is_active = <%= Thread.current[:user].is_active %>;
-      var user_a_is_admin = <%=if Thread.current[:user].is_admin then "true" else "false"%>;
-      var user_b_is_admin = sessionStorage.getItem('link_account_is_admin');
+      var user_a_is_admin = <%=if Thread.current[:user].is_admin then "true" else "false" end %>;
+      var user_b_is_admin = (sessionStorage.getItem('link_account_is_admin') == "true");
       <% end %>
 
       $("#new-user-token-input").val(sessionStorage.getItem('link_account_api_token'));
 
-      if (user_b_is_admin && !user_a_is_admin) {
+      if (!user_a_is_active) {
+        $("#will-link-to").html("<p>Cannot link "+user_b+" to inactive account "+user_a+".</p>");
+        $("#link-account-submit").prop("disabled", true);
+      } else if (user_b_is_admin && !user_a_is_admin) {
         $("#will-link-to").html("<p>Cannot link admin account "+user_b+" to non-admin account "+user_a+".</p>");
         $("#link-account-submit").prop("disabled", true);
       } else {
-        if (user_a_is_active) {
-          $("#will-link-to").html("<p>Clicking 'Link accounts' will link "+user_b+" created on <%=Thread.current[:user].created_at%> to "+
-            user_a+" created at <b>"+sessionStorage.getItem('link_account_created_at')+"</b>.</p>"+
-            "<p>After linking, logging in as "+user_b+" will log you into the same account as "+user_a+
-            ".</p>  <p>Any objects owned by "+user_b+" will be transferred to "+user_a+".</p>");
-        } else {
-          $("#will-link-to").html("<p>Cannot link "+user_b+" to inactive account "+user_a+".</p>");
-          $("#link-account-submit").prop("disabled", true);
-       }
+        $("#will-link-to").html("<p>Clicking 'Link accounts' will link "+user_b+" created on <%=Thread.current[:user].created_at%> to "+
+          user_a+" created at <b>"+sessionStorage.getItem('link_account_created_at')+"</b>.</p>"+
+          "<p>After linking, logging in as "+user_b+" will log you into the same account as "+user_a+
+          ".</p>  <p>Any objects owned by "+user_b+" will be transferred to "+user_a+".</p>");
       }
     } else {
       $("#ready-to-link").css({"display": "none"});
@@ -46,6 +44,7 @@
     sessionStorage.removeItem('link_account_email');
     sessionStorage.removeItem('link_account_created_at');
     sessionStorage.removeItem('link_account_is_active');
+    sessionStorage.removeItem('link_account_is_admin');
   };
 
   $(window).on("load", function() {

diff --git a/apps/workbench/test/integration/link_account_test.rb b/apps/workbench/test/integration/link_account_test.rb
index 9a543d2cb0cccb624e173538418469f58449cc55..b27542060e4f108ac29820804f54fb8a9951b0c6 100644 (file)
--- a/apps/workbench/test/integration/link_account_test.rb
+++ b/apps/workbench/test/integration/link_account_test.rb
@@ -108,6 +108,8 @@ class LinkAccountTest < ActionDispatch::IntegrationTest
 
     assert_text "Cannot link active-user@arvados.local"
 
+    assert find("#link-account-submit")['disabled']
+
     find("button", text: "Cancel").click
 
     find("#notifications-menu").click
@@ -139,4 +141,28 @@ class LinkAccountTest < ActionDispatch::IntegrationTest
     assert_text "active-user@arvados.local"
   end
 
+  test "Admin cannot link to non-admin" do
+    visit page_with_token('admin_trustedclient')
+    stub = start_sso_stub(api_fixture('api_client_authorizations')['active_trustedclient']['api_token'])
+    Rails.configuration.arvados_login_base = stub + "login"
+
+    find("#notifications-menu").click
+    assert_text "admin@arvados.local"
+
+    find("a", text: "Link account").click
+    find("button", text: "Use this login to access another account").click
+
+    find("#notifications-menu").click
+    assert_text "active-user@arvados.local"
+
+    assert_text "Cannot link admin account admin@arvados.local"
+
+    assert find("#link-account-submit")['disabled']
+
+    find("button", text: "Cancel").click
+
+    find("#notifications-menu").click
+    assert_text "admin@arvados.local"
+  end
+
 end