Do not self-activate, or show user agreements to, an uninvited user:

i.e., one who is neither invited by default according to site policy,
nor a member of the "All users" group.

refs #1706

diff --git a/services/api/app/controllers/arvados/v1/user_agreements_controller.rb b/services/api/app/controllers/arvados/v1/user_agreements_controller.rb
index ac74f3d8d2fc98c4109324751444de9896b33e77..c79e4722ef7bcb0ec1e1bca6b04d50214e0ea0fa 100644 (file)
--- a/services/api/app/controllers/arvados/v1/user_agreements_controller.rb
+++ b/services/api/app/controllers/arvados/v1/user_agreements_controller.rb
@@ -6,16 +6,22 @@ class Arvados::V1::UserAgreementsController < ApplicationController
   end
 
   def index
-    current_user_uuid = current_user.uuid
-    act_as_system_user do
-      uuids = Link.where(owner_uuid: system_user_uuid,
-                         link_class: 'signature',
-                         name: 'require',
-                         tail_kind: 'arvados#user',
-                         tail_uuid: system_user_uuid,
-                         head_kind: 'arvados#collection').
-        collect &:head_uuid
-      @objects = Collection.where('uuid in (?)', uuids)
+    if not current_user.is_invited
+      # New users cannot see user agreements until/unless invited to
+      # use this installation.
+      @objects = []
+    else
+      current_user_uuid = current_user.uuid
+      act_as_system_user do
+        uuids = Link.where(owner_uuid: system_user_uuid,
+                           link_class: 'signature',
+                           name: 'require',
+                           tail_kind: 'arvados#user',
+                           tail_uuid: system_user_uuid,
+                           head_kind: 'arvados#collection').
+          collect &:head_uuid
+        @objects = Collection.where('uuid in (?)', uuids)
+      end
     end
     @response_resource_name = 'collection'
     super

diff --git a/services/api/app/controllers/arvados/v1/users_controller.rb b/services/api/app/controllers/arvados/v1/users_controller.rb
index 5498619729b6cbd1a5779399ea7c28aecdbff9eb..441db9947e88480f5bc4de3968f889164601302b 100644 (file)
--- a/services/api/app/controllers/arvados/v1/users_controller.rb
+++ b/services/api/app/controllers/arvados/v1/users_controller.rb
@@ -43,12 +43,16 @@ class Arvados::V1::UsersController < ApplicationController
 
   def activate
     if current_user.andand.is_admin && params[:uuid]
-      @user = User.find params[:uuid]
+      @object = User.find params[:uuid]
     else
-      @user = current_user
+      @object = current_user
     end
-    if not @user.is_active
-      target_user_uuid = @user.uuid
+    if not @object.is_active
+      if not (current_user.is_admin or @object.is_invited)
+        logger.warn "User #{@object.uuid} called users.activate " +
+          "but is not invited"
+        raise ArgumentError.new "Cannot activate without being invited."
+      end
       act_as_system_user do
         required_uuids = Link.where(owner_uuid: system_user_uuid,
                                     link_class: 'signature',
@@ -60,23 +64,22 @@ class Arvados::V1::UsersController < ApplicationController
                                   link_class: 'signature',
                                   name: 'click',
                                   tail_kind: 'arvados#user',
-                                  tail_uuid: target_user_uuid,
+                                  tail_uuid: @object.uuid,
                                   head_kind: 'arvados#collection',
                                   head_uuid: required_uuids).
           collect(&:head_uuid)
         todo_uuids = required_uuids - signed_uuids
         if todo_uuids == []
-          @user.update_attributes is_active: true
-          logger.info "User #{@user.uuid} activated"
+          @object.update_attributes is_active: true
+          logger.info "User #{@object.uuid} activated"
         else
-          logger.warn "User #{@user.uuid} called users.activate " +
+          logger.warn "User #{@object.uuid} called users.activate " +
             "before signing agreements #{todo_uuids.inspect}"
           raise ArgumentError.new \
           "Cannot activate without user agreements #{todo_uuids.inspect}."
         end
       end
     end
-    @object = @user
     show
   end
 end

diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index e628edaa3fab292e21ade5c5f24903001aa54fb7..0364c08f5024060c2e3584d7c7047111914d8156 100644 (file)
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -19,6 +19,7 @@ class User < ArvadosModel
     t.add :identity_url
     t.add :is_active
     t.add :is_admin
+    t.add :is_invited
     t.add :prefs
   end
 
@@ -28,6 +29,12 @@ class User < ArvadosModel
     "#{first_name} #{last_name}"
   end
 
+  def is_invited
+    (self.is_active ||
+     Rails.configuration.new_users_are_active ||
+     self.groups_i_can(:read).select { |x| x.match /-f+$/ }.first)
+  end
+
   def groups_i_can(verb)
     self.group_permissions.select { |uuid, mask| mask[verb] }.keys
   end

diff --git a/services/api/test/fixtures/api_client_authorizations.yml b/services/api/test/fixtures/api_client_authorizations.yml
index 7effb2f534378bc15463888494cc25482b6349bc..94dabf99cfebd4a9359f92e2b37b573417857ad2 100644 (file)
--- a/services/api/test/fixtures/api_client_authorizations.yml
+++ b/services/api/test/fixtures/api_client_authorizations.yml
@@ -30,6 +30,12 @@ inactive:
   api_token: 5s29oj2hzmcmpq80hx9cta0rl5wuf3xfd6r7disusaptz7h9m0
   expires_at: 2038-01-01 00:00:00
 
