[SERVER-10318] getLastError() always returns "getLastError command failed: need to login" Created: 24/Jul/13  Updated: 10/Mar/23  Resolved: 29/Jul/13

Status: Closed
Project: Core Server
Component/s: Internal Client
Affects Version/s: 2.2.4, 2.2.5
Fix Version/s: None

Type: Bug Priority: Major - P3
Reporter: Balint Szente Assignee: Spencer Brody (Inactive)
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified
Environment:

Debian Linux 7 x86_64, mongodb-10gen=2.2.5


Issue Links:
Related
related to CXX-107 The C++ Driver should define a versio... Closed
is related to DOCS-1749 Document default form of GLE that def... Closed
Operating System: Linux
Steps To Reproduce:

Steps to reproduce:

  1. Install Debian 7 x86_64 add the 10gen repo
  2. apt-get install mongodb-10gen=2.2.5
  3. Enable auth = true in /etc/mongodb.conf
  4. /etc/init.d/mongodb restart
  5. Create the test database:

    > use admin
    > db.addUser("admin", "admin")
    > use testdb
    > db.addUser("testuser", "testpass")

  6. Get the mongodb-src-r2.2.5.tar.gz tarball
  7. Compile only the client with the following command:

    scons --use-system-boost --release mongoclient

  8. Compile the following simple C++ test.cpp program:

    test.cpp

    #include <iostream>
    #include <string>
    #include <mongo/client/dbclient.h>
     
    using namespace std;
     
    int main() {
      mongo::DBClientConnection connection;
      string host = "localhost:27017";
      connection.connect(host);
     
      string error;
      connection.auth("testdb", "testuser", "testpass", error, true);
      cout << "Auth: " << error << endl;
     
      mongo::BSONObj obj = BSON("name" << "Dummy text");
     
      connection.insert("testdb.unittest", obj);
      error = connection.getLastError();
      cout << "insert getLastError(): " << error << endl;
     
      connection.query("testdb.unittest", BSON("name" << "Dummy text"));
      error = connection.getLastError();
      cout << "query getLastError(): " << error << endl;
     
      return 0;
    }

  9. g++ test.cpp -lmongoclient -lboost_filesystem -lboost_thread -lboost_system
  10. Run a.out

Current result:

$ ./a.out
Auth: 
insert getLastError(): getLastError command failed: need to login
query getLastError(): getLastError command failed: need to login

Expected result:

$ ./a.out
Auth: 
insert getLastError(): 
query getLastError():

Some notes:

  • The {{ {"name": "Dummy text"}

    }} object will be actually inserted into the database each time when the program is run, so getLastError() should NOT return error.

  • Version 2.2.3 inclusive works fine.
  • Between version 2.2.3 and 2.2.4 there were changes in the authentication table implementation.
  • Versions 2.4.x seems to be fine as well.
Participants:

 Description   

Starting from the version 2.2.4 of the Mongo Client C++ driver calling the getLastError() after a successful operation always results in "getLastError command failed: need to login" error when authentication is enabled.



 Comments   
Comment by Spencer Brody (Inactive) [ 30/Jul/13 ]

My pleasure! Glad I could help.

Comment by Balint Szente [ 30/Jul/13 ]

I'm sorry, I was not online today. It is OK that you have closed the issue. Everything is clear now.

Thank you very much for your support.

Comment by Spencer Brody (Inactive) [ 29/Jul/13 ]

Hi Balint,
I'm going to go ahead and resolve this issue as I believe all of your questions about this have been answered, but please let us know if you have any further questions.

Cheers,
-Spencer

Comment by Spencer Brody (Inactive) [ 29/Jul/13 ]

Yep, that's right.

Comment by Balint Szente [ 29/Jul/13 ]

I'm sorry, it is my mistake at point 2.2. Copy paste typo: it should be without db, like this:

if server is 2.2.x then call getLastError(db) for client >= 2.2.1 and getLastError() for client <=2.2.0

I think this way it makes sense I will mark as deleted the db in my previous comment to avoid confusion.

So for fixing the conclusion:

  1. In case of client <= 2.2.0 in combination with server 2.2.x will/should always give need to login error because of runCommand("admin", getlasterrorcmdobj, info)?
  2. And in order to circumvent this, manual invoking is necessary for the correct database, like runCommand("testdb", getlasterrorcmdobj, info), right?
