diff --git a/internal/api/ui/login/external_provider_handler.go b/internal/api/ui/login/external_provider_handler.go index 6b312317be..c60e0eb0bb 100644 --- a/internal/api/ui/login/external_provider_handler.go +++ b/internal/api/ui/login/external_provider_handler.go @@ -3,6 +3,7 @@ package login import ( "context" "net/http" + "net/url" "strings" "github.com/crewjam/saml/samlsp" @@ -36,6 +37,9 @@ import ( const ( queryIDPConfigID = "idpConfigID" + queryState = "state" + queryRelayState = "RelayState" + queryMethod = "method" tmplExternalNotFoundOption = "externalnotfoundoption" ) @@ -214,9 +218,9 @@ func (l *Login) handleExternalLoginCallbackForm(w http.ResponseWriter, r *http.R l.renderLogin(w, r, nil, err) return } - state := r.Form.Get("state") + state := r.Form.Get(queryState) if state == "" { - state = r.Form.Get("RelayState") + state = r.Form.Get(queryRelayState) } if state == "" { l.renderLogin(w, r, nil, zerrors.ThrowInvalidArgument(nil, "LOGIN-dsg3f", "Errors.AuthRequest.NotFound")) @@ -227,12 +231,23 @@ func (l *Login) handleExternalLoginCallbackForm(w http.ResponseWriter, r *http.R State: state, Form: r.Form, }) - http.Redirect(w, r, HandlerPrefix+EndpointExternalLoginCallback+"?method=POST&state="+state, 302) + v := url.Values{} + v.Set(queryMethod, http.MethodPost) + v.Set(queryState, state) + http.Redirect(w, r, HandlerPrefix+EndpointExternalLoginCallback+"?"+v.Encode(), 302) } // handleExternalLoginCallback handles the callback from a IDP // and tries to extract the user with the provided data func (l *Login) handleExternalLoginCallback(w http.ResponseWriter, r *http.Request) { + // workaround because of CSRF on external identity provider flows using form_post + if r.URL.Query().Get(queryMethod) == http.MethodPost { + if err := l.setDataFromFormCallback(r, r.URL.Query().Get(queryState)); err != nil { + l.renderLogin(w, r, nil, err) + return + } + } + data := new(externalIDPCallbackData) err := l.getParseData(r, data) if err != nil { @@ -242,10 +257,6 @@ func (l *Login) handleExternalLoginCallback(w http.ResponseWriter, r *http.Reque if data.State == "" { data.State = data.RelayState } - // workaround because of CSRF on external identity provider flows - if data.Method == http.MethodPost { - l.setDataFromFormCallback(r, data.State) - } userAgentID, _ := http_mw.UserAgentIDFromCtx(r.Context()) authReq, err := l.authRepo.AuthRequestByID(r.Context(), data.State, userAgentID) @@ -356,15 +367,23 @@ func (l *Login) handleExternalLoginCallback(w http.ResponseWriter, r *http.Reque l.handleExternalUserAuthenticated(w, r, authReq, identityProvider, session, user, l.renderNextStep) } -func (l *Login) setDataFromFormCallback(r *http.Request, state string) { +func (l *Login) setDataFromFormCallback(r *http.Request, state string) error { r.Method = http.MethodPost + err := r.ParseForm() + if err != nil { + return err + } // fallback to the form data in case the request was started before the cache was implemented r.PostForm = r.Form idpCallback, ok := l.caches.idpFormCallbacks.Get(r.Context(), idpFormCallbackIndexRequestID, idpFormCallbackKey(authz.GetInstance(r.Context()).InstanceID(), state)) if ok { r.PostForm = idpCallback.Form + // We need to set the form as well to make sure the data is parsed correctly. + // Form precedes PostForm in the parsing order. + r.Form = idpCallback.Form } + return nil } func (l *Login) tryMigrateExternalUserID(r *http.Request, session idp.Session, authReq *domain.AuthRequest, externalUser *domain.ExternalUser) (previousIDMatched bool, err error) {