Skip to content

Connexion : Ne plus rendre obligatoire ProConnect [GEN-2246]#5664

Merged
tonial merged 5 commits intomasterfrom
alaurent/allow_proconnect_bypass
Feb 25, 2025
Merged

Connexion : Ne plus rendre obligatoire ProConnect [GEN-2246]#5664
tonial merged 5 commits intomasterfrom
alaurent/allow_proconnect_bypass

Conversation

@tonial
Copy link
Copy Markdown
Contributor

@tonial tonial commented Feb 21, 2025

🤔 Pourquoi ?

Certains utilisateurs sont bloqués depuis 2 mois et le support ProConnect n'arrive pas à les aider.

On va donc permettre de se connecter (pas de créer un compte pour le moment) avec les crédentials Django.

🍰 Comment ?

Décrivez en quelques mots la solution retenue et mise en oeuvre, les difficultés ou problèmes rencontrés. Attirez l'attention sur les décisions d'architecture ou de conception importantes.

🚨 À vérifier

  • Mettre à jour le CHANGELOG_breaking_changes.md ?
  • Ajouter l'étiquette « Bug » ?

🏝️ Comment tester ?

Les instructions pour reproduire le problème, les profils de test, le parcours spécifique à utiliser, etc. Si vous disposez d'une recette jetable, mettre l'URL pour tester dans cette partie.

💻 Captures d'écran

@tonial tonial added the modifié Modifié dans le changelog. label Feb 21, 2025
@tonial tonial self-assigned this Feb 21, 2025
@notion-workspace
Copy link
Copy Markdown

Copy link
Copy Markdown
Member

@francoisfreitag francoisfreitag left a comment

Choose a reason for hiding this comment

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

🧹 Pas mal de nettoyage, ça simplifie !

Comment thread itou/templates/account/activate_pro_connect_account.html
Comment thread itou/templates/account/activate_pro_connect_account.html
Comment on lines 52 to 53
if not request.user.is_authenticated:
return False
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Je ne suis pas convaincu de la pertinence de ce check? (pour une autre PR)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

en effet, ça ne devrait jamais arriver

Comment thread itou/www/invitations_views/helpers.py Outdated
Comment thread itou/www/login/forms.py Outdated

def login(self, request, redirect_url=None):
ret = super().login(request=request, redirect_url=redirect_url)
accept_all_pending_invitations(request)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ce n’est pas de ton fait, mais je trouve étrange qu’on accepte toutes les invitations en attente à la connexion. Un utilisateur n’a peut-être pas envie d’être rattaché à une structure ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Uhuh c'est vrai qu'avec l'abandon partiel de ProConnect, https://gip-inclusion.slack.com/archives/C01AQKD7MAN/p1731421358591279 revient en force 🙈

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

C'est un bon point.
On peut sans doute ne pas accepter automatiquement en se connectant avec Django tout en le laissant pour ProConnect pour le moment.
Et même le retirer pour ProConnect tout en mettant un bandeau warning 'vous avez des invitations non acceptées" avec eventuellement un lien pour renvoyer l'email ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oui. Et proposer d’accepter l’invitation directement depuis le site ? (pas besoin d’envoyer un email)

Copy link
Copy Markdown
Contributor Author

@tonial tonial Feb 21, 2025

Choose a reason for hiding this comment

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

Le soucis ce sont les pro sans entreprise. il faudrait les rediriger de force sur une page pour accepter les invits s'ils en ont une (là ou pour le moment on les déconnecte de force)

Comment thread tests/www/login/tests.py Outdated
Comment thread tests/www/login/tests.py Outdated
@francoisfreitag
Copy link
Copy Markdown
Member

En se connectant en tant qu’admin au site:

💥
Environment:

Request Method: POST
Request URL: http://localhost:8000/login/existing/63b13e12-5b59-4bec-8ac6-cfeffce746e5?back_url=/login/job_seeker

