RFR 8050427 LoginContext tests to cover JDK-4703361

[Amanda Jiang]

Hi Max,

Thanks for your suggestions!  Two places were fixed and I also edited 
codes a little bit to make test clear.
Please check latest webrev: 
http://cr.openjdk.java.net/~amjiang/8050427/webrev.03/

Thanks,
Amanda

On 9/22/15, 6:45 PM, Weijun Wang wrote:
> OK, the code is much easier to understand now.
>
> In some places, there are modern and simple ways to code. For example, 
> in SmartLoginModule.java,
>
>  170         // verify the username/password
>  171         boolean usernameCorrect = false;
>  172         boolean passwordCorrect = true;
>  173         if (username.equals(myUsername)) {
>  174             usernameCorrect = true;
>  175         }
>  176
>  177         if (!usernameCorrect || password.length != 13) {
>  178             passwordCorrect = false;
>  179         }
>  180
>  181         for (int i = 0; i < password.length; i++) {
>  182             if (password[i] != myPassword[i]) {
>  183                 passwordCorrect = false;
>  184             }
>  185
>
> can be as simple as
>
>   if (username.equals(myUsername)
>       && Arrays.equals(password, myPassword))
>
> Also, in MyConfiguration.java:
>
>   52         AppConfigurationEntry.LoginModuleControlFlag[] flags
>   53                 = new 
> AppConfigurationEntry.LoginModuleControlFlag[2];
>   54         flags[0] = optionOrder ? OPTIONAL : REQUIRED;
>   55         flags[1] = optionOrder ? SUFFICIENT : REQUIRED;
>   56         ptAE[0] = new AppConfigurationEntry("SmartLoginModule",
>   57                 flags[0],
>   58                 map);
>   59         ptAE[1] = new AppConfigurationEntry("DummyLoginModule",
>   60                 flags[1],
>   61                 map);
>
> It seems unnecessary to introduce the flags variable.
>
> Thanks
> Max
>
> On 09/22/2015 11:49 PM, Amanda Jiang wrote:
>> Hi Max,
>>
>> Thanks for your comments ,please check updated webrev and my replies
>> inline.
>> webrev: http://cr.openjdk.java.net/~amjiang/8050427/webrev.02/
>>
>> Regards,
>> Amanda
>>
>> On 8/17/15, 12:29 AM, Weijun Wang wrote:
>>> There are something I don't understand.
>>>
>>> MyConfiguration.java:
>>>
>>> - optionOrder: Is it possible to make this an argument of the
>>> constructor?
>>      Fixed, made this an argument of the constructor.
>>>
>>> - getConfiguration/setConfiguration: If these are useless, why adding
>>> these methods?
>>      Removed useless methods.
>>>
>>> SmartLoginModule.java:
>>>
>>> - initialize: Why don't you use the callbackHandler argument?
>>      See below.
>>>
>>> - shouldSucceed: It is always very confusing to make a field
>>> accessible from outside a class.
>>      Originally the test tried to test with different password ,
>> "shouldSucceed" is used to control which password will be set.
>>      MycallbackHandler is called in initialize( ) method and
>> "shouldSucceed" is parsed as an argument to control which password would
>> be set to "PasswdCallback"
>>
>>   123         this.callbackHandler = new MyCallbackHandler(myUsername,
>> myPassword,
>>   124                 shouldSucceed);
>>
>>   341     public MyCallbackHandler(String username, char[] password,
>> boolean action) {
>>   342         super();
>>   343         userName = username;
>>   344         this.password = password;
>>   345         this.action = action;
>>   346     }
>>
>>   361                 PasswordCallback pc = (PasswordCallback) callback;
>>   362                 if (action) {
>>   363                     pc.setPassword(password);
>>   364                 } else {
>>   365                     pc.setPassword(wrongpd);
>>   366                 }
>>
>>      I agree with you this may be confusing, so I simplify this test,
>> "shouldSucceed" value is remove, please check updated webrev.
>>>
>>> DynamicConfigurationTest.java:
>>>
>>> - test: why the if checks are based on both isNegative and success?
>>> Why not only on isNegative? If you want to test 2 stages (initialize
>>> and login), you can provide 2 isNegative flags.
>>           "success" was for controlling correct/wrong passwords,
>> "isNegative" was for checking if exception is expected.
>>            Tests are re-organized , so I do not use these two flags
>> now,  please check updated webrev.
>>>
>>> Thanks
>>> Max
>>>
>>>
>>> On 08/17/2015 01:39 PM, Amanda Jiang wrote:
>>>> Hi All,
>>>>
>>>> Please be free to review these new tests for Dynamic configuration of
>>>> authentication modules.
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8050427
>>>> webrev:http://cr.openjdk.java.net/~amjiang/8050427/webrev.01/
>>>>
>>>>
>>>> Thanks,
>>>> Amanda
>>>>
>>>>
>>
>>
>>