From 97b8f590e1ccfaac4bd4a3d12144ec11e8a629d7 Mon Sep 17 00:00:00 2001 From: Andrew Stadler Date: Thu, 12 Aug 2010 14:56:32 -0700 Subject: [PATCH] Clear password related policies in PolicySet when p/w not required Merge from master of c263810b08943541135a24e2b7520692152455cc Bug: 2883736 Change-Id: Iec4ed0e320d67aee8a89092ac650c0960540057b --- src/com/android/email/SecurityPolicy.java | 39 +++++++++++-------- .../android/email/SecurityPolicyTests.java | 31 +++++++++------ 2 files changed, 42 insertions(+), 28 deletions(-) diff --git a/src/com/android/email/SecurityPolicy.java b/src/com/android/email/SecurityPolicy.java index bacb39b53..35613ece0 100644 --- a/src/com/android/email/SecurityPolicy.java +++ b/src/com/android/email/SecurityPolicy.java @@ -453,23 +453,30 @@ public class SecurityPolicy { */ public PolicySet(int minPasswordLength, int passwordMode, int maxPasswordFails, int maxScreenLockTime, boolean requireRemoteWipe) throws IllegalArgumentException { - // Check against hard limits - // EAS doesn't generate values outside these limits anyway - if (minPasswordLength > PASSWORD_LENGTH_MAX) { - throw new IllegalArgumentException("password length"); + // If we're not enforcing passwords, make sure we clean up related values, since EAS + // can send non-zero values for any or all of these + if (passwordMode == PASSWORD_MODE_NONE) { + maxPasswordFails = 0; + maxScreenLockTime = 0; + minPasswordLength = 0; + } else { + if ((passwordMode != PASSWORD_MODE_SIMPLE) && + (passwordMode != PASSWORD_MODE_STRONG)) { + throw new IllegalArgumentException("password mode"); + } + // The next value has a hard limit which cannot be supported if exceeded. + if (minPasswordLength > PASSWORD_LENGTH_MAX) { + throw new IllegalArgumentException("password length"); + } + // This value can be reduced (which actually increases security) if necessary + if (maxPasswordFails > PASSWORD_MAX_FAILS_MAX) { + maxPasswordFails = PASSWORD_MAX_FAILS_MAX; + } + // This value can be reduced (which actually increases security) if necessary + if (maxScreenLockTime > SCREEN_LOCK_TIME_MAX) { + maxScreenLockTime = SCREEN_LOCK_TIME_MAX; + } } - if (passwordMode < PASSWORD_MODE_NONE || passwordMode > PASSWORD_MODE_STRONG) { - throw new IllegalArgumentException("password mode"); - } - // This value can be reduced (which actually increases security) if necessary - if (maxPasswordFails > PASSWORD_MAX_FAILS_MAX) { - maxPasswordFails = PASSWORD_MAX_FAILS_MAX; - } - // This value can be reduced (which actually increases security) if necessary - if (maxScreenLockTime > SCREEN_LOCK_TIME_MAX) { - maxScreenLockTime = SCREEN_LOCK_TIME_MAX; - } - mMinPasswordLength = minPasswordLength; mPasswordMode = passwordMode; mMaxPasswordFails = maxPasswordFails; diff --git a/tests/src/com/android/email/SecurityPolicyTests.java b/tests/src/com/android/email/SecurityPolicyTests.java index 1e25050ca..5924bceba 100644 --- a/tests/src/com/android/email/SecurityPolicyTests.java +++ b/tests/src/com/android/email/SecurityPolicyTests.java @@ -95,7 +95,7 @@ public class SecurityPolicyTests extends ProviderTestCase2 { // We know that EMPTY_POLICY_SET doesn't generate an Exception or we wouldn't be here // Try some illegal parameters try { - new PolicySet(100, PolicySet.PASSWORD_MODE_NONE, 0, 0, false); + new PolicySet(100, PolicySet.PASSWORD_MODE_SIMPLE, 0, 0, false); fail("Too-long password allowed"); } catch (IllegalArgumentException e) { } @@ -104,12 +104,19 @@ public class SecurityPolicyTests extends ProviderTestCase2 { fail("Illegal password mode allowed"); } catch (IllegalArgumentException e) { } - PolicySet ps = new PolicySet(0, PolicySet.PASSWORD_MODE_NONE, 0, + PolicySet ps = new PolicySet(0, PolicySet.PASSWORD_MODE_SIMPLE, 0, PolicySet.SCREEN_LOCK_TIME_MAX + 1, false); assertEquals(PolicySet.SCREEN_LOCK_TIME_MAX, ps.getMaxScreenLockTime()); - ps = new PolicySet(0, PolicySet.PASSWORD_MODE_NONE, + ps = new PolicySet(0, PolicySet.PASSWORD_MODE_SIMPLE, PolicySet.PASSWORD_MAX_FAILS_MAX + 1, 0, false); assertEquals(PolicySet.PASSWORD_MAX_FAILS_MAX, ps.getMaxPasswordFails()); + // All password related fields should be zero when password mode is NONE + // Illegal values for these fields should be ignored + ps = new PolicySet(999/*length*/, PolicySet.PASSWORD_MODE_NONE, + 999/*fails*/, 9999/*screenlock*/, false); + assertEquals(0, ps.mMinPasswordLength); + assertEquals(0, ps.mMaxScreenLockTime); + assertEquals(0, ps.mMaxPasswordFails); } /** @@ -165,7 +172,7 @@ public class SecurityPolicyTests extends ProviderTestCase2 { // pw length and pw mode - max logic - will *not* change because smaller #s here // fail count and lock timer - min logic - will change because smaller #s here // wipe required - OR logic - will change here because true - PolicySet p5in = new PolicySet(4, PolicySet.PASSWORD_MODE_NONE, 5, 6, true); + PolicySet p5in = new PolicySet(4, PolicySet.PASSWORD_MODE_SIMPLE, 5, 6, true); Account a5 = ProviderTestUtils.setupAccount("sec-5", false, mMockContext); p5in.writeAccount(a5, null, true, mMockContext); PolicySet p5out = sp.computeAggregatePolicy(); @@ -202,9 +209,9 @@ public class SecurityPolicyTests extends ProviderTestCase2 { */ @SmallTest public void testFieldIsolation() { - PolicySet p = new PolicySet(PolicySet.PASSWORD_LENGTH_MAX, 0, 0, 0, false); + PolicySet p = new PolicySet(PolicySet.PASSWORD_LENGTH_MAX, PolicySet.PASSWORD_MODE_SIMPLE, + 0, 0, false); assertEquals(PolicySet.PASSWORD_LENGTH_MAX, p.mMinPasswordLength); - assertEquals(0, p.mPasswordMode); assertEquals(0, p.mMaxPasswordFails); assertEquals(0, p.mMaxScreenLockTime); assertFalse(p.mRequireRemoteWipe); @@ -212,27 +219,27 @@ public class SecurityPolicyTests extends ProviderTestCase2 { p = new PolicySet(0, PolicySet.PASSWORD_MODE_STRONG, 0, 0, false); assertEquals(0, p.mMinPasswordLength); assertEquals(PolicySet.PASSWORD_MODE_STRONG, p.mPasswordMode); + assertEquals(0, p.mMinPasswordLength); assertEquals(0, p.mMaxPasswordFails); assertEquals(0, p.mMaxScreenLockTime); assertFalse(p.mRequireRemoteWipe); - p = new PolicySet(0, 0, PolicySet.PASSWORD_MAX_FAILS_MAX, 0, false); + p = new PolicySet(0, PolicySet.PASSWORD_MODE_SIMPLE, PolicySet.PASSWORD_MAX_FAILS_MAX, 0, + false); assertEquals(0, p.mMinPasswordLength); - assertEquals(0, p.mPasswordMode); assertEquals(PolicySet.PASSWORD_MAX_FAILS_MAX, p.mMaxPasswordFails); assertEquals(0, p.mMaxScreenLockTime); assertFalse(p.mRequireRemoteWipe); - p = new PolicySet(0, 0, 0, PolicySet.SCREEN_LOCK_TIME_MAX, false); + p = new PolicySet(0, PolicySet.PASSWORD_MODE_SIMPLE, 0, PolicySet.SCREEN_LOCK_TIME_MAX, + false); assertEquals(0, p.mMinPasswordLength); - assertEquals(0, p.mPasswordMode); assertEquals(0, p.mMaxPasswordFails); assertEquals(PolicySet.SCREEN_LOCK_TIME_MAX, p.mMaxScreenLockTime); assertFalse(p.mRequireRemoteWipe); - p = new PolicySet(0, 0, 0, 0, true); + p = new PolicySet(0, PolicySet.PASSWORD_MODE_NONE, 0, 0, true); assertEquals(0, p.mMinPasswordLength); - assertEquals(0, p.mPasswordMode); assertEquals(0, p.mMaxPasswordFails); assertEquals(0, p.mMaxScreenLockTime); assertTrue(p.mRequireRemoteWipe);