From 0476b92dd82e726adab200d5a77944a5f4f3883c Mon Sep 17 00:00:00 2001 From: "Robert (Bobby) Evans" Date: Wed, 7 Oct 2015 21:03:38 +0000 Subject: [PATCH] STORM-1096: Fix some issues with impersonation on the UI --- conf/defaults.yaml | 1 + .../src/clj/backtype/storm/daemon/drpc.clj | 26 +++-- storm-core/src/clj/backtype/storm/ui/core.clj | 99 +++++++++++-------- .../storm/security/auth/ReqContext.java | 7 ++ .../auth/kerberos/ServerCallbackHandler.java | 2 + .../DefaultHttpCredentialsPlugin_test.clj | 40 ++++---- 6 files changed, 104 insertions(+), 71 deletions(-) diff --git a/conf/defaults.yaml b/conf/defaults.yaml index 4a8e354b1ea..495cec1350a 100644 --- a/conf/defaults.yaml +++ b/conf/defaults.yaml @@ -71,6 +71,7 @@ nimbus.topology.validator: "backtype.storm.nimbus.DefaultTopologyValidator" topology.min.replication.count: 1 topology.max.replication.wait.time.sec: 60 nimbus.credential.renewers.freq.secs: 600 +nimbus.impersonation.authorizer: "backtype.storm.security.auth.authorizer.ImpersonationAuthorizer" ### ui.* configs are for the master ui.host: 0.0.0.0 diff --git a/storm-core/src/clj/backtype/storm/daemon/drpc.clj b/storm-core/src/clj/backtype/storm/daemon/drpc.clj index bbc96116ed2..94759107b03 100644 --- a/storm-core/src/clj/backtype/storm/daemon/drpc.clj +++ b/storm-core/src/clj/backtype/storm/daemon/drpc.clj @@ -158,35 +158,31 @@ (fn [request] (handler request))) +(defn populate-context! + "Populate the Storm RequestContext from an servlet-request. This should be called in each handler" + [http-creds-handler servlet-request] + (when http-creds-handler + (.populateContext http-creds-handler (ReqContext/context) servlet-request))) + (defn webapp [handler http-creds-handler] (-> (routes (POST "/drpc/:func" [:as {:keys [body servlet-request]} func & m] (let [args (slurp body)] - (if http-creds-handler - (.populateContext http-creds-handler (ReqContext/context) - servlet-request)) + (populate-context! http-creds-handler servlet-request) (.execute handler func args))) (POST "/drpc/:func/" [:as {:keys [body servlet-request]} func & m] (let [args (slurp body)] - (if http-creds-handler - (.populateContext http-creds-handler (ReqContext/context) - servlet-request)) + (populate-context! http-creds-handler servlet-request) (.execute handler func args))) (GET "/drpc/:func/:args" [:as {:keys [servlet-request]} func args & m] - (if http-creds-handler - (.populateContext http-creds-handler (ReqContext/context) - servlet-request)) + (populate-context! http-creds-handler servlet-request) (.execute handler func args)) (GET "/drpc/:func/" [:as {:keys [servlet-request]} func & m] - (if http-creds-handler - (.populateContext http-creds-handler (ReqContext/context) - servlet-request)) + (populate-context! http-creds-handler servlet-request) (.execute handler func "")) (GET "/drpc/:func" [:as {:keys [servlet-request]} func & m] - (if http-creds-handler - (.populateContext http-creds-handler (ReqContext/context) - servlet-request)) + (populate-context! http-creds-handler servlet-request) (.execute handler func ""))) (wrap-reload '[backtype.storm.daemon.drpc]) handle-request)) diff --git a/storm-core/src/clj/backtype/storm/ui/core.clj b/storm-core/src/clj/backtype/storm/ui/core.clj index 5fa487feb4d..1f940985e2c 100644 --- a/storm-core/src/clj/backtype/storm/ui/core.clj +++ b/storm-core/src/clj/backtype/storm/ui/core.clj @@ -57,12 +57,10 @@ (def http-creds-handler (AuthUtils/GetUiHttpCredentialsPlugin *STORM-CONF*)) (defn assert-authorized-user - ([servlet-request op] - (assert-authorized-user servlet-request op nil)) - ([servlet-request op topology-conf] + ([op] + (assert-authorized-user op nil)) + ([op topology-conf] (let [context (ReqContext/context)] - (if http-creds-handler (.populateContext http-creds-handler context servlet-request)) - (if (.isImpersonating context) (if *UI-IMPERSONATION-HANDLER* (if-not (.permit *UI-IMPERSONATION-HANDLER* context op topology-conf) @@ -823,43 +821,62 @@ (def http-creds-handler (AuthUtils/GetUiHttpCredentialsPlugin *STORM-CONF*)) +(defn populate-context! + "Populate the Storm RequestContext from an servlet-request. This should be called in each handler" + [servlet-request] + (when http-creds-handler + (.populateContext http-creds-handler (ReqContext/context) servlet-request))) + +(defn get-user-name + [servlet-request] + (.getUserName http-creds-handler servlet-request)) + (defroutes main-routes (GET "/api/v1/cluster/configuration" [& m] - (json-response (cluster-configuration) - (:callback m) :serialize-fn identity)) + (json-response (cluster-configuration) + (:callback m) :serialize-fn identity)) (GET "/api/v1/cluster/summary" [:as {:keys [cookies servlet-request]} & m] - (let [user (.getUserName http-creds-handler servlet-request)] - (assert-authorized-user servlet-request "getClusterInfo") - (json-response (cluster-summary user) (:callback m)))) + (populate-context! servlet-request) + (assert-authorized-user "getClusterInfo") + (let [user (get-user-name servlet-request)] + (json-response (cluster-summary user) (:callback m)))) (GET "/api/v1/nimbus/summary" [:as {:keys [cookies servlet-request]} & m] - (assert-authorized-user servlet-request "getClusterInfo") + (populate-context! servlet-request) + (assert-authorized-user "getClusterInfo") (json-response (nimbus-summary) (:callback m))) (GET "/api/v1/supervisor/summary" [:as {:keys [cookies servlet-request]} & m] - (assert-authorized-user servlet-request "getClusterInfo") - (json-response (supervisor-summary) (:callback m))) + (populate-context! servlet-request) + (assert-authorized-user "getClusterInfo") + (json-response (supervisor-summary) (:callback m))) (GET "/api/v1/topology/summary" [:as {:keys [cookies servlet-request]} & m] - (assert-authorized-user servlet-request "getClusterInfo") - (json-response (all-topologies-summary) (:callback m))) - (GET "/api/v1/topology/:id" [:as {:keys [cookies servlet-request schema]} id & m] - (let [user (.getUserName http-creds-handler servlet-request)] - (assert-authorized-user servlet-request "getTopology" (topology-config id)) - (json-response (topology-page id (:window m) (check-include-sys? (:sys m)) user (= schema :https)) (:callback m)))) + (populate-context! servlet-request) + (assert-authorized-user "getClusterInfo") + (json-response (all-topologies-summary) (:callback m))) + (GET "/api/v1/topology/:id" [:as {:keys [cookies servlet-request schema]} id & m] + (populate-context! servlet-request) + (assert-authorized-user "getTopology" (topology-config id)) + (let [user (get-user-name servlet-request)] + (json-response (topology-page id (:window m) (check-include-sys? (:sys m)) user (= schema :https)) (:callback m)))) (GET "/api/v1/topology/:id/visualization" [:as {:keys [cookies servlet-request]} id & m] - (assert-authorized-user servlet-request "getTopology" (topology-config id)) - (json-response (mk-visualization-data id (:window m) (check-include-sys? (:sys m))) (:callback m))) + (populate-context! servlet-request) + (assert-authorized-user "getTopology" (topology-config id)) + (json-response (mk-visualization-data id (:window m) (check-include-sys? (:sys m))) (:callback m))) (GET "/api/v1/topology/:id/component/:component" [:as {:keys [cookies servlet-request scheme]} id component & m] - (let [user (.getUserName http-creds-handler servlet-request)] - (assert-authorized-user servlet-request "getTopology" (topology-config id)) - (json-response + (populate-context! servlet-request) + (assert-authorized-user "getTopology" (topology-config id)) + (let [user (get-user-name servlet-request)] + (json-response (component-page id component (:window m) (check-include-sys? (:sys m)) user (= scheme :https)) (:callback m)))) (GET "/api/v1/topology/:id/logconfig" [:as {:keys [cookies servlet-request]} id & m] - (assert-authorized-user servlet-request "getTopology" (topology-config id)) + (populate-context! servlet-request) + (assert-authorized-user "getTopology" (topology-config id)) (json-response (log-config id) (:callback m))) (POST "/api/v1/topology/:id/activate" [:as {:keys [cookies servlet-request]} id & m] + (populate-context! servlet-request) + (assert-authorized-user "activate" (topology-config id)) (thrift/with-configured-nimbus-connection nimbus - (assert-authorized-user servlet-request "activate" (topology-config id)) - (let [tplg (->> (doto + (let [tplg (->> (doto (GetInfoOptions.) (.set_num_err_choice NumErrorsChoice/NONE)) (.getTopologyInfoWithOpts ^Nimbus$Client nimbus id)) @@ -868,9 +885,10 @@ (log-message "Activating topology '" name "'"))) (json-response (topology-op-response id "activate") (m "callback"))) (POST "/api/v1/topology/:id/deactivate" [:as {:keys [cookies servlet-request]} id & m] + (populate-context! servlet-request) + (assert-authorized-user "deactivate" (topology-config id)) (thrift/with-configured-nimbus-connection nimbus - (assert-authorized-user servlet-request "deactivate" (topology-config id)) - (let [tplg (->> (doto + (let [tplg (->> (doto (GetInfoOptions.) (.set_num_err_choice NumErrorsChoice/NONE)) (.getTopologyInfoWithOpts ^Nimbus$Client nimbus id)) @@ -879,9 +897,10 @@ (log-message "Deactivating topology '" name "'"))) (json-response (topology-op-response id "deactivate") (m "callback"))) (POST "/api/v1/topology/:id/debug/:action/:spct" [:as {:keys [cookies servlet-request]} id action spct & m] - (assert-authorized-user servlet-request "debug" (topology-config id)) + (populate-context! servlet-request) + (assert-authorized-user "debug" (topology-config id)) (thrift/with-configured-nimbus-connection nimbus - (let [tplg (->> (doto + (let [tplg (->> (doto (GetInfoOptions.) (.set_num_err_choice NumErrorsChoice/NONE)) (.getTopologyInfoWithOpts ^Nimbus$Client nimbus id)) @@ -889,9 +908,10 @@ enable? (= "enable" action)] (.debug nimbus name "" enable? (Integer/parseInt spct)) (log-message "Debug topology [" name "] action [" action "] sampling pct [" spct "]"))) - (json-response (topology-op-response id (str "debug/" action)) (m "callback"))) + (json-response (topology-op-response id (str "debug/" action)) (m "callback"))) (POST "/api/v1/topology/:id/component/:component/debug/:action/:spct" [:as {:keys [cookies servlet-request]} id component action spct & m] - (assert-authorized-user servlet-request "debug" (topology-config id)) + (populate-context! servlet-request) + (assert-authorized-user "debug" (topology-config id)) (thrift/with-configured-nimbus-connection nimbus (let [tplg (->> (doto (GetInfoOptions.) @@ -903,8 +923,9 @@ (log-message "Debug topology [" name "] component [" component "] action [" action "] sampling pct [" spct "]"))) (json-response (component-op-response id component (str "/debug/" action)) (m "callback"))) (POST "/api/v1/topology/:id/rebalance/:wait-time" [:as {:keys [cookies servlet-request]} id wait-time & m] + (populate-context! servlet-request) + (assert-authorized-user "rebalance" (topology-config id)) (thrift/with-configured-nimbus-connection nimbus - (assert-authorized-user servlet-request "rebalance" (topology-config id)) (let [tplg (->> (doto (GetInfoOptions.) (.set_num_err_choice NumErrorsChoice/NONE)) @@ -922,7 +943,8 @@ (log-message "Rebalancing topology '" name "' with wait time: " wait-time " secs"))) (json-response (topology-op-response id "rebalance") (m "callback"))) (POST "/api/v1/topology/:id/kill/:wait-time" [:as {:keys [cookies servlet-request]} id wait-time & m] - (assert-authorized-user servlet-request "killTopology" (topology-config id)) + (populate-context! servlet-request) + (assert-authorized-user "killTopology" (topology-config id)) (thrift/with-configured-nimbus-connection nimbus (let [tplg (->> (doto (GetInfoOptions.) @@ -934,9 +956,9 @@ (.killTopologyWithOpts nimbus name options) (log-message "Killing topology '" name "' with wait time: " wait-time " secs"))) (json-response (topology-op-response id "kill") (m "callback"))) - (POST "/api/v1/topology/:id/logconfig" [:as {:keys [cookies servlet-request]} id namedLoggerLevels & m] - (assert-authorized-user servlet-request "setLogConfig" (topology-config id)) + (populate-context! servlet-request) + (assert-authorized-user "setLogConfig" (topology-config id)) (thrift/with-configured-nimbus-connection nimbus (let [new-log-config (LogConfig.)] @@ -961,9 +983,8 @@ (log-message "Setting topology " id " log config " new-log-config) (.setLogConfig nimbus id new-log-config) (json-response (log-config id) (m "callback"))))) - (GET "/" [:as {cookies :cookies}] - (resp/redirect "/index.html")) + (resp/redirect "/index.html")) (route/resources "/") (route/not-found "Page not found")) diff --git a/storm-core/src/jvm/backtype/storm/security/auth/ReqContext.java b/storm-core/src/jvm/backtype/storm/security/auth/ReqContext.java index a252f85b831..2f18982b6a8 100644 --- a/storm-core/src/jvm/backtype/storm/security/auth/ReqContext.java +++ b/storm-core/src/jvm/backtype/storm/security/auth/ReqContext.java @@ -55,6 +55,13 @@ public static ReqContext context() { return ctxt.get(); } + /** + * Reset the context back to a default. used for testing. + */ + public static void reset() { + ctxt.remove(); + } + //each thread will have its own request context private static final ThreadLocal < ReqContext > ctxt = new ThreadLocal < ReqContext > () { diff --git a/storm-core/src/jvm/backtype/storm/security/auth/kerberos/ServerCallbackHandler.java b/storm-core/src/jvm/backtype/storm/security/auth/kerberos/ServerCallbackHandler.java index 7b143f0d6d1..46ffa612d63 100644 --- a/storm-core/src/jvm/backtype/storm/security/auth/kerberos/ServerCallbackHandler.java +++ b/storm-core/src/jvm/backtype/storm/security/auth/kerberos/ServerCallbackHandler.java @@ -87,6 +87,8 @@ private void handleAuthorizeCallback(AuthorizeCallback ac) { //add the authNid as the real user in reqContext's subject which will be used during authorization. if(!ac.getAuthenticationID().equals(ac.getAuthorizationID())) { ReqContext.context().setRealPrincipal(new SaslTransportPlugin.User(ac.getAuthenticationID())); + } else { + ReqContext.context().setRealPrincipal(null); } ac.setAuthorized(true); diff --git a/storm-core/test/clj/backtype/storm/security/auth/DefaultHttpCredentialsPlugin_test.clj b/storm-core/test/clj/backtype/storm/security/auth/DefaultHttpCredentialsPlugin_test.clj index e7b44cfe850..00c1d761aa0 100644 --- a/storm-core/test/clj/backtype/storm/security/auth/DefaultHttpCredentialsPlugin_test.clj +++ b/storm-core/test/clj/backtype/storm/security/auth/DefaultHttpCredentialsPlugin_test.clj @@ -48,22 +48,28 @@ (is (.equals exp-name (.getUserName handler req))))) (testing "returns doAsUser from requests principal when Header has doAsUser param set" - (let [exp-name "Alice" - do-as-user-name "Bob" - princ (SingleUserPrincipal. exp-name) - req (Mockito/mock HttpServletRequest) - _ (. (Mockito/when (. req getUserPrincipal)) - thenReturn princ) - _ (. (Mockito/when (. req getHeader "doAsUser")) - thenReturn do-as-user-name) - context (.populateContext handler (ReqContext/context) req)] - (is (= true (.isImpersonating context))) - (is (.equals exp-name (.getName (.realPrincipal context)))) - (is (.equals do-as-user-name (.getName (.principal context)))))))) + (try + (let [exp-name "Alice" + do-as-user-name "Bob" + princ (SingleUserPrincipal. exp-name) + req (Mockito/mock HttpServletRequest) + _ (. (Mockito/when (. req getUserPrincipal)) + thenReturn princ) + _ (. (Mockito/when (. req getHeader "doAsUser")) + thenReturn do-as-user-name) + context (.populateContext handler (ReqContext/context) req)] + (is (= true (.isImpersonating context))) + (is (.equals exp-name (.getName (.realPrincipal context)))) + (is (.equals do-as-user-name (.getName (.principal context))))) + (finally + (ReqContext/reset)))))) (deftest test-populate-req-context-on-null-user - (let [req (Mockito/mock HttpServletRequest) - handler (doto (DefaultHttpCredentialsPlugin.) (.prepare {})) - subj (Subject. false (set [(SingleUserPrincipal. "test")]) (set []) (set [])) - context (ReqContext. subj)] - (is (= 0 (-> handler (.populateContext context req) (.subject) (.getPrincipals) (.size)))))) + (try + (let [req (Mockito/mock HttpServletRequest) + handler (doto (DefaultHttpCredentialsPlugin.) (.prepare {})) + subj (Subject. false (set [(SingleUserPrincipal. "test")]) (set []) (set [])) + context (ReqContext. subj)] + (is (= 0 (-> handler (.populateContext context req) (.subject) (.getPrincipals) (.size))))) + (finally + (ReqContext/reset))))