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

Add all IdPs to the export when ACL allowAll=true #672

Merged
merged 1 commit into from
Aug 22, 2024

Conversation

MKodde
Copy link
Contributor

@MKodde MKodde commented Aug 22, 2024

It makes sense to show all connected IdP's in the CSV export for the surfconext representative. When the ACL is set to allow all IdPs connected to the SP, we should list all IdPs of the test env there.

This can be a long list. But now we would only show the Organizations IdPs, and the explicitly marked test-idps.

See: https://www.pivotaltracker.com/story/show/187805216/comments/242257758

@MKodde MKodde requested a review from quartje August 22, 2024 11:23
@@ -42,7 +42,7 @@
"_id": "0c3febd2-3f67-4b8a-b90d-ce56a3b0abb6",
"version": 0,
"data": {
"entityid": "https://engine.dev.support.surfconext.nl/authentication/idp/metadata2",
"entityid": "https://engine.dev.support.surfconext.nl/authentication/idp/metadata3",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This would not be possible in Manage, have two entities with the same entityId, fixed that in test

🌲

It makes sense to show all connected IdP's in the CSV export for the
surfconext representative. When the ACL is set to allow all IdPs
connected to the SP, we should list all IdPs of the test env there.

This can be a long list. But now we would only show the Organizations
IdPs, and the explicitly marked test-idps.

See: https://www.pivotaltracker.com/story/show/187805216/comments/242257758
@MKodde MKodde force-pushed the feature/add-allow-all-idps-to-export branch from df10cc4 to e0c29f5 Compare August 22, 2024 11:25
@MKodde MKodde requested a review from johanib August 22, 2024 11:37
Copy link
Contributor

@johanib johanib left a comment

Choose a reason for hiding this comment

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

I don't know the context, but it looks OK to me.

@MKodde
Copy link
Contributor Author

MKodde commented Aug 22, 2024

@quartje and @teunfransen will ensure this feature is tested for functional correctness

@MKodde MKodde merged commit 6e5bdc5 into main Aug 22, 2024
3 checks passed
@MKodde MKodde deleted the feature/add-allow-all-idps-to-export branch August 22, 2024 11:42
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