Skip to content
Open
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
e78616e
Add openid role and mfa parsing to Orcid provider; support MFA status…
dauglyon Jul 15, 2025
984fdb2
move MFA status to token endpoint; remove mfaStatus endpoint; clean u…
dauglyon Jul 15, 2025
e23c5dd
fix build errors
dauglyon Jul 15, 2025
5255abf
fix test failures from MFA implementation
dauglyon Jul 16, 2025
99f0573
reorder expected html for test
dauglyon Jul 16, 2025
748c8a2
Improve test coverage; improve error handling
dauglyon Jul 16, 2025
589d52a
fix exception errors
dauglyon Jul 16, 2025
099ca8d
Update ORCID provider tests to use MockServer
dauglyon Jul 16, 2025
4cbf2ec
remove duplicate tests
dauglyon Jul 16, 2025
ef094fd
fix TokenEndpointTest.getToken
dauglyon Jul 17, 2025
ac606a5
fix TokenEndpointTest.getToken
dauglyon Jul 17, 2025
9dfc0a7
Store MFA authentication status directly on tokens
dauglyon Jul 17, 2025
53ed176
update tests
dauglyon Jul 17, 2025
ae74a21
fix tests
dauglyon Jul 17, 2025
935f193
slightly better documentation
dauglyon Jul 17, 2025
7bbf759
update token field name
dauglyon Jul 17, 2025
3d9e93e
consolidate test logic
dauglyon Jul 17, 2025
8d85ad9
Change mfa value to enum, from boolean
dauglyon Jul 18, 2025
2db7814
rename all uses MfaAuthenticated -> mfa
dauglyon Jul 18, 2025
328704c
fix tests
dauglyon Jul 18, 2025
90ce1af
rename all uses MfaAuthenticated -> mfa
dauglyon Jul 18, 2025
58a2aec
rename jwt field for clarity
dauglyon Jul 18, 2025
c766704
fix tests, equalities
dauglyon Jul 18, 2025
37d2ef6
throw errors for jwt parsing issues
dauglyon Jul 18, 2025
62d6066
add check for null/missing AMR claim, fix tests
dauglyon Jul 18, 2025
f45fb6e
Fix hashCode implementations
dauglyon Aug 12, 2025
bd080e6
Fix hascode expectations
dauglyon Aug 12, 2025
ec619c6
Fix test ordering expectation for identity list
dauglyon Aug 12, 2025
d43d01e
include both scopes, minor refactor
dauglyon Aug 28, 2025
6768eaf
Fix null fullname handling in ORCID OAuth flow
dauglyon Sep 8, 2025
bba441b
Fix null MFA field handling for backward compatibility
dauglyon Sep 8, 2025
c46b2da
Update ORCID provider tests for OpenID Connect requirements
dauglyon Sep 8, 2025
697b8c9
Add MFA status to all token responses
dauglyon Sep 9, 2025
164e3cb
Replace hashcode assertions with equaility assertions, backed by equa…
dauglyon Sep 22, 2025
a4a0262
Improve ORCID JWT error handling and test robustness
dauglyon Sep 22, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 20 additions & 8 deletions src/main/java/us/kbase/auth2/lib/Authentication.java
Original file line number Diff line number Diff line change
Expand Up @@ -745,13 +745,19 @@ public void forceResetAllPasswords(final IncomingToken token)

private NewToken login(final UserName userName, final TokenCreationContext tokenCtx)
throws AuthStorageException {
return login(userName, tokenCtx, false);
}

