UID-209: Password reset process - ensure proper response for missing user credentials. #493
Conversation
…user credentials.
|
zburke
left a comment
zburke
left a comment
There was a problem hiding this comment.
This is fine, but it's misleading. The problem is not that existing code chokes on an error response, although that's the fix that is implemented here. The problem is that mod-login-keycloak has implemented /authn/credentials-existence incorrectly. When this request fails to find a user, the correct response is a 200 with a body of
{ credentialsExist: false }
as the documentation states both for mod-login and mod-login-keycloak. Instead, the actual response is a 404 with a body of
{
"errors": [
{
"message": "Keycloak user doesn't exist with the given 'user_id' attribute: ...",
"type": "NotFoundException",
"code": "not_found_error",
"parameters": []
}
],
"total_records": 1
}
Personally, I'd prefer to see the actual bug fixed in mod-login-keycloak rather than accommodating its incorrect behavior here in the UI. That said, I understand the value of a bird in-hand and will not object to merging this.
| .then(() => mutator.isLocalPasswordSet.GET({ params: { userId } })) | ||
| .then(async () => { | ||
| try { | ||
| const response = await mutator.isLocalPasswordSet.GET({ params: { userId } }); | ||
| return response; | ||
| } catch (e) { | ||
| return { | ||
| credentialsExist: false, | ||
| }; | ||
| } | ||
| }) |
There was a problem hiding this comment.
Instead of mixing a promise chain and async/await, it would be better to convert the whole thing to async/await.
There was a problem hiding this comment.
Since this is a CSP, I didn't want to make too many changes.
|
@zburke You're absolutely right that the root cause is returning a 404 instead of the 200 response with |
I've created https://folio-org.atlassian.net/browse/MODLOGINKC-60 to fix this in mod-login-keycloak. |
Purpose
Password reset process:
authn/credentials-existencerequest fails (throwErrors: false).Description
The
authn/credentials-existencefails when a user doesn't have roles assigned. Instead of executing theif(!res.credentialsExist)condition, which creates credentials, the error is passed to thecatchblock. The solution is to return{ credentialsExist: false }when an error occurs, so thatif (!res.credentialsExist)works.Issues
https://folio-org.atlassian.net/browse/UID-209
Screencasts
2025-12-30_17h06_37.mp4