Comment by Spencer Brody (Inactive) [ 29/Jul/13 ]

I'm not sure I understand point 2.2, you say to use the same call for getLastError both for clients older and newer than 2.2.1. Given that the default form of getLastError that defaults to the admin database is the only form available to clients before 2.2.1, and the dbname is necessary for GLE when talking to a 2.2.x server, I don't think you can use the built-in helper for calling GLE when using a client <= 2.2.0 with a server that is 2.2.x. In those cases, I think you'll have to use runCommand to issue the getLastError command manually. You can see an example of how to do this by looking at DBClientWithCommands::getLastErrorDetailed.

So I think your conclusion is correct except that I would change the last point to say to use runCommand directly when the client is <= 2.2.0 and you're talking to a 2.2.x server.

Comment by Balint Szente [ 29/Jul/13 ]

Thank you for the detailed explanation. Yes, I was using mongodb-10gen=2.2.5 server for all the above C++ client tests.

I made a new set of tests with mongodb-10gen=2.4.5 and all the above C++ clients. Everything was fine: both getLastError() and getLastError(db) return success with every client version.

So, as a conclusion:

  1. simple getLastError() is not deprecated, only some of the later 2.2 server and client combinations are broken.
  2. So basically the following algorithm should be used to correctly get the last error:
    1. if server is NOT 2.2.x then just call getLastError() no matter the client version
    2. if server is 2.2.x then call getLastError(db) for client >= 2.2.1 and getLastError(db) for client <=2.2.0

Unfortunately there is no way to determine the client version at compile time in order to call the correct function prototype, thus I filed the SERVER-10369 bug report for this.

Comment by Spencer Brody (Inactive) [ 26/Jul/13 ]

Hi Balint,
Thank you for the detailed analysis of the problem, and I apologize for the confusion in this area.

I assume you were using something in the 2.2 series for the server for all the tests, is that correct? I'm a bit surprised about the results above, I would have expected all versions of the C++ driver in 2.2 to exhibit the same behavior, and all driver versions of 2.4 to exhibit the same behavior.

Let's see if I can give you some context around what happened here. Historically, the GLE command did not require authorization to run. Anyone could run GLE, even on an unauthenticated connection, and it would work. Because it only shows errors from previous writes on this same connection, this wasn't a security hole, because to get a meaningful error you must have been authenticated and authorized to perform a write. In 2.2, GLE was changed to require authorization, as part of an attempt to have better security practice in general. This, however, caused a lot of problems for drivers and broke what was then the only way to call GLE in the C++ driver, as it defaulted to the "admin" database, but now you had to send GLE to a database you actually had authorization for. That is why in 2.2.1 we added the version of GLE to the C++ driver that takes a database name.

In 2.4, however, we removed the authorization requirement from GLE, bringing it back to its pre-2.2 behavior, since it wasn't really a security hole to begin with and only succeeded in complicating the logic for clients and drivers. So all these problems should only matter on 2.2 versions of the server, all other versions, past and future, should work with sending GLE to any database and thus the default driver call should work fine.

I filed DOCS-1749 to add back the default version of GLE to the docs for the C++ driver.

I'm sorry about the confusion around this, we've been investing a lot into security recently and this was a case were we traded off usability for a false sense of security (which was rectified in the next major release, but is still frustrating to those who are still actively using v2.2). Hopefully this explanation cleared some things up around what happened here and why - please let me know if you have any further questions.

-Spencer

Comment by Balint Szente [ 26/Jul/13 ]

Very good catch! I was aware of the two function prototypes, but the implicit char* to bool conversion is so illogical that I never thought about it. Unfortunately the implicit type conversions in C++ are black magic. I can confirm the proposed solution works:

  connection.getLastError(string("testdb"));

In order to avoid this very confusing situation please add also a signature with char*:

string getLastError(const char* db, bool fsync = false, bool j = false, int w = 0, int wtimeout = 0);