private NewToken login(final UserName userName, final TokenCreationContext tokenCtx,
final Boolean mfaAuthenticated) throws AuthStorageException {
final NewToken nt = new NewToken(StoredToken.getBuilder(
TokenType.LOGIN, randGen.randomUUID(), userName)
.withLifeTime(clock.instant(),
cfg.getAppConfig().getTokenLifetimeMS(TokenLifetimeType.LOGIN))
.withContext(tokenCtx)
.build(),
randGen.getToken());
TokenType.LOGIN, randGen.randomUUID(), userName)
.withLifeTime(clock.instant(),
cfg.getAppConfig().getTokenLifetimeMS(TokenLifetimeType.LOGIN))
.withContext(tokenCtx)
.withMfaAuthenticated(mfaAuthenticated)
.build(),
randGen.getToken());
storage.storeToken(nt.getStoredToken(), nt.getTokenHash());
setLastLogin(userName);
logInfo("Logged in user {} with token {}",
Expand Down Expand Up @@ -905,7 +911,9 @@ public NewToken createToken(
final NewToken nt = new NewToken(StoredToken.getBuilder(tokenType, id, au.getUserName())
.withLifeTime(clock.instant(), life)
.withContext(tokenCtx)
.withTokenName(tokenName).build(),
.withTokenName(tokenName)
.withMfaAuthenticated(null) // Agent/Dev/Serv tokens don't have MFA status
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the distinction between null and false for MFA?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, yes, it helps us know how to instruct a user to enable MFA (false, they need to enable it with their provider; null, the used login provider is not supported)

We could represent this as three string instead, or an enum perhaps. But there are indeed three distinct states.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I would go with an Enum in that case, easier to read, no nulls.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new Enum is all caps, whereas all the other enums that are exposed are CamelCase (I think, been a while since I touched this codebase):


Can we make the new enum the same?

.build(),
randGen.getToken());
storage.storeToken(nt.getStoredToken(), nt.getTokenHash());
logInfo("User {} created {} token {}", au.getUserName().getName(), tokenType, id);
Expand Down Expand Up @@ -2043,6 +2051,7 @@ public NewToken testModeCreateToken(
final NewToken nt = new NewToken(StoredToken.getBuilder(tokenType, id, userName)
.withLifeTime(clock.instant(), TEST_MODE_DATA_LIFETIME_MS)
.withNullableTokenName(tokenName)
.withMfaAuthenticated(null) // Test mode tokens don't have MFA status
.build(),
randGen.getToken());
storage.testModeStoreToken(nt.getStoredToken(), nt.getTokenHash());
Expand Down Expand Up @@ -2339,7 +2348,9 @@ public NewToken login(
linked, u.get().getUserName().getName());
}
}
return login(u.get().getUserName(), tokenCtx);
final Boolean mfaStatus = ri.get().getDetails() != null ?
ri.get().getDetails().getMfaAuthenticated() : null;
return login(u.get().getUserName(), tokenCtx, mfaStatus);
}