Django Version: 5.1.6
Python Version: 3.13.2
Installed Applications:
['django.contrib.admin',
'django.contrib.auth',
'django.contrib.contenttypes',
'django.contrib.sessions',
'django.contrib.messages',
'django.contrib.staticfiles',
'django.contrib.sites',
'django.contrib.gis',
'django.contrib.postgres',
'django.forms',
'anymail',
'citext',
'django_bootstrap5',
'django_select2',
'formtools',
'huey.contrib.djhuey',
'markdownify',
'rest_framework',
'rest_framework.authtoken',
'drf_spectacular',
'django_filters',
'django_htmx',
'hijack',
'hijack.contrib.admin',
'pgtrigger',
'itou.allauth_adapters',
'allauth',
'allauth.account',
'itou.utils',
'itou.cities',
'itou.companies',
'itou.emails',
'itou.jobs',
'itou.users',
'itou.prescribers',
'itou.institutions',
'itou.files',
'itou.job_applications',
'itou.approvals',
'itou.eligibility',
'itou.openid_connect.france_connect',
'itou.openid_connect.pe_connect',
'itou.openid_connect.pro_connect',
'itou.invitations',
'itou.external_data',
'itou.metabase',
'itou.asp',
'itou.employee_record',
'itou.siae_evaluations',
'itou.geiq',
'itou.geo',
'itou.www.apply',
'itou.www.approvals_views',
'itou.www.autocomplete',
'itou.www.dashboard',
'itou.www.eligibility_views',
'itou.www.employees_views',
'itou.www.home',
'itou.www.prescribers_views',
'itou.www.search',
'itou.www.companies_views',
'itou.www.signup',
'itou.www.invitations_views',
'itou.www.stats',
'itou.www.welcoming_tour',
'itou.www.employee_record_views',
'itou.www.siae_evaluations_views',
'itou.api',
'itou.status',
'itou.antivirus',
'itou.scripts',
'itou.analytics',
'itou.communications',
'itou.gps',
'itou.rdv_insertion',
'django_extensions',
'debug_toolbar']
Installed Middleware:
['django_datadog_logger.middleware.request_id.RequestIdMiddleware',
'itou.www.middleware.public_health_check',
'django.middleware.gzip.GZipMiddleware',
'django.middleware.security.SecurityMiddleware',
'django.contrib.sessions.middleware.SessionMiddleware',
'django.middleware.common.CommonMiddleware',
'django.middleware.csrf.CsrfViewMiddleware',
'django.contrib.auth.middleware.AuthenticationMiddleware',
'django.contrib.auth.middleware.LoginRequiredMiddleware',
'django.contrib.messages.middleware.MessageMiddleware',
'django.middleware.clickjacking.XFrameOptionsMiddleware',
'allauth.account.middleware.AccountMiddleware',
'csp.middleware.CSPMiddleware',
'django_htmx.middleware.HtmxMiddleware',
'hijack.middleware.HijackUserMiddleware',
'itou.utils.perms.middleware.ItouCurrentOrganizationMiddleware',
'itou.www.middleware.never_cache',
'itou.openid_connect.pro_connect.middleware.ProConnectLoginMiddleware',
'django_datadog_logger.middleware.request_log.RequestLoggingMiddleware',
'debug_toolbar.middleware.DebugToolbarMiddleware']

