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

feat(TPA): Add middleware to handle TPA exceptions #171

Merged
merged 1 commit into from
Aug 18, 2021

Conversation

MoisesGSalas
Copy link
Member

@MoisesGSalas MoisesGSalas commented Aug 13, 2021

Description

During the third party authentication process there are some errors that are not correctly handled and result in a misleading 500 error page.

As far as we know, this issue happens under any of this three conditions, so far:

  1. The connection with the Identity Provider (IdP) results in an HTTP Error. See social_core/backends/base.py
  2. The pipeline step safer_associate_by_email raises EoxTenantAuthException when trying to associate a staff/admin account or when an account with the same email already exists. This is because EoxTenantAuthException doesn't inherit from SocialAuthBaseException.
  3. If the step safer_associate_by_email is not defined in the pipeline a database IntegrityError is raised due to conflicting emails.

To solve the issue a new middleware class was created that process this exceptions and passes the correct AuthException to the next layer in the middleware chain.

Testing Instructions

  1. To test (1) configure your OIDC provider such as that OIDC_ENDPOINT points to the wrong url.
  2. To test (2) try to login with an IdP that returns the email of an account with staff/admin access.
  3. To test (3) try to login with an IdP that returns the email of an account that already exists without defining safer_associate_by_email in the pipeline.

Aditional details

The error message should look like this:

http_error

@mariajgrimaldi
Copy link
Contributor

Can you add a before and after screenshot?

@MoisesGSalas
Copy link
Member Author

@mariajgrimaldi Can you add a before and after screenshot?

The before screen is a regular 500 error page. I added a screenshot with how the error message is displayed.

@MoisesGSalas MoisesGSalas merged commit d167364 into master Aug 18, 2021
johanseto added a commit to eduNEXT/eox-tenant that referenced this pull request Dec 19, 2023
This change the heritance of the eox-tenant-auth-exception.

This with the purpose of the exception to be handled by the social tpa middleware exception
process.

You can see here that the new class is also based in value error exception.
https://github.com/python-social-auth/social-core/blob/29cbbd22b98d81d569a886b1cc0bd9a316cd124f/social_core/exceptions.py#L1

But the change is related with this PR:
eduNEXT/eox-core#171
So now as the exception is family  of the SocialAuthBaseException.

The the tpa middleware could managed it.
https://github.com/python-social-auth/social-app-django/blob/5.4.0/social_django/middleware.py#L35

Keep in mind that this middleware is parent of edx-platform middleware.
https://github.com/openedx/edx-platform/blob/ebcbe1cd9208191c0589d7fe538c6ac13470abe6/common/djangoapps/third_party_auth/middleware.py#L18
johanseto added a commit to eduNEXT/eox-tenant that referenced this pull request Dec 19, 2023
This change the heritance of the eox-tenant-auth-exception.

This with the purpose of the exception to be handled by the social tpa middleware exception
process.

You can see here that the new class is also based in value error exception.
https://github.com/python-social-auth/social-core/blob/29cbbd22b98d81d569a886b1cc0bd9a316cd124f/social_core/exceptions.py#L1

But the change is related with this PR:
eduNEXT/eox-core#171
So now as the exception is family  of the SocialAuthBaseException.

The the tpa middleware could managed it.
https://github.com/python-social-auth/social-app-django/blob/5.4.0/social_django/middleware.py#L35

Keep in mind that this middleware is parent of edx-platform middleware.
https://github.com/openedx/edx-platform/blob/ebcbe1cd9208191c0589d7fe538c6ac13470abe6/common/djangoapps/third_party_auth/middleware.py#L18
johanseto added a commit to eduNEXT/eox-tenant that referenced this pull request Jan 4, 2024
This change the heritance of the eox-tenant-auth-exception.

This with the purpose of the exception to be handled by the social tpa middleware exception
process.

You can see here that the new class is also based in value error exception.
https://github.com/python-social-auth/social-core/blob/29cbbd22b98d81d569a886b1cc0bd9a316cd124f/social_core/exceptions.py#L1

But the change is related with this PR:
eduNEXT/eox-core#171
So now as the exception is family  of the SocialAuthBaseException.

The the tpa middleware could managed it.
https://github.com/python-social-auth/social-app-django/blob/5.4.0/social_django/middleware.py#L35

Keep in mind that this middleware is parent of edx-platform middleware.
https://github.com/openedx/edx-platform/blob/ebcbe1cd9208191c0589d7fe538c6ac13470abe6/common/djangoapps/third_party_auth/middleware.py#L18
johanseto added a commit to eduNEXT/eox-tenant that referenced this pull request Jan 4, 2024
* refactor: inherit from social auth exception

This change the heritance of the eox-tenant-auth-exception.

This with the purpose of the exception to be handled by the social tpa middleware exception
process.

You can see here that the new class is also based in value error exception.
https://github.com/python-social-auth/social-core/blob/29cbbd22b98d81d569a886b1cc0bd9a316cd124f/social_core/exceptions.py#L1

But the change is related with this PR:
eduNEXT/eox-core#171
So now as the exception is family  of the SocialAuthBaseException.

The the tpa middleware could managed it.
https://github.com/python-social-auth/social-app-django/blob/5.4.0/social_django/middleware.py#L35

Keep in mind that this middleware is parent of edx-platform middleware.
https://github.com/openedx/edx-platform/blob/ebcbe1cd9208191c0589d7fe538c6ac13470abe6/common/djangoapps/third_party_auth/middleware.py#L18

