Merge pull request #408 from MrCreosote/develop

PTV-1890: Fix login bug wrt anonymous IDs

src/us/kbase/auth2/lib/storage/mongo/MongoStorage.java

@@ -748,33 +748,49 @@ private Document getUserDoc(
 			final boolean local)
 			throws AuthStorageException, NoSuchUserException {
 		requireNonNull(userName, "userName");
-		Document user = getUserDocWithoutPassword(collection, userName);
+		return getUserDoc(
+				collection, new Document(Fields.USER_NAME, userName.getName()), local, userName);
+
+	}
+
+	private Document getUserDoc(
+			final String collection,
+			final Document query,
+			final boolean local,
+			final UserName nameForException)
+			throws AuthStorageException, NoSuchUserException {
+		Document user = getUserDocWithoutPassword(collection, query);
 		if (user == null) {
-			throw new NoSuchUserException(userName.getName());
+			if (nameForException != null) {
+				throw new NoSuchUserException(nameForException.getName());
+			} else {
+				return null;
+			}
 		}
+		final String userName = user.getString(Fields.USER_NAME);
 		if (user.getString(Fields.USER_ANONYMOUS_ID) == null) {
 			user = updateUserAnonID(collection, userName);
 		}
 		if (local && !user.getBoolean(Fields.USER_LOCAL)) {
-			throw new NoSuchLocalUserException(userName.getName());
+			throw new NoSuchLocalUserException(userName);
 		}
 		return user;
 	}
 
-	private Document getUserDocWithoutPassword(final String collection, final UserName userName)
+	private Document getUserDocWithoutPassword(final String collection, final Document query)
 			throws AuthStorageException {
 		return findOne(
 				collection,
-				new Document(Fields.USER_NAME, userName.getName()),
+				query,
 				new Document(Fields.USER_PWD_HSH, 0).append(Fields.USER_SALT, 0));
 	}
 
-	private Document updateUserAnonID(final String collection, final UserName userName)
+	private Document updateUserAnonID(final String collection, final String userName)
 			throws AuthStorageException {
 		/* Added in version 0.6.0 when anonymous IDs were added to users. This method lazily
 		 * backfills the anonymous ID on an as-needed basis.
 		 */
-		final Document query = new Document(Fields.USER_NAME, userName.getName())
+		final Document query = new Document(Fields.USER_NAME, userName)
 				// only modify if not already done to avoid race conditions
 				.append(Fields.USER_ANONYMOUS_ID, null);
 		final Document update = new Document(
@@ -788,10 +804,11 @@ private Document updateUserAnonID(final String collection, final UserName userNa
 					"Connection to database failed: " + e.getMessage(), e);
 		}
 		// pull the user from the DB again to avoid a race condition here
-		final Document userdoc = getUserDocWithoutPassword(collection, userName);
+		final Document userdoc = getUserDocWithoutPassword(
+				collection, new Document(Fields.USER_NAME, userName));
 		if (userdoc == null) {
 			throw new RuntimeException(
-					"User unexpectedly not found in database: " + userName.getName());
+					"User unexpectedly not found in database: " + userName);
 		}
 		return userdoc;
 	}
@@ -1526,12 +1543,12 @@ Fields.ROLES_ID, new Document("$in", roles))).stream()
 
 	@Override
 	public Optional<AuthUser> getUser(final RemoteIdentity remoteID) throws AuthStorageException {
-		final Document query = makeUserQuery(remoteID);
-		//note a user with identities should never have these fields, but
-		//doesn't hurt to be safe
-		final Document projection = new Document(Fields.USER_PWD_HSH, 0)
-				.append(Fields.USER_SALT, 0);
-		final Document u = findOne(COL_USERS, query, projection);
+		final Document u;
+		try {
+			u = getUserDoc(COL_USERS, makeUserQuery(remoteID), false, null);
+		} catch (NoSuchUserException e) {
+			throw new RuntimeException("This should be impossible", e);
+		}
 		if (u == null) {
 			return Optional.empty();
 		}

src/us/kbase/test/auth2/lib/storage/mongo/MongoStorageAnonymousIDTest.java

@@ -116,6 +116,35 @@ private void backfillMissingAnonIDStdUser(final Document update) throws Exceptio
 		assertThat("incorrect user", got, is(expected));
 	}
 
+	@Test
+	public void backFillMissingAnonIDNoFieldStdUserGetByIdentity() throws Exception {
+		backFillMissingAnonIDNoFieldStdUserGetByIdentity(
+				new Document("$unset", new Document("anonid", "")));
+	}
+
+	@Test
+	public void backFillMissingAnonIDWithFieldStdUserGetByIdentity() throws Exception {
+		backFillMissingAnonIDNoFieldStdUserGetByIdentity(
+				new Document("$set", new Document("anonid", null)));
+	}
+
+	private void backFillMissingAnonIDNoFieldStdUserGetByIdentity(final Document update)
+			throws Exception {
+		storage.createUser(NewUser.getBuilder(
+				new UserName("foo"), UID, new DisplayName("d"), NOW, REMOTE1).build());
+		db.getCollection("users").updateOne(new Document("user", "foo"), update);
+
+		final UUID uid2 = UUID.randomUUID();
+		when(manager.mockRand.randomUUID()).thenReturn(uid2, (UUID) null);
+
+		final AuthUser got = storage.getUser(REMOTE1).get();
+		final AuthUser expected = AuthUser.getBuilder(
+				new UserName("foo"), uid2, new DisplayName("d"), NOW)
+				.withIdentity(REMOTE1)
+				.build();
+		assertThat("incorrect user", got, is(expected));
+	}
+
 	/*
 	 * The following tests test race conditions by accessing the internal update method
 	 * directly. At this point the userdoc has already been pulled from the database and
@@ -125,7 +154,7 @@ private void backfillMissingAnonIDStdUser(final Document update) throws Exceptio
 
 	private Method getAnonIDUpdateMethod() throws Exception {
 		final Method m = MongoStorage.class
-				.getDeclaredMethod("updateUserAnonID", String.class, UserName.class);
+				.getDeclaredMethod("updateUserAnonID", String.class, String.class);
 		m.setAccessible(true);
 		return m;
 	}
@@ -143,7 +172,7 @@ public void updateUserAnonIDWithRaceOnUpdate() throws Exception {
 		when(manager.mockRand.randomUUID()).thenReturn(UUID.randomUUID(), (UUID) null);
 
 		final Document userdoc = (Document) getAnonIDUpdateMethod()
-				.invoke(storage, "users", new UserName("foo"));
+				.invoke(storage, "users", "foo");
 
 		// only check a few properties, the methods above test exhaustively
 		assertThat("incorrect user", userdoc.getString("user"), is("foo"));
@@ -167,7 +196,7 @@ public void updateUserAnonWithMissingDocument() throws Exception {
 		when(manager.mockRand.randomUUID()).thenReturn(UUID.randomUUID(), (UUID) null);
 
 		try {
-			getAnonIDUpdateMethod().invoke(storage, "users", new UserName("bar"));
+			getAnonIDUpdateMethod().invoke(storage, "users", "bar");
 			fail("expected exception");
 		} catch (InvocationTargetException e) {
 			assertExceptionCorrect(e.getCause(), new RuntimeException(