Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for multiple/mixed attribute types. #245

Open
wants to merge 23 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
dabcaa4
Support both single and multi attribute values
SxDx Feb 11, 2020
3ff376f
Updated README with a single/multi attribute example
SxDx Feb 11, 2020
c199b86
Merge branch 'master' of github.com:apokalipto/devise_saml_authentica…
doconnor-clintel Dec 6, 2023
7ded2d0
Major version bump due to configuration changes
doconnor-clintel Dec 6, 2023
da2aee3
Update config
doconnor-clintel Dec 6, 2023
d836a5f
Refactor
doconnor-clintel Dec 6, 2023
8231f06
Fix model spec
doconnor-clintel Dec 6, 2023
c6a77e7
Fix tests
doconnor-clintel Dec 6, 2023
4456c4b
Fix tests
doconnor-clintel Dec 6, 2023
b04a026
Update config
doconnor-clintel Dec 6, 2023
8f9a92d
Render multiple attribute values if there is an array, such as with g…
doconnor-clintel Dec 6, 2023
73871e3
Pass in groups
doconnor-clintel Dec 6, 2023
58f8ab5
Add failing spec
doconnor-clintel Dec 6, 2023
aee236a
Passing specs
doconnor-clintel Dec 6, 2023
258a4ed
Remove redundant changes
doconnor-clintel Dec 6, 2023
bc3bec4
Remove mutation of global state in favour of original way, just actua…
doconnor-clintel Dec 6, 2023
44e8cf1
Add guard clause
doconnor-clintel Dec 19, 2023
1acc62b
Adjust spec
doconnor-clintel Dec 26, 2023
8503111
Adjust test setup
doconnor-clintel Dec 26, 2023
531099f
Fix tests
doconnor-clintel Dec 26, 2023
57f9434
Adjust the test expectations to work regardless of global state, as w…
doconnor-clintel Dec 26, 2023
1915d80
By default, inform the user if they have not mapped everything and ha…
doconnor-clintel Dec 26, 2023
977acb2
Ensure the custom mapper also maps multiples
doconnor-clintel Dec 26, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,18 @@ Create a YAML file (`config/attribute-map.yml`) that maps SAML attributes with y
```yaml
# attribute-map.yml

"urn:mace:dir:attribute-def:uid": "user_name"
"urn:mace:dir:attribute-def:email": "email"
"urn:mace:dir:attribute-def:name": "last_name"
"urn:mace:dir:attribute-def:givenName": "name"
"urn:mace:dir:attribute-def:email":
"resource_key": "email"
"attribute_type": "single"
"urn:mace:dir:attribute-def:name":
"resource_key": "last_name"
"attribute_type": "single"
"urn:mace:dir:attribute-def:name":
"resource_key": "first_name"
"attribute_type": "single"
"urn:mace:dir:attribute-def:roles":
"resource_key": "roles"
"attribute_type": "multi"
```

##### Attribute map initializer
Expand Down
25 changes: 19 additions & 6 deletions lib/devise_saml_authenticatable/saml_mapped_attributes.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
module SamlAuthenticatable
class SamlMappedAttributes
def initialize(attributes, attribute_map)
raise ArgumentError.new("Expected OneLogin::RubySaml::Attributes, got #{attributes.class.name}") unless attributes.kind_of?(OneLogin::RubySaml::Attributes)

@attributes = attributes
@attribute_map = attribute_map
end
Expand All @@ -10,29 +12,40 @@ def saml_attribute_keys
end

def resource_keys
@attribute_map.values
@attribute_map.values.map { |h| h["resource_key"] }
end

def value_by_resource_key(key)
str_key = String(key)

# Find all of the SAML attributes that map to the resource key
attribute_map_for_key = @attribute_map.select { |_, resource_key| String(resource_key) == str_key }
attribute_map_for_key = @attribute_map.select { |_, config| String(config["resource_key"]) == str_key }

# When an attribute is "multi", return the entire array
# When the attribute is "single", return the first value
saml_value = nil

# Find the first non-nil value
attribute_map_for_key.each_key do |saml_key|
saml_value = value_by_saml_attribute_key(saml_key)
attribute_map_for_key.each_pair do |saml_key, config|
saml_value = value_by_saml_attribute_key(saml_key, config)

break unless saml_value.nil?
end

saml_value
end

def value_by_saml_attribute_key(key)
@attributes[String(key)]
def value_by_saml_attribute_key(key, config)
case config["attribute_type"]
when "multi"
@attributes.multi(String(key))
when "single"
@attributes.single(String(key))
else
warn("SAML attribute behaviour not specified. This relies on Ruby-SAML's OneLogin::RubySaml::Attributes.single_value_compatibility settings. Update attributes-map.yml or your custom resource hook to specify `attribute_type` and `resource_name`")

