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

Email otp #48

Merged
merged 48 commits into from
Apr 18, 2024
Merged

Email otp #48

merged 48 commits into from
Apr 18, 2024

Conversation

folix-01
Copy link
Member

No description provided.


manager = getUtility(IKeyManager)

return md5((uid + email + manager.secret()).encode()).hexdigest()[:10]
Copy link
Contributor

@mamico mamico Apr 11, 2024

Choose a reason for hiding this comment

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

md5 is not OTP although the secret rotates periodically (can't tell you with what timing/logic), you have to enter a form of expiration. You can directly use https://github.com/pyauth/pyotp?tab=readme-ov-file#time-based-otps using as secret the data you used here, and change the validation from "==" to totp.verify

Otherwise everything is ok for me.

Copy link
Member Author

@folix-01 folix-01 Apr 11, 2024

Choose a reason for hiding this comment

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

Si, infatti stavo pensado che il secret cambia, in caso si può anche aggiungere nel hash il datetime slottato per 15 min for example

Copy link
Member Author

Choose a reason for hiding this comment

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

done

Copy link
Collaborator

@cekk cekk left a comment

Choose a reason for hiding this comment

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

There are also some typos in the README.

Implementation is ok for me

@cekk
Copy link
Collaborator

cekk commented Apr 11, 2024

Also, fix actions please

Copy link
Collaborator

@cekk cekk left a comment

Choose a reason for hiding this comment

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

please double-check what you write, because there are typos

CHANGES.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated
@validate-email-token
---------------------

Supposed to validate the OTP code recieved by user via email.
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo

Copy link
Member Author

Choose a reason for hiding this comment

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

solved

README.rst Outdated Show resolved Hide resolved
@folix-01 folix-01 requested a review from cekk April 12, 2024 08:01
@coveralls
Copy link

coveralls commented Apr 12, 2024

Pull Request Test Coverage Report for Build 8720476249

Details

  • 62 of 106 (58.49%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-4.1%) to 87.314%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/collective/volto/formsupport/restapi/services/submit_form/post.py 22 24 91.67%
src/collective/volto/formsupport/browser/email_confirm_view.py 7 12 58.33%
src/collective/volto/formsupport/restapi/services/validation/email.py 20 57 35.09%
Totals Coverage Status
Change from base Build 8705235686: -4.1%
Covered Lines: 764
Relevant Lines: 875

💛 - Coveralls

@cekk cekk merged commit 2ad7373 into main Apr 18, 2024
6 checks passed
@cekk cekk deleted the email_otp branch April 18, 2024 07:11
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.

None yet

4 participants