Skip to content

Passwords are being logged in plain text in beforeSave #2680

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

Closed
all-iver opened this issue Sep 9, 2016 · 10 comments · Fixed by #2755
Closed

Passwords are being logged in plain text in beforeSave #2680

all-iver opened this issue Sep 9, 2016 · 10 comments · Fixed by #2755

Comments

@all-iver
Copy link

all-iver commented Sep 9, 2016

Issue Description

Possibly related to #1704.

Passwords are being logged in plain text for me in beforeSave when I create a new user. They show up in my nodejs.log and in parse-dashboard. This is with verbose mode turned off.

Steps to reproduce

In your beforeSave, do something like the following (not sure if this is a necessary step but it's what I'm doing):

    if (!user.existed()) {
        user.set('someOtherKey', 0);
        response.success();
        return;
    } 

Then, create a new user.

Expected Results

No passwords should ever be displayed or stored in plain text.

Actual Outcome

I get this in my logs:

info: beforeSave triggered for _User for user undefined:
  Input: {"email":"[email protected]","username":"[email protected]","password":"my-password","someOtherKey":0}
  Result: {"object":{"email":"[email protected]","username":"[email protected]","password":"my-password","someOtherKey":0}} className=_User, triggerType=beforeSave, user=undefined

Environment Setup

  • Server
    • parse-server version (Be specific! Don't say 'latest'.) : 2.2.18
    • Operating System: Mac OSX or Linux
    • Hardware: Macbook Pro or AWS instance
    • Localhost or remote server? (AWS, Heroku, Azure, Digital Ocean, etc): any
  • Database
    • MongoDB version: any
    • Storage engine: any
    • Hardware: any
    • Localhost or remote server? (AWS, mLab, ObjectRocket, Digital Ocean, etc): any

Logs/Trace

See above.

@all-iver
Copy link
Author

Bump on this - is there a quick or simple workaround? We need to migrate very soon to allow users enough time to upgrade before the cutoff. I can't in good conscience have users' plain text passwords be stored in log files on our instances. But having logs is important too.

@flovilmart
Copy link
Contributor

Can you update to 2.2.21 and let us know if the error still reproduces?

@all-iver
Copy link
Author

Hey, thanks for the response - yes, I just updated to 2.2.21 and can confirm it still happens. The first log message after I try to create an account is a beforeSave that contains the user's plain-text password.

@acinader
Copy link
Contributor

ugh, i'm guessing that this is cause we're logging the params in the hooks. I can cook up a failing unit test.

@flovilmart
Copy link
Contributor

@acinader go ahead!

@acinader
Copy link
Contributor

@flovilmart failing unit test: master...acinader:password-obsfucate-cloud-triggers

the first approach that comes to mind is to change logger.truncateLogMessage to logger.cleanAndTruncate....

And just search and replace on "password":"[^"]*"

thoughts?

@flovilmart
Copy link
Contributor

flovilmart commented Sep 20, 2016

@acinader that seems reasonable, I opened the PR with the failing test so we can keep track of the progress. any log for those should go through the cleaner :) Better perhaps, that should be at the loggerController level no? like implementing logRequest(level, ...args) and logResponse(level, ...args) 

@flovilmart
Copy link
Contributor

@all-iver you can use: parseplatform/parse-server.git#latest if you can't wait till next release. Just pin to that version in your package.json.

@all-iver
Copy link
Author

Great, thanks a lot for the quick fix!

acinader pushed a commit to acinader/parse-server that referenced this issue Sep 21, 2016
Move password masking functionality into LoggerController.

The is a more aggresive approach to masking password string in the logs.

Cleaning the url is still in the PromiseRouter because picking it out of the log string
would be fragile.

This will cause more log messages to be scanned for password strings, and may cause a password
string to be obsfucated that is not neccesarily part of parse internals -- but i think that is
still a good thing....

see: parse-community#2755 & parse-community#2680
flovilmart pushed a commit that referenced this issue Sep 22, 2016
Move password masking functionality into LoggerController.

The is a more aggresive approach to masking password string in the logs.

Cleaning the url is still in the PromiseRouter because picking it out of the log string
would be fragile.

This will cause more log messages to be scanned for password strings, and may cause a password
string to be obsfucated that is not neccesarily part of parse internals -- but i think that is
still a good thing....

see: #2755 & #2680
@ashish4rawat
Copy link

but this is now fixed

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 a pull request may close this issue.

4 participants