* refactor: social-auth-core pkg inclusion
johanseto added a commit to eduNEXT/eox-tenant that referenced this pull request Jan 4, 2024
* refactor: inherit from social auth exception

This change the heritance of the eox-tenant-auth-exception.

This with the purpose of the exception to be handled by the social tpa middleware exception
process.

You can see here that the new class is also based in value error exception.
https://github.com/python-social-auth/social-core/blob/29cbbd22b98d81d569a886b1cc0bd9a316cd124f/social_core/exceptions.py#L1

But the change is related with this PR:
eduNEXT/eox-core#171
So now as the exception is family  of the SocialAuthBaseException.

The the tpa middleware could managed it.
https://github.com/python-social-auth/social-app-django/blob/5.4.0/social_django/middleware.py#L35

Keep in mind that this middleware is parent of edx-platform middleware.
https://github.com/openedx/edx-platform/blob/ebcbe1cd9208191c0589d7fe538c6ac13470abe6/common/djangoapps/third_party_auth/middleware.py#L18

* refactor: social-auth-core pkg inclusion

(cherry picked from commit c542749)
johanseto added a commit to eduNEXT/eox-tenant that referenced this pull request Feb 9, 2024
* refactor: inherit from social auth exception

This change the heritance of the eox-tenant-auth-exception.

This with the purpose of the exception to be handled by the social tpa middleware exception
process.

You can see here that the new class is also based in value error exception.
https://github.com/python-social-auth/social-core/blob/29cbbd22b98d81d569a886b1cc0bd9a316cd124f/social_core/exceptions.py#L1

But the change is related with this PR:
eduNEXT/eox-core#171
So now as the exception is family  of the SocialAuthBaseException.

The the tpa middleware could managed it.
https://github.com/python-social-auth/social-app-django/blob/5.4.0/social_django/middleware.py#L35

Keep in mind that this middleware is parent of edx-platform middleware.
https://github.com/openedx/edx-platform/blob/ebcbe1cd9208191c0589d7fe538c6ac13470abe6/common/djangoapps/third_party_auth/middleware.py#L18

* refactor: social-auth-core pkg inclusion

(cherry picked from commit c542749)
johanseto added a commit to eduNEXT/eox-tenant that referenced this pull request Feb 9, 2024
* refactor: inherit from social auth exception

This change the heritance of the eox-tenant-auth-exception.

This with the purpose of the exception to be handled by the social tpa middleware exception
process.

You can see here that the new class is also based in value error exception.
https://github.com/python-social-auth/social-core/blob/29cbbd22b98d81d569a886b1cc0bd9a316cd124f/social_core/exceptions.py#L1

But the change is related with this PR:
eduNEXT/eox-core#171
So now as the exception is family  of the SocialAuthBaseException.

The the tpa middleware could managed it.
https://github.com/python-social-auth/social-app-django/blob/5.4.0/social_django/middleware.py#L35

Keep in mind that this middleware is parent of edx-platform middleware.
https://github.com/openedx/edx-platform/blob/ebcbe1cd9208191c0589d7fe538c6ac13470abe6/common/djangoapps/third_party_auth/middleware.py#L18

* refactor: social-auth-core pkg inclusion

(cherry picked from commit c542749)

chore: run make upgrade
johanseto added a commit to eduNEXT/eox-tenant that referenced this pull request Mar 4, 2024
* refactor: inherit from social auth exception

This change the heritance of the eox-tenant-auth-exception.

This with the purpose of the exception to be handled by the social tpa middleware exception
process.

You can see here that the new class is also based in value error exception.
https://github.com/python-social-auth/social-core/blob/29cbbd22b98d81d569a886b1cc0bd9a316cd124f/social_core/exceptions.py#L1

But the change is related with this PR:
eduNEXT/eox-core#171
So now as the exception is family  of the SocialAuthBaseException.

The the tpa middleware could managed it.
https://github.com/python-social-auth/social-app-django/blob/5.4.0/social_django/middleware.py#L35

Keep in mind that this middleware is parent of edx-platform middleware.
https://github.com/openedx/edx-platform/blob/ebcbe1cd9208191c0589d7fe538c6ac13470abe6/common/djangoapps/third_party_auth/middleware.py#L18

* refactor: social-auth-core pkg inclusion

(cherry picked from commit c542749)

chore: run make upgrade
MaferMazu pushed a commit to eduNEXT/eox-tenant that referenced this pull request Apr 11, 2024
* refactor: inherit from social auth exception

* refactor: inherit from social auth exception

This change the heritance of the eox-tenant-auth-exception.

This with the purpose of the exception to be handled by the social tpa middleware exception
process.

You can see here that the new class is also based in value error exception.
https://github.com/python-social-auth/social-core/blob/29cbbd22b98d81d569a886b1cc0bd9a316cd124f/social_core/exceptions.py#L1

But the change is related with this PR:
eduNEXT/eox-core#171
So now as the exception is family  of the SocialAuthBaseException.

The the tpa middleware could managed it.
https://github.com/python-social-auth/social-app-django/blob/5.4.0/social_django/middleware.py#L35

Keep in mind that this middleware is parent of edx-platform middleware.
https://github.com/openedx/edx-platform/blob/ebcbe1cd9208191c0589d7fe538c6ac13470abe6/common/djangoapps/third_party_auth/middleware.py#L18

* refactor: social-auth-core pkg inclusion

(cherry picked from commit c542749)

chore: run make upgrade

* fix: pylint update change way to load config

add use-yield-from (R1737) due pylint update to 3.1.0
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.

3 participants