Problem/Motivation
When doing local development on a site that has SF sync, but is not authorized, we get lots of unexpected crashes. Our site is using a plugin to integrate webform with the mapping API, and you can't edit webforms when your JWT connection isn't authorized. This might seem obscure, but it's caused by RestClient::isInit() crashing inappropriately. If we aren't authorized, we should be able to check "isInit()", get "FALSE", and behave appropriately. Instead, we call isInit(), which tries to refresh the token, and get an unhandled exception from `firebase/php-jwt/src/JWT.php`.
Steps to reproduce
Create an unauthorized Auth config (try importing this one, after creating the key listed)
langcode: en
status: true
dependencies:
config:
- key.key.salesforce_oauth_jwt
module:
- salesforce
- salesforce_jwt
id: salesforce_oauth_jwt
label: 'Salesforce OAuth JWT'
provider: jwt
provider_settings:
consumer_key: null
login_user: null
login_url: 'https://login.salesforce.com'
encrypt_key: salesforce_oauth_jwtTry to create a mapping, and you'll get this error.
Proposed resolution
I'm not sure the best way to test. It seems like JWT explodes before you can catch the exception, but I could be wrong. Perhaps validate that our JWT is fully configured before trying to refresh the token? I tried wrapping a try/catch around the call to refreshToken but it never bubbled up that far.
Remaining tasks
User interface changes
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | 3224747-3.patch | 2.7 KB | dspachos |
Comments
Comment #2
gcbComment #3
dspachos commentedI can reproduce the issue on 5.x. - attached a really quick patch
Comment #4
aaronbaumanI'm doing some cleanup on unsupported branches.
Please re-open this issue and update the version to 5.0.x-dev if this issue is still applicable to the latest release.
Comment #5
gcbConfirmed this is still a bug. Here's a stack trace of where the exception occurs when we make a call to
\Drupal\salesforce\Rest\RestClient::objectDescribe()There's a call there:
It's throwing an exception! And no one catches it. Walking it back up:
GuzzleHttp\Client->request()states "@throws GuzzleException"That bring us into salesforce module code:
Drupal\salesforce\Client\HttpClientWrapper->retrieveResponse()I think the trouble begins here, as this states:
* @throws TokenResponseException-- I don't think that includes a GuzzleException, so we probably need another "throws" here. Note that this comment is via @inheritdoc. Anyhow, the code doesn't acknowledge/handle the exception otherwise.SalesforceJWTPlugin->requestAccessToken()also says it throws the token exception while ignoring it.After that, inherited comments start to acknowledge a MissingRefreshTokenException, but again, not handling them.
Based on the names and purposes of those functions, I do think it's reasonable for them to just throw exceptions -- they can't do the thing they want to do, which is to get a token!
This takes us back to
->isInit(), which presumes to return a boolean telling you whether or not the RestClient is initialized. In this case, I think we can confidently say "it is not" and we know that because an attempt to refresh the access token causes an Exception to be thrown. We should catch this exception, log it, and return "FALSE".TLDR: This is still an issue and @dspachos patch looks good to me. I'm not entirely sure why the change to `SalesforceJWTPlugin` is needed, but it seems like a clean and happy piece of code.
Comment #7
aaronbaumanVery helpful, thank you for unpacking all that, and thanks for the patch.
Committed to latest dev