@attributes[String(key)]
end
end
end
end
2 changes: 1 addition & 1 deletion lib/devise_saml_authenticatable/version.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
module DeviseSamlAuthenticatable
VERSION = "1.9.1"
VERSION = "2.0.0"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this actually a breaking change? It seems like it's backwards-compatible, which is great! In which case, we could release 1.10.0.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this as a major version bump, as the reconfiguration of the attributes-map.yml is a bit of a pain; and people will have to adjust their custom attribute resolvers slightly.

end
4 changes: 2 additions & 2 deletions spec/devise_saml_authenticatable/model_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ def logger; end
Class.new(::DeviseSamlAuthenticatable::DefaultAttributeMapResolver) do
def attribute_map
{
"saml-email-format" => "email",
"saml-name-format" => "name",
"saml-email-format" => { "resource_key" => "email", "attribute_type" => "single" },
"saml-name-format" => { "resource_key" => "name", "attribute_type" => "single" },
}
end
end
Expand Down
29 changes: 24 additions & 5 deletions spec/devise_saml_authenticatable/saml_mapped_attributes_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@
let(:saml_attributes) do
{
doconnor-clintel marked this conversation as resolved.
Show resolved Hide resolved
"first_name" => ["John"],
"last_name"=>["Smith"],
"email"=>["[email protected]"]
"last_name"=> ["Smith"],
"email"=> ["[email protected]"],
"groups" => ["admin", "reporting"],
"multiple_but_single" => ["1", "2", "3"],
}
end

Expand All @@ -27,23 +29,40 @@
saml_keys = ['urn:mace:dir:attribute-def:first_name', 'first_name', 'firstName', 'firstname']

saml_keys.each do |saml_key|
include_examples 'correctly maps the value of the resource key', saml_key, 'first_name', ['John']
include_examples 'correctly maps the value of the resource key', saml_key, 'first_name', 'John'
end
end

context 'last_name' do
saml_keys = ['urn:mace:dir:attribute-def:last_name', 'last_name', 'lastName', 'lastname']

saml_keys.each do |saml_key|
include_examples 'correctly maps the value of the resource key', saml_key, 'last_name', ['Smith']
include_examples 'correctly maps the value of the resource key', saml_key, 'last_name', 'Smith'
end
end

context 'email' do
saml_keys = ['urn:mace:dir:attribute-def:email', 'email_address', 'emailAddress', 'email']

saml_keys.each do |saml_key|
include_examples 'correctly maps the value of the resource key', saml_key, 'email', ['[email protected]']
include_examples 'correctly maps the value of the resource key', saml_key, 'email', '[email protected]'
end
end

context 'multiple groups' do
saml_keys = ['groups']

saml_keys.each do |saml_key|
include_examples 'correctly maps the value of the resource key', saml_key, 'groups', ["admin", "reporting"]
end
end


context 'multiple values, but configured as single' do
saml_keys = ['multiple_but_single']

saml_keys.each do |saml_key|
include_examples 'correctly maps the value of the resource key', saml_key, 'multiple_but_single', "1"
end
end
end
Expand Down
4 changes: 4 additions & 0 deletions spec/features/saml_authentication_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@
click_on "Sign in"
expect(page).to have_content("[email protected]")
expect(page).to have_content("A User")
expect(page).to have_content("GroupA")
expect(page).to have_content("GroupB")
expect(page).to have_content("GroupC")

expect(current_url).to eq("http://localhost:8020/")
end

