Skip to content

Fix GLPI 11 compatibility with current Metabase API versions - #150

Open
whamulti wants to merge 1 commit into
pluginsGLPI:mainfrom
whamulti:fix/glpi11-metabase-compat-and-embed
Open

Fix GLPI 11 compatibility with current Metabase API versions#150
whamulti wants to merge 1 commit into
pluginsGLPI:mainfrom
whamulti:fix/glpi11-metabase-compat-and-embed

Conversation

@whamulti

Copy link
Copy Markdown

Fixes #148

Adds backward-compatible ?? fallbacks for a handful of Metabase API fields that got renamed in current versions (ordered_cardsdashcards, sizeX/sizeYsize_x/size_y, and the new MBQL 5 dataset_query.stages[0].* shape for native questions), plus the root-collection collection_id normalization described in the issue.

Also fixes the more severe regression from 1.4.2: the embedded_token upgrade migration sets is_embedded_token_encrypted=1 without ever actually encrypting the existing plain-text value, so dashboard.class.php's unconditional GLPIKey()->decrypt() call returns an empty string and the JWT signer throws InvalidKeyProvided on every visit to the embedded dashboard tab. Fixed to actually encrypt, mirroring the password migration 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.

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
Comment thread inc/config.class.php
// 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']);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@stonebuzz

Copy link
Copy Markdown
Contributor

Can you adapt CHANGELOG.md ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"Metabase 1.4.2 breaks on GLPI 11: unhandled API renames break extraction, and a broken migration breaks the embedded dashboard entirely"

2 participants