-
-
Notifications
You must be signed in to change notification settings - Fork 424
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FIX] auth_saml: decrypt matching_value response to compare output response.name_id.text #552
[FIX] auth_saml: decrypt matching_value response to compare output response.name_id.text #552
Conversation
…sponse.name_id.text Decrypt matching_value response to compare output response with response.name_id.text. It shows AttributeError: 'NoneType' object has no attribute 'text'. ```python 2023-09-17 02:02:54,015 4049 ERROR odoo odoo.addons.auth_saml.controllers.main: SAML2: failure - 'NoneType' object has no attribute 'text' Traceback (most recent call last): File "/home/odoo/instance/extra_addons/server-auth/auth_saml/controllers/main.py", line 219, in signin .auth_saml( File "/home/odoo/instance/extra_addons/server-auth/auth_saml/models/res_users.py", line 64, in auth_saml validation = self._auth_saml_validate(provider, saml_response, base_url) File "/home/odoo/instance/extra_addons/server-auth/auth_saml/models/res_users.py", line 29, in _auth_saml_validate return provider._validate_auth_response(token, base_url) File "/home/odoo/instance/extra_addons/server-auth/auth_saml/models/auth_saml_provider.py", line 296, in _validate_auth_response matching_value = response.name_id.text AttributeError: 'NoneType' object has no attribute 'text' ``` Check [class SecurityContext in saml2 lib](https://github.com/IdentityPython/pysaml2/blob/master/src/saml2/sigver.py#L1186)
Hi @vincent-hatakeyama, |
Hi, There’s no issue opened on the subject so in which case do you get the message? Can it be reproduced by adding a test as it is not raised in the current tests. Regards. |
@vincent-hatakeyama What are you waiting for? Add a test where an error is shown without those parameters? cc: @rolandojduartem |
@vincent-hatakeyama also coverage is increased. cc: @rolandojduartem |
That would be best if possible. The version 15.0 of the module is in production without having such a fix, and the error is not raised. |
On AD server side, we have a 256 encryption key, so a Also, (I hope, in a short time) to make an MR where the metadata can be signed with other certificate than CA Certificate.
Now I'm running on v16.0 production server. cc: @rolandojduartem |
Hi @vincent-hatakeyama do you have any news? regards, |
I’ve tested auth_saml in odoo 16 again with a production keycloak server. It works correctly, so this can not be a fix. There’s encryption configuration in keycloak. When enabled, the authentication does not work anymore, but that’s to be expected as the module does not handle encryption. The changes can not be merged as is because they will break all installations of auth_saml. This needs to be configurable by SAML provider like other options are, and ideally, the keys used need for the encryption should be configurable too. |
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
Decrypt matching_value response to compare output response with response.name_id.text. It shows AttributeError: 'NoneType' object has no attribute 'text'.
Check class SecurityContext in saml2 lib
Fixes #553