Skip to content
Snippets Groups Projects
This project is mirrored from https://github.com/metabase/metabase. Pull mirroring updated .
  1. Sep 19, 2022
    • dpsutton's avatar
      Don't use persisted model tables for segmented users (#25347) · 32403f0d
      dpsutton authored
      * Don't use persisted model tables for segmented users
      
      This actually isn't a bug, but due to very subtle and arbitrary reasons.
      
      For background about why we need to ensure this never happens, we cannot
      use persisted models when sandboxing is at play. A simple example is as
      follows: make a model on a products table that does not select the
      category. Have a sandbox on category such that someone can only see
      products of category "Gizmo". the model lacks the category column but we
      insert a where clause that still works. When the model is persisted,
      there is no category column in the underlying table so sandboxing cannot
      possibly work: the data necessary to filter is no longer associated with
      the rest of the data in the model.
      
      The fix for this is quite simple: in
      `metabase.query-processor.middleware.fetch-source-query` we only splice
      in the persisted query if the user is not a segmented user (product name
      for sandboxing).
      
      ```clojure
      (and persisted-info/*allow-persisted-substitution*
           (not (segmented-user?))  ;; <----- new check
           (:active card)
           (:definition card)
           (:query_hash card)
           (= (:query_hash card) (persisted-info/query-hash (:dataset_query card)))
           (= (:definition card) (persisted-info/metadata->definition (:result_metadata card)
                                                                      (:table_name card)))
           (= (:state card) "persisted"))
      ```
      
      Technical details about why this bug did not manifest
      
      When swapping out a card__<id> to a source query, if its a model we will
      see if it is persisted, and if so, we will use the native sql to select
      from the persisted table. It does this by adding the native sql at a key
      called `:persisted-info/native` and a middleware
      `#'qp.persistence/substitute-persisted-query` walks the query replacing
      the query with the native:
      
      ```clojure
      ;; metabase.query-processor.middleware.persistence
      
          (mbql.u/replace query
            (x :guard (every-pred map? :persisted-info/native))
            {:native (:persisted-info/native x)})
      ```
      
      There is also a middleware that walks through the query looking for
      tables with gtaps on them and replacing them. By change, the sandboxing
      middleware runs immediately before the substitute-persisted middleware!
      
      ```clojure
         ;; literally the previous middleware
         (resolve 'ee.sandbox.rows/apply-sandboxing)
         #'qp.persistence/substitute-persisted-query
      ```
      
      If you swap the order of these two sandboxing is broken. As is, it
      "works" but not by design, just by happenstance. The sandboxing
      middleware just did not know that the `:persisted-info/native` key meant
      that a native query was to be substituted. In the reverse order, the
      native query is already substituted and there is no change for the
      sandboxing to occur.
      
      The obvious fix is to ensure that we never even attempt to use the
      persisted tables and that is what this PR does.
      
      * Eastwood doesn't like shadowing like this
      
      * Rearrange check order for tests
      
      `segmented-user?` throws if there is no bound user. A test in
      `fetch-source-query-test` was failing because there was no user bound,
      but it wasn't attempting to swap out a persisted table, it just didn't
      expect to need a user.
      
      Moving it lower lets it short circuit on other bits that are bound to
      fail (definition, query_hash, etc) requiring persistence before we check
      for a bound user
      32403f0d
    • Noah Moss's avatar
      LDAP setup improvements (#25421) · b18d53e8
      Noah Moss authored
      
      * first pass at LDAP setup improvements
      
      * fix lint errors
      
      * fix LDAP api tests
      
      * WIP test for new setting setter
      
      * fix setting test
      
      * set ldap-enabled to true in ldap server macro
      
      * try to fix java11 tests
      
      * Update src/metabase/integrations/ldap.clj
      
      Co-authored-by: default avatarmetamben <103100869+metamben@users.noreply.github.com>
      
      * add transaction
      
      * remove ldap-ever-enabled? setting and revert some of the logic that is no longer necessary
      
      * set ldap-enabled via the ldap api and add tests
      
      * fix tests and lint
      
      * fix error on settings save
      
      * fix cypress test
      
      * actually fix cypress
      
      Co-authored-by: default avatarmetamben <103100869+metamben@users.noreply.github.com>
      b18d53e8
  2. Sep 15, 2022
  3. Sep 14, 2022
  4. Sep 12, 2022
  5. Sep 06, 2022
  6. Aug 31, 2022
  7. Aug 29, 2022
  8. Aug 23, 2022
    • Cam Saul's avatar
      [Toucan 2 Prep] Replace the `IObjectPermissions` protocol with multimethods (#24917) · 241f2179
      Cam Saul authored
      * [Toucan 2 prep] Don't invoke Toucan models as functions
      
      * Some fixes
      
      * Test fixes
      
      * Test fix
      
      * [Toucan 2 prep] Don't call `type` or `class` on Toucan models
      
      * Test fixes
      
      * More test fixes :wrench:
      
      * Replace perms protocol with multimethods; derive models from perms policy keywords
      
      * Test fixes :wrench:
      
      * Appease Eastwood
      
      * Fix errors now that App has been merged in
      
      * Empty commit to trigger CI
      241f2179
  9. Aug 22, 2022
  10. Aug 19, 2022
  11. Aug 18, 2022
  12. Aug 17, 2022
    • Cal Herries's avatar
      Fix for #24715: "Remember Me" functionality doesn't work (#24744) · 4532d07e
      Cal Herries authored
      * Separate setting the timeout cookie's expires attribute from the session cookie's max-age
      
      * Add back public-settings/session-cookies tests and add docstrings
      
      * Fix setting cookies with full-app-embedding
      
      * Add docstring
      
      * Change logout to always delete the session
      
      * Remove extra code
      
      * Change session-cookie-name from multimethod to simple case expression
      
      * Refactor: move use-permanent-cookies? closer to usage
      
      * Fix FE unit test
      4532d07e
  13. Aug 16, 2022
  14. Aug 15, 2022
  15. Aug 12, 2022
    • Cam Saul's avatar
      Enable Kondo for tests part 2: enable `:unused-binding` linter and fix warnings (#24748) · adf45182
      Cam Saul authored
      * Fix some small things
      
      * Add Kondo to deps.edn to be able to debug custom hooks from REPL
      
      * Fix macroexpansion hook for with-temp* without values
      
      * Test config (WIP)
      
      * More misc fixes
      
      * Disable :inline-def for tests
      
      * More misc fixes
      
      * Fix $ids and mbql-query kondo hooks.
      
      * Fix with-temporary-setting-values with namespaced symbols
      
      * More misc fixes
      
      * Fix the rest of the easy ones
      
      * Fix hook for mt/dataset
      
      * Horrible hack to work around https://github.com/clj-kondo/clj-kondo/issues/1773 . Custom linter for mbql-query macro
      
      * Fix places calling mbql-query with a keyword table name
      
      * Fix the last few errors in test/
      
      * Fix errors in enterprise/test and shared/test
      
      * Fix driver test errors
      
      * Enable linters on CI
      
      * Enable unresolved-namespace linter for tests
      
      * Appease the namespace linter again
      
      * Test fixes
      
      * Enable unused-binding linter for test/ => 293 warnings
      
      * 259 warnings
      
      * 234 warnings
      
      * => 114 warnings
      
      * Fix the rest of the unused binding warnings in test/
      
      * Fix unused binding errors in enterprise/backend/test
      
      * Fix unused binding lint errors in driver tests
      
      * Test fix :wrench:
      
      * Assure Kondo that something is in fact used
      adf45182
    • Cam Saul's avatar
      Enable Kondo for tests (part 1) (#24736) · bc4acbd2
      Cam Saul authored
      * Fix some small things
      
      * Add Kondo to deps.edn to be able to debug custom hooks from REPL
      
      * Fix macroexpansion hook for with-temp* without values
      
      * Test config (WIP)
      
      * More misc fixes
      
      * Disable :inline-def for tests
      
      * More misc fixes
      
      * Fix $ids and mbql-query kondo hooks.
      
      * Fix with-temporary-setting-values with namespaced symbols
      
      * More misc fixes
      
      * Fix the rest of the easy ones
      
      * Fix hook for mt/dataset
      
      * Horrible hack to work around https://github.com/clj-kondo/clj-kondo/issues/1773 . Custom linter for mbql-query macro
      
      * Fix places calling mbql-query with a keyword table name
      
      * Fix the last few errors in test/
      
      * Fix errors in enterprise/test and shared/test
      
      * Fix driver test errors
      
      * Enable linters on CI
      
      * Enable unresolved-namespace linter for tests
      
      * Appease the namespace linter again
      
      * Test fixes
      bc4acbd2
  16. Aug 10, 2022
    • Case Nelson's avatar
      [Actions] Simplify emitter schema model (#24570) · 98bbb001
      Case Nelson authored
      * Move writeback migrations to 45
      
      * Empty commit to trigger GitHub Actions
      
      * [Actions] Simplify emitter schema model
      
      emitter_action was dropped since emitters just have a singular action
      and the join table was unecessary.
      
      emitter_action.action_id columns moved onto emitter table.
      
      Dropped CardEmitter and DashboardEmitter pre-insert, pre-update,
      pre-delete since they were used in tests only and normal operation
      would see the emitter inserted first.
      
      Since previous code may have 'orphaned' emitters without an action, we
      delete emitters without action to be safe.
      
      * Handle flakiness with geojson java.net.UnknownHostException errors (#24523)
      
      * Handle flakiness with geojson java.net.UnknownHostException errors
      
      In CI seems like we are getting errant errors:
      
      ```clojure
      geojson.clj:62
      It validates URLs and files appropriately
      http://0xc0000200
      expected: (valid? geojson)
        actual: #error {
       :cause "Invalid IP address literal: 0xc0000200"
       :via
       [{:type clojure.lang.ExceptionInfo
         :message "Invalid GeoJSON file location: must either start with http:// or https:// or be a relative path to a file on the classpath. URLs referring to hosts that supply internal hosting metadata are prohibited."
         :data {:status-code 400, :url "http://0xc0000200"}
         :at [metabase.api.geojson$valid_url_QMARK_ invokeStatic "geojson.clj" 62]}
        {:type java.net.UnknownHostException
         :message "0xc0000200"
         :at [java.net.InetAddress getAllByName "InetAddress.java" 1340]}
        {:type java.lang.IllegalArgumentException
         :message "Invalid IP address literal: 0xc0000200"
         :at [sun.net.util.IPAddressUtil validateNumericFormatV4 "IPAddressUtil.java" 150]}]
      ```
      
      Not clear if this change has a hope of fixing it: if it doesn't resolve
      once its possible it is cached somewhere in the network stack, or it
      won't resolve if you ask again.
      
      But gonna give it a shot.
      
      Set the property `"networkaddress.cache.negative.ttl"` to `"0"`
      
      > networkaddress.cache.negative.ttl (default: 10)
      >    Indicates the caching policy for un-successful name lookups from the name service. The value is specified as an integer to indicate the number of seconds to cache the failure for un-successful lookups.
      
      >    A value of 0 indicates "never cache". A value of -1 indicates "cache forever".
      
      From
      https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/net/InetAddress.html
      
      
      in the hopes that we can try multiple times. Restores the original value
      after the test completes so we don't inadvertently change behavior
      elsewhere.
      
      If we get an error of java.net.UnknownHostException we try again if we
      have attempts remaining. If we get a boolean it means the ip resolution
      worked so we can rely on the response (checking if it resolves locally
      or not)
      
      * add a delay
      
      * comment out test
      
      Co-authored-by: default avatarCam Saul <github@camsaul.com>
      Co-authored-by: default avatardpsutton <dan@dpsutton.com>
      98bbb001
  17. Aug 08, 2022
  18. Aug 05, 2022
  19. Aug 04, 2022
  20. Aug 03, 2022
    • Braden Shepherdson's avatar
      Serdes v2: Handle other embedded MBQL fragments (#24537) · cb9e9aed
      Braden Shepherdson authored
      This PR handles the other JSON-encoded MBQL snippets I was able to find.
      
      These snippets contain `Table` and `Field` IDs, and so are not portable. These
      fields are expanded during serialization and the IDs replaced with portable
      references, then converted back in deserialization.
      
      Note that the referenced field must already be loaded before it has a valid ID.
      `serdes-dependencies` defines this order, therefore each entity depends on those
      tables and fields referenced in its MBQL fragments.
      
      The complete set of fields I found to convert:
      
      - `Metric.definition`
      - `Segment.definition`
      - `DashboardCard.parameter_mappings`
      - `Card.parameter_mappings`
      cb9e9aed
  21. Aug 02, 2022
  22. Jul 26, 2022
  23. Jul 25, 2022
    • adam-james's avatar
      Add a check to PUT /user/:id to disallow name edits if an SSO user (#23752) · 3795b56c
      adam-james authored
      * Add a check to PUT /user/:id to disallow name edits if an SSO user
      
      * Clean up After SAML SSO tests
      
      The `:sso_source` key is set for the Rasta user in some SAML tests, but is expeted to be nil in subsequent tests, so
      we clean up in the SAML test ns.
      
      * Add a test to ensure SSO user names can't be changed via API
      
      * Missed a change I had made while adjusting tests
      
      * valid-name-update? take 1 name, to allow better error messages
      
      Let's the user know that first or last name is the cause of a problem, rather than just 'names'.
      
      * Remove unneeded thread macro
      
      * Use partial=
      
      * slight change to local fn for reusability
      3795b56c
    • Braden Shepherdson's avatar
      Serdes v2: Add `--v2 true` flag to the `dump` and `load` commands (#24230) · 6d46ef12
      Braden Shepherdson authored
      * Serdes v2: Add `--v2 true` flag to the `dump` and `load` commands
      
      This is the last piece for end-to-end serialization and deserialization.
      The results should be similar to v1, though probably not identical.
      6d46ef12
  24. Jul 22, 2022
Loading