Skip to content
This repository has been archived by the owner on Jun 24, 2024. It is now read-only.

Feature YG-1071 enumeration-attack-proof email signup #857

Merged
merged 13 commits into from
Dec 20, 2023

Conversation

maltherd
Copy link
Collaborator

@maltherd maltherd commented Aug 2, 2023

  • Update django-axes
  • Adapt django-axes settings in order to avoid unwanted locked users
  • ⚠️ Adapt .env config

@maltherd maltherd force-pushed the feature/yg-1071-enumeration-attack-proof-email-signup branch from 539f727 to 870c74d Compare August 2, 2023 11:34
@maltherd maltherd marked this pull request as draft August 2, 2023 11:42
@maltherd maltherd marked this pull request as ready for review August 4, 2023 16:18
@monodo
Copy link
Collaborator

monodo commented Aug 16, 2023

@maltherd the easy login buttons do no longer work locally, could yo take a look ? I think you need to adapt the element name in the js login function that is called upon click on the role login buttons:

image

@monodo
Copy link
Collaborator

monodo commented Aug 16, 2023

@maltherd @rbovard should we not display (readonly ?) the username (if exists) and the email in the /profile/edit/ view ?
image

@monodo
Copy link
Collaborator

monodo commented Aug 16, 2023

@maltherd rename the placeholder to "Email ou Identifiant" in order to avoid confusion by user who might try to write [email protected]/johny ?
image

@rbovard
Copy link
Collaborator

rbovard commented Aug 16, 2023

@maltherd @rbovard should we not display (readonly ?) the username (if exists) and the email in the /profile/edit/ view ?

Of course, even editable like now, no? Because how can I update my email if I want to?

@rbovard
Copy link
Collaborator

rbovard commented Aug 16, 2023

@maltherd rename the placeholder to "Email ou Identifiant" in order to avoid confusion by user who might try to write [email protected]/johny ?

+1 I would suggest "Email ou identifiant" (with i)

@monodo
Copy link
Collaborator

monodo commented Aug 16, 2023

@maltherd @rbovard should we not display (readonly ?) the username (if exists) and the email in the /profile/edit/ view ?

Of course, even editable like now, no? Because how can I update my email if I want to?

Only email, right ? Don't display username if None and read username readonly, as we don't want to use it in the future, right ?

@rbovard
Copy link
Collaborator

rbovard commented Aug 16, 2023

Only email, right ? Don't display username if None and read username readonly, as we don't want to use it in the future, right ?

+1

And which value is displayed on the top right menu?

@maltherd
Copy link
Collaborator Author

@maltherd the easy login buttons do no longer work locally, could yo take a look ? I think you need to adapt the element name in the js login function that is called upon click on the role login buttons:

image

The js login function is provided in the APPLICATION_DESCRIPTION constance setting which is provided by the fixturize management command. The issue is due to the field changing name (id_usernameid_email_or_username).

The fixturize code was patched (see here) but for existing installations, the only choice (I think) is to rerun fixturize or to change the constance setting. There aren't constance migrations, are there ?

@maltherd
Copy link
Collaborator Author

Only email, right ? Don't display username if None and read username readonly, as we don't want to use it in the future, right ?

+1

And which value is displayed on the top right menu?

In this PR's code, the full name (first name + last name) is currently displayed in the top right (see here)

maltherd pushed a commit that referenced this pull request Aug 21, 2023
* allow to change own email
* show readonly username in profile view if it's not autogenerated
* change email/username placeholder in login views
maltherd pushed a commit that referenced this pull request Aug 21, 2023
@maltherd
Copy link
Collaborator Author

Solved comments have been reacted to with 👍

maltherd pushed a commit that referenced this pull request Aug 21, 2023
@maltherd maltherd force-pushed the feature/yg-1071-enumeration-attack-proof-email-signup branch from 759bd7e to 67ff4c2 Compare August 21, 2023 12:26
maltherd pushed a commit that referenced this pull request Aug 21, 2023
@maltherd maltherd force-pushed the feature/yg-1071-enumeration-attack-proof-email-signup branch from 67ff4c2 to 670f3bf Compare August 21, 2023 13:56
maltherd pushed a commit that referenced this pull request Aug 28, 2023
* allow to change own email
* show readonly username in profile view if it's not autogenerated
* change email/username placeholder in login views
maltherd pushed a commit that referenced this pull request Aug 28, 2023
@maltherd maltherd force-pushed the feature/yg-1071-enumeration-attack-proof-email-signup branch from 670f3bf to 508c9b9 Compare August 28, 2023 11:54
Arnaud Gaudard added 6 commits August 28, 2023 17:42
* allow to change own email
* show readonly username in profile view if it's not autogenerated
* change email/username placeholder in login views
@maltherd maltherd force-pushed the feature/yg-1071-enumeration-attack-proof-email-signup branch from 508c9b9 to 1eacdc4 Compare August 28, 2023 15:42
@monodo
Copy link
Collaborator

monodo commented Sep 4, 2023

@maltherd it looks like with the new login system, tested on preprod instance, Django-Axes lockout configuration AXES_LOCK_OUT_BY_COMBINATION_USER_AND_IP (https://django-axes.readthedocs.io/en/latest/4_configuration.html ) does not manage to discriminate between users and locks any account coming from the same IP address after 3 failing attemps.
Would you adapt the login system or configure Django-Axes differently ?

@monodo monodo merged commit c0ac8f7 into develop Dec 20, 2023
2 checks passed
@monodo monodo deleted the feature/yg-1071-enumeration-attack-proof-email-signup branch December 20, 2023 13:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants