This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
Hi, I use the db explorer in NB a lot. It would be very nice to have NB remember the db password not just for the current session, but also in between sessions. That way NB users won't have to enter the db password again after starting up NB. Perhaps this feature could be configurable on a per database basis. Thanks in advance, Wouter van Reeven
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
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