mirror of
https://github.com/zitadel/zitadel.git
synced 2024-12-12 02:54:20 +00:00
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
This commit is contained in:
parent
ca69ba41ee
commit
85d7536d44
@ -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,
|
||||
|
@ -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
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user