fix(login): Add session id verification during login
Codecov Report
:x: Patch coverage is 0% with 60 lines in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 44.77%. Comparing base (004d817) to head (7e00374).
:warning: Report is 1 commits behind head on develop.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| press/api/site_login.py | 0.00% | 60 Missing :warning: |
:x: Your patch check has failed because the patch coverage (0.00%) is below the target coverage (75.00%). You can increase the patch coverage or adjust the target coverage.
Additional details and impacted files
@@ Coverage Diff @@
## develop #3518 +/- ##
===========================================
- Coverage 44.81% 44.77% -0.04%
===========================================
Files 723 723
Lines 49504 49539 +35
===========================================
- Hits 22185 22182 -3
- Misses 27319 27357 +38
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
I have a question (kind of suggestion).
Why we are not using Frappe Cloud as an pure auth layer only ?
Ideally:
- We shouldn’t need to sync and store all site user email addresses on Frappe Cloud.
- Authentication should work independently of whether a user has a Frappe Cloud account or not.
Putting some wire diagram and flows here, which might be simple to implement and maintain
Only cons in this design is - You need to contact site for the info, but that's much more secured way of performing this auth imo.
@Bowrna @BreadGenie @ssiyad wdyt ?
@tanmoysrt looks good. we may need to confirm user before sending the mail though. we don't want to send otp mails to random emails.
@ssiyad yeah we can avoid sending actual mails, based on the info in site.
@ssiyad i have a doubt. during a signup process, any email id was random one to system and we send OTP right. why do you think we will need to restrict sending otp?
There is a difference is mails sent during signup and login right? When signing up we will be sending a mail saying "This is your OTP to activate the site". But during login, we send an email saying "This is your OTP to login to your site", which may not exist.
@tanmoysrt @ssiyad If FC is put as a pure auth layer, then the current flow, where we show the list of product sites to user wouldn't be available. So i keeping the existing flow intact and worked on this PR. To avoid sending random OTP, i verified if that email id has at least one active site associated with it. During login, still i use the "Site User" doctype to redirect to login as site. I have added a function "verify_user_in_site" that runs sql remotely and give response. However i have not put it to use as we are still collecting data at "Site User". Please review this and let me know if i have to improve on any place.