From 19ed7b468e8748c8b6ec3535cfd5206518db2da1 Mon Sep 17 00:00:00 2001 From: Alex Dolski Date: Wed, 24 Jan 2024 16:31:23 -0600 Subject: [PATCH] Remove Shibboleth --- Gemfile | 1 - Gemfile.lock | 3 - README.md | 10 +- app/assets/javascripts/institutions.js | 7 -- app/controllers/institutions_controller.rb | 14 --- app/controllers/sessions_controller.rb | 13 --- app/models/affiliation.rb | 26 +++--- app/models/department.rb | 11 ++- app/models/institution.rb | 34 +------ app/models/login.rb | 20 ++-- app/models/user.rb | 93 +++---------------- app/models/user_group.rb | 2 +- app/policies/institution_policy.rb | 4 - .../_shibboleth_authentication_form.html.haml | 38 -------- .../_show_authentication_tab.html.haml | 58 ------------ app/views/sessions/_login_form.html.haml | 19 ++-- ...m.html.haml => _uiuc_login_form.html.haml} | 0 app/views/user_groups/show.html.haml | 16 ++-- config/initializers/omniauth.rb | 40 +------- config/routes.rb | 2 - ...4_remove_institution_shibboleth_columns.rb | 16 ++++ db/schema.rb | 8 +- .../institutions_controller_test.rb | 54 +++-------- test/fixtures/institutions.yml | 2 - test/fixtures/users.yml | 10 +- .../cache_submittable_collections_job_test.rb | 8 +- test/models/affiliation_test.rb | 74 +++++++-------- test/models/department_test.rb | 14 +++ test/models/institution_test.rb | 24 +---- test/models/login_test.rb | 19 +++- test/models/user_test.rb | 81 +--------------- test/policies/credential_policy_test.rb | 4 +- test/policies/institution_policy_test.rb | 53 ----------- test/policies/user_policy_test.rb | 22 ++--- test/test_helper.rb | 31 +------ 35 files changed, 197 insertions(+), 634 deletions(-) delete mode 100644 app/views/institutions/_shibboleth_authentication_form.html.haml rename app/views/sessions/{_shibboleth_login_form.html.haml => _uiuc_login_form.html.haml} (100%) create mode 100644 db/migrate/20240118235844_remove_institution_shibboleth_columns.rb diff --git a/Gemfile b/Gemfile index de3261bdd..ec068691b 100644 --- a/Gemfile +++ b/Gemfile @@ -50,7 +50,6 @@ gem "netaddr" gem "omniauth-identity", git: "https://github.com/medusa-project/omniauth-identity.git" gem "omniauth-rails_csrf_protection" gem "omniauth-saml" -gem "omniauth-shibboleth" # Our database gem "pg" # Our application server diff --git a/Gemfile.lock b/Gemfile.lock index aac78d332..e675f99f8 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -275,8 +275,6 @@ GEM omniauth-saml (2.1.0) omniauth (~> 2.0) ruby-saml (~> 1.12) - omniauth-shibboleth (1.3.0) - omniauth (>= 1.0.0) parallel (1.23.0) parser (3.2.2.4) ast (~> 2.4.1) @@ -444,7 +442,6 @@ DEPENDENCIES omniauth-identity! omniauth-rails_csrf_protection omniauth-saml - omniauth-shibboleth pg puma rails (~> 7.1) diff --git a/README.md b/README.md index 3e7b3dc3c..006be77bf 100644 --- a/README.md +++ b/README.md @@ -196,10 +196,7 @@ These tasks are run via cron in the demo and production environments: Authentication is handled by [OmniAuth](https://github.com/omniauth/omniauth). OmniAuth supports many different providers, but the ones used by this app are: -* [Shibboleth](https://github.com/toyokazu/omniauth-shibboleth): handles SSO - authentication for UIUC only. -* [SAML](https://github.com/omniauth/omniauth-saml): handles SSO authentication - for many CARLI member institutions, as well as CARLI itself. +* [SAML](https://github.com/omniauth/omniauth-saml): handles SSO authentication. * [Local identity](https://github.com/omniauth/omniauth-identity): credentials are stored in a `credentials` database table associated with the `users` table. @@ -212,9 +209,8 @@ provider) calls back to `/auth/:provider/callback`, which is handled by OmniAuth and its providers are configured in `config/initializers/omniauth.rb`. Providers can be configured globally when they work the same across all -institutions (local identity), or work with only one institution (Shibboleth). -But the SAML provider in particular needs to be customized per-institution, so -that each institution can work with its own identity provider (IdP). +institutions (local identity), but the SAML provider in particular needs to be +customized per-institution. Users who have associated local credentials can also log in via some other provider, if configured, but not vice versa. diff --git a/app/assets/javascripts/institutions.js b/app/assets/javascripts/institutions.js index da798aa72..5fad68923 100644 --- a/app/assets/javascripts/institutions.js +++ b/app/assets/javascripts/institutions.js @@ -122,13 +122,6 @@ const InstitutionView = { modalBody.html(data); }); }); - $("button.edit-shibboleth-authentication").on("click", function () { - const url = ROOT_URL + "/institutions/" + institutionKey + "/edit-shibboleth-authentication"; - $.get(url, function (data) { - const modalBody = $("#edit-shibboleth-authentication-modal .modal-body"); - modalBody.html(data); - }); - }); }); }); diff --git a/app/controllers/institutions_controller.rb b/app/controllers/institutions_controller.rb index 13ac6c2fe..08be41854 100644 --- a/app/controllers/institutions_controller.rb +++ b/app/controllers/institutions_controller.rb @@ -146,14 +146,6 @@ def edit_settings locals: { institution: @institution } end - ## - # Responds to `GET /institutions/:key/edit-shibboleth-authentication` - # - def edit_shibboleth_authentication - render partial: "institutions/shibboleth_authentication_form", - locals: { institution: @institution } - end - ## # Used for editing the theme. # @@ -958,12 +950,6 @@ def settings_params :saml_sp_private_key, :saml_sp_public_cert, :saml_idp_sso_binding_urn, - :shibboleth_auth_enabled, - :shibboleth_email_attribute, - :shibboleth_extra_attributes, - :shibboleth_host, - :shibboleth_name_attributes, - :shibboleth_org_dn, :sso_federation, # Theme tab :active_link_color, diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index f815b1adc..dd39c6141 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -17,19 +17,6 @@ def new session[:login_failure_url] = login_url end - ## - # Redirects to the Shibboleth login flow. Responds to - # `GET/POST /netid-login`. - # - def new_netid - if Rails.env.development? || Rails.env.test? - redirect_to "/auth/developer" - else - target = "https://#{current_institution.fqdn}/auth/shibboleth/callback" - redirect_to "/Shibboleth.sso/Login?target=#{target}" - end - end - ## # Handles callbacks from the auth provider (OmniAuth). Responsible for # translating an authentication hash into a {User}, assigning the user to diff --git a/app/models/affiliation.rb b/app/models/affiliation.rb index 27b6fd6b8..9738a1f9b 100644 --- a/app/models/affiliation.rb +++ b/app/models/affiliation.rb @@ -1,8 +1,7 @@ # frozen_string_literal: true ## -# University affiliation associated with {UserGroup} and Shibboleth- -# authenticated users. +# University affiliation associated with {UserGroup} and UIUC users. # # # Attributes # @@ -22,27 +21,32 @@ class Affiliation < ApplicationRecord PHD_STUDENT_KEY = "phd" UNDERGRADUATE_STUDENT_KEY = "undergrad" + ITRUST_AFFILIATION_ATTRIBUTE = "urn:oid:1.3.6.1.4.1.11483.101.1" + ITRUST_LEVEL_CODE_ATTRIBUTE = "urn:oid:1.3.6.1.4.1.11483.1.42" + ITRUST_PROGRAM_CODE_ATTRIBUTE = "urn:oid:1.3.6.1.4.1.11483.1.40" + ## - # @param info [Hash] Shibboleth auth hash. + # @param info [Hash] OmniAuth auth hash. # - def self.from_shibboleth(info) + def self.from_omniauth(info) key = nil info = info.dig("extra", "raw_info") if info # this will be nil when using the OmniAuth developer strategy - info = info.symbolize_keys + info = info.deep_stringify_keys # Explanation of this logic: # https://uofi.app.box.com/notes/801448983786?s=5k6iiozlhp5mui5b4vrbskn3pu968j8r - if info[:iTrustAffiliation].match?(/staff|allied/) + if info[ITRUST_AFFILIATION_ATTRIBUTE].include?("staff") || + info[ITRUST_AFFILIATION_ATTRIBUTE].include?("allied") key = FACULTY_STAFF_KEY - elsif info[:iTrustAffiliation].include?("student") - if %w(1G 1V 1M 1L).include?(info[:levelCode]) + elsif info[ITRUST_AFFILIATION_ATTRIBUTE].include?("student") + if %w(1G 1V 1M 1L).include?(info[ITRUST_LEVEL_CODE_ATTRIBUTE]) key = GRADUATE_STUDENT_KEY - elsif info[:levelCode] == "1U" + elsif info[ITRUST_LEVEL_CODE_ATTRIBUTE] == "1U" key = UNDERGRADUATE_STUDENT_KEY end - if %w(PHD CAS).include?(info[:programCode]) + if %w(PHD CAS).include?(info[ITRUST_PROGRAM_CODE_ATTRIBUTE]) key = PHD_STUDENT_KEY - elsif info[:programCode].present? + elsif info[ITRUST_PROGRAM_CODE_ATTRIBUTE].present? key = MASTERS_STUDENT_KEY end end diff --git a/app/models/department.rb b/app/models/department.rb index 2d0e01be0..8bf2b2072 100644 --- a/app/models/department.rb +++ b/app/models/department.rb @@ -1,8 +1,7 @@ # frozen_string_literal: true ## -# University department associated with a {UserGroup} and a -# {#User::AuthType::SHIBBOLETH} user. +# University department associated with a {UserGroup} and a UIUC user. # # This table is not normalized--multiple same-named departments may exist, # associated with different entities, with {name} being used for comparison. @@ -16,9 +15,17 @@ # class Department < ApplicationRecord + ITRUST_DEPARTMENT_CODE_ATTRIBUTE = "urn:oid:1.3.6.1.4.1.11483.1.122" + belongs_to :user, optional: true belongs_to :user_group, optional: true normalizes :name, with: -> (value) { value.squish } + def self.from_omniauth(auth) + auth = auth.deep_stringify_keys + name = auth.dig("extra", "raw_info", ITRUST_DEPARTMENT_CODE_ATTRIBUTE) + Department.new(name: name) + end + end diff --git a/app/models/institution.rb b/app/models/institution.rb index 9a2b1ec5a..d1bccdd0d 100644 --- a/app/models/institution.rb +++ b/app/models/institution.rb @@ -138,17 +138,6 @@ # * `service_name` Name of the service that the # institution is running. For example, at # UIUC, this would be "IDEALS." -# * `shibboleth_auth_enabled` Whether Shibboleth authentication is -# enabled. -# * `shibboleth_email_attribute` Shibboleth email attribute. -# * `shibboleth_extra_attributes` Array of extra attributes to request -# from the Shibboleth IdP. This can also -# be set to a comma-separated string -# which will be transformed into an array -# upon save. -# * `shibboleth_name_attribute` Shibboleth name attribute. -# * `shibboleth_org_dn` Value of an `eduPersonOrgDN` attribute -# from the Shibboleth IdP. # * `sso_federation` Set to one of the # {Institution::SSOFederation} constant # values. @@ -248,8 +237,6 @@ def self.label_for(value) has_many :users has_many :vocabularies - serialize :shibboleth_extra_attributes, coder: JSON - normalizes :copyright_notice, :google_analytics_measurement_id, :saml_idp_encryption_cert, :saml_idp_encryption_cert2, :saml_idp_signing_cert, :saml_idp_signing_cert2, @@ -307,8 +294,6 @@ def self.label_for(value) :add_default_metadata_profile, :add_default_submission_profile, :add_default_index_pages, :add_default_user_groups - before_save :arrayize_shibboleth_extra_attributes_csv - ## # @param extension [String] # @return [String] @@ -436,7 +421,7 @@ def self.item_counts(include_buried: false) # @return [Boolean] Whether at least one authentication method is enabled. # def auth_enabled? - local_auth_enabled || saml_auth_enabled || shibboleth_auth_enabled + local_auth_enabled || saml_auth_enabled end ## @@ -732,7 +717,7 @@ def scope_url # @return [Boolean] # def sso_enabled? - saml_auth_enabled || shibboleth_auth_enabled + saml_auth_enabled end ## @@ -1146,21 +1131,6 @@ def add_default_user_groups self.administrator_groups.build(user_group: admins).save! end - ## - # {shibboleth_extra_attributes} is a serialized array. This method allows us - # to set it to a comma-separated string (from e.g. a textarea) and have it - # auto-transformed into an array. - # - def arrayize_shibboleth_extra_attributes_csv - if self.shibboleth_extra_attributes.kind_of?(String) && - self.shibboleth_extra_attributes&.include?(",") && - !self.shibboleth_extra_attributes.start_with?("[") && - !self.shibboleth_extra_attributes.end_with?("]") - self.shibboleth_extra_attributes = - self.shibboleth_extra_attributes.split(",").map(&:strip) - end - end - def disallow_key_changes if !new_record? && key_changed? errors.add(:key, "cannot be changed") diff --git a/app/models/login.rb b/app/models/login.rb index 92b374be3..1747b7d57 100644 --- a/app/models/login.rb +++ b/app/models/login.rb @@ -18,13 +18,13 @@ class Login < ApplicationRecord class Provider - # Credentials are stored in the `credentials` table. - LOCAL = 0 - # Used only by UIUC. - SHIBBOLETH = 1 + LOCAL = 0 # Used by many CARLI member institutions, and CARLI itself. - SAML = 2 + SAML = 2 + ## + # @return [Enumerable] + # def self.all self.constants.map{ |c| self.const_get(c) }.sort end @@ -35,7 +35,9 @@ def self.label_for(value) "Local" when SAML "SAML" - when SHIBBOLETH + when 1 + # SHIBBOLETH (removed, but there are still some old Logins in the + # database associated with this provider) "Shibboleth" else "Unknown" @@ -65,16 +67,12 @@ def auth_hash=(auth) auth[:extra][:raw_info] = auth[:extra][:raw_info].attributes auth[:extra][:response_object] = nil end - case auth[:provider] - when "shibboleth", "developer" - self.provider = Provider::SHIBBOLETH - when "saml" + when "saml", "developer" self.provider = Provider::SAML when "identity" self.provider = Provider::LOCAL end - super(auth) end diff --git a/app/models/user.rb b/app/models/user.rb index ea1c1cc89..ca027c6a1 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -11,9 +11,6 @@ # * For local authentication, the user either requests to join a particular # institution, or is invited into a particular institution by a sysadmin at # the time they are invited to register. -# * For Shibboleth authentication, the user's "org DN" provided by the IdP is -# matched against an institution's {Institution#shibboleth_org_dn} property -# at login. # * For SAML authentication, the user is made a member of the institution # matching the request host at login. # @@ -42,12 +39,12 @@ class User < ApplicationRecord include Breadcrumb - # Only Shibboleth users will have one of these. + # Only UIUC users will have one of these. belongs_to :affiliation, optional: true belongs_to :caching_submittable_collections_task, class_name: "Task", optional: true belongs_to :institution, optional: true - # Only Shibboleth users will have one of these. + # Only UIUC users will have one of these. has_one :department has_one :credential, inverse_of: :user # This relationship is not "live" - it needs to be populated manually @@ -118,17 +115,6 @@ def self.fetch_from_omniauth_saml(auth, User.find_by_email(email) end - ## - # @param auth [OmniAuth::AuthHash] - # @return [User] Instance corresponding to the given auth hash, or nil if not - # found. - # - def self.fetch_from_omniauth_shibboleth(auth) - auth = auth.deep_symbolize_keys - email = auth.dig(:info, :email) - User.find_by_email(email) - end - ## # @param string [String] Autocomplete text field string. # @return [User] Instance corresponding to the given string. May be `nil`. @@ -147,18 +133,14 @@ def self.from_autocomplete_string(string) ## # @param auth [OmniAuth::AuthHash, Hash] # @param institution [Institution] The returned user will be assigned to this - # institution. (Only applies to SAML users. Shibboleth users will - # instead be assigned to the institution matching the "org DN" - # attribute in the auth hash, and local credential users will be - # assigned to the institution of the corresponding {Invitee}.) + # institution. (Only applies to SAML users. Local credential users + # will be assigned to the institution of the corresponding {Invitee}.) # @return [User] Instance corresponding to the given auth hash. If one was # not found, it is created. # def self.from_omniauth(auth, institution:) case auth[:provider] - when "developer", "shibboleth" - User.from_omniauth_shibboleth(auth) - when "saml" + when "saml", "developer" User.from_omniauth_saml(auth, institution: institution) when "identity" User.from_omniauth_local(auth) @@ -194,25 +176,13 @@ def self.from_omniauth_saml(auth, institution:) email_location: institution.saml_email_location, email_attribute: institution.saml_email_attribute) user ||= User.new - user.send(:update_from_saml, auth, institution) - user - end - - ## - # @private - # - def self.from_omniauth_shibboleth(auth) - user = User.fetch_from_omniauth_shibboleth(auth) || User.new - user.send(:update_from_shibboleth, auth) + user.send(:update_from_omniauth_saml, auth, institution) user end ## # Performs an LDAP query to determine whether the instance belongs to the - # given AD group. Groups are assigned only to - # {AuthMethod::SHIBBOLETH Shibboleth users}, but this method will work for - # users who have signed in via Shibboleth before, had groups assigned to them - # then, and then signed in via some other method later. + # given AD group. Groups are exposed only to UIUC users. # # N.B.: in development and test environments, no query is executed, and # instead the return value is `true` if the `ad.groups` key in the @@ -498,7 +468,7 @@ def invitee ## # @return [String] The NetID (the user component of the email). This works # regardless of authentication method, even though - # technically only UofI Shibboleth users have NetIDs. + # technically only UofI users have NetIDs. # def netid return nil unless self.email.respond_to?(:split) @@ -574,7 +544,7 @@ def sync_credential_properties # @param institution [Institution] Only set if the instance does not already # have an institution set. # - def update_from_saml(auth, institution) + def update_from_omniauth_saml(auth, institution) auth = auth.deep_symbolize_keys attrs = auth[:extra][:raw_info].attributes.deep_stringify_keys @@ -593,6 +563,10 @@ def update_from_saml(auth, institution) self.name = [attrs[institution.saml_first_name_attribute]&.first, attrs[institution.saml_last_name_attribute]&.first].join(" ").strip self.name = self.email if self.name.blank? + + # Only UIUC users will be expected to have these. + self.affiliation = Affiliation.from_omniauth(auth) + self.department = Department.from_omniauth(auth) begin self.save! rescue => e @@ -605,45 +579,4 @@ def update_from_saml(auth, institution) end end - ## - # @param auth [OmniAuth::AuthHash] - # - def update_from_shibboleth(auth) - auth = auth.deep_stringify_keys - # By design, logging in overwrites certain existing user properties with - # current information from the Shib IdP. By supplying this custom - # attribute, we can preserve the user properties that are set up in test - # fixture data. - return if auth.dig("extra", "raw_info", "overwriteUserAttrs") == "false" - - # N.B.: we have to be careful accessing this hash because not all providers - # will populate all properties. - self.email = auth["info"]["email"] - self.name = "#{auth.dig("extra", "raw_info", "givenName")} "\ - "#{auth.dig("extra", "raw_info", "sn")}" - self.name = self.email if self.name.blank? - org_dn = auth.dig("extra", "raw_info", "org-dn") - # UIS accounts will not have an org DN--eventually these users will be - # converted into OpenAthens/SAML users and moved into the UIS space, and we - # will fall back to nil here instead of UIUC. - unless self.institution - self.institution = org_dn.present? ? - Institution.find_by_shibboleth_org_dn(org_dn) : - Institution.find_by_key("uiuc") - end - self.affiliation = Affiliation.from_shibboleth(auth) - dept = auth.dig("extra", "raw_info", "departmentCode") - self.department = Department.create!(name: dept) if dept - begin - self.save! - rescue => e - @message = IdealsMailer.error_body(e, - detail: "[user: #{self.as_json}]\n\n"\ - "[auth hash: #{auth.as_json}]", - user: self) - Rails.logger.error(@message) - IdealsMailer.error(@message).deliver_later unless Rails.env.development? - end - end - end diff --git a/app/models/user_group.rb b/app/models/user_group.rb index 1f29e4c08..9084a8093 100644 --- a/app/models/user_group.rb +++ b/app/models/user_group.rb @@ -110,7 +110,7 @@ def breadcrumb_parent # OR (has an IP address or hostname matching one of the host patterns) # OR (belongs to an associated department) # OR (is of an associated affiliation) - # OR (is a Shibboleth user AND belongs to an associated AD Group) + # OR (belongs to an associated AD Group) # ``` # # @param user [User] diff --git a/app/policies/institution_policy.rb b/app/policies/institution_policy.rb index f8d69ae6b..b5f8ee013 100644 --- a/app/policies/institution_policy.rb +++ b/app/policies/institution_policy.rb @@ -63,10 +63,6 @@ def edit_settings update_settings end - def edit_shibboleth_authentication - edit_local_authentication - end - def edit_theme edit_settings end diff --git a/app/views/institutions/_shibboleth_authentication_form.html.haml b/app/views/institutions/_shibboleth_authentication_form.html.haml deleted file mode 100644 index b5c151655..000000000 --- a/app/views/institutions/_shibboleth_authentication_form.html.haml +++ /dev/null @@ -1,38 +0,0 @@ --# @param institution [Institution] - -= form_for(institution, url: institution_settings_path(institution), remote: true) do |f| - .error-messages - -# populated via ajax by shared/_validation_messages.js.erb - - .mb-3 - .form-check - = f.check_box(:shibboleth_auth_enabled, - value: "true", - checked: f.object.shibboleth_auth_enabled, - id: "shibboleth-auth-enabled", - class: "form-check-input") - %label.form-check-label{for: "shibboleth-auth-enabled"} - Enabled - - .mb-3 - = f.label :shibboleth_org_dn, "Organization DN", class: "required" - = f.text_field :shibboleth_org_dn, class: "form-control" - .mb-3 - = f.label :shibboleth_email_attribute, "Email Attribute", class: "required" - = f.text_field :shibboleth_email_attribute, class: "form-control" - .mb-3 - = f.label :shibboleth_name_attribute, "Name Attribute", class: "required" - = f.text_field :shibboleth_name_attribute, class: "form-control" - .mb-3 - = f.label :shibboleth_extra_attributes, "Extra Attributes" - = text_area_tag "institution[shibboleth_extra_attributes]", - f.object.shibboleth_extra_attributes&.join(","), - rows: 5, class: "form-control" - %p.form-text - = icon_for(:info) - Separate attributes with commas. - - .clearfix - .float-end - %button.btn.btn-light{"data-bs-dismiss": "modal", type: "button"} Cancel - = f.submit("Save Changes", class: "btn btn-primary") diff --git a/app/views/institutions/_show_authentication_tab.html.haml b/app/views/institutions/_show_authentication_tab.html.haml index a57b80dc1..ff0c3630f 100644 --- a/app/views/institutions/_show_authentication_tab.html.haml +++ b/app/views/institutions/_show_authentication_tab.html.haml @@ -13,14 +13,6 @@ role: "tab", "aria-controls": "saml-tab-pane", "aria-selected": "false"} SAML - - if @institution.key == "uiuc" - %li.nav-item{role: "presentation"} - %button#shibboleth-tab.nav-link{"data-bs-toggle": "tab", - "data-bs-target": "#shibboleth-tab-pane", - type: "button", - role: "tab", - "aria-controls": "shibboleth-tab-pane", - "aria-selected": "true"} Shibboleth .tab-content #local-tab-pane.tab-pane.fade.show.active{role: "tabpanel", @@ -238,59 +230,9 @@ - else = boolean(false, style: :word, false_value: "NOT SET") - - if @institution.key == "uiuc" - #shibboleth-tab-pane.tab-pane.fade{role: "tabpanel", - "aria-labelledby": "shibboleth-tab", - tabindex: "0"} - - if policy(@institution).edit_shibboleth_authentication? - .btn-group.float-end{role: "group"} - -# Edit button - %button.btn.btn-light.edit-shibboleth-authentication{"data-bs-target": "#edit-shibboleth-authentication-modal", - "data-bs-toggle": "modal", - type: "button", - "data-institution-key": @institution.key } - %i.fa.fa-pencil-alt - Edit - - .clearfix.mb-3 - - %dl - %dt Enabled - %dd - = boolean(@institution.shibboleth_auth_enabled, style: :word) - %dt Organization DN - %dd - - if @institution.shibboleth_org_dn.present? - %code= @institution.shibboleth_org_dn - - else - = boolean(false, style: :word, false_value: "NOT SET") - %dt Email Attribute - %dd - - if @institution.shibboleth_email_attribute.present? - %code= @institution.shibboleth_email_attribute - - else - = boolean(false, style: :word, false_value: "NOT SET") - %dt Name Attribute - %dd - - if @institution.shibboleth_name_attribute.present? - %code= @institution.shibboleth_name_attribute - - else - = boolean(false, style: :word, false_value: "NOT SET") - %dt Extra Attributes - %dd - - if @institution.shibboleth_extra_attributes.present? - %ul - - @institution.shibboleth_extra_attributes.sort.each do |attr| - %li - %code= attr - - else - = boolean(false, style: :word, false_value: "NONE") - = render partial: "shared/xhr_modal", locals: { id: "edit-local-authentication-modal", title: "Edit Local Authentication" } = render partial: "shared/xhr_modal", locals: { id: "edit-saml-authentication-modal", title: "Edit SAML Authentication", size: :xl } = render partial: "shared/xhr_modal", locals: { id: "supply-saml-configuration-modal", title: "Supply SAML Configuration" } -= render partial: "shared/xhr_modal", - locals: { id: "edit-shibboleth-authentication-modal", title: "Edit Shibboleth Authentication" } diff --git a/app/views/sessions/_login_form.html.haml b/app/views/sessions/_login_form.html.haml index 8e3bb6b88..9cff3fb92 100644 --- a/app/views/sessions/_login_form.html.haml +++ b/app/views/sessions/_login_form.html.haml @@ -4,21 +4,20 @@ .carousel-inner -# OmniAuth Developer - if Rails.env.development? - = form_tag("/auth/developer", method: :post) do - = submit_tag("Log in as OmniAuth Developer (DEVELOPMENT)", - class: "btn btn-primary p-4 mb-3") + .text-center + = form_tag("/auth/developer", method: :post) do + = submit_tag("Log in as OmniAuth Developer (DEVELOPMENT)", + class: "btn btn-secondary p-4 mb-3") -# SAML - - if current_institution&.saml_auth_enabled + - if current_institution&.key == "uiuc" + .carousel-item.uiuc.active + = render partial: "sessions/uiuc_login_form" + - elsif current_institution&.saml_auth_enabled .carousel-item.openathens.active = render partial: "sessions/openathens_login_form" - -# Shibboleth - - if current_institution&.shibboleth_auth_enabled - .carousel-item.shibboleth.active - = render partial: "sessions/shibboleth_login_form" - -# Local Credentials - if current_institution&.local_auth_enabled - .carousel-item.local-credential.p-1{class: (current_institution&.sso_enabled?) ? "" : "active"} + .carousel-item.local-credential.p-1{class: (current_institution&.key == "uiuc" || current_institution&.saml_auth_enabled?) ? "" : "active"} = render partial: "sessions/local_login_form" diff --git a/app/views/sessions/_shibboleth_login_form.html.haml b/app/views/sessions/_uiuc_login_form.html.haml similarity index 100% rename from app/views/sessions/_shibboleth_login_form.html.haml rename to app/views/sessions/_uiuc_login_form.html.haml diff --git a/app/views/user_groups/show.html.haml b/app/views/user_groups/show.html.haml index 6d08abb03..025049dd4 100644 --- a/app/views/user_groups/show.html.haml +++ b/app/views/user_groups/show.html.haml @@ -1,8 +1,8 @@ - provide :body_id, "show_user_group" - provide :title, @user_group.name -- can_edit = policy(@user_group).edit? -- show_shibboleth_dimensions = @user_group.institution&.shibboleth_auth_enabled || !@user_group.institution +- can_edit = policy(@user_group).edit? +- show_uiuc_dimensions = @user_group.institution&.key == "uiuc" || !@user_group.institution .btn-group.float-end{role: "group"} - if can_edit @@ -98,7 +98,7 @@ .row.mb-3 .col-sm-4 - - if show_shibboleth_dimensions + - if show_uiuc_dimensions .card .card-body - if can_edit @@ -112,7 +112,7 @@ %h4.card-title Departments %small - %span.badge.bg-primary SHIBBOLETH ONLY + %span.badge.bg-primary UIUC ONLY - if @departments.any? %ul - @departments.each do |department| @@ -120,7 +120,7 @@ - else %p None .col-sm-4 - - if show_shibboleth_dimensions + - if show_uiuc_dimensions .card .card-body - if can_edit @@ -134,7 +134,7 @@ %h4.card-title Affiliations %small - %span.badge.bg-primary SHIBBOLETH ONLY + %span.badge.bg-primary UIUC ONLY - if @affiliations.any? %ul - @affiliations.each do |affiliation| @@ -143,7 +143,7 @@ %p None .col-sm-4 - - if show_shibboleth_dimensions + - if show_uiuc_dimensions .card .card-body - if can_edit @@ -157,7 +157,7 @@ %h4.card-title AD Groups %small - %span.badge.bg-primary SHIBBOLETH ONLY + %span.badge.bg-primary UIUC ONLY - if @ad_groups.any? %ul - @ad_groups.each do |group| diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb index c6f275277..00d392e26 100644 --- a/config/initializers/omniauth.rb +++ b/config/initializers/omniauth.rb @@ -43,6 +43,11 @@ end Rails.application.config.middleware.use OmniAuth::Builder do + # The developer provider is available only in development and test. + if Rails.env.development? || Rails.env.test? + provider :developer + end + # The identity provider (for local password logins) is available in all # environments. provider :identity, @@ -50,41 +55,6 @@ fields: [:email, :name], locate_conditions: -> (request) { { model.auth_key => request.params['auth_key']&.downcase } } - # The Shibboleth (UIUC) developer provider is available only in development - # and test. - if Rails.env.development? || Rails.env.test? - provider :developer - else - # The real Shibboleth provider is available in all other environments. - # UIUC is the only tenant that uses this, and only because it's what we - # knew how to do long before we added the SAML provider, but switching to - # the SAML provider would require updating our iTrust registrations. - provider :shibboleth, { - uid_field: "eppn", - # N.B.: overwriteUserAttrs is a custom property needed in testing - extra_fields: %w[ - eppn - unscoped-affiliation - uid - sn - org-dn - nickname - givenName - member - iTrustAffiliation - departmentName - programCode - levelCode - overwriteUserAttrs - ], - request_type: "header", - info_fields: { - name: "displayName", - email: "mail" - } - } - end - # SAML (everybody else) is available in all environments. # N.B.: this provider needs further setup at request time, as its properties # will vary depending on which institution's host is being accessed. diff --git a/config/routes.rb b/config/routes.rb index 89654bb0b..ec2102634 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -134,8 +134,6 @@ constraints: lambda { |request| request.xhr? } match "/edit-settings", to: "institutions#edit_settings", via: :get, constraints: lambda { |request| request.xhr? } - match "/edit-shibboleth-authentication", to: "institutions#edit_shibboleth_authentication", via: :get, - constraints: lambda { |request| request.xhr? } match "/edit-theme", to: "institutions#edit_theme", via: :get, constraints: lambda { |request| request.xhr? } match "/element-mappings", to: "institutions#show_element_mappings", via: :get, diff --git a/db/migrate/20240118235844_remove_institution_shibboleth_columns.rb b/db/migrate/20240118235844_remove_institution_shibboleth_columns.rb new file mode 100644 index 000000000..99e636c40 --- /dev/null +++ b/db/migrate/20240118235844_remove_institution_shibboleth_columns.rb @@ -0,0 +1,16 @@ +class RemoveInstitutionShibbolethColumns < ActiveRecord::Migration[7.1] + def up + remove_column :institutions, :shibboleth_org_dn + remove_column :institutions, :shibboleth_auth_enabled + remove_column :institutions, :shibboleth_email_attribute + remove_column :institutions, :shibboleth_extra_attributes + remove_column :institutions, :shibboleth_name_attribute + end + def down + add_column :institutions, :shibboleth_org_dn, :string + add_column :institutions, :shibboleth_auth_enabled, :string + add_column :institutions, :shibboleth_email_attribute, :string + add_column :institutions, :shibboleth_extra_attributes, :string + add_column :institutions, :shibboleth_name_attribute, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index f5dc8673b..a9c7f973b 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[7.1].define(version: 2024_01_19_023719) do +ActiveRecord::Schema[7.1].define(version: 2024_01_18_235844) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" enable_extension "unaccent" @@ -379,7 +379,6 @@ create_table "institutions", force: :cascade do |t| t.string "key", null: false t.string "name", null: false - t.string "shibboleth_org_dn" t.datetime "created_at", null: false t.datetime "updated_at", null: false t.string "fqdn", null: false @@ -429,10 +428,6 @@ t.string "google_analytics_measurement_id" t.boolean "local_auth_enabled", default: true, null: false t.boolean "saml_auth_enabled", default: false, null: false - t.boolean "shibboleth_auth_enabled", default: false, null: false - t.string "shibboleth_email_attribute", default: "mail" - t.string "shibboleth_name_attribute", default: "displayName" - t.text "shibboleth_extra_attributes", default: "[]" t.text "saml_sp_public_cert" t.text "saml_sp_private_key" t.text "deposit_form_disagreement_help", default: "The selections you have made indicate that you are not ready to deposit your dataset. Our curators are available to discuss your dataset with you. Please contact us!", null: false @@ -456,7 +451,6 @@ t.index ["name"], name: "index_institutions_on_name", unique: true t.index ["outgoing_message_queue"], name: "index_institutions_on_outgoing_message_queue", unique: true t.index ["saml_auto_cert_rotation"], name: "index_institutions_on_saml_auto_cert_rotation" - t.index ["shibboleth_org_dn"], name: "index_institutions_on_shibboleth_org_dn", unique: true end create_table "invitees", force: :cascade do |t| diff --git a/test/controllers/institutions_controller_test.rb b/test/controllers/institutions_controller_test.rb index 80832e381..ea3b3e36b 100644 --- a/test/controllers/institutions_controller_test.rb +++ b/test/controllers/institutions_controller_test.rb @@ -31,10 +31,9 @@ class InstitutionsControllerTest < ActionDispatch::IntegrationTest xhr: true, params: { institution: { - name: "New Institution", - key: "new", - fqdn: "new.org", - shibboleth_org_dn: "new" + name: "New Institution", + key: "new", + fqdn: "new.org" } } assert_response :forbidden @@ -46,12 +45,11 @@ class InstitutionsControllerTest < ActionDispatch::IntegrationTest xhr: true, params: { institution: { - name: "New Institution", - service_name: "New", - key: "new", - fqdn: "new.org", - shibboleth_org_dn: "new", - main_website_url: "https://new.org" + name: "New Institution", + service_name: "New", + key: "new", + fqdn: "new.org", + main_website_url: "https://new.org" } } assert_response :ok @@ -65,12 +63,11 @@ class InstitutionsControllerTest < ActionDispatch::IntegrationTest xhr: true, params: { institution: { - name: "New Institution", - service_name: "New", - key: "new", - fqdn: "new.org", - shibboleth_org_dn: "new", - main_website_url: "https://new.org" + name: "New Institution", + service_name: "New", + key: "new", + fqdn: "new.org", + main_website_url: "https://new.org" } } end @@ -416,31 +413,6 @@ class InstitutionsControllerTest < ActionDispatch::IntegrationTest assert_response :ok end - # edit_shibboleth_authentication() - - test "edit_shibboleth_authentication() returns HTTP 404 for unscoped requests" do - host! ::Configuration.instance.main_host - get institution_edit_shibboleth_authentication_path(@institution), xhr: true - assert_response :not_found - end - - test "edit_shibboleth_authentication() returns HTTP 403 for logged-out users" do - get institution_edit_shibboleth_authentication_path(@institution), xhr: true - assert_response :forbidden - end - - test "edit_shibboleth_authentication() returns HTTP 403 for unauthorized users" do - log_in_as(users(:southwest)) - get institution_edit_shibboleth_authentication_path(@institution), xhr: true - assert_response :forbidden - end - - test "edit_shibboleth_authentication() returns HTTP 200" do - log_in_as(users(:southwest_admin)) - get institution_edit_shibboleth_authentication_path(@institution), xhr: true - assert_response :ok - end - # edit_theme() test "edit_theme() returns HTTP 404 for unscoped requests" do diff --git a/test/fixtures/institutions.yml b/test/fixtures/institutions.yml index ce2253d56..b2f0083dc 100644 --- a/test/fixtures/institutions.yml +++ b/test/fixtures/institutions.yml @@ -49,8 +49,6 @@ southeast: fqdn: southeast.edu name: Southeast University service_name: Southeast IR - shibboleth_org_dn: o=Southeast University,dc=southeast,dc=edu - shibboleth_extra_attributes: "<%= JSON.generate(%w(eppn unscoped-affiliation uid sn org-dn givenName member iTrustAffiliation departmentName programCode levelCode overwriteUserAttrs)).gsub('"', '\"') %>" main_website_url: https://southeast.edu feedback_email: feedback@southeast.edu outgoing_message_queue: southeast_to_nowhere diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index 4f1b0e01b..58be55ca1 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -2,8 +2,8 @@ # N.B.: every local-auth user in this file MUST have a corresponding # Credential in `credentials.yml`. # -# N.B. 2: to place a Shibboleth user in an AD group, edit the `ad.groups` key -# in the config/environments/test.yml file. +# N.B. 2: to place a SAML user in an AD group, edit the `ad.groups` key in +# config/environments/test.yml. # ################################# northeast ################################# @@ -52,12 +52,6 @@ southwest_saml: email: saml@southwest.edu enabled: true -southwest_shibboleth: - institution: southwest - name: Southwest University Shibboleth User - email: shib@southwest.edu - enabled: true - ################################### southeast ################################### southeast: diff --git a/test/jobs/cache_submittable_collections_job_test.rb b/test/jobs/cache_submittable_collections_job_test.rb index 2bda4c89b..88c9174ec 100644 --- a/test/jobs/cache_submittable_collections_job_test.rb +++ b/test/jobs/cache_submittable_collections_job_test.rb @@ -3,7 +3,7 @@ class CacheSubmittableCollectionsJobTest < ActiveSupport::TestCase test "perform() updates the Task given to it" do - user = users(:southwest_shibboleth) + user = users(:southwest_saml) task = tasks(:pending) CacheSubmittableCollectionsJob.perform_now(user: user, @@ -23,7 +23,7 @@ class CacheSubmittableCollectionsJobTest < ActiveSupport::TestCase end test "perform() associates submittable Collections with the User" do - user = users(:southwest_shibboleth) + user = users(:southwest_saml) collection = collections(:southwest_unit1_collection1) collection.submitting_users << user collection.save! @@ -39,7 +39,7 @@ class CacheSubmittableCollectionsJobTest < ActiveSupport::TestCase end test "perform() sets submittable_collections_cached_at" do - user = users(:southwest_shibboleth) + user = users(:southwest_saml) CacheSubmittableCollectionsJob.perform_now(user: user, client_ip: "127.0.0.1", @@ -50,7 +50,7 @@ class CacheSubmittableCollectionsJobTest < ActiveSupport::TestCase test "perform() nullifies caching_submittable_collections_task_id when complete" do - user = users(:southwest_shibboleth) + user = users(:southwest_saml) CacheSubmittableCollectionsJob.perform_now(user: user, client_ip: "127.0.0.1", diff --git a/test/models/affiliation_test.rb b/test/models/affiliation_test.rb index 6a0d862a1..4e05914b8 100644 --- a/test/models/affiliation_test.rb +++ b/test/models/affiliation_test.rb @@ -2,110 +2,110 @@ class AffiliationTest < ActiveSupport::TestCase - test "from_shibboleth() returns a correct instance for a UIUC undergraduate + test "from_omniauth() returns a correct instance for a UIUC undergraduate student" do info = { extra: { raw_info: { - iTrustAffiliation: "student;person;phone", - programCode: "", - levelCode: "1U" + Affiliation::ITRUST_AFFILIATION_ATTRIBUTE => %w[student person phone], + Affiliation::ITRUST_PROGRAM_CODE_ATTRIBUTE => "", + Affiliation::ITRUST_LEVEL_CODE_ATTRIBUTE => "1U" } } }.deep_stringify_keys - affiliation = Affiliation.from_shibboleth(info) + affiliation = Affiliation.from_omniauth(info) assert_equal Affiliation::UNDERGRADUATE_STUDENT_KEY, affiliation.key end - test "from_shibboleth() returns a correct instance for a UIUC graduate + test "from_omniauth() returns a correct instance for a UIUC graduate student" do info = { extra: { raw_info: { - iTrustAffiliation: "student;person;phone", - programCode: "", - levelCode: "1V" + Affiliation::ITRUST_AFFILIATION_ATTRIBUTE => %w[student person phone], + Affiliation::ITRUST_PROGRAM_CODE_ATTRIBUTE => "", + Affiliation::ITRUST_LEVEL_CODE_ATTRIBUTE => "1V" } } }.deep_stringify_keys - affiliation = Affiliation.from_shibboleth(info) + affiliation = Affiliation.from_omniauth(info) assert_equal Affiliation::GRADUATE_STUDENT_KEY, affiliation.key end - test "from_shibboleth() returns a correct instance for a UIUC masters + test "from_omniauth() returns a correct instance for a UIUC masters student" do info = { extra: { raw_info: { - iTrustAffiliation: "student;person;phone", - programCode: "something", - levelCode: "1V" + Affiliation::ITRUST_AFFILIATION_ATTRIBUTE => %w[student person phone], + Affiliation::ITRUST_PROGRAM_CODE_ATTRIBUTE => "something", + Affiliation::ITRUST_LEVEL_CODE_ATTRIBUTE => "1V" } } }.deep_stringify_keys - affiliation = Affiliation.from_shibboleth(info) + affiliation = Affiliation.from_omniauth(info) assert_equal Affiliation::MASTERS_STUDENT_KEY, affiliation.key end - test "from_shibboleth() returns a correct instance for a UIUC Ph.D student" do + test "from_omniauth() returns a correct instance for a UIUC Ph.D student" do info = { extra: { raw_info: { - iTrustAffiliation: "student;person;phone", - programCode: "PHD", - levelCode: "" + Affiliation::ITRUST_AFFILIATION_ATTRIBUTE => %w[student person phone], + Affiliation::ITRUST_PROGRAM_CODE_ATTRIBUTE => "PHD", + Affiliation::ITRUST_LEVEL_CODE_ATTRIBUTE => "" } } }.deep_stringify_keys - affiliation = Affiliation.from_shibboleth(info) + affiliation = Affiliation.from_omniauth(info) assert_equal Affiliation::PHD_STUDENT_KEY, affiliation.key end - test "from_shibboleth() returns a correct instance for a UIUC staff member" do + test "from_omniauth() returns a correct instance for a UIUC staff member" do info = { extra: { raw_info: { - iTrustAffiliation: "staff;person;phone", - programCode: nil, - levelCode: nil + Affiliation::ITRUST_AFFILIATION_ATTRIBUTE => %w[staff person phone], + Affiliation::ITRUST_PROGRAM_CODE_ATTRIBUTE => nil, + Affiliation::ITRUST_LEVEL_CODE_ATTRIBUTE => nil } } }.deep_stringify_keys - affiliation = Affiliation.from_shibboleth(info) + affiliation = Affiliation.from_omniauth(info) assert_equal Affiliation::FACULTY_STAFF_KEY, affiliation.key end - test "from_shibboleth() returns nil for an unrecognized affiliation" do + test "from_omniauth() returns nil for an unrecognized affiliation" do info = { extra: { raw_info: { - iTrustAffiliation: "bogus;cats;dogs", - programCode: "", - levelCode: "1U" + Affiliation::ITRUST_AFFILIATION_ATTRIBUTE => %w[bogus cats dogs], + Affiliation::ITRUST_PROGRAM_CODE_ATTRIBUTE => "", + Affiliation::ITRUST_LEVEL_CODE_ATTRIBUTE => "1U" } } }.deep_stringify_keys - assert_nil Affiliation.from_shibboleth(info) + assert_nil Affiliation.from_omniauth(info) end - test "from_shibboleth() returns nil for an unrecognized level code" do + test "from_omniauth() returns nil for an unrecognized level code" do info = { extra: { raw_info: { - iTrustAffiliation: "student;cats;dogs", - programCode: "", - levelCode: "bogus" + Affiliation::ITRUST_AFFILIATION_ATTRIBUTE => %w[student cats dogs], + Affiliation::ITRUST_PROGRAM_CODE_ATTRIBUTE => "", + Affiliation::ITRUST_LEVEL_CODE_ATTRIBUTE => "bogus" } } }.deep_stringify_keys - assert_nil Affiliation.from_shibboleth(info) + assert_nil Affiliation.from_omniauth(info) end - test "from_shibboleth() returns nil for a missing raw_info hash" do + test "from_omniauth() returns nil for a missing raw_info hash" do info = { extra: {} }.deep_stringify_keys - assert_nil Affiliation.from_shibboleth(info) + assert_nil Affiliation.from_omniauth(info) end end diff --git a/test/models/department_test.rb b/test/models/department_test.rb index 457e554a0..9595f83c3 100644 --- a/test/models/department_test.rb +++ b/test/models/department_test.rb @@ -6,6 +6,20 @@ class DepartmentTest < ActiveSupport::TestCase @instance = departments(:basket_weaving) end + # from_omniauth() + + test "from_omniauth() returns a correct instance" do + info = { + extra: { + raw_info: { + Department::ITRUST_DEPARTMENT_CODE_ATTRIBUTE => "bugs" + } + } + } + dept = Department.from_omniauth(info) + assert_equal "bugs", dept.name + end + # name test "name is normalized" do diff --git a/test/models/institution_test.rb b/test/models/institution_test.rb index 76895a7d1..7a0ebbd8d 100644 --- a/test/models/institution_test.rb +++ b/test/models/institution_test.rb @@ -266,14 +266,12 @@ class InstitutionTest < ActiveSupport::TestCase test "auth_enabled?() returns false if no authentication methods are enabled" do @instance.local_auth_enabled = false @instance.saml_auth_enabled = false - @instance.shibboleth_auth_enabled = false assert !@instance.auth_enabled? end test "auth_enabled?() returns false if an authentication method is enabled" do @instance.local_auth_enabled = true @instance.saml_auth_enabled = false - @instance.shibboleth_auth_enabled = false assert @instance.auth_enabled? end @@ -1031,31 +1029,15 @@ class InstitutionTest < ActiveSupport::TestCase assert_equal "test test", @instance.service_name end - # shibboleth_extra_attributes - - test "shibboleth_extra_attributes can be set to a CSV string" do - @instance.update!(shibboleth_extra_attributes: "dogs, cats, foxes") - assert_equal %w(dogs cats foxes), @instance.shibboleth_extra_attributes - end - # sso_enabled?() test "sso_enabled?() returns true when saml_auth_enabled is true" do - @instance.saml_auth_enabled = true - @instance.shibboleth_auth_enabled = false + @instance.saml_auth_enabled = true assert @instance.sso_enabled? end - test "sso_enabled?() returns true when shibboleth_auth_enabled is true" do - @instance.saml_auth_enabled = false - @instance.shibboleth_auth_enabled = true - assert @instance.sso_enabled? - end - - test "sso_enabled?() returns false when both saml_auth_enabled and - shibboleth_auth_enabled are false" do - @instance.saml_auth_enabled = false - @instance.shibboleth_auth_enabled = false + test "sso_enabled?() returns false when saml_auth_enabled is false" do + @instance.saml_auth_enabled = false assert !@instance.sso_enabled? end diff --git a/test/models/login_test.rb b/test/models/login_test.rb index d5b359281..46ee9f4a8 100644 --- a/test/models/login_test.rb +++ b/test/models/login_test.rb @@ -2,6 +2,20 @@ class LoginTest < ActiveSupport::TestCase + class ProviderTest < ActiveSupport::TestCase + + test "all() returns all providers" do + assert_equal [0, 2], Login::Provider.all + end + + test "label_for() returns a correct value" do + assert_equal "Local", Login::Provider.label_for(Login::Provider::LOCAL) + assert_equal "Shibboleth", Login::Provider.label_for(1) + assert_equal "SAML", Login::Provider.label_for(Login::Provider::SAML) + end + + end + setup do @instance = Login.new end @@ -24,11 +38,8 @@ class LoginTest < ActiveSupport::TestCase @instance.auth_hash = { provider: "saml" } assert_equal Login::Provider::SAML, @instance.provider - @instance.auth_hash = { provider: "shibboleth" } - assert_equal Login::Provider::SHIBBOLETH, @instance.provider - @instance.auth_hash = { provider: "developer" } - assert_equal Login::Provider::SHIBBOLETH, @instance.provider + assert_equal Login::Provider::SAML, @instance.provider @instance.auth_hash = { provider: "identity" } assert_equal Login::Provider::LOCAL, @instance.provider diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 623425c44..4b5bb9567 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -46,32 +46,6 @@ class UserTest < ActiveSupport::TestCase } } - SHIBBOLETH_AUTH_HASH = { - provider: "shibboleth", - uid: "ShibbolethUser@example.org", - info: { - name: "Shib Boleth", - email: "ShibbolethUser@example.org" - }, - credentials: {}, - extra: { - raw_info: { - eppn: "ShibbolethUser@example.org", - "unscoped-affiliation": "member;staff;employee", - uid: "example", - sn: "Boleth", - "org-dn": "o=Southeast University,dc=southeast,dc=edu", - nickname: "", - givenName: "Shib", - member: "urn:mace:southeast.edu:urbana:library:units:ideals:library ideals admin", - iTrustAffiliation: "member;staff;employee", - departmentCode: "Example Department", - programCode: nil, - levelCode: nil - } - } - } - # fetch_from_omniauth_local() test "fetch_from_omniauth_local() with a matching email returns the user" do @@ -144,29 +118,6 @@ class UserTest < ActiveSupport::TestCase email_location: Institution::SAMLEmailLocation::ATTRIBUTE) end - # fetch_from_omniauth_shibboleth() - - test "fetch_from_omniauth_shibboleth() with a matching email returns the user" do - user = users(:southwest) - auth = { - provider: "shibboleth", - info: { - email: user.email - } - } - assert_equal user, User.fetch_from_omniauth_shibboleth(auth) - end - - test "fetch_from_omniauth_shibboleth() with a non-matching email returns nil" do - auth = { - provider: "shibboleth", - info: { - email: "bogus@example.org" - } - } - assert_nil User.fetch_from_omniauth_shibboleth(auth) - end - # from_autocomplete_string() test "from_autocomplete_string() returns a user" do @@ -260,36 +211,8 @@ class UserTest < ActiveSupport::TestCase assert_equal "OpenAthensUser@example.org", user.email end - test "from_omniauth() returns a new user when given a Shibboleth auth - hash for which no database match exists" do - user = User.from_omniauth(SHIBBOLETH_AUTH_HASH, - institution: institutions(:southwest)) - assert_equal "Shib Boleth", user.name - assert_equal "ShibbolethUser@example.org", user.email - assert_equal institutions(:southeast), user.institution - assert_equal "Example Department", user.department.name - assert_equal Affiliation.find_by_key(Affiliation::FACULTY_STAFF_KEY), - user.affiliation - end - - test "from_omniauth() returns an existing user when given a Shibboleth auth - hash for which a database match exists" do - @user.update!(email: SHIBBOLETH_AUTH_HASH[:info][:email]) - - user = User.from_omniauth(SHIBBOLETH_AUTH_HASH, - institution: institutions(:southeast)) - assert_equal @user, user - assert_equal "Shib Boleth", user.name - assert_equal "ShibbolethUser@example.org", user.email - # the institution shouldn't change - assert_equal institutions(:southwest), user.institution - assert_equal "Example Department", user.department.name - assert_equal Affiliation.find_by_key(Affiliation::FACULTY_STAFF_KEY), - user.affiliation - end - - test "from_omniauth() returns an updated user when given a Shibboleth - developer auth hash" do + test "from_omniauth() returns an updated user when given a developer auth + hash" do assert_not_nil User.from_omniauth( { provider: "developer", diff --git a/test/policies/credential_policy_test.rb b/test/policies/credential_policy_test.rb index 38d2fbc63..e350b692e 100644 --- a/test/policies/credential_policy_test.rb +++ b/test/policies/credential_policy_test.rb @@ -57,7 +57,7 @@ class CredentialPolicyTest < ActiveSupport::TestCase test "edit_password?() does not authorize non-sysadmins other than the user being edited" do - user = users(:southwest_shibboleth) + user = users(:southwest_saml) context = RequestContext.new(user: user, institution: @credential.user.institution) policy = CredentialPolicy.new(context, @credential) @@ -238,7 +238,7 @@ class CredentialPolicyTest < ActiveSupport::TestCase test "update_password?() does not authorize non-sysadmins other than the user being updated" do - user = users(:southwest_shibboleth) + user = users(:southwest_saml) context = RequestContext.new(user: user, institution: @credential.user.institution) policy = CredentialPolicy.new(context, @credential) diff --git a/test/policies/institution_policy_test.rb b/test/policies/institution_policy_test.rb index 13095ffe0..26e5d4096 100644 --- a/test/policies/institution_policy_test.rb +++ b/test/policies/institution_policy_test.rb @@ -623,59 +623,6 @@ class InstitutionPolicyTest < ActiveSupport::TestCase assert !policy.edit_settings? end - # edit_shibboleth_authentication?() - - test "edit_shibboleth_authentication?() returns false with a nil user" do - context = RequestContext.new(user: nil, - institution: @institution) - policy = InstitutionPolicy.new(context, @institution) - assert !policy.edit_shibboleth_authentication? - end - - test "edit_shibboleth_authentication?() is restrictive by default" do - user = users(:southwest) - context = RequestContext.new(user: user, - institution: @institution) - policy = InstitutionPolicy.new(context, @institution) - assert !policy.edit_shibboleth_authentication? - end - - test "edit_shibboleth_authentication?() authorizes sysadmins" do - user = users(:southwest_sysadmin) - context = RequestContext.new(user: user, - institution: @institution) - policy = InstitutionPolicy.new(context, @institution) - assert policy.edit_shibboleth_authentication? - end - - test "edit_shibboleth_authentication?() authorizes administrators of the same - institution" do - user = users(:southwest_admin) - context = RequestContext.new(user: user, - institution: @institution) - policy = InstitutionPolicy.new(context, user.institution) - assert policy.edit_shibboleth_authentication? - end - - test "edit_shibboleth_authentication?() does not authorize administrators of different - institutions" do - user = users(:southwest_admin) - context = RequestContext.new(user: user, - institution: @institution) - policy = InstitutionPolicy.new(context, @institution) - assert policy.edit_shibboleth_authentication? - end - - test "edit_shibboleth_authentication?() respects role limits" do - # sysadmin user limited to an insufficient role - user = users(:southwest_sysadmin) - context = RequestContext.new(user: user, - institution: user.institution, - role_limit: Role::LOGGED_IN) - policy = InstitutionPolicy.new(context, @institution) - assert !policy.edit_shibboleth_authentication? - end - # edit_theme?() test "edit_theme?() returns false with a nil user" do diff --git a/test/policies/user_policy_test.rb b/test/policies/user_policy_test.rb index a88fdd1c8..5cc69f3d4 100644 --- a/test/policies/user_policy_test.rb +++ b/test/policies/user_policy_test.rb @@ -54,7 +54,7 @@ class UserPolicyTest < ActiveSupport::TestCase test "edit_properties?() does not authorize non-sysadmins other than the one being edited" do - user = users(:southwest_shibboleth) + user = users(:southwest_saml) context = RequestContext.new(user: user, institution: user.institution) policy = UserPolicy.new(context, @object_user) @@ -351,7 +351,7 @@ class UserPolicyTest < ActiveSupport::TestCase end test "show?() does not authorize non-sysadmins" do - user = users(:southwest_shibboleth) + user = users(:southwest_saml) context = RequestContext.new(user: user, institution: user.institution) policy = UserPolicy.new(context, @object_user) @@ -404,7 +404,7 @@ class UserPolicyTest < ActiveSupport::TestCase end test "show_credentials?() does not authorize non-sysadmins" do - user = users(:southwest_shibboleth) + user = users(:southwest_saml) context = RequestContext.new(user: user, institution: user.institution) policy = UserPolicy.new(context, @object_user) @@ -439,7 +439,7 @@ class UserPolicyTest < ActiveSupport::TestCase end test "show_logins?() does not authorize non-sysadmins" do - user = users(:southwest_shibboleth) + user = users(:southwest_saml) context = RequestContext.new(user: user, institution: user.institution) policy = UserPolicy.new(context, @object_user) @@ -474,7 +474,7 @@ class UserPolicyTest < ActiveSupport::TestCase end test "show_properties?() does not authorize non-sysadmins" do - user = users(:southwest_shibboleth) + user = users(:southwest_saml) context = RequestContext.new(user: user, institution: user.institution) policy = UserPolicy.new(context, @object_user) @@ -509,7 +509,7 @@ class UserPolicyTest < ActiveSupport::TestCase end test "show_submittable_collections?() does not authorize non-sysadmins" do - user = users(:southwest_shibboleth) + user = users(:southwest_saml) context = RequestContext.new(user: user, institution: user.institution) policy = UserPolicy.new(context, @object_user) @@ -544,7 +544,7 @@ class UserPolicyTest < ActiveSupport::TestCase end test "show_submitted_items?() does not authorize non-sysadmins" do - user = users(:southwest_shibboleth) + user = users(:southwest_saml) context = RequestContext.new(user: user, institution: user.institution) policy = UserPolicy.new(context, @object_user) @@ -579,7 +579,7 @@ class UserPolicyTest < ActiveSupport::TestCase end test "show_submissions_in_progress?() does not authorize non-sysadmins" do - user = users(:southwest_shibboleth) + user = users(:southwest_saml) context = RequestContext.new(user: user, institution: user.institution) policy = UserPolicy.new(context, @object_user) @@ -614,7 +614,7 @@ class UserPolicyTest < ActiveSupport::TestCase end test "submitted_item_results?() does not authorize non-sysadmins" do - user = users(:southwest_shibboleth) + user = users(:southwest_saml) context = RequestContext.new(user: user, institution: user.institution) policy = UserPolicy.new(context, @object_user) @@ -649,7 +649,7 @@ class UserPolicyTest < ActiveSupport::TestCase end test "update_properties?() does not authorize non-sysadmins" do - user = users(:southwest_shibboleth) + user = users(:southwest_saml) context = RequestContext.new(user: user, institution: user.institution) policy = UserPolicy.new(context, @object_user) @@ -712,7 +712,7 @@ class UserPolicyTest < ActiveSupport::TestCase end test "update_submittable_collections?() does not authorize non-sysadmins" do - user = users(:southwest_shibboleth) + user = users(:southwest_saml) context = RequestContext.new(user: user, institution: user.institution) policy = UserPolicy.new(context, @object_user) diff --git a/test/test_helper.rb b/test/test_helper.rb index 94cb764db..dbaf710b0 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -44,9 +44,9 @@ def fix_bitstream_keys(bitstream) ## # @param user [User] - # @param provider [Symbol] `:saml`, `:shibboleth`, or `:local`. If omitted, - # and the user has an associated {Credential}, - # local is assumed; otherwise :shibboleth. + # @param provider [Symbol] `:saml` or `:local`. If omitted, and the user has + # an associated {Credential}, local is assumed; + # otherwise saml. # def log_in_as(user, provider = nil) raise "User is nil" if user.nil? @@ -55,8 +55,6 @@ def log_in_as(user, provider = nil) unless provider if user.credential provider = :local - elsif user.institution.key == "southeast" - provider = :shibboleth else provider = :saml end @@ -78,29 +76,6 @@ def log_in_as(user, provider = nil) } } } - when :shibboleth - # N.B. 1: See "request_type option" section for info about using - # omniauth-shibboleth in development: - # https://github.com/toyokazu/omniauth-shibboleth - # - # N.B. 2: the keys in the auth hash must be present in - # config/shibboleth.xml. - post "/auth/shibboleth/callback", env: { - "omniauth.auth": { - provider: "shibboleth", - "Shib-Session-ID": SecureRandom.hex, - uid: user.email, - info: { - email: user.email - }, - extra: { - raw_info: { - "org-dn": user.institution.shibboleth_org_dn, - overwriteUserAttrs: "false" - } - } - } - } else post "/auth/identity/callback", params: { auth_key: user.email,