From 1c46c8f22fc75379f67a71d85dd18bc945a41d24 Mon Sep 17 00:00:00 2001 From: budak Date: Thu, 16 Nov 2017 15:17:58 -0800 Subject: [PATCH] (#51) Use SAML authentication --- Gemfile | 5 +- Gemfile.lock | 30 ++-------- admins | 3 - app/controllers/callbacks_controller.rb | 26 --------- app/models/user.rb | 23 ++------ app/views/_user_util_links.html.erb | 4 +- config/attribute-map.yml | 1 + config/initializers/devise.rb | 57 ++++++++++++++++--- config/routes.rb | 4 +- .../20171101154805_update_users_for_saml.rb | 21 +++++++ db/schema.rb | 19 +------ spec/controllers/callbacks_controller_spec.rb | 49 ---------------- spec/models/user_spec.rb | 37 ------------ 13 files changed, 91 insertions(+), 188 deletions(-) delete mode 100644 admins delete mode 100644 app/controllers/callbacks_controller.rb create mode 100644 config/attribute-map.yml create mode 100644 db/migrate/20171101154805_update_users_for_saml.rb delete mode 100644 spec/controllers/callbacks_controller_spec.rb delete mode 100644 spec/models/user_spec.rb diff --git a/Gemfile b/Gemfile index 02c0a6b..dd82278 100644 --- a/Gemfile +++ b/Gemfile @@ -73,5 +73,6 @@ end gem 'rsolr', '>= 1.0' gem 'jquery-rails' gem 'devise' -gem 'devise-guests', '~> 0.6' -gem 'omniauth-google-oauth2' + +# Use devise_saml_authenticatable for SAML authentication +gem 'devise_saml_authenticatable' diff --git a/Gemfile.lock b/Gemfile.lock index 46a8927..2d05411 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -94,8 +94,9 @@ GEM railties (>= 4.1.0, < 5.2) responders warden (~> 1.2.3) - devise-guests (0.6.0) - devise + devise_saml_authenticatable (1.3.2) + devise (> 2.0.0) + ruby-saml (~> 1.3) diff-lcs (1.3) erubi (1.7.0) execjs (2.7.0) @@ -133,7 +134,6 @@ GEM git (1.2.9.1) globalid (0.4.1) activesupport (>= 4.2.0) - hashie (3.5.6) i18n (0.9.0) concurrent-ruby (~> 1.0) jbuilder (2.7.0) @@ -145,7 +145,6 @@ GEM thor (>= 0.14, < 2.0) json-schema (2.8.0) addressable (>= 2.4) - jwt (1.5.6) kaminari (1.1.1) activesupport (>= 4.1.0) kaminari-actionview (= 1.1.1) @@ -173,30 +172,12 @@ GEM mini_portile2 (2.3.0) minitest (5.10.3) multi_json (1.12.2) - multi_xml (0.6.0) multipart-post (2.0.0) nio4r (2.1.0) nokogiri (1.8.1) mini_portile2 (~> 2.3.0) nokogumbo (1.4.13) nokogiri - oauth2 (1.4.0) - faraday (>= 0.8, < 0.13) - jwt (~> 1.0) - multi_json (~> 1.3) - multi_xml (~> 0.5) - rack (>= 1.2, < 3) - omniauth (1.6.1) - hashie (>= 3.4.6, < 3.6.0) - rack (>= 1.6.2, < 3) - omniauth-google-oauth2 (0.5.2) - jwt (~> 1.5) - multi_json (~> 1.3) - omniauth (>= 1.1.1) - omniauth-oauth2 (>= 1.3.1) - omniauth-oauth2 (1.4.0) - oauth2 (~> 1.0) - omniauth (~> 1.2) orm_adapter (0.5.0) pg (0.21.0) public_suffix (3.0.0) @@ -256,6 +237,8 @@ GEM rspec-support (~> 3.6.0) rspec-support (3.6.0) ruby-progressbar (1.9.0) + ruby-saml (1.5.0) + nokogiri (>= 1.5.10) ruby_dep (1.5.0) rubyzip (1.2.1) sanitize (4.5.0) @@ -337,7 +320,7 @@ DEPENDENCIES capybara (~> 2.13) coffee-rails (~> 4.2) devise - devise-guests (~> 0.6) + devise_saml_authenticatable figs geo_combine geoblacklight (>= 1.4) @@ -345,7 +328,6 @@ DEPENDENCIES jbuilder (~> 2.5) jquery-rails listen (>= 3.0.5, < 3.2) - omniauth-google-oauth2 pg puma (~> 3.7) rails (~> 5.1.4) diff --git a/admins b/admins deleted file mode 100644 index 928ffb3..0000000 --- a/admins +++ /dev/null @@ -1,3 +0,0 @@ -jeremym@lclark.edu -budak@lclark.edu -rishijavia@lclark.edu \ No newline at end of file diff --git a/app/controllers/callbacks_controller.rb b/app/controllers/callbacks_controller.rb deleted file mode 100644 index a0079df..0000000 --- a/app/controllers/callbacks_controller.rb +++ /dev/null @@ -1,26 +0,0 @@ -class CallbacksController < Devise::OmniauthCallbacksController - - # Google OAuth2.0 method to receive data about user after they authenticate - # using their lclark account - def google_oauth2 - unless request.env["omniauth.auth"].info.email.split("@").last == "lclark.edu" - redirect_to '/', alert: 'Please use your lclark email to sign in.' - return - end - @user = User.from_omniauth(request.env["omniauth.auth"]) - set_user_role @user - sign_in_and_redirect @user - end - - # Check admins file from project root and make user admin if listed there - def set_user_role(user) - admins_file = File.read(Rails.root.join('admins')) - admins = admins_file.split("\n") - if admins.include?(user.email) - user.update_attribute :admin, true - else - user.update_attribute :admin, false - end - end - -end diff --git a/app/models/user.rb b/app/models/user.rb index 3470555..4876300 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,32 +1,17 @@ class User < ApplicationRecord if Blacklight::Utils.needs_attr_accessible? - attr_accessible :email, :password, :password_confirmation + attr_accessible :username end # Connects this user object to Blacklights Bookmarks. include Blacklight::User - # Include default devise modules. Others available are: - # :confirmable, :lockable, :timeoutable and :omniauthable - devise :database_authenticatable, :registerable, - :recoverable, :rememberable, :trackable, :validatable, - :omniauthable, :omniauth_providers => [:google_oauth2] + # Use SAML and trackable only + devise :saml_authenticatable, :trackable # Method added by Blacklight; Blacklight uses #to_s on your # user class to get a user-displayable login/identifier for # the account. def to_s - email + username end - - # Find or create a user - def self.from_omniauth(auth) - where(provider: auth.provider, uid: auth.uid).first_or_create do |user| - user.provider = auth.provider - user.uid = auth.uid - user.email = auth.info.email - user.name = auth.info.name - user.password = Devise.friendly_token[0,20] - end - end - end diff --git a/app/views/_user_util_links.html.erb b/app/views/_user_util_links.html.erb index 1e61d0b..6fd1003 100644 --- a/app/views/_user_util_links.html.erb +++ b/app/views/_user_util_links.html.erb @@ -14,12 +14,12 @@ <% unless current_user.to_s.blank? -%> <% end %> <% else %> <% end %> <% end %> diff --git a/config/attribute-map.yml b/config/attribute-map.yml new file mode 100644 index 0000000..8f50f15 --- /dev/null +++ b/config/attribute-map.yml @@ -0,0 +1 @@ +"urn:mace:dir:attribute-def:uid": "username" diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 7930e71..d1fc433 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -105,7 +105,7 @@ # a value less than 10 in other environments. Note that, for bcrypt (the default # algorithm), the cost increases exponentially with the number of stretches (e.g. # a value of 20 is already extremely slow: approx. 60 seconds for 1 calculation). - config.stretches = Rails.env.test? ? 1 : 11 + # config.stretches = Rails.env.test? ? 1 : 11 # Set up a pepper to generate the hashed password. # config.pepper = 'c2021a76707084fd6f6cf02c474ccbae00c95644c86691ff3d79328b80fded1b2c8077cae5578e3846c5ff91fca5c58cdf4a3965d5b88a3d703fedbfe00fd8f0' @@ -136,7 +136,7 @@ # initial account confirmation) to be applied. Requires additional unconfirmed_email # db field (see migrations). Until confirmed, new email is stored in # unconfirmed_email column, and copied to email column on successful confirmation. - config.reconfirmable = true + # config.reconfirmable = true # Defines which key will be used when confirming an account # config.confirmation_keys = [:email] @@ -146,7 +146,7 @@ # config.remember_for = 2.weeks # Invalidates all the remember me tokens when the user signs out. - config.expire_all_remember_me_on_sign_out = true + # config.expire_all_remember_me_on_sign_out = true # If true, extends the user's remember period when remembered via cookie. # config.extend_remember_period = false @@ -157,12 +157,12 @@ # ==> Configuration for :validatable # Range for password length. - config.password_length = 6..128 + # config.password_length = 6..128 # Email regex used to validate email formats. It simply asserts that # one (and only one) @ exists in the given string. This is mainly # to give user feedback and not to assert the e-mail validity. - config.email_regexp = /\A[^@\s]+@[^@\s]+\z/ + # config.email_regexp = /\A[^@\s]+@[^@\s]+\z/ # ==> Configuration for :timeoutable # The time you want to timeout the user session without activity. After this @@ -203,7 +203,7 @@ # Time interval you can reset your password with a reset password key. # Don't put a too small interval or your users won't have the time to # change their passwords. - config.reset_password_within = 6.hours + # config.reset_password_within = 6.hours # When set to false, does not sign a user in automatically after their password is # reset. Defaults to true, so a user is signed in automatically after a reset. @@ -251,7 +251,7 @@ # Add a new OmniAuth provider. Check the wiki for more information on setting # up on your models and hooks. # config.omniauth :github, 'APP_ID', 'APP_SECRET', scope: 'user,public_repo' - config.omniauth :google_oauth2, ENV['GOOGLE_CLIENT_ID'], ENV['GOOGLE_CLIENT_SECRET'] + # config.omniauth :google_oauth2, ENV['GOOGLE_CLIENT_ID'], ENV['GOOGLE_CLIENT_SECRET'] # ==> Warden configuration # If you want to use other strategies, that are not supported by Devise, or @@ -275,4 +275,47 @@ # When using OmniAuth, Devise cannot automatically set OmniAuth path, # so you need to do it manually. For the users scope, it would be: # config.omniauth_path_prefix = '/my_engine/users/auth' + + # ==> Configuration for :saml_authenticatable + # Create user if the user does not exist. (Default is false) + config.saml_create_user = true + + # Update the attributes of the user after a successful login. (Default is false) + config.saml_update_user = true + + # Set the default user key. The user will be looked up by this key. Make + # sure that the Authentication Response includes the attribute. + config.saml_default_user_key = :username + + # Optional. This stores the session index defined by the IDP during login. If provided it will be used as a salt + # for the user's session to facilitate an IDP initiated logout request. + config.saml_session_index_key = :session_index + + # You can set this value to use Subject or SAML assertation as info to which email will be compared. + # If you don't set it then email will be extracted from SAML assertation attributes. + config.saml_use_subject = true + + # You can support multiple IdPs by setting this value to a class that implements a #settings method which takes + # an IdP entity id as an argument and returns a hash of idp settings for the corresponding IdP. + config.idp_settings_adapter = nil + + # You provide you own method to find the idp_entity_id in a SAML message in the case of multiple IdPs + # by setting this to a custom reader class, or use the default. + # config.idp_entity_id_reader = DeviseSamlAuthenticatable::DefaultIdpEntityIdReader + + # You can set a handler object that takes the response for a failed SAML request and the strategy, + # and implements a #handle method. This method can then redirect the user, return error messages, etc. + # config.saml_failed_callback = nil + + # Configure with your SAML settings (see ruby-saml's README for more information: https://github.com/onelogin/ruby-saml). + config.saml_configure do |settings| + # assertion_consumer_service_url is required starting with ruby-saml 1.4.3: https://github.com/onelogin/ruby-saml#updating-from-142-to-143 + settings.assertion_consumer_service_url = "https://geo.watzek.cloud/users/saml/auth" + settings.assertion_consumer_service_binding = "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" + settings.name_identifier_format = "urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified" + settings.issuer = "https://geo.watzek.cloud" + settings.idp_slo_target_url = "" + settings.idp_sso_target_url = "https://sso.connect.pingidentity.com/sso/idp/SSO.saml2?saasid=8655b747-222d-49d5-9575-2e12b494552d&idpid=7e56e0ab-c5d9-4f27-9044-d8b05854a008" + settings.idp_cert = ENV['IDP_CERT'] + end end diff --git a/config/routes.rb b/config/routes.rb index 39b7b18..566c982 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,5 +1,5 @@ Rails.application.routes.draw do - + concern :gbl_exportable, Geoblacklight::Routes::Exportable.new resources :solr_documents, only: [:show], path: '/catalog', controller: 'catalog' do concerns :gbl_exportable @@ -23,7 +23,7 @@ concerns :searchable end - devise_for :users, :controllers => { :omniauth_callbacks => "callbacks" } + devise_for :users concern :exportable, Blacklight::Routes::Exportable.new resources :solr_documents, only: [:show], path: '/catalog', controller: 'catalog' do diff --git a/db/migrate/20171101154805_update_users_for_saml.rb b/db/migrate/20171101154805_update_users_for_saml.rb new file mode 100644 index 0000000..9edc97c --- /dev/null +++ b/db/migrate/20171101154805_update_users_for_saml.rb @@ -0,0 +1,21 @@ +class UpdateUsersForSaml < ActiveRecord::Migration[5.1] + def up + drop_table :users + create_table :users do |t| + + ## SAML authenticatable + t.string :username, null: false + + ## Trackable + t.integer :sign_in_count, default: 0, null: false + t.datetime :current_sign_in_at + t.datetime :last_sign_in_at + t.string :current_sign_in_ip + t.string :last_sign_in_ip + end + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/schema.rb b/db/schema.rb index d0ae4e5..4e74789 100644 --- 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: 20170927230653) do +ActiveRecord::Schema.define(version: 20171101154805) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -37,27 +37,12 @@ end create_table "users", force: :cascade do |t| - t.string "email", default: "", null: false - t.string "encrypted_password", default: "", null: false - t.string "reset_password_token" - t.datetime "reset_password_sent_at" - t.datetime "remember_created_at" + t.string "username", null: false t.integer "sign_in_count", default: 0, null: false t.datetime "current_sign_in_at" t.datetime "last_sign_in_at" t.string "current_sign_in_ip" t.string "last_sign_in_ip" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.boolean "guest", default: false - t.string "provider" - t.string "uid" - t.string "name" - t.boolean "admin", default: false - t.index ["email"], name: "index_users_on_email", unique: true - t.index ["provider"], name: "index_users_on_provider" - t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true - t.index ["uid"], name: "index_users_on_uid" end end diff --git a/spec/controllers/callbacks_controller_spec.rb b/spec/controllers/callbacks_controller_spec.rb deleted file mode 100644 index 88b7faa..0000000 --- a/spec/controllers/callbacks_controller_spec.rb +++ /dev/null @@ -1,49 +0,0 @@ -require 'rails_helper' - -describe CallbacksController do - - describe '/POST from google_oauth2' do - - before do - @request.env['devise.mapping'] = Devise.mappings[:user] - @omniauth = OmniAuth::AuthHash.new({ - 'uid' => '12345', - 'provider' => 'google_oauth2', - 'info' => OmniAuth::AuthHash::InfoHash.new('email' => 'test@lclark.edu') - }) - @user = instance_double('User', - :uid => '12345', - :provider => 'google_oauth2', - :admin => false, - :email => 'test@lclark.edu') - request.env['omniauth.auth'] = @omniauth - end - - it 'should reject email address that are not lclark' do - request.env['omniauth.auth']['info']['email'] = 'test@gmail.com' - post :google_oauth2 - expect(response).to redirect_to('/') - end - - it 'should call from_omniauth method and sign_in_and_redirect' do - expect(User).to receive(:from_omniauth).with(request.env['omniauth.auth']).and_return(@user) - expect(subject).to receive(:set_user_role).with(@user) - expect(subject).to receive(:sign_in_and_redirect).with(@user) - post :google_oauth2 - end - - it 'should give admin attribute if a user is admin' do - expect(File).to receive(:read).and_return("test@lclark.edu\ntest2@lclark.edu") - expect(@user).to receive(:update_attribute).with(:admin, true) - CallbacksController.new.set_user_role(@user) - end - - it 'should revoke admin attribute if a user is not admin' do - expect(File).to receive(:read).and_return("nouser@lclark.edu") - expect(@user).to receive(:update_attribute).with(:admin, false) - CallbacksController.new.set_user_role(@user) - end - - end - -end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb deleted file mode 100644 index f7b873d..0000000 --- a/spec/models/user_spec.rb +++ /dev/null @@ -1,37 +0,0 @@ -require 'rails_helper' - -describe User, :type => :model do - - describe '.from_omniauth', :type => :feature do - - before do - # first, set OmniAuth to run in test mode - OmniAuth.config.test_mode = true - # then, provide a set of fake oauth data that - # omniauth will use when a user tries to authenticate: - OmniAuth.config.mock_auth[:google_oauth2] = OmniAuth::AuthHash.new({ - 'uid' => '12345', - 'provider' => 'google_oauth2', - 'info' => OmniAuth::AuthHash::InfoHash.new('email' => 'test@lclark.edu', 'name' => 'Test User')}) - end - - it 'should login valid user' do - visit '/' - expect(page).to have_link("Login") - click_link "Login" - expect(page).to have_link("Test User") - expect(page).to have_link("Log Out") - end - - it 'should not login invalid user' do - OmniAuth.config.mock_auth[:google_oauth2].info.email = 'test@gmail.com' - visit '/' - expect(page).to have_link("Login") - click_link "Login" - expect(page).to have_link("Login") - expect(page).to have_content("Please use your lclark email to sign in.") - end - - end - -end