From 85d7536d4410879037f2cd132ca8f8149f518800 Mon Sep 17 00:00:00 2001
From: Livio Spring <livio.a@gmail.com>
Date: Mon, 17 Jun 2024 11:09:00 +0200
Subject: [PATCH] fix(oauth): ensure client error is prioritized over token
 error (#8133)

# Which Problems Are Solved

Introduced with #6909, the authentication check (API client) and the
token verification on the introspection endpoint where parallelized to
improve performance. Only the first error would be considered and
returned (and the second completely ignored).
This could lead to situations where both the client authentication and
token verification failed and the response would result in a 200 OK with
`active: false`.

# How the Problems Are Solved

- The client authentication check error will always be prioritized.
- An error in the token check will no longer terminate the client
authentication check.

# Additional Changes

None.

# Additional Context

- reported in Discord:
https://discord.com/channels/927474939156643850/1242770807105781760
---
 internal/api/oidc/client_integration_test.go |  9 ++++++++
 internal/api/oidc/introspect.go              | 23 ++++++++++----------
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/internal/api/oidc/client_integration_test.go b/internal/api/oidc/client_integration_test.go
index 21d54a59d..65cc9309d 100644
--- a/internal/api/oidc/client_integration_test.go
+++ b/internal/api/oidc/client_integration_test.go
@@ -140,6 +140,15 @@ func TestServer_Introspect(t *testing.T) {
 	}
 }
 
+func TestServer_Introspect_invalid_auth_invalid_token(t *testing.T) {
+	// ensure that when an invalid authentication and token is sent, the authentication error is returned
+	// https://github.com/zitadel/zitadel/pull/8133
+	resourceServer, err := Tester.CreateResourceServerClientCredentials(CTX, "xxxxx", "xxxxx")
+	require.NoError(t, err)
+	_, err = rs.Introspect[*oidc.IntrospectionResponse](context.Background(), resourceServer, "xxxxx")
+	require.Error(t, err)
+}
+
 func assertIntrospection(
 	t *testing.T,
 	introspection *oidc.IntrospectionResponse,
diff --git a/internal/api/oidc/introspect.go b/internal/api/oidc/introspect.go
index b0881b6d6..99602393c 100644
--- a/internal/api/oidc/introspect.go
+++ b/internal/api/oidc/introspect.go
@@ -54,19 +54,20 @@ func (s *Server) Introspect(ctx context.Context, r *op.Request[op.IntrospectionR
 		select {
 		case client = <-clientChan:
 			resErr = client.err
+			if resErr != nil {
+				// we prioritize the client error over the token error
+				err = resErr
+				cancel()
+			}
 		case token = <-tokenChan:
 			resErr = token.err
-		}
-
-		if resErr == nil {
-			continue
-		}
-		cancel()
-
-		// we only care for the first error that occurred,
-		// as the next error is most probably a context error.
-		if err == nil {
-			err = resErr
+			if resErr == nil {
+				continue
+			}
+			// we prioritize the client error over the token error
+			if err == nil {
+				err = resErr
+			}
 		}
 	}