FR: Improve flexibility of extension

This is a feature request to improve the flexibility of the Passport extension and is primarily sourced from my experience catering this extension to support Phabricator for the new Solus Flarum-based Forums.

Issues

Currently, there is no support for passing optional params (with any sort of replacers / keywords) for the app_user_url.
ResourceOwner result only handles a response which has an array of key/vals which correspond to the id, email, and name of the user.
Currently, there is no support for specifying what keys from the result should be used for the id, email, and name.

Why these are an issue

For Phabricator, the app_user_url is an endpoint ending with /user.whoami, which requires an access token to interact with an internal Conduit API. This access token is passed as a query param access_token. My change to this extension required changing the getResourceOwnerDetailsUrl function from:

return $this->settings->get('flagrow.passport.app_user_url');

To the following:

return $this->settings->get('flagrow.passport.app_user_url')."?access_token=".((string) $token->getToken());

Obviously this wouldn't be flexible if implemented exactly how I did, however I do propose a solution in the solutions section of this FR. Carrying on, essentially passing the token which we receive as $token->getToken() (to ensure it is a string) allows us to get to the state of being able to get information such as the following:

{
  "result": {
    "phid": "PHID-USER-(REDACTED)",
    "userName": "JoshStroblTest",
    "realName": "Joshua Strobl (Test)",
    "image": "I_AM_A_URL",
    "uri": "I_AM_A_URL",
    "roles": [
      "verified",
      "approved",
      "activated"
    ],
    "primaryEmail": "test@example.com"
  },
  "error_code": null,
  "error_info": null
}

As you can see, rather than a response with the keys (such as phid) being provided directly in the JSON as a key/val, it is nested within a "result" Object. To enable us to get the correct keys, I added a private $result as a variable, and added the following in the constructor:

$this->result = $response["result"];

Obviously this isn't ideal for a universal solution, it would probably make more sense for $this->response to change instead.

Now that we were getting the right results, I had to ensure the keys were being adjusted in ResourceOwner's getValueByKey function, since we get:

phid instead of id
primaryEmail instead of email
userName instead of name

Additionally I needed to change our fetching of those keys to using $this->result instead.

Potential Solutions

For the current issue of there not being any support for optional params, I propose the addition of a setting, maybe flagrow.passport.app_user_url_params which gets parsed in getResourceOwnerDetailsUrl (or elsewhere). This would support a limited amount of keywords that would get replaced in the function, such as:

token which would be replaced with the value presented in $token->getToken()
resourceOwnerId which would return the value presented in $token->getResourceOwnerId()

As an example, the params could be set as access_token=:token (or whatever syntax sugar you'd want to add for the keywords).

For the current issue of not handling nested keys, there's two solutions that immediately come to my mind:

Handling strictly the case of results being nested
Iterating over the keys, breaking when you find either id or the corresponding id-equivelant we're looking for and if it's an Array / Object, iterate over its items, continuing down the tree.

For the current issue of solely checking for id, email, and name, we could default to those keys and provide further settings options for setting what we should look for / use as id, email, and name.

Hope this was found to be useful.

FriendsOfFlarum / passport

FR: Improve flexibility of extension #4