Skip to content

Commit

Permalink
Adds saml config params to auth and to settings yml
Browse files Browse the repository at this point in the history
* This commit also changes the uid to point to the urn for PPID
and adds additional attrs to statements.
  • Loading branch information
devanshu-m committed Nov 17, 2020
1 parent 25d6cc2 commit 4c95641
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 35 deletions.
8 changes: 4 additions & 4 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class User < ActiveRecord::Base
# Include default devise modules. Others available are:
# :confirmable, :lockable, :timeoutable
# Registration is controlled via settings.yml
devise_list = [:omniauthable, :rememberable, :trackable, omniauth_providers: [:shibboleth], authentication_keys: [:login]]
devise_list = [:omniauthable, :rememberable, :trackable, omniauth_providers: [:saml], authentication_keys: [:login]]
devise_list.prepend(:database_authenticatable) if AuthConfig.use_database_auth?

devise(*devise_list)
Expand Down Expand Up @@ -172,15 +172,15 @@ def self.from_omniauth(auth)
log_omniauth_error(auth)
return User.new
end
user.assign_attributes(display_name: auth.info.display_name, ppid: auth.uid, uid: auth.info.uid)
user.assign_attributes(display_name: auth.info.first_name, ppid: auth.uid, uid: auth.info.net_id)
# tezprox@emory.edu isn't a real email address
user.email = auth.info.uid + '@emory.edu' unless auth.info.uid == 'tezprox'
user.email = auth.info.net_id + '@emory.edu' unless auth.info.net_id == 'tezprox'
user.save
user
end

def self.log_omniauth_error(auth)
if auth.info.uid.empty?
if auth.uid.empty?
Rails.logger.error "Nil user detected: Shibboleth didn't pass a uid for #{auth.inspect}"
else
# Log unauthorized logins to error.
Expand Down
10 changes: 7 additions & 3 deletions config/environments/production.rb
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,14 @@
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.attribute_statements = {
:net_id => ["urn:oid:0.9.2342.19200300.100.1.1"],
:first_name => ["urn:oid:1.3.6.1.4.1.5923.1.1.1.2"],
:last_name => ["urn:oid:2.5.4.4"]
}
config.uid_attribute = "urn:oid:2.5.4.5"
config.security = {
:want_assertions_encrypted => true, #makes a 2nd KeyDescriptor, this one says use="encryption"
:want_assertions_signed => true, # goes on md SPSSODescriptor tag
:want_assertions_signed => true # goes on md SPSSODescriptor tag
}
end
12 changes: 4 additions & 8 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,10 @@
end

devise_for :users, controllers: { omniauth_callbacks: 'users/omniauth_callbacks', sessions: 'users/sessions' }, format: false
unless AuthConfig.use_database_auth?
devise_scope :user do
get 'sign_in', to: 'users/sessions#new', as: :new_user_session
get 'sign_out', to: 'users/sessions#destroy', as: :destroy_user_session
match '/users/auth/:provider', to: 'users/omniauth_callbacks#passthru', as: :user_omniauth_authorize, via: [:get, :post]
Avalon::Authentication::Providers.collect { |provider| provider[:provider] }.uniq.each do |provider_name|
match "/users/auth/#{provider_name}/callback", to: "users/omniauth_callbacks##{provider_name}", as: "user_omniauth_callback_#{provider_name}".to_sym, via: [:get, :post]
end
devise_scope :user do
match '/users/auth/:provider', to: 'users/omniauth_callbacks#passthru', as: :user_omniauth_authorize, via: [:get, :post]
Avalon::Authentication::Providers.collect { |provider| provider[:provider] }.uniq.each do |provider_name|
match "/users/auth/#{provider_name}/callback", to: "users/omniauth_callbacks##{provider_name}", as: "user_omniauth_callback_#{provider_name}".to_sym, via: [:get, :post]
end
end

Expand Down
5 changes: 5 additions & 0 deletions config/settings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -82,4 +82,9 @@ auth:
:oauth_credentials:
<%= ENV['LTI_AUTH_KEY'] %>: <%= ENV['LTI_AUTH_SECRET'] %>
<% end %>
<% if ENV['SP_KEY'] %>
- :name: Saml
:provider: :saml
:hidden: false
<% end %>
# google_analytics_tracking_id: "someid"
13 changes: 12 additions & 1 deletion lib/avalon/authentication.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,18 @@ def self.load_configs
end

if ENV['SP_KEY']
Config << { name: 'saml', provider: :saml, hidden: false, params: {} }
Config << { name: 'saml', provider: :saml, hidden: false, params: { :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

Providers = Config.reject {|provider| provider[:provider].blank? }
Expand Down
8 changes: 4 additions & 4 deletions spec/controllers/omniauth_callbacks_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,8 @@
provider: 'shibboleth',
uid: "P0000001",
info: {
display_name: "Brian Wilson",
uid: 'brianbboys1967'
first_name: "Brian Wilson",
net_id: 'brianbboys1967'
}
)

Expand All @@ -164,14 +164,14 @@
end

it "redirects to origin" do
post :shibboleth
post :saml
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
post :saml
expect(response.redirect_url).to include 'http://test.host/'
end
end
Expand Down
30 changes: 15 additions & 15 deletions spec/models/user_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,8 @@
provider: 'shibboleth',
uid: "P0000001",
info: {
display_name: "Brian Wilson",
uid: 'brianbboys1967'
first_name: "Brian Wilson",
net_id: 'brianbboys1967'
}
)
end
Expand All @@ -186,10 +186,10 @@
expect(user.provider).to eq 'shibboleth'
end
it "has a uid" do
expect(user.uid).to eq auth_hash.info.uid
expect(user.uid).to eq auth_hash.info.net_id
end
it "has a name" do
expect(user.display_name).to eq auth_hash.info.display_name
expect(user.display_name).to eq auth_hash.info.first_name
end
it "has a PPID" do
expect(user.ppid).to eq auth_hash.uid
Expand All @@ -202,23 +202,23 @@
provider: 'shibboleth',
uid: "P0000001",
info: {
display_name: "Boaty McBoatface",
uid: 'brianbboys1968'
first_name: "Boaty McBoatface",
net_id: 'brianbboys1968'
}
)
end

it "updates ppid and display_name with values from shibboleth" do
expect(user.uid).to eq auth_hash.info.uid
expect(user.uid).to eq auth_hash.info.net_id
expect(user.ppid).to eq auth_hash.uid
expect(user.display_name).to eq auth_hash.info.display_name
expect(user.display_name).to eq auth_hash.info.first_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.uid).not_to eq auth_hash.info.net_id
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
expect(user.display_name).not_to eq auth_hash.info.first_name
expect(user.display_name).to eq updated_auth_hash.info.first_name
end
end

Expand All @@ -236,8 +236,8 @@
provider: 'shibboleth',
uid: 'P0000003',
info: {
display_name: 'Fake Person',
uid: 'egnetid'
first_name: 'Fake Person',
net_id: 'egnetid'
}
)
end
Expand All @@ -258,8 +258,8 @@
provider: 'shibboleth',
uid: '',
info: {
display_name: '',
uid: ''
first_name: '',
net_id: ''
}
)
end
Expand Down

0 comments on commit 4c95641

Please sign in to comment.