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: Unpin SAML library. #32167

Merged
merged 1 commit into from
May 3, 2023
Merged

fix: Unpin SAML library. #32167

merged 1 commit into from
May 3, 2023

Conversation

dianakhuang
Copy link
Contributor

@dianakhuang dianakhuang commented May 2, 2023

We were using an old version of the python3-saml library, which was causing issues with newer versions of social-core.

The reason it was pinned was because our etree implementation didn't support several fields that the saml library did, so we are now importing those entities as well.

This primarily affected devstack and any other version of Open edX that was running on a non-standard port.

edx/edx-arch-experiments#281

@dianakhuang dianakhuang force-pushed the diana/fix-saml-lib branch from 5713766 to 75b4429 Compare May 2, 2023 17:54
@timmc-edx timmc-edx self-assigned this May 2, 2023
Copy link
Contributor

@timmc-edx timmc-edx left a comment

Choose a reason for hiding this comment

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

I think this seems fine, but it might be nice to have a comment explaining the imports a little more for the next time someone is looking at it. :-)

We were using an old version of the python3-saml library,
which was causing issues with newer versions of social-core.

The reason it was pinned was because our etree implementation
didn't support several fields that the saml library did, so
we are now importing those entities as well.
@dianakhuang dianakhuang force-pushed the diana/fix-saml-lib branch from 9687eb2 to 8bef01c Compare May 3, 2023 14:23
Copy link
Contributor

@timmc-edx timmc-edx left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@dianakhuang dianakhuang merged commit 2d08a2a into master May 3, 2023
@dianakhuang dianakhuang deleted the diana/fix-saml-lib branch May 3, 2023 15:08
@edx-pipeline-bot
Copy link
Contributor

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

EdX Release Notice: This PR has been deployed to the production environment.

timmc-edx added a commit that referenced this pull request May 30, 2023
We're seeing an issue with repeated attributes causing auth failure for
some IdPs, and need to take some time to investigate. See unpinning issue
#32327 for details.

This is expected to reintroduce some bugs in testing SAML in devstack.

This has the effect of reverting the following PRs:
- Unpinned: #32167 (partial)
- Upgraded: #32168

However, we're keeping the safe_lxml adjustments from the first PR, so it
should be fine on that front to re-upgrade whenever we're ready.
timmc-edx added a commit that referenced this pull request May 30, 2023
We're seeing an issue with repeated attributes causing auth failure for
some IdPs, and need to take some time to investigate. See unpinning issue
#32327 for details.

This is expected to reintroduce some bugs in testing SAML in devstack.

This has the effect of reverting the following PRs:
- Unpinned: #32167 (partial)
- Upgraded: #32168

However, we're keeping the safe_lxml adjustments from the first PR, so it
should be fine on that front to re-upgrade whenever we're ready.
Yagnesh1998 pushed a commit to ManpraXSoftware/edx-platform that referenced this pull request Jun 8, 2023
…32328)

We're seeing an issue with repeated attributes causing auth failure for
some IdPs, and need to take some time to investigate. See unpinning issue
openedx#32327 for details.

This is expected to reintroduce some bugs in testing SAML in devstack.

This has the effect of reverting the following PRs:
- Unpinned: openedx#32167 (partial)
- Upgraded: openedx#32168

However, we're keeping the safe_lxml adjustments from the first PR, so it
should be fine on that front to re-upgrade whenever we're ready.
Yagnesh1998 pushed a commit to ManpraXSoftware/edx-platform that referenced this pull request Jun 8, 2023
…32328)

We're seeing an issue with repeated attributes causing auth failure for
some IdPs, and need to take some time to investigate. See unpinning issue
openedx#32327 for details.

This is expected to reintroduce some bugs in testing SAML in devstack.

This has the effect of reverting the following PRs:
- Unpinned: openedx#32167 (partial)
- Upgraded: openedx#32168

However, we're keeping the safe_lxml adjustments from the first PR, so it
should be fine on that front to re-upgrade whenever we're ready.
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.

5 participants