Skip to content
Snippets Groups Projects
  • Dennis Schridde's avatar
    592360c9
    Hide attached DWH database details (#47247) · 592360c9
    Dennis Schridde authored
    == Goal ==
    
    Hide attached DWH database details from anyone incl. admins:
    * Do not show them in the UI
    * Do not permit to change them
    * Do not serialize them
    
    The aim is that customers cannot gain access to (parts of) credentials,
    and they cannot break a feature they are paying for by changing
    connection details.
    
    == Implementation ==
    
    The Metabase backend already contains provisions in the implementation
    of `metabase.models.interface/to-json` for `:model/Database` to hide the
    `details` of the database in HTTP responses, if the user lacks write
    permission on the database.  We utilize this by adding an
    `is_attached_dwh` column to the `database` table and rejecting
    `metabase.models.interface/can-write?` when this flag is enabled.  In
    the "admin" UI, we show a replacement text instead of the edit form when
    the flag is set.  (It might be correct to show this whenever `details`
    is absent.  See below for possible follow-up work.)
    
    However, several sections of the frontend code expected the `details`
    field to always be present.  In order to make `details` optional, as the
    backend seems to handle it, we fix the respective code to treat this
    case in the way that appears appropriate in the context.
    
    Database details are already generally excluded from H2 dump snapshots
    (see `metabase.cmd.copy/*copy-h2-database-details*`), thus nothing
    changes there.
    
    == How to test ==
    
    === New behaviour ===
    
    Setting the `is_attached_dwh` field hides the database details:
    
    1. Configure a database as described in https://www.metabase.com/docs/latest/configuring-metabase/config-file#databases.
       - In addition to the fields you would normally set, also set
         `is_attached_dwh: true`.
       - This also works when adding this flag to a database that previously
         did not have this flag set.
    2. Start your Metabase instance.
    3. Verify the database shows up in the "admin" section
       (`/admin/databases`).
    4. Verify that clicking the database to see its details only reveals
       "This database cannot be modified."
    5. Verify that responses from the backend do not include a `details`
       field for this database.
    
    === Original behaviour ===
    
    Behaviour without setting the `is_attached_dwh` field is unchanged:
    
    1. Configure a database as described in https://www.metabase.com/docs/latest/configuring-metabase/config-file#databases.
       - Only set the fields you would normally set.  Do not set
         `is_attached_dwh` (or set it to `false`).
    2. Start your Metabase instance.
    3. Verify the database shows up in the "admin" section
       (`/admin/databases`).
    4. Verify that clicking the database to see its details only reveal the
       regular edit form, showing connection fields like `host`, `user`,
       etc. with the values you configured.
    
    == How this will be rolled out ==
    
    1. Upgrade existing Metabase Cloud instances with data warehouse to a
       Metabase version that supports `is_attached_dwh`.
    2. Set `is_attached_dwh` in the database section of the config file for
       Metabase Cloud instances with a data warehouse.
    
    == Possible follow-up work ==
    
    In https://github.com/metabase/metabase/issues/25715, absent
    `database.details` was identified as a bug.  Since then, `details` was
    made `NOT NULL` in the application database, so this bug can no longer
    occur.  However, today backend responses can be missing the `details`
    field, if the current user lacks write permission to the database
    setting (see above).  Fully re-evaluating the fix to #25715 in this
    context is outside the scope of this PR.
    
    Closes: https://github.com/metabase/harbormaster/issues/5051
    Hide attached DWH database details (#47247)
    Dennis Schridde authored
    == Goal ==
    
    Hide attached DWH database details from anyone incl. admins:
    * Do not show them in the UI
    * Do not permit to change them
    * Do not serialize them
    
    The aim is that customers cannot gain access to (parts of) credentials,
    and they cannot break a feature they are paying for by changing
    connection details.
    
    == Implementation ==
    
    The Metabase backend already contains provisions in the implementation
    of `metabase.models.interface/to-json` for `:model/Database` to hide the
    `details` of the database in HTTP responses, if the user lacks write
    permission on the database.  We utilize this by adding an
    `is_attached_dwh` column to the `database` table and rejecting
    `metabase.models.interface/can-write?` when this flag is enabled.  In
    the "admin" UI, we show a replacement text instead of the edit form when
    the flag is set.  (It might be correct to show this whenever `details`
    is absent.  See below for possible follow-up work.)
    
    However, several sections of the frontend code expected the `details`
    field to always be present.  In order to make `details` optional, as the
    backend seems to handle it, we fix the respective code to treat this
    case in the way that appears appropriate in the context.
    
    Database details are already generally excluded from H2 dump snapshots
    (see `metabase.cmd.copy/*copy-h2-database-details*`), thus nothing
    changes there.
    
    == How to test ==
    
    === New behaviour ===
    
    Setting the `is_attached_dwh` field hides the database details:
    
    1. Configure a database as described in https://www.metabase.com/docs/latest/configuring-metabase/config-file#databases.
       - In addition to the fields you would normally set, also set
         `is_attached_dwh: true`.
       - This also works when adding this flag to a database that previously
         did not have this flag set.
    2. Start your Metabase instance.
    3. Verify the database shows up in the "admin" section
       (`/admin/databases`).
    4. Verify that clicking the database to see its details only reveals
       "This database cannot be modified."
    5. Verify that responses from the backend do not include a `details`
       field for this database.
    
    === Original behaviour ===
    
    Behaviour without setting the `is_attached_dwh` field is unchanged:
    
    1. Configure a database as described in https://www.metabase.com/docs/latest/configuring-metabase/config-file#databases.
       - Only set the fields you would normally set.  Do not set
         `is_attached_dwh` (or set it to `false`).
    2. Start your Metabase instance.
    3. Verify the database shows up in the "admin" section
       (`/admin/databases`).
    4. Verify that clicking the database to see its details only reveal the
       regular edit form, showing connection fields like `host`, `user`,
       etc. with the values you configured.
    
    == How this will be rolled out ==
    
    1. Upgrade existing Metabase Cloud instances with data warehouse to a
       Metabase version that supports `is_attached_dwh`.
    2. Set `is_attached_dwh` in the database section of the config file for
       Metabase Cloud instances with a data warehouse.
    
    == Possible follow-up work ==
    
    In https://github.com/metabase/metabase/issues/25715, absent
    `database.details` was identified as a bug.  Since then, `details` was
    made `NOT NULL` in the application database, so this bug can no longer
    occur.  However, today backend responses can be missing the `details`
    field, if the current user lacks write permission to the database
    setting (see above).  Fully re-evaluating the fix to #25715 in this
    context is outside the scope of this PR.
    
    Closes: https://github.com/metabase/harbormaster/issues/5051
Code owners
Assign users and groups as approvers for specific file changes. Learn more.