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

Error occurring using beforeLogin for users logging in with AuthData #6871

Open
kvnkuang opened this issue Aug 20, 2020 · 8 comments
Open

Error occurring using beforeLogin for users logging in with AuthData #6871

kvnkuang opened this issue Aug 20, 2020 · 8 comments
Labels
type:bug Impaired feature or lacking behavior that is likely assumed

Comments

@kvnkuang
Copy link
Contributor

Issue Description

When logging in with AuthData, an error (similar to #5998) occurs if that user has one or more files associated.

If no files associated, beforeLogin can be executed without error.

Steps to reproduce

  1. Define a beforeLogin trigger like the following:
Parse.Cloud.beforeLogin(async request => {
  const { object: user }  = request;
  if(user.get('isBanned')) {
   throw new Error('Access denied, you have been banned.')
  }
});
  1. Log in with an Auth provider. No matter if the user has isBanned set to true, the following error occurs.
UnhandledPromiseRejectionWarning: Error: Tried to encode an unsaved file.
    at encode (C:\Users\kevin\Documents\GitHub\maveregistry-parse-server\node_modules\parse\lib\node\encode.js:73:13)
    at _default (C:\Users\kevin\Documents\GitHub\maveregistry-parse-server\node_modules\parse\lib\node\encode.js:126:10)
    at ParseUser.toJSON (C:\Users\kevin\Documents\GitHub\maveregistry-parse-server\node_modules\parse\lib\node\ParseObject.js:604:42)
    at C:\Users\kevin\Documents\GitHub\maveregistry-parse-server\node_modules\parse-server\lib\triggers.js:598:81
    at error (C:\Users\kevin\Documents\GitHub\maveregistry-parse-server\node_modules\parse-server\lib\triggers.js:379:9)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)

Actual Outcome

I spent some time investigating this problem and found that this error was triggered by this block of code:

logTriggerSuccessBeforeHook(
triggerType,
'Parse.File',
{ ...fileObject.file.toJSON(), fileSize: fileObject.fileSize },
result,
auth
);
return result || fileObject;
} catch (error) {
logTriggerErrorBeforeHook(
triggerType,
'Parse.File',
{ ...fileObject.file.toJSON(), fileSize: fileObject.fileSize },
auth,
error
);

Specifically, when encoding a file object with toJSON(), because the object was not expanded, it has no url property and hence cause this block of code to throw an error:

  if (value instanceof ParseFile) {
    if (!value.url()) {
      throw new Error('Tried to encode an unsaved file.');
    }
    return value.toJSON();
  }

Similar to the fix introduced previously (#6001), all we need is to expand file objects before passing them to the trigger runBeforeLoginTrigger in RestWrite.js:

  // Cloud code gets a bit of extra data for its objects
  const extraData = { className: this.className };

  // PROPOSED FIX: expand file objects
  this.config.filesController.expandFilesInObject(this.config, userData)
  
  const user = triggers.inflate(extraData, userData);

  // no need to return a response
  await triggers.maybeRunTrigger(
    triggers.Types.beforeLogin,
    this.auth,
    user,
    null,
    this.config,
    this.context
  );

Environment

Server

  • Parse Server version: 4.3.0
  • Operating system: Windows
  • Local or remote host (AWS, Azure, Google Cloud, Heroku, Digital Ocean, etc): local

Client

  • SDK (iOS, Android, JavaScript, PHP, Unity, etc): JavaScript
  • SDK version: 2.15.0
@mtrezza mtrezza added the type:bug Impaired feature or lacking behavior that is likely assumed label Aug 21, 2020
@mtrezza
Copy link
Member

mtrezza commented Aug 21, 2020

Thanks for reporting and the detailed analysis. You even already submitted a PR, way to go! 🚀

@mtrezza
Copy link
Member

mtrezza commented Aug 25, 2020

I am closing this as it seems to be resolved by #6872. Feel free to comment if you have any questions and we can re-open this issue.

@mtrezza mtrezza closed this as completed Aug 25, 2020
@alann-maulana
Copy link
Contributor

I'm using parse-server: ^5.2.4 along with auth login google. Even without Parse.Cloud.beforeLogin on my cloud code, parse server still throw error

Error: Tried to encode an unsaved file.
    at encode (/home/datok/datok-server/node_modules/parse-server/node_modules/parse/lib/node/encode.js:73:13)
    at _default (/home/datok/datok-server/node_modules/parse-server/node_modules/parse/lib/node/encode.js:126:10)
    at ParseUser.toJSON (/home/datok/datok-server/node_modules/parse-server/node_modules/parse/lib/node/ParseObject.js:605:42)
    at object (/home/datok/datok-server/node_modules/parse-server/lib/triggers.js:546:83)
    at success (/home/datok/datok-server/node_modules/parse-server/lib/triggers.js:339:14)
    at process._tickCallback (internal/process/next_tick.js:68:7)

This only happen on _User data on ParseFile column photo is set. When I unset this column on specific user, they can login using authData without issue. Is there any workaround to fix this?

@mtrezza
Copy link
Member

mtrezza commented Aug 6, 2022

@alann-maulana A failing test would help to proof that this (or a similar) issue still exists; could you open a new issue and submit a PR with a failing test?

@alann-maulana
Copy link
Contributor

@mtrezza Ok, I will set it up. Thanks

@hariprasadiit
Copy link

hariprasadiit commented Sep 23, 2022

this seems to be happening if File is created with existing file url and if that file url results in redirects.

the following file, if used in any parse object, fails with the above error.

new Parse.File('test', {uri: 'URL that redirects to final url'})

If the URI doesn't redirect and it's the final url, then it works

Seems File download logic is not following redirects?

@mtrezza mtrezza reopened this Sep 23, 2022
@parse-github-assistant
Copy link

Thanks for opening this issue!

  • ❌ Please edit your post and use the provided template when creating a new issue. This helps everyone to understand your post better and asks for essential information to quicker review the issue.

@mtrezza
Copy link
Member

mtrezza commented Sep 23, 2022

Could you submit a PR with a failing test?

Please also take a took at #6872 which has a test.ñ for this issue. What is different in the test vs. your scenario?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Impaired feature or lacking behavior that is likely assumed
Projects
None yet
Development

No branches or pull requests

4 participants