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

Handle multiple apps returned by SAML app data source, where one app exactly matches. #1894

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

MatthewJohn
Copy link
Contributor

In the case where there are two apps: "MyApp" and "MyApp 2".
If the data source for saml apps queried with label "MyApp" and the API returns "MyApp 2" as the first app in the list, the data source will fail.
This change now iterates through all of the apps returned and returning an exact match, if "label" has been specified.

MatthewJohn and others added 2 commits February 8, 2024 08:39
…exactly matches.

In the case where there are two apps: "MyApp" and "MyApp 2".
If the data source for saml apps queried with label "MyApp", if the API returns "MyApp 2" as the first app in the list, the data source will fail.
This change now iterate through all of the apps returned and returning an exact match, if "label" has been specified.
@MatthewJohn MatthewJohn marked this pull request as ready for review February 8, 2024 09:01
@duytiennguyen-okta
Copy link
Contributor

@MatthewJohn Thanks. We will review it

@MatthewJohn
Copy link
Contributor Author

Thanks @duytiennguyen-okta :)

for _, appItx := range appList {
if appItx.Label == filters.Label {
app = appItx
foundMatch = true
Copy link
Contributor

@duytiennguyen-okta duytiennguyen-okta Feb 23, 2024

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh course, yes, apologies!

@duytiennguyen-okta
Copy link
Contributor

If you're ok with the suggestion, I will bring the PR in and update it @MatthewJohn

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