There is one more thing to clarify regarding the getLastError() API:

  1. Doxygen for getLastError() in 1.8.x, 2.0.x and 2.2.0 says the following:

    Get error result from the last write operation (insert/update/delete) on this connection.

    We use this in our code.

  2. Doxygen for getLastError(db, ...) in 2.2.1 introduced the mandatory db argument and from Doxygen disappeared the variant without the arguments. The simple getlastError() became undocumented function and its behavior was changed in 2.2.4. This is API break.

Function behavior summary:

C++ Driver API 1.8.x 2.2.0 2.2.1 2.2.2 2.2.3 2.2.4 2.2.5 2.4.0 2.4.1 2.4.2 2.4.3 2.4.4 2.4.5
getLastError()
getLastError(db)

Legend:
– works as designed, function performed with success
– works as designed, function returns need to login error because of the default admin database
– does not work as designed, should give error but instead returns success as in pre 2.2.1 versions creating confusion for the API user (that's why we did not even observed the new getLastError implementation, because our project compiled fine and worked without any problem)

So as a summary – the correct call of getLastError function to obtain the error for last operation on this connection would be:

  1. always getLastError() – if Mongo Client version <= 2.2.0, because it's the only available function
  2. always the new getLastError(db) – if Mongo Client version >=2.2.1, because the implementation of the "legacy" getLastError() has changed as well and became inconsistent, buggy and undocumented

Questions:

  1. Is this above call summary correct?
  2. How can I check the Mongo Client version at compile time to select the proper signature? Is there any defined macro to detect the version?
  3. What was the reason to add the db argument, if the current connection is authenticated anyway and the function returns the status of last operation per connection?
  4. Why the getLastError() became undocumented and silently defaulting to admin changing the behavior, instead of defaulting to the currently authenticated and modified database to keep the API behavior as it was until 2.2.0?
  5. What was the reason to make an API break in a bugfix release (2.2.1) and not in a feature/development release (2.3.0+)?

MongoDB is really a great project and I would like to understand the above decisions. I'm sure there were good reasons that are not obvious at first sight, especially for a non MongoDB developer. Thank you for your time and understanding.

Comment by Spencer Brody (Inactive) [ 25/Jul/13 ]

Okay, I was able to reproduce the behavior you saw.

The problem is that the getLastError method is overloaded. There are two signatures for it:

string getLastError(const std::string& db, bool fsync = false, bool j = false, int w = 0, int wtimeout = 0);

and

string getLastError(bool fsync = false, bool j = false, int w = 0, int wtimeout = 0);

When you call getLastError("testdb"), "testdb" is a char* under the hood. char* can be implicitly converted to a std::string, but it can also be implicitly converted to a bool. It is the latter conversion that is happening here, causing the second form of getLastError to be called, thus setting the fync parameter to true, but not changing the db away from the default of "admin". If you change your call from getLastError("testdb") to getLastError(std::string("testdb")), that should tell the compiler to interpret the first argument to getLastError as a string not a bool, so it should then do the right thing.

Please try that out and let me know if it fixes things for you. Sorry for the confusion.

Comment by Balint Szente [ 25/Jul/13 ]

I'm afraid this is not the case. Using

connection.getLastError("testdb")

does not help. The error is the same.

Using connection.getLastError() produces the following server log:

Thu Jul 25 08:41:52 [initandlisten] connection accepted from 127.0.0.1:54677 #25 (9 connections now open)
Thu Jul 25 08:41:52 [conn25] run command testdb.$cmd { getnonce: 1 }
Thu Jul 25 08:41:52 [conn25] command testdb.$cmd command: { getnonce: 1 } ntoreturn:1 keyUpdates:0  reslen:65 0ms
Thu Jul 25 08:41:52 [conn25] run command testdb.$cmd { authenticate: 1, nonce: "6a3309f56725e353", user: "testuser", key: "4d611fdfa906bb0f65af0a980edbca87" }
Thu Jul 25 08:41:52 [conn25]  authenticate db: testdb { authenticate: 1, nonce: "6a3309f56725e353", user: "testuser", key: "4d611fdfa906bb0f65af0a980edbca87"}
Thu Jul 25 08:41:52 [conn25] command testdb.$cmd command: { authenticate: 1, nonce: "6a3309f56725e353", user: "testuser", key: "4d611fdfa906bb0f65af0a980edbca87" } ntoreturn:1 keyUpdates:0 locks(micros) r:90 reslen:86 0ms
Thu Jul 25 08:41:52 [conn25] insert testdb.unittest keyUpdates:0 locks(micros) w:76 0ms
Thu Jul 25 08:41:52 [conn25] run command admin.$cmd { getlasterror: 1 }
Thu Jul 25 08:41:52 [conn25] command admin.$cmd command: { getlasterror: 1 } ntoreturn:1 keyUpdates:0  reslen:63 0ms
Thu Jul 25 08:41:52 [conn25] query testdb.unittest query: { name: "Dummy text" } ntoreturn:0 keyUpdates:0 locks(micros) r:65 nreturned:9 reslen:407 0ms
Thu Jul 25 08:41:52 [conn25] run command admin.$cmd { getlasterror: 1 }
Thu Jul 25 08:41:52 [conn25] command admin.$cmd command: { getlasterror: 1 } ntoreturn:1 keyUpdates:0  reslen:63 0ms
Thu Jul 25 08:41:52 [conn25] end connection 127.0.0.1:54677 (8 connections now open)

Specifying the database connection.getLastError("testdb") generates the very same log:

Thu Jul 25 08:45:53 [initandlisten] connection accepted from 127.0.0.1:54915 #26 (9 connections now open)
Thu Jul 25 08:45:53 [conn26] run command testdb.$cmd { getnonce: 1 }
Thu Jul 25 08:45:53 [conn26] command testdb.$cmd command: { getnonce: 1 } ntoreturn:1 keyUpdates:0  reslen:65 0ms
Thu Jul 25 08:45:53 [conn26] run command testdb.$cmd { authenticate: 1, nonce: "6aac6cef259dbfd4", user: "testuser", key: "d9e1c69f56a5a07ad3cc8891881a613e" }
Thu Jul 25 08:45:53 [conn26]  authenticate db: testdb { authenticate: 1, nonce: "6aac6cef259dbfd4", user: "testuser", key: "d9e1c69f56a5a07ad3cc8891881a613e"}
Thu Jul 25 08:45:53 [conn26] command testdb.$cmd command: { authenticate: 1, nonce: "6aac6cef259dbfd4", user: "testuser", key: "d9e1c69f56a5a07ad3cc8891881a613e" } ntoreturn:1 keyUpdates:0 locks(micros) r:85 reslen:86 0ms
Thu Jul 25 08:45:53 [conn26] insert testdb.unittest keyUpdates:0 locks(micros) w:85 0ms
Thu Jul 25 08:45:53 [conn26] run command admin.$cmd { getlasterror: 1, fsync: 1 }
Thu Jul 25 08:45:53 [conn26] command admin.$cmd command: { getlasterror: 1, fsync: 1 } ntoreturn:1 keyUpdates:0  reslen:63 0ms
Thu Jul 25 08:45:53 [conn26] query testdb.unittest query: { name: "Dummy text" } ntoreturn:0 keyUpdates:0 locks(micros) r:85 nreturned:10 reslen:450 0ms
Thu Jul 25 08:45:53 [conn26] run command admin.$cmd { getlasterror: 1, fsync: 1 }
Thu Jul 25 08:45:53 [conn26] command admin.$cmd command: { getlasterror: 1, fsync: 1 } ntoreturn:1 keyUpdates:0  reslen:63 0ms
Thu Jul 25 08:45:53 [conn26] end connection 127.0.0.1:54915 (8 connections now open)

Notes:

  1. The problem is only with 2.2.4 and 2.2.5, where the authentication code was touched.
  2. Please keep in mind that 1.8.x, 2.0.x, 2.2.0-2.2.3 and 2.4.x works well even without specifying the database as argument. It returns the proper error message for the last database operation. However, no matter what database argument do you put for getLastError(...), in the server logs admin will appear. And this leads to the question: what is the db argument really used for in getLastError(...)?

Could you please test the steps described and reopen the issue? Thank you.

Comment by Spencer Brody (Inactive) [ 24/Jul/13 ]

This is because getLastError defaults to being sent to the "admin" database, but you're writing to the "testdb" database. If you use

connection.getLastError("testdb")

it should work.

Generated at Thu Feb 08 03:22:51 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.