Fix crash creating new user during initial login. Add test.

diff --git a/services/api/app/controllers/user_sessions_controller.rb b/services/api/app/controllers/user_sessions_controller.rb
index 3d4b05af4a0db5dbf37057ec71adb2b6b69e9b61..0b80877bc25624e9b66a38f8c0c35c75b468cc0f 100644 (file)
--- a/services/api/app/controllers/user_sessions_controller.rb
+++ b/services/api/app/controllers/user_sessions_controller.rb
@@ -9,7 +9,6 @@ class UserSessionsController < ApplicationController
   # omniauth callback method
   def create
     omniauth = env['omniauth.auth']
-    #logger.debug "+++ #{omniauth}"
 
     identity_url_ok = (omniauth['info']['identity_url'].length > 0) rescue false
     unless identity_url_ok
@@ -58,7 +57,7 @@ class UserSessionsController < ApplicationController
     # "unauthorized":
     Thread.current[:user] = user
 
-    user.save!
+    user.save or raise Exception.new(user.errors.messages)
 
     omniauth.delete('extra')
 

diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 2bf1376d6d82f25297c786c49ff8edaafac739bb..006eb90e12fd550e2110906a1b1c42cc64e291e8 100644 (file)
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -247,6 +247,7 @@ class ArvadosModel < ActiveRecord::Base
 
   def maybe_update_modified_by_fields
     update_modified_by_fields if self.changed? or self.new_record?
+    true
   end
 
   def update_modified_by_fields
@@ -255,6 +256,7 @@ class ArvadosModel < ActiveRecord::Base
     self.modified_at = Time.now
     self.modified_by_user_uuid = current_user ? current_user.uuid : nil
     self.modified_by_client_uuid = current_api_client ? current_api_client.uuid : nil
+    true
   end
 
   def ensure_serialized_attribute_type

diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index 81cae987a20d023f61d96a0ade93c9dd75d60c84..6bba194c24dac1f5460f5ce82fe90e87c202919c 100644 (file)
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -177,6 +177,10 @@ class User < ArvadosModel
 
   protected
 
+  def ensure_ownership_path_leads_to_user
+    true
+  end
+
   def permission_to_update
     # users must be able to update themselves (even if they are
     # inactive) in order to create sessions

diff --git a/services/api/test/integration/user_sessions_test.rb b/services/api/test/integration/user_sessions_test.rb
new file mode 100644 (file)
index 0000000..321a5ac
--- /dev/null
+++ b/services/api/test/integration/user_sessions_test.rb
@@ -0,0 +1,28 @@
+require 'test_helper'
+
+class UserSessionsApiTest < ActionDispatch::IntegrationTest
+  test 'create new user during omniauth callback' do
+    mock = {
+      'provider' => 'josh_id',
+      'uid' => 'https://edward.example.com',
+      'info' => {
+        'identity_url' => 'https://edward.example.com',
+        'name' => 'Edward Example',
+        'first_name' => 'Edward',
+        'last_name' => 'Example',
+        'email' => 'edward@example.com',
+      },
+    }
+    client_url = 'https://wb.example.com'
+    post('/auth/josh_id/callback',
+         {return_to: client_url},
+         {'omniauth.auth' => mock})
+    assert_response :redirect, 'Did not redirect to client with token'
+    assert_equal(0, @response.redirect_url.index(client_url),
+                 'Redirected to wrong address after succesful login: was ' +
+                 @response.redirect_url + ', expected ' + client_url + '[...]')
+    assert_not_nil(@response.redirect_url.index('api_token='),
+                   'Expected api_token in query string of redirect url ' +
+                   @response.redirect_url)
+  end
+end