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

Upgrade django-allauth to 0.57.0 #4715

Merged
merged 22 commits into from Nov 17, 2023
Merged

Upgrade django-allauth to 0.57.0 #4715

merged 22 commits into from Nov 17, 2023

Conversation

LMNTL
Copy link
Contributor

@LMNTL LMNTL commented Nov 3, 2023

Checklist

  1. If you've added code that should be tested, add tests
  2. If you've changed APIs, update (or create!) the documentation
  3. Ensure the tests pass
  4. Make sure that your code lints and that you've followed our coding style
  5. Write a title and, if necessary, a description of your work suitable for publishing in our release notes
  6. Mention any related issues in this repository (as #ISSUE) and in other repositories (as kobotoolbox/other#ISSUE)
  7. Open an issue in the docs if there are UI/UX changes

Notes

This PR bumps the version of django-allauth from 0.54.0 to 0.57.0 (changelog). The major benefit is that SSO providers are able to be configured directly from the django admin, instead of through environment variables.

Since the provider_id field is required by the {% provider_login_url %} field, and the model itself can't be changed from our side, this PR also makes the provider_id field mandatory in the allauth admin.

@LMNTL LMNTL requested a review from bufke November 3, 2023 21:17
Copy link

Upgrade django-allauth

Copy link

Copy link

Copy link

Copy link

@LMNTL LMNTL changed the base branch from main to beta November 10, 2023 15:26
Copy link

1 similar comment
Copy link

@bufke
Copy link
Contributor

bufke commented Nov 15, 2023

  1. Create two open id providers. In my test one was Azure and the other was junk.
  2. Go to http://localhost/accounts/login/

MultipleObjectsReturned at /accounts/login/

You could probably develop a unit test for this as I don't think it matters what the social apps are. They can be fake junk as long as they are oidc.

self.fields['provider_id'].required = True

def clean_provider_id(self):
reserved_keywords = ['kobo']
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO if we are blocking in django admin, it would be fine to remove the "Please do not use kobo as the id value" check. But I'll defer to you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, especially since the code to parse OIDC sub-providers has been removed - it's now guarding against an impossible-to-achieve state (at least for someone not trying to break it.)

@LMNTL LMNTL requested a review from bufke November 15, 2023 20:37
@bufke bufke merged commit 5ec0ce5 into beta Nov 17, 2023
4 checks passed
@LMNTL LMNTL deleted the django-allauth-0-57-0 branch November 27, 2023 17:29
@LMNTL LMNTL mentioned this pull request Dec 29, 2023
7 tasks
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.

None yet

2 participants