Bug 87600

This is a bit controversial, since it is insecure: it requires saving an
unencrypted password on the disk. But perhaps it could be implemented, provided
a big warning message is displayed.

For my work I use Oracle's JDeveloper a lot. That saves the db password in an
encrypted form on the disk. Perhaps this could be an option for NetBeans as well?


Thanks for considering this.


Wouter

See issue 84654

Why did you close this as WONTFIX?

Re. JDeveloper: are you sure the password is really encrypted? Isn't is just
scrambled, and as such easy to recover? If it is encrypted, do you know how? Did
you provide an encryption key, or does JDeveloper just somehow encrypt it?

Hold on, I am sorry, but what I said is wrong. The password isn't encrypted at
all. It is stored in plain text.
I share your concern about the security issue involved here. But still it
appeals to me. I like your idea about the security warning. Would you please
consider that?


Greets, Wouter

Yes. Reopening then.

It would be good to at least parts of issue 68991 and the changes to the Connect
and New DB Connection dialogs specified in

http://j2ee.netbeans.org/docs/promog/persistence-ui-spec.html#Step_1_New_Database_Connection_Wizard

together with this.

I once started to write a data-oriented NB module that should connect to a database in the background without
any user intervention. With the current dbexplorer API this is not possible, because the database password is
not stored in the connection description and the implementation has a bug that makes the dialog pop up, even if all
parameters to connect to the database have been specified, including the password.

My opinion on the security concern: persistence.xml contains database passwords, too. Nobody ever complained.

Cheers,
Georg

Right, I have since come to agree that storing passwords in plain text is not a big problem. I still plan to scramble
them to prevent accidental reveal, but I don't want to pretend that scrambling offers any security.

Re. bug mentioned in desc9: it you have reproducible steps please file it.

Andrei,

the bug mentioned is described under item 3 in this detailed post to openide-dev from September 2006:
http://www.nabble.com/Experimenting-with-the-Database-Explorer-API-tf2280369.html#a6334571

Issue http://www.netbeans.org/issues/show_bug.cgi?id=108422 has just been created.

I do not know to which degree the db explorer API has changed since then, but if some of the issues mentioned in this
post have been fixed or ceased to exist, it would be really great.

(OBTW: The request to store the password in the connection description is under item 5).

Best regards,
Georg

I just wanted to make sure we're all clear that this is a piece of the puzzle around providing the ability for users to
opt-in to having a DB connection automatically opened on NB startup, and not the whole solution?

I have made issue 99730 dependent on this change, does that make sense?

FYI, Java Studio Creator did store the password on the disk in encrypted form (using javax.crypto.Cipher).  With the
integration to NB6.0 and the move to open source, we've moved away from that.

*** Issue 112551 has been marked as a duplicate of this issue. ***

Will this be fixed for 6.0?
Or should this be reassigned?

I would *love* to fix this.  But since we're in beta, and this is marked as an enhancement, I think it can't go into
6.0.  Does anyone believe otherwise?

I disagree this is an enhancement, from a user's point of view.
There is a related/dependent issue that is marked as a defect
http://www.netbeans.org/issues/show_bug.cgi?id=108422
I suppose 108422 is blocked by this issue, I'll let the owner decide

Considering the comments in
http://www.netbeans.org/issues/show_bug.cgi?id=108422
I consider this issue as a bug. Would you please try and get this fixed for 6.0? Thanks a lot!

This issue has two aspects:

1. Storing the password so that the user doesn't have to enter it each time when making a connection.
2. Not displaying the Connect dialog when making a connection and the password is known.

#1 does not necessarily imply #2. We might choose to do #1, but still always display the Connect dialog (with the
passwoed pre-filled) to allow the user to enter another user name, password or schema. Or we might alternatively do #1
and #2 and have a nice Properties dialog on the connection to allow the user to change the schema, etc. We don't have
that dialog now, nor do we have the resources to implement it for 6.0. So if we do anything, it should only be #1.

Re. 108422: that is an enhancement, not a defect. There was never a plan to support connecting to the database without
displaying an UI.

John, do you (or anyone else) have cycles to work on this?

A couple of comments:

- I think it would be a nice EOU enhancement to let users choose to auto-connect to a connection so they don't have to
manually connect each time. I have received this complaint during my interviews as well.  So I know it's never been
something we've planned to do, but maybe we should look at that - *post* 6.0, I agree this is an enhancement, not a defect.

- As a small step towards that, storing the password so it's pre-filled in across runs of NB is not a big UI change
(we'd have to either remove the prompt to store the password for the session, or add a new prompt to also store it
permanently).  I'm not sure if it's too late to put in such a small UI change.

- I would also like to know if you have time to work on this, John.  Let's make sure we get someone on it.

David

I must have heard wrong at one of the meetings, that Andrei would resolve this issue 87600.
And also it was stated that 108422 required 87600 to be fixed first.  Now in desc20, this has been
 re-clarified, thank you.  It doesn't appear that the 2 aspects are interdependent and for my expectation
(below) I don't think this issue 87600 needs to be fixed first.

We don't need to create another issue: to connect without posting the connection dialog or at least 
filling in the password is tracked by 108422.

My expectation is when the password is provided to DatabaseConnection object and this object is used
to show the Connection dialog then at minimum I expect the password to be filled in the dialog.

I agree this issue 87600 is an enhancement.

David, the small step should be doable - it doesn't sound like much of a change to pre-fill in the password 
of the Connection dialog

The Connection dialog relies on parameters from the Connection node, not the parameters
passed to the DatabaseConnection.  DatabaseConnection knows the password but not the Connection node.
I presume because a connection to the connection node hasn't been established.

Once connected to an existing connection (node) then the Connection dialog will know the password and instead
of displaying the the dialog with the username/password, a progress dialog is displayed.

I guess information is retrieved from the connection node to be sure the DatabaseConnection object
pertains to an existing connection (any parameters could be passed even if a connection (node) doesn't exists)

Seems like extra work.  Instead of obtaining connection information from the connection node, instead
be sure a connection node matches up with the DatabaseConnection information.  Unless there's another
usage of DatabaseConnection that I don't know about.

A suggested fix (hack)

1. In ConnectAction.ConnectionDialogDisplayer.showDialog
an instance of DatabaseConnection is being retrieved already (creating a dependency on DatabaseConnection).

If the pwd from the Connection node is null, check if DatabaseConnection's pwd is also null and if it's not the
use DatabaseConnection pwd  (still the parameters of the Connection node will be used so there's no chance
of a connection to a non-existant connection)

sample code:


if (pwd == null) {
    if (dbcon.getPassword() != null) {
        pwd = dbcon.getPassword();
    }
}

To be sure hack is not too limited - need to make sure all relevant use cases work

desc24 doesn't work, dbcon.getPassword() is still null

Andrei's comment in desc20 lays things out pretty clearly.  There are two separate issues here.

This issue (#87600) is a pre-cursor to any of the other things we want to do, like not bringing up a dialog (#108422). 
So, we have to start here.

This change would entail at least some UI impact, albeit not large; maybe just change the wording in the New Database
Connection dialog.

In terms of functionality, we would probably have to provide some encryption.  Note that the secret key wouldn't be so
secret for an open-source product.  I think users would accept that it's not ultra-secure; web browsers do this sort of
thing all the time.

The reason we haven't done this has been largely a question of priority.  We've been working on other things.  There are
(only) two votes for this enhancement, although David has reported that he got comments on this in the interviews.

If we think it's a big usability issue, I think we could make a case for taking it on.  Is there a consensus that this
is worth pushing for?

John Baker says:
Andrei's comment in desc20 lays things out pretty clearly.  There are two separate issues here.

This issue (#87600) is a pre-cursor to any of the other things we want to do, like not bringing up a dialog (#108422). 
So, we have to start here.

This change would entail at least some UI impact, albeit not large; maybe just change the wording in the New Database
Connection dialog.

David: agreed

John Baker says:
In terms of functionality, we would probably have to provide some encryption.  Note that the secret key wouldn't be so
secret for an open-source product.  I think users would accept that it's not ultra-secure; web browsers do this sort of
thing all the time.

David: I don't know about encryption.  That's generally not a requirement by ARC.  My memory is that, as long as the
password file is created with appropriate permissions (e.g. rw----), then that is satisfactory.  It might be worth
investigating what is really required by ARC here.

If we do do encryption, I am not sure what you mean by "the secret key is not so secret since we're in open source."  I
haven't done this encryption before, but the UNIX passwd file is encrypted, and Solaris is open source.  Maybe you could
go and see how they do it.

John Baker says:
The reason we haven't done this has been largely a question of priority.  We've been working on other things.  There are
(only) two votes for this enhancement, although David has reported that he got comments on this in the interviews.
 
If we think it's a big usability issue, I think we could make a case for taking it on.  Is there a consensus that this
is worth pushing for?

David says: Well, I think if it's not a huge amount of work, it's the kind of "thoughtfulness" that makes the product
better to work with.  Many things we can't do right now because of UI freeze, but this is something that I think could
have a good positive impact.  IMHO.  Note that most issues don't have *any* votes for them, so two votes is pretty
significant.  IMHO. :)

David, in the last comment desc27, you were quoting Jim not me :-)

After looking at the code, I think this is more of a design issue than limitation.

It doesn't make sense that a new DatabaseConnection object can be used to show the Connection dialog since
in the showDialog method there is a node.getDatabaseConnection()

In other words, there's a responsibility issue here.
The Connection dialog should know who invoked it.
Currently, showDialog gets the connection object from the node, which is a hack to me.

The current ability to show the dialog is flawed since the Connection dialog has a tight integration with the node, so 
it doesn't make sense for any DatabaseConnection object to invoke the Connection dialog

Instead the dialog could be invoked something like node.getDatabaseConnection().showDialog
or by a new DatabaseConnection object.  Then the showDialog method should know which DatabaseConnection to use.

I think the encryption is handled by the database.  I don't think encryption should be implemented in the dialog.

Sorry, you're right, I was responding to Jim's comments, not John Baker's...

John Baker says:
After looking at the code, I think this is more of a design issue than limitation.

It doesn't make sense that a new DatabaseConnection object can be used to show the Connection dialog since
in the showDialog method there is a node.getDatabaseConnection()

In other words, there's a responsibility issue here.
The Connection dialog should know who invoked it.
Currently, showDialog gets the connection object from the node, which is a hack to me.

The current ability to show the dialog is flawed since the Connection dialog has a tight integration with the node, so 
it doesn't make sense for any DatabaseConnection object to invoke the Connection dialog

Instead the dialog could be invoked something like node.getDatabaseConnection().showDialog
or by a new DatabaseConnection object.  Then the showDialog method should know which DatabaseConnection to use.

David:
If I follow things correctly, I think what you're saying is:
- DatabaseConnection may or may not have an active connection
- If it doesn't, then the right approach is to bring up the Connection dialog to establish a new "live" connection to
the database
- Rather than associating the dialog with the node, and then having the node figure out what DatabaseConnection this is
for, the dialog should be associated with a DatabaseConnection, and keep the node out of the picture, e.g.

         DatabaseConnection
                 |
      -----------------------
      |                     |
    ConnectionDialog     JDBCConnection

Right?  

Are there any important reasons I'm missing why the dialog needs to be associated with the connection node rather than
the DatabaseConnection?


John says:
I think the encryption is handled by the database.  I don't think encryption should be implemented in the dialog.

David: Um, I'm not sure what you mean.  I was thinking we were talking about encryption of the passwords in the password
file we use so we can remember passwords...  Are you proposing to store these passwords in a database?  

I wasn't sure whether to make comments in the issue or by email.  To track the discussion, I opted for 
entering comments in the issue.  

> David:
> If I follow things correctly, I think what you're saying is:
> - DatabaseConnection may or may not have an active connection
> - If it doesn't, then the right approach is to bring up the Connection dialog to establish a new "live" connection to
> the database
> - Rather than associating the dialog with the node, and then having the node figure out what DatabaseConnection this is
> for, the dialog should be associated with a DatabaseConnection, and keep the node out of the picture, e.g.

>          DatabaseConnection
>                  |
>       -----------------------
>       |                     |
>     ConnectionDialog     JDBCConnection
> 

Yes, something like this.  It doesn't make sense to me that I can create a DatabaseConnection object
then later a different DatabaseConnection object from the node is used to make the connection.
Yes, I think the Connection dialog should know which DatabaseConnection requests the connection.

> 
> David: Um, I'm not sure what you mean.  I was thinking we were talking about encryption of the passwords in the password
> file we use so we can remember passwords...  Are you proposing to store these passwords in a database?  

No not in a database.  We seem to be going in different directions.  I didn't realize Jim was referring
to using encryption to store the password between sessions. I don't see a need to store passwords
between sessions but instead safely change or extend the existing API 

I don't really understand the discussion about DatabaseConnection and nodes. It is true that the dependency on nodes is
wrong and that the dialog should only work with DatabaseConnection's. But does that need to be fixed at this moment?
Currently the dialog extracts the database URL and the user name from the database connection that the user is
connecting. Why can't it just to the same with the password?

Re. encryption: there is no point in doing it, it can never be secure (given the constraints). Unix can be secure
because it doesn't store the password, it stores just a hash (e.g., MD5) of it. When the user logs in, the hash of the
entered password is compared to the stored hash. The password is never retrieved from the stored hash (that's not
possible, because the hash is one-way). But we need to retrieve the password in order to send it to the database, so we
can't use a hash.

What might make sense is scrambling the password in order to prevent accidental reveal. Not sure this is needed, and it
definitely complicates things.

The password needs to be stored in the XML connection file in the userdir. (Note: this file, currently in version 1.0,
needs to be upgraded to version 1.1.) So we would add a password element to the DTD containing the scrambled value:

<password value="5cramb13d"/>

However, it is possible for a module writer to register connections in his module by putting such XML files in the
module layer. So the scrambling algorithm needs to be documented as an API. If we do the scrambling, I propose something
simple, such as base64 of the original password xor-ed with a constant like 42.

I don't understand the last sentence in desc31. This issue *is* about storing the password between sessions.

There have been some questions about encryption of passwords, so let me elaborate.

I brought this up because it's how Creator did things.  We encrypted the password using a secret key that was baked into
the Creator source code.  The encrypted password was stored as part of the DataSource (in context.xml in the userdir),
which could be exported from one machine to another.  It's actually a pretty secure method.

It may not be necessary to do anything like this.  That's fine.  I only bring it up as an option, and to point out that
the Creator method will no longer work, because our source code is now open.

One other point: like Andrei, I'm a little puzzled about the sentence, "I don't see a need to store passwords between
sessions but instead safely change or extend the existing API".  I'm not sure what that is getting at.

I see this issue is about saving passwords between sessions.  My comments earlier concern the design of the API.

It's been suggested that saving the password would solve other issues such as 108422, which may be true.
It sounds like this won't get solved in 6.0

I was hoping to resolve 108422 without dealing with saving the password.

As Jim mentioned in desc33, Creator encrypted the password that stored along with the other data source information and
persisted in file, context.xml.

Creator only used simple encryption with a secret pass key.  Yes, at least use the BASE64Encoder to encrypt

>I don't really understand the discussion about DatabaseConnection and nodes. It is true that the dependency on nodes is
>wrong and that the dialog should only work with DatabaseConnection's. But does that need to be fixed at this moment?
>Currently the dialog extracts the database URL and the user name from the database connection that the user is
>connecting. Why can't it just to the same with the password?

dbcon is created from the node - cne.getDatabaseConnection()
and in the showDialog method, dbcon.getPassword() is null 
since the password isn't known until the connection is made (user provides password or password is remembered or stored)

Instead, we can discuss this more in 108422.  Previously I wanted to keep discussions in one issue but it's getting off
the topic

dbcon.getPassword() is null when there is no stored password. But when you check the "Store password for this session"
checkbox in the Connect dialog, disconnect and connect again, dbcon.getPassword() will start returning the stored value.

At today's I-Team, I agreed to follow up on this.

It looks like there are three things to do:

A. The technical work, which is mainly writing code to store the password into the xml connection file in
<userdir>/config/Databases/Connections.  Also, work to retrieve it and pre-populate the password field when connecting.  
 
  I'll put together a more detailed spec, but that's basically it.  I don't know whether it's worth scrambling the password.

B. UI change.  I propose changing "Remember password during this session" to "Remember password", with Help that
explains more.  This needs to be cleared at: <http://wiki.netbeans.org/wiki/view/NB6LateUIChanges>

C. ARC case, since we're storing a password.  Indications from David are this this should be OK.  I'll check with Petr
Suchomel, who is apparently the person doing NB ARC.

Sound reasonable?  (B) and (C) are the approvals that we need.  Anything else?

This looks good to me.

New Wiki page added, linked in the URL field.

Two issues need to be decided, both fairly minor.

Now you can save the password across sessions.  I'm not closing because I want to discuss the user being able to not
have to see the dialog each time, to make it an even better experience for the common case where you just want to
reconnect quickly.

IDE: [10/4/07 3:18 PM] Committing "Database Explorer" started
cvs server: scheduling file `src/org/netbeans/modules/db/resources/connection-1_1.dtd' for addition
cvs server: scheduling file `src/org/netbeans/modules/db/util/Base64.java' for addition
cvs server: scheduling file `test/unit/src/org/netbeans/modules/db/util/Base64Test.java' for addition
cvs server: use 'cvs commit' to add these files permanently
Checking in src/org/netbeans/modules/db/explorer/infos/RootNodeInfo.java;
/cvs/db/src/org/netbeans/modules/db/explorer/infos/RootNodeInfo.java,v  <--  RootNodeInfo.java
new revision: 1.35; previous revision: 1.34
done
Checking in src/org/netbeans/modules/db/explorer/infos/DatabaseNodeInfo.java;
/cvs/db/src/org/netbeans/modules/db/explorer/infos/DatabaseNodeInfo.java,v  <--  DatabaseNodeInfo.java
new revision: 1.58; previous revision: 1.57
done
Checking in src/org/netbeans/modules/db/explorer/infos/ConnectionNodeInfo.java;
/cvs/db/src/org/netbeans/modules/db/explorer/infos/ConnectionNodeInfo.java,v  <--  ConnectionNodeInfo.java
new revision: 1.47; previous revision: 1.46
done
Checking in src/org/netbeans/modules/db/explorer/DatabaseConnection.java;
/cvs/db/src/org/netbeans/modules/db/explorer/DatabaseConnection.java,v  <--  DatabaseConnection.java
new revision: 1.53; previous revision: 1.52
done
Checking in src/org/netbeans/modules/db/explorer/DatabaseConnectionConvertor.java;
/cvs/db/src/org/netbeans/modules/db/explorer/DatabaseConnectionConvertor.java,v  <--  DatabaseConnectionConvertor.java
new revision: 1.11; previous revision: 1.10
done
Checking in src/org/netbeans/modules/db/explorer/dlg/ConnectPanel.form;
/cvs/db/src/org/netbeans/modules/db/explorer/dlg/ConnectPanel.form,v  <--  ConnectPanel.form
new revision: 1.6; previous revision: 1.5
done
Checking in src/org/netbeans/modules/db/explorer/dlg/ConnectPanel.java;
/cvs/db/src/org/netbeans/modules/db/explorer/dlg/ConnectPanel.java,v  <--  ConnectPanel.java
new revision: 1.23; previous revision: 1.22
done
Checking in src/org/netbeans/modules/db/resources/mf-layer.xml;
/cvs/db/src/org/netbeans/modules/db/resources/mf-layer.xml,v  <--  mf-layer.xml
new revision: 1.18; previous revision: 1.17
done
Checking in src/org/netbeans/modules/db/resources/Bundle.properties;
/cvs/db/src/org/netbeans/modules/db/resources/Bundle.properties,v  <--  Bundle.properties
new revision: 1.113; previous revision: 1.112
done
RCS file: /cvs/db/src/org/netbeans/modules/db/resources/connection-1_1.dtd,v
done
Checking in src/org/netbeans/modules/db/resources/connection-1_1.dtd;
/cvs/db/src/org/netbeans/modules/db/resources/connection-1_1.dtd,v  <--  connection-1_1.dtd
initial revision: 1.1
done
RCS file: /cvs/db/test/unit/src/org/netbeans/modules/db/util/Base64Test.java,v
done
Checking in test/unit/src/org/netbeans/modules/db/util/Base64Test.java;
/cvs/db/test/unit/src/org/netbeans/modules/db/util/Base64Test.java,v  <--  Base64Test.java
initial revision: 1.1
done
Checking in src/org/netbeans/modules/db/explorer/actions/ConnectAction.java;
/cvs/db/src/org/netbeans/modules/db/explorer/actions/ConnectAction.java,v  <--  ConnectAction.java
new revision: 1.46; previous revision: 1.45
done
Checking in test/unit/src/org/netbeans/modules/db/explorer/DatabaseConnectionConvertorTest.java;
/cvs/db/test/unit/src/org/netbeans/modules/db/explorer/DatabaseConnectionConvertorTest.java,v  <-- 
DatabaseConnectionConvertorTest.java
new revision: 1.11; previous revision: 1.10
done
Checking in test/unit/src/org/netbeans/modules/db/explorer/bar-connection.xml;
/cvs/db/test/unit/src/org/netbeans/modules/db/explorer/bar-connection.xml,v  <--  bar-connection.xml
new revision: 1.5; previous revision: 1.4
done
RCS file: /cvs/db/src/org/netbeans/modules/db/util/Base64.java,v
done
Checking in src/org/netbeans/modules/db/util/Base64.java;
/cvs/db/src/org/netbeans/modules/db/util/Base64.java,v  <--  Base64.java
initial revision: 1.1
done
IDE: [10/4/07 3:18 PM] Committing "Database Explorer" finished

Nice work, David. I have some comments though:

AB01: I strongly suggest against having the whole Base64 class in the repository. It does too much. A simple
implementation such as the one in java.util.prefs.Base64 would suit the needs of the DB Explorer just as well, and,
unlike Base64, it can actually be understood in a couple of minutes. Now that NetBeans is under GPL as well we probably
can just take java.util.prefs.Base64.

AB02: I don't think Base64.encodeObject() is the method you want to call. What you want is to convert the password
string to UTF-8 bytes and convert that to base64. Consider that we are allowing module writers to specify a password
when registering a connection in the layer, which is a new API. What are you going to tell an API client? "The password
needs to be encoded by the Base64.encodeObject() method"? You don't want to tell him that. Base64 is not in the API, and
what encodeObject() does is too Java-specific. You need a method like encodeUTF8Bytes().

AB03: The new password element and its value needs to be documented in arch.xml in the use cases section.

AB04: No need for the base64 constant in DatabaseConnectionConvertorTest. Just call Base64.encodeUTF8Bytes('password').

AB05: Instead of calling "conn.setRememberPassword(true)" in DatabaseConnectionConvertorTest can't you just use the new
constructor in DatabaseConnection taking remembedPassword as a parameter?

AB06: You need to upload connection-1_1.dtd to http://www.netbeans.org/dtds/connection-1_1.dtd by putting it in the
www/www/dtds directory in CVS.

AB01: I strongly suggest against having the whole Base64 class in the repository. It does too much. A simple
implementation such as the one in java.util.prefs.Base64 would suit the needs of the DB Explorer just as well, and,
unlike Base64, it can actually be understood in a couple of minutes. Now that NetBeans is under GPL as well we probably
can just take java.util.prefs.Base64.

David: I didn't know java.util.prefs.Base64 existed, and very good point that since we're both GPL now (OpenJDK as well
as NB) then I should be able to use it.  Checking with legal to make sure we're kosher.

AB02: I don't think Base64.encodeObject() is the method you want to call. What you want is to convert the password
string to UTF-8 bytes and convert that to base64. Consider that we are allowing module writers to specify a password
when registering a connection in the layer, which is a new API. What are you going to tell an API client? "The password
needs to be encoded by the Base64.encodeObject() method"? You don't want to tell him that. Base64 is not in the API, and
what encodeObject() does is too Java-specific. You need a method like encodeUTF8Bytes().

David: Ah, I see.  I missed that users can register their own connections, thanks for alerting me to that.  And yes, I
see that the password string needs to be an encoding of bytes, not of a Java object, that makes total sense.  But
clients of the API are still going to need to encode the password if they want to include it in their XML file to
auto-register a connection.  What are we going to suggest they do to achieve that?  "You need to encode the password as
a Base64 encoding of a UTF8 string."?  I guess so... But if I were an API client, I'd probably just not specify the
password...

Another question: Does this change need an API review since I'm adding password to the DTD?

AB03: The new password element and its value needs to be documented in arch.xml in the use cases section.

David: OK.

AB04: No need for the base64 constant in DatabaseConnectionConvertorTest. Just call Base64.encodeUTF8Bytes('password').

David: OK, makes sense, thanks.

AB05: Instead of calling "conn.setRememberPassword(true)" in DatabaseConnectionConvertorTest can't you just use the new
constructor in DatabaseConnection taking rememberPassword as a parameter?

David: I'll take a look at that.

AB06: You need to upload connection-1_1.dtd to http://www.netbeans.org/dtds/connection-1_1.dtd by putting it in the
www/www/dtds directory in CVS.

David: Cheez, how am I supposed to know this stuff? :)  OK, will do.

> What are we going to suggest they do to achieve that?  "You need to encode
> the password as a Base64 encoding of a UTF8 string."?

Yeah, just that. It might look complex at first, but usually it will not be. Most people will use only ASCII characters
in their passwords, whose UTF-8 encoding is exactly those characters (UTF-8 for 'foo' is 'foo'). So all they need to do
is feed the password through a base64 online encoder, for example.

> Another question: Does this change need an API review since I'm adding
> password to the DTD?

Very good catch. Yes, it would be good to do a fast-track review for this.

> Cheez, how am I supposed to know this stuff? :)

You aren't :-) I don't know if this is documented somewhere or it is just passed down through generations. I was told
about it in IssueZilla too.

This is a request for a fast-track review for this API change to the DB explorer.  

I am adding a new 'password' element to the connection DTD that is used for storing connection information, upgrading
the version from 1.0 to 1.1.

I will attach a diff to the change between versions 1.0 and 1.1 of the DTD, as well as changes to the arch.xml and
apichanges.xml files.

Andrei and I caught that this needed a review only after checking in the change.  Please let me know if you need
anything else for this review.

Here is the diff between connection-1_0.dtd and connection-1_1.dtd:

a1
> <?xml version="1.0" encoding="UTF-8"?>
21c22
< <!ELEMENT connection (driver-class,driver-name,database-url,schema,user)>
---
> <!ELEMENT connection (driver-class,driver-name,database-url,schema,user,password)>
25c26
51a53,57
> <!--- The database password (hashed). -->
> <!ELEMENT password EMPTY>
> <!ATTLIST password
>     value CDATA #REQUIRED

Created attachment 50293 [details]
Changes to the arch.xml and apichanges.xml files

AB01: You need to increment the specification version to 1.23 in project.properties.

AB02: The class element in apichanges.xml mentions a non-API DatabaseConnection class. Actually probably there is no
need for a class element at all.

AB03: Since the password is optional, the connection element in connection-1_1.dtd should be defined as

<!ELEMENT connection (driver-class,driver-name,database-url,schema,user,password?)>

AB04: Suggest not mentioning any external URLs such as http://www.motobit.com/util/base64-decoder-encoder.asp, since
they can become broken at any time. Actually, suggest the following explanation:

The password element is optional, but if it is included, its value must be the Base64 encoding of the UTF-8
representation of the password. Note the Base64 encoding serves as a simple scrambling to prevent accidental reveal and
it is not indended to offer any real security. Also note that the UTF-8 representation of passwords composed entirely of
ASCII characters is the same as their ASCII representation, so for such passwords all that needs to be done is
converting them to Base64.

Thanks, Andrei, I'll take care of these.

I agree with the change.

Just re-adding the API and API_REVIEW_FAST keywords.

Requesting another late UI change for this issue.  This was not initially proposed, but further work with this feature
has made it clear we should automatically connect without using the Connect dialog when the password has been saved.  

The user experience will change like this:

*Password not saved*
No change from current experience

*Password saved*
User chooses Connect... from the context menu for a connection, and a progress bar is displayed until the connection is
established.  If the connection fails, the Connect dialog is displayed along with an error message, so the user has the
opportunity to change the parameters for the connection.

If the user wants to stop saving the password or otherwise view the Connect dialog, they can choose Properties... from
the context menu for the connection, and unclick 'Save Password'.  Then they can connect and the Connect dialog is
displayed.

The justification for this change is that for most users, once they save the password, they want to quickly reconnect. 
The exception case is if the user wants to change the password or stop saving it. So we should optimize for the common
case where a quick reconnect is desired.

Committing changes based on API review feedback.

Awaiting late UI approval before committing change so that the connect dialog is not brought up if the password is saved.

Checking in src/org/netbeans/modules/db/explorer/DatabaseConnectionConvertor.java;
/cvs/db/src/org/netbeans/modules/db/explorer/DatabaseConnectionConvertor.java,v  <--  DatabaseConnectionConvertor.java
new revision: 1.12; previous revision: 1.11
done
Checking in src/org/netbeans/modules/db/explorer/DatabaseConnection.java;
/cvs/db/src/org/netbeans/modules/db/explorer/DatabaseConnection.java,v  <--  DatabaseConnection.java
new revision: 1.54; previous revision: 1.53
done
Checking in test/unit/src/org/netbeans/modules/db/explorer/DatabaseConnectionConvertorTest.java;
/cvs/db/test/unit/src/org/netbeans/modules/db/explorer/DatabaseConnectionConvertorTest.java,v  <-- 
DatabaseConnectionConvertorTest.java
new revision: 1.12; previous revision: 1.11
done
Checking in test/unit/src/org/netbeans/modules/db/explorer/bar-connection.xml;
/cvs/db/test/unit/src/org/netbeans/modules/db/explorer/bar-connection.xml,v  <--  bar-connection.xml
new revision: 1.6; previous revision: 1.5
done
Checking in test/unit/src/org/netbeans/modules/db/util/Base64Test.java;
/cvs/db/test/unit/src/org/netbeans/modules/db/util/Base64Test.java,v  <--  Base64Test.java
new revision: 1.2; previous revision: 1.1
done
Checking in apichanges.xml;
/cvs/db/apichanges.xml,v  <--  apichanges.xml
new revision: 1.9; previous revision: 1.8
done
Checking in arch.xml;
/cvs/db/arch.xml,v  <--  arch.xml
new revision: 1.18; previous revision: 1.17
done
Checking in src/org/netbeans/modules/db/resources/connection-1_1.dtd;
/cvs/db/src/org/netbeans/modules/db/resources/connection-1_1.dtd,v  <--  connection-1_1.dtd
new revision: 1.2; previous revision: 1.1
done
Checking in nbproject/project.properties;
/cvs/db/nbproject/project.properties,v  <--  project.properties
new revision: 1.28; previous revision: 1.27
done
Checking in src/org/netbeans/modules/db/util/Base64.java;
/cvs/db/src/org/netbeans/modules/db/util/Base64.java,v  <--  Base64.java
new revision: 1.2; previous revision: 1.1
done

In the Connection dialog, the label next to the Remember password mentions to look in the help
It would be better to have a Help button or have a link to an article / whitepaper on how the passwords
are stored

I think that the message next to remember password should be removed. please consider moving it to the line below or to
an inline text area below. 

I would agree with a help id and will approve such a UI change.
I can add the help topic explaining the new behaviour (though cannot promise for beta2 at this stage, but possibly if id
is there before branch) and I would suggest the help topic also have a link to the wiki faqs with greater detail on how
the password is saved and on how to secure the password.

Jim had said the UI change to save the password already passed Late UI Change review, and his change didn't include a
help button.  So  this code is already checked in, I thought we were cool with this.

But anyway, water under the bridge, we can try to fix this in the next few days.

I didn't hear any objections about not bringing up the dialog, which was the focus of the Late UI Change review request
I created.  Does that means it's approved?  I'm not sure how this process works...

Thanks,

David

The late UI process:  the late UI page will get updated with an OK for this task
Jano approved and if "OK" wasn't added for the late UI item  then mark it OK with a 
commment - approved by Jano

 Added Help button to the dialog, removed extra checkbox text in the dialog and created help id "db_save_password" 
 After removing the extra text, the dialog was packed, thus shrunk, so had to set the preferred size (prototype display
value) of the schema dropdown
 

cvs commit: Examining db/src/org/netbeans/modules/db/explorer/dlg
cvs commit: Examining db/src/org/netbeans/modules/db/resources
Checking in db/src/org/netbeans/modules/db/explorer/dlg/ConnectionDialog.java;
/cvs/db/src/org/netbeans/modules/db/explorer/dlg/ConnectionDialog.java,v  <--  ConnectionDialog.java
new revision: 1.15; previous revision: 1.14
done
Checking in db/src/org/netbeans/modules/db/explorer/dlg/SchemaPanel.form;
/cvs/db/src/org/netbeans/modules/db/explorer/dlg/SchemaPanel.form,v  <--  SchemaPanel.form
new revision: 1.7; previous revision: 1.6
done
Checking in db/src/org/netbeans/modules/db/explorer/dlg/SchemaPanel.java;
/cvs/db/src/org/netbeans/modules/db/explorer/dlg/SchemaPanel.java,v  <--  SchemaPanel.java
new revision: 1.19; previous revision: 1.18
done
Checking in db/src/org/netbeans/modules/db/resources/Bundle.properties;
/cvs/db/src/org/netbeans/modules/db/resources/Bundle.properties,v  <--  Bundle.properties
new revision: 1.114; previous revision: 1.113
done
Checking in db/src/org/netbeans/modules/db/resources/org-netbeans-modules-db-mainpage.xml;
/cvs/db/src/org/netbeans/modules/db/resources/org-netbeans-modules-db-mainpage.xml,v  <-- 
org-netbeans-modules-db-mainpage.xml
new revision: 1.4; previous revision: 1.3
done

I ended up removing the extra text for the checkbox 

Before:
&Remember password (potentially insecure; see help for how to secure your password)
After:
&Remember password

Is this sufficient or should the warning about a potentially insecure password stand out?
This will be documented in the Help topic, I assume ?

Ken requested distinct help ids for the Connect and New Database Connection dialog
I will attach diffs

Created attachment 50843 [details]
diff of changes to make for using distinct help ids

This looks good to me.

Connect dialog now is not brought up - just progress bar - when the password is saved and you connect from DB Explorer.

Checking in ConnectAction.java;
/cvs/db/src/org/netbeans/modules/db/explorer/actions/ConnectAction.java,v  <--  ConnectAction.java
new revision: 1.47; previous revision: 1.46
done

There is one problem remaining with this issue.  If the password is changed for a user, then an error dialog is raised,
but after pressing [OK] the Connect dialog should be brought up but it's not.  I'm working on this.  

I am still working on solving this problem, but I am not going to push to get this fix in for Beta 2.  

The behavior:

- You save the password
- Administrator changes the password for that user on the database
- You try to connect, and the connection fails, and you don't get the Connect dialog

The reasonable workaround is to disable 'save password' and then you get the Connect dialog.

I'll log a separate P3 bug for this and I'm going to mark this issue as fixed.

improved the implementation of Help for the Connection dialogs

Checking in db/src/org/netbeans/modules/db/explorer/actions/ConnectAction.java;
/cvs/db/src/org/netbeans/modules/db/explorer/actions/ConnectAction.java,v  <--  ConnectAction.java
new revision: 1.48; previous revision: 1.47
done
Checking in db/src/org/netbeans/modules/db/explorer/actions/ConnectUsingDriverAction.java;
/cvs/db/src/org/netbeans/modules/db/explorer/actions/ConnectUsingDriverAction.java,v  <--  ConnectUsingDriverAction.java
new revision: 1.38; previous revision: 1.37
done
Checking in db/src/org/netbeans/modules/db/explorer/dlg/ConnectionDialog.java;
/cvs/db/src/org/netbeans/modules/db/explorer/dlg/ConnectionDialog.java,v  <--  ConnectionDialog.java
new revision: 1.18; previous revision: 1.17