[sip-comm-dev] Password Storage: Week 6


#1

Hi all,

I think the UI for the master password is ready. I also did some
refactoring/additions to the credentials service. Everything seems to
work, but it's possible that there are some hidden bugs. It would be
great to hear some feedback and maybe go over the code with the
mentors while it's fresh in my mind. The code is commited into my
branch.

Cheers,
Dmitri

···

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#2

Sure, the screenshots are worth a thousand words.
Some comments about them:
1. When you tick the "Use a master password" checkbox this dialog pops
up. It's the same dialog used for changing, only the current password
field is disabled. The checkbox and change button remain disabled
until the password is set correctly.
2. The change dialog, this time current password is enabled. I also
added confirmation popups.
3. This is how the master password input looks like in all places
where it's needed. I had a separate dialog for it, but then decided to
just use the existing simple popup.
4. The saved passwords table where you can view and remove passwords.
5. Same table with the show passwords button clicked. If a master
password is set the input popup (3) is presented first before showing
the hidden passwords column.
When the "Use a master password" checkbox is unticked to remove the
MP, the input popup is also presented.

Cheers,
Dmitri

···

On Wed, Jun 23, 2010 at 1:39 AM, Emil Ivov <emcho@sip-communicator.org> wrote:

Hey Dmitri,

On 22 juin 2010, at 23:46, Dmitri Melnikov <dmitri807@gmail.com> wrote:

Hi all,

I think the UI for the master password is ready. I also did some
refactoring/additions to the credentials service.

Cool! That's good to hear!

You may also want to send a couple of screenshots here for those that won't
have the time to checkout your branch.

Cheers,
Emil

Everything seems to
work, but it's possible that there are some hidden bugs. It would be
great to hear some feedback and maybe go over the code with the
mentors while it's fresh in my mind. The code is commited into my
branch.

Cheers,
Dmitri

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#3

Hi Dmitri, all,

Thank you for this complete update, I'll have a look at your source
and send you some comments as soon as possible.

From the screenshots you've sent, everything seems very good. However,

when the user changes the master password, it seems to me that the
popup message indicating that the password has been changed could be
annoying. What do you all think?

Also I've seen in Firefox a password strength indicator, wouldn't it
be a good idea to add one when the user type the master password? It
could make the user a bit more concerned about its security, isn't it?
A quick google search gave me those tips to evaluate a password
strength: http://justwild.us/examples/password/

Cheers,
Ben

···

On Wed, Jun 23, 2010 at 11:34, Dmitri Melnikov <dmitri807@gmail.com> wrote:

Sure, the screenshots are worth a thousand words.
Some comments about them:
1. When you tick the "Use a master password" checkbox this dialog pops
up. It's the same dialog used for changing, only the current password
field is disabled. The checkbox and change button remain disabled
until the password is set correctly.
2. The change dialog, this time current password is enabled. I also
added confirmation popups.
3. This is how the master password input looks like in all places
where it's needed. I had a separate dialog for it, but then decided to
just use the existing simple popup.
4. The saved passwords table where you can view and remove passwords.
5. Same table with the show passwords button clicked. If a master
password is set the input popup (3) is presented first before showing
the hidden passwords column.
When the "Use a master password" checkbox is unticked to remove the
MP, the input popup is also presented.

Cheers,
Dmitri

On Wed, Jun 23, 2010 at 1:39 AM, Emil Ivov <emcho@sip-communicator.org> wrote:

Hey Dmitri,

On 22 juin 2010, at 23:46, Dmitri Melnikov <dmitri807@gmail.com> wrote:

Hi all,

I think the UI for the master password is ready. I also did some
refactoring/additions to the credentials service.

Cool! That's good to hear!

You may also want to send a couple of screenshots here for those that won't
have the time to checkout your branch.

Cheers,
Emil

Everything seems to
work, but it's possible that there are some hidden bugs. It would be
great to hear some feedback and maybe go over the code with the
mentors while it's fresh in my mind. The code is commited into my
branch.

Cheers,
Dmitri

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#4

Hello Dmitri, all,

I had a thorough look at the implementation, here are my thoughts.

From a functional perspective, well, it "just works" and I like it a lot.

Some comments/remarks about the code

1. Mostly followed the SIP Communicator coding convention (proper
comments, both code and javadoc are limited at column 80, e.t.c.)!
2. Code is internationalization ready, no hardcoded messages!
3. As a general practice, we use package imports (package.*) and we do
not import classes themselves.
4. AESCrypto.getKey method, really necessary? It could be incorporated
in the constructor.

Off course, the last 2 comments are of minuscule importance, the overall
code quality seems to be good and, when the test cases are furnished, I
think we will have production ready code.

What's exactly left to implement? Ben's idea of a password strength
indicator is a nice feature to have and unit tests are obligatory, off
course. Anything else? I'm starting to think of project number 2..

Have a nice day everyone :slight_smile:

Cheers,
George

···

On 06/23/2010 11:34 AM, Dmitri Melnikov wrote:

Sure, the screenshots are worth a thousand words.
Some comments about them:
1. When you tick the "Use a master password" checkbox this dialog pops
up. It's the same dialog used for changing, only the current password
field is disabled. The checkbox and change button remain disabled
until the password is set correctly.
2. The change dialog, this time current password is enabled. I also
added confirmation popups.
3. This is how the master password input looks like in all places
where it's needed. I had a separate dialog for it, but then decided to
just use the existing simple popup.
4. The saved passwords table where you can view and remove passwords.
5. Same table with the show passwords button clicked. If a master
password is set the input popup (3) is presented first before showing
the hidden passwords column.
When the "Use a master password" checkbox is unticked to remove the
MP, the input popup is also presented.

Cheers,
Dmitri

On Wed, Jun 23, 2010 at 1:39 AM, Emil Ivov <emcho@sip-communicator.org> wrote:

Hey Dmitri,

On 22 juin 2010, at 23:46, Dmitri Melnikov <dmitri807@gmail.com> wrote:

Hi all,

I think the UI for the master password is ready. I also did some
refactoring/additions to the credentials service.

Cool! That's good to hear!

You may also want to send a couple of screenshots here for those that won't
have the time to checkout your branch.

Cheers,
Emil

Everything seems to
work, but it's possible that there are some hidden bugs. It would be
great to hear some feedback and maybe go over the code with the
mentors while it's fresh in my mind. The code is commited into my
branch.

Cheers,
Dmitri

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#5

Hi Dmitri, all,

Thank you for this complete update, I'll have a look at your source
and send you some comments as soon as possible.

Thanks, looking forward to it.

