Merge pull request #98 from emory-libraries/shib

alexBLR · web-flow · commit a0516384b4c4 · 2020-10-26T08:37:54.000-05:00
Adds initial shib config
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -11,6 +11,7 @@ jobs:
         - FEDORA_URL=http://localhost:8080/fcrepo/rest
         - FEDORA_TIMEOUT=300
         - RAILS_ENV=test
+        - DATABASE_AUTH=true
      # Secondary container image on common network.
      - image: postgres:10-alpine
        environment:
diff --git a/Gemfile b/Gemfile
@@ -59,6 +59,7 @@ gem 'ims-lti', '~> 1.1.13'
 gem 'net-ldap'
 gem 'omniauth-identity'
 gem 'omniauth-lti', git: "https://github.com/avalonmediasystem/omniauth-lti.git", tag: 'avalon-r4'
+gem 'omniauth-saml', '~> 1.10', '>= 1.10.3'
 
 # Media Access & Transcoding
 gem 'active_encode', '~> 0.7.0'
diff --git a/Gemfile.lock b/Gemfile.lock
@@ -582,6 +582,9 @@ GEM
     omniauth-identity (1.1.1)
       bcrypt-ruby (~> 3.0)
       omniauth (~> 1.0)
+    omniauth-saml (1.10.3)
+      omniauth (~> 1.3, >= 1.3.2)
+      ruby-saml (~> 1.9)
     orm_adapter (0.5.0)
     os (1.0.1)
     parallel (1.17.0)
@@ -757,6 +760,8 @@ GEM
       multipart-post
       oauth2
     ruby-progressbar (1.10.0)
+    ruby-saml (1.11.0)
+      nokogiri (>= 1.5.10)
     ruby_dep (1.5.0)
     rubydora (1.9.1)
       activemodel
@@ -978,6 +983,7 @@ DEPENDENCIES
   noid-rails (~> 3.0.1)
   omniauth-identity
   omniauth-lti!
+  omniauth-saml (~> 1.10, >= 1.10.3)
   parallel
   pg
   pry-byebug
diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb
@@ -74,6 +74,21 @@ def find_user(auth_type)
     end
   end
 
+  def shibboleth
+    @user = User.from_omniauth(request.env["omniauth.auth"])
+
+    if @user.persisted?
+      sign_in @user
+      # This will help the user go back to where they came.
+      # Refer: https://github.com/omniauth/omniauth/wiki/Saving-User-Location
+      redirect_to request.env["omniauth.origin"] || root_url
+      set_flash_message(:notice, :success, kind: "Shibboleth")
+    else
+      redirect_to root_path
+      set_flash_message(:notice, :failure, kind: "Shibboleth", reason: "you aren't authorized to use this application.")
+    end
+  end
+
   protected :find_user
 
   rescue_from Avalon::MissingUserId do |exception|
diff --git a/app/models/auth_config.rb b/app/models/auth_config.rb
@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+class AuthConfig
+  # In production, we use Shibboleth for user authentication,
+  # but in development mode, you may want to use local database
+  # authentication instead.
+  def self.use_database_auth?
+    ENV['DATABASE_AUTH'] == 'true'
+  end
+end
diff --git a/app/models/user.rb b/app/models/user.rb
@@ -26,10 +26,8 @@ class User < ActiveRecord::Base
   # Include default devise modules. Others available are:
   # :confirmable, :lockable, :timeoutable
   # Registration is controlled via settings.yml
-  devise_list = [ :database_authenticatable, :invitable, :omniauthable,
-                  :recoverable, :rememberable, :trackable, :validatable ]
-  devise_list << :registerable if Settings.auth.registerable
-  devise_list << { authentication_keys: [:login] }
+  devise_list = [:omniauthable, :rememberable, :trackable, omniauth_providers: [:shibboleth], authentication_keys: [:login]]
+  devise_list.prepend(:database_authenticatable) if AuthConfig.use_database_auth?
 
   devise(*devise_list)
 
@@ -158,6 +156,32 @@ def self.walk_ldap_groups(groups, seen)
     end
     seen
   end
+
+  # When a user authenticates via shibboleth, find their User object or make
+  # a new one. Populate it with data we get from shibboleth.
+  # @param [OmniAuth::AuthHash] auth
+  def self.from_omniauth(auth)
+    begin
+      user = find_by!(provider: auth.provider, username: auth.uid.downcase)
+    rescue ActiveRecord::RecordNotFound
+      log_omniauth_error(auth)
+      return User.new
+    end
+    user.assign_attributes(display_name: auth.info.display_name, ppid: auth.uid, uid: auth.info.uid)
+    # tezprox@emory.edu isn't a real email address
+    user.email = auth.info.uid + '@emory.edu' unless auth.info.uid == 'tezprox'
+    user.save
+    user
+  end
+
+  def self.log_omniauth_error(auth)
+    if auth.info.uid.empty?
+      Rails.logger.error "Nil user detected: Shibboleth didn't pass a uid for #{auth.inspect}"
+    else
+      # Log unauthorized logins to error.
+      Rails.logger.error "Unauthorized user attemped login: #{auth.inspect}"
+    end
+  end
 end
 
 class Avalon::MissingUserId < StandardError; end
