Refactoring user cas_extra_attributes

3 files changed, 38 insertions, 31 deletions

diff --git a/app/models/user.rb b/app/models/user.rb
index ffdf632f8..5534347cb 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -29,20 +29,13 @@ class User < ActiveRecord::Base
   after_destroy :check_destroy_organisation
 
   def cas_extra_attributes=(extra_attributes)
-    extra_attributes.each do |name, value|
-      case name.to_sym
-      when :full_name
-        self.name = value
-      when :email
-        self.email = value
-      when :username
-        self.username = value
-      end
-    end
+    extra      = extra_attributes.inject({}){|memo,(k,v)| memo[k.to_sym] = v; memo}
+    self.name  = extra[:full_name]
+    self.email = extra[:email]
     self.organisation = self.cas_assign_or_create_organisation(
       {
-        code: extra_attributes[:organisation_code], 
-        name: extra_attributes[:organisation_name]
+        code: extra[:organisation_code],
+        name: extra[:organisation_name]
       }
     )
   end
@@ -51,6 +44,7 @@ class User < ActiveRecord::Base
     Organisation.find_or_create_by(code: code) do |organisation|
       organisation.name = name
       organisation.code = code
+      Rails.logger.debug "Cas auth - creating new organisation name: #{name} code: #{code}"
     end
   end
 
diff --git a/config/environments/test.rb b/config/environments/test.rb
index c239c6c89..3e0c486e6 100644
--- a/config/environments/test.rb
+++ b/config/environments/test.rb
@@ -51,7 +51,7 @@ Rails.application.configure do
   # }
   config.chouette_authentication_settings = {
     type: "cas",
-    cas_server: "http://localhost:3000/sessions"
+    cas_server: "http://stif-portail-dev.af83.priv/sessions"
   }
 
   # file to data for demo
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 9c9efce4b..5d88fb209 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -8,32 +8,45 @@ describe User, :type => :model do
     let(:ticket) do
       CASClient::ServiceTicket.new("ST-test", nil).tap do |ticket|
         ticket.extra_attributes = {
-          full_name: 'john doe',
-          username: 'xinhui.xu',
-          email: 'john.doe@af83.com',
-          organisation_code: '0083',
-          organisation_name: 'af83'
+          :full_name         => 'john doe',
+          :username          => 'john.doe',
+          :email             => 'john.doe@af83.com',
+          :organisation_code => '0083',
+          :organisation_name => 'af83'
         }
-        ticket.user    = "xinhui.xu"
+        ticket.user    = "john.doe"
         ticket.success = true
       end
     end
 
-    it 'should create a new user if user is not registered' do
-      expect{User.authenticate_with_cas_ticket(ticket)}.to change{ User.count }
-      user = User.find_by(username: 'xinhui.xu')
-      expect(user.email).to eq(ticket.extra_attributes[:email])
-      expect(user.name).to  eq(ticket.extra_attributes[:full_name])
-    end
+    context 'First time sign on' do
+      it 'should create a new user if user is not registered' do
+        expect{User.authenticate_with_cas_ticket(ticket)}.to change{ User.count }
+        user = User.find_by(username: 'john.doe')
+        expect(user.email).to eq(ticket.extra_attributes[:email])
+        expect(user.name).to  eq(ticket.extra_attributes[:full_name])
+      end
+
+      it 'should create a new organisation if organisation is not present' do
+        expect{User.authenticate_with_cas_ticket(ticket)}.to change{ Organisation.count }
+        expect(Organisation.find_by(code: ticket.extra_attributes[:organisation_code])).to be_truthy
+      end
 
-    it 'should create a new organisation if organisation is not present' do
-      expect{User.authenticate_with_cas_ticket(ticket)}.to change{ Organisation.count }
+      it 'should not create a new organisation if organisation is already present' do
+        ticket.extra_attributes[:organisation_code] = create(:organisation).code
+        expect{User.authenticate_with_cas_ticket(ticket)}.not_to change{ Organisation.count }
+      end
     end
 
-    it 'should not create a new organisation if organisation is already present' do
-      organisation = create :organisation
-      ticket.extra_attributes[:organisation_code] = organisation.code
-      expect{User.authenticate_with_cas_ticket(ticket)}.not_to change{ Organisation.count }
+    context 'Update attributes on sign on' do
+      let!(:organisation) { create(:organisation) }
+      let!(:user) { create(:user, username: 'john.doe', name:'fake name' , email: 'test@example.com', :organisation => organisation) }
+
+      it 'should update user attributes on sign on' do
+        User.authenticate_with_cas_ticket(ticket)
+        expect(user.reload.email).to eq(ticket.extra_attributes[:email])
+        expect(user.reload.name).to  eq(ticket.extra_attributes[:full_name])
+      end
     end
   end