Skip to content

Commit bba441b

Browse files
committed
Fix null MFA field handling for backward compatibility
- Add getMfaStatus helper method to handle null MFA fields in database - Update token and identity deserialization to use helper method - Add tests for backward compatibility with existing database records - Prevents NullPointerException when MFA field is missing from stored data
1 parent 6768eaf commit bba441b

File tree

3 files changed

+59
-2
lines changed

3 files changed

+59
-2
lines changed

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -569,6 +569,10 @@ private DisplayName getDisplayName(final String displayName) throws AuthStorageE
569569
}
570570
}
571571

572+
private MfaStatus getMfaStatus(final String mfaString) {
573+
return mfaString != null ? MfaStatus.valueOf(mfaString) : MfaStatus.UNKNOWN;
574+
}
575+
572576
private EmailAddress getEmail(final String email) throws AuthStorageException {
573577
if (email == null) {
574578
return EmailAddress.UNKNOWN;
@@ -971,7 +975,7 @@ private StoredToken getToken(final Document t) throws AuthStorageException {
971975
t.getDate(Fields.TOKEN_EXPIRY).toInstant())
972976
.withNullableTokenName(getTokenName(t.getString(Fields.TOKEN_NAME)))
973977
.withContext(toTokenCreationContext(t))
974-
.withMfa(MfaStatus.valueOf(t.getString(Fields.TOKEN_MFA)))
978+
.withMfa(getMfaStatus(t.getString(Fields.TOKEN_MFA)))
975979
.build();
976980
}
977981

@@ -1795,7 +1799,7 @@ private Set<RemoteIdentity> toIdentities(final List<Document> ids) {
17951799
i.getString(Fields.IDENTITIES_USER),
17961800
i.getString(Fields.IDENTITIES_NAME),
17971801
i.getString(Fields.IDENTITIES_EMAIL),
1798-
MfaStatus.valueOf(i.getString(Fields.IDENTITIES_MFA)));
1802+
getMfaStatus(i.getString(Fields.IDENTITIES_MFA)));
17991803
ret.add(new RemoteIdentity(rid, det));
18001804
}
18011805
return ret;

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,30 @@ TokenType.LOGIN, id, new UserName("bar"))
126126
assertThat("incorrect token", st, is(expected));
127127
}
128128

129+
@Test
130+
public void getWithNullMfa() throws Exception {
131+
/* Tests backwards compatibility with old tokens that don't have an MFA field
132+
* in the db.
133+
*/
134+
final UUID id = UUID.randomUUID();
135+
final Instant now = Instant.now().truncatedTo(ChronoUnit.MILLIS); // mongo truncates
136+
final StoredToken ht = StoredToken.getBuilder(
137+
TokenType.LOGIN, id, new UserName("bar"))
138+
.withLifeTime(now, now.plusSeconds(10)).build();
139+
storage.storeToken(ht, "nJKFR6Xc4vzCeI3jT+FjlC9k5Q/qVw0zd0gi1erL8ew=");
140+
141+
// Remove the MFA field to simulate old database records
142+
db.getCollection("tokens").updateOne(new Document("id", id.toString()),
143+
new Document("$unset", new Document("mfa", "")));
144+
145+
final StoredToken expected = StoredToken.getBuilder(
146+
TokenType.LOGIN, id, new UserName("bar"))
147+
.withLifeTime(now, now.plusSeconds(10)).build();
148+
149+
final StoredToken st = storage.getToken(new IncomingToken("sometoken").getHashedToken());
150+
assertThat("incorrect token", st, is(expected));
151+
}
152+
129153
@Test
130154
public void storeTokenFailNull() throws Exception {
131155
final StoredToken st = StoredToken.getBuilder(

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import java.util.Optional;
1414
import java.util.UUID;
1515

16+
import org.bson.Document;
1617
import org.junit.Test;
1718

1819
import com.google.common.collect.ImmutableMap;
@@ -653,4 +654,32 @@ public void getUserByRemoteIdWithMfa() throws Exception {
653654
assertThat("incorrect is disabled", u.isDisabled(), is(false));
654655
assertThat("incorrect is local", u.isLocal(), is(false));
655656
}
657+
658+
@Test
659+
public void getUserWithNullMfaIdentity() throws Exception {
660+
/* Tests backwards compatibility with old identities that don't have an MFA field
661+
* in the db.
662+
*/
663+
storage.createUser(NewUser.getBuilder(
664+
new UserName("olduser"), UID, new DisplayName("Old User"), NOW, REMOTE_MFA_NULL)
665+
.withEmailAddress(new EmailAddress("[email protected]"))
666+
.build());
667+
668+
// Remove the MFA field from the identity to simulate old database records
669+
db.getCollection("users").updateOne(
670+
new Document("user", "olduser"),
671+
new Document("$unset", new Document("identities.0.mfa", "")));
672+
673+
final AuthUser u = storage.getUser(REMOTE_MFA_NULL).get();
674+
675+
// Should retrieve successfully with MFA defaulting to UNKNOWN
676+
final RemoteIdentity expectedIdentity = new RemoteIdentity(
677+
new RemoteIdentityID("orcid", "0000-0001-1234-0000"),
678+
new RemoteIdentityDetails("orciduser3", "ORCID User 3", "[email protected]", MfaStatus.UNKNOWN));
679+
680+
assertThat("incorrect identities", u.getIdentities(), is(set(expectedIdentity)));
681+
assertThat("incorrect username", u.getUserName(), is(new UserName("olduser")));
682+
assertThat("incorrect display name", u.getDisplayName(), is(new DisplayName("Old User")));
683+
assertThat("incorrect email", u.getEmail(), is(new EmailAddress("[email protected]")));
684+
}
656685
}

0 commit comments

Comments
 (0)