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

T1652 - Add 2FA #41

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

T1652 - Add 2FA #41

wants to merge 13 commits into from

Conversation

clementcharmillot
Copy link
Contributor

@clementcharmillot clementcharmillot commented Jul 26, 2024

Main Changes

  • Fix error handling and correctly redirect to the login screen on account error.
  • Use an auth token after authentication kept in local storage in place of the password. Affects both 2FA and non-2FA accounts.
    After authentication, the local storage contains the following:
{
  "username":"admin",
  "userId":1973,
  "accessToken":"...",
  "refreshToken":"...",
  "accessTokenExpiresAt":"2024-07-31T16:09:59Z"
}
  • Add a 2FA field, hidden by default as 2FA is only for internal users, to reduce friction with the current flow
    image
    image

  • When the user tries to login without 2FA on an account with 2FA enabled, the field appears and a corresponding error message is shown:
    image

TODO before merging

  • Store access + refresh token in Secure, HttpOnly, SameSite strict cookie (if possible, prevents token extraction through XSS)

Minor Changes

  • Make the login form responsive
    image

Related PR

CompassionCH/compassion-switzerland#1638

@clementcharmillot clementcharmillot self-assigned this Jul 30, 2024
@clementcharmillot clementcharmillot marked this pull request as ready for review September 3, 2024 13:51
@nlachat-compassion nlachat-compassion self-assigned this Oct 17, 2024
@nlachat-compassion nlachat-compassion marked this pull request as draft October 17, 2024 14:38
@nlachat-compassion nlachat-compassion marked this pull request as ready for review October 29, 2024 14:31
notyf.error(_('Oops! An error occurred. Please contact Compassion for further assistance.'));

}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The method is missing a return statement, in case of error

Suggested change
}
}
return;

@NoeBerdoz NoeBerdoz self-requested a review November 27, 2024 13:09
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.

3 participants