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

Incorrect documentation for AuthAdapter methods #9495

Open
rietbergenm opened this issue Dec 17, 2024 · 1 comment
Open

Incorrect documentation for AuthAdapter methods #9495

rietbergenm opened this issue Dec 17, 2024 · 1 comment
Labels
type:docs Only change in the docs or README

Comments

@rietbergenm
Copy link

New Issue Checklist

This has briefly been mentioned in #8785, but no action was taken. I could find no other references (issues, pull requests) of this problem.

Issue Description

The documentation regarding the authAdapter interface is inconsistent with the implementation: some functions you need to implement when writing your own authAdapter are incorrectly declared in src/Adapters/Auth/AuthAdapter.js:

  /**
   * Legacy usage, if provided it will be triggered when authData related to this provider is touched (signup/update/login)
   * otherwise you should implement validateSetup, validateLogin and validateUpdate
   * @param {Object} authData The client provided authData
   * @param {Parse.Cloud.TriggerRequest} request
   * @param {Object} options additional adapter options
   * @returns {Promise<ParseAuthResponse|void|undefined>}
   */
  validateAuthData(authData, request, options) {
    return Promise.resolve({});
  }

  /**
   * Triggered when user provide for the first time this auth provider
   * could be a register or the user adding a new auth service
   * @param {Object} authData The client provided authData
   * @param {Parse.Cloud.TriggerRequest} request
   * @param {Object} options additional adapter options
   * @returns {Promise<ParseAuthResponse|void|undefined>}
   */
  validateSetUp(authData, req, options) {
    return Promise.resolve({});
  }

  /**
   * Triggered when user provide authData related to this provider
   * The user is not logged in and has already set this provider before
   * @param {Object} authData The client provided authData
   * @param {Parse.Cloud.TriggerRequest} request
   * @param {Object} options additional adapter options
   * @returns {Promise<ParseAuthResponse|void|undefined>}
   */
  validateLogin(authData, req, options) {
    return Promise.resolve({});
  }

  /**
   * Triggered when user provide authData related to this provider
   * the user is logged in and has already set this provider before
   * @param {Object} authData The client provided authData
   * @param {Object} options additional adapter options
   * @param {Parse.Cloud.TriggerRequest} request
   * @returns {Promise<ParseAuthResponse|void|undefined>}
   */
  validateUpdate(authData, req, options) {
    return Promise.resolve({});
  }

The problem is that the functions declared here don't take their parameters in the right order. In src/Adapters/Auth/index.js we see how these functions are called:

    if (hasAuthDataConfigured) {
        return {
          method: 'validateUpdate',
          validator: () => adapter.validateUpdate(authData, options, requestObject),
        };
      }
      // Set up if the user does not have the provider configured
      return {
        method: 'validateSetUp',
        validator: () => adapter.validateSetUp(authData, options, requestObject),
      };
    }

    // Not logged in and authData is configured on the user
    if (hasAuthDataConfigured) {
      return {
        method: 'validateLogin',
        validator: () => adapter.validateLogin(authData, options, requestObject),
      };
    }

    // User not logged in and the provider is not set up, for example when a new user
    // signs up or an existing user uses a new auth provider
    return {
      method: 'validateSetUp',
      validator: () => adapter.validateSetUp(authData, options, requestObject),

I assume the right way is to specify options first, then requestObject, as that is what existing authAdapters implement.

validateAppId(appIds, authData, options, request) and challenge(challengeData, authData, options, request) are also defined in this way

The result of this is that the documentation on the server api is citing the "incorrect" funcition signatures for these for functions. When trying to work with the documentation, I was very confused by this.

This issue affects the following functions:

  • validateUpdate
  • validateLogin
  • validateSetUp
  • validateAuthData

These methods are not affected:

  • challenge
  • validateAppId
  • afterFind
  • validateOptions

A fix would be trivial, just swap the parameters and update the comments. I would be happy to open a pull request for this.

Steps to reproduce

  1. Read the documentation.
  2. Implement an AuthAdapter using options or requestObjects.
  3. Your code does not work 😮.

Actual Outcome

The documentation (jsdoc comments) does not match the code it is documenting.

Expected Outcome

The documentation/comments should match the code.

Environment

Encountered on a running version of parse-server 6.1.0. The code that introduced this error was contributed in commit 5bbf9cade9a527787fd1002072d4013ab5d8db2b. This was merged in pull request #8156. It is clearly visible in git though, that from this commit on, the code has not been touched and thus all versions of parse-server from 6.0.0-alpha.2 are most likely affected. The most recent documentation (7.3.0) still has this flaw.

Server

  • Parse Server version: >=6.0.0-alpha.2
  • Operating system: Linux
  • Local or remote host (AWS, Azure, Google Cloud, Heroku, Digital Ocean, etc): local

Database

  • System (MongoDB or Postgres): MongoDB (shouldn't matter)
  • Database version: 7.0.14
  • Local or remote host (MongoDB Atlas, mLab, AWS, Azure, Google Cloud, etc): local

Client

  • SDK (iOS, Android, JavaScript, PHP, Unity, etc): all clients
  • SDK version: any version
Copy link

Thanks for opening this issue!

  • 🚀 You can help us to fix this issue faster by opening a pull request with a failing test. See our Contribution Guide for how to make a pull request, or read our New Contributor's Guide if this is your first time contributing.

@mtrezza mtrezza added the type:docs Only change in the docs or README label Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:docs Only change in the docs or README
Projects
None yet
Development

No branches or pull requests

2 participants