diff --git a/config/environments/production.rb b/config/environments/production.rb
@@ -115,4 +115,20 @@
 
   # Additional production specific initializers
   Dir["config/environments/production/*.rb"].each {|file| load file }
+
+  # OmniAuth configuration settings
+  config.idp_slo_target_url = ENV['IDP_SLO_TARGET_URL']
+  config.assertion_consumer_service_url = ENV['ASSERTION_CS_URL']
+  config.assertion_consumer_logout_service_url = ENV['ASSERTION_LOGOUT_URL']
+  config.issuer = ENV['ISSUER']
+  config.idp_sso_target_url = ENV['IDP_SSO_TARGET_URL']
+  config.idp_cert = ENV['IDP_CERT']
+  config.certificate = ENV['SP_CERT']
+  config.private_key = ENV['SP_KEY']
+  config.attribute_statements = {}
+  config.uid_attribute = "urn:oid:0.9.2342.19200300.100.1.1"
+  config.security = {
+     :want_assertions_encrypted  => true, #makes a 2nd KeyDescriptor, this one says use="encryption"
+     :want_assertions_signed  => true, # goes on md SPSSODescriptor tag
+  }
 end
diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb
@@ -0,0 +1,16 @@
+Rails.application.config.middleware.use OmniAuth::Builder do
+  if !Rails.env.development? && !Rails.env.test?
+    provider :shibboleth,
+      :assertion_consumer_service_url        => Rails.application.config.assertion_consumer_service_url,
+      :assertion_consumer_logout_service_url => Rails.application.config.assertion_consumer_logout_service_url,
+      :issuer                                => Rails.application.config.issuer,
+      :idp_sso_target_url                    => Rails.application.config.idp_sso_target_url,
+      :idp_slo_target_url                    => Rails.application.config.idp_slo_target_url,
+      :idp_cert                              => Rails.application.config.idp_cert,
+      :certificate                           => Rails.application.config.certificate,
+      :private_key                           => Rails.application.config.private_key,
+      :attribute_statements                  => Rails.application.config.attribute_statements,
+      :uid_attribute                         => Rails.application.config.uid_attribute,
+      :security                              => Rails.application.config.security
+  end
+end
diff --git a/db/migrate/20201020210257_add_omniauth_to_users.rb b/db/migrate/20201020210257_add_omniauth_to_users.rb
@@ -0,0 +1,6 @@
+class AddOmniauthToUsers < ActiveRecord::Migration[5.2]
+  def change
+    add_column :users, :display_name, :string
+    add_column :users, :ppid, :string
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2019_10_16_195828) do
+ActiveRecord::Schema.define(version: 2020_10_20_210257) do
 
   create_table "active_encode_encode_records", force: :cascade do |t|
     t.string "global_id"
@@ -233,6 +233,8 @@
     t.integer "invited_by_id"
     t.string "invited_by_type"
     t.datetime "deleted_at"
+    t.string "display_name"
+    t.string "ppid"
     t.index ["deleted_at"], name: "index_users_on_deleted_at"
     t.index ["email"], name: "index_users_on_email", unique: true
     t.index ["invitation_token"], name: "index_users_on_invitation_token", unique: true
diff --git a/docker-compose.yml b/docker-compose.yml
@@ -122,6 +122,7 @@ services:
       - SETTINGS__STREAMING__SERVER=nginx
       - SETTINGS__STREAMING__STREAM_TOKEN_TTL=20
       - SYSTEM_GROUPS=administrator,group_manager,manager
+      - DATABASE_AUTH=true
     volumes:
       - .:/home/app/avalon
       - npms:/home/app/avalon/node_modules
@@ -150,6 +151,7 @@ services:
       - RAILS_ENV=test
       - BUNDLE_FLAGS=--with aws test postgres --without production
       - DATABASE_CLEANER_ALLOW_REMOTE_DATABASE_URL=true
+      - DATABASE_AUTH=true
     ports: []
 
   worker:
diff --git a/spec/controllers/omniauth_callbacks_controller_spec.rb b/spec/controllers/omniauth_callbacks_controller_spec.rb
@@ -136,4 +136,44 @@
       end
     end
   end
+
+  describe 'shibboleth login' do
+    before do
+      User.create(provider:     'shibboleth',
+                  uid:          'brianbboys1967',
+                  ppid:         'P0000001',
+                  display_name: 'Brian Wilson',
+                  email:        'brianbboys1967@emory.edu',
+                  username:     'P0000001')
+      request.env["devise.mapping"] = Devise.mappings[:user]
+      request.env["omniauth.auth"] = OmniAuth.config.mock_auth[:shib]
+    end
+    OmniAuth.config.mock_auth[:shib] =
+      OmniAuth::AuthHash.new(
+        provider: 'shibboleth',
+        uid:      "P0000001",
+        info:     {
+          display_name: "Brian Wilson",
+          uid:          'brianbboys1967'
+        }
+      )
+
+    context "when origin is present" do
+      before do
+        request.env["omniauth.origin"] = '/example'
+      end
+
+      it "redirects to origin" do
+        post :shibboleth
+        expect(response.redirect_url).to eq 'http://test.host/example'
+      end
+    end
+
+    context "when origin is missing" do
+      it "redirects to dashboard" do
+        post :shibboleth
+        expect(response.redirect_url).to include 'http://test.host/'
+      end
+    end
+  end
 end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
@@ -14,9 +14,9 @@
 
 require 'rails_helper'
 
-describe User do
+describe User, :clean do
   subject { user }
-  let!(:user) { FactoryBot.build(:user) }
+  let(:user) { FactoryBot.build(:user) }
   let!(:list) { 0.upto(rand(5)).collect { Faker::Internet.email } }
 
   describe "validations" do
@@ -156,4 +156,122 @@
       expect(user.timeline_tags).to eq ['foo']
     end
   end
+
+  describe 'from omniauth' do
+    before do
+      # User must exists before tests can run
+      described_class.create(provider:     'shibboleth',
+                             uid:          'brianbboys1967',
+                             ppid:         'P0000001',
+                             display_name: 'Brian Wilson',
+                             email:        'brianbboys1967@emory.edu',
+                             username:     'P0000001')
+    end
+
+    let(:auth_hash) do
+      OmniAuth::AuthHash.new(
+        provider: 'shibboleth',
+        uid:      "P0000001",
+        info:     {
+          display_name: "Brian Wilson",
+          uid:          'brianbboys1967'
+        }
+      )
+    end
+
+    let(:user) { described_class.from_omniauth(auth_hash) }
+
+    context "has attributes" do
+      it "has Shibboleth as a provider" do
+        expect(user.provider).to eq 'shibboleth'
+      end
+      it "has a uid" do
+        expect(user.uid).to eq auth_hash.info.uid
+      end
+      it "has a name" do
+        expect(user.display_name).to eq auth_hash.info.display_name
+      end
+      it "has a PPID" do
+        expect(user.ppid).to eq auth_hash.uid
+      end
+    end
+
+    context "updating an existing user" do
+      let(:updated_auth_hash) do
+        OmniAuth::AuthHash.new(
+          provider: 'shibboleth',
+          uid:      "P0000001",
+          info:     {
+            display_name: "Boaty McBoatface",
+            uid:          'brianbboys1968'
+          }
+        )
+      end
+
+      it "updates ppid and display_name with values from shibboleth" do
+        expect(user.uid).to eq auth_hash.info.uid
+        expect(user.ppid).to eq auth_hash.uid
+        expect(user.display_name).to eq auth_hash.info.display_name
+        described_class.from_omniauth(updated_auth_hash)
+        user.reload
+        expect(user.ppid).to eq updated_auth_hash.uid
+        expect(user.uid).not_to eq auth_hash.info.uid
+        expect(user.ppid).to eq updated_auth_hash.uid
+        expect(user.display_name).not_to eq auth_hash.info.display_name
+        expect(user.display_name).to eq updated_auth_hash.info.display_name
+      end
+    end
+
+    context "signing in twice" do
+      it "finds the original account instead of trying to make a new one" do
+        # login existing user second time
+        expect { described_class.from_omniauth(auth_hash) }
+          .not_to change { described_class.count }
+      end
+    end
+
+    context "attempting to sign in a new user" do
+      let(:new_auth_hash) do
+        OmniAuth::AuthHash.new(
+          provider: 'shibboleth',
+          uid:      'P0000003',
+          info:     {
+            display_name: 'Fake Person',
+            uid:          'egnetid'
+          }
+        )
+      end
+
+      it "does not allow a new user to sign in" do
+        expect { described_class.from_omniauth(new_auth_hash) }
+          .not_to change { described_class.count }
+        expect(Rails.logger).to receive(:error)
+        u = described_class.from_omniauth(new_auth_hash)
+        expect(u.class.name).to eql 'User'
+        expect(u.persisted?).to be false
+      end
+    end
+
+    context "invalid shibboleth data" do
+      let(:invalid_auth_hash) do
+        OmniAuth::AuthHash.new(
+          provider: 'shibboleth',
+          uid:      '',
+          info:     {
+            display_name: '',
+            uid:          ''
+          }
+        )
+      end
+
+      it "does not register new users" do
+        expect { described_class.from_omniauth(invalid_auth_hash) }
+          .not_to change { described_class.count }
+        expect(Rails.logger).to receive(:error)
+        u = described_class.from_omniauth(invalid_auth_hash)
+        expect(u.class.name).to eql 'User'
+        expect(u.persisted?).to be false
+      end
+    end
+  end
 end
