Fix GLPI 11 compatibility with current Metabase API versions - #150
Fix GLPI 11 compatibility with current Metabase API versions#150whamulti wants to merge 1 commit into
Conversation
Several API response fields the plugin reads under their old names
were renamed by Metabase, and the newer native-question query format
("MBQL 5") wasn't handled at all:
- ordered_cards -> dashcards on GET /api/dashboard/:id
- sizeX/sizeY -> size_x/size_y on dashboard cards
- dataset_query.native.{query,template-tags} moved to
dataset_query.stages[0].{native,template-tags}
- getCards('root') never matched cards in the root collection, since
the API returns id:"root" for that collection but collection_id:null
on the cards themselves
All fixed with backward-compatible `??` fallbacks so older Metabase
instances keep working.
Also fixes a more severe regression introduced in 1.4.2: embedded_token
was added to secured_configs and is expected to be sodium-encrypted,
but the upgrade migration only flipped the is_embedded_token_encrypted
flag without ever actually encrypting the existing plain-text value.
dashboard.class.php unconditionally decrypts embedded_token before
signing the dashboard JWT, so any site that already had a token
configured before upgrading got an empty signing key and an uncaught
Lcobucci\JWT\Signer\InvalidKeyProvided exception on every visit to the
embedded dashboard tab. The migration now actually encrypts the value,
mirroring what the password migration a few lines above already does.
Fixes pluginsGLPI#148
| // Encrypt embedded_token, previously stored in plain text | ||
| if (!array_key_exists('is_embedded_token_encrypted', $current_config) || !$current_config['is_embedded_token_encrypted']) { | ||
| if (!empty($current_config['embedded_token'])) { | ||
| $current_config['embedded_token'] = (new GLPIKey())->encrypt($current_config['embedded_token']); |
There was a problem hiding this comment.
Rather than securing the token directly here, since this class extends GLPI's Config class, you can simply use the Hooks::SECURED_CONFIGS hook.
For example, add the following to the plugin's setup.php file:
$PLUGIN_HOOKS[Hooks::SECURED_CONFIGS]['metabase'] = [
'embedded_token',
];This allows GLPI to handle the token as a secured configuration value using the standard mechanism.
There was a problem hiding this comment.
embedded_token is already registered in secured_configs (setup.php:96), so Config::setConfigurationValues() (src/Config.php:1513-1514) auto-encrypts it on every write. The added manual encrypt() call double-encrypts; dashboard.class.php's single decrypt() then yields ciphertext, not the token, so the JWT is signed with a wrong secret and Metabase still rejects the embed. Same root cause the password block above avoids by passing plaintext through.
| $current_config['embedded_token'] = (new GLPIKey())->encrypt($current_config['embedded_token']); |
|
Can you adapt CHANGELOG.md ? |
| // Encrypt embedded_token, previously stored in plain text | ||
| if (!array_key_exists('is_embedded_token_encrypted', $current_config) || !$current_config['is_embedded_token_encrypted']) { | ||
| if (!empty($current_config['embedded_token'])) { | ||
| $current_config['embedded_token'] = (new GLPIKey())->encrypt($current_config['embedded_token']); |
There was a problem hiding this comment.
embedded_token is already registered in secured_configs (setup.php:96), so Config::setConfigurationValues() (src/Config.php:1513-1514) auto-encrypts it on every write. The added manual encrypt() call double-encrypts; dashboard.class.php's single decrypt() then yields ciphertext, not the token, so the JWT is signed with a wrong secret and Metabase still rejects the embed. Same root cause the password block above avoids by passing plaintext through.
| $current_config['embedded_token'] = (new GLPIKey())->encrypt($current_config['embedded_token']); |
3 participants
Fixes #148
Adds backward-compatible
??fallbacks for a handful of Metabase API fields that got renamed in current versions (ordered_cards→dashcards,sizeX/sizeY→size_x/size_y, and the new MBQL 5dataset_query.stages[0].*shape for native questions), plus the root-collectioncollection_idnormalization described in the issue.Also fixes the more severe regression from
1.4.2: theembedded_tokenupgrade migration setsis_embedded_token_encrypted=1without ever actually encrypting the existing plain-text value, sodashboard.class.php's unconditionalGLPIKey()->decrypt()call returns an empty string and the JWT signer throwsInvalidKeyProvidedon every visit to the embedded dashboard tab. Fixed to actually encrypt, mirroring thepasswordmigration right above it.All of this has been running in production against a live Metabase v0.63.2 instance for a few days now (both the extraction features and the embedded dashboard), see the issue for full repro steps and context. Happy to adjust anything if you'd prefer a different approach.