Skip to content

Conversation

@emilhdiaz
Copy link

@lyang this PR adds support for mapping a list of groups/roles in the SAML response to a list of groups/roles that the application expects.

I was able to use this to map group IDs returned by my SAML IdP into application roles expected by my application.

@lyang
Copy link
Owner

lyang commented Nov 17, 2022

Hi @emilhdiaz , thanks for the PR.
I will take a look later. Will likely add test coverage before merging.
Thanks!

@emilhdiaz
Copy link
Author

@lyang Thanks for responding. I'm happy to help with the test coverage if you think the changes are headed in the right direction.

@lyang
Copy link
Owner

lyang commented Nov 21, 2022

Correct me if I'm wrong.

My understanding is that, you not only want to extract a certain (multi-valued) attribute from the response, but also transform the individual raw values in some way.

For e.g. if the response returned a groups attribute that had 3 values [1, 2, 3], you want it to become something like GROUPS: dev, admin, support in the header.

  1. For extracting the multi-value part, I'm more than happy to change the implementation to always use saml_response.attributes.multi(attr). Seems would work for both single-value and multi-value attributes.
  2. For transforming the raw value part. I'm not sure if this should happen in the saml proxy, or else where. It could happen in nginx, for e.g. It could also be changed in your IdP (if you have control over it).

I need to think about the second part a bit more.

Thanks!

@emilhdiaz
Copy link
Author

emilhdiaz commented Nov 22, 2022

Hi @lyang. Yes, that is precisely the idea here: extract a multivalue attribute and translate the values.

  1. Switching to saml_response.attributes.multi(attr) makes sense to me as well 👍🏻
  2. I wish in our use case we had better control over the attributes sent by the IdP. We're using AWS Identity Center (formerly AWS SSO) and unfortunately we can only inject group IDs into the SAML response, not the group names. The application we're protecting has some pre-defined group names we need to map to. Moving this to nginx is definitely an option, but I figured it would be nice to have all the SAML response processing logic consolidated in one place as opposed to spread out over multiple components of the solution. Just some more context in case it helps as you think through this!

Thanks!

@lyang
Copy link
Owner

lyang commented Nov 28, 2022

Just to update, I'm still thinking about a more generalized solution for this, for e.g. what if someone want the value converted to upper or lower case, instead of a static look up?

@emilhdiaz
Copy link
Author

emilhdiaz commented Nov 29, 2022

That is a good point. I guess it would depend on how expressive you'd want the transformation expressions to be:

Single Finite Operations

We could consider simply adding additional mapping configuration options such as

SAML_GROUP_ATTRIBUTE_TRANSFORM_UPPER: true
SAML_GROUP_ATTRIBUTE_TRANSFORM_LOWER: true
SAML_GROUP_ATTRIBUTE_TRANSFORM_CAPITALIZE: true
etc...

Multiple/Chained but Finite Operations

However, if you're thinking of a more generalized transformation language, that would allow for combinations of operations then I think we'd have to consider supporting a list of rules with a simple expression language. Perhaps something like this:

# format

SAML_GROUP_MAPPINGS_<IDX>: "INPUT_VALUE_OR_RE >> op($value [, op($value)])"


# the following are interchangeable examples of capitalizing 
# the input value 'admins' to generate output value 'Admins'

SAML_GROUP_MAPPINGS_0: "admins >> capitalize($value)"
SAML_GROUP_MAPPINGS_0: "admins >> concat( upper(substring($value, 0, 1)), lower(substring($value, 1)) )"

# this one would also capitalize 'admin' -> 'Admin'
SAML_GROUP_MAPPINGS_2: "r/^admins?$/ >> capitalize($value)" 

NOTE:

  • The list of rules are zero indexed and would determine matching and evaluation priority
  • The use of the >> separator helps avoid matching issues with other special characters that may appear in the input value itself. > needs to be escaped in XML and the probability of a value having 2 consecutive escaped >'s seems low to me...

Unbound Operations

Now, if we're talking full access to turing complete language then we're likely using Ruby's eval() function. But that seems both dangerous to me.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 7, 2025

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 9, 2026

Quality Gate Failed Quality Gate failed

Failed conditions
3 Security Hotspots

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants