revert: "feat(IDP): use single callback endpoint"

This reverts commit e126ccc9aa. # Which Problems Are Solved #8295 introduced the possibility to handle idps on a single callback, but broke current setups. # How the Problems Are Solved - Revert the change until a proper solution is found. Revert is needed as docs were also changed. # Additional Changes None. # Additional Context - relates to #8295
2025-08-12 03:57:32 +00:00 · 2024-07-24 14:29:05 +02:00
parent c3f8439a49
commit 8d13247413
24 changed files with 65 additions and 80 deletions
--- a/internal/api/idp/idp.go
+++ b/internal/api/idp/idp.go
@@ -9,11 +9,13 @@ import (
 	"io"
 	"net/http"

+	"github.com/crewjam/saml"
 	"github.com/gorilla/mux"
 	"github.com/zitadel/logging"

 	"github.com/zitadel/zitadel/internal/api/authz"
 	http_utils "github.com/zitadel/zitadel/internal/api/http"
+	"github.com/zitadel/zitadel/internal/api/ui/login"
 	"github.com/zitadel/zitadel/internal/command"
 	"github.com/zitadel/zitadel/internal/crypto"
 	"github.com/zitadel/zitadel/internal/form"
@@ -51,13 +53,13 @@ const (
 )

 type Handler struct {
-	commands                *command.Commands
-	queries                 *query.Queries
-	parser                  *form.Parser
-	encryptionAlgorithm     crypto.EncryptionAlgorithm
-	callbackURL             func(ctx context.Context) string
-	samlRootURL             func(ctx context.Context, idpID string) string
-	loginUICallbackRedirect func(w http.ResponseWriter, r *http.Request, state string) bool
+	commands            *command.Commands
+	queries             *query.Queries
+	parser              *form.Parser
+	encryptionAlgorithm crypto.EncryptionAlgorithm
+	callbackURL         func(ctx context.Context) string
+	samlRootURL         func(ctx context.Context, idpID string) string
+	loginSAMLRootURL    func(ctx context.Context) string
 }

 type externalIDPCallbackData struct {
@@ -89,22 +91,27 @@ func SAMLRootURL(externalSecure bool) func(ctx context.Context, idpID string) st
 	}
 }

+func LoginSAMLRootURL(externalSecure bool) func(ctx context.Context) string {
+	return func(ctx context.Context) string {
+		return http_utils.BuildOrigin(authz.GetInstance(ctx).RequestedHost(), externalSecure) + login.HandlerPrefix + login.EndpointSAMLACS
+	}
+}
+
 func NewHandler(
 	commands *command.Commands,
 	queries *query.Queries,
 	encryptionAlgorithm crypto.EncryptionAlgorithm,
 	externalSecure bool,
 	instanceInterceptor func(next http.Handler) http.Handler,
-	loginUICallbackRedirect func(w http.ResponseWriter, r *http.Request, state string) bool,
 ) http.Handler {
 	h := &Handler{
-		commands:                commands,
-		queries:                 queries,
-		parser:                  form.NewParser(),
-		encryptionAlgorithm:     encryptionAlgorithm,
-		callbackURL:             CallbackURL(externalSecure),
-		loginUICallbackRedirect: loginUICallbackRedirect,
-		samlRootURL:             SAMLRootURL(externalSecure),
+		commands:            commands,
+		queries:             queries,
+		parser:              form.NewParser(),
+		encryptionAlgorithm: encryptionAlgorithm,
+		callbackURL:         CallbackURL(externalSecure),
+		samlRootURL:         SAMLRootURL(externalSecure),
+		loginSAMLRootURL:    LoginSAMLRootURL(externalSecure),
 	}

 	router := mux.NewRouter()
@@ -182,6 +189,22 @@ func (h *Handler) handleMetadata(w http.ResponseWriter, r *http.Request) {
 	}
 	metadata := sp.ServiceProvider.Metadata()

+	for i, spDesc := range metadata.SPSSODescriptors {
+		spDesc.AssertionConsumerServices = append(
+			spDesc.AssertionConsumerServices,
+			saml.IndexedEndpoint{
+				Binding:  saml.HTTPPostBinding,
+				Location: h.loginSAMLRootURL(ctx),
+				Index:    len(spDesc.AssertionConsumerServices) + 1,
+			}, saml.IndexedEndpoint{
+				Binding:  saml.HTTPArtifactBinding,
+				Location: h.loginSAMLRootURL(ctx),
+				Index:    len(spDesc.AssertionConsumerServices) + 2,
+			},
+		)
+		metadata.SPSSODescriptors[i] = spDesc
+	}
+
 	buf, _ := xml.MarshalIndent(metadata, "", "  ")
 	w.Header().Set("Content-Type", "application/samlmetadata+xml")
 	_, err = w.Write(buf)
@@ -195,9 +218,6 @@ func (h *Handler) handleACS(w http.ResponseWriter, r *http.Request) {
 	ctx := r.Context()
 	data := parseSAMLRequest(r)

-	if h.loginUICallbackRedirect(w, r, data.RelayState) {
-		return
-	}
 	provider, err := h.getProvider(ctx, data.IDPID)
 	if err != nil {
 		http.Error(w, err.Error(), http.StatusBadRequest)
@@ -252,9 +272,6 @@ func (h *Handler) handleCallback(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, err.Error(), http.StatusBadRequest)
 		return
 	}
-	if h.loginUICallbackRedirect(w, r, data.State) {
-		return
-	}
 	intent, err := h.commands.GetActiveIntent(ctx, data.State)
 	if err != nil {
 		if zerrors.IsNotFound(err) {
--- a/internal/api/ui/login/external_provider_handler.go
+++ b/internal/api/ui/login/external_provider_handler.go
@@ -6,7 +6,6 @@ import (
 	"strings"

 	"github.com/crewjam/saml/samlsp"
-	"github.com/muhlemmer/gu"
 	"github.com/zitadel/logging"
 	"github.com/zitadel/oidc/v3/pkg/client/rp"
 	"github.com/zitadel/oidc/v3/pkg/oidc"
@@ -14,8 +13,8 @@ import (
 	"golang.org/x/text/language"

 	"github.com/zitadel/zitadel/internal/api/authz"
+	http_utils "github.com/zitadel/zitadel/internal/api/http"
 	http_mw "github.com/zitadel/zitadel/internal/api/http/middleware"
-	idp_api "github.com/zitadel/zitadel/internal/api/idp"
 	"github.com/zitadel/zitadel/internal/crypto"
 	"github.com/zitadel/zitadel/internal/domain"
 	"github.com/zitadel/zitadel/internal/eventstore/v1/models"
@@ -38,8 +37,6 @@ import (
 const (
 	queryIDPConfigID           = "idpConfigID"
 	tmplExternalNotFoundOption = "externalnotfoundoption"
-
-	prefixLoginState = "LOGIN_"
 )

 type externalIDPData struct {
@@ -101,36 +98,6 @@ type externalRegisterFormData struct {
 	TermsConfirm           bool                `schema:"terms-confirm"`
 }

-// IDPCallbackRedirect checks if the state parameter was set by the login UI
-// and redirects the request to the login UI idp callback handlers (GET and POST).
-// The function is used in the /idps/callback endpoint to distinguish between requests
-// starte in the login UI and IdP intents.
-func IDPCallbackRedirect(w http.ResponseWriter, r *http.Request, state string) bool {
-	state, loginUI := authRequestIDFromState(state)
-	if !loginUI {
-		return false
-	}
-	callback := EndpointExternalLoginCallback
-	if r.Method == http.MethodPost {
-		callback = EndpointExternalLoginCallbackFormPost
-	}
-	target := gu.PtrCopy(r.URL)
-	target.Path = HandlerPrefix + callback
-	q := target.Query()
-	q.Set("state", state)
-	target.RawQuery = q.Encode()
-	http.Redirect(w, r, target.String(), http.StatusTemporaryRedirect)
-	return true
-}
-
-func stateFromAuthRequest(authRequestID string) string {
-	return prefixLoginState + authRequestID
-}
-
-func authRequestIDFromState(state string) (string, bool) {
-	return strings.CutPrefix(state, prefixLoginState)
-}
-
 // handleExternalLoginStep is called as nextStep
 func (l *Login) handleExternalLoginStep(w http.ResponseWriter, r *http.Request, authReq *domain.AuthRequest, selectedIDPID string) {
 	for _, idp := range authReq.AllowedExternalIDPs {
@@ -220,8 +187,7 @@ func (l *Login) handleIDP(w http.ResponseWriter, r *http.Request, authReq *domai
 		return
 	}
 	params := l.sessionParamsFromAuthRequest(r.Context(), authReq, identityProvider.ID)
-	state := stateFromAuthRequest(authReq.ID)
-	session, err := provider.BeginAuth(r.Context(), state, params...)
+	session, err := provider.BeginAuth(r.Context(), authReq.ID, params...)
 	if err != nil {
 		l.renderLogin(w, r, authReq, err)
 		return
@@ -693,7 +659,7 @@ func (l *Login) handleExternalNotFoundOptionCheck(w http.ResponseWriter, r *http
 	data := new(externalNotFoundOptionFormData)
 	authReq, err := l.ensureAuthRequestAndParseData(r, data)
 	if err != nil {
-		l.renderError(w, r, authReq, err)
+		l.renderExternalNotFoundOption(w, r, authReq, nil, nil, nil, err)
 		return
 	}

@@ -995,7 +961,7 @@ func (l *Login) googleProvider(ctx context.Context, identityProvider *query.IDPT
 	return google.New(
 		identityProvider.GoogleIDPTemplate.ClientID,
 		secret,
-		idp_api.CallbackURL(l.externalSecure)(ctx),
+		l.baseURL(ctx)+EndpointExternalLoginCallback,
 		identityProvider.GoogleIDPTemplate.Scopes,
 	)
 }
@@ -1014,7 +980,7 @@ func (l *Login) oidcProvider(ctx context.Context, identityProvider *query.IDPTem
 		identityProvider.OIDCIDPTemplate.Issuer,
 		identityProvider.OIDCIDPTemplate.ClientID,
 		secret,
-		idp_api.CallbackURL(l.externalSecure)(ctx),
+		l.baseURL(ctx)+EndpointExternalLoginCallback,
 		identityProvider.OIDCIDPTemplate.Scopes,
 		openid.DefaultMapper,
 		opts...,
@@ -1044,7 +1010,7 @@ func (l *Login) oauthProvider(ctx context.Context, identityProvider *query.IDPTe
 			AuthURL:  identityProvider.OAuthIDPTemplate.AuthorizationEndpoint,
 			TokenURL: identityProvider.OAuthIDPTemplate.TokenEndpoint,
 		},
-		RedirectURL: idp_api.CallbackURL(l.externalSecure)(ctx),
+		RedirectURL: l.baseURL(ctx) + EndpointExternalLoginCallback,
 		Scopes:      identityProvider.OAuthIDPTemplate.Scopes,
 	}
 	return oauth.New(
@@ -1062,7 +1028,6 @@ func (l *Login) samlProvider(ctx context.Context, identityProvider *query.IDPTem
 	if err != nil {
 		return nil, err
 	}
-	rootURL := idp_api.SAMLRootURL(l.externalSecure)(ctx, identityProvider.ID)
 	opts := make([]saml.ProviderOpts, 0, 6)
 	if identityProvider.SAMLIDPTemplate.WithSignedRequest {
 		opts = append(opts, saml.WithSignedRequest())
@@ -1077,12 +1042,11 @@ func (l *Login) samlProvider(ctx context.Context, identityProvider *query.IDPTem
 		opts = append(opts, saml.WithTransientMappingAttributeName(identityProvider.SAMLIDPTemplate.TransientMappingAttributeName))
 	}
 	opts = append(opts,
-		saml.WithEntityID(rootURL+"saml/metadata"),
+		saml.WithEntityID(http_utils.BuildOrigin(authz.GetInstance(ctx).RequestedHost(), l.externalSecure)+"/idps/"+identityProvider.ID+"/saml/metadata"),
 		saml.WithCustomRequestTracker(
 			requesttracker.New(
-				func(ctx context.Context, state, samlRequestID string) error {
+				func(ctx context.Context, authRequestID, samlRequestID string) error {
 					useragent, _ := http_mw.UserAgentIDFromCtx(ctx)
-					authRequestID, _ := authRequestIDFromState(state)
 					return l.authRepo.SaveSAMLRequestID(ctx, authRequestID, samlRequestID, useragent)
 				},
 				func(ctx context.Context, authRequestID string) (*samlsp.TrackedRequest, error) {
@@ -1100,7 +1064,7 @@ func (l *Login) samlProvider(ctx context.Context, identityProvider *query.IDPTem
 		))
 	return saml.New(
 		identityProvider.Name,
-		rootURL,
+		l.baseURL(ctx)+EndpointExternalLogin+"/",
 		identityProvider.SAMLIDPTemplate.Metadata,
 		identityProvider.SAMLIDPTemplate.Certificate,
 		key,
@@ -1124,7 +1088,7 @@ func (l *Login) azureProvider(ctx context.Context, identityProvider *query.IDPTe
 		identityProvider.Name,
 		identityProvider.AzureADIDPTemplate.ClientID,
 		secret,
-		idp_api.CallbackURL(l.externalSecure)(ctx),
+		l.baseURL(ctx)+EndpointExternalLoginCallback,
 		identityProvider.AzureADIDPTemplate.Scopes,
 		opts...,
 	)
@@ -1138,7 +1102,7 @@ func (l *Login) githubProvider(ctx context.Context, identityProvider *query.IDPT
 	return github.New(
 		identityProvider.GitHubIDPTemplate.ClientID,
 		secret,
-		idp_api.CallbackURL(l.externalSecure)(ctx),
+		l.baseURL(ctx)+EndpointExternalLoginCallback,
 		identityProvider.GitHubIDPTemplate.Scopes,
 	)
 }
@@ -1152,7 +1116,7 @@ func (l *Login) githubEnterpriseProvider(ctx context.Context, identityProvider *
 		identityProvider.Name,
 		identityProvider.GitHubIDPTemplate.ClientID,
 		secret,
-		idp_api.CallbackURL(l.externalSecure)(ctx),
+		l.baseURL(ctx)+EndpointExternalLoginCallback,
 		identityProvider.GitHubEnterpriseIDPTemplate.AuthorizationEndpoint,
 		identityProvider.GitHubEnterpriseIDPTemplate.TokenEndpoint,
 		identityProvider.GitHubEnterpriseIDPTemplate.UserEndpoint,
@@ -1168,7 +1132,7 @@ func (l *Login) gitlabProvider(ctx context.Context, identityProvider *query.IDPT
 	return gitlab.New(
 		identityProvider.GitLabIDPTemplate.ClientID,
 		secret,
-		idp_api.CallbackURL(l.externalSecure)(ctx),
+		l.baseURL(ctx)+EndpointExternalLoginCallback,
 		identityProvider.GitLabIDPTemplate.Scopes,
 	)
 }
@@ -1183,7 +1147,7 @@ func (l *Login) gitlabSelfHostedProvider(ctx context.Context, identityProvider *
 		identityProvider.GitLabSelfHostedIDPTemplate.Issuer,
 		identityProvider.GitLabSelfHostedIDPTemplate.ClientID,
 		secret,
-		idp_api.CallbackURL(l.externalSecure)(ctx),
+		l.baseURL(ctx)+EndpointExternalLoginCallback,
 		identityProvider.GitLabSelfHostedIDPTemplate.Scopes,
 	)
 }
@@ -1197,7 +1161,7 @@ func (l *Login) appleProvider(ctx context.Context, identityProvider *query.IDPTe
 		identityProvider.AppleIDPTemplate.ClientID,
 		identityProvider.AppleIDPTemplate.TeamID,
 		identityProvider.AppleIDPTemplate.KeyID,
-		idp_api.CallbackURL(l.externalSecure)(ctx),
+		l.baseURL(ctx)+EndpointExternalLoginCallbackFormPost,
 		privateKey,
 		identityProvider.AppleIDPTemplate.Scopes,
 	)