Expand Down
54 changes: 42 additions & 12 deletions spec/support/attribute-map.yml
Original file line number Diff line number Diff line change
@@ -1,12 +1,42 @@
"urn:mace:dir:attribute-def:first_name": "first_name"
"first_name": "first_name"
"firstName": "first_name"
"firstname": "first_name"
"urn:mace:dir:attribute-def:last_name": "last_name"
"last_name": "last_name"
"lastName": "last_name"
"lastname": "last_name"
"urn:mace:dir:attribute-def:email": "email"
"email_address": "email"
"emailAddress": "email"
"email": "email"
"urn:mace:dir:attribute-def:first_name":
"resource_key": "first_name"
"attribute_type": "single"
"first_name":
"resource_key": "first_name"
"attribute_type": "single"
"firstName":
"resource_key": "first_name"
"attribute_type": "single"
"firstname":
"resource_key": "first_name"
"attribute_type": "single"
"urn:mace:dir:attribute-def:last_name":
"resource_key": "last_name"
"attribute_type": "single"
"last_name":
"resource_key": "last_name"
"attribute_type": "single"
"lastName":
"resource_key": "last_name"
"attribute_type": "single"
"lastname":
"resource_key": "last_name"
"attribute_type": "single"
"urn:mace:dir:attribute-def:email":
"resource_key": "email"
"attribute_type": "single"
"email_address":
"resource_key": "email"
"attribute_type": "single"
"emailAddress":
"resource_key": "email"
"attribute_type": "single"
"email":
"resource_key": "email"
"attribute_type": "single"
"groups":
"resource_key": "groups"
"attribute_type": "multi"
"multiple_but_single":
"resource_key": "multiple_but_single"
"attribute_type": "single"
4 changes: 2 additions & 2 deletions spec/support/attribute_map_resolver.rb.erb
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ class AttributeMapResolver < DeviseSamlAuthenticatable::DefaultAttributeMapResol
Rails.logger.info("[#{self.class.name}] issuer=#{issuer.inspect}")
if issuer == "http://localhost:8009/saml/auth"
{
"myemailaddress" => "email",
"myname" => "name",
"myemailaddress" => {"resource_key" => "email", "attribute_type" => "single"},
"myname" => {"resource_key" => "name", "attribute_type" => "single"},
}
else
{}
Expand Down
11 changes: 10 additions & 1 deletion spec/support/saml_idp_controller.rb.erb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ class SamlIdpController < StubSamlIdp::IdpController
def idp_make_saml_response(_)
attributes = {
name_attribute_key => "A User",
"groups" => ["GroupA", "GroupB", "GroupC"]
}
if include_subject_in_attributes
attributes[email_address_attribute_key] = "[email protected]"
Expand Down Expand Up @@ -50,7 +51,15 @@ class SamlIdpController < StubSamlIdp::IdpController
attributes = opts.fetch(:attributes, {})
if attributes.any?
attribute_items = attributes.map { |format, value|
%[<Attribute Name="#{format}"><AttributeValue>#{value}</AttributeValue></Attribute>]
if value.kind_of?(Array)
generated = %[<Attribute Name="#{format}">]
value.map {|v| generated += %[<AttributeValue>#{v}</AttributeValue>] }
generated += %[</Attribute>]

generated
else
%[<Attribute Name="#{format}"><AttributeValue>#{value}</AttributeValue></Attribute>]
end
}
attribute_statement = %[<AttributeStatement>#{attribute_items.join}</AttributeStatement>]
else
Expand Down
18 changes: 13 additions & 5 deletions spec/support/sp_template.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,15 @@
if attribute_map_resolver == "nil"
create_file 'config/attribute-map.yml', <<-ATTRIBUTES
---
"http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress": email
"http://schemas.xmlsoap.org/ws/2005/05/identity/claims/name": name
"http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress":
"resource_key": "email"
"attribute_type" : "single"
"http://schemas.xmlsoap.org/ws/2005/05/identity/claims/name":
"resource_key": "name"
"attribute_type" : "single"
"groups":
"resource_key": "groups"
"attribute_type" : "multi"
ATTRIBUTES
end

Expand Down Expand Up @@ -111,19 +118,20 @@ def self.entity_id(params)
}
insert_into_file('app/views/home/index.html.erb', after: /\z/) {
<<-HOME
<%= current_user.email %> <%= current_user.name %>
<%= current_user.email %> <%= current_user.name %> <%= current_user.groups %>
<%= form_tag destroy_user_session_path(entity_id: "http://localhost:8020/saml/metadata"), method: :delete do %>
<%= submit_tag "Log out" %>
<% end %>
HOME
}
route "root to: 'home#index'"

# TODO: https://github.com/heartcombo/devise/blob/main/lib/generators/active_record/templates/migration.rb - string vs array
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, potentially there should be some fancy string replacement done in the migration to make it

t.string :groups, array: true

if Rails::VERSION::MAJOR < 6
generate :devise, "user", "email:string", "name:string", "session_index:string"
generate :devise, "user", "email:string", "name:string", "session_index:string", "groups:string"
else
# devise seems to add `email` by default in Rails 6
generate :devise, "user", "name:string", "session_index:string"
generate :devise, "user", "name:string", "session_index:string", "groups:string"
end
gsub_file 'app/models/user.rb', /database_authenticatable.*\n.*/, 'saml_authenticatable'
route "resources :users, only: [:create]"
Expand Down
Loading