Username leakage from password recovery when no email service setup or no useremail

This is a follow on issue#1771
We need to have allways same response when user exists/notExists

PR fix issue 1770: #1773 resolves the case when email service is setup
when email service is not setup the server should return Email service is not set up also when username do not exist
when email service is setup, and email do not exist the server returns Account recovery email has not been provided
these are/should be rare cases when email is setup and only on really experimental server.
When username do not exist I propose to return the actual no error

Adding (in username if (!exists)) a call to verifyEmailDependencies ()

node-solid-server/lib/models/account-manager.js

Lines 535 to 543 in 88d3a86

    
           verifyEmailDependencies (userAccount) { 
        
             if (!this.emailService) { 
        
               throw new Error('Email service is not set up') 
        
             } 
        
             if (!userAccount.email) { 
        
               throw new Error('Account recovery email has not been provided') 
        
             } 
        
           }

Also replacing throw error with success

node-solid-server/lib/models/account-manager.js

Lines 540 to 542 in 88d3a86

    
           if (!userAccount.email) { 
        
             throw new Error('Account recovery email has not been provided') 
        
           }

with

if (userAccount && !userAccount.email) {
  return resetLinkMessage ()

fix branch created fix/issue#1774

Also replacing throw error with success

node-solid-server/lib/models/account-manager.js

Lines 540 to 542 in 88d3a86

if (!userAccount.email) {

throw new Error('Account recovery email has not been provided')

}

with
if (userAccount && !userAccount.email) {
  return resetLinkMessage ()
fix branch created fix/issue#1774

The only ways to get the resetLinkMessage() call into the AccountManager class is to either make it static from the PasswordResetEmailRequest class, copy it into the AccountManager class, or to pass the instance of PasswordResetEmailRequest class into the AccountManager class. I currently have where if it throws the Error in verifyEmailDependencies with specific message, do renderSuccess() in PasswordResetEmailRequest.loadUser if !exists

Thank you for looking at this

if (userAccount && !userAccount.email) {
 return resetLinkMessage ()

This was an error
You should keep the throw Error, with the check on userAccount the function will not return an Error when the username do not exist
So we should have

 if (userAccount && !userAccount.email) { 
   throw new Error('Account recovery email has not been provided')
 }

And you will have simply something like

  if(!exist) {
   this.accountManager.verifyEmailDependencies() // without userAccount, this will only throw Error on `email service not setup`
   return this.resetLinkMessage() // this is the expected email sent message
  }

Well there is alsoa logic issue with the 2 PRs you will have a conflict with the last merged one.

So you have the choice to keep only one PR for both issues.

Or a bit more complex

to merge in main on github PR Username leakage from password reset mechanism #1770
to locally on main branch make a git pull
change branch to fix/issue#1774
make the changes commit and push to github

I took the changes I made from 1770 and integrated them with this fix/issue#1774 request, so fix/issue#1770 pull request can be closed without merge as all changes are incorporated into the PR which fixes 1774, as it fixes both 1770 and 1774.

Navigation Menu

Repository navigation

Username leakage from password recovery when no email service setup or no useremail #1774

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	verifyEmailDependencies (userAccount) {
	if (!this.emailService) {
	throw new Error('Email service is not set up')
	}

	if (!userAccount.email) {
	throw new Error('Account recovery email has not been provided')
	}
	}

Username leakage from password recovery when no email service setup or no useremail #1774

Description

Activity

zg009 commented on Mar 16, 2024

bourgeoa commented on Mar 16, 2024

bourgeoa commented on Mar 16, 2024

zg009 commented on Mar 16, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions