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

SAML Initiated Flow #3250

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

Conversation

MostafaAmer2200
Copy link

Overview

This was a requested feature in PR #1514. I've rebased the branch now everything should be up to date. In addition to some tests.

What this PR does / why we need it

This PR will allow users to authenticate using an IdP initiated flow. Before this feature, this only allows authentication through SP flow. This feature will allow to have both.

This addresses #1514 which was never merged since it became outdated. There seems to be a demand for this feature from four years ago and I hope this resolve. Happy to take any feedback to get this moving forward :)

Special notes for your reviewer

@@ -14,6 +14,13 @@ message Client {
bool public = 5;
string name = 6;
string logo_url = 7;
SamlInitiatedConfig saml_initiated = 8;
Copy link
Member

Choose a reason for hiding this comment

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

Adding settings for a particular connector into generic structures is not good. Introducing a special handler for SAML is okay, but adding these options to ALL connectors could be better.

Copy link
Author

Choose a reason for hiding this comment

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

What do you think is an alternative here?

Documentation/connectors/saml.md Outdated Show resolved Hide resolved
api/v2/api_grpc.pb.go Outdated Show resolved Hide resolved
@erhudy
Copy link

erhudy commented Aug 14, 2024

@nabokihms what can we do to get this moving again? We've been using this in production for several months now and it would be very nice to get it integrated into mainline Dex.

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.

4 participants