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
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
a02f953
bump django-allauth version
LMNTL Sep 19, 2023
9a90aa7
bump allauth from 0.56.0 to 0.56.1 due to yank
LMNTL Sep 19, 2023
87fbbb6
use allauth.account.middleware.AccountMiddleware
LMNTL Sep 19, 2023
1ccd66f
add provider_id to /environment endpoint
LMNTL Sep 20, 2023
909ff13
fix link to SSO registration
LMNTL Sep 20, 2023
3155a2b
remove deprecated provider.get_app()
LMNTL Sep 20, 2023
2844ce3
use provider id in login template
LMNTL Sep 20, 2023
d5fc924
change SERVERS to APPS for OIDC provider env vars
LMNTL Sep 20, 2023
a6b323a
Merge branch 'release/2.023.37' into django-allauth-0-56-1
LMNTL Sep 20, 2023
a2498b0
Merge remote-tracking branch 'origin/release-2.023.37-with-usage-fron…
LMNTL Sep 22, 2023
39141fa
Merge remote-tracking branch 'origin/release/2.023.37' into django-al…
LMNTL Sep 22, 2023
29ca302
bump django-allauth to 0.57.0
LMNTL Oct 4, 2023
36058cc
don't parse env vars for sso providers, to avoid clashing with provid…
LMNTL Oct 4, 2023
3b51aec
use provider id instead of provider in provider_login_url
LMNTL Oct 4, 2023
60404be
make provider_id required in django-allauth admin
LMNTL Nov 3, 2023
8487141
don't check settings for socialaccounts before making queries, since …
LMNTL Nov 6, 2023
28347f2
Merge branch 'beta' into django-allauth-0-57-0
LMNTL Nov 9, 2023
db3b44a
don't error if SOCIALACCOUNT_PROVIDERS is missing, check ID for non-O…
LMNTL Nov 9, 2023
39c103f
don't allow 'kobo' as a provider_id
LMNTL Nov 9, 2023
373bdfa
require provider_id and provider be unique at the admin level
LMNTL Nov 15, 2023
6bdc04e
use provider_id instead of provider in provider_login_url
LMNTL Nov 15, 2023
b964fa5
remove check for 'kobo' id set through env vars
LMNTL Nov 15, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion dependencies/pip/dev_requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ django==3.2.15
# jsonfield
# kobo-service-account
# model-bakery
django-allauth==0.54.0
django-allauth==0.57.0
# via -r dependencies/pip/requirements.in
django-amazon-ses==4.0.1
# via -r dependencies/pip/requirements.in
Expand Down
2 changes: 1 addition & 1 deletion dependencies/pip/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ django==3.2.15
# djangorestframework
# jsonfield
# kobo-service-account
django-allauth==0.54.0
django-allauth==0.57.0
# via -r dependencies/pip/requirements.in
django-amazon-ses==4.0.1
# via -r dependencies/pip/requirements.in
Expand Down
3 changes: 1 addition & 2 deletions jsapp/js/account/security/sso/ssoSection.component.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import React from 'react';
import {observer} from 'mobx-react-lite';
import sessionStore from 'js/stores/session';
import {PATHS} from 'js/router/routerConstants';
import styles from './ssoSection.module.scss';
import {deleteSocialAccount} from './sso.api';
import Button from 'jsapp/js/components/common/button';
Expand Down Expand Up @@ -53,7 +52,7 @@ const SsoSection = observer(() => {
<a
href={
'accounts/' +
socialApp.provider +
(socialApp.provider_id || socialApp.provider) +
'/login/?process=connect&next=%2F%23%2Faccount%2Fsecurity'
}
className={styles.passwordLink}
Expand Down
1 change: 1 addition & 0 deletions jsapp/js/dataInterface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,7 @@ export type PermissionsConfigResponse = PaginatedResponse<PermissionDefinition>;

interface SocialAccount {
provider: string;
provider_id: string;
uid: string;
last_login: string;
date_joined: string;
Expand Down
1 change: 1 addition & 0 deletions jsapp/js/envStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ export interface EnvStoreFieldItem {
export interface SocialApp {
name: string;
provider: string;
provider_id: string;
client_id: string;
}

Expand Down
28 changes: 13 additions & 15 deletions kobo/apps/accounts/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,24 +13,22 @@ class AccountExtrasConfig(AppConfig):
@register()
def check_socialaccount_providers(app_configs, **kwargs):
"""
Don't allow `kobo` to be set as the `id` value in `SOCIALACCOUNT_PROVIDERS`
Don't allow `kobo` to be set as the `id` value in `SOCIALACCOUNT_PROVIDERS`
settings because it breaks the login page redirect when language is changed.
"""
errors = []
social_app_ids = [
apps['id']
for apps in settings.SOCIALACCOUNT_PROVIDERS['openid_connect'][
'SERVERS'
if hasattr(settings, 'SOCIALACCOUNT_PROVIDERS'):
social_app_ids = [
apps.get('APPS', []) for apps in settings.SOCIALACCOUNT_PROVIDERS
]
]
if 'kobo' in social_app_ids:
errors.append(
Error(
f'Please do not use `kobo` as the `id` value in '
'`SOCIALACCOUNT_PROVIDERS` settings.',
hint='`kobo` is not a valid value for this setting.',
obj=settings,
id='kobo.apps.accounts.E001',
if 'kobo' in social_app_ids:
errors.append(
Error(
f'Please do not use `kobo` as the `id` value in '
'`SOCIALACCOUNT_PROVIDERS` settings.',
hint='`kobo` is not a valid value for this setting.',
obj=settings,
id='kobo.apps.accounts.E001',
)
)
)
return errors
6 changes: 2 additions & 4 deletions kobo/apps/accounts/templatetags/get_provider_appname.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def get_provider_appname(context, provider=None):
provider = provider or context['provider']
request = context['request']
try:
appname = provider.get_app(request).name
appname = provider.app.name
if appname:
return appname
return SocialApp.objects.get_current(provider, request).name
Expand All @@ -36,6 +36,4 @@ def get_provider_appname(context, provider=None):

@register.simple_tag()
def get_social_apps():
if settings.SOCIALACCOUNT_PROVIDERS:
return SocialApp.objects.filter(custom_data__isnull=True)
return []
return SocialApp.objects.filter(custom_data__isnull=True)
Empty file.
48 changes: 48 additions & 0 deletions kobo/apps/django_allauth/admin.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
from allauth.socialaccount.admin import SocialAppForm, SocialAppAdmin
from allauth.socialaccount.models import SocialApp
from django.contrib import admin
from django.core.exceptions import ValidationError
from django.db.models import Q


class RequireProviderIdSocialAppForm(SocialAppForm):
def __init__(self, *args, **kwargs):
super(SocialAppForm, self).__init__(*args, **kwargs)
# require the provider_id in the admin, since we can't make it required on allauth's model
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.)

provider_id = self.cleaned_data.get('provider_id')
# we check this already for the ID in kobo/apps/accounts/apps.py
if provider_id in reserved_keywords:
raise ValidationError(
f'`{provider_id}` is not a valid value for the `provider_id` setting.'
)

"""
By default, django-allauth only supports showing one provider on the login screen.
But OIDC providers allow multiple subproviders, so kpi has some additional code to display multiple providers.
Because of that, we need to make sure that the `provider` and `provider_id` fields are unique.
django-allauth (as of 0.57.0) technically enforces this on the model level, but in practice it's flawed.
"""
if SocialApp.objects.filter(
Q(provider_id=provider_id) |
Q(provider=provider_id)
).exists():
raise ValidationError(
"""The Provider ID value must be unique and cannot match an existing Provider name.
Please use a different value."""
)
return provider_id


class RequireProviderIdSocialAppAdmin(SocialAppAdmin):
form = RequireProviderIdSocialAppForm

class Meta:
proxy = True


admin.site.unregister(SocialApp)
admin.site.register(SocialApp, RequireProviderIdSocialAppAdmin)
57 changes: 2 additions & 55 deletions kobo/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,13 +128,15 @@
'kobo.apps.trash_bin.TrashBinAppConfig',
'kobo.apps.markdownx_uploader.MarkdownxUploaderAppConfig',
'kobo.apps.form_disclaimer.FormDisclaimerAppConfig',
'kobo.apps.django_allauth',
)

MIDDLEWARE = [
'corsheaders.middleware.CorsMiddleware',
'django.middleware.security.SecurityMiddleware',
'django.contrib.sessions.middleware.SessionMiddleware',
'hub.middleware.LocaleMiddleware',
'allauth.account.middleware.AccountMiddleware',
'django.middleware.common.CommonMiddleware',
'django.middleware.csrf.CsrfViewMiddleware',
'django.contrib.auth.middleware.AuthenticationMiddleware',
Expand Down Expand Up @@ -1031,61 +1033,6 @@ def dj_stripe_request_callback_method():
"UNSAFE_SSO_REGISTRATION_EMAIL_DISABLE", False
)

# See https://django-allauth.readthedocs.io/en/latest/configuration.html
# Map env vars to upstream dict values, include exact case. Underscores for delimiter.
# Example: SOCIALACCOUNT_PROVIDERS_provider_SETTING
# Use numbers for arrays such as _1_FOO, _1_BAR, _2_FOO, _2_BAR
SOCIALACCOUNT_PROVIDERS = {}
if MICROSOFT_TENANT := env.str('SOCIALACCOUNT_PROVIDERS_microsoft_TENANT', None):
SOCIALACCOUNT_PROVIDERS['microsoft'] = {'TENANT': MICROSOFT_TENANT}
# Parse oidc settings as nested dict in array. Example:
# SOCIALACCOUNT_PROVIDERS_openid_connect_SERVERS_0_id: "google" # Must be unique
# SOCIALACCOUNT_PROVIDERS_openid_connect_SERVERS_0_server_url: "https://accounts.google.com"
# SOCIALACCOUNT_PROVIDERS_openid_connect_SERVERS_0_name: "Kobo Google Apps"
# Only OIDC supports multiple providers. For example, to add two Google Apps sign ins - use
# OIDC and assign them a different server number. Do not use the allauth google provider.
oidc_prefix = "SOCIALACCOUNT_PROVIDERS_openid_connect_SERVERS_"
oidc_pattern = re.compile(r"{prefix}\w+".format(prefix=oidc_prefix))
oidc_servers = {}
oidc_nested_keys = ['APP', 'SCOPE', 'AUTH_PARAMS']

for key, value in {
key.replace(oidc_prefix, ""): val
for key, val in os.environ.items()
if oidc_pattern.match(key)
}.items():
number, setting = key.split("_", 1)
parsed_key = None
nested_key = filter(lambda setting_key : setting.startswith(setting_key), oidc_nested_keys)
nested_key = list(nested_key)
if len(nested_key):
_, parsed_key = setting.split(nested_key[0] + "_", 1)
setting = nested_key[0]
if number in oidc_servers:
if parsed_key:
if setting in oidc_servers[number]:
if parsed_key.isdigit():
oidc_servers[number][setting].append(value)
else:
oidc_servers[number][setting][parsed_key] = value
else:
if parsed_key.isdigit():
oidc_servers[number][setting] = [value]
else:
oidc_servers[number][setting] = {parsed_key: value}
else:
oidc_servers[number][setting] = value
else:
if parsed_key:
if parsed_key.isdigit():
oidc_servers[number] = {setting: [value]}
else:
oidc_servers[number] = {setting: {parsed_key: value}}
else:
oidc_servers[number] = {setting: value}
oidc_servers = [x for x in oidc_servers.values()]
SOCIALACCOUNT_PROVIDERS["openid_connect"] = {"SERVERS": oidc_servers}

WEBPACK_LOADER = {
'DEFAULT': {
'BUNDLE_DIR_NAME': 'jsapp/compiled/',
Expand Down
13 changes: 4 additions & 9 deletions kpi/views/environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,16 +156,11 @@ def process_user_metadata_configs(request):
def process_other_configs(request):
data = {}

# django-allauth social apps are configured in both settings and the
# database. Optimize by avoiding extra DB call when unnecessary
social_apps = []
if settings.SOCIALACCOUNT_PROVIDERS:
social_apps = list(
SocialApp.objects.filter(custom_data__isnull=True).values(
'provider', 'name', 'client_id'
)
data['social_apps'] = list(
SocialApp.objects.filter(custom_data__isnull=True).values(
'provider', 'name', 'client_id', 'provider_id'
)
data['social_apps'] = social_apps
)

data['asr_mt_features_enabled'] = _check_asr_mt_access_for_user(
request.user
Expand Down