Skip to content

Commit

Permalink
Fix/discovery database permissions error (#33)
Browse files Browse the repository at this point in the history 
* Test to confirm failing case

* Try/catch for this specific error and fix test
  • Loading branch information
@dmosorast
dmosorast authored May 15, 2020
1 parent a629eb7 commit f1362ea
Show file tree
Hide file tree
Showing 2 changed files with 116 additions and 8 deletions.
25 changes: 17 additions & 8 deletions src/tap_mssql/catalog.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,23 @@
;; :table_catalog (database name)
;; :table_schem (schema name)
[config database]
(conj (filter #(not (nil? (:table_catalog %)))
(jdbc/with-db-metadata [md (assoc (config/->conn-map config)
:dbname
(:table_cat database))]
(jdbc/metadata-result (.getSchemas md))))
;; Calling getSchemas does not return dbo so that's added
;; for each database
{:table_catalog (:table_cat database) :table_schem "dbo"}))
(try
(conj (filter #(not (nil? (:table_catalog %)))
(jdbc/with-db-metadata [md (assoc (config/->conn-map config)
:dbname
(:table_cat database))]
(jdbc/metadata-result (.getSchemas md))))
;; Calling getSchemas does not return dbo so that's added
;; for each database
{:table_catalog (:table_cat database) :table_schem "dbo"})
(catch com.microsoft.sqlserver.jdbc.SQLServerException ex
;; NB: 4060 is indicative of a lack of permissions or failed login
;; https://docs.microsoft.com/en-us/sql/relational-databases/errors-events/database-engine-events-and-errors?view=sql-server-ver15#errors-4000-to-4999
(when-not (= 4060 (.getErrorCode ex))
(throw ex))

(log/warnf "%s - Skipping due to error discovering tables: %s" (:table_cat database) (.getMessage ex))
[])))

(defn get-databases
[config]
Expand Down
99 changes: 99 additions & 0 deletions test/tap_mssql/discover_permissions_test.clj
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
(ns tap-mssql.discover-permissions-test
(:require [tap-mssql.catalog :as catalog]
[tap-mssql.config :as config]
[clojure.test :refer [is deftest use-fixtures]]
[clojure.java.io :as io]
[clojure.java.jdbc :as jdbc]
[clojure.set :as set]
[clojure.string :as string]
[tap-mssql.core :refer :all]
[tap-mssql.test-utils :refer [with-out-and-err-to-dev-null
test-db-config]]))

(defn get-destroy-database-command
[database]
(format "DROP DATABASE IF EXISTS %s" (:table_cat database)))

(defn maybe-destroy-test-db
[]
(let [destroy-database-commands (->> [{:table_cat "not_authorized_database"}
{:table_cat "database_with_a_table"}
{:table_cat "not_authorized_database_too"}]
(filter catalog/non-system-database?)
(map get-destroy-database-command))]
(let [db-spec (config/->conn-map test-db-config)]
(jdbc/db-do-commands db-spec destroy-database-commands))))

(defn create-test-db
[]
(let [db-spec (config/->conn-map test-db-config)]
(jdbc/db-do-commands db-spec ["CREATE DATABASE not_authorized_database"
"CREATE DATABASE database_with_a_table"
"CREATE DATABASE not_authorized_database_too"])
;; Create Tables
(jdbc/db-do-commands (assoc db-spec :dbname "database_with_a_table")
[(jdbc/create-table-ddl :empty_table [[:id "int"]])])
(jdbc/db-do-commands (assoc db-spec :dbname "not_authorized_database")
[(jdbc/create-table-ddl :empty_table [[:id "int"]])])
(jdbc/db-do-commands (assoc db-spec :dbname "not_authorized_database_too")
[(jdbc/create-table-ddl :empty_table [[:id "int"]])])
;; Create view/tvf for fun
(jdbc/db-do-commands (assoc db-spec :dbname "database_with_a_table")
["CREATE VIEW empty_table_ids
AS
SELECT id FROM empty_table"])
(jdbc/db-do-commands (assoc db-spec :dbname "database_with_a_table")
["CREATE FUNCTION table_valued_test(@input_value int)
RETURNS @result table (a_value int)
AS
BEGIN
INSERT INTO @result VALUES(@input_value + 1)
RETURN
END"])
;; Create User if not exists
(when (empty? (jdbc/query (assoc db-spec :dbname "database_with_a_table")
"SELECT principal_id FROM sys.server_principals WHERE name = 'SingerTestUser'"))
(jdbc/db-do-commands (assoc db-spec :dbname "database_with_a_table")
["CREATE LOGIN SingerTestUser WITH PASSWORD = 'ABCD12345$%'"]))
(when (empty? (jdbc/query (assoc db-spec :dbname "database_with_a_table")
"SELECT principal_id FROM sys.database_principals WHERE name = 'SingerTestUser'"))
(jdbc/db-do-commands (assoc db-spec :dbname "database_with_a_table")
["CREATE USER SingerTestUser FOR LOGIN SingerTestUser"
"GRANT SELECT ON dbo.empty_table TO SingerTestUser"
"GRANT SELECT ON dbo.empty_table_ids TO SingerTestUser"]))))

(defn test-db-fixture [f]
(with-out-and-err-to-dev-null
(maybe-destroy-test-db)
(create-test-db)
(f)))

(use-fixtures :each test-db-fixture)

(deftest ^:integration verify-populated-catalog
(is (let [stream-names (set (map #(get % "stream") (vals ((catalog/discover (assoc test-db-config
"user" "SingerTestUser"
"password" "ABCD12345$%"))
"streams"))))]
(stream-names "empty_table")))
(is (let [stream-names (set (map #(get % "stream") (vals ((catalog/discover (assoc test-db-config
"user" "SingerTestUser"
"password" "ABCD12345$%"))
"streams"))))]
(stream-names "empty_table_ids")))
;; Table-Valued functions should not be discovered
(is (nil? (let [stream-names (set (map #(get % "stream") (vals ((catalog/discover (assoc test-db-config
"user" "SingerTestUser"
"password" "ABCD12345$%"))
"streams"))))]
(stream-names "table_valued_test"))))

;; Should not discover tables in non-permitted databases
(is (let [discovered-dbs (->> (get (catalog/discover (assoc test-db-config
"user" "SingerTestUser"
"password" "ABCD12345$%")) "streams")
(map (fn [[_ schema]] (get-in schema ["metadata" "database-name"])))
set)]
(empty? (clojure.set/intersection
discovered-dbs
#{"not_authorized_database" "not_authorized_database_too"})))))

0 comments on commit f1362ea

Please sign in to comment.