Skip to content
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

Conversation

randall-vx
Copy link

@randall-vx randall-vx commented Sep 18, 2023

Decrypt matching_value response to compare output response with response.name_id.text. It shows AttributeError: 'NoneType' object has no attribute 'text'.

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

Fixes #553

…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)
@OCA-git-bot
Copy link
Contributor

Hi @vincent-hatakeyama,
some modules you are maintaining are being modified, check this out!

@vincent-hatakeyama
Copy link
Contributor

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.

@randall-vx
Copy link
Author

@vincent-hatakeyama What are you waiting for? Add a test where an error is shown without those parameters?

cc: @rolandojduartem

@randall-vx
Copy link
Author

@vincent-hatakeyama also coverage is increased.

cc: @rolandojduartem

@vincent-hatakeyama
Copy link
Contributor

What are you waiting for? Add a test where an error is shown without those parameters?

That would be best if possible.
Otherwise, a simple scenario to reproduce the error would help. I didn’t see the error when testing with a local keycloak, but it might be linked with an option I did not test or something similar.

The version 15.0 of the module is in production without having such a fix, and the error is not raised.

@randall-vx
Copy link
Author

@vincent-hatakeyama

That would be best if possible. Otherwise, a simple scenario to reproduce the error would help. I didn’t see the error when testing with a local keycloak, but it might be linked with an option I did not test or something similar.

On AD server side, we have a 256 encryption key, so a _validate_auth_response(token, base_url) returns a matching_value doesn't match with a response.name_id.text. Without encryption_keypairs cannot decrypt /tmp/file1.xml to compare with /tmp/file2.xml.

Also, (I hope, in a short time) to make an MR where the metadata can be signed with other certificate than CA Certificate.

The version 15.0 of the module is in production without having such a fix, and the error is not raised.

Now I'm running on v16.0 production server.

cc: @rolandojduartem

@randall-vx
Copy link
Author

Hi @vincent-hatakeyama do you have any news?

regards,

@vincent-hatakeyama
Copy link
Contributor

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.

Copy link

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.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Feb 11, 2024
@github-actions github-actions bot closed this Mar 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale PR/Issue without recent activity, it'll be soon closed automatically.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[16.0] auth_saml: Error decrypt matching_value response to compare output response.name_id.text
3 participants