[ARVADOS] updated: 1.1.4-276-g076055c

Git user git at public.curoverse.com
Fri May 18 15:38:19 EDT 2018


Summary of changes:
 apps/workbench/app/controllers/users_controller.rb |   2 +-
 apps/workbench/app/models/user.rb                  |   3 +-
 .../app/views/users/link_account.html.erb          |  23 ++-
 .../test/integration/link_account_test.rb          | 168 +++++++++++++++++++++
 .../test/fixtures/api_client_authorizations.yml    |   7 +
 5 files changed, 193 insertions(+), 10 deletions(-)
 create mode 100644 apps/workbench/test/integration/link_account_test.rb

       via  076055cab16b637147d80f5bbc428434dd5b2497 (commit)
       via  6e28c722b3662f6ea769487c47ec14398e805a82 (commit)
      from  4aa2e9342254971e92b5836a56728015e9cfc714 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 076055cab16b637147d80f5bbc428434dd5b2497
Author: Peter Amstutz <pamstutz at veritasgenetics.com>
Date:   Fri May 18 15:37:23 2018 -0400

    12995: Don't let admins lose admin privs by linking a non-admin account.
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at 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 84a1e13..a0fb41b 100644
--- 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 9a543d2..b275420 100644
--- 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 at 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 at 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 at 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 at arvados.local"
+
+    assert_text "Cannot link admin account admin at arvados.local"
+
+    assert find("#link-account-submit")['disabled']
+
+    find("button", text: "Cancel").click
+
+    find("#notifications-menu").click
+    assert_text "admin at arvados.local"
+  end
+
 end

commit 6e28c722b3662f6ea769487c47ec14398e805a82
Author: Peter Amstutz <pamstutz at veritasgenetics.com>
Date:   Fri May 18 14:34:43 2018 -0400

    12995: Add testing
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>

diff --git a/apps/workbench/app/controllers/users_controller.rb b/apps/workbench/app/controllers/users_controller.rb
index d5325d4..8cfc2c1 100644
--- a/apps/workbench/app/controllers/users_controller.rb
+++ b/apps/workbench/app/controllers/users_controller.rb
@@ -5,7 +5,7 @@
 class UsersController < ApplicationController
   skip_around_filter :require_thread_api_token, only: :welcome
   skip_before_filter :check_user_agreements, only: [:welcome, :inactive, :link_account, :merge]
-  skip_before_filter :check_user_profile, only: [:welcome, :inactive, :profile]
+  skip_before_filter :check_user_profile, only: [:welcome, :inactive, :profile, :link_account, :merge]
   skip_before_filter :find_object_by_uuid, only: [:welcome, :activity, :storage]
   before_filter :ensure_current_user_is_admin, only: [:sudo, :unsetup, :setup]
 
diff --git a/apps/workbench/app/models/user.rb b/apps/workbench/app/models/user.rb
index d30b472..865ff6e 100644
--- a/apps/workbench/app/models/user.rb
+++ b/apps/workbench/app/models/user.rb
@@ -40,7 +40,8 @@ class User < ArvadosBase
     # Create a project owned by user_a to accept everything owned by user_b
     res = arvados_api_client.api Group, nil, {:group => {
                                                 :name => new_group_name,
-                                                :group_class => "project"}},
+                                                :group_class => "project"},
+                                              :ensure_unique_name => true},
                                  {:arvados_api_token => user_a}, false
     target = arvados_api_client.unpack_api_response(res)
 
diff --git a/apps/workbench/app/views/users/link_account.html.erb b/apps/workbench/app/views/users/link_account.html.erb
index 09ce588..84a1e13 100644
--- a/apps/workbench/app/views/users/link_account.html.erb
+++ b/apps/workbench/app/views/users/link_account.html.erb
@@ -10,24 +10,31 @@
       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"%>;
       <% 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_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');
       <% end %>
 
-      console.log("User a "+user_a_is_active);
-
       $("#new-user-token-input").val(sessionStorage.getItem('link_account_api_token'));
 
-      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>");
+      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);
+	}
       }
     } else {
       $("#ready-to-link").css({"display": "none"});
@@ -51,6 +58,7 @@
     sessionStorage.setItem('link_account_uuid', '<%= Thread.current[:user].uuid %>');
     sessionStorage.setItem('link_account_created_at', '<%= Thread.current[:user].created_at %>');
     sessionStorage.setItem('link_account_is_active', <%= if Thread.current[:user].is_active then "true" else "false" end %>);
+    sessionStorage.setItem('link_account_is_admin', <%= if Thread.current[:user].is_admin then "true" else "false" end %>);
     window.location.replace('<%=arvados_api_client.arvados_login_url(return_to: "#{strip_token_from_path(request.url)}?direction=")%>'+dir);
   }
 
