Skip to content
Snippets Groups Projects
This project is mirrored from https://github.com/metabase/metabase. Pull mirroring updated .
  1. Sep 14, 2022
  2. Aug 31, 2022
  3. Aug 29, 2022
  4. 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
      Unverified
      241f2179
  5. Aug 22, 2022
  6. Aug 19, 2022
  7. Aug 16, 2022
  8. Aug 15, 2022
  9. 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
      Unverified
      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
      Unverified
      bc4acbd2
  10. 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>
      Unverified
      98bbb001
  11. Aug 05, 2022
  12. Aug 04, 2022
  13. 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`
      Unverified
      cb9e9aed
  14. Aug 02, 2022
  15. Jul 26, 2022
  16. 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
      Unverified
      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.
      Unverified
      6d46ef12
  17. Jul 22, 2022
  18. Jul 21, 2022
  19. Jul 20, 2022
  20. Jul 18, 2022
  21. Jul 15, 2022
    • dpsutton's avatar
      Async card metadata (#23672) · 21aa0053
      dpsutton authored
      
      * Lets use the main thread
      
      * Strip out channel stuff and rename
      
      * 202 -> 200 response
      
      When returning a channel we return a 202. A map is just a 200. Since we
      no longer need to have the main stuff async (as opposed to the metadata
      stuff) we can just return the map with a 200 instead of this long
      running channel stuff and a 202.
      
      * Last test
      
      * renames, logging, ensure query is the same before saving metadata
      
      * Sandbox test 202 -> 200
      
      * Another 202 -> 200
      
      * Put timeout on async metadata saving
      
      timeout of 15 minutes before we give up on the async metadata saving. It
      is possible this cuts things off but hard to tell if work is still being
      done at that point.
      
      * outdated comment
      
      * Return json error message, not text/plain
      
      this is a subtle one that I'm not happy about. Our error handling will
      return text/plain if you throw an `(ex-info "something" {:status-code
      400})`.
      
      ```shell
      ❯ http post localhost:3000/api/timeline-event/ name=some-name description=Bob timestamp=2022 timezone=America/Central time_matters:=false timeline_id:=1629  Cookie:$COOKIE
      HTTP/1.1 404 Not Found
      Content-Type: text/plain
      
      Timeline with id 1,629 not found
      ```
      
      But if you add extra information to the map to the ex-info, you get
      json!
      
      ```clojure
      (defmethod api-exception-response Throwable
        [^Throwable e]
        (let [{:keys [status-code], :as info} (ex-data e)
              other-info                      (dissoc info :status-code :schema :type)
              body                            (cond
                                                (and status-code (empty? other-info))
                                                ;; If status code was specified but other data wasn't, it's something like a
                                                ;; 404. Return message as the (plain-text) body.
                                                (.getMessage e)
      
                                                ;; if the response includes `:errors`, (e.g., it's something like a generic
                                                ;; parameter validation exception), just return the `other-info` from the
                                                ;; ex-data.
                                                (and status-code (:errors other-info))
                                                other-info
      
                                                ;; Otherwise return the full `Throwable->map` representation with Stacktrace
                                                ;; and ex-data
                                                :else
                                                (merge
                                                 (Throwable->map e)
                                                 {:message (.getMessage e)}
                                                 other-info))]
          {:status  (or status-code 500)
           :headers (mw.security/security-headers)
           :body    body}))
      ```
      
      So this fix is a _very_ subtle way to get to what we want, although it
      does add a bunch of extra junk to our response.
      
      ```javascript
      {
       ...
       "message": "Invalid Field Filter: Field 26 \"PRODUCTS\".\"CATEGORY\"
      belongs to Database 1 \"Sample Database\", but the query is against
      Database 990 \"copy of sample dataset\"",
       "data": {
         "status-code": 400,
         "query-database": 990,
         "field-filter-database": 1}
       ...
      }
      ```
      
      Reminder of what we want: the frontend is saving a card. We added a
      field filter on a database and then changed the source database of the
      query. So the backend needs to reject the field filter as being on the
      wrong db. The FE expects a json response with a message and will then
      show that message in the save/edit modal.
      
      Why did this come up now: these endpoints for saving and editing a card
      used to always return 202 streaming responses. This means they would
      have to tuck any errors inside of an already open 202 response. Which is
      why you get a message as a json response. But now that they are sync,
      the api can just return a proper 400 with a reason. But we still want
      that to be a json response for the FE.
      
      * error layout fix
      
      * Several Cleanups
      
      - make sure numbers have units at the end (-ms)
      - use (u/minutes->ms 15) rather than a more opaque (* 15 60 1000)
      - move the scheduled metadata saving into its own function
      
      This last bit is actually a bit more than that. I was previously
      throwing everything in a thread submitted to the pooled executor. I'm
      now using a `future` but with a caveat: All of the waiting for the
      timeout, checking if we got metadata is done in a simple `a/go` block
      now, and the thread does just the last bit of IO. We have to select the
      card as it is now to ensure the query is the same and then save.
      
      Refresher on why we have to check if the query is the same. We have up
      to 15 minutes to wait for the query metadata to come back. Completely
      possible for them to have edited the query and then our metadata is
      useless.
      
      Co-authored-by: default avatarAleksandr Lesnenko <alxnddr@gmail.com>
      Unverified
      21aa0053
    • Ngoc Khuat's avatar
      Store linked-filter fieldvalues (#23699) · e8d98d95
      Ngoc Khuat authored
      * first pass storing linked-filter fieldvalues
      
      * limit linked-filter to not explode
      
      * no check perms for fields when getting params values on dashboard
      Unverified
      e8d98d95
    • Alexander Polyankin's avatar
  22. Jul 14, 2022
  23. Jul 13, 2022
    • Cal Herries's avatar
      Session activity timeout (#23349) · 0defe7f8
      Cal Herries authored
      
      * logout when session expires, login when session appears
      
      * add setting UI
      
      * Add last_activity column to session table
      
      * Start implementing session middleware to check for expired sessions
      
      * Change last_activity field to include timezone offset
      
      * Update session middleware to check user activity timeout
      
      * Update last_activity after checking the timeout, or not at all if the setting is nil
      
      * Move session-timeout settings to server.middleware.session
      
      * Handcode timeout for testing
      
      * Fix migrations validation error
      
      * Fix whitespace
      
      * Change session timeout to use metabase.TIMEOUT cookie with expiry
      
      * Remove migration for last_activity column on session table
      
      * Revert changes to logout endpoint
      
      * Revert change to Session model pre-update
      
      * Remove tap>
      
      * Fix tests to include cookie value
      
      * Fix timeout when user is logged out. Timeout loop should only start when a user is logged in
      
      * Update comment and date format
      
      * Store the session-timeout setting as json and convert it to seconds on the fly
      
      * Set zoned date time to use GMT instead of default time zone
      
      * Refactor for testing
      
      * refactor session listener (#23686)
      
      * remove old session listener
      
      * Clear the timeout cookie when user signs out
      
      * Clear session cookie if the timeout cookie expires
      
      * fe tweaks
      
      * Update expires attribute for session and timeout cookies together
      
      * Reapply minimum limit on session-timeout
      
      * Rename functions and fix lint warnings
      
      * Fix resetting session-timeout
      
      * Fix sign out
      
      * Fix tests
      
      * Whitespace
      
      * Get full-app-embeds working
      
      * Add test for embedded session
      
      * session timeout ui tweaks
      
      * fix security issue
      
      * Fix test
      
      * Fix tests
      
      * Do not redirect to "/" if there isn't any redirect URL
      
      * Add test for session-cookies setting
      
      * Fix bug when toggling off timeout and adjust tests
      
      Co-authored-by: default avatarAleksandr Lesnenko <alxnddr@gmail.com>
      Co-authored-by: default avatarAleksandr Lesnenko <alxnddr@users.noreply.github.com>
      Unverified
      0defe7f8
    • Braden Shepherdson's avatar
      Serdes v2 for Metrics (#23868) · 4e32716f
      Braden Shepherdson authored
      Unverified
      4e32716f
    • Braden Shepherdson's avatar
    • Howon Lee's avatar
      Deal with SAML responses having whitespace (#23451) (#23633) · a9ca102c
      Howon Lee authored
      Pursuant to #23451.
      
      The end effect of whitespace existing in a SAML response is us choking on it as reported in #23451. Two possible interpretations of causes of this bug:
      
      There was an upstream change in our fork of the clojure SAML lib as flamber noted,
      The decoding of base64 in our SAML endpoint (which uses the SAML lib) chokes on whitespace.
      The proximate cause is the second one and ultimate cause is the first. However, I tend to believe that fixing the second one would be the better fix. For comparison, onelogin's first party SAML thing for java decodes base64 (https://github.com/onelogin/java-saml/blob/master/core/src/main/java/com/onelogin/saml2/util/Util.java) via apache's lib, which seems to do the thing that a lot of base64 decoders do of skipping whitespace.
      Unverified
      a9ca102c
Loading