Attachment 216745 Details for Bug 379787 – patch to add test "API" to prohibit access to secure storage

[patch] patch to add test "API" to prohibit access to secure storage

379787a.txt (text/plain), 13.39 KB, created by David Dykstal

on 2012-06-03 23:37:19 EDT

(hide)

Description:

Filename:

MIME Type:

Creator: David Dykstal

Created: 2012-06-03 23:37:19 EDT

Size: 13.39 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.rse.core
>Index: src/org/eclipse/rse/core/PasswordPersistenceManager.java
>===================================================================
>RCS file: /cvsroot/tools/org.eclipse.tm.rse/plugins/org.eclipse.rse.core/src/org/eclipse/rse/core/PasswordPersistenceManager.java,v
>retrieving revision 1.19
>diff -u -r1.19 PasswordPersistenceManager.java
>--- src/org/eclipse/rse/core/PasswordPersistenceManager.java	24 Mar 2012 16:49:04 -0000	1.19
>+++ src/org/eclipse/rse/core/PasswordPersistenceManager.java	4 Jun 2012 03:36:41 -0000
>@@ -20,6 +20,7 @@
>  * Martin Oberhuber (Wind River) - [cleanup] Add API "since" Javadoc tags
>  * David Dykstal (IBM) - [210474] Deny save password function missing
>  * David Dykstal (IBM) - [225320] Use equinox secure storage for passwords
>+ * David Dykstal (IBM) - [379787] add test API to prevent secure storage access while running JUnit tests
>  ********************************************************************************/
> 
> package org.eclipse.rse.core;
>@@ -302,6 +303,13 @@
> 
> 	private String mapLocation = null;
> 	private RegisteredSystemType[] systemTypes;
>+	
>+	/**
>+	 * Used to allow access to secure preference nodes.
>+	 * The default is true meaning that access is allowed.
>+	 * Used for testing only.
>+	 */
>+	private boolean isSecureStorageAccessAllowed = true;
> 
> 	/**
> 	 * Singleton so this is a private constructor
>@@ -382,7 +390,9 @@
> 
> 	/**
> 	 * Returns the preferences node that matches the system type.
>-	 * It will not return null but will create the node if it does not exist.
>+	 * It will return null only if secure storage access is not allowed.
>+	 * If secure storage access is allowed then
>+	 * it will not return null but will create the node if it does not exist.
> 	 * If the node does not previous exist then an attempt will be made
> 	 * to migrate the values from the old map form to this newly created node
> 	 * of the secure preferences tree.
>@@ -390,14 +400,16 @@
> 	 * @return the matching secure preferences node. 
> 	 */
> 	private ISecurePreferences getNode(IRSESystemType systemType) {
>-		String id = systemType.getId();
>-		ISecurePreferences preferences = SecurePreferencesFactory.getDefault();
>-		ISecurePreferences rseNode = preferences.node("org.eclipse.rse.core.security"); //$NON-NLS-1$
> 		ISecurePreferences systemTypeNode = null;
>-		if (!rseNode.nodeExists(id)) {
>-			migrateMap(rseNode, id);
>+		if (isSecureStorageAccessAllowed) {			
>+			String id = systemType.getId();
>+			ISecurePreferences preferences = SecurePreferencesFactory.getDefault();
>+			ISecurePreferences rseNode = preferences.node("org.eclipse.rse.core.security"); //$NON-NLS-1$
>+			if (!rseNode.nodeExists(id)) {
>+				migrateMap(rseNode, id);
>+			}
>+			systemTypeNode = rseNode.node(id);
> 		}
>-		systemTypeNode = rseNode.node(id);
> 		return systemTypeNode;
> 	}
> 
>@@ -473,20 +485,24 @@
> 	 * @return the number of passwords removed.
> 	 */
> 	private int removePassword(IRSESystemType systemType, String hostName, String userId) {
>+		int passwordsRemoved = 0;
> 		ISecurePreferences passwords = getNode(systemType);
>-		boolean respectCase = isUserIDCaseSensitive(systemType);
>-		String keys[] = getMatchingKeys(passwords.keys(), hostName, userId, respectCase, false);
>-		if (keys.length == 0) {
>-			keys = getMatchingKeys(passwords.keys(), hostName, userId, respectCase, true);
>-		}
>-		for (int i = 0; i < keys.length; i++) {
>-			String key = keys[i];
>-			basicRemove(passwords, key);
>-		}
>-		if (keys.length > 0) {
>-			basicSave(passwords);
>+		if (passwords != null) {
>+			boolean respectCase = isUserIDCaseSensitive(systemType);
>+			String keys[] = getMatchingKeys(passwords.keys(), hostName, userId, respectCase, false);
>+			if (keys.length == 0) {
>+				keys = getMatchingKeys(passwords.keys(), hostName, userId, respectCase, true);
>+			}
>+			for (int i = 0; i < keys.length; i++) {
>+				String key = keys[i];
>+				basicRemove(passwords, key);
>+			}
>+			if (keys.length > 0) {
>+				basicSave(passwords);
>+			}
>+			passwordsRemoved = keys.length;
> 		}
>-		return keys.length;
>+		return passwordsRemoved;
> 	}
> 	
> 	/**
>@@ -502,14 +518,16 @@
> 	private String findPassword(IRSESystemType systemType, String hostName, String userId) {
> 		String password = null;
> 		ISecurePreferences passwords = getNode(systemType);
>-		boolean respectCase = isUserIDCaseSensitive(systemType);
>-		String keys[] = getMatchingKeys(passwords.keys(), hostName, userId, respectCase, false);
>-		if (keys.length == 0) {
>-			keys = getMatchingKeys(passwords.keys(), hostName, userId, respectCase, true);
>-		}
>-		if (keys.length > 0) {
>-			String key = keys[0];
>-			password = basicGet(passwords, key);
>+		if (passwords != null) {
>+			boolean respectCase = isUserIDCaseSensitive(systemType);
>+			String keys[] = getMatchingKeys(passwords.keys(), hostName, userId, respectCase, false);
>+			if (keys.length == 0) {
>+				keys = getMatchingKeys(passwords.keys(), hostName, userId, respectCase, true);
>+			}
>+			if (keys.length > 0) {
>+				String key = keys[0];
>+				password = basicGet(passwords, key);
>+			}
> 		}
> 		return password;
> 	}
>@@ -525,9 +543,11 @@
> 	 */
> 	private void updatePassword(IRSESystemType systemType, String hostName, String userId, String password) {
> 		ISecurePreferences passwords = getNode(systemType);
>-		String key = getKey(hostName, userId);
>-		basicPut(passwords, key, password);
>-		basicSave(passwords);
>+		if (passwords != null) {
>+			String key = getKey(hostName, userId);
>+			basicPut(passwords, key, password);
>+			basicSave(passwords);
>+		}
> 	}
> 	
> 	/**
>@@ -546,6 +566,37 @@
> 	}
> 	
> 	/**
>+	 * Disables secure storage access from this {@link PasswordPersistenceManager}.
>+	 * This is not API but for testing purposes only.
>+	 * @noreference This method is not intended to be referenced by clients.
>+	 * @since org.eclipse.rse.core 3.4
>+	 */
>+	public void disableSecureStorageAccess() {
>+		isSecureStorageAccessAllowed = false;
>+	}
>+	
>+	/**
>+	 * Enable secure storage access from this {@link PasswordPersistenceManager}.
>+	 * This is not API but for testing purposes only.
>+	 * @noreference This method is not intended to be referenced by clients.
>+	 * @since org.eclipse.rse.core 3.4
>+	 */
>+	public void enableSecureStorageAccess() {
>+		isSecureStorageAccessAllowed = true;
>+	}
>+	
>+	/**
>+	 * Test if secure storage access is allowed.
>+	 * This is not API but for testing purposes only.
>+	 * @noreference This method is not intended to be referenced by clients.
>+	 * @since org.eclipse.rse.core 3.4
>+	 * @return true if it is allowed
>+	 */
>+	public boolean isSecureStorageAccessAllowed() {
>+		return isSecureStorageAccessAllowed;
>+	}
>+	
>+	/**
> 	 * Add a password to the password database.
> 	 * This will not update the entry for the default system type
> 	 * @param info The signon information to store
>@@ -729,13 +780,15 @@
> 		for (int i = 0; i < systemTypes.length; i++) {
> 			IRSESystemType systemType = systemTypes[i];
> 			ISecurePreferences node = getNode(systemType);
>-			String[] keys = node.keys();
>-			for (int j = 0; j < keys.length; j++) {
>-				String key = keys[j];
>-				String hostName = getHostNameFromKey(key);
>-				String userId = getUserIdFromKey(key);
>-				SystemSignonInformation info = new SystemSignonInformation(hostName, userId, systemType);
>-				savedUserIDs.add(info);
>+			if (node != null) {
>+				String[] keys = node.keys();
>+				for (int j = 0; j < keys.length; j++) {
>+					String key = keys[j];
>+					String hostName = getHostNameFromKey(key);
>+					String userId = getUserIdFromKey(key);
>+					SystemSignonInformation info = new SystemSignonInformation(hostName, userId, systemType);
>+					savedUserIDs.add(info);
>+				}				
> 			}
> 		}
> 		return savedUserIDs;
>#P org.eclipse.rse.tests
>Index: src/org/eclipse/rse/tests/core/passwords/PasswordsTest.java
>===================================================================
>RCS file: /cvsroot/tools/org.eclipse.tm.rse/tests/org.eclipse.rse.tests/src/org/eclipse/rse/tests/core/passwords/PasswordsTest.java,v
>retrieving revision 1.6
>diff -u -r1.6 PasswordsTest.java
>--- src/org/eclipse/rse/tests/core/passwords/PasswordsTest.java	24 May 2012 15:46:49 -0000	1.6
>+++ src/org/eclipse/rse/tests/core/passwords/PasswordsTest.java	4 Jun 2012 03:36:42 -0000
>@@ -6,6 +6,7 @@
>  *
>  * Contributors:
>  * David Dykstal (IBM) - [210474] Deny save password function missing
>+ * David Dykstal (IBM) - [379787] update tests to handle lack of secure storage access
>  ********************************************************************************/
> 
> package org.eclipse.rse.tests.core.passwords;
>@@ -19,10 +20,9 @@
> 
> /**
>  * Tests for {@link PasswordPersistenceManager}.
>- * Test various aspects of mnemonic generation and assignment.
>  */
> public class PasswordsTest extends RSECoreTestCase {
>-
>+	
> 	/* (non-Javadoc)
> 	 * @see org.eclipse.rse.tests.core.RSECoreTestCase#setUp()
> 	 */
>@@ -56,14 +56,24 @@
> 		int result = ppm.add(info, true, true);
> 		assertEquals("result of add was not what was expected", PasswordPersistenceManager.RC_OK, result);
> 		SystemSignonInformation returnedInfo = ppm.find(systemType, hostAddress, userId);
>-		assertEquals("passwords are not equal", password, returnedInfo.getPassword());
>-		assertEquals("system type not what was expected", systemType, returnedInfo.getSystemType());
>+		if (ppm.isSecureStorageAccessAllowed()) {
>+			assertNotNull(returnedInfo);
>+			assertEquals("passwords are not equal", password, returnedInfo.getPassword());
>+			assertEquals("system type not what was expected", systemType, returnedInfo.getSystemType());
>+		} else {
>+			assertNull(returnedInfo);
>+		}
> 
> 		// remove the password for the system type
> 		ppm.remove(systemType, hostAddress, userId); // removes only the entry for the system type
> 		returnedInfo = ppm.find(systemType, hostAddress, userId, true);
>-		assertEquals("passwords are not equal", password, returnedInfo.getPassword());
>-		assertEquals("system type not what was expected", defaultSystemType, returnedInfo.getSystemType());
>+		if (ppm.isSecureStorageAccessAllowed()) {
>+			assertNotNull(returnedInfo);
>+			assertEquals("passwords are not equal", password, returnedInfo.getPassword());
>+			assertEquals("system type not what was expected", defaultSystemType, returnedInfo.getSystemType());
>+		} else {
>+			assertNull(returnedInfo);
>+		}
> 		returnedInfo = ppm.find(systemType, hostAddress, userId, false);
> 		assertNull("signon info was found but should not be", returnedInfo);
> 
>@@ -78,8 +88,13 @@
> 		result = ppm.add(info, true, false);
> 		assertEquals("result of add was not what was expected", PasswordPersistenceManager.RC_OK, result);
> 		returnedInfo = ppm.find(systemType, hostAddress, userId);
>-		assertEquals("passwords are not equal", password, returnedInfo.getPassword());
>-		assertEquals("system type not what was expected", systemType, returnedInfo.getSystemType());
>+		if (ppm.isSecureStorageAccessAllowed()) {
>+			assertNotNull(returnedInfo);
>+			assertEquals("passwords are not equal", password, returnedInfo.getPassword());
>+			assertEquals("system type not what was expected", systemType, returnedInfo.getSystemType());
>+		} else {
>+			assertNull(returnedInfo);
>+		}
> 		returnedInfo = ppm.find(defaultSystemType, hostAddress, userId);
> 		assertNull("signon info was found but should not be", returnedInfo);
> 
>@@ -109,8 +124,13 @@
> 		int result = ppm.add(info, true, true);
> 		assertEquals("result of add was not what was expected", PasswordPersistenceManager.RC_OK, result);
> 		SystemSignonInformation returnedInfo = ppm.find(systemType, hostAddress, userId);
>-		assertEquals("passwords are not equal", password, returnedInfo.getPassword());
>-		assertEquals("system type not what was expected", systemType, returnedInfo.getSystemType());
>+		if (ppm.isSecureStorageAccessAllowed()) {
>+			assertNotNull(returnedInfo);
>+			assertEquals("passwords are not equal", password, returnedInfo.getPassword());
>+			assertEquals("system type not what was expected", systemType, returnedInfo.getSystemType());
>+		} else {
>+			assertNull(returnedInfo);
>+		}
> 
> 		// change the preference for this system type, should erase all the passwords, including the default system type
> 		RSEPreferencesManager.setDenyPasswordSave(systemType, hostAddress, true);
>@@ -176,14 +196,18 @@
> 		PasswordPersistenceManager ppm = PasswordPersistenceManager.getInstance();
> 		ppm.add(new SystemSignonInformation("LOUDHOST.mycompany.com", "thatguy", "abc", systemType), true, false);
> 		SystemSignonInformation foundInfo = ppm.find(systemType, "LOUDHOST.mycompany.com", "thatguy");
>-		assertNotNull(foundInfo);
>-		assertEquals(foundInfo.getPassword(), "abc");
>-		foundInfo = ppm.find(systemType, "loudhost.mycompany.com", "thatguy");
>-		assertNotNull(foundInfo);
>-		assertEquals(foundInfo.getPassword(), "abc");
>-		foundInfo = ppm.find(systemType, "loudhost.MyCompany.com", "thatguy");
>-		assertNotNull(foundInfo);
>-		assertEquals(foundInfo.getPassword(), "abc");
>+		if (ppm.isSecureStorageAccessAllowed()) {
>+			assertNotNull(foundInfo);
>+			assertEquals(foundInfo.getPassword(), "abc");
>+			foundInfo = ppm.find(systemType, "loudhost.mycompany.com", "thatguy");
>+			assertNotNull(foundInfo);
>+			assertEquals(foundInfo.getPassword(), "abc");
>+			foundInfo = ppm.find(systemType, "loudhost.MyCompany.com", "thatguy");
>+			assertNotNull(foundInfo);
>+			assertEquals(foundInfo.getPassword(), "abc");
>+		} else {
>+			assertNull(foundInfo);
>+		}
> 	}
> 	
> 	public void testBadArgs() {

Actions: View | Diff

Attachments on bug 379787: 216745 | 225643