From d9acbae9b1650d482d2facd8a76d401360c0203c Mon Sep 17 00:00:00 2001 From: Michiel de Mare Date: Mon, 30 Sep 2024 09:08:22 +0200 Subject: [PATCH] Clean up handling secrets in files (#332) --- src/nl/surf/eduhub_rio_mapper/config.clj | 86 ++++++------------- .../nl/surf/eduhub_rio_mapper/config_test.clj | 81 +++++++++++++++++ 2 files changed, 108 insertions(+), 59 deletions(-) create mode 100644 test/nl/surf/eduhub_rio_mapper/config_test.clj diff --git a/src/nl/surf/eduhub_rio_mapper/config.clj b/src/nl/surf/eduhub_rio_mapper/config.clj index 54d24a16..41e84588 100644 --- a/src/nl/surf/eduhub_rio_mapper/config.clj +++ b/src/nl/surf/eduhub_rio_mapper/config.clj @@ -38,28 +38,16 @@ :gateway-user ["OOAPI Gateway Username" :str :in [:gateway-credentials :username]] :gateway-password ["OOAPI Gateway Password" :str - :default nil :in [:gateway-credentials :password]] - :gateway-password-file ["OOAPI Gateway Password File" :str - :default nil - :in [:gateway-credentials :password-file]] :gateway-root-url ["OOAPI Gateway Root URL" :http] :keystore ["Path to keystore" :file] :keystore-password ["Keystore password" :str - :default nil :in [:keystore-pass]] ; name compatibility with clj-http - :keystore-password-file ["Keystore password file" :str - :default nil - :in [:keystore-pass-file]] :keystore-alias ["Key alias in keystore" :str] :truststore ["Path to trust-store" :file :in [:trust-store]] ; name compatibility with clj-http :truststore-password ["Trust-store password" :str - :default nil :in [:trust-store-pass]] ; name compatibility with clj-http - :truststore-password-file ["Trust-store password file" :str - :default nil - :in [:trust-store-pass-file]] :rio-read-url ["RIO Services Read URL" :str :in [:rio-config :read-url]] :rio-update-url ["RIO Services Update URL" :str @@ -71,11 +59,7 @@ :surf-conext-client-id ["SurfCONEXT client id for Mapper service" :str :in [:auth-config :client-id]] :surf-conext-client-secret ["SurfCONEXT client secret for Mapper service" :str - :default nil :in [:auth-config :client-secret]] - :surf-conext-client-secret-file ["SurfCONEXT client secret for Mapper service file" :str - :default nil - :in [:auth-config :client-secret-file]] :api-port ["HTTP port for serving web API" :int :default 8080 :in [:api-config :port]] @@ -97,9 +81,6 @@ :redis-uri ["URI to redis" :str :default "redis://localhost" :in [:redis-conn :spec :uri]] - :redis-uri-file ["URI to redis file" :str - :default nil - :in [:redis-conn :spec :uri-file]] :redis-key-prefix ["Prefix for redis keys" :str :default "eduhub-rio-mapper" :in [:redis-key-prefix]] @@ -121,53 +102,41 @@ [f] [nil (str "not a file: `" s "`")]))) -(defn dissoc-in - "Return nested map with path removed." - [m ks] - (let [path (butlast ks) - node (last ks)] - (if (empty? path) - (dissoc m node) - (update-in m path dissoc node)))) +(defn- file-secret-loader-reducer [env-map value-key] + (let [file-key (keyword (str (name value-key) "-file")) + path (file-key env-map)] + (cond + (nil? path) + env-map -(defn- load-secret-from-file [config k] - (let [file-key-node (keyword (str (name (last k)) "-file")) - root-key-path (pop k) - file-key-path (conj root-key-path file-key-node) - path (get-in config file-key-path) ; File path to secret - config (dissoc-in config file-key-path)] ; Remove -file key from config - (if (nil? path) - config - (if (.exists (io/file path)) - (assoc-in config k (str/trim (slurp path))) ; Overwrite config with secret from file - (throw (ex-info (str "ENV var contains filename that does not exist: " path) {:filename path, :env-path k})))))) + (not (.exists (io/file path))) + (throw (ex-info (str "ENV var contains filename that does not exist: " path) + {:filename path, :env-path file-key})) -(def key-value-pairs-with-optional-secret-files - {:gateway-password [:gateway-credentials :password] - :keystore-password [:keystore-pass] - :redis-uri [:redis-conn :spec :uri] - :surf-conext-client-secret [:auth-config :client-secret] - :truststore-password [:trust-store-pass]}) + (value-key env-map) + (throw (ex-info "ENV var contains secret both as file and as value" + {:env-path [value-key file-key]})) -(defn- validate-required-secrets [config] - (let [missing-env (reduce - (fn [m [k v]] (if (get-in config v) - m - (assoc m k "missing"))) - {} - key-value-pairs-with-optional-secret-files)] - (when (not-empty missing-env) - (.println *err* "Configuration error") - (.println *err* (envopts/errs-description missing-env)) - (System/exit 1)) - config)) + :else + (-> env-map + (assoc value-key (str/trim (slurp path))) + (dissoc file-key))))) + +;; These ENV keys may alternatively have a form in which the secret is contained in a file. +;; These ENV keys have a -file suffix, e.g.: gateway-basic-auth-pass-file +(def env-keys-with-alternate-file-secret + [:gateway-password :keystore-password :truststore-password :surf-conext-client-secret :redis-uri]) + +(defn load-config-from-env [env-map] + (-> (reduce file-secret-loader-reducer env-map env-keys-with-alternate-file-secret) + (envopts/opts opts-spec))) (defn make-config ([] (make-config env)) ([env] {:post [(some? (-> % :rio-config :credentials :certificate))]} - (let [[config errs] (envopts/opts env opts-spec)] + (let [[config errs] (load-config-from-env env)] (when errs (.println *err* "Configuration error") @@ -179,9 +148,8 @@ keystore-alias trust-store trust-store-pass] :as cfg} - (reduce load-secret-from-file config (vals key-value-pairs-with-optional-secret-files))] + config] (-> cfg - (validate-required-secrets) (assoc-in [:rio-config :credentials] (keystore/credentials keystore keystore-pass diff --git a/test/nl/surf/eduhub_rio_mapper/config_test.clj b/test/nl/surf/eduhub_rio_mapper/config_test.clj new file mode 100644 index 00000000..c7ee4ea3 --- /dev/null +++ b/test/nl/surf/eduhub_rio_mapper/config_test.clj @@ -0,0 +1,81 @@ +;; This file is part of eduhub-mapper +;; +;; Copyright (C) 2022 SURFnet B.V. +;; +;; This program is free software: you can redistribute it and/or +;; modify it under the terms of the GNU Affero General Public License +;; as published by the Free Software Foundation, either version 3 of +;; the License, or (at your option) any later version. +;; +;; This program is distributed in the hope that it will be useful, but +;; WITHOUT ANY WARRANTY; without even the implied warranty of +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +;; Affero General Public License for more details. +;; +;; You should have received a copy of the GNU Affero General Public +;; License along with this program. If not, see +;; . + +(ns nl.surf.eduhub-rio-mapper.config-test + (:require [clojure.test :refer [deftest is]] + [nl.surf.eduhub-rio-mapper.config :as config]) + (:import [clojure.lang ExceptionInfo] + [java.io File])) + +(def default-env {:surf-conext-introspection-endpoint "https://gateway.dev.surfnet.nl/", + :keystore-alias "default", + :gateway-password "default", + :keystore "test/keystore.jks", + :clients-info-path "test/test-clients.json", + :rio-recipient-oin "default", + :truststore "truststore.jks", + :keystore-password "default", + :rio-update-url "default", + :gateway-user "default", + :surf-conext-client-id "default", + :surf-conext-client-secret "default", + :gateway-root-url "https://gateway.test.surfeduhub.nl/", + :rio-read-url "default"}) + +(def default-expected-value {:keystore-pass "default", + :gateway-credentials + {:password "default", :username "default"}, + :trust-store-pass "repelsteeltje!", + :redis-conn {:spec {:uri "redis://localhost"}}}) + +(defn- test-env [env] + (config/load-config-from-env (merge default-env env))) + +(deftest missing-secret + (is (= {:truststore-password "missing"} + (last (test-env {}))))) + +(deftest only-value-secret + (let [env {:truststore-password "repelsteeltje!"}] + (is (= default-expected-value + (-> env test-env first (select-keys [:keystore-pass :gateway-credentials :trust-store-pass :redis-conn])))))) + +(deftest only-file-secret + (let [path (.getAbsolutePath (File/createTempFile "test-secret" ".txt")) + env {:truststore-password-file path}] + (spit path "repelsteeltje!") + (is (= default-expected-value + (-> env test-env first (select-keys [:keystore-pass :gateway-credentials :trust-store-pass :redis-conn])))))) + +(deftest file-secret-for-env-with-default + (let [path (.getAbsolutePath (File/createTempFile "test-secret" ".txt")) + env {:redis-uri-file path :truststore-password "repelsteeltje!"}] + (spit path "redis://localhost:6381") + (is (= (assoc-in default-expected-value [:redis-conn :spec :uri] "redis://localhost:6381") + (-> env test-env first (select-keys [:keystore-pass :gateway-credentials :trust-store-pass :redis-conn])))))) + +(deftest only-file-secret-file-missing + (let [env {:truststore-password-file "missing-file"}] + (is (thrown? ExceptionInfo (test-env env))))) + +(deftest both-types-of-secret-specified + (let [path (.getAbsolutePath (File/createTempFile "test-secret" ".txt")) + env {:truststore-password "repelsteeltje!" + :truststore-password-file path}] + (spit path "repelsteeltje!") + (is (thrown? ExceptionInfo (test-env env)))))