+inactive_uninvited:
+  api_client: untrusted
+  user: inactive_uninvited
+  api_token: 62mhllc0otp78v08e3rpa3nsmf8q8ogk47f7u5z4erp5gpj9al
+  expires_at: 2038-01-01 00:00:00
+
 inactive_but_signed_user_agreement:
   api_client: untrusted
   user: inactive_but_signed_user_agreement

diff --git a/services/api/test/fixtures/groups.yml b/services/api/test/fixtures/groups.yml
index ebf22341a6febe7d2052083e868a0983b0debc1f..c9b52dcfc7c3337d169fd604413135ec366a7a4b 100644 (file)
--- a/services/api/test/fixtures/groups.yml
+++ b/services/api/test/fixtures/groups.yml
@@ -3,3 +3,8 @@ public:
   owner_uuid: zzzzz-tpzed-d9tiejq69daie8f
   name: Public
   description: Public Group
+
+all_users:
+  uuid: zzzzz-j7d0g-fffffffffffffff
+  owner_uuid: zzzzz-tpzed-d9tiejq69daie8f
+  name: All users

diff --git a/services/api/test/fixtures/links.yml b/services/api/test/fixtures/links.yml
index 4d5bfe5bcb4be842dca125e5a6a56047b623800f..e871c9bcc1b9cd2e70f9ff2e49945971d0f812fc 100644 (file)
--- a/services/api/test/fixtures/links.yml
+++ b/services/api/test/fixtures/links.yml
@@ -45,3 +45,35 @@ user_agreement_signed_by_inactive:
   head_kind: arvados#collection
   head_uuid: b519d9cb706a29fc7ea24dbea2f05851
   properties: {}
+
+inactive_user_member_of_all_users_group:
+  uuid: zzzzz-o0j2j-osckxpy5hl5fjk5
+  owner_uuid: zzzzz-tpzed-000000000000000
+  created_at: 2013-12-26T20:52:21Z
+  modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
+  modified_by_user_uuid: zzzzz-tpzed-7sg468ezxwnodxs
+  modified_at: 2013-12-26T20:52:21Z
+  updated_at: 2013-12-26T20:52:21Z
+  tail_kind: arvados#user
+  tail_uuid: zzzzz-tpzed-x9kqpd79egh49c7
+  link_class: permission
+  name: can_read
+  head_kind: arvados#group
+  head_uuid: zzzzz-j7d0g-fffffffffffffff
+  properties: {}
+
+inactive_signed_ua_user_member_of_all_users_group:
+  uuid: zzzzz-o0j2j-qkhyjcr6tidk652
+  owner_uuid: zzzzz-tpzed-000000000000000
+  created_at: 2013-12-26T20:52:21Z
+  modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
+  modified_by_user_uuid: zzzzz-tpzed-7sg468ezxwnodxs
+  modified_at: 2013-12-26T20:52:21Z
+  updated_at: 2013-12-26T20:52:21Z
+  tail_kind: arvados#user
+  tail_uuid: zzzzz-tpzed-7sg468ezxwnodxs
+  link_class: permission
+  name: can_read
+  head_kind: arvados#group
+  head_uuid: zzzzz-j7d0g-fffffffffffffff
+  properties: {}

diff --git a/services/api/test/fixtures/users.yml b/services/api/test/fixtures/users.yml
index c6f25d641b8e8ebb2b75e6d805e241163ac326f2..ab43907da00a62cac8089aa18b6cfd45535c48d8 100644 (file)
--- a/services/api/test/fixtures/users.yml
+++ b/services/api/test/fixtures/users.yml
@@ -20,6 +20,16 @@ active:
   is_admin: false
   prefs: {}
 
+inactive_uninvited:
+  uuid: zzzzz-tpzed-rf2ec3ryh4vb5ma
+  email: inactive-uninvited-user@arvados.local
+  first_name: Inactive and Uninvited
+  last_name: User
+  identity_url: https://inactive-uninvited-user.openid.local
+  is_active: false
+  is_admin: false
+  prefs: {}
+
 inactive:
   uuid: zzzzz-tpzed-x9kqpd79egh49c7
   email: inactive-user@arvados.local

diff --git a/services/api/test/functional/arvados/v1/user_agreements_controller_test.rb b/services/api/test/functional/arvados/v1/user_agreements_controller_test.rb
index 41c81cb87f50c0869fe8380a0fe551a94fc172d0..05bdef56a0b90d73a59dca1aaeb4b43ec0839f7a 100644 (file)
--- a/services/api/test/functional/arvados/v1/user_agreements_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/user_agreements_controller_test.rb
@@ -33,4 +33,14 @@ class Arvados::V1::UserAgreementsControllerTest < ActionController::TestCase
     assert_not_nil agreements_list['items'][0]
   end
 
+  test "uninvited user receives empty list of user agreements" do
+    authorize_with :inactive_uninvited
+    get :index
+    assert_response :success
+    assert_not_nil assigns(:objects)
+    agreements_list = JSON.parse(@response.body)
+    assert_not_nil agreements_list['items']
+    assert_nil agreements_list['items'][0]
+  end
+
 end

diff --git a/services/api/test/test_helper.rb b/services/api/test/test_helper.rb
index 9955a3500ffe9830c41974679954e6956c6f4f21..8c12ffb5df4718df4c5d19dbd5eae5d109da764a 100644 (file)
--- a/services/api/test/test_helper.rb
+++ b/services/api/test/test_helper.rb
@@ -19,3 +19,6 @@ class ActiveSupport::TestCase
 
   # Add more helper methods to be used by all tests here...
 end
+
+# Ensure permissions are computed from the test fixtures.
+User.invalidate_permissions_cache