Traceback (most recent call last):
File "/home/freitafr/dev/itou-review/.venv/lib/python3.13/site-packages/django/core/handlers/exception.py", line 55, in inner
response = get_response(request)
^^^^^^^^^^^^^^^^^^^^^
File "/home/freitafr/dev/itou-review/.venv/lib/python3.13/site-packages/django/core/handlers/base.py", line 197, in _get_response
response = wrapped_callback(request, *callback_args, **callback_kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/freitafr/dev/itou-review/.venv/lib/python3.13/site-packages/sentry_sdk/integrations/django/views.py", line 94, in sentry_wrapped_callback
return callback(request, *args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.13/contextlib.py", line 85, in inner
return func(*args, **kwds)
^^^^^^^^^^^^^^^^^^^
File "/home/freitafr/dev/itou-review/.venv/lib/python3.13/site-packages/django/views/generic/base.py", line 104, in view
return self.dispatch(request, *args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/freitafr/dev/itou-review/itou/www/login/views.py", line 71, in dispatch
return super().dispatch(request, *args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/freitafr/dev/itou-review/.venv/lib/python3.13/site-packages/django/utils/decorators.py", line 48, in _wrapper
return bound_method(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/freitafr/dev/itou-review/.venv/lib/python3.13/site-packages/allauth/decorators.py", line 12, in wrap
resp = function(request, *args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/freitafr/dev/itou-review/.venv/lib/python3.13/site-packages/django/utils/decorators.py", line 48, in _wrapper
return bound_method(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/freitafr/dev/itou-review/.venv/lib/python3.13/site-packages/django/utils/decorators.py", line 48, in _wrapper
return bound_method(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/freitafr/dev/itou-review/.venv/lib/python3.13/site-packages/django/views/decorators/debug.py", line 143, in sensitive_post_parameters_wrapper
return view(request, *args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/freitafr/dev/itou-review/.venv/lib/python3.13/site-packages/django/utils/decorators.py", line 48, in _wrapper
return bound_method(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/freitafr/dev/itou-review/.venv/lib/python3.13/site-packages/django/views/decorators/cache.py", line 80, in _view_wrapper
response = view_func(request, *args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/freitafr/dev/itou-review/.venv/lib/python3.13/site-packages/allauth/account/views.py", line 93, in dispatch
return super().dispatch(request, *args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/freitafr/dev/itou-review/.venv/lib/python3.13/site-packages/django/utils/decorators.py", line 48, in _wrapper
return bound_method(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/freitafr/dev/itou-review/.venv/lib/python3.13/site-packages/django/utils/decorators.py", line 48, in _wrapper
return bound_method(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/freitafr/dev/itou-review/.venv/lib/python3.13/site-packages/django/views/decorators/cache.py", line 80, in _view_wrapper
response = view_func(request, *args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/freitafr/dev/itou-review/.venv/lib/python3.13/site-packages/allauth/account/mixins.py", line 53, in dispatch
response = super().dispatch(request, *args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/freitafr/dev/itou-review/.venv/lib/python3.13/site-packages/django/views/generic/base.py", line 143, in dispatch
return handler(request, *args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/freitafr/dev/itou-review/.venv/lib/python3.13/site-packages/allauth/account/mixins.py", line 82, in post
response = self.form_valid(form)
^^^^^^^^^^^^^^^^^^^^^
File "/home/freitafr/dev/itou-review/.venv/lib/python3.13/site-packages/allauth/account/views.py", line 106, in form_valid
return form.login(self.request, redirect_url=redirect_url)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/freitafr/dev/itou-review/itou/www/login/forms.py", line 100, in login
accept_all_pending_invitations(request)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/freitafr/dev/itou-review/itou/www/invitations_views/helpers.py", line 64, in accept_all_pending_invitations
InvitationModel, handle_invitation = MAPPING[request.user.kind]
^^^^^^^^^^^^^^^^^^^^^^^^^^

Exception Type: KeyError at /login/existing/63b13e12-5b59-4bec-8ac6-cfeffce746e5
Exception Value: 'itou_staff'

@francoisfreitag
Copy link
Copy Markdown
Member

Pour rejoindre une entreprise (inscription), on ne propose que pro-connect :
image

SIREN d’exemple: 344456389

@tonial
Copy link
Copy Markdown
Contributor Author

tonial commented Feb 21, 2025

Après discussion avec le métier, on va privilégier la connexion des utilisateurs existants.
Je vais donc remettre à une autre PR la création de compte avec Django, et la refonte des invitations.

@tonial tonial force-pushed the alaurent/allow_proconnect_bypass branch 4 times, most recently from 391eb8b to 0ecc3f5 Compare February 24, 2025 12:30
@tonial tonial force-pushed the alaurent/allow_proconnect_bypass branch from 0ecc3f5 to f63a5d1 Compare February 25, 2025 08:44
Comment thread itou/www/login/forms.py
Comment thread itou/www/login/forms.py
# Allow ProConnect and Inclusion Connect users to bypass ProConnect if FORCE_PROCONNECT_LOGIN is False
and (
settings.FORCE_PROCONNECT_LOGIN
or user.identity_provider not in [IdentityProvider.PRO_CONNECT, IdentityProvider.INCLUSION_CONNECT]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Je me demande si on ne devrait pas changer l’identity provider de IC vers DJANGO plutôt que de forcer à PC?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Le seul soucis c'est qu'on cache de la logique.
Mais on pourrait en effet tous les passer à DJANGO en créant l'EmailAddress correspondant

has_completed_welcoming_tour=True,
**ProConnectPrescriberData.user_info_mapping_dict(user_info),
identity_provider=IdentityProvider.DJANGO,
with_verified_email=True,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Pour un utilisateur inscrit via IC/PC, il n’aura pas d’email vérifié. On entrera donc dans le parcours de vérification d’email d’allauth, avec un message disant qu’on a bien enregistré leur inscription.

Est-ce qu’on veut une migration pour ajouter un email vérifié dans allauth pour les PC et IC et éviter cette étape ?

allauth gérait ça via social login, qui appelle https://github.com/pennersr/django-allauth/blob/c11e1429d90aa12373fb97705e18b1d8c602c417/allauth/account/utils.py#L246-L280.

Copy link
Copy Markdown
Contributor Author

@tonial tonial Feb 25, 2025

Choose a reason for hiding this comment

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

C'était en partie ce que calum avait commencé à faire, et qu'on a droppé car trop chronophage et qu'on souhaite supprimer allauth à terme.

Copy link
Copy Markdown
Member

@francoisfreitag francoisfreitag Feb 25, 2025

Choose a reason for hiding this comment

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

Yep 👍
En l’état, c’est très bizarre que le système demande de confirmer l’inscription au moment de la connexion par Django.

Copy link
Copy Markdown
Contributor Author

@tonial tonial Feb 25, 2025

Choose a reason for hiding this comment

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

Je n'ai pas réussi à ajouter à la volée l'EmailAdress manquante au moment du login.
Le code ne passe pas par l'adapter pour trouver l'EmailAddress : https://github.com/pennersr/django-allauth/blob/main/allauth/account/stages.py#L143-L150

Dans les possibilités :

  • on laisse comme ça, de toute manière c'est normalement une solution de contournement temporaire pour une faible partie des utilisateurs
  • envoyer un email mais ne pas bloquer la connexion (aka le traiter en Optional) (cf le commit que j'ai revert juste après pour que la recette garde le fonctionnement "utilisateur bloqué")
  • monkey patcher has_verified_email() pour dire OK pour les utilisateurs SSO
  • synchroniser les EmailAddress avec un cron toutes les heures/jours
  • enregistrer proprement les EmailAddress au login avec un SSO (et donc virer allauth avant -> trop de boulot)

Je serais d'avis de laisser comme ça, parce que c'est la solution la moins coûteuse, et d'organiser un point avec le métier pour discuter de ce qu'on fait en plus sur le sujet connexion sans ProConnet :

  • gestion de la création de comptes
  • gestion de la validation des emails
  • gestion des invitations

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

J’attends la résolution de https://gip-inclusion.slack.com/archives/CU8ATF54L/p1740484789002009.

Pourquoi est-ce qu’une migration qui ajouterait une EmailAddress(verified=True) pour les utilisateurs de PC/IC ne suffirait pas ?

  • S’ils peuvent se connecter avec PC, a priori pas concernés par ces changements.
  • Les utilisateurs bloqués ne peuvent pas se créer de compte sur IC ni avec Django, donc en injectant les EmailAddress(verified=True) en une fois (migration), on devrait leur permettre de se connecter sans confirmation. Et on gérera les nouveaux comptes lorsqu’on permettra à nouveau l’inscription via les emplois ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

En effet, ça va fonctionner

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

commit ajouté

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

J'ai commencé par nettoyer tous les objets qui pouvaient poser problème:
Tous les EmailAddress sur les comptes IC et PC (que ce soit la bonne adresse ou non)
Tous les EmailAddress qui utilisent une adresse email d'un compte IC ou PC et qui sont donc liés à des comptes non prescripteurs et employeurs vu l'étape précédent. Malheureusement il y en a.
Puis j'ai créé les objets qui vont bien pour les comptes IC et PC

Comment thread tests/www/invitations_views/test_company_accept.py Outdated
@tonial tonial force-pushed the alaurent/allow_proconnect_bypass branch from f63a5d1 to 876106d Compare February 25, 2025 10:32
@tonial tonial added the 1-recette-jetable [Payé à l’heure] Crée une recette jetable sur CC label Feb 25, 2025
@github-actions
Copy link
Copy Markdown

🥁 La recette jetable est prête ! 👉 Je veux tester cette PR !

@tonial tonial force-pushed the alaurent/allow_proconnect_bypass branch 3 times, most recently from c7aceb0 to 209dfa9 Compare February 25, 2025 14:06
Comment thread itou/openid_connect/pro_connect/migrations/0002_mark_email_as_verified.py Outdated
@tonial tonial force-pushed the alaurent/allow_proconnect_bypass branch from 209dfa9 to 633e081 Compare February 25, 2025 14:29
@tonial tonial removed the 1-recette-jetable [Payé à l’heure] Crée une recette jetable sur CC label Feb 25, 2025
# Normally all the IC/PC users
users_qs = User.objects.filter(
identity_provider__in=[IdentityProvider.INCLUSION_CONNECT, IdentityProvider.PRO_CONNECT]
).exclude(Exists(EmailAddress.objects.filter(user=OuterRef("pk"))))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

On vient de les supprimer, le exclude me semble inutile?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Tout à fait, je retire ainsi que le commentaire.

Copy link
Copy Markdown
Member

@francoisfreitag francoisfreitag left a comment

Choose a reason for hiding this comment

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

Autrement ça devrait être bon 🤞

@tonial tonial force-pushed the alaurent/allow_proconnect_bypass branch from 633e081 to 1017c0a Compare February 25, 2025 20:32
@tonial tonial force-pushed the alaurent/allow_proconnect_bypass branch from 1017c0a to acdfd53 Compare February 25, 2025 20:33
@tonial tonial added this pull request to the merge queue Feb 25, 2025
Merged via the queue into master with commit ff364f7 Feb 25, 2025
@tonial tonial deleted the alaurent/allow_proconnect_bypass branch February 25, 2025 20:51
@francoisfreitag francoisfreitag changed the title Connexion: Ne plus rendre obligatoire ProConnect [GEN-2246] Connexion : Ne plus rendre obligatoire ProConnect [GEN-2246] Feb 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

modifié Modifié dans le changelog.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants