From b4806113a7b06a93ea737f681e966701397e539c 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 Conflicts: storm-core/src/clj/backtype/storm/ui/core.clj --- bin/storm.py | 7 +- conf/defaults.yaml | 1 + .../src/clj/backtype/storm/daemon/drpc.clj | 26 +++--- storm-core/src/clj/backtype/storm/ui/core.clj | 80 ++++++++++++------- .../storm/security/auth/ReqContext.java | 7 ++ .../auth/kerberos/ServerCallbackHandler.java | 2 + .../DefaultHttpCredentialsPlugin_test.clj | 40 ++++++---- 7 files changed, 95 insertions(+), 68 deletions(-) diff --git a/bin/storm.py b/bin/storm.py index 0dddcdb5ce6..a4144085e61 100755 --- a/bin/storm.py +++ b/bin/storm.py @@ -375,13 +375,8 @@ def supervisor(klass="backtype.storm.daemon.supervisor"): """ cppaths = [CLUSTER_CONF_DIR] jvmopts = parse_args(confvalue("supervisor.childopts", cppaths)) + [ -<<<<<<< HEAD - "-Dlogfile.name=supervisor.log", - "-Dlog4j.configurationFile=" + os.path.join(get_log4j_conf_dir(), "cluster.xml"), -======= "-Dlogfile.name=" + STORM_SUPERVISOR_LOG_FILE, - "-Dlog4j.configurationFile=" + os.path.join(get_log4j2_conf_dir(), "cluster.xml"), ->>>>>>> 3813930... configurable supervisor log filename + "-Dlog4j.configurationFile=" + os.path.join(get_log4j_conf_dir(), "cluster.xml"), ] exec_storm_class( klass, diff --git a/conf/defaults.yaml b/conf/defaults.yaml index c3fa3722911..8ff0c128532 100644 --- a/conf/defaults.yaml +++ b/conf/defaults.yaml @@ -65,6 +65,7 @@ nimbus.reassign: true nimbus.file.copy.expiration.secs: 600 nimbus.topology.validator: "backtype.storm.nimbus.DefaultTopologyValidator" 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 bd1795959b0..3d06d321456 100644 --- a/storm-core/src/clj/backtype/storm/ui/core.clj +++ b/storm-core/src/clj/backtype/storm/ui/core.clj @@ -59,12 +59,10 @@ ~@body))) (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) @@ -937,35 +935,54 @@ (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/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]} 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) (: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]} 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) (: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]} id component & m] - (let [user (.getUserName http-creds-handler servlet-request)] - (assert-authorized-user servlet-request "getTopology" (topology-config id)) - (json-response (component-page id component (:window m) (check-include-sys? (:sys m)) user) (:callback m)))) + (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) + (:callback m)))) (POST "/api/v1/topology/:id/activate" [:as {:keys [cookies servlet-request]} id & m] - (assert-authorized-user servlet-request "activate" (topology-config id)) + (populate-context! servlet-request) + (assert-authorized-user "activate" (topology-config id)) (with-nimbus nimbus - (let [tplg (->> (doto + (let [tplg (->> (doto (GetInfoOptions.) (.set_num_err_choice NumErrorsChoice/NONE)) (.getTopologyInfoWithOpts ^Nimbus$Client nimbus id)) @@ -974,9 +991,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] - (assert-authorized-user servlet-request "deactivate" (topology-config id)) + (populate-context! servlet-request) + (assert-authorized-user "deactivate" (topology-config id)) (with-nimbus nimbus - (let [tplg (->> (doto + (let [tplg (->> (doto (GetInfoOptions.) (.set_num_err_choice NumErrorsChoice/NONE)) (.getTopologyInfoWithOpts ^Nimbus$Client nimbus id)) @@ -985,7 +1003,8 @@ (log-message "Deactivating topology '" name "'"))) (json-response (topology-op-response id "deactivate") (m "callback"))) (POST "/api/v1/topology/:id/rebalance/:wait-time" [:as {:keys [cookies servlet-request]} id wait-time & m] - (assert-authorized-user servlet-request "rebalance" (topology-config id)) + (populate-context! servlet-request) + (assert-authorized-user "rebalance" (topology-config id)) (with-nimbus nimbus (let [tplg (->> (doto (GetInfoOptions.) @@ -1004,7 +1023,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)) (with-nimbus nimbus (let [tplg (->> (doto (GetInfoOptions.) @@ -1017,7 +1037,7 @@ (log-message "Killing topology '" name "' with wait time: " wait-time " secs"))) (json-response (topology-op-response id "kill") (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))))