[ARVADOS] updated: 6fc29fa2b44072bc9a1cd015282dde30a0ee72c4

git at public.curoverse.com git at public.curoverse.com
Sat Mar 22 23:45:02 EDT 2014


Summary of changes:
 .../app/controllers/arvados/v1/users_controller.rb |   15 +++++++++---
 services/api/app/models/user.rb                    |   23 ++++++++-----------
 services/api/config/application.default.yml        |    3 --
 services/api/script/setup-new-user.rb              |   19 ++++++----------
 .../functional/arvados/v1/users_controller_test.rb |    7 ++---
 services/api/test/unit/user_test.rb                |   22 +++++++++++++++++++
 6 files changed, 53 insertions(+), 36 deletions(-)

       via  6fc29fa2b44072bc9a1cd015282dde30a0ee72c4 (commit)
       via  ee1abbc5a99dc2fa67de7713107ccd5473bd94a5 (commit)
       via  1896a42e2da493b024dce40266a4814883c08003 (commit)
      from  65a085e1a2812e48a6f4b21d5229430549fb8791 (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 6fc29fa2b44072bc9a1cd015282dde30a0ee72c4
Author: radhika chippada <radhika at radhika.curoverse>
Date:   Sat Mar 22 23:44:04 2014 -0400

    Repo name and VM are optional to the setup method

diff --git a/services/api/app/controllers/arvados/v1/users_controller.rb b/services/api/app/controllers/arvados/v1/users_controller.rb
index 226c0a7..c3b8ec9 100644
--- a/services/api/app/controllers/arvados/v1/users_controller.rb
+++ b/services/api/app/controllers/arvados/v1/users_controller.rb
@@ -96,11 +96,11 @@ class Arvados::V1::UsersController < ApplicationController
         return render_404_if_no_object
       end
     else
-      @object = model_class.create! resource_attrs
+      @object = model_class.new resource_attrs
     end
 
-    @object = User.setup @object, params[:repo_name], params[:vm_uuid], 
-        params[:openid_prefix]
+    @object = User.setup @object, params[:openid_prefix], 
+                params[:repo_name], params[:vm_uuid]
 
     show
   end
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index 6fdf95d..90d18e8 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -109,9 +109,8 @@ class User < ArvadosModel
     end
   end
 
-  def self.setup(user, repo_name=nil, vm_uuid=nil, openid_prefix=nil)
+  def self.setup(user, openid_prefix, repo_name=nil, vm_uuid=nil)
     login_perm_props = {identity_url_prefix: openid_prefix}
-
     if user[:uuid]
       found = User.find_by_uuid user[:uuid]
     end
@@ -120,13 +119,14 @@ class User < ArvadosModel
       if !user[:email]
         raise "No email found in the input. Aborting user creation."
       end
+
       if user.save
         oid_login_perm = Link.where(tail_uuid: user[:email],
                                     head_kind: 'arvados#user',
                                     link_class: 'permission',
                                     name: 'can_login')
 
-        if [] == oid_login_perm
+        if !oid_login_perm.any?
           # create openid login permission
           oid_login_perm = Link.create(link_class: 'permission',
                                        name: 'can_login',
@@ -232,7 +232,8 @@ class User < ArvadosModel
     end
 
     # Check for an existing repository with the same name we're about to use.
-    repo = (repos = Repository.where(name: repo_name)) != nil ? repos.first : nil
+    repo = Repository.where(name: repo_name).first
+
     if repo
       logger.warn "Repository exists for #{repo_name}: #{repo[:uuid]}."
 
@@ -242,7 +243,7 @@ class User < ArvadosModel
                               head_uuid: repo[:uuid],
                               link_class: 'permission',
                               name: 'can_write')
-      if [] != repo_perms
+      if repo_perms.any?
         logger.warn "User already has repository access " + 
             repo_perms.collect { |p| p[:uuid] }.inspect
         return
@@ -266,7 +267,8 @@ class User < ArvadosModel
   def create_vm_login_permission_link(vm_uuid, repo_name)
     # Look up the given virtual machine just to make sure it really exists.
     begin
-      vm = (vms = VirtualMachine.where(uuid: vm_uuid)) != nil ? vms.first : nil
+      vm = VirtualMachine.where(uuid: vm_uuid).first
+
       if not vm
         logger.warn "Could not find virtual machine for #{vm_uuid.inspect}"
         return
@@ -279,7 +281,7 @@ class User < ArvadosModel
                               head_kind: 'arvados#virtualMachine',
                               link_class: 'permission',
                               name: 'can_login')