diff --git a/apps/workbench/test/integration/link_account_test.rb b/apps/workbench/test/integration/link_account_test.rb
new file mode 100644
index 0000000..9a543d2
--- /dev/null
+++ b/apps/workbench/test/integration/link_account_test.rb
@@ -0,0 +1,142 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+require 'integration_helper'
+require 'webrick'
+
+class LinkAccountTest < ActionDispatch::IntegrationTest
+  setup do
+    need_javascript
+  end
+
+  def start_sso_stub token
+    port = available_port('sso_stub')
+
+    s = WEBrick::HTTPServer.new(
+      :Port => port,
+      :BindAddress => 'localhost',
+      :Logger => WEBrick::Log.new('/dev/null', WEBrick::BasicLog::DEBUG),
+      :AccessLog => [nil,nil]
+    )
+
+    s.mount_proc("/login"){|req, res|
+      res.set_redirect(WEBrick::HTTPStatus::TemporaryRedirect, req.query["return_to"] + "&api_token=#{token}")
+      s.shutdown
+    }
+
+    Thread.new do
+      s.start
+    end
+
+    "http://localhost:#{port}/"
+  end
+
+  test "Add another login to this account" do
+    visit page_with_token('active_trustedclient')
+    stub = start_sso_stub(api_fixture('api_client_authorizations')['project_viewer_trustedclient']['api_token'])
+    Rails.configuration.arvados_login_base = stub + "login"
+
+    find("#notifications-menu").click
+    assert_text "active-user at arvados.local"
+
+    find("a", text: "Link account").click
+    find("button", text: "Add another login to this account").click
+
+    find("#notifications-menu").click
+    assert_text "project-viewer at arvados.local"
+
+    find("button", text: "Link accounts").click
+
+    find("#notifications-menu").click
+    assert_text "active-user at arvados.local"
+  end
+
+  test "Use this login to access another account" do
+    visit page_with_token('project_viewer_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 "project-viewer at 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 at arvados.local"
+
+    find("button", text: "Link accounts").click
+
+    find("#notifications-menu").click
+    assert_text "active-user at arvados.local"
+  end
+
+  test "Link login of inactive user to this account" do
+    visit page_with_token('active_trustedclient')
+    stub = start_sso_stub(api_fixture('api_client_authorizations')['inactive_uninvited_trustedclient']['api_token'])
+    Rails.configuration.arvados_login_base = stub + "login"
+
+    find("#notifications-menu").click
+    assert_text "active-user at arvados.local"
+
+    find("a", text: "Link account").click
+    find("button", text: "Add another login to this account").click
+
+    find("#notifications-menu").click
+    assert_text "inactive-uninvited-user at arvados.local"
+
+    find("button", text: "Link accounts").click
+
+    find("#notifications-menu").click
+    assert_text "active-user at arvados.local"
+  end
+
+  test "Cannot link to inactive user" do
+    visit page_with_token('active_trustedclient')
+    stub = start_sso_stub(api_fixture('api_client_authorizations')['inactive_uninvited_trustedclient']['api_token'])
+    Rails.configuration.arvados_login_base = stub + "login"
+
+    find("#notifications-menu").click
+    assert_text "active-user at 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 "inactive-uninvited-user at arvados.local"
+
+    assert_text "Cannot link active-user at arvados.local"
+
+    find("button", text: "Cancel").click
+
+    find("#notifications-menu").click
+    assert_text "active-user at arvados.local"
+  end
+
+  test "Inactive user can link to active account" do
+    visit page_with_token('inactive_uninvited_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 "inactive-uninvited-user at arvados.local"
+
+    assert_text "Already have an account with a different login?"
+
+    find("a", text: "Link this login to your existing account").click
+
+    assert_no_text "Add another login to this account"
+
+    find("button", text: "Use this login to access another account").click
+
+    find("#notifications-menu").click
+    assert_text "active-user at arvados.local"
+
+    find("button", text: "Link accounts").click
+
+    find("#notifications-menu").click
+    assert_text "active-user at arvados.local"
+  end
+
+end
diff --git a/services/api/test/fixtures/api_client_authorizations.yml b/services/api/test/fixtures/api_client_authorizations.yml
index 8d9fc53..92bd7cf 100644
--- a/services/api/test/fixtures/api_client_authorizations.yml
+++ b/services/api/test/fixtures/api_client_authorizations.yml
@@ -183,6 +183,13 @@ inactive_uninvited:
   api_token: 62mhllc0otp78v08e3rpa3nsmf8q8ogk47f7u5z4erp5gpj9al
   expires_at: 2038-01-01 00:00:00
 
+inactive_uninvited_trustedclient:
+  uuid: zzzzz-gj3su-228z32aux8dg2s1
+  api_client: trusted_workbench
+  user: inactive_uninvited
+  api_token: 7s29oj2hzmcmpq80hx9cta0rl5wuf3xfd6r7disusaptz7h9m0
+  expires_at: 2038-01-01 00:00:00
+
 inactive_but_signed_user_agreement:
   uuid: zzzzz-gj3su-247z32aux8dg2s1
   api_client: untrusted

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list