private Optional<RemoteIdentity> getIdentity(
Expand Down Expand Up @@ -3142,6 +3153,7 @@ public long getSuggestedTokenCacheTime() throws AuthStorageException {
return cfg.getAppConfig().getTokenLifetimeMS(TokenLifetimeType.EXT_CACHE);
}


/** Get the external configuration without providing any credentials.
Comment on lines 3152 to 3155
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}
/** Get the external configuration without providing any credentials.
}
/** Get the external configuration without providing any credentials.

*
* This method should not be exposed in a public API.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ public class RemoteIdentityDetails {
private final String username;
private final String fullname;
private final String email;
private final Boolean mfaAuthenticated;

/** Create a new set of details.
* @param username the user name of the identity.
Expand All @@ -20,6 +21,22 @@ public RemoteIdentityDetails(
final String username,
final String fullname,
final String email) {
this(username, fullname, email, null);
}

/** Create a new set of details.
* @param username the user name of the identity.
* @param fullname the full name of the identity. Null is acceptable.
* @param email the email address of the identity. Null is acceptable.
* @param mfaAuthenticated whether the user authenticated using multi-factor authentication.
* True if MFA was used, false if password only, null if MFA status is unknown or not
* supported by the provider.
*/
public RemoteIdentityDetails(
final String username,
final String fullname,
final String email,
final Boolean mfaAuthenticated) {
super();
if (username == null || username.trim().isEmpty()) {
throw new IllegalArgumentException(
Expand All @@ -36,6 +53,7 @@ public RemoteIdentityDetails(
} else {
this.email = email.trim();
}
this.mfaAuthenticated = mfaAuthenticated;
}

/** Get the user name for the identity.
Expand All @@ -57,13 +75,22 @@ public String getFullname() {
public String getEmail() {
return email;
}

/** Get whether the user authenticated using multi-factor authentication.
* @return true if the user authenticated with MFA, false if not, null if MFA status
* is unknown or not supported by the provider.
*/
public Boolean getMfaAuthenticated() {
return mfaAuthenticated;
}

@Override
public int hashCode() {
final int prime = 31;
int result = 1;
result = prime * result + ((email == null) ? 0 : email.hashCode());
result = prime * result + ((fullname == null) ? 0 : fullname.hashCode());
result = prime * result + ((mfaAuthenticated == null) ? 0 : mfaAuthenticated.hashCode());
result = prime * result + ((username == null) ? 0 : username.hashCode());
return result;
}
Expand Down Expand Up @@ -94,6 +121,13 @@ public boolean equals(Object obj) {
} else if (!fullname.equals(other.fullname)) {
return false;
}
if (mfaAuthenticated == null) {
if (other.mfaAuthenticated != null) {
return false;
}
} else if (!mfaAuthenticated.equals(other.mfaAuthenticated)) {
return false;
}
if (username == null) {
if (other.username != null) {
return false;
Expand All @@ -113,6 +147,8 @@ public String toString() {
builder.append(fullname);
builder.append(", email=");
builder.append(email);
builder.append(", mfaAuthenticated=");
builder.append(mfaAuthenticated);
builder.append("]");
return builder.toString();
}
Expand Down
4 changes: 4 additions & 0 deletions src/main/java/us/kbase/auth2/lib/storage/mongo/Fields.java
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ public class Fields {
public static final String IDENTITIES_NAME = "fullname";
/** The email address of the identity. */
public static final String IDENTITIES_EMAIL = "email";
/** Whether the identity was authenticated with multi-factor authentication. */
public static final String IDENTITIES_MFA = "mfa";

/* **************
* token fields
Expand Down Expand Up @@ -135,6 +137,8 @@ public class Fields {
public static final String TOKEN_CUSTOM_KEY = "k";
/** A value for a custom context key / value pair. */
public static final String TOKEN_CUSTOM_VALUE = "v";
/** Whether the token was created with multi-factor authentication. */
public static final String TOKEN_MFA_AUTHENTICATED = "mfaauth";

/* ************************
* temporary session data fields
Expand Down
13 changes: 9 additions & 4 deletions src/main/java/us/kbase/auth2/lib/storage/mongo/MongoStorage.java
Original file line number Diff line number Diff line change
Expand Up @@ -871,7 +871,8 @@ private void storeToken(final String collection, final StoredToken token, final
.append(Fields.TOKEN_DEVICE, ctx.getDevice().orElse(null))
.append(Fields.TOKEN_IP, ctx.getIpAddress().isPresent() ?
ctx.getIpAddress().get().getHostAddress() : null)
.append(Fields.TOKEN_CUSTOM_CONTEXT, toCustomContextList(ctx.getCustomContext()));
.append(Fields.TOKEN_CUSTOM_CONTEXT, toCustomContextList(ctx.getCustomContext()))
.append(Fields.TOKEN_MFA_AUTHENTICATED, token.getMfaAuthenticated());
try {
db.getCollection(collection).insertOne(td);
} catch (MongoWriteException mwe) {
Expand Down Expand Up @@ -969,6 +970,7 @@ private StoredToken getToken(final Document t) throws AuthStorageException {
t.getDate(Fields.TOKEN_EXPIRY).toInstant())
.withNullableTokenName(getTokenName(t.getString(Fields.TOKEN_NAME)))
.withContext(toTokenCreationContext(t))
.withMfaAuthenticated(t.getBoolean(Fields.TOKEN_MFA_AUTHENTICATED))
.build();
}

Expand Down Expand Up @@ -1596,7 +1598,8 @@ private void updateIdentity(final RemoteIdentity remoteID)
final Document update = new Document("$set",
new Document(pre + Fields.IDENTITIES_USER, rid.getUsername())
.append(pre + Fields.IDENTITIES_EMAIL, rid.getEmail())
.append(pre + Fields.IDENTITIES_NAME, rid.getFullname()));
.append(pre + Fields.IDENTITIES_NAME, rid.getFullname())
.append(pre + Fields.IDENTITIES_MFA, rid.getMfaAuthenticated()));
try {
// id might have been unlinked, so we just assume
// the update worked. If it was just unlinked we don't care.
Expand Down Expand Up @@ -1729,7 +1732,8 @@ private Document toDocument(final RemoteIdentity id) {
.append(Fields.IDENTITIES_PROV_ID, id.getRemoteID().getProviderIdentityId())
.append(Fields.IDENTITIES_USER, rid.getUsername())
.append(Fields.IDENTITIES_NAME, rid.getFullname())
.append(Fields.IDENTITIES_EMAIL, rid.getEmail());
.append(Fields.IDENTITIES_EMAIL, rid.getEmail())
.append(Fields.IDENTITIES_MFA, rid.getMfaAuthenticated());
}

@Override
Expand Down Expand Up @@ -1789,7 +1793,8 @@ private Set<RemoteIdentity> toIdentities(final List<Document> ids) {
final RemoteIdentityDetails det = new RemoteIdentityDetails(
i.getString(Fields.IDENTITIES_USER),
i.getString(Fields.IDENTITIES_NAME),
i.getString(Fields.IDENTITIES_EMAIL));
i.getString(Fields.IDENTITIES_EMAIL),
i.getBoolean(Fields.IDENTITIES_MFA));
ret.add(new RemoteIdentity(rid, det));
}
return ret;
Expand Down
35 changes: 33 additions & 2 deletions src/main/java/us/kbase/auth2/lib/token/StoredToken.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public class StoredToken {
private final UserName userName;
private final Instant creationDate;
private final Instant expirationDate;
private final Boolean mfaAuthenticated;

private StoredToken(
final UUID id,
Expand All @@ -31,7 +32,8 @@ private StoredToken(
final UserName userName,
final TokenCreationContext context,
final Instant creationDate,
final Instant expirationDate) {
final Instant expirationDate,
final Boolean mfaAuthenticated) {
// this stuff is here just in case naughty users use casting to skip a builder step
requireNonNull(creationDate, "created");
// no way to test this one
Expand All @@ -43,6 +45,7 @@ private StoredToken(
this.expirationDate = expirationDate;
this.creationDate = creationDate;
this.id = id;
this.mfaAuthenticated = mfaAuthenticated;
}

/** Get the type of the token.
Expand Down Expand Up @@ -93,6 +96,13 @@ public Instant getCreationDate() {
public Instant getExpirationDate() {
return expirationDate;
}

/** Get whether the token was created with multi-factor authentication.
* @return true if the token was created with MFA, false if not, null if unknown.
*/
public Boolean getMfaAuthenticated() {
return mfaAuthenticated;
}

@Override
public int hashCode() {
Expand All @@ -105,6 +115,7 @@ public int hashCode() {
result = prime * result + ((tokenName == null) ? 0 : tokenName.hashCode());
result = prime * result + ((type == null) ? 0 : type.hashCode());
result = prime * result + ((userName == null) ? 0 : userName.hashCode());
result = prime * result + ((mfaAuthenticated == null) ? 0 : mfaAuthenticated.hashCode());
return result;
}

Expand Down Expand Up @@ -165,6 +176,13 @@ public boolean equals(Object obj) {
} else if (!userName.equals(other.userName)) {
return false;
}
if (mfaAuthenticated == null) {
if (other.mfaAuthenticated != null) {
return false;
}
} else if (!mfaAuthenticated.equals(other.mfaAuthenticated)) {
return false;
}
return true;
}

Expand Down Expand Up @@ -224,6 +242,12 @@ public interface OptionalsStep {
*/
OptionalsStep withContext(TokenCreationContext context);

/** Specify whether the token was created with multi-factor authentication.
* @param mfaAuthenticated true if MFA was used, false if not, null if unknown.
* @return this builder.
*/
OptionalsStep withMfaAuthenticated(Boolean mfaAuthenticated);

/** Build the token.
* @return a new StoredToken.
*/
Expand All @@ -239,6 +263,7 @@ private static class Builder implements LifeStep, OptionalsStep {
private final UserName userName;
private Instant creationDate;
private Instant expirationDate;
private Boolean mfaAuthenticated;

private Builder(final TokenType type, final UUID id, final UserName userName) {
requireNonNull(type, "type");
Expand Down Expand Up @@ -269,10 +294,16 @@ public OptionalsStep withContext(final TokenCreationContext context) {
return this;
}

@Override
public OptionalsStep withMfaAuthenticated(final Boolean mfaAuthenticated) {
this.mfaAuthenticated = mfaAuthenticated;
return this;
}

@Override
public StoredToken build() {
return new StoredToken(id, type, tokenName, userName, context,
creationDate, expirationDate);
creationDate, expirationDate, mfaAuthenticated);
}

@Override
Expand Down
Loading
Loading