-      if [] == login_perm
+      if !login_perm.any?
         login_perm = Link.create(tail_kind: 'arvados#user',
                                  tail_uuid: self.uuid,
                                  head_kind: 'arvados#virtualMachine',
@@ -311,7 +313,7 @@ class User < ArvadosModel
                               link_class: 'permission',
                               name: 'can_read')
 
-      if [] == group_perm
+      if !group_perm.any?
         group_perm = Link.create(tail_kind: 'arvados#user',
                                  tail_uuid: self.uuid,
                                  head_kind: 'arvados#group',
diff --git a/services/api/test/unit/user_test.rb b/services/api/test/unit/user_test.rb
index b7571cc..c1da8fa 100644
--- a/services/api/test/unit/user_test.rb
+++ b/services/api/test/unit/user_test.rb
@@ -154,7 +154,7 @@ class UserTest < ActiveSupport::TestCase
       user = User.new
       user.email = 'abc at xyz.com'
   		
-      User.setup user
+      User.setup user, 'http://openid/prefix'
     rescue ArvadosModel::PermissionDeniedError
     end
 	end

commit ee1abbc5a99dc2fa67de7713107ccd5473bd94a5
Author: radhika chippada <radhika at radhika.curoverse>
Date:   Sat Mar 22 22:59:32 2014 -0400

    Render 404 if no user is found for the uuid provided

diff --git a/services/api/app/controllers/arvados/v1/users_controller.rb b/services/api/app/controllers/arvados/v1/users_controller.rb
index 49e12d5..226c0a7 100644
--- a/services/api/app/controllers/arvados/v1/users_controller.rb
+++ b/services/api/app/controllers/arvados/v1/users_controller.rb
@@ -90,12 +90,19 @@ class Arvados::V1::UsersController < ApplicationController
 
   # create user object and all the needed links
   def setup
-    @object = model_class.new resource_attrs
+    if params[:uuid]
+      @object = User.find_by_uuid params[:uuid]
+      if !@object
+        return render_404_if_no_object
+      end
+    else
+      @object = model_class.create! resource_attrs
+    end
 
     @object = User.setup @object, params[:repo_name], params[:vm_uuid], 
         params[:openid_prefix]
 
-    show  
+    show
   end
 
 end
diff --git a/services/api/script/setup-new-user.rb b/services/api/script/setup-new-user.rb
index 1eedbea..10df472 100755
--- a/services/api/script/setup-new-user.rb
+++ b/services/api/script/setup-new-user.rb
@@ -53,14 +53,15 @@ rescue Arvados::TransactionFailedError
   end
 end
 
-# Invoke user setup method 
+# Invoke user setup method
 if (found_user)
-  user = {uuid: found_user[:uuid]}
+  user = arv.user.setup uuid: found_user[:uuid], repo_name: user_repo_name,
+        vm_uuid: vm_uuid, openid_prefix: opts.openid_prefix
 else
-  user = {email: user_arg}
+  user = arv.user.setup user: {email: user_arg}, repo_name: user_repo_name,
+        vm_uuid: vm_uuid, openid_prefix: opts.openid_prefix
 end
 
-user = arv.user.setup user: user, repo_name: user_repo_name, vm_uuid: vm_uuid,
-      openid_prefix: opts.openid_prefix
-
 log.info {"user uuid: " + user[:uuid]}
+
+puts user.inspect
diff --git a/services/api/test/functional/arvados/v1/users_controller_test.rb b/services/api/test/functional/arvados/v1/users_controller_test.rb
index ca0ab84..010169e 100644
--- a/services/api/test/functional/arvados/v1/users_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/users_controller_test.rb
@@ -101,15 +101,14 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
     authorize_with :admin
 
     post :setup, {
-      user: {uuid: 'not_an_existing_uuid_and_not_email_format'},
+      uuid: 'not_an_existing_uuid_and_not_email_format',
       repo_name: 'test_repo',
       vm_uuid: 'no_such_vm'
     }
     response_body = JSON.parse(@response.body)
     response_errors = response_body['errors']
     assert_not_nil response_errors, 'Expected error in response'
-    incorrectly_formatted = response_errors.first.include?('No email')
-    assert incorrectly_formatted, 'Expected not valid email format error'
+    assert (response_errors.first.include? 'Path not found'), 'Expected 404'
   end
 
   test "create user with existing uuid, vm and repo and verify links" do

commit 1896a42e2da493b024dce40266a4814883c08003
Author: radhika chippada <radhika at radhika.curoverse>
Date:   Sat Mar 22 20:21:12 2014 -0400

    Expect openid_prefix from the clients instead of managing it on the api server.

diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index 1147589..6fdf95d 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -109,12 +109,7 @@ class User < ArvadosModel
     end
   end
 
-  def self.setup(user, repo_name, vm_uuid, openid_prefix)
-    # check if default openid_prefix needs to be overridden
-    if !openid_prefix
-      openid_prefix = Rails.configuration.openid_prefix
-    end
-
+  def self.setup(user, repo_name=nil, vm_uuid=nil, openid_prefix=nil)
     login_perm_props = {identity_url_prefix: openid_prefix}
 
     if user[:uuid]
diff --git a/services/api/config/application.default.yml b/services/api/config/application.default.yml
index 550d2a0..e46ff5d 100644
--- a/services/api/config/application.default.yml
+++ b/services/api/config/application.default.yml
@@ -99,6 +99,3 @@ common:
 
   # Version of your assets, change this if you want to expire all your assets
   assets.version: "1.0"
-
-  # default openid prefix to authenticate user
-  openid_prefix: "https://www.google.com/accounts/o8/id"
diff --git a/services/api/script/setup-new-user.rb b/services/api/script/setup-new-user.rb
index 8eaf724..1eedbea 100755
--- a/services/api/script/setup-new-user.rb
+++ b/services/api/script/setup-new-user.rb
@@ -23,8 +23,6 @@ claim the account.
   eos
 end
 
-default_openid_prefix = 'https://www.google.com/accounts/o8/id'
-
 log.level = (ENV['DEBUG'] || opts.debug) ? Logger::DEBUG : Logger::WARN
     
 if ARGV.count != 3
@@ -62,11 +60,7 @@ else
   user = {email: user_arg}
 end
 
-if opts.openid_prefix == default_openid_prefix
-  user = arv.user.setup user: user, repo_name: user_repo_name, vm_uuid: vm_uuid
-else
-  user = arv.user.setup user: user, repo_name: user_repo_name, vm_uuid: vm_uuid,
+user = arv.user.setup user: user, repo_name: user_repo_name, vm_uuid: vm_uuid,
       openid_prefix: opts.openid_prefix
-end
 
 log.info {"user uuid: " + user[:uuid]}
diff --git a/services/api/test/functional/arvados/v1/users_controller_test.rb b/services/api/test/functional/arvados/v1/users_controller_test.rb
index d74f8e7..ca0ab84 100644
--- a/services/api/test/functional/arvados/v1/users_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/users_controller_test.rb
@@ -183,7 +183,7 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
       }
     }
 
-    #assert_response :success
+    assert_response :success
     response_object = JSON.parse(@response.body)
     assert_not_nil response_object['uuid'], 'expected uuid for new user'
     assert_equal response_object['email'], 'abc at xyz.com', 'expected given email'
diff --git a/services/api/test/unit/user_test.rb b/services/api/test/unit/user_test.rb
index 9922ef4..b7571cc 100644
--- a/services/api/test/unit/user_test.rb
+++ b/services/api/test/unit/user_test.rb
@@ -137,5 +137,27 @@ class UserTest < ActiveSupport::TestCase
 	
 	end
 
+	test "create new user as non-admin user" do
+		Thread.current[:user] = @active_user
+
+    begin
+  		user = User.new
+	  	user.save
+    rescue ArvadosModel::PermissionDeniedError
+    end
+	end
+
+	test "setup new user as non-admin user" do
+		Thread.current[:user] = @active_user
+
+    begin
+      user = User.new
+      user.email = 'abc at xyz.com'
+  		
+      User.setup user
+    rescue ArvadosModel::PermissionDeniedError
+    end
+	end
+
 end
 

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list