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

fix: omit empty string from name & use case-insensitive equality for comparing SAML attributes #1654

Merged
merged 4 commits into from
Jul 12, 2024

Conversation

kangmingtay
Copy link
Member

@kangmingtay kangmingtay commented Jul 11, 2024

What kind of change does this PR introduce?

  • Fixes an issue where the SAML mapping is incorrect when a Default is specified in the mapping and the FriendlyName in the SAML Assertion is an empty string
  • Use case-insensitive equality for mapping SAML attributes
  • Tested with a trial okta account

What is the current behavior?

  • During the SAML assertion process, the SAMLAttributeMapping struct defaults the Name to an empty string. When combined with a SAML assertion that has an empty string set in the FriendlyName, this causes an incorrect mapping when a default mapping is provided. (see test case
  • Use case-sensitive equality for mapping SAML attributes

Please link any relevant issues here.

What is the new behavior?

Feel free to include screenshots if it includes visual changes.

Additional context

Add any other context or screenshots.

@kangmingtay kangmingtay requested a review from a team as a code owner July 11, 2024 22:55
@kangmingtay kangmingtay changed the title fix: omit empty string from name fix: omit empty string from name & use case-insensitive equality for comparing SAML attributes Jul 11, 2024
@coveralls
Copy link

coveralls commented Jul 11, 2024

Pull Request Test Coverage Report for Build 9905453274

Details

  • 5 of 5 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.006%) to 58.028%

Totals Coverage Status
Change from base Build 9899452117: 0.006%
Covered Lines: 8815
Relevant Lines: 15191

💛 - Coveralls

@J0 J0 merged commit bf5381a into master Jul 12, 2024
2 checks passed
@J0 J0 deleted the km/fix-saml-assertion branch July 12, 2024 09:43
J0 pushed a commit that referenced this pull request Jul 12, 2024
🤖 I have created a release *beep* *boop*
---


##
[2.155.2](v2.155.1...v2.155.2)
(2024-07-12)


### Bug Fixes

* improve session error logging
([#1655](#1655))
([5a6793e](5a6793e))
* omit empty string from name & use case-insensitive equality for
comparing SAML attributes
([#1654](#1654))
([bf5381a](bf5381a))
* set rate limit log level to warn
([#1652](#1652))
([10ca9c8](10ca9c8))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
uxodb pushed a commit to uxodb/auth that referenced this pull request Nov 13, 2024
…comparing SAML attributes (supabase#1654)

## What kind of change does this PR introduce?
* Fixes an issue where the SAML mapping is incorrect when a `Default` is
specified in the mapping and the `FriendlyName` in the SAML Assertion is
an empty string
* Use case-insensitive equality for mapping SAML attributes
* Tested with a trial okta account

## What is the current behavior?
* During the SAML assertion process, the `SAMLAttributeMapping` struct
defaults the `Name` to an empty string. When combined with a SAML
assertion that has an empty string set in the `FriendlyName`, this
causes an incorrect mapping when a default mapping is provided. (see
[test
case](https://github.com/supabase/auth/pull/1654/files#diff-e708a1335880a92063e8b45f3a06eb96931ab0ee46b94cdf6bf5f566640f7a3cR247-R272)
* Use case-sensitive equality for mapping SAML attributes

Please link any relevant issues here.

## What is the new behavior?

Feel free to include screenshots if it includes visual changes.

## Additional context

Add any other context or screenshots.
uxodb pushed a commit to uxodb/auth that referenced this pull request Nov 13, 2024
🤖 I have created a release *beep* *boop*
---


##
[2.155.2](supabase/auth@v2.155.1...v2.155.2)
(2024-07-12)


### Bug Fixes

* improve session error logging
([supabase#1655](supabase#1655))
([5a6793e](supabase@5a6793e))
* omit empty string from name & use case-insensitive equality for
comparing SAML attributes
([supabase#1654](supabase#1654))
([bf5381a](supabase@bf5381a))
* set rate limit log level to warn
([supabase#1652](supabase#1652))
([10ca9c8](supabase@10ca9c8))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
LashaJini pushed a commit to LashaJini/auth that referenced this pull request Nov 13, 2024
…comparing SAML attributes (supabase#1654)

## What kind of change does this PR introduce?
* Fixes an issue where the SAML mapping is incorrect when a `Default` is
specified in the mapping and the `FriendlyName` in the SAML Assertion is
an empty string
* Use case-insensitive equality for mapping SAML attributes
* Tested with a trial okta account

## What is the current behavior?
* During the SAML assertion process, the `SAMLAttributeMapping` struct
defaults the `Name` to an empty string. When combined with a SAML
assertion that has an empty string set in the `FriendlyName`, this
causes an incorrect mapping when a default mapping is provided. (see
[test
case](https://github.com/supabase/auth/pull/1654/files#diff-e708a1335880a92063e8b45f3a06eb96931ab0ee46b94cdf6bf5f566640f7a3cR247-R272)
* Use case-sensitive equality for mapping SAML attributes

Please link any relevant issues here.

## What is the new behavior?

Feel free to include screenshots if it includes visual changes.

## Additional context

Add any other context or screenshots.
LashaJini pushed a commit to LashaJini/auth that referenced this pull request Nov 13, 2024
🤖 I have created a release *beep* *boop*
---


##
[2.155.2](supabase/auth@v2.155.1...v2.155.2)
(2024-07-12)


### Bug Fixes

* improve session error logging
([supabase#1655](supabase#1655))
([5a6793e](supabase@5a6793e))
* omit empty string from name & use case-insensitive equality for
comparing SAML attributes
([supabase#1654](supabase#1654))
([bf5381a](supabase@bf5381a))
* set rate limit log level to warn
([supabase#1652](supabase#1652))
([10ca9c8](supabase@10ca9c8))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
LashaJini pushed a commit to LashaJini/auth that referenced this pull request Nov 15, 2024
…comparing SAML attributes (supabase#1654)

## What kind of change does this PR introduce?
* Fixes an issue where the SAML mapping is incorrect when a `Default` is
specified in the mapping and the `FriendlyName` in the SAML Assertion is
an empty string
* Use case-insensitive equality for mapping SAML attributes
* Tested with a trial okta account

## What is the current behavior?
* During the SAML assertion process, the `SAMLAttributeMapping` struct
defaults the `Name` to an empty string. When combined with a SAML
assertion that has an empty string set in the `FriendlyName`, this
causes an incorrect mapping when a default mapping is provided. (see
[test
case](https://github.com/supabase/auth/pull/1654/files#diff-e708a1335880a92063e8b45f3a06eb96931ab0ee46b94cdf6bf5f566640f7a3cR247-R272)
* Use case-sensitive equality for mapping SAML attributes

Please link any relevant issues here.

## What is the new behavior?

Feel free to include screenshots if it includes visual changes.

## Additional context

Add any other context or screenshots.
LashaJini pushed a commit to LashaJini/auth that referenced this pull request Nov 15, 2024
🤖 I have created a release *beep* *boop*
---


##
[2.155.2](supabase/auth@v2.155.1...v2.155.2)
(2024-07-12)


### Bug Fixes

* improve session error logging
([supabase#1655](supabase#1655))
([5a6793e](supabase@5a6793e))
* omit empty string from name & use case-insensitive equality for
comparing SAML attributes
([supabase#1654](supabase#1654))
([bf5381a](supabase@bf5381a))
* set rate limit log level to warn
([supabase#1652](supabase#1652))
([10ca9c8](supabase@10ca9c8))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

3 participants