WEB-267 Remove tenant identifier for OAuth token requests #2580
Conversation
There was a problem hiding this comment.
Please create a Jira ticket, assign it to you, update its status and reference it in your PR description: https://mifosforge.jira.com/jira/software/c/projects/WEB/list?direction=DESC&sortBy=key This is a requirement for PRs. Thank you for contributing!
sidhantgoel
changed the title
|
@steinwinde Thanks for the update, please let me know if it looks ok now? |
|
@sidhantgoel I think this change oveheads the validation of any OIDC tool. Keycloak is a reference but there are others. Which is the benefit of removing the fineract tenant id header? |
|
@IOhacker In my opinion, we shouldn't send headers to the OIDC or any other server if they're not expected or used. Most likely, the provider will reject them. While CORS doesn't inherently block specific headers, the provider defines which headers are allowed. If a header isn't included in that list, the browser will block the request. Is there any downside to simply removing it? |
|
Cors is for browsers only, not for api calls.
You can see that the current code is working for oauth
https://oauth.mifos.community mifos and password are the credentials
The fineract tenant id is requiered for the Apache Fineract
Regards
Victor
El dom., 10 de agosto de 2025 10:42 p. m., Sidhant Goel <
***@***.***> escribió:
… *sidhantgoel* left a comment (openMF/web-app#2580)
<#2580 (comment)>
@IOhacker <https://github.com/IOhacker> IMO we should not send any header
to OIDC or any other server that it won't consume, most likely it will get
rejected by the provider, CORS don't reject any specific header in general
but provider sends the list of allowed headers and if the header is not
part of allowed headers browser will not consume the request, is there any
harm in removing it?
—
Reply to this email directly, view it on GitHub
<#2580 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALD2ZAQY6YQPRAOYGCO4AOL3NANLTAVCNFSM6AAAAACDKKVDVOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTCNZTGI2TIOJYGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@IOhacker CORS applies to API calls made through the browser—it's necessary for Fineract, but not for OAuth. So, there's no point in sending it with OAuth requests. I've only removed it for OAuth, not for Fineract. Also, just because it works with one auth provider doesn't guarantee it will work with others. We should adhere to the OIDC specification, and this header isn't defined there. Just to clarify, this change won't remove the header from any Fineract requests—even when OAuth is enabled. It only removes the header from requests sent to the OAuth server by the web app. |
|
I think this change is not relevant. Because the header is not part of the
request. I have show you that it is working. There are changes like
managing the users and roles using the Mifos admin UI and these changes are
more relevant.
El lun., 11 de agosto de 2025 12:05 a. m., Sidhant Goel <
***@***.***> escribió:
… *sidhantgoel* left a comment (openMF/web-app#2580)
<#2580 (comment)>
@IOhacker <https://github.com/IOhacker> CORS applies to API calls made
through the browser—it's necessary for Fineract, but not for OAuth. So,
there's no point in sending it with OAuth requests. I've only removed it
for OAuth, not for Fineract. Also, just because it works with one auth
provider doesn't guarantee it will work with others. We should adhere to
the OIDC specification, and this header isn't defined there.
—
Reply to this email directly, view it on GitHub
<#2580 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALD2ZAUSXWVBQI3CXZQYOTT3NAXDDAVCNFSM6AAAAACDKKVDVOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTCNZTGM3DSMBUGE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Also the console.log must be removed. |
|
Sure will remove console.log but this header is part of token request and hence it is not working with keyclock. Please see the screenshot captured from the url shared. |
|
I dont see any error in the screen shared. You got a 200 which means that the flow is working. Even the screen shows the next api calls. This ticket is not showing any previous error. |
|
Here is the video of the instance working with all the endpoints (login, logout, refresh token) https://youtu.be/xXCCPsxqvR4 son then I don't think this change must be merged. |
|
t's not working with Keycloak. I'm not saying the instance itself is broken—I shared a screenshot to show that the header is indeed being sent to the OIDC server. |
|
And if you're saying it should only work with your configuration and that Keycloak or other providers don't need to be supported—well, so be it 😄 |
|
This is using Keycloak. Please take some time to read the video description and my previous comments. Until now this change is not approved by me. |
|
Can you tell me which version ov Keycloak it is because it is definetly not working with latest version? |
|
Please read the issue already raised with Keycloak keycloak/keycloak#12682 |
|
It is using the latest Keycloak version 26.3.2-0 The issue is at another point. Please check the image. 
I insist this code change is not the solution. |
|
Can you point me towards the right fix? The header is being sent from the interceptor wich is common for oauth and fineract requests, should we not use interceptor in case of oauth requests? |
|
Take you time to debug it. But this code is not the solution. |
|
I fixed the issue that you have shown in the screenshot, the CORS issue came after that. |
|
The fix is not present in the code in the PR or maybe you missed
making the commit. This is just a header removal. By the way you must
squash and commit to all the PR sent to the repository.
El lun, 11 ago 2025 a las 1:49, Sidhant Goel ***@***.***>)
escribió:
… *sidhantgoel* left a comment (openMF/web-app#2580)
<#2580 (comment)>
I fixed the issue that you have shown in the screenshot, the CORS issue
came after that.
—
Reply to this email directly, view it on GitHub
<#2580 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALD2ZAXXMSW6MQ4UD3QE3F33NBDHDAVCNFSM6AAAAACDKKVDVOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTCNZTGYYTSMBYGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
The issue here is that we're trying to augment Augmenting in this case either won’t work at all or would require a lot of hacks and/or copying code from the original module that provides I didn’t commit that code because I simply commented out the If required I can commit that code also. |
|
Do you have a video of your fix working?
El lun., 11 de agosto de 2025 3:02 a. m., Sidhant Goel <
***@***.***> escribió:
… *sidhantgoel* left a comment (openMF/web-app#2580)
<#2580 (comment)>
The issue here is that we're trying to augment HttpClient, which is
provided via provideHttpClient. Because of this, the provider
configuration we set up for creating HttpService doesn't take effect—
HttpClient is always injected instead of HttpService, and HttpClient
doesn't have the disableApiPrefix method.
Augmenting in this case either won’t work at all or would require a lot of
hacks and/or copying code from the original module that provides
HttpClient, which I don't recommend.
I didn’t commit that code because I simply commented out the
disableApiPrefix call. The handling for full URLs is already covered by
the ApiPrefixInterceptor, and we can always pass a full URL when we don’t
want the API prefix added.
If required I can commit that code also.
—
Reply to this email directly, view it on GitHub
<#2580 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALD2ZAWCNICS56QAZ7QT2VD3NBL2VAVCNFSM6AAAAACDKKVDVOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTCNZTHA2DGMRSGE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Screen.Recording.2025-08-11.at.2.48.33.PM.movHere is the screen recording |
|
It is possible to show the developer mode? I mean to view the keycloak
enpoints being called.
El lun., 11 de agosto de 2025 3:20 a. m., Sidhant Goel <
***@***.***> escribió:
… *sidhantgoel* left a comment (openMF/web-app#2580)
<#2580 (comment)>
https://github.com/user-attachments/assets/ed3b51b9-21e0-4506-966e-5e93e4afdf64
Here is the screen recording
—
Reply to this email directly, view it on GitHub
<#2580 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALD2ZAU5S7X7I674IOLTTMD3NBN4JAVCNFSM6AAAAACDKKVDVOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTCNZTHEYDKMJVGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Screen.Recording.2025-08-11.at.3.05.07.PM.movHere you go |
|
Squash and commit the pr
El lun., 11 de agosto de 2025 3:36 a. m., Sidhant Goel <
***@***.***> escribió:
… *sidhantgoel* left a comment (openMF/web-app#2580)
<#2580 (comment)>
https://github.com/user-attachments/assets/1111bd2e-8e85-408b-adc4-eae69a9b631c
Here you go
—
Reply to this email directly, view it on GitHub
<#2580 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALD2ZASBBKOF2IKAEFTK4S33NBPZNAVCNFSM6AAAAACDKKVDVOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTCNZTHE2TKOJTHA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
sidhantgoel
force-pushed
the
fix/oauth-keycloak
branch
from
550af74 to
2e21361
Compare
|
done, please check |
| environment.oauth.enabled && | ||
| (request.url.includes(`${environment.oauth.serverUrl}/token`) || | ||
| request.url.includes(`${environment.oauth.serverUrl}/logout`)) | ||
| ) { |
There was a problem hiding this comment.
The changes on this file are not required. Please remove from the PR
Description
The problem is Fineract-Platform-TenantId header is rejected by CORS policy on the endpoints exposed by keycloak.
This is a fix to remove this header from the requests to keycloack oauth server.
Related issues and discussion
WEB-267
Screenshots, if any
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
If you have multiple commits please combine them into one commit by squashing them.
Read and understood the contribution guidelines at
web-app/.github/CONTRIBUTING.md.