CLOUDSTACK-7455: Fix possible case for NPE

yadvr · yadvr · commit 33a249e77aea · 2014-08-31T14:42:18.000+02:00
NPE can happen if Spring fails to inject api authenticator, so better check
and set list of commands if the authenticator is not null or returning null cmds

Signed-off-by: Rohit Yadav &lt;rohit.yadav@shapeblue.com&gt;
diff --git a/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/GetServiceProviderMetaDataCmd.java b/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/GetServiceProviderMetaDataCmd.java
@@ -191,7 +191,7 @@ public APIAuthenticationType getAPIType() {
     @Override
     public void setAuthenticators(List<PluggableAPIAuthenticator> authenticators) {
         for (PluggableAPIAuthenticator authManager: authenticators) {
-            if (authManager instanceof SAML2AuthManager) {
+            if (authManager != null && authManager instanceof SAML2AuthManager) {
                 _samlAuthManager = (SAML2AuthManager) authManager;
             }
         }
diff --git a/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java b/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java
@@ -292,7 +292,7 @@ public APIAuthenticationType getAPIType() {
     @Override
     public void setAuthenticators(List<PluggableAPIAuthenticator> authenticators) {
         for (PluggableAPIAuthenticator authManager: authenticators) {
-            if (authManager instanceof SAML2AuthManager) {
+            if (authManager != null && authManager instanceof SAML2AuthManager) {
                 _samlAuthManager = (SAML2AuthManager) authManager;
             }
         }
diff --git a/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LogoutAPIAuthenticatorCmd.java b/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LogoutAPIAuthenticatorCmd.java
@@ -158,7 +158,7 @@ public APIAuthenticationType getAPIType() {
     @Override
     public void setAuthenticators(List<PluggableAPIAuthenticator> authenticators) {
         for (PluggableAPIAuthenticator authManager: authenticators) {
-            if (authManager instanceof SAML2AuthManager) {
+            if (authManager != null && authManager instanceof SAML2AuthManager) {
                 _samlAuthManager = (SAML2AuthManager) authManager;
             }
         }
diff --git a/server/src/com/cloud/api/auth/APIAuthenticationManagerImpl.java b/server/src/com/cloud/api/auth/APIAuthenticationManagerImpl.java
@@ -69,7 +69,12 @@ public List<Class<?>> getCommands() {
         cmdList.add(DefaultLoginAPIAuthenticatorCmd.class);
         cmdList.add(DefaultLogoutAPIAuthenticatorCmd.class);
         for (PluggableAPIAuthenticator apiAuthenticator: _apiAuthenticators) {
-            cmdList.addAll(apiAuthenticator.getAuthCommands());
+            List<Class<?>> commands = apiAuthenticator.getAuthCommands();
+            if (commands != null) {
+                cmdList.addAll(commands);
+            } else {
+                s_logger.warn("API Authenticator returned null api commands:" + apiAuthenticator.getName());
+            }
         }
         return cmdList;
     }

Original file line number	Diff line number	Diff line change
`@@ -191,7 +191,7 @@ public APIAuthenticationType getAPIType() {`
`191`	`191`	`@Override`
`192`	`192`	`public void setAuthenticators(List<PluggableAPIAuthenticator> authenticators) {`
`193`	`193`	`for (PluggableAPIAuthenticator authManager: authenticators) {`
`194`		`- if (authManager instanceof SAML2AuthManager) {`
	`194`	`+ if (authManager != null && authManager instanceof SAML2AuthManager) {`
`195`	`195`	`_samlAuthManager = (SAML2AuthManager) authManager;`
`196`	`196`	`}`
`197`	`197`	`}`
Original file line number	Diff line number	Diff line change
`@@ -292,7 +292,7 @@ public APIAuthenticationType getAPIType() {`
`292`	`292`	`@Override`
`293`	`293`	`public void setAuthenticators(List<PluggableAPIAuthenticator> authenticators) {`
`294`	`294`	`for (PluggableAPIAuthenticator authManager: authenticators) {`
`295`		`- if (authManager instanceof SAML2AuthManager) {`
	`295`	`+ if (authManager != null && authManager instanceof SAML2AuthManager) {`
`296`	`296`	`_samlAuthManager = (SAML2AuthManager) authManager;`
`297`	`297`	`}`
`298`	`298`	`}`
Original file line number	Diff line number	Diff line change
`@@ -158,7 +158,7 @@ public APIAuthenticationType getAPIType() {`
`158`	`158`	`@Override`
`159`	`159`	`public void setAuthenticators(List<PluggableAPIAuthenticator> authenticators) {`
`160`	`160`	`for (PluggableAPIAuthenticator authManager: authenticators) {`
`161`		`- if (authManager instanceof SAML2AuthManager) {`
	`161`	`+ if (authManager != null && authManager instanceof SAML2AuthManager) {`
`162`	`162`	`_samlAuthManager = (SAML2AuthManager) authManager;`
`163`	`163`	`}`
`164`	`164`	`}`
Original file line number	Diff line number	Diff line change
`@@ -69,7 +69,12 @@ public List<Class<?>> getCommands() {`
`69`	`69`	`cmdList.add(DefaultLoginAPIAuthenticatorCmd.class);`
`70`	`70`	`cmdList.add(DefaultLogoutAPIAuthenticatorCmd.class);`
`71`	`71`	`for (PluggableAPIAuthenticator apiAuthenticator: _apiAuthenticators) {`
`72`		`- cmdList.addAll(apiAuthenticator.getAuthCommands());`
	`72`	`+ List<Class<?>> commands = apiAuthenticator.getAuthCommands();`
	`73`	`+ if (commands != null) {`
	`74`	`+ cmdList.addAll(commands);`
	`75`	`+ } else {`
	`76`	`+ s_logger.warn("API Authenticator returned null api commands:" + apiAuthenticator.getName());`
	`77`	`+ }`
`73`	`78`	`}`
`74`	`79`	`return cmdList;`
`75`	`80`	`}`