bug: openid-connect throws 500 error with no session state found when the client_secret is incorrect

[kayx23]

Current Behavior

When using openid-connect for auth code grant, if you configure an invalid client_secret, APISIX throws 500 error with no session state found, which isn't an intuitive error msg.

My understanding: no session state found was thrown because the authentication wasn't successful, so no token was returned and stored in session.

To reproduce, you could follow the keycloak doc or azure AD doc and just key in a random string for client_secret while keeping other details correct (in reality you should be able to repro regardless of the OP you use).

Expected Behavior

Not throwing 500 internal server error and print a more meaningful error msg.

Error Logs

2023/12/21 03:53:03 [error] 50#50: *16139 [lua] openidc.lua:1484: authenticate(): request to the redirect_uri path but there's no session state found, client: 172.24.0.1, server: _, request: "GET /anything/callback?state=3e50d27a1737b08f220a91e01ed1eb94&session_state=62258339-4084-4354-aa25-d0e8004a55c7&code=8dd96e6b-6fed-4595-9aec-8bb8f01a5cc6.62258339-4084-4354-aa25-d0e8004a55c7.7b1804de-f8f0-42b5-b242-b450477b13c3 HTTP/1.1", host: "localhost:9080"
2023/12/21 03:53:03 [error] 50#50: *16139 [lua] openid-connect.lua:393: phase_func(): OIDC authentication failed: request to the redirect_uri path but there's no session state found, client: 172.24.0.1, server: _, request: "GET /anything/callback?state=3e50d27a1737b08f220a91e01ed1eb94&session_state=62258339-4084-4354-aa25-d0e8004a55c7&code=8dd96e6b-6fed-4595-9aec-8bb8f01a5cc6.62258339-4084-4354-aa25-d0e8004a55c7.7b1804de-f8f0-42b5-b242-b450477b13c3 HTTP/1.1", host: "localhost:9080"

Environment

• APISIX version (run apisix version): 3.7.0

[shreemaan-abhishek]

@kayx23 can this be a good first issue from your POV?

[kayx23]

I think so. You could have one look into it and assess if it's possible to catch the case where client_id is invalid. I am a bit doubtful but didn't look too deep.

[kayx23]

#6803 here is a relevant but slightly different issue. It also aims for improving the error msg when openid-connect throws 500 error with no session state found but for when redirect_uri is misconfigured (e.g. the same as the route uri path). Might be worthwhile to assess together.

[moonming]

@luoluoyuyu please take a look

[luoluoyuyu]

Hi @kayx23 , I don't think this is a bug, for login and logout processing need to rely on the authenticate function in the upstream library openidc, log contains a lot of error log returns, it is difficult to get a really valid log. https://github.com/apache/apisix/blob/master/apisix/plugins/openid-connect.lua#L507-L518

[kayx23]

it is difficult to get a really valid log. https://github.com/apache/apisix/blob/master/apisix/plugins/openid-connect.lua#L507-L518

Hi @luoluoyuyu - I've seen that block, which is why I said "have one look into it and assess if it's possible to catch the case where client_id is invalid. I am a bit doubtful". I will leave this issue to you to decide how to proceed.

Whether it is a bug from an engineering perspective is up for debate. From a user perspective, users only know APISIX errored out with 500 internal server error due to session not found, and 500 insinuates an unwanted/unhandled case, so it could feel like a bug.

In reality, there are a number of scenarios one could end up with this error, and incorrect client ID is only one of them.

@moonming - if this error is left unhandled, or, the specific causes that crash the program cannot be handled case by case (because the error takes place in lua-resty-oidc), we should still keep the troubleshooting section in the doc listing all the reasons for no session state found to help users navigate through working with this plugin.

[luoluoyuyu]

we should still keep the troubleshooting section in the doc listing all the reasons for no session state found to help users navigate through working with this plugin.

We should add this section to the documentation，@kayx23 @shreemaan-abhishek @moonming please let me try to resolve this issue

[luoluoyuyu]

@kayx23
I tried to use keyclcoak to reproduce this error, and the results were consistent with yours. But when using auth0 to reproduce, 500 is returned but the log is

OIDC authentication failed: response indicates failure, status=401, body={"error":"access_denied","error_description":"Unauthorized"}, client: ::1, server: _, request: "GET /anything/callback?code=lY7htrBc5PBWMM6_Jfgg2rf6N0UazdUqddNXCISNe_WhR&state=0aa86e3582e7a25f0b5005fe8feebf3d HTTP/1.1", host: "localhost:9080"

Different openid providers return different logs for the same error. This will be a problem that needs to be faced to improve the documentation.

[kayx23]

hm ok I'll check again

[kayx23]

I tried to use keyclcoak to reproduce this error, and the results were consistent with yours.

@luoluoyuyu interestingly I misread your message and thought you got that error for Keycloak, and I didn't want to manually test again, so I modified client secret in openid-connect.t and ran the CI in my repo to see error I would get.

It produces an error that matches what you experienced for auth0:

I went back to APISIX logs of my local instance (same one I tested with) to search for this error but couldn't find it (strange).

[kayx23]

@luoluoyuyu

Different openid providers return different logs for the same error.

Please see my last comment.

This will be a problem that needs to be faced to improve the documentation.

I was debating if I should document "using the incorrect client secret" as one of the reasons one could run into 500 error no session state found. I didn't expect different error logs for the same misconfigured client secret : /

[luoluoyuyu]

I was debating if I should document "using the incorrect client secret" as one of the reasons one could run into 500 error . I didn't expect different error logs for the same misconfigured client secret : /no session state found

@kayx23 I think it should be stated. But the reason why there are two different logs needs to be analyzed. So that it can be explained to the user in the documentation why no session state found appears.

[kayx23]

I added this as a cause to the new doc.

Since this issue cannot be addressed programmatically from our side (per @luoluoyuyu's evaluation above), this issue could be closed?

[luoluoyuyu]

@kayx23 you could close this issue i think.