From d058a2bc8a6cf663f7e639db7eec57656c3b235f Mon Sep 17 00:00:00 2001
From: Livio Spring <livio.a@gmail.com>
Date: Fri, 24 May 2024 16:58:45 +0200
Subject: [PATCH] fix(login): ensure auth request (#8004)

# Which Problems Are Solved

Potential nil pointers leading to a panic in the login UI.

# How the Problems Are Solved

As of now the login UI did not always check if the authRequest was
actually retrieved form the database, which is ok for some endpoints
which can also be called outside of an auth request.
There are now methods added to ensure the request is loaded.

# Additional Changes

None

# Additional Context

Closes https://github.com/zitadel/DevOps/issues/55
---
 internal/api/ui/login/auth_request.go          | 18 ++++++++++++++++++
 .../api/ui/login/change_password_handler.go    |  2 +-
 internal/api/ui/login/device_auth.go           |  6 ++----
 .../api/ui/login/external_provider_handler.go  |  4 ++--
 internal/api/ui/login/ldap_handler.go          |  2 +-
 internal/api/ui/login/link_prompt_handler.go   |  2 +-
 internal/api/ui/login/mfa_init_sms.go          |  2 +-
 internal/api/ui/login/mfa_init_u2f.go          |  2 +-
 .../api/ui/login/mfa_init_verify_handler.go    |  2 +-
 internal/api/ui/login/mfa_prompt_handler.go    |  2 +-
 internal/api/ui/login/mfa_verify_handler.go    |  2 +-
 .../api/ui/login/mfa_verify_otp_handler.go     |  4 ++--
 .../api/ui/login/mfa_verify_u2f_handler.go     |  2 +-
 internal/api/ui/login/password_handler.go      |  2 +-
 .../api/ui/login/password_reset_handler.go     |  2 +-
 .../api/ui/login/passwordless_login_handler.go |  2 +-
 internal/api/ui/login/select_user_handler.go   |  2 +-
 .../api/ui/login/username_change_handler.go    |  2 +-
 18 files changed, 38 insertions(+), 22 deletions(-)

diff --git a/internal/api/ui/login/auth_request.go b/internal/api/ui/login/auth_request.go
index f2dcce57d..4da045efa 100644
--- a/internal/api/ui/login/auth_request.go
+++ b/internal/api/ui/login/auth_request.go
@@ -7,6 +7,7 @@ import (
 
 	http_mw "github.com/zitadel/zitadel/internal/api/http/middleware"
 	"github.com/zitadel/zitadel/internal/domain"
+	"github.com/zitadel/zitadel/internal/zerrors"
 )
 
 const (
@@ -23,6 +24,14 @@ func (l *Login) getAuthRequest(r *http.Request) (*domain.AuthRequest, error) {
 	return l.authRepo.AuthRequestByID(r.Context(), authRequestID, userAgentID)
 }
 
+func (l *Login) ensureAuthRequest(r *http.Request) (*domain.AuthRequest, error) {
+	authRequest, err := l.getAuthRequest(r)
+	if authRequest != nil || err != nil {
+		return authRequest, err
+	}
+	return nil, zerrors.ThrowInvalidArgument(nil, "LOGIN-OLah9", "invalid or missing auth request")
+}
+
 func (l *Login) getAuthRequestAndParseData(r *http.Request, data interface{}) (*domain.AuthRequest, error) {
 	authReq, err := l.getAuthRequest(r)
 	if err != nil {
@@ -32,6 +41,15 @@ func (l *Login) getAuthRequestAndParseData(r *http.Request, data interface{}) (*
 	return authReq, err
 }
 
+func (l *Login) ensureAuthRequestAndParseData(r *http.Request, data interface{}) (*domain.AuthRequest, error) {
+	authReq, err := l.ensureAuthRequest(r)
+	if err != nil {
+		return authReq, err
+	}
+	err = l.parser.Parse(r, data)
+	return authReq, err
+}
+
 func (l *Login) getParseData(r *http.Request, data interface{}) error {
 	return l.parser.Parse(r, data)
 }
diff --git a/internal/api/ui/login/change_password_handler.go b/internal/api/ui/login/change_password_handler.go
index 1eacdcebd..5c36d0f74 100644
--- a/internal/api/ui/login/change_password_handler.go
+++ b/internal/api/ui/login/change_password_handler.go
@@ -20,7 +20,7 @@ type changePasswordData struct {
 
 func (l *Login) handleChangePassword(w http.ResponseWriter, r *http.Request) {
 	data := new(changePasswordData)
-	authReq, err := l.getAuthRequestAndParseData(r, data)
+	authReq, err := l.ensureAuthRequestAndParseData(r, data)
 	if err != nil {
 		l.renderError(w, r, authReq, err)
 		return
diff --git a/internal/api/ui/login/device_auth.go b/internal/api/ui/login/device_auth.go
index 659f2ac14..0d5349903 100644
--- a/internal/api/ui/login/device_auth.go
+++ b/internal/api/ui/login/device_auth.go
@@ -14,7 +14,6 @@ import (
 	"github.com/zitadel/zitadel/internal/api/authz"
 	"github.com/zitadel/zitadel/internal/api/http/middleware"
 	"github.com/zitadel/zitadel/internal/domain"
-	"github.com/zitadel/zitadel/internal/zerrors"
 )
 
 const (
@@ -145,9 +144,8 @@ func (l *Login) redirectDeviceAuthStart(w http.ResponseWriter, r *http.Request,
 // When the action of "allowed" or "denied", the device authorization is updated accordingly.
 // Else the user is presented with a page where they can choose / submit either action.
 func (l *Login) handleDeviceAuthAction(w http.ResponseWriter, r *http.Request) {
-	authReq, err := l.getAuthRequest(r)
-	if authReq == nil {
-		err = zerrors.ThrowInvalidArgument(err, "LOGIN-OLah8", "invalid or missing auth request")
+	authReq, err := l.ensureAuthRequest(r)
+	if err != nil {
 		l.redirectDeviceAuthStart(w, r, err.Error())
 		return
 	}
diff --git a/internal/api/ui/login/external_provider_handler.go b/internal/api/ui/login/external_provider_handler.go
index e03b32086..ab0d98b99 100644
--- a/internal/api/ui/login/external_provider_handler.go
+++ b/internal/api/ui/login/external_provider_handler.go
@@ -127,7 +127,7 @@ func (l *Login) handleExternalLogin(w http.ResponseWriter, r *http.Request) {
 // handleExternalRegister is called when a user selects the idp on the register options page
 func (l *Login) handleExternalRegister(w http.ResponseWriter, r *http.Request) {
 	data := new(externalIDPData)
-	authReq, err := l.getAuthRequestAndParseData(r, data)
+	authReq, err := l.ensureAuthRequestAndParseData(r, data)
 	if err != nil {
 		l.renderError(w, r, authReq, err)
 		return
@@ -657,7 +657,7 @@ func (l *Login) renderExternalNotFoundOption(w http.ResponseWriter, r *http.Requ
 // and either links or creates an externalUser
 func (l *Login) handleExternalNotFoundOptionCheck(w http.ResponseWriter, r *http.Request) {
 	data := new(externalNotFoundOptionFormData)
-	authReq, err := l.getAuthRequestAndParseData(r, data)
+	authReq, err := l.ensureAuthRequestAndParseData(r, data)
 	if err != nil {
 		l.renderExternalNotFoundOption(w, r, authReq, nil, nil, nil, err)
 		return
diff --git a/internal/api/ui/login/ldap_handler.go b/internal/api/ui/login/ldap_handler.go
index 93590458f..0fd47c5a6 100644
--- a/internal/api/ui/login/ldap_handler.go
+++ b/internal/api/ui/login/ldap_handler.go
@@ -42,7 +42,7 @@ func (l *Login) renderLDAPLogin(w http.ResponseWriter, r *http.Request, authReq
 
 func (l *Login) handleLDAPCallback(w http.ResponseWriter, r *http.Request) {
 	data := new(ldapFormData)
-	authReq, err := l.getAuthRequestAndParseData(r, data)
+	authReq, err := l.ensureAuthRequestAndParseData(r, data)
 	if err != nil {
 		l.renderError(w, r, authReq, err)
 		return
diff --git a/internal/api/ui/login/link_prompt_handler.go b/internal/api/ui/login/link_prompt_handler.go
index 04293bd86..30ac18629 100644
--- a/internal/api/ui/login/link_prompt_handler.go
+++ b/internal/api/ui/login/link_prompt_handler.go
@@ -44,7 +44,7 @@ func (l *Login) renderLinkingUserPrompt(w http.ResponseWriter, r *http.Request,
 
 func (l *Login) handleLinkingUserPrompt(w http.ResponseWriter, r *http.Request) {
 	data := new(linkingUserPromptFormData)
-	authReq, err := l.getAuthRequestAndParseData(r, data)
+	authReq, err := l.ensureAuthRequestAndParseData(r, data)
 	if err != nil {
 		l.renderLogin(w, r, authReq, err)
 		return
diff --git a/internal/api/ui/login/mfa_init_sms.go b/internal/api/ui/login/mfa_init_sms.go
index 9aaa744a3..03f2c3201 100644
--- a/internal/api/ui/login/mfa_init_sms.go
+++ b/internal/api/ui/login/mfa_init_sms.go
@@ -71,7 +71,7 @@ func (l *Login) renderRegisterSMS(w http.ResponseWriter, r *http.Request, authRe
 // and a successful OTP SMS check will be added to the auth request.
 func (l *Login) handleRegisterSMSCheck(w http.ResponseWriter, r *http.Request) {
 	formData := new(smsInitFormData)
-	authReq, err := l.getAuthRequestAndParseData(r, formData)
+	authReq, err := l.ensureAuthRequestAndParseData(r, formData)
 	if err != nil {
 		l.renderError(w, r, authReq, err)
 		return
diff --git a/internal/api/ui/login/mfa_init_u2f.go b/internal/api/ui/login/mfa_init_u2f.go
index 0b7718bb8..c84948796 100644
--- a/internal/api/ui/login/mfa_init_u2f.go
+++ b/internal/api/ui/login/mfa_init_u2f.go
@@ -42,7 +42,7 @@ func (l *Login) renderRegisterU2F(w http.ResponseWriter, r *http.Request, authRe
 
 func (l *Login) handleRegisterU2F(w http.ResponseWriter, r *http.Request) {
 	data := new(webAuthNFormData)
-	authReq, err := l.getAuthRequestAndParseData(r, data)
+	authReq, err := l.ensureAuthRequestAndParseData(r, data)
 	if err != nil {
 		l.renderError(w, r, authReq, err)
 		return
diff --git a/internal/api/ui/login/mfa_init_verify_handler.go b/internal/api/ui/login/mfa_init_verify_handler.go
index 0b6645143..cd6a9091e 100644
--- a/internal/api/ui/login/mfa_init_verify_handler.go
+++ b/internal/api/ui/login/mfa_init_verify_handler.go
@@ -26,7 +26,7 @@ type mfaInitVerifyData struct {
 
 func (l *Login) handleMFAInitVerify(w http.ResponseWriter, r *http.Request) {
 	data := new(mfaInitVerifyData)
-	authReq, err := l.getAuthRequestAndParseData(r, data)
+	authReq, err := l.ensureAuthRequestAndParseData(r, data)
 	if err != nil {
 		l.renderError(w, r, authReq, err)
 		return
diff --git a/internal/api/ui/login/mfa_prompt_handler.go b/internal/api/ui/login/mfa_prompt_handler.go
index ade1b5322..ce1b7240e 100644
--- a/internal/api/ui/login/mfa_prompt_handler.go
+++ b/internal/api/ui/login/mfa_prompt_handler.go
@@ -18,7 +18,7 @@ type mfaPromptData struct {
 
 func (l *Login) handleMFAPrompt(w http.ResponseWriter, r *http.Request) {
 	data := new(mfaPromptData)
-	authReq, err := l.getAuthRequestAndParseData(r, data)
+	authReq, err := l.ensureAuthRequestAndParseData(r, data)
 	if err != nil {
 		l.renderError(w, r, authReq, err)
 		return
diff --git a/internal/api/ui/login/mfa_verify_handler.go b/internal/api/ui/login/mfa_verify_handler.go
index 80f1c94e2..cfffc6fce 100644
--- a/internal/api/ui/login/mfa_verify_handler.go
+++ b/internal/api/ui/login/mfa_verify_handler.go
@@ -19,7 +19,7 @@ type mfaVerifyFormData struct {
 
 func (l *Login) handleMFAVerify(w http.ResponseWriter, r *http.Request) {
 	data := new(mfaVerifyFormData)
-	authReq, err := l.getAuthRequestAndParseData(r, data)
+	authReq, err := l.ensureAuthRequestAndParseData(r, data)
 	if err != nil {
 		l.renderError(w, r, authReq, err)
 		return
diff --git a/internal/api/ui/login/mfa_verify_otp_handler.go b/internal/api/ui/login/mfa_verify_otp_handler.go
index 2c0d5d856..b39605e66 100644
--- a/internal/api/ui/login/mfa_verify_otp_handler.go
+++ b/internal/api/ui/login/mfa_verify_otp_handler.go
@@ -75,8 +75,8 @@ func (l *Login) renderOTPVerification(w http.ResponseWriter, r *http.Request, au
 // A user is also able to request a code resend or choose another provider.
 func (l *Login) handleOTPVerificationCheck(w http.ResponseWriter, r *http.Request) {
 	formData := new(mfaOTPFormData)
-	authReq, err := l.getAuthRequestAndParseData(r, formData)
-	if authReq == nil || err != nil {
+	authReq, err := l.ensureAuthRequestAndParseData(r, formData)
+	if err != nil {
 		l.renderError(w, r, authReq, err)
 		return
 	}
diff --git a/internal/api/ui/login/mfa_verify_u2f_handler.go b/internal/api/ui/login/mfa_verify_u2f_handler.go
index 3afa41682..787346861 100644
--- a/internal/api/ui/login/mfa_verify_u2f_handler.go
+++ b/internal/api/ui/login/mfa_verify_u2f_handler.go
@@ -50,7 +50,7 @@ func (l *Login) renderU2FVerification(w http.ResponseWriter, r *http.Request, au
 
 func (l *Login) handleU2FVerification(w http.ResponseWriter, r *http.Request) {
 	formData := new(mfaU2FFormData)
-	authReq, err := l.getAuthRequestAndParseData(r, formData)
+	authReq, err := l.ensureAuthRequestAndParseData(r, formData)
 	if err != nil {
 		l.renderError(w, r, authReq, err)
 		return
diff --git a/internal/api/ui/login/password_handler.go b/internal/api/ui/login/password_handler.go
index 28baf4a1e..026963bbd 100644
--- a/internal/api/ui/login/password_handler.go
+++ b/internal/api/ui/login/password_handler.go
@@ -34,7 +34,7 @@ func (l *Login) renderPassword(w http.ResponseWriter, r *http.Request, authReq *
 
 func (l *Login) handlePasswordCheck(w http.ResponseWriter, r *http.Request) {
 	data := new(passwordFormData)
-	authReq, err := l.getAuthRequestAndParseData(r, data)
+	authReq, err := l.ensureAuthRequestAndParseData(r, data)
 	if err != nil {
 		l.renderError(w, r, authReq, err)
 		return
diff --git a/internal/api/ui/login/password_reset_handler.go b/internal/api/ui/login/password_reset_handler.go
index ff57321e9..bc9a5069d 100644
--- a/internal/api/ui/login/password_reset_handler.go
+++ b/internal/api/ui/login/password_reset_handler.go
@@ -12,7 +12,7 @@ const (
 )
 
 func (l *Login) handlePasswordReset(w http.ResponseWriter, r *http.Request) {
-	authReq, err := l.getAuthRequest(r)
+	authReq, err := l.ensureAuthRequest(r)
 	if err != nil {
 		l.renderError(w, r, authReq, err)
 		return
diff --git a/internal/api/ui/login/passwordless_login_handler.go b/internal/api/ui/login/passwordless_login_handler.go
index 8373a8fbd..52b9d06fe 100644
--- a/internal/api/ui/login/passwordless_login_handler.go
+++ b/internal/api/ui/login/passwordless_login_handler.go
@@ -49,7 +49,7 @@ func (l *Login) renderPasswordlessVerification(w http.ResponseWriter, r *http.Re
 
 func (l *Login) handlePasswordlessVerification(w http.ResponseWriter, r *http.Request) {
 	formData := new(passwordlessFormData)
-	authReq, err := l.getAuthRequestAndParseData(r, formData)
+	authReq, err := l.ensureAuthRequestAndParseData(r, formData)
 	if err != nil {
 		l.renderError(w, r, authReq, err)
 		return
diff --git a/internal/api/ui/login/select_user_handler.go b/internal/api/ui/login/select_user_handler.go
index 3febbaed7..98c399337 100644
--- a/internal/api/ui/login/select_user_handler.go
+++ b/internal/api/ui/login/select_user_handler.go
@@ -36,7 +36,7 @@ func (l *Login) renderUserSelection(w http.ResponseWriter, r *http.Request, auth
 
 func (l *Login) handleSelectUser(w http.ResponseWriter, r *http.Request) {
 	data := new(userSelectionFormData)
-	authSession, err := l.getAuthRequestAndParseData(r, data)
+	authSession, err := l.ensureAuthRequestAndParseData(r, data)
 	if err != nil {
 		l.renderError(w, r, authSession, err)
 		return
diff --git a/internal/api/ui/login/username_change_handler.go b/internal/api/ui/login/username_change_handler.go
index 7a497c4eb..f11fe43a7 100644
--- a/internal/api/ui/login/username_change_handler.go
+++ b/internal/api/ui/login/username_change_handler.go
@@ -27,7 +27,7 @@ func (l *Login) renderChangeUsername(w http.ResponseWriter, r *http.Request, aut
 
 func (l *Login) handleChangeUsername(w http.ResponseWriter, r *http.Request) {
 	data := new(changeUsernameData)
-	authReq, err := l.getAuthRequestAndParseData(r, data)
+	authReq, err := l.ensureAuthRequestAndParseData(r, data)
 	if err != nil {
 		l.renderError(w, r, authReq, err)
 		return