Skip to content

Conversation

c00kiemon5ter
Copy link
Member

ref: IdentityPython/pysaml2#819

All Submissions:

  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Have you added an explanation of what problem you are trying to solve with this PR?
  • Have you added information on what your changes do and why you chose this as your solution?
  • Have you written new tests for your changes?
  • Does your submission pass tests?
  • This project follows PEP8 style guide. Have you run your code against the 'flake8' linter?

@c00kiemon5ter
Copy link
Member Author

@vladimir-mencl-eresearch in reference to IdentityPython/pysaml2#819 would like to test this?

logger.debug(logline)
raise SATOSAAuthenticationError(context.state, msg)
self.outstanding_queries[req_id] = req
self.outstanding_queries[req_id] = req_id
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that this looks weird.

outstanding_queries is used as a dict throughout the code. It seems that the value content is not used; the value is only checked to be non-null (value is not None). This allows us to put anything we want as the value, except for None.

I think that outstanding_queries could be turned into a list/set. But that would require to sync changes under pysaml2. We can look into that later.

In general, the way we check for unsolicited responses should be refactored.

@vladimir-mencl-eresearch
Copy link
Contributor

Hi @c00kiemon5ter ,

Thanks - yes, I'm happy to confirm this works in my environment and does the right thing - SAML AuthnRequest is only signed via external Signature parameter when authn_requests_signed is True, or not at all when authn_requests_signed is False - but the request XML no longer carries embedded signature.

Thanks for the fix - and sorry, did not get to it yet myself.

Cheers,
Vlad

@c00kiemon5ter c00kiemon5ter force-pushed the refactor-saml-backend-create-authnreq branch from 9bdcfef to 7ed0774 Compare August 27, 2021 19:08
@c00kiemon5ter c00kiemon5ter merged commit ad1efcb into master Aug 27, 2021
@c00kiemon5ter c00kiemon5ter deleted the refactor-saml-backend-create-authnreq branch August 27, 2021 21:04
jonathanperret added a commit to proconnect-gouv/oidc2fer that referenced this pull request Sep 18, 2025
The current version of pysaml2 (7.5.2) has an issue where AuthNRequests
are both signed in the XML and with an extra `Signature` queryparam.
This was reported initially in 2021:
    IdentityPython/pysaml2#819
And it was fixed by a changed in SATOSA:
    IdentityPython/SATOSA#380
But it reappeared apparently and the original reporter has a PR open
against pysaml2 that is supposed to fix it:
    IdentityPython/pysaml2#973
They report that the regression was introduced in pysaml2 by
   IdentityPython/pysaml2#834
We try here to pin pysaml2 to the last version before this PR was
merged. Unfortunately this is quite an old version, but from basic
testing it seems to still be compatible with the current SATOSA
version.
Hopefully this can be temporary.
jonathanperret added a commit to proconnect-gouv/oidc2fer that referenced this pull request Sep 25, 2025
The current version of pysaml2 (7.5.2) has an issue where AuthNRequests
are both signed in the XML and with an extra `Signature` queryparam.
This was reported initially in 2021:
    IdentityPython/pysaml2#819
And it was fixed by a changed in SATOSA:
    IdentityPython/SATOSA#380
But it reappeared apparently and the original reporter has a PR open
against pysaml2 that is supposed to fix it:
    IdentityPython/pysaml2#973
They report that the regression was introduced in pysaml2 by
   IdentityPython/pysaml2#834
We try here to pin pysaml2 to the last version before this PR was
merged. Unfortunately this is quite an old version, but from basic
testing it seems to still be compatible with the current SATOSA
version.
Hopefully this can be temporary.
jonathanperret added a commit to proconnect-gouv/oidc2fer that referenced this pull request Sep 25, 2025
The current version of pysaml2 (7.5.2) has an issue where AuthNRequests
are both signed in the XML and with an extra `Signature` queryparam.
This was reported initially in 2021:
    IdentityPython/pysaml2#819
And it was fixed by a changed in SATOSA:
    IdentityPython/SATOSA#380
But it reappeared apparently and the original reporter has a PR open
against pysaml2 that is supposed to fix it:
    IdentityPython/pysaml2#973
They report that the regression was introduced in pysaml2 by
   IdentityPython/pysaml2#834
We try here to pin pysaml2 to the last version before this PR was
merged. Unfortunately this is quite an old version, but from basic
testing it seems to still be compatible with the current SATOSA
version.
This in turn forces us to also pin xmlschema to avoid
    IdentityPython/pysaml2#947

Hopefully this can be temporary.
jonathanperret added a commit to proconnect-gouv/oidc2fer that referenced this pull request Sep 25, 2025
The current version of pysaml2 (7.5.2) has an issue where AuthNRequests
are both signed in the XML and with an extra `Signature` queryparam.
This was reported initially in 2021:
    IdentityPython/pysaml2#819
And it was fixed by a changed in SATOSA:
    IdentityPython/SATOSA#380
But it reappeared apparently and the original reporter has a PR open
against pysaml2 that is supposed to fix it:
    IdentityPython/pysaml2#973
They report that the regression was introduced in pysaml2 by
   IdentityPython/pysaml2#834
We try here to pin pysaml2 to the last version before this PR was
merged. Unfortunately this is quite an old version, but from basic
testing it seems to still be compatible with the current SATOSA
version.
This in turn forces us to also pin xmlschema to avoid
    IdentityPython/pysaml2#947

Hopefully this can be temporary.
jonathanperret added a commit to proconnect-gouv/oidc2fer that referenced this pull request Sep 25, 2025
The current version of pysaml2 (7.5.2) has an issue where AuthNRequests
are both signed in the XML and with an extra `Signature` queryparam.
This was reported initially in 2021:
    IdentityPython/pysaml2#819
And it was fixed by a changed in SATOSA:
    IdentityPython/SATOSA#380
But it reappeared apparently and the original reporter has a PR open
against pysaml2 that is supposed to fix it:
    IdentityPython/pysaml2#973
They report that the regression was introduced in pysaml2 by
   IdentityPython/pysaml2#834
We try here to pin pysaml2 to the last version before this PR was
merged. Unfortunately this is quite an old version, but from basic
testing it seems to still be compatible with the current SATOSA
version.
This in turn forces us to also pin xmlschema to avoid
    IdentityPython/pysaml2#947

Hopefully this can be temporary.
jonathanperret added a commit to proconnect-gouv/oidc2fer that referenced this pull request Sep 25, 2025
The current version of pysaml2 (7.5.2) has an issue where AuthNRequests
are both signed in the XML and with an extra `Signature` queryparam.
This was reported initially in 2021:
    IdentityPython/pysaml2#819
And it was fixed by a changed in SATOSA:
    IdentityPython/SATOSA#380
But it reappeared apparently and the original reporter has a PR open
against pysaml2 that is supposed to fix it:
    IdentityPython/pysaml2#973
They report that the regression was introduced in pysaml2 by
   IdentityPython/pysaml2#834
We try here to pin pysaml2 to the last version before this PR was
merged. Unfortunately this is quite an old version, but from basic
testing it seems to still be compatible with the current SATOSA
version.
This in turn forces us to also pin xmlschema to avoid
    IdentityPython/pysaml2#947

Hopefully this can be temporary.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants