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

Improve login screen and process #3695

Closed
3 tasks done
joeribekker opened this issue Dec 15, 2023 · 1 comment · Fixed by #3844 or #3936
Closed
3 tasks done

Improve login screen and process #3695

joeribekker opened this issue Dec 15, 2023 · 1 comment · Fixed by #3844 or #3936

Comments

@joeribekker
Copy link
Contributor

joeribekker commented Dec 15, 2023

Thema / Theme

Admin

Omschrijving / Description

The login process is suboptimal. 2FA is globally set but when using OIDC this should be decided by the OIDC-provider. If OIDC is used, you can still use local accounts. This should be discouraged visually.

Tasks

  • Update the two-factor library to use upstream version again
  • Differentiate between OIDC-login and regular logins whether to use 2FA or not (thus, make sure this is not some global setting but based on the session that holds the login-backend information). This probably require library code changes or some custom code in Open Formulieren.
  • Make the Django LOGIN_URL an environment variable, and document it. This allows us to set it to /oidc/authenticate/ to automatically go to OIDC provider instead of showing the regular login form.

2FA upgrade is also required for #3049

Added value / Toegevoegde waarde

No response

Aanvullende opmerkingen / Additional context

No response

@joeribekker joeribekker added triage Issue needs to be validated. Remove this label if the issue considered valid. enhancement and removed triage Issue needs to be validated. Remove this label if the issue considered valid. labels Dec 15, 2023
@joeribekker joeribekker added this to the Release 2.6.0 milestone Dec 18, 2023
@joeribekker joeribekker assigned SilviaAmAm and Viicos and unassigned SilviaAmAm Jan 16, 2024
sergei-maertens added a commit that referenced this issue Feb 2, 2024
Also addresses part of #3695

* Drop the maykin-django-two-factor-auth dependency - it is replaced
  with the upstream package through maykin-2fa
* Drop phonenumbers, instead phonenumberslite is enforced by maykin-2fa
sergei-maertens added a commit that referenced this issue Feb 2, 2024
Also addresses part of #3695

* Drop the maykin-django-two-factor-auth dependency - it is replaced
  with the upstream package through maykin-2fa
* Drop phonenumbers, instead phonenumberslite is enforced by maykin-2fa
sergei-maertens added a commit that referenced this issue Feb 3, 2024
Also addresses part of #3695

* Drop the maykin-django-two-factor-auth dependency - it is replaced
  with the upstream package through maykin-2fa
* Drop phonenumbers, instead phonenumberslite is enforced by maykin-2fa
sergei-maertens added a commit that referenced this issue Feb 5, 2024
Also addresses part of #3695

* Drop the maykin-django-two-factor-auth dependency - it is replaced
  with the upstream package through maykin-2fa
* Drop phonenumbers, instead phonenumberslite is enforced by maykin-2fa
sergei-maertens added a commit that referenced this issue Feb 7, 2024
Also addresses part of #3695

* Drop the maykin-django-two-factor-auth dependency - it is replaced
  with the upstream package through maykin-2fa
* Drop phonenumbers, instead phonenumberslite is enforced by maykin-2fa
@sergei-maertens
Copy link
Member

Need to handle the auto-redirect/LOGIN_URL envvar aspect

sergei-maertens added a commit that referenced this issue Feb 26, 2024
Hooking up a different url under the admin:login name was not possible,
while you can replace/override that namespace, it turns out it breaks
the admin:index reverse URL.

Instead, overriding the actual URL with a redirect view that sends you
to either the OIDC provider (while retaining the ?next param) or the
classic username/password authentication page works better. It also
makes it easier to test the behaviour by just toggling a setting rather
than conditionally including/overriding some URLs.
sergei-maertens added a commit that referenced this issue Feb 27, 2024
Hooking up a different url under the admin:login name was not possible,
while you can replace/override that namespace, it turns out it breaks
the admin:index reverse URL.

Instead, overriding the actual URL with a redirect view that sends you
to either the OIDC provider (while retaining the ?next param) or the
classic username/password authentication page works better. It also
makes it easier to test the behaviour by just toggling a setting rather
than conditionally including/overriding some URLs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment