From 8bed692ae1fe22194a435351b34f20feb989ddd7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tim=20M=C3=B6hlmann?= <tim+github@zitadel.com>
Date: Tue, 20 Aug 2024 18:22:22 +0300
Subject: [PATCH] fix(op): do not redirect to unverified uri on error

Closes #627
---
 pkg/op/auth_request.go | 46 +++++++++++++++++++++++++++---------------
 1 file changed, 30 insertions(+), 16 deletions(-)

diff --git a/pkg/op/auth_request.go b/pkg/op/auth_request.go
index fe73180e..52fda2e4 100644
--- a/pkg/op/auth_request.go
+++ b/pkg/op/auth_request.go
@@ -83,19 +83,27 @@ func Authorize(w http.ResponseWriter, r *http.Request, authorizer Authorizer) {
 	if authReq.RequestParam != "" && authorizer.RequestObjectSupported() {
 		err = ParseRequestObject(ctx, authReq, authorizer.Storage(), IssuerFromContext(ctx))
 		if err != nil {
-			AuthRequestError(w, r, authReq, err, authorizer)
+			AuthRequestError(w, r, nil, err, authorizer)
 			return
 		}
 	}
 	if authReq.ClientID == "" {
-		AuthRequestError(w, r, authReq, fmt.Errorf("auth request is missing client_id"), authorizer)
+		AuthRequestError(w, r, nil, fmt.Errorf("auth request is missing client_id"), authorizer)
 		return
 	}
 	if authReq.RedirectURI == "" {
-		AuthRequestError(w, r, authReq, fmt.Errorf("auth request is missing redirect_uri"), authorizer)
+		AuthRequestError(w, r, nil, fmt.Errorf("auth request is missing redirect_uri"), authorizer)
 		return
 	}
-	validation := ValidateAuthRequest
+
+	var client Client
+	validation := func(ctx context.Context, authReq *oidc.AuthRequest, storage Storage, verifier *IDTokenHintVerifier) (sub string, err error) {
+		client, err = authorizer.Storage().GetClientByClientID(ctx, authReq.ClientID)
+		if err != nil {
+			return "", oidc.ErrInvalidRequestRedirectURI().WithDescription("unable to retrieve client by id").WithParent(err)
+		}
+		return ValidateAuthRequestClient(ctx, authReq, client, verifier)
+	}
 	if validater, ok := authorizer.(AuthorizeValidator); ok {
 		validation = validater.ValidateAuthRequest
 	}
@@ -113,11 +121,6 @@ func Authorize(w http.ResponseWriter, r *http.Request, authorizer Authorizer) {
 		AuthRequestError(w, r, authReq, oidc.DefaultToServerError(err, "unable to save auth request"), authorizer)
 		return
 	}
-	client, err := authorizer.Storage().GetClientByClientID(ctx, req.GetClientID())
-	if err != nil {
-		AuthRequestError(w, r, req, oidc.DefaultToServerError(err, "unable to retrieve client by id"), authorizer)
-		return
-	}
 	RedirectToLogin(req.GetID(), client, w, r)
 }
 
@@ -212,26 +215,37 @@ func CopyRequestObjectToAuthRequest(authReq *oidc.AuthRequest, requestObject *oi
 	authReq.RequestParam = ""
 }
 
-// ValidateAuthRequest validates the authorize parameters and returns the userID of the id_token_hint if passed
+// ValidateAuthRequest validates the authorize parameters and returns the userID of the id_token_hint if passed.
+//
+// Deprecated: Use [ValidateAuthRequestClient] to prevent querying for the Client twice.
 func ValidateAuthRequest(ctx context.Context, authReq *oidc.AuthRequest, storage Storage, verifier *IDTokenHintVerifier) (sub string, err error) {
 	ctx, span := tracer.Start(ctx, "ValidateAuthRequest")
 	defer span.End()
 
-	authReq.MaxAge, err = ValidateAuthReqPrompt(authReq.Prompt, authReq.MaxAge)
+	client, err := storage.GetClientByClientID(ctx, authReq.ClientID)
 	if err != nil {
+		return "", oidc.ErrInvalidRequestRedirectURI().WithDescription("unable to retrieve client by id").WithParent(err)
+	}
+	return ValidateAuthRequestClient(ctx, authReq, client, verifier)
+}
+
+// ValidateAuthRequestClient validates the Auth request against the passed client.
+// If id_token_hint is part of the request, the subject of the token is returned.
+func ValidateAuthRequestClient(ctx context.Context, authReq *oidc.AuthRequest, client Client, verifier *IDTokenHintVerifier) (sub string, err error) {
+	ctx, span := tracer.Start(ctx, "ValidateAuthRequestClient")
+	defer span.End()
+
+	if err := ValidateAuthReqRedirectURI(client, authReq.RedirectURI, authReq.ResponseType); err != nil {
 		return "", err
 	}
-	client, err := storage.GetClientByClientID(ctx, authReq.ClientID)
+	authReq.MaxAge, err = ValidateAuthReqPrompt(authReq.Prompt, authReq.MaxAge)
 	if err != nil {
-		return "", oidc.DefaultToServerError(err, "unable to retrieve client by id")
+		return "", err
 	}
 	authReq.Scopes, err = ValidateAuthReqScopes(client, authReq.Scopes)
 	if err != nil {
 		return "", err
 	}
-	if err := ValidateAuthReqRedirectURI(client, authReq.RedirectURI, authReq.ResponseType); err != nil {
-		return "", err
-	}
 	if err := ValidateAuthReqResponseType(client, authReq.ResponseType); err != nil {
 		return "", err
 	}