From the screenshots you've sent, everything seems very good. However,
when the user changes the master password, it seems to me that the
popup message indicating that the password has been changed could be
annoying. What do you all think?

Firefox does it (at least my 3.5.7 version). I don't think the users
will change it very often, so maybe it's not that annoying. But in any
case it's easy to remove.

Also I've seen in Firefox a password strength indicator, wouldn't it
be a good idea to add one when the user type the master password? It
could make the user a bit more concerned about its security, isn't it?
A quick google search gave me those tips to evaluate a password
strength: http://justwild.us/examples/password/

Good idea, it would be nice to have it. I'll look into that.

···

On Wed, Jun 23, 2010 at 2:44 PM, Benoit Pradelle <b.pradelle@gmail.com> wrote:

Cheers,
Ben

On Wed, Jun 23, 2010 at 11:34, Dmitri Melnikov <dmitri807@gmail.com> wrote:

Sure, the screenshots are worth a thousand words.
Some comments about them:
1. When you tick the "Use a master password" checkbox this dialog pops
up. It's the same dialog used for changing, only the current password
field is disabled. The checkbox and change button remain disabled
until the password is set correctly.
2. The change dialog, this time current password is enabled. I also
added confirmation popups.
3. This is how the master password input looks like in all places
where it's needed. I had a separate dialog for it, but then decided to
just use the existing simple popup.
4. The saved passwords table where you can view and remove passwords.
5. Same table with the show passwords button clicked. If a master
password is set the input popup (3) is presented first before showing
the hidden passwords column.
When the "Use a master password" checkbox is unticked to remove the
MP, the input popup is also presented.

Cheers,
Dmitri

On Wed, Jun 23, 2010 at 1:39 AM, Emil Ivov <emcho@sip-communicator.org> wrote:

Hey Dmitri,

On 22 juin 2010, at 23:46, Dmitri Melnikov <dmitri807@gmail.com> wrote:

Hi all,

I think the UI for the master password is ready. I also did some
refactoring/additions to the credentials service.

Cool! That's good to hear!

You may also want to send a couple of screenshots here for those that won't
have the time to checkout your branch.

Cheers,
Emil

Everything seems to
work, but it's possible that there are some hidden bugs. It would be
great to hear some feedback and maybe go over the code with the
mentors while it's fresh in my mind. The code is commited into my
branch.

Cheers,
Dmitri

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#6

Hi,
I added the tests to
net.java.sip.communicator.slick.credentialsstorage, fixed the package
imports and
did other small refactoring. That should cover all issues that George
pointed out.
I also added a simple password quality meter, here's a screenshot of
how it looks like.

If there's anything else, I'd be happy to add/fix it. If not, maybe
it's time to move on to project 2?

Have a nice weekend,
Dmitri

···

On Thu, Jun 24, 2010 at 12:21 AM, George Politis <666f6f@gmail.com> wrote:

Hello Dmitri, all,

I had a thorough look at the implementation, here are my thoughts.

From a functional perspective, well, it "just works" and I like it a lot.

Some comments/remarks about the code

1. Mostly followed the SIP Communicator coding convention (proper
comments, both code and javadoc are limited at column 80, e.t.c.)!
2. Code is internationalization ready, no hardcoded messages!
3. As a general practice, we use package imports (package.*) and we do
not import classes themselves.
4. AESCrypto.getKey method, really necessary? It could be incorporated
in the constructor.

Off course, the last 2 comments are of minuscule importance, the overall
code quality seems to be good and, when the test cases are furnished, I
think we will have production ready code.

What's exactly left to implement? Ben's idea of a password strength
indicator is a nice feature to have and unit tests are obligatory, off
course. Anything else? I'm starting to think of project number 2..

Have a nice day everyone :slight_smile:

Cheers,
George

On 06/23/2010 11:34 AM, Dmitri Melnikov wrote:

Sure, the screenshots are worth a thousand words.
Some comments about them:
1. When you tick the "Use a master password" checkbox this dialog pops
up. It's the same dialog used for changing, only the current password
field is disabled. The checkbox and change button remain disabled
until the password is set correctly.
2. The change dialog, this time current password is enabled. I also
added confirmation popups.
3. This is how the master password input looks like in all places
where it's needed. I had a separate dialog for it, but then decided to
just use the existing simple popup.
4. The saved passwords table where you can view and remove passwords.
5. Same table with the show passwords button clicked. If a master
password is set the input popup (3) is presented first before showing
the hidden passwords column.
When the "Use a master password" checkbox is unticked to remove the
MP, the input popup is also presented.

Cheers,
Dmitri

On Wed, Jun 23, 2010 at 1:39 AM, Emil Ivov <emcho@sip-communicator.org> wrote:

Hey Dmitri,

On 22 juin 2010, at 23:46, Dmitri Melnikov <dmitri807@gmail.com> wrote:

Hi all,

I think the UI for the master password is ready. I also did some
refactoring/additions to the credentials service.

Cool! That's good to hear!

You may also want to send a couple of screenshots here for those that won't
have the time to checkout your branch.

Cheers,
Emil

Everything seems to
work, but it's possible that there are some hidden bugs. It would be
great to hear some feedback and maybe go over the code with the
mentors while it's fresh in my mind. The code is commited into my
branch.

Cheers,
Dmitri

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#7

Hello Dmitri, all

The password quality meter seems great! I will know try to find/produce
stronger passwords... As for the test cases, I haven't had the time to
check them out. I'll do that probably tonight, and send a follow up message.

I did try once more the implementation and I've found some more minor
issues we might be interested in fixing.

I. The user usually expects dialogs to respond to Enter and Escape keys.
That already works in MasterPasswordChangeDialog (nice!) and we should
do the same in MasterPasswordPanel.

sc-oups.txt.zip (11.7 KB)

···

-

II. Assume the following conditions:
1. you have an XMPP server that uses a self-signed certificate for SSL
connections (the default Openfire connection scenario).
2. that your password is not saved/persisted.

Here's what happens when the user tries to connect to the server..

1. The password input box is presented to the user (correct, see step1.gif).
2. The certificate verification dialog is presented to the user
(correct, see step2.gif).
3. The password input box is presented to the user once more (ouch!, see
step3.gif).

The (current version of) trunk is not affected by this problem.

-

III. Input validation

Clicking OK in the MasterPasswordPanel without a password being entered
causes a java.lang.RuntimeException: Invalid key specification and the
MasterPasswordPanel dies. The expected behavior would be to show a
message to the user letting him know that empty strings are not allowed,
or something similar.

-

IV. Hmmm...

Finally something interesting. Under some unknown and rare conditions, I
noticed an overall X slowdown and dialogs not drawing. I don't have any
steps to reproduce and it only happened twice.. the only thing I have is
a big log stuffed with exceptions (attached).

I can't blame our code for the time being. Maybe it's an OpenJDK bug,
since it seems to be a low-level error/problem. However, I'm letting you
know to be vigilant.

Great work overall!

Cheers,
George

On 06/26/2010 03:29 PM, Dmitri Melnikov wrote:

Hi,
I added the tests to
net.java.sip.communicator.slick.credentialsstorage, fixed the package
imports and
did other small refactoring. That should cover all issues that George
pointed out.
I also added a simple password quality meter, here's a screenshot of
how it looks like.

If there's anything else, I'd be happy to add/fix it. If not, maybe
it's time to move on to project 2?

Have a nice weekend,
Dmitri

On Thu, Jun 24, 2010 at 12:21 AM, George Politis <666f6f@gmail.com> wrote:

Hello Dmitri, all,

I had a thorough look at the implementation, here are my thoughts.

From a functional perspective, well, it "just works" and I like it a lot.

Some comments/remarks about the code

1. Mostly followed the SIP Communicator coding convention (proper
comments, both code and javadoc are limited at column 80, e.t.c.)!
2. Code is internationalization ready, no hardcoded messages!
3. As a general practice, we use package imports (package.*) and we do
not import classes themselves.
4. AESCrypto.getKey method, really necessary? It could be incorporated
in the constructor.

Off course, the last 2 comments are of minuscule importance, the overall
code quality seems to be good and, when the test cases are furnished, I
think we will have production ready code.

What's exactly left to implement? Ben's idea of a password strength
indicator is a nice feature to have and unit tests are obligatory, off
course. Anything else? I'm starting to think of project number 2..

Have a nice day everyone :slight_smile:

Cheers,
George

On 06/23/2010 11:34 AM, Dmitri Melnikov wrote:

Sure, the screenshots are worth a thousand words.
Some comments about them:
1. When you tick the "Use a master password" checkbox this dialog pops
up. It's the same dialog used for changing, only the current password
field is disabled. The checkbox and change button remain disabled
until the password is set correctly.
2. The change dialog, this time current password is enabled. I also
added confirmation popups.
3. This is how the master password input looks like in all places
where it's needed. I had a separate dialog for it, but then decided to
just use the existing simple popup.
4. The saved passwords table where you can view and remove passwords.
5. Same table with the show passwords button clicked. If a master
password is set the input popup (3) is presented first before showing
the hidden passwords column.
When the "Use a master password" checkbox is unticked to remove the
MP, the input popup is also presented.

Cheers,
Dmitri

On Wed, Jun 23, 2010 at 1:39 AM, Emil Ivov <emcho@sip-communicator.org> wrote:

Hey Dmitri,

On 22 juin 2010, at 23:46, Dmitri Melnikov <dmitri807@gmail.com> wrote:

Hi all,

I think the UI for the master password is ready. I also did some
refactoring/additions to the credentials service.

Cool! That's good to hear!

You may also want to send a couple of screenshots here for those that won't
have the time to checkout your branch.

Cheers,
Emil

Everything seems to
work, but it's possible that there are some hidden bugs. It would be
great to hear some feedback and maybe go over the code with the
mentors while it's fresh in my mind. The code is commited into my
branch.

Cheers,
Dmitri

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#8

Ladies, Gentlemen,

I just want to report a minor gui issue in the Preferences-Menue on MAC OS. Tab "Off-the-Record Messaging.

If you have a fingerprint on a private key, the button is not visible anymore. Just half.

I am on alpha6.2772 on Mac OS/X10.3.6

Kind regards
Walter


#9

Hi George,

First of all thanks for the quick feedback and for reporting these issues.

I.
Fixed.

II.
The problem is that my branch is too old. The
ProtocolProviderServiceJabberImpl connectAndLogin() method is where
the problem arises is different in the trunk and indeed does not have
this problem. I tried copy-pasting the trunk version into my branch
and it worked like it should, so nothing to do here.

III.
Fixed. I added an error pop-up with the message "The Master Password
cannot be empty" if it's empty.

IV.
I have never seen this, but I'm using sun's jdk for linux, not OpenJDK.

Cheers,
Dmitri

···

On Mon, Jun 28, 2010 at 5:04 PM, George Politis <666f6f@gmail.com> wrote:

Hello Dmitri, all

The password quality meter seems great! I will know try to find/produce
stronger passwords... As for the test cases, I haven't had the time to
check them out. I'll do that probably tonight, and send a follow up message.

I did try once more the implementation and I've found some more minor
issues we might be interested in fixing.

I. The user usually expects dialogs to respond to Enter and Escape keys.
That already works in MasterPasswordChangeDialog (nice!) and we should
do the same in MasterPasswordPanel.

-

II. Assume the following conditions:
1. you have an XMPP server that uses a self-signed certificate for SSL
connections (the default Openfire connection scenario).
2. that your password is not saved/persisted.

Here's what happens when the user tries to connect to the server..

1. The password input box is presented to the user (correct, see step1.gif).
2. The certificate verification dialog is presented to the user
(correct, see step2.gif).
3. The password input box is presented to the user once more (ouch!, see
step3.gif).

The (current version of) trunk is not affected by this problem.

-

III. Input validation

Clicking OK in the MasterPasswordPanel without a password being entered
causes a java.lang.RuntimeException: Invalid key specification and the
MasterPasswordPanel dies. The expected behavior would be to show a
message to the user letting him know that empty strings are not allowed,
or something similar.

-

IV. Hmmm...

Finally something interesting. Under some unknown and rare conditions, I
noticed an overall X slowdown and dialogs not drawing. I don't have any
steps to reproduce and it only happened twice.. the only thing I have is
a big log stuffed with exceptions (attached).

I can't blame our code for the time being. Maybe it's an OpenJDK bug,
since it seems to be a low-level error/problem. However, I'm letting you
know to be vigilant.

Great work overall!

Cheers,
George

On 06/26/2010 03:29 PM, Dmitri Melnikov wrote:

Hi,
I added the tests to
net.java.sip.communicator.slick.credentialsstorage, fixed the package
imports and
did other small refactoring. That should cover all issues that George
pointed out.
I also added a simple password quality meter, here's a screenshot of
how it looks like.

If there's anything else, I'd be happy to add/fix it. If not, maybe
it's time to move on to project 2?

Have a nice weekend,
Dmitri

On Thu, Jun 24, 2010 at 12:21 AM, George Politis <666f6f@gmail.com> wrote:

Hello Dmitri, all,

I had a thorough look at the implementation, here are my thoughts.

From a functional perspective, well, it "just works" and I like it a lot.

Some comments/remarks about the code

1. Mostly followed the SIP Communicator coding convention (proper
comments, both code and javadoc are limited at column 80, e.t.c.)!
2. Code is internationalization ready, no hardcoded messages!
3. As a general practice, we use package imports (package.*) and we do
not import classes themselves.
4. AESCrypto.getKey method, really necessary? It could be incorporated
in the constructor.

Off course, the last 2 comments are of minuscule importance, the overall
code quality seems to be good and, when the test cases are furnished, I
think we will have production ready code.

What's exactly left to implement? Ben's idea of a password strength
indicator is a nice feature to have and unit tests are obligatory, off
course. Anything else? I'm starting to think of project number 2..

Have a nice day everyone :slight_smile:

Cheers,
George

On 06/23/2010 11:34 AM, Dmitri Melnikov wrote:

Sure, the screenshots are worth a thousand words.
Some comments about them:
1. When you tick the "Use a master password" checkbox this dialog pops
up. It's the same dialog used for changing, only the current password
field is disabled. The checkbox and change button remain disabled
until the password is set correctly.
2. The change dialog, this time current password is enabled. I also
added confirmation popups.
3. This is how the master password input looks like in all places
where it's needed. I had a separate dialog for it, but then decided to
just use the existing simple popup.
4. The saved passwords table where you can view and remove passwords.
5. Same table with the show passwords button clicked. If a master
password is set the input popup (3) is presented first before showing
the hidden passwords column.
When the "Use a master password" checkbox is unticked to remove the
MP, the input popup is also presented.

Cheers,
Dmitri

On Wed, Jun 23, 2010 at 1:39 AM, Emil Ivov <emcho@sip-communicator.org> wrote:

Hey Dmitri,

On 22 juin 2010, at 23:46, Dmitri Melnikov <dmitri807@gmail.com> wrote:

Hi all,

I think the UI for the master password is ready. I also did some
refactoring/additions to the credentials service.

Cool! That's good to hear!

You may also want to send a couple of screenshots here for those that won't
have the time to checkout your branch.

Cheers,
Emil

Everything seems to
work, but it's possible that there are some hidden bugs. It would be
great to hear some feedback and maybe go over the code with the
mentors while it's fresh in my mind. The code is commited into my
branch.

Cheers,
Dmitri

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#10

Hi Walter,

Thanks for reporting this! I'm currently working on improving our configuration window and will address this asap.

Our new configuration form is coming soon, so stay tuned:)

Cheers,
Yana

···

On Jun 28, 2010, at 5:52 PM, Walter Kreutzner wrote:

Ladies, Gentlemen,

I just want to report a minor gui issue in the Preferences-Menue on MAC OS. Tab "Off-the-Record Messaging.

If you have a fingerprint on a private key, the button is not visible anymore. Just half.

I am on alpha6.2772 on Mac OS/X10.3.6

Kind regards
Walter

<Screen shot 2010-06-28 at 21.27.20.png>---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#11

Hi Dmitri, all,

I'm wonderring about this empty master password message. In firefox
actually the Ok button remains gray until the user types something in
the input fields, shouldn't we do the same thing?

And now that everything seems to be more or less done you should maybe
consider starting the second part of your SoC, isn't it?

Cheers,

Ben.

···

On Tue, Jun 29, 2010 at 21:00, Dmitri Melnikov <dmitri807@gmail.com> wrote:

Hi George,

First of all thanks for the quick feedback and for reporting these issues.

I.
Fixed.

II.
The problem is that my branch is too old. The
ProtocolProviderServiceJabberImpl connectAndLogin() method is where
the problem arises is different in the trunk and indeed does not have
this problem. I tried copy-pasting the trunk version into my branch
and it worked like it should, so nothing to do here.

III.
Fixed. I added an error pop-up with the message "The Master Password
cannot be empty" if it's empty.

IV.
I have never seen this, but I'm using sun's jdk for linux, not OpenJDK.

Cheers,
Dmitri

On Mon, Jun 28, 2010 at 5:04 PM, George Politis <666f6f@gmail.com> wrote:

Hello Dmitri, all

The password quality meter seems great! I will know try to find/produce
stronger passwords... As for the test cases, I haven't had the time to
check them out. I'll do that probably tonight, and send a follow up message.

I did try once more the implementation and I've found some more minor
issues we might be interested in fixing.

I. The user usually expects dialogs to respond to Enter and Escape keys.
That already works in MasterPasswordChangeDialog (nice!) and we should
do the same in MasterPasswordPanel.

-

II. Assume the following conditions:
1. you have an XMPP server that uses a self-signed certificate for SSL
connections (the default Openfire connection scenario).
2. that your password is not saved/persisted.

Here's what happens when the user tries to connect to the server..

1. The password input box is presented to the user (correct, see step1.gif).
2. The certificate verification dialog is presented to the user
(correct, see step2.gif).
3. The password input box is presented to the user once more (ouch!, see
step3.gif).

The (current version of) trunk is not affected by this problem.

-

III. Input validation

Clicking OK in the MasterPasswordPanel without a password being entered
causes a java.lang.RuntimeException: Invalid key specification and the
MasterPasswordPanel dies. The expected behavior would be to show a
message to the user letting him know that empty strings are not allowed,
or something similar.

-

IV. Hmmm...

Finally something interesting. Under some unknown and rare conditions, I
noticed an overall X slowdown and dialogs not drawing. I don't have any
steps to reproduce and it only happened twice.. the only thing I have is
a big log stuffed with exceptions (attached).

I can't blame our code for the time being. Maybe it's an OpenJDK bug,
since it seems to be a low-level error/problem. However, I'm letting you
know to be vigilant.

Great work overall!

Cheers,
George

On 06/26/2010 03:29 PM, Dmitri Melnikov wrote:

Hi,
I added the tests to
net.java.sip.communicator.slick.credentialsstorage, fixed the package
imports and
did other small refactoring. That should cover all issues that George
pointed out.
I also added a simple password quality meter, here's a screenshot of
how it looks like.

If there's anything else, I'd be happy to add/fix it. If not, maybe
it's time to move on to project 2?

Have a nice weekend,
Dmitri

On Thu, Jun 24, 2010 at 12:21 AM, George Politis <666f6f@gmail.com> wrote:

Hello Dmitri, all,

I had a thorough look at the implementation, here are my thoughts.

From a functional perspective, well, it "just works" and I like it a lot.

Some comments/remarks about the code

1. Mostly followed the SIP Communicator coding convention (proper
comments, both code and javadoc are limited at column 80, e.t.c.)!
2. Code is internationalization ready, no hardcoded messages!
3. As a general practice, we use package imports (package.*) and we do
not import classes themselves.
4. AESCrypto.getKey method, really necessary? It could be incorporated
in the constructor.

Off course, the last 2 comments are of minuscule importance, the overall
code quality seems to be good and, when the test cases are furnished, I
think we will have production ready code.

What's exactly left to implement? Ben's idea of a password strength
indicator is a nice feature to have and unit tests are obligatory, off
course. Anything else? I'm starting to think of project number 2..

Have a nice day everyone :slight_smile:

Cheers,
George

On 06/23/2010 11:34 AM, Dmitri Melnikov wrote:

Sure, the screenshots are worth a thousand words.
Some comments about them:
1. When you tick the "Use a master password" checkbox this dialog pops
up. It's the same dialog used for changing, only the current password
field is disabled. The checkbox and change button remain disabled
until the password is set correctly.
2. The change dialog, this time current password is enabled. I also
added confirmation popups.
3. This is how the master password input looks like in all places
where it's needed. I had a separate dialog for it, but then decided to
just use the existing simple popup.
4. The saved passwords table where you can view and remove passwords.
5. Same table with the show passwords button clicked. If a master
password is set the input popup (3) is presented first before showing
the hidden passwords column.
When the "Use a master password" checkbox is unticked to remove the
MP, the input popup is also presented.

Cheers,
Dmitri

On Wed, Jun 23, 2010 at 1:39 AM, Emil Ivov <emcho@sip-communicator.org> wrote:

Hey Dmitri,

On 22 juin 2010, at 23:46, Dmitri Melnikov <dmitri807@gmail.com> wrote:

Hi all,

I think the UI for the master password is ready. I also did some
refactoring/additions to the credentials service.

Cool! That's good to hear!

You may also want to send a couple of screenshots here for those that won't
have the time to checkout your branch.

Cheers,
Emil

Everything seems to
work, but it's possible that there are some hidden bugs. It would be
great to hear some feedback and maybe go over the code with the
mentors while it's fresh in my mind. The code is commited into my
branch.

Cheers,
Dmitri

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#12

Hey Dmitri,

На 29.06.10 21:00, Dmitri Melnikov написа:

II.
The problem is that my branch is too old. The
ProtocolProviderServiceJabberImpl connectAndLogin() method is where
the problem arises is different in the trunk and indeed does not have
this problem. I tried copy-pasting the trunk version into my branch
and it worked like it should, so nothing to do here.

Oh well ... then we need to either integrate your code very soon (and
rushing it might be somewhat unreasonable given how important it is), or
you'd need to experience the joys of SVN merging :). You should try
keeping your branch up to date as much as possible so that generating a
patch against trunk would be easier when the time comes.

If this turns out to be too much of a hassle we'll try to think of
something else (e.g. asking you to apply your changes on your local
trunk sandbox and then generate a patch once you are done). Still, you
should first give merging a try. It's a good thing to learn.

Cheers,
Emil

···

III.
Fixed. I added an error pop-up with the message "The Master Password
cannot be empty" if it's empty.

IV.
I have never seen this, but I'm using sun's jdk for linux, not OpenJDK.

Cheers,
Dmitri

On Mon, Jun 28, 2010 at 5:04 PM, George Politis <666f6f@gmail.com> wrote:

Hello Dmitri, all

The password quality meter seems great! I will know try to find/produce
stronger passwords... As for the test cases, I haven't had the time to
check them out. I'll do that probably tonight, and send a follow up message.

I did try once more the implementation and I've found some more minor
issues we might be interested in fixing.

I. The user usually expects dialogs to respond to Enter and Escape keys.
That already works in MasterPasswordChangeDialog (nice!) and we should
do the same in MasterPasswordPanel.

-

II. Assume the following conditions:
1. you have an XMPP server that uses a self-signed certificate for SSL
connections (the default Openfire connection scenario).
2. that your password is not saved/persisted.

Here's what happens when the user tries to connect to the server..

1. The password input box is presented to the user (correct, see step1.gif).
2. The certificate verification dialog is presented to the user
(correct, see step2.gif).
3. The password input box is presented to the user once more (ouch!, see
step3.gif).

The (current version of) trunk is not affected by this problem.

-

III. Input validation

Clicking OK in the MasterPasswordPanel without a password being entered
causes a java.lang.RuntimeException: Invalid key specification and the
MasterPasswordPanel dies. The expected behavior would be to show a
message to the user letting him know that empty strings are not allowed,
or something similar.

-

IV. Hmmm...

Finally something interesting. Under some unknown and rare conditions, I
noticed an overall X slowdown and dialogs not drawing. I don't have any
steps to reproduce and it only happened twice.. the only thing I have is
a big log stuffed with exceptions (attached).

I can't blame our code for the time being. Maybe it's an OpenJDK bug,
since it seems to be a low-level error/problem. However, I'm letting you
know to be vigilant.

Great work overall!

Cheers,
George

On 06/26/2010 03:29 PM, Dmitri Melnikov wrote:

Hi,
I added the tests to
net.java.sip.communicator.slick.credentialsstorage, fixed the package
imports and
did other small refactoring. That should cover all issues that George
pointed out.
I also added a simple password quality meter, here's a screenshot of
how it looks like.

If there's anything else, I'd be happy to add/fix it. If not, maybe
it's time to move on to project 2?

Have a nice weekend,
Dmitri

On Thu, Jun 24, 2010 at 12:21 AM, George Politis <666f6f@gmail.com> wrote:

Hello Dmitri, all,

I had a thorough look at the implementation, here are my thoughts.

From a functional perspective, well, it "just works" and I like it a lot.

Some comments/remarks about the code

1. Mostly followed the SIP Communicator coding convention (proper
comments, both code and javadoc are limited at column 80, e.t.c.)!
2. Code is internationalization ready, no hardcoded messages!
3. As a general practice, we use package imports (package.*) and we do
not import classes themselves.
4. AESCrypto.getKey method, really necessary? It could be incorporated
in the constructor.

Off course, the last 2 comments are of minuscule importance, the overall
code quality seems to be good and, when the test cases are furnished, I
think we will have production ready code.

What's exactly left to implement? Ben's idea of a password strength
indicator is a nice feature to have and unit tests are obligatory, off
course. Anything else? I'm starting to think of project number 2..

Have a nice day everyone :slight_smile:

Cheers,
George

On 06/23/2010 11:34 AM, Dmitri Melnikov wrote:

Sure, the screenshots are worth a thousand words.
Some comments about them:
1. When you tick the "Use a master password" checkbox this dialog pops
up. It's the same dialog used for changing, only the current password
field is disabled. The checkbox and change button remain disabled
until the password is set correctly.
2. The change dialog, this time current password is enabled. I also
added confirmation popups.
3. This is how the master password input looks like in all places
where it's needed. I had a separate dialog for it, but then decided to
just use the existing simple popup.
4. The saved passwords table where you can view and remove passwords.
5. Same table with the show passwords button clicked. If a master
password is set the input popup (3) is presented first before showing
the hidden passwords column.
When the "Use a master password" checkbox is unticked to remove the
MP, the input popup is also presented.

Cheers,
Dmitri

On Wed, Jun 23, 2010 at 1:39 AM, Emil Ivov <emcho@sip-communicator.org> wrote:

Hey Dmitri,

On 22 juin 2010, at 23:46, Dmitri Melnikov <dmitri807@gmail.com> wrote:

Hi all,

I think the UI for the master password is ready. I also did some
refactoring/additions to the credentials service.

Cool! That's good to hear!

You may also want to send a couple of screenshots here for those that won't
have the time to checkout your branch.

Cheers,
Emil

Everything seems to
work, but it's possible that there are some hidden bugs. It would be
great to hear some feedback and maybe go over the code with the
mentors while it's fresh in my mind. The code is commited into my
branch.

Cheers,
Dmitri

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

--
Emil Ivov, Ph.D. 67000 Strasbourg,
Project Lead France
SIP Communicator
emcho@sip-communicator.org PHONE: +33.1.77.62.43.30
http://sip-communicator.org FAX: +33.1.77.62.47.31

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#13

Hi,

I also feel that making it grey would be nicer. However, using
JOptionPane.showOptionDialog for input, is it possible to get a
reference to it's Ok button? I haven't found a way to do it.

Yes, I guess I should move on with the next project. Do you have any
ideas about where it would be better to start?

Have a nice day,
Dmitri

···

On Wed, Jun 30, 2010 at 11:38 AM, Benoit Pradelle <b.pradelle@gmail.com> wrote:

Hi Dmitri, all,

I'm wonderring about this empty master password message. In firefox
actually the Ok button remains gray until the user types something in
the input fields, shouldn't we do the same thing?

And now that everything seems to be more or less done you should maybe
consider starting the second part of your SoC, isn't it?

Cheers,

Ben.

On Tue, Jun 29, 2010 at 21:00, Dmitri Melnikov <dmitri807@gmail.com> wrote:

Hi George,

First of all thanks for the quick feedback and for reporting these issues.

I.
Fixed.

II.
The problem is that my branch is too old. The
ProtocolProviderServiceJabberImpl connectAndLogin() method is where
the problem arises is different in the trunk and indeed does not have
this problem. I tried copy-pasting the trunk version into my branch
and it worked like it should, so nothing to do here.

III.
Fixed. I added an error pop-up with the message "The Master Password
cannot be empty" if it's empty.

IV.
I have never seen this, but I'm using sun's jdk for linux, not OpenJDK.

Cheers,
Dmitri

On Mon, Jun 28, 2010 at 5:04 PM, George Politis <666f6f@gmail.com> wrote:

Hello Dmitri, all

The password quality meter seems great! I will know try to find/produce
stronger passwords... As for the test cases, I haven't had the time to
check them out. I'll do that probably tonight, and send a follow up message.

I did try once more the implementation and I've found some more minor
issues we might be interested in fixing.

I. The user usually expects dialogs to respond to Enter and Escape keys.
That already works in MasterPasswordChangeDialog (nice!) and we should
do the same in MasterPasswordPanel.

-

II. Assume the following conditions:
1. you have an XMPP server that uses a self-signed certificate for SSL
connections (the default Openfire connection scenario).
2. that your password is not saved/persisted.

Here's what happens when the user tries to connect to the server..

1. The password input box is presented to the user (correct, see step1.gif).
2. The certificate verification dialog is presented to the user
(correct, see step2.gif).
3. The password input box is presented to the user once more (ouch!, see
step3.gif).

The (current version of) trunk is not affected by this problem.

-

III. Input validation

Clicking OK in the MasterPasswordPanel without a password being entered
causes a java.lang.RuntimeException: Invalid key specification and the
MasterPasswordPanel dies. The expected behavior would be to show a
message to the user letting him know that empty strings are not allowed,
or something similar.

-

IV. Hmmm...

Finally something interesting. Under some unknown and rare conditions, I
noticed an overall X slowdown and dialogs not drawing. I don't have any
steps to reproduce and it only happened twice.. the only thing I have is
a big log stuffed with exceptions (attached).

I can't blame our code for the time being. Maybe it's an OpenJDK bug,
since it seems to be a low-level error/problem. However, I'm letting you
know to be vigilant.

Great work overall!

Cheers,
George

On 06/26/2010 03:29 PM, Dmitri Melnikov wrote:

Hi,
I added the tests to
net.java.sip.communicator.slick.credentialsstorage, fixed the package
imports and
did other small refactoring. That should cover all issues that George
pointed out.
I also added a simple password quality meter, here's a screenshot of
how it looks like.

If there's anything else, I'd be happy to add/fix it. If not, maybe
it's time to move on to project 2?

Have a nice weekend,
Dmitri

On Thu, Jun 24, 2010 at 12:21 AM, George Politis <666f6f@gmail.com> wrote:

Hello Dmitri, all,

I had a thorough look at the implementation, here are my thoughts.

From a functional perspective, well, it "just works" and I like it a lot.

Some comments/remarks about the code

1. Mostly followed the SIP Communicator coding convention (proper
comments, both code and javadoc are limited at column 80, e.t.c.)!
2. Code is internationalization ready, no hardcoded messages!
3. As a general practice, we use package imports (package.*) and we do
not import classes themselves.
4. AESCrypto.getKey method, really necessary? It could be incorporated
in the constructor.

Off course, the last 2 comments are of minuscule importance, the overall
code quality seems to be good and, when the test cases are furnished, I
think we will have production ready code.

What's exactly left to implement? Ben's idea of a password strength
indicator is a nice feature to have and unit tests are obligatory, off
course. Anything else? I'm starting to think of project number 2..

Have a nice day everyone :slight_smile:

Cheers,
George

On 06/23/2010 11:34 AM, Dmitri Melnikov wrote:

Sure, the screenshots are worth a thousand words.
Some comments about them:
1. When you tick the "Use a master password" checkbox this dialog pops
up. It's the same dialog used for changing, only the current password
field is disabled. The checkbox and change button remain disabled
until the password is set correctly.
2. The change dialog, this time current password is enabled. I also
added confirmation popups.
3. This is how the master password input looks like in all places
where it's needed. I had a separate dialog for it, but then decided to
just use the existing simple popup.
4. The saved passwords table where you can view and remove passwords.
5. Same table with the show passwords button clicked. If a master
password is set the input popup (3) is presented first before showing
the hidden passwords column.
When the "Use a master password" checkbox is unticked to remove the
MP, the input popup is also presented.

Cheers,
Dmitri

On Wed, Jun 23, 2010 at 1:39 AM, Emil Ivov <emcho@sip-communicator.org> wrote:

Hey Dmitri,

On 22 juin 2010, at 23:46, Dmitri Melnikov <dmitri807@gmail.com> wrote:

Hi all,

I think the UI for the master password is ready. I also did some
refactoring/additions to the credentials service.

Cool! That's good to hear!

You may also want to send a couple of screenshots here for those that won't
have the time to checkout your branch.

Cheers,
Emil

Everything seems to
work, but it's possible that there are some hidden bugs. It would be
great to hear some feedback and maybe go over the code with the
mentors while it's fresh in my mind. The code is commited into my
branch.

Cheers,
Dmitri

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#14

Hey again Dmitri, all,

I forgot to mention in my previous mail that I really like what you've
done so far and I am looking forward to seeing it in trunk!

(more inline)

На 30.06.10 10:59, Dmitri Melnikov написа:

Hi,

I also feel that making it grey would be nicer. However, using
JOptionPane.showOptionDialog for input, is it possible to get a
reference to it's Ok button? I haven't found a way to do it.

I am a bit lost here. Are you guys talking about the window that allows
users to define a master password or the one that asks them to enter it
when they need to use it?

According to your screenshot, the former already seems to be disabling
the OK button in some cases. As for the latter it could simply handle
empty passwords as it does wrong ones (like firefox).

Yes, I guess I should move on with the next project. Do you have any
ideas about where it would be better to start?

Ideally we should implement call recording using the same mixers that we
use for conferencing. This would most probably mean that we should
launch all calls with a mixer audio device rather than the default
capture device so that the recorder could be plugged as soon as the user
presses the OK button. At least that's the idea. We just had a quick
chat with Lubomir and given that getting this right from a design
perspective would be a bit tricky, he wanted to think about it a bit
more. He'll get back to you as soon as he's ready.

Until then (if you happen to finish your last changes on the credentials
service before he's done) you could start by familiarizing yourself with
the way that audio mixers work.

A few other minor (but important) comments:

* I saw in a few locations things like:

@param oldPassword <nothing>
@param master <nothing>
@return <nothing>
...

Most IDEs allow support configuration options that allow displaying such
javadoc problems as warnings or errors. Turning these would would help
you track down such issues.

* Also, we generally try to leave an empty line between the actual
comments and the @param enumeration (separating @param-s, @throws and
@return with extra empty lines is also cool although not officially
requested by the convention).

* Also, we use <tt> rather than <code> tags.

* I saw that you are explicitly setting asterisk as the echo char of
your passwd fields.

passwordField.setEchoChar('*');

I understand that this may look better to some but uniformity gets
higher priority than taste so, given that the rest of SC uses the
default char ... :slight_smile:

I believe that's all for now. Congrats again for the good job!

Emil

···

Have a nice day,
Dmitri

On Wed, Jun 30, 2010 at 11:38 AM, Benoit Pradelle <b.pradelle@gmail.com> wrote:

Hi Dmitri, all,

I'm wonderring about this empty master password message. In firefox
actually the Ok button remains gray until the user types something in
the input fields, shouldn't we do the same thing?

And now that everything seems to be more or less done you should maybe
consider starting the second part of your SoC, isn't it?

Cheers,

Ben.

On Tue, Jun 29, 2010 at 21:00, Dmitri Melnikov <dmitri807@gmail.com> wrote:

Hi George,

First of all thanks for the quick feedback and for reporting these issues.

I.
Fixed.

II.
The problem is that my branch is too old. The
ProtocolProviderServiceJabberImpl connectAndLogin() method is where
the problem arises is different in the trunk and indeed does not have
this problem. I tried copy-pasting the trunk version into my branch
and it worked like it should, so nothing to do here.

III.
Fixed. I added an error pop-up with the message "The Master Password
cannot be empty" if it's empty.

IV.
I have never seen this, but I'm using sun's jdk for linux, not OpenJDK.

Cheers,
Dmitri

On Mon, Jun 28, 2010 at 5:04 PM, George Politis <666f6f@gmail.com> wrote:

Hello Dmitri, all

The password quality meter seems great! I will know try to find/produce
stronger passwords... As for the test cases, I haven't had the time to
check them out. I'll do that probably tonight, and send a follow up message.

I did try once more the implementation and I've found some more minor
issues we might be interested in fixing.

I. The user usually expects dialogs to respond to Enter and Escape keys.
That already works in MasterPasswordChangeDialog (nice!) and we should
do the same in MasterPasswordPanel.

-

II. Assume the following conditions:
1. you have an XMPP server that uses a self-signed certificate for SSL
connections (the default Openfire connection scenario).
2. that your password is not saved/persisted.

Here's what happens when the user tries to connect to the server..

1. The password input box is presented to the user (correct, see step1.gif).
2. The certificate verification dialog is presented to the user
(correct, see step2.gif).
3. The password input box is presented to the user once more (ouch!, see
step3.gif).

The (current version of) trunk is not affected by this problem.

-

III. Input validation

Clicking OK in the MasterPasswordPanel without a password being entered
causes a java.lang.RuntimeException: Invalid key specification and the
MasterPasswordPanel dies. The expected behavior would be to show a
message to the user letting him know that empty strings are not allowed,
or something similar.

-

IV. Hmmm...

Finally something interesting. Under some unknown and rare conditions, I
noticed an overall X slowdown and dialogs not drawing. I don't have any
steps to reproduce and it only happened twice.. the only thing I have is
a big log stuffed with exceptions (attached).

I can't blame our code for the time being. Maybe it's an OpenJDK bug,
since it seems to be a low-level error/problem. However, I'm letting you
know to be vigilant.

Great work overall!

Cheers,
George

On 06/26/2010 03:29 PM, Dmitri Melnikov wrote:

Hi,
I added the tests to
net.java.sip.communicator.slick.credentialsstorage, fixed the package
imports and
did other small refactoring. That should cover all issues that George
pointed out.
I also added a simple password quality meter, here's a screenshot of
how it looks like.

If there's anything else, I'd be happy to add/fix it. If not, maybe
it's time to move on to project 2?

Have a nice weekend,
Dmitri

On Thu, Jun 24, 2010 at 12:21 AM, George Politis <666f6f@gmail.com> wrote:

Hello Dmitri, all,

I had a thorough look at the implementation, here are my thoughts.

From a functional perspective, well, it "just works" and I like it a lot.

Some comments/remarks about the code

1. Mostly followed the SIP Communicator coding convention (proper
comments, both code and javadoc are limited at column 80, e.t.c.)!
2. Code is internationalization ready, no hardcoded messages!
3. As a general practice, we use package imports (package.*) and we do
not import classes themselves.
4. AESCrypto.getKey method, really necessary? It could be incorporated
in the constructor.

Off course, the last 2 comments are of minuscule importance, the overall
code quality seems to be good and, when the test cases are furnished, I
think we will have production ready code.

What's exactly left to implement? Ben's idea of a password strength
indicator is a nice feature to have and unit tests are obligatory, off
course. Anything else? I'm starting to think of project number 2..

Have a nice day everyone :slight_smile:

Cheers,
George

On 06/23/2010 11:34 AM, Dmitri Melnikov wrote:

Sure, the screenshots are worth a thousand words.
Some comments about them:
1. When you tick the "Use a master password" checkbox this dialog pops
up. It's the same dialog used for changing, only the current password
field is disabled. The checkbox and change button remain disabled
until the password is set correctly.
2. The change dialog, this time current password is enabled. I also
added confirmation popups.
3. This is how the master password input looks like in all places
where it's needed. I had a separate dialog for it, but then decided to
just use the existing simple popup.
4. The saved passwords table where you can view and remove passwords.
5. Same table with the show passwords button clicked. If a master
password is set the input popup (3) is presented first before showing
the hidden passwords column.
When the "Use a master password" checkbox is unticked to remove the
MP, the input popup is also presented.

Cheers,
Dmitri

On Wed, Jun 23, 2010 at 1:39 AM, Emil Ivov <emcho@sip-communicator.org> wrote:

Hey Dmitri,

On 22 juin 2010, at 23:46, Dmitri Melnikov <dmitri807@gmail.com> wrote:

Hi all,

I think the UI for the master password is ready. I also did some
refactoring/additions to the credentials service.

Cool! That's good to hear!

You may also want to send a couple of screenshots here for those that won't
have the time to checkout your branch.

Cheers,
Emil

Everything seems to
work, but it's possible that there are some hidden bugs. It would be
great to hear some feedback and maybe go over the code with the
mentors while it's fresh in my mind. The code is commited into my
branch.

Cheers,
Dmitri

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

--
Emil Ivov, Ph.D. 67000 Strasbourg,
Project Lead France
SIP Communicator
emcho@sip-communicator.org PHONE: +33.1.77.62.43.30
http://sip-communicator.org FAX: +33.1.77.62.47.31

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#15

Hi Emil, all,

I've just updated my branch to the trunk state. Wasn't hard, just a
couple of small conflicts.

I forgot to mention in my previous mail that I really like what you've
done so far and I am looking forward to seeing it in trunk!

Thanks!

(more inline)

На 30.06.10 10:59, Dmitri Melnikov написа:

Hi,

I also feel that making it grey would be nicer. However, using
JOptionPane.showOptionDialog for input, is it possible to get a
reference to it's Ok button? I haven't found a way to do it.

I am a bit lost here. Are you guys talking about the window that allows
users to define a master password or the one that asks them to enter it
when they need to use it?

I was talking about the one that asks to enter it.

According to your screenshot, the former already seems to be disabling
the OK button in some cases. As for the latter it could simply handle
empty passwords as it does wrong ones (like firefox).

Well, right now on wrong password it gives a pop up with "The Master
Password is not correct. Please try again"
and on empty one it gives the same pop up with "The Master Password
cannot be empty". We can show only the first message in all cases,
it's a matter of taste. (My) Firefox for example just show the input
pop up in a loop until the password is correct without any error pop
ups. And when removing the MP in firefox, it's just grey until the
password is correct.

Yes, I guess I should move on with the next project. Do you have any
ideas about where it would be better to start?

Ideally we should implement call recording using the same mixers that we
use for conferencing. This would most probably mean that we should
launch all calls with a mixer audio device rather than the default
capture device so that the recorder could be plugged as soon as the user
presses the OK button. At least that's the idea. We just had a quick
chat with Lubomir and given that getting this right from a design
perspective would be a bit tricky, he wanted to think about it a bit
more. He'll get back to you as soon as he's ready.

Until then (if you happen to finish your last changes on the credentials
service before he's done) you could start by familiarizing yourself with
the way that audio mixers work.

Seems interesting, but how do you test these things? Is there some
service that I can use to call and experiment?

A few other minor (but important) comments:

* I saw in a few locations things like:

@param oldPassword <nothing>
@param master <nothing>
@return <nothing>
...

Most IDEs allow support configuration options that allow displaying such
javadoc problems as warnings or errors. Turning these would would help
you track down such issues.

* Also, we generally try to leave an empty line between the actual
comments and the @param enumeration (separating @param-s, @throws and
@return with extra empty lines is also cool although not officially
requested by the convention).

* Also, we use <tt> rather than <code> tags.

I'll add the missing javadocs.

* I saw that you are explicitly setting asterisk as the echo char of
your passwd fields.

passwordField.setEchoChar('*');

I understand that this may look better to some but uniformity gets
higher priority than taste so, given that the rest of SC uses the
default char ... :slight_smile:

Thanks for noticing, removed.

Cheers,
Dmitri

···

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#16

Hey Dmitri,

На 01.07.10 21:28, Dmitri Melnikov написа:

Hi Emil, all,

I've just updated my branch to the trunk state. Wasn't hard, just a
couple of small conflicts.

Great! Glad to hear it wasn't traumatising :).

I am a bit lost here. Are you guys talking about the window that allows
users to define a master password or the one that asks them to enter it
when they need to use it?

I was talking about the one that asks to enter it.

Well, right now on wrong password it gives a pop up with "The Master
Password is not correct. Please try again"
and on empty one it gives the same pop up with "The Master Password
cannot be empty". We can show only the first message in all cases,
it's a matter of taste.

What worries me here is popping one extra dialog. I guess this is also
the reason why Ben suggested having a disabled OK. If you feel that this
may confuse some users who would be wondering why they see a new pass
prompt when they just entered one, you could add an extra red label
displaying the messages from the error dialogs you currently use.

It's just that we need to avoid the multiple windows as according to our
experience the less we have them, the better.

(My) Firefox for example just show the input
pop up in a loop until the password is correct without any error pop

Yes, that's exactly what I was suggesting.

Cheers,
Emil

···

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net