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 Concord Login #3486

Merged
merged 3 commits into from
Feb 24, 2022
Merged

Fix Concord Login #3486

merged 3 commits into from
Feb 24, 2022

Conversation

rajadain
Copy link
Member

@rajadain rajadain commented Feb 22, 2022

Overview

Users of the educational platform Concord Learn Portal have been unable to login since the Django upgrade. This happened because the signature that Django looks for in custom backends changed. By changing the signature of our custom backends to bring them back in line with what Django expects, we ensure that Concord users can log in again.

Note that this targets release/1.33.3, not develop.

Connects #3467

Demo

2022-02-22 16 39 56

Testing Instructions

Setup

  • Copy the secret password from LastPass for oAuth (MMW_ITSI_SECRET_KEY / MMW_CONCORD_SECRET_KEY)
  • Go into the app VM and populate the MMW_CONCORD_SECRET_KEY variable:
    vagrant ssh app -c 'echo *** | sudo tee /etc/mmw.d/env/MMW_CONCORD_SECRET_KEY'
  • Restart the mmw-app service so it picks up the new variable:
    vagrant ssh app -c 'sudo service mmw-app restart'
  • Go to http://localhost:8000/ and ensure you are not logged in
  • Try to log in using the "Learn Portal" link
    • In the Learn Portal, use the credentials from the MMW Teacher Learn Portal Concord entry in LastPass
    • Ensure a new account is created correctly for you
  • Log out of the account, and try to log back in using Lean Portal again
    • Ensure you are able to log back in to the same account

@rajadain rajadain added the NSF Funding Source: National Science Foundation label Feb 22, 2022
@rajadain rajadain requested a review from emilyhu0106 February 22, 2022 22:44
Rather than a simple object, to make it more compliant with
other backends.
Without this, the signature of our custom backend does not
match that of the function call. This signature is tested
in django.contrib.auth.authenticate here: https://github.com/django/django/blob/fdf209eab8949ddc345aa0212b349c79fc6fdebb/django/contrib/auth/__init__.py#L69
and `request` was added to that signature in Django 1.11
in django/django@4b9330c.

With this, the Concord users are authenticated correctly.
Previously we assumed that the error would have a message
attribute. This is not always the case. Now we only access
that when it is available.
@rajadain rajadain force-pushed the tt/fix-concord-login branch from 6926333 to f5f990c Compare February 23, 2022 17:54
Copy link
Contributor

@emilyhu0106 emilyhu0106 left a comment

Choose a reason for hiding this comment

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

Very excited to see this special part of MMW and learn about custom Django backend module

@rajadain rajadain merged commit a65680c into release/1.33.3 Feb 24, 2022
@rajadain rajadain deleted the tt/fix-concord-login branch February 24, 2022 05:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NSF Funding Source: National Science Foundation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants