From e78616e521cd31c7de3007ed5e7278d8ae1877d0 Mon Sep 17 00:00:00 2001 From: David Lyon <5115845+dauglyon@users.noreply.github.com> Date: Tue, 15 Jul 2025 10:55:55 -0700 Subject: [PATCH 01/25] Add openid role and mfa parsing to Orcid provider; support MFA status; add MFA tests --- .../lib/identity/RemoteIdentityDetails.java | 36 ++ .../kbase/auth2/lib/storage/mongo/Fields.java | 2 + .../auth2/lib/storage/mongo/MongoStorage.java | 9 +- .../OrcIDIdentityProviderFactory.java | 69 +++- .../us/kbase/auth2/service/api/APIPaths.java | 3 + .../us/kbase/auth2/service/api/MfaStatus.java | 89 +++++ .../lib/identity/RemoteIdentityTest.java | 55 ++- .../mongo/MongoStorageUserCreateGetTest.java | 74 ++++ .../providers/OrcIDIdentityProviderTest.java | 284 +++++++++++++++ .../test/auth2/service/api/MfaStatusTest.java | 322 ++++++++++++++++++ 10 files changed, 932 insertions(+), 11 deletions(-) create mode 100644 src/main/java/us/kbase/auth2/service/api/MfaStatus.java create mode 100644 src/test/java/us/kbase/test/auth2/service/api/MfaStatusTest.java diff --git a/src/main/java/us/kbase/auth2/lib/identity/RemoteIdentityDetails.java b/src/main/java/us/kbase/auth2/lib/identity/RemoteIdentityDetails.java index dcbf6e01..dbfd637a 100644 --- a/src/main/java/us/kbase/auth2/lib/identity/RemoteIdentityDetails.java +++ b/src/main/java/us/kbase/auth2/lib/identity/RemoteIdentityDetails.java @@ -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. @@ -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( @@ -36,6 +53,7 @@ public RemoteIdentityDetails( } else { this.email = email.trim(); } + this.mfaAuthenticated = mfaAuthenticated; } /** Get the user name for the identity. @@ -57,6 +75,14 @@ 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 isMfaAuthenticated() { + return mfaAuthenticated; + } @Override public int hashCode() { @@ -64,6 +90,7 @@ public int hashCode() { 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; } @@ -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; @@ -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(); } diff --git a/src/main/java/us/kbase/auth2/lib/storage/mongo/Fields.java b/src/main/java/us/kbase/auth2/lib/storage/mongo/Fields.java index b05aba6d..48fcc8ce 100644 --- a/src/main/java/us/kbase/auth2/lib/storage/mongo/Fields.java +++ b/src/main/java/us/kbase/auth2/lib/storage/mongo/Fields.java @@ -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 diff --git a/src/main/java/us/kbase/auth2/lib/storage/mongo/MongoStorage.java b/src/main/java/us/kbase/auth2/lib/storage/mongo/MongoStorage.java index f2e520f6..ee730530 100644 --- a/src/main/java/us/kbase/auth2/lib/storage/mongo/MongoStorage.java +++ b/src/main/java/us/kbase/auth2/lib/storage/mongo/MongoStorage.java @@ -1596,7 +1596,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.isMfaAuthenticated())); try { // id might have been unlinked, so we just assume // the update worked. If it was just unlinked we don't care. @@ -1729,7 +1730,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.isMfaAuthenticated()); } @Override @@ -1789,7 +1791,8 @@ private Set toIdentities(final List 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; diff --git a/src/main/java/us/kbase/auth2/providers/OrcIDIdentityProviderFactory.java b/src/main/java/us/kbase/auth2/providers/OrcIDIdentityProviderFactory.java index e93a357b..ddc1808e 100644 --- a/src/main/java/us/kbase/auth2/providers/OrcIDIdentityProviderFactory.java +++ b/src/main/java/us/kbase/auth2/providers/OrcIDIdentityProviderFactory.java @@ -7,6 +7,7 @@ import java.net.URISyntaxException; import java.net.URL; import java.util.Arrays; +import java.util.Base64; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -59,7 +60,7 @@ public static class OrcIDIdentityProvider implements IdentityProvider { /* Get creds: https://sandbox.orcid.org/developer-tools */ private static final String NAME = "OrcID"; - private static final String SCOPE = "/authenticate"; + private static final String SCOPE = "openid"; private static final String LOGIN_PATH = "/oauth/authorize"; private static final String TOKEN_PATH = "/oauth/token"; private static final String RECORD_PATH = "/v2.1"; @@ -174,7 +175,8 @@ private RemoteIdentity getIdentity(final OrcIDAccessTokenResponse accessToken) new RemoteIdentityDetails( accessToken.orcID, accessToken.fullName, - email)); + email, + accessToken.mfaAuthenticated)); } private Map orcIDGetRequest( @@ -211,11 +213,14 @@ private static class OrcIDAccessTokenResponse { private final String accessToken; private final String fullName; private final String orcID; + private final String idToken; + private final Boolean mfaAuthenticated; private OrcIDAccessTokenResponse( final String accessToken, final String fullName, - final String orcID) + final String orcID, + final String idToken) throws IdentityRetrievalException { if (accessToken == null || accessToken.trim().isEmpty()) { throw new IdentityRetrievalException( @@ -228,6 +233,61 @@ private OrcIDAccessTokenResponse( this.accessToken = accessToken.trim(); this.fullName = fullName == null ? null : fullName.trim(); this.orcID = orcID.trim(); + this.idToken = idToken == null ? null : idToken.trim(); + this.mfaAuthenticated = parseAmrClaim(this.idToken); + } + + /** + * Parses the Authentication Method Reference (AMR) claim from an OpenID Connect ID token + * to determine if multi-factor authentication was used. + * + * @param idToken the JWT ID token from ORCID + * @return true if MFA was used, false if password only, null if unknown or parsing failed + */ + private Boolean parseAmrClaim(final String idToken) { + if (idToken == null || idToken.trim().isEmpty()) { + return null; + } + + try { + // JWT format: header.payload.signature + final String[] parts = idToken.split("\\."); + if (parts.length != 3) { + // Invalid JWT format + return null; + } + + // Decode the payload (second part) - URL-safe base64 + final String payload = new String(Base64.getUrlDecoder().decode(parts[1])); + + // Parse JSON payload to extract claims + @SuppressWarnings("unchecked") + final Map claims = MAPPER.readValue(payload, Map.class); + + final Object amrClaim = claims.get("amr"); + if (amrClaim instanceof List) { + // OpenID Connect spec: AMR should be an array of strings + @SuppressWarnings("unchecked") + final List amrList = (List) amrClaim; + return amrList.contains("mfa"); + } else if (amrClaim instanceof String) { + // ORCID may return single string - handle as fallback + return "mfa".equals(amrClaim); + } + + // AMR claim present but in unexpected format + return null; + + } catch (IllegalArgumentException e) { + // Base64 decoding failed - invalid JWT + return null; + } catch (IOException e) { + // JSON parsing failed - malformed payload + return null; + } catch (Exception e) { + // Other unexpected errors - don't fail authentication + return null; + } } } @@ -259,7 +319,8 @@ private OrcIDAccessTokenResponse getAccessToken( return new OrcIDAccessTokenResponse( (String) m.get("access_token"), (String) m.get("name"), - (String) m.get("orcid")); + (String) m.get("orcid"), + (String) m.get("id_token")); } private Map orcIDPostRequest( diff --git a/src/main/java/us/kbase/auth2/service/api/APIPaths.java b/src/main/java/us/kbase/auth2/service/api/APIPaths.java index acfeb1d5..5367abe7 100644 --- a/src/main/java/us/kbase/auth2/service/api/APIPaths.java +++ b/src/main/java/us/kbase/auth2/service/api/APIPaths.java @@ -54,6 +54,9 @@ public class APIPaths { /** The me endpoint location. */ public static final String API_V2_ME = API_V2 + SEP + ME; + /** The MFA status endpoint location. */ + public static final String API_V2_MFA_STATUS = API_V2 + SEP + "mfastatus"; + /* test mode endpoints. */ /** The testmode root endpoint. */ diff --git a/src/main/java/us/kbase/auth2/service/api/MfaStatus.java b/src/main/java/us/kbase/auth2/service/api/MfaStatus.java new file mode 100644 index 00000000..ae4c42c7 --- /dev/null +++ b/src/main/java/us/kbase/auth2/service/api/MfaStatus.java @@ -0,0 +1,89 @@ +package us.kbase.auth2.service.api; + +import static us.kbase.auth2.service.common.ServiceCommon.getToken; + +import java.util.Map; +import java.util.Set; + +import javax.inject.Inject; +import javax.ws.rs.GET; +import javax.ws.rs.HeaderParam; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.core.MediaType; + +import com.google.common.collect.ImmutableMap; + +import us.kbase.auth2.lib.Authentication; +import us.kbase.auth2.lib.exceptions.DisabledUserException; +import us.kbase.auth2.lib.exceptions.InvalidTokenException; +import us.kbase.auth2.lib.exceptions.NoTokenProvidedException; +import us.kbase.auth2.lib.identity.RemoteIdentity; +import us.kbase.auth2.lib.storage.exceptions.AuthStorageException; +import us.kbase.auth2.lib.user.AuthUser; + +/** + * API endpoint for checking multi-factor authentication status of user tokens. + * + * This endpoint allows clients to determine if a user's current session was + * authenticated using multi-factor authentication via supported identity providers. + */ +@Path(APIPaths.API_V2_MFA_STATUS) +public class MfaStatus { + + @Inject + private Authentication auth; + + /** + * Returns the MFA authentication status for the current user token. + * + * Checks if the user authenticated using multi-factor authentication through + * supported identity providers (currently ORCID with OpenID Connect). + * + * @param token the user's authentication token in the Authorization header + * @return JSON object containing MFA status information + * @throws InvalidTokenException if the token is invalid or expired + * @throws AuthStorageException if there's a database access error + * @throws NoTokenProvidedException if no token is provided + * @throws DisabledUserException if the user account is disabled + */ + @GET + @Produces(MediaType.APPLICATION_JSON) + public Map getMfaStatus( + @HeaderParam("Authorization") final String token) + throws InvalidTokenException, AuthStorageException, NoTokenProvidedException, + DisabledUserException { + + final AuthUser user = auth.getUser(getToken(token)); + + // Check for ORCID identities with MFA information + Boolean orcidMfaUsed = null; + String providerName = null; + + final Set identities = user.getIdentities(); + for (final RemoteIdentity identity : identities) { + if ("OrcID".equals(identity.getRemoteID().getProviderName())) { + final Boolean mfaStatus = identity.getDetails().isMfaAuthenticated(); + if (mfaStatus != null) { + orcidMfaUsed = mfaStatus; + providerName = identity.getRemoteID().getProviderName(); + break; // Use the first ORCID identity with MFA information + } + } + } + + if (orcidMfaUsed != null) { + return ImmutableMap.of( + "provider", providerName, + "mfa_used", orcidMfaUsed, + "status", orcidMfaUsed ? "mfa_authenticated" : "password_only" + ); + } else { + return ImmutableMap.of( + "provider", "none", + "mfa_used", (Object) null, + "status", "no_mfa_info_available" + ); + } + } +} \ No newline at end of file diff --git a/src/test/java/us/kbase/test/auth2/lib/identity/RemoteIdentityTest.java b/src/test/java/us/kbase/test/auth2/lib/identity/RemoteIdentityTest.java index a8b48f3d..7b8c5332 100644 --- a/src/test/java/us/kbase/test/auth2/lib/identity/RemoteIdentityTest.java +++ b/src/test/java/us/kbase/test/auth2/lib/identity/RemoteIdentityTest.java @@ -20,9 +20,10 @@ public void remoteDetailsWithAllFields() throws Exception { assertThat("incorrect username", dets.getUsername(), is("user")); assertThat("incorrect fullname", dets.getFullname(), is("full")); assertThat("incorrect email", dets.getEmail(), is("email")); + assertThat("incorrect mfa authenticated", dets.getMfaAuthenticated(), is((Boolean) null)); assertThat("incorrect hashcode", dets.hashCode(), is(-1536596969)); assertThat("incorrect toString()", dets.toString(), - is("RemoteIdentityDetails [username=user, fullname=full, email=email]")); + is("RemoteIdentityDetails [username=user, fullname=full, email=email, mfaAuthenticated=null]")); } @Test @@ -31,17 +32,19 @@ public void remoteDetailsWithEmptyFields() throws Exception { assertThat("incorrect username", dets.getUsername(), is("user")); assertThat("incorrect fullname", dets.getFullname(), is((String) null)); assertThat("incorrect email", dets.getEmail(), is((String) null)); + assertThat("incorrect mfa authenticated", dets.getMfaAuthenticated(), is((Boolean) null)); assertThat("incorrect hashcode", dets.hashCode(), is(3629098)); assertThat("incorrect toString()", dets.toString(), - is("RemoteIdentityDetails [username=user, fullname=null, email=null]")); + is("RemoteIdentityDetails [username=user, fullname=null, email=null, mfaAuthenticated=null]")); final RemoteIdentityDetails dets2 = new RemoteIdentityDetails("user", null, null); assertThat("incorrect username", dets2.getUsername(), is("user")); assertThat("incorrect fullname", dets2.getFullname(), is((String) null)); assertThat("incorrect email", dets2.getEmail(), is((String) null)); + assertThat("incorrect mfa authenticated", dets2.getMfaAuthenticated(), is((Boolean) null)); assertThat("incorrect hashcode", dets2.hashCode(), is(3629098)); assertThat("incorrect toString()", dets2.toString(), - is("RemoteIdentityDetails [username=user, fullname=null, email=null]")); + is("RemoteIdentityDetails [username=user, fullname=null, email=null, mfaAuthenticated=null]")); } @Test @@ -115,7 +118,7 @@ public void identity() throws Exception { assertThat("incorrect hashcode", ri.hashCode(), is(4039350)); assertThat("incorrect toString()", ri.toString(), is("RemoteIdentity [remoteID=RemoteIdentityID [provider=p, id=i], " + - "details=RemoteIdentityDetails [username=u, fullname=f, email=e]]")); + "details=RemoteIdentityDetails [username=u, fullname=f, email=e, mfaAuthenticated=null]]")); } @Test @@ -140,4 +143,48 @@ private void failCreateIdentity( assertThat("incorrect exception message", e.getMessage(), is(exception)); } } + + @Test + public void remoteDetailsWithMfaTrue() throws Exception { + final RemoteIdentityDetails dets = new RemoteIdentityDetails("user", "full", "email", true); + assertThat("incorrect username", dets.getUsername(), is("user")); + assertThat("incorrect fullname", dets.getFullname(), is("full")); + assertThat("incorrect email", dets.getEmail(), is("email")); + assertThat("incorrect mfa authenticated", dets.getMfaAuthenticated(), is(true)); + assertThat("incorrect toString()", dets.toString(), + is("RemoteIdentityDetails [username=user, fullname=full, email=email, mfaAuthenticated=true]")); + } + + @Test + public void remoteDetailsWithMfaFalse() throws Exception { + final RemoteIdentityDetails dets = new RemoteIdentityDetails("user", "full", "email", false); + assertThat("incorrect username", dets.getUsername(), is("user")); + assertThat("incorrect fullname", dets.getFullname(), is("full")); + assertThat("incorrect email", dets.getEmail(), is("email")); + assertThat("incorrect mfa authenticated", dets.getMfaAuthenticated(), is(false)); + assertThat("incorrect toString()", dets.toString(), + is("RemoteIdentityDetails [username=user, fullname=full, email=email, mfaAuthenticated=false]")); + } + + @Test + public void remoteDetailsWithMfaNull() throws Exception { + final RemoteIdentityDetails dets = new RemoteIdentityDetails("user", "full", "email", null); + assertThat("incorrect username", dets.getUsername(), is("user")); + assertThat("incorrect fullname", dets.getFullname(), is("full")); + assertThat("incorrect email", dets.getEmail(), is("email")); + assertThat("incorrect mfa authenticated", dets.getMfaAuthenticated(), is((Boolean) null)); + assertThat("incorrect toString()", dets.toString(), + is("RemoteIdentityDetails [username=user, fullname=full, email=email, mfaAuthenticated=null]")); + } + + @Test + public void remoteDetailsMfaFailWithNullUser() throws Exception { + try { + new RemoteIdentityDetails(null, "full", "email", true); + fail("created bad details with mfa"); + } catch (IllegalArgumentException e) { + assertThat("incorrect exception msg", e.getMessage(), + is("username cannot be null or empty")); + } + } } diff --git a/src/test/java/us/kbase/test/auth2/lib/storage/mongo/MongoStorageUserCreateGetTest.java b/src/test/java/us/kbase/test/auth2/lib/storage/mongo/MongoStorageUserCreateGetTest.java index 5abe138c..d10bfcad 100644 --- a/src/test/java/us/kbase/test/auth2/lib/storage/mongo/MongoStorageUserCreateGetTest.java +++ b/src/test/java/us/kbase/test/auth2/lib/storage/mongo/MongoStorageUserCreateGetTest.java @@ -56,6 +56,18 @@ public class MongoStorageUserCreateGetTest extends MongoStorageTester { private static final RemoteIdentity REMOTE2 = new RemoteIdentity( new RemoteIdentityID("prov", "bar2"), new RemoteIdentityDetails("user2", "full2", "email2")); + + private static final RemoteIdentity REMOTE_MFA_TRUE = new RemoteIdentity( + new RemoteIdentityID("orcid", "0000-0001-1234-5678"), + new RemoteIdentityDetails("orciduser", "ORCID User", "orcid@example.com", true)); + + private static final RemoteIdentity REMOTE_MFA_FALSE = new RemoteIdentity( + new RemoteIdentityID("orcid", "0000-0001-1234-9999"), + new RemoteIdentityDetails("orciduser2", "ORCID User 2", "orcid2@example.com", false)); + + private static final RemoteIdentity REMOTE_MFA_NULL = new RemoteIdentity( + new RemoteIdentityID("orcid", "0000-0001-1234-0000"), + new RemoteIdentityDetails("orciduser3", "ORCID User 3", "orcid3@example.com", null)); @Test public void createGetLocalUserMinimal() throws Exception { @@ -593,4 +605,66 @@ public void getUserAndUpdateRemoteId() throws Exception { assertThat("incorrect email", au.getEmail(), is(new EmailAddress("e@g1.com"))); // ok, thats enough } + + @Test + public void createUserWithMfaTrue() throws Exception { + storage.createUser(NewUser.getBuilder( + new UserName("mfauser"), UID, new DisplayName("MFA User"), NOW, REMOTE_MFA_TRUE) + .withEmailAddress(new EmailAddress("mfa@example.com")) + .build()); + + final AuthUser u = storage.getUser(new UserName("mfauser")); + + assertThat("incorrect identities", u.getIdentities(), is(set(REMOTE_MFA_TRUE))); + assertThat("incorrect username", u.getUserName(), is(new UserName("mfauser"))); + assertThat("incorrect display name", u.getDisplayName(), is(new DisplayName("MFA User"))); + assertThat("incorrect email", u.getEmail(), is(new EmailAddress("mfa@example.com"))); + } + + @Test + public void createUserWithMfaFalse() throws Exception { + storage.createUser(NewUser.getBuilder( + new UserName("nomfauser"), UID, new DisplayName("No MFA User"), NOW, REMOTE_MFA_FALSE) + .withEmailAddress(new EmailAddress("nomfa@example.com")) + .build()); + + final AuthUser u = storage.getUser(new UserName("nomfauser")); + + assertThat("incorrect identities", u.getIdentities(), is(set(REMOTE_MFA_FALSE))); + assertThat("incorrect username", u.getUserName(), is(new UserName("nomfauser"))); + assertThat("incorrect display name", u.getDisplayName(), is(new DisplayName("No MFA User"))); + assertThat("incorrect email", u.getEmail(), is(new EmailAddress("nomfa@example.com"))); + } + + @Test + public void createUserWithMfaNull() throws Exception { + storage.createUser(NewUser.getBuilder( + new UserName("unknownmfauser"), UID, new DisplayName("Unknown MFA User"), NOW, REMOTE_MFA_NULL) + .withEmailAddress(new EmailAddress("unknownmfa@example.com")) + .build()); + + final AuthUser u = storage.getUser(new UserName("unknownmfauser")); + + assertThat("incorrect identities", u.getIdentities(), is(set(REMOTE_MFA_NULL))); + assertThat("incorrect username", u.getUserName(), is(new UserName("unknownmfauser"))); + assertThat("incorrect display name", u.getDisplayName(), is(new DisplayName("Unknown MFA User"))); + assertThat("incorrect email", u.getEmail(), is(new EmailAddress("unknownmfa@example.com"))); + } + + @Test + public void getUserByRemoteIdWithMfa() throws Exception { + storage.createUser(NewUser.getBuilder( + new UserName("mfauser"), UID, new DisplayName("MFA User"), NOW, REMOTE_MFA_TRUE) + .withEmailAddress(new EmailAddress("mfa@example.com")) + .build()); + + final AuthUser u = storage.getUser(REMOTE_MFA_TRUE).get(); + + assertThat("incorrect identities", u.getIdentities(), is(set(REMOTE_MFA_TRUE))); + assertThat("incorrect username", u.getUserName(), is(new UserName("mfauser"))); + assertThat("incorrect display name", u.getDisplayName(), is(new DisplayName("MFA User"))); + assertThat("incorrect email", u.getEmail(), is(new EmailAddress("mfa@example.com"))); + assertThat("incorrect is disabled", u.isDisabled(), is(false)); + assertThat("incorrect is local", u.isLocal(), is(false)); + } } diff --git a/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java b/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java index f061dbda..0525869d 100644 --- a/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java +++ b/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java @@ -494,6 +494,105 @@ public void getIdentityWithLinkURLAndEnvironment() throws Exception { assertThat("incorrect ident set", rids, is(expected)); } + @Test + public void getIdentityWithMfaTrue() throws Exception { + final String authCode = "authcodeWithMfa"; + final IdentityProviderConfig idconfig = getTestIDConfig(); + final IdentityProvider idp = new OrcIDIdentityProvider(idconfig); + final String orcID = "0000-0001-1234-5678"; + final String idToken = createValidJWTWithMfa(orcID, true); + + setUpCallAuthTokenWithIdToken(authCode, "footoken3", "https://ologinredir.com", + idconfig.getClientID(), idconfig.getClientSecret(), " My name ", orcID, idToken); + setupCallID("footoken3", orcID, APP_JSON, 200, MAPPER.writeValueAsString( + map("email", Arrays.asList(map("email", "mfa@test.com"))))); + final Set rids = idp.getIdentities(authCode, "pkce", false, null); + assertThat("incorrect number of idents", rids.size(), is(1)); + final Set expected = new HashSet<>(); + expected.add(new RemoteIdentity(new RemoteIdentityID(ORCID, orcID), + new RemoteIdentityDetails(orcID, "My name", "mfa@test.com", true))); + assertThat("incorrect ident set", rids, is(expected)); + } + + @Test + public void getIdentityWithMfaFalse() throws Exception { + final String authCode = "authcodeNoMfa"; + final IdentityProviderConfig idconfig = getTestIDConfig(); + final IdentityProvider idp = new OrcIDIdentityProvider(idconfig); + final String orcID = "0000-0001-1234-5678"; + final String idToken = createValidJWTWithMfa(orcID, false); + + setUpCallAuthTokenWithIdToken(authCode, "footoken4", "https://ologinredir.com", + idconfig.getClientID(), idconfig.getClientSecret(), " My name ", orcID, idToken); + setupCallID("footoken4", orcID, APP_JSON, 200, MAPPER.writeValueAsString( + map("email", Arrays.asList(map("email", "nomfa@test.com"))))); + final Set rids = idp.getIdentities(authCode, "pkce", false, null); + assertThat("incorrect number of idents", rids.size(), is(1)); + final Set expected = new HashSet<>(); + expected.add(new RemoteIdentity(new RemoteIdentityID(ORCID, orcID), + new RemoteIdentityDetails(orcID, "My name", "nomfa@test.com", false))); + assertThat("incorrect ident set", rids, is(expected)); + } + + @Test + public void getIdentityWithNoIdToken() throws Exception { + final String authCode = "authcodeNoIdToken"; + final IdentityProviderConfig idconfig = getTestIDConfig(); + final IdentityProvider idp = new OrcIDIdentityProvider(idconfig); + final String orcID = "0000-0001-1234-5678"; + + setUpCallAuthToken(authCode, "footoken5", "https://ologinredir.com", + idconfig.getClientID(), idconfig.getClientSecret(), " My name ", orcID); + setupCallID("footoken5", orcID, APP_JSON, 200, MAPPER.writeValueAsString( + map("email", Arrays.asList(map("email", "noid@test.com"))))); + final Set rids = idp.getIdentities(authCode, "pkce", false, null); + assertThat("incorrect number of idents", rids.size(), is(1)); + final Set expected = new HashSet<>(); + expected.add(new RemoteIdentity(new RemoteIdentityID(ORCID, orcID), + new RemoteIdentityDetails(orcID, "My name", "noid@test.com", null))); + assertThat("incorrect ident set", rids, is(expected)); + } + + @Test + public void getIdentityWithInvalidJWT() throws Exception { + final String authCode = "authcodeInvalidJWT"; + final IdentityProviderConfig idconfig = getTestIDConfig(); + final IdentityProvider idp = new OrcIDIdentityProvider(idconfig); + final String orcID = "0000-0001-1234-5678"; + final String invalidIdToken = "invalid.jwt.token"; + + setUpCallAuthTokenWithIdToken(authCode, "footoken6", "https://ologinredir.com", + idconfig.getClientID(), idconfig.getClientSecret(), " My name ", orcID, invalidIdToken); + setupCallID("footoken6", orcID, APP_JSON, 200, MAPPER.writeValueAsString( + map("email", Arrays.asList(map("email", "invalid@test.com"))))); + final Set rids = idp.getIdentities(authCode, "pkce", false, null); + assertThat("incorrect number of idents", rids.size(), is(1)); + final Set expected = new HashSet<>(); + expected.add(new RemoteIdentity(new RemoteIdentityID(ORCID, orcID), + new RemoteIdentityDetails(orcID, "My name", "invalid@test.com", null))); + assertThat("incorrect ident set", rids, is(expected)); + } + + @Test + public void getIdentityWithStringAmr() throws Exception { + final String authCode = "authcodeStringAmr"; + final IdentityProviderConfig idconfig = getTestIDConfig(); + final IdentityProvider idp = new OrcIDIdentityProvider(idconfig); + final String orcID = "0000-0001-1234-5678"; + final String idToken = createJWTWithStringAmr(orcID, "mfa"); + + setUpCallAuthTokenWithIdToken(authCode, "footoken7", "https://ologinredir.com", + idconfig.getClientID(), idconfig.getClientSecret(), " My name ", orcID, idToken); + setupCallID("footoken7", orcID, APP_JSON, 200, MAPPER.writeValueAsString( + map("email", Arrays.asList(map("email", "stringmfa@test.com"))))); + final Set rids = idp.getIdentities(authCode, "pkce", false, null); + assertThat("incorrect number of idents", rids.size(), is(1)); + final Set expected = new HashSet<>(); + expected.add(new RemoteIdentity(new RemoteIdentityID(ORCID, orcID), + new RemoteIdentityDetails(orcID, "My name", "stringmfa@test.com", true))); + assertThat("incorrect ident set", rids, is(expected)); + } + private void setUpCallAuthToken( final String authCode, final String authtoken, @@ -591,4 +690,189 @@ private Map map(final Object... entries) { } return ret; } + + private void setUpCallAuthTokenWithIdToken( + final String authCode, + final String authtoken, + final String redirect, + final String clientID, + final String clientSecret, + final String name, + final String orcID, + final String idToken) + throws Exception { + mockClientAndServer.when( + new HttpRequest() + .withMethod("POST") + .withPath("/oauth/token") + .withHeader(ACCEPT, APP_JSON) + .withBody(new ParameterBody( + new Parameter("code", authCode), + new Parameter("grant_type", "authorization_code"), + new Parameter("redirect_uri", redirect), + new Parameter("client_id", clientID), + new Parameter("client_secret", clientSecret)) + ), + Times.exactly(1) + ).respond( + new HttpResponse() + .withStatusCode(200) + .withHeader(CONTENT_TYPE, APP_JSON) + .withBody(MAPPER.writeValueAsString(map( + "access_token", authtoken, + "name", name, + "orcid", orcID, + "id_token", idToken + ))) + ); + } + + private String createValidJWTWithMfa(final String orcID, final boolean usedMfa) { + final String header = "{\"alg\":\"HS256\",\"typ\":\"JWT\"}"; + final String payload = usedMfa ? + "{\"sub\":\"" + orcID + "\",\"amr\":[\"mfa\",\"pwd\"]}" : + "{\"sub\":\"" + orcID + "\",\"amr\":[\"pwd\"]}"; + final String encodedHeader = java.util.Base64.getUrlEncoder().withoutPadding() + .encodeToString(header.getBytes()); + final String encodedPayload = java.util.Base64.getUrlEncoder().withoutPadding() + .encodeToString(payload.getBytes()); + return encodedHeader + "." + encodedPayload + ".signature"; + } + + private String createJWTWithStringAmr(final String orcID, final String amrValue) { + final String header = "{\"alg\":\"HS256\",\"typ\":\"JWT\"}"; + final String payload = "{\"sub\":\"" + orcID + "\",\"amr\":\"" + amrValue + "\"}"; + final String encodedHeader = java.util.Base64.getUrlEncoder().withoutPadding() + .encodeToString(header.getBytes()); + final String encodedPayload = java.util.Base64.getUrlEncoder().withoutPadding() + .encodeToString(payload.getBytes()); + return encodedHeader + "." + encodedPayload + ".signature"; + } + + @Test + public void getIdentityWithMalformedJWT() throws Exception { + final String authCode = "authcodeMalformed"; + final IdentityProviderConfig idconfig = getTestIDConfig(); + final IdentityProvider idp = new OrcIDIdentityProvider(idconfig); + final String orcID = "0000-0001-1234-5678"; + + // Test JWT with only 2 parts + final String invalidJWT = "header.payload"; + setUpCallAuthTokenWithIdToken(authCode, "footoken8", "https://ologinredir.com", + idconfig.getClientID(), idconfig.getClientSecret(), " My name ", orcID, invalidJWT); + setupCallID("footoken8", orcID, APP_JSON, 200, MAPPER.writeValueAsString( + map("email", Arrays.asList(map("email", "malformed@test.com"))))); + final Set rids = idp.getIdentities(authCode, "pkce", false, null); + assertThat("incorrect number of idents", rids.size(), is(1)); + final Set expected = new HashSet<>(); + expected.add(new RemoteIdentity(new RemoteIdentityID(ORCID, orcID), + new RemoteIdentityDetails(orcID, "My name", "malformed@test.com", null))); + assertThat("incorrect ident set", rids, is(expected)); + } + + @Test + public void getIdentityWithInvalidBase64JWT() throws Exception { + final String authCode = "authcodeInvalidB64"; + final IdentityProviderConfig idconfig = getTestIDConfig(); + final IdentityProvider idp = new OrcIDIdentityProvider(idconfig); + final String orcID = "0000-0001-1234-5678"; + + // JWT with invalid base64 in payload + final String invalidJWT = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.invalid==base64!!.signature"; + setUpCallAuthTokenWithIdToken(authCode, "footoken9", "https://ologinredir.com", + idconfig.getClientID(), idconfig.getClientSecret(), " My name ", orcID, invalidJWT); + setupCallID("footoken9", orcID, APP_JSON, 200, MAPPER.writeValueAsString( + map("email", Arrays.asList(map("email", "invalidb64@test.com"))))); + final Set rids = idp.getIdentities(authCode, "pkce", false, null); + assertThat("incorrect number of idents", rids.size(), is(1)); + final Set expected = new HashSet<>(); + expected.add(new RemoteIdentity(new RemoteIdentityID(ORCID, orcID), + new RemoteIdentityDetails(orcID, "My name", "invalidb64@test.com", null))); + assertThat("incorrect ident set", rids, is(expected)); + } + + @Test + public void getIdentityWithMalformedJSONJWT() throws Exception { + final String authCode = "authcodeMalformedJSON"; + final IdentityProviderConfig idconfig = getTestIDConfig(); + final IdentityProvider idp = new OrcIDIdentityProvider(idconfig); + final String orcID = "0000-0001-1234-5678"; + + // JWT with invalid JSON in payload + final String invalidJSON = "{\"sub\":\"" + orcID + "\",\"amr\":}"; + final String encodedPayload = java.util.Base64.getUrlEncoder().withoutPadding() + .encodeToString(invalidJSON.getBytes()); + final String invalidJWT = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9." + encodedPayload + ".signature"; + + setUpCallAuthTokenWithIdToken(authCode, "footoken10", "https://ologinredir.com", + idconfig.getClientID(), idconfig.getClientSecret(), " My name ", orcID, invalidJWT); + setupCallID("footoken10", orcID, APP_JSON, 200, MAPPER.writeValueAsString( + map("email", Arrays.asList(map("email", "malformedjson@test.com"))))); + final Set rids = idp.getIdentities(authCode, "pkce", false, null); + assertThat("incorrect number of idents", rids.size(), is(1)); + final Set expected = new HashSet<>(); + expected.add(new RemoteIdentity(new RemoteIdentityID(ORCID, orcID), + new RemoteIdentityDetails(orcID, "My name", "malformedjson@test.com", null))); + assertThat("incorrect ident set", rids, is(expected)); + } + + @Test + public void getIdentityWithEmptyAmrJWT() throws Exception { + final String authCode = "authcodeEmptyAmr"; + final IdentityProviderConfig idconfig = getTestIDConfig(); + final IdentityProvider idp = new OrcIDIdentityProvider(idconfig); + final String orcID = "0000-0001-1234-5678"; + final String idToken = createJWTWithEmptyAmr(orcID); + + setUpCallAuthTokenWithIdToken(authCode, "footoken11", "https://ologinredir.com", + idconfig.getClientID(), idconfig.getClientSecret(), " My name ", orcID, idToken); + setupCallID("footoken11", orcID, APP_JSON, 200, MAPPER.writeValueAsString( + map("email", Arrays.asList(map("email", "emptyamr@test.com"))))); + final Set rids = idp.getIdentities(authCode, "pkce", false, null); + assertThat("incorrect number of idents", rids.size(), is(1)); + final Set expected = new HashSet<>(); + expected.add(new RemoteIdentity(new RemoteIdentityID(ORCID, orcID), + new RemoteIdentityDetails(orcID, "My name", "emptyamr@test.com", false))); + assertThat("incorrect ident set", rids, is(expected)); + } + + @Test + public void getIdentityWithNoAmrClaimJWT() throws Exception { + final String authCode = "authcodeNoAmr"; + final IdentityProviderConfig idconfig = getTestIDConfig(); + final IdentityProvider idp = new OrcIDIdentityProvider(idconfig); + final String orcID = "0000-0001-1234-5678"; + final String idToken = createJWTWithoutAmr(orcID); + + setUpCallAuthTokenWithIdToken(authCode, "footoken12", "https://ologinredir.com", + idconfig.getClientID(), idconfig.getClientSecret(), " My name ", orcID, idToken); + setupCallID("footoken12", orcID, APP_JSON, 200, MAPPER.writeValueAsString( + map("email", Arrays.asList(map("email", "noamr@test.com"))))); + final Set rids = idp.getIdentities(authCode, "pkce", false, null); + assertThat("incorrect number of idents", rids.size(), is(1)); + final Set expected = new HashSet<>(); + expected.add(new RemoteIdentity(new RemoteIdentityID(ORCID, orcID), + new RemoteIdentityDetails(orcID, "My name", "noamr@test.com", null))); + assertThat("incorrect ident set", rids, is(expected)); + } + + private String createJWTWithEmptyAmr(final String orcID) { + final String header = "{\"alg\":\"HS256\",\"typ\":\"JWT\"}"; + final String payload = "{\"sub\":\"" + orcID + "\",\"amr\":[]}"; + final String encodedHeader = java.util.Base64.getUrlEncoder().withoutPadding() + .encodeToString(header.getBytes()); + final String encodedPayload = java.util.Base64.getUrlEncoder().withoutPadding() + .encodeToString(payload.getBytes()); + return encodedHeader + "." + encodedPayload + ".signature"; + } + + private String createJWTWithoutAmr(final String orcID) { + final String header = "{\"alg\":\"HS256\",\"typ\":\"JWT\"}"; + final String payload = "{\"sub\":\"" + orcID + "\",\"iss\":\"https://orcid.org\"}"; + final String encodedHeader = java.util.Base64.getUrlEncoder().withoutPadding() + .encodeToString(header.getBytes()); + final String encodedPayload = java.util.Base64.getUrlEncoder().withoutPadding() + .encodeToString(payload.getBytes()); + return encodedHeader + "." + encodedPayload + ".signature"; + } } diff --git a/src/test/java/us/kbase/test/auth2/service/api/MfaStatusTest.java b/src/test/java/us/kbase/test/auth2/service/api/MfaStatusTest.java new file mode 100644 index 00000000..1a8a70ef --- /dev/null +++ b/src/test/java/us/kbase/test/auth2/service/api/MfaStatusTest.java @@ -0,0 +1,322 @@ +package us.kbase.test.auth2.service.api; + +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertThat; +import static us.kbase.test.auth2.TestCommon.inst; +import static us.kbase.test.auth2.service.ServiceTestUtils.failRequestJSON; + +import java.net.URI; +import java.time.Instant; +import java.util.Map; +import java.util.UUID; + +import javax.ws.rs.client.Client; +import javax.ws.rs.client.ClientBuilder; +import javax.ws.rs.client.WebTarget; +import javax.ws.rs.client.Invocation.Builder; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; +import javax.ws.rs.core.UriBuilder; + +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; + +import us.kbase.auth2.kbase.KBaseAuthConfig; +import us.kbase.auth2.lib.DisplayName; +import us.kbase.auth2.lib.EmailAddress; +import us.kbase.auth2.lib.PasswordHashAndSalt; +import us.kbase.auth2.lib.TokenCreationContext; +import us.kbase.auth2.lib.UserName; +import us.kbase.auth2.lib.exceptions.NoTokenProvidedException; +import us.kbase.auth2.lib.exceptions.InvalidTokenException; +import us.kbase.auth2.lib.identity.RemoteIdentity; +import us.kbase.auth2.lib.identity.RemoteIdentityDetails; +import us.kbase.auth2.lib.identity.RemoteIdentityID; +import us.kbase.auth2.lib.token.IncomingToken; +import us.kbase.auth2.lib.token.NewToken; +import us.kbase.auth2.lib.token.StoredToken; +import us.kbase.auth2.lib.token.TokenType; +import us.kbase.auth2.lib.user.LocalUser; +import us.kbase.test.auth2.MongoStorageTestManager; +import us.kbase.test.auth2.StandaloneAuthServer; +import us.kbase.test.auth2.TestCommon; +import us.kbase.test.auth2.StandaloneAuthServer.ServerThread; +import us.kbase.test.auth2.service.ServiceTestUtils; + +public class MfaStatusTest { + + private static final String DB_NAME = "test_mfa_status_api"; + private static final String COOKIE_NAME = "login-cookie"; + + private static final Client CLI = ClientBuilder.newClient(); + + private static MongoStorageTestManager manager = null; + private static StandaloneAuthServer server = null; + private static int port = -1; + private static String host = null; + + @BeforeClass + public static void beforeClass() throws Exception { + TestCommon.stfuLoggers(); + manager = new MongoStorageTestManager(DB_NAME); + final java.nio.file.Path cfgfile = ServiceTestUtils.generateTempConfigFile(manager, DB_NAME, COOKIE_NAME); + TestCommon.getenv().put("KB_DEPLOYMENT_CONFIG", cfgfile.toString()); + server = new StandaloneAuthServer(KBaseAuthConfig.class.getName()); + new ServerThread(server).start(); + System.out.println("Main thread waiting for server to start up"); + while (server.getPort() == null) { + Thread.sleep(1000); + } + port = server.getPort(); + host = "http://localhost:" + port; + } + + @AfterClass + public static void afterClass() throws Exception { + if (server != null) { + server.stop(); + } + if (manager != null) { + manager.destroy(); + } + } + + @Before + public void beforeTest() throws Exception { + ServiceTestUtils.resetServer(manager, host, COOKIE_NAME); + } + + @Test + public void getMfaStatusWithMfaTrue() throws Exception { + final NewToken nt = setUpUserWithMfa(true); + final URI target = UriBuilder.fromUri(host).path("/api/V2/mfastatus").build(); + + final WebTarget wt = CLI.target(target); + final Builder req = wt.request() + .header("authorization", nt.getToken()); + + final Response res = req.get(); + + assertThat("incorrect response code", res.getStatus(), is(200)); + + @SuppressWarnings("unchecked") + final Map response = res.readEntity(Map.class); + + assertThat("incorrect provider", response.get("provider"), is("OrcID")); + assertThat("incorrect mfa_used", response.get("mfa_used"), is(true)); + assertThat("incorrect status", response.get("status"), is("mfa_authenticated")); + } + + @Test + public void getMfaStatusWithMfaFalse() throws Exception { + final NewToken nt = setUpUserWithMfa(false); + final URI target = UriBuilder.fromUri(host).path("/api/V2/mfastatus").build(); + + final WebTarget wt = CLI.target(target); + final Builder req = wt.request() + .header("authorization", nt.getToken()); + + final Response res = req.get(); + + assertThat("incorrect response code", res.getStatus(), is(200)); + + @SuppressWarnings("unchecked") + final Map response = res.readEntity(Map.class); + + assertThat("incorrect provider", response.get("provider"), is("OrcID")); + assertThat("incorrect mfa_used", response.get("mfa_used"), is(false)); + assertThat("incorrect status", response.get("status"), is("password_only")); + } + + @Test + public void getMfaStatusWithMfaNull() throws Exception { + final NewToken nt = setUpUserWithMfa(null); + final URI target = UriBuilder.fromUri(host).path("/api/V2/mfastatus").build(); + + final WebTarget wt = CLI.target(target); + final Builder req = wt.request() + .header("authorization", nt.getToken()); + + final Response res = req.get(); + + assertThat("incorrect response code", res.getStatus(), is(200)); + + @SuppressWarnings("unchecked") + final Map response = res.readEntity(Map.class); + + assertThat("incorrect provider", response.get("provider"), is("none")); + assertThat("incorrect mfa_used", response.get("mfa_used"), is((Object) null)); + assertThat("incorrect status", response.get("status"), is("no_mfa_info_available")); + } + + @Test + public void getMfaStatusFailNoToken() throws Exception { + final URI target = UriBuilder.fromUri(host).path("/api/V2/mfastatus").build(); + + final WebTarget wt = CLI.target(target); + final Builder req = wt.request() + .header("accept", MediaType.APPLICATION_JSON); + + final Response res = req.get(); + + failRequestJSON(res, 400, "Bad Request", + new NoTokenProvidedException("No user token provided")); + } + + @Test + public void getMfaStatusFailBadToken() throws Exception { + final URI target = UriBuilder.fromUri(host).path("/api/V2/mfastatus").build(); + + final WebTarget wt = CLI.target(target); + final Builder req = wt.request() + .header("authorization", "invalidtoken") + .header("accept", MediaType.APPLICATION_JSON); + + final Response res = req.get(); + + failRequestJSON(res, 401, "Unauthorized", new InvalidTokenException()); + } + + private NewToken setUpUserWithMfa(final Boolean mfaAuthenticated) throws Exception { + final RemoteIdentity remoteId = new RemoteIdentity( + new RemoteIdentityID("OrcID", "testid123"), + new RemoteIdentityDetails("testuser", "Test User", "test@example.com", mfaAuthenticated)); + + manager.storage.createUser(us.kbase.auth2.lib.user.NewUser.getBuilder( + new UserName("testuser"), UUID.randomUUID(), new DisplayName("Test User"), inst(10000), remoteId) + .withEmailAddress(new EmailAddress("test@example.com")).build()); + + final NewToken nt = new NewToken(StoredToken.getBuilder( + TokenType.LOGIN, UUID.randomUUID(), new UserName("testuser")) + .withLifeTime(Instant.ofEpochMilli(10000), + Instant.ofEpochMilli(1000000000000000L)) + .build(), + "testtokenvalue"); + manager.storage.storeToken(nt.getStoredToken(), nt.getTokenHash()); + return nt; + } + + @Test + public void getMfaStatusWithNonOrcidProvider() throws Exception { + final RemoteIdentity remoteId = new RemoteIdentity( + new RemoteIdentityID("Google", "googleid123"), + new RemoteIdentityDetails("googleuser", "Google User", "google@example.com", true)); + + manager.storage.createUser(us.kbase.auth2.lib.user.NewUser.getBuilder( + new UserName("googleuser"), UUID.randomUUID(), new DisplayName("Google User"), inst(10000), remoteId) + .withEmailAddress(new EmailAddress("google@example.com")).build()); + + final NewToken nt = new NewToken(StoredToken.getBuilder( + TokenType.LOGIN, UUID.randomUUID(), new UserName("googleuser")) + .withLifeTime(Instant.ofEpochMilli(10000), + Instant.ofEpochMilli(1000000000000000L)) + .build(), + "googletokenvalue"); + manager.storage.storeToken(nt.getStoredToken(), nt.getTokenHash()); + + final URI target = UriBuilder.fromUri(host).path("/api/V2/mfastatus").build(); + + final WebTarget wt = CLI.target(target); + final Builder req = wt.request() + .header("authorization", nt.getToken()); + + final Response res = req.get(); + + assertThat("incorrect response code", res.getStatus(), is(200)); + + @SuppressWarnings("unchecked") + final Map response = res.readEntity(Map.class); + + assertThat("incorrect provider", response.get("provider"), is("none")); + assertThat("incorrect mfa_used", response.get("mfa_used"), is((Object) null)); + assertThat("incorrect status", response.get("status"), is("no_mfa_info_available")); + } + + @Test + public void getMfaStatusWithMultipleOrcidIdentities() throws Exception { + final RemoteIdentity orcidId1 = new RemoteIdentity( + new RemoteIdentityID("OrcID", "0000-0001-1234-5678"), + new RemoteIdentityDetails("orciduser1", "ORCID User 1", "orcid1@example.com", false)); + + final RemoteIdentity orcidId2 = new RemoteIdentity( + new RemoteIdentityID("OrcID", "0000-0001-1234-9999"), + new RemoteIdentityDetails("orciduser2", "ORCID User 2", "orcid2@example.com", true)); + + manager.storage.createUser(us.kbase.auth2.lib.user.NewUser.getBuilder( + new UserName("multiorciduser"), UUID.randomUUID(), new DisplayName("Multi ORCID User"), inst(10000), orcidId1) + .withEmailAddress(new EmailAddress("multi@example.com")).build()); + + // Link second ORCID identity + manager.storage.link(new UserName("multiorciduser"), orcidId2); + + final NewToken nt = new NewToken(StoredToken.getBuilder( + TokenType.LOGIN, UUID.randomUUID(), new UserName("multiorciduser")) + .withLifeTime(Instant.ofEpochMilli(10000), + Instant.ofEpochMilli(1000000000000000L)) + .build(), + "multiorcidtokenvalue"); + manager.storage.storeToken(nt.getStoredToken(), nt.getTokenHash()); + + final URI target = UriBuilder.fromUri(host).path("/api/V2/mfastatus").build(); + + final WebTarget wt = CLI.target(target); + final Builder req = wt.request() + .header("authorization", nt.getToken()); + + final Response res = req.get(); + + assertThat("incorrect response code", res.getStatus(), is(200)); + + @SuppressWarnings("unchecked") + final Map response = res.readEntity(Map.class); + + // Should return first ORCID identity with MFA info (false in this case) + assertThat("incorrect provider", response.get("provider"), is("OrcID")); + assertThat("incorrect mfa_used", response.get("mfa_used"), is(false)); + assertThat("incorrect status", response.get("status"), is("password_only")); + } + + @Test + public void getMfaStatusWithNoIdentities() throws Exception { + // This test is theoretical - in practice users always have at least one identity + // But we test the edge case for completeness + final RemoteIdentity tempId = new RemoteIdentity( + new RemoteIdentityID("TempProvider", "tempid123"), + new RemoteIdentityDetails("tempuser", "Temp User", "temp@example.com", null)); + + manager.storage.createUser(us.kbase.auth2.lib.user.NewUser.getBuilder( + new UserName("noidentuser"), UUID.randomUUID(), new DisplayName("No Ident User"), inst(10000), tempId) + .withEmailAddress(new EmailAddress("noident@example.com")).build()); + + // Manually remove all identities (simulating edge case) + // Note: This would be impossible in normal operation + manager.storage.unlink(new UserName("noidentuser"), tempId.getRemoteID()); + + final NewToken nt = new NewToken(StoredToken.getBuilder( + TokenType.LOGIN, UUID.randomUUID(), new UserName("noidentuser")) + .withLifeTime(Instant.ofEpochMilli(10000), + Instant.ofEpochMilli(1000000000000000L)) + .build(), + "noidenttokenvalue"); + manager.storage.storeToken(nt.getStoredToken(), nt.getTokenHash()); + + final URI target = UriBuilder.fromUri(host).path("/api/V2/mfastatus").build(); + + final WebTarget wt = CLI.target(target); + final Builder req = wt.request() + .header("authorization", nt.getToken()); + + final Response res = req.get(); + + assertThat("incorrect response code", res.getStatus(), is(200)); + + @SuppressWarnings("unchecked") + final Map response = res.readEntity(Map.class); + + assertThat("incorrect provider", response.get("provider"), is("none")); + assertThat("incorrect mfa_used", response.get("mfa_used"), is((Object) null)); + assertThat("incorrect status", response.get("status"), is("no_mfa_info_available")); + } +} \ No newline at end of file From 984fdb2a3bf3c85b197d8299e27099bdd6e796b5 Mon Sep 17 00:00:00 2001 From: David Lyon <5115845+dauglyon@users.noreply.github.com> Date: Tue, 15 Jul 2025 12:05:08 -0700 Subject: [PATCH 02/25] move MFA status to token endpoint; remove mfaStatus endpoint; clean up code --- .../us/kbase/auth2/lib/Authentication.java | 28 ++ .../us/kbase/auth2/service/api/APIPaths.java | 3 - .../us/kbase/auth2/service/api/APIToken.java | 41 +++ .../us/kbase/auth2/service/api/MfaStatus.java | 89 ----- .../us/kbase/auth2/service/api/Token.java | 6 +- .../test/auth2/service/api/APITokenTest.java | 1 + .../test/auth2/service/api/MfaStatusTest.java | 322 ------------------ .../auth2/service/api/TokenEndpointTest.java | 154 +++++++++ 8 files changed, 228 insertions(+), 416 deletions(-) delete mode 100644 src/main/java/us/kbase/auth2/service/api/MfaStatus.java delete mode 100644 src/test/java/us/kbase/test/auth2/service/api/MfaStatusTest.java diff --git a/src/main/java/us/kbase/auth2/lib/Authentication.java b/src/main/java/us/kbase/auth2/lib/Authentication.java index 72fc0b7d..0e8eafbe 100644 --- a/src/main/java/us/kbase/auth2/lib/Authentication.java +++ b/src/main/java/us/kbase/auth2/lib/Authentication.java @@ -3142,6 +3142,34 @@ public long getSuggestedTokenCacheTime() throws AuthStorageException { return cfg.getAppConfig().getTokenLifetimeMS(TokenLifetimeType.EXT_CACHE); } + /** Get MFA status for a token by checking the token user's identities. + * @param token the token to check. + * @return true if MFA was used, false if password only, null if unknown or not supported. + * @throws AuthStorageException if an error occurred accessing the storage system. + */ + public Boolean getMfaStatus(final IncomingToken token) throws AuthStorageException { + if (token == null) { + return null; + } + try { + final AuthUser user = getUser(token); + final Set identities = user.getIdentities(); + + // Check for identities with MFA information from supported providers + for (final us.kbase.auth2.lib.identity.RemoteIdentity identity : identities) { + final Boolean mfaStatus = identity.getDetails().isMfaAuthenticated(); + if (mfaStatus != null) { + return mfaStatus; + } + } + + return null; // No MFA information available + } catch (Exception e) { + // For any errors, return null rather than failing the request + return null; + } + } + /** Get the external configuration without providing any credentials. * * This method should not be exposed in a public API. diff --git a/src/main/java/us/kbase/auth2/service/api/APIPaths.java b/src/main/java/us/kbase/auth2/service/api/APIPaths.java index 5367abe7..acfeb1d5 100644 --- a/src/main/java/us/kbase/auth2/service/api/APIPaths.java +++ b/src/main/java/us/kbase/auth2/service/api/APIPaths.java @@ -54,9 +54,6 @@ public class APIPaths { /** The me endpoint location. */ public static final String API_V2_ME = API_V2 + SEP + ME; - /** The MFA status endpoint location. */ - public static final String API_V2_MFA_STATUS = API_V2 + SEP + "mfastatus"; - /* test mode endpoints. */ /** The testmode root endpoint. */ diff --git a/src/main/java/us/kbase/auth2/service/api/APIToken.java b/src/main/java/us/kbase/auth2/service/api/APIToken.java index a7cffa31..394c8236 100644 --- a/src/main/java/us/kbase/auth2/service/api/APIToken.java +++ b/src/main/java/us/kbase/auth2/service/api/APIToken.java @@ -8,21 +8,55 @@ public class APIToken extends ExternalToken { //TODO JAVADOC or swagger private final long cachefor; + private final Boolean mfaAuthenticated; + /** + * Constructor without MFA status calculation. + * MFA status will be set to null. + * + * @param token the stored token + * @param tokenCacheTimeMillis the token cache time in milliseconds + */ public APIToken(final StoredToken token, final long tokenCacheTimeMillis) { super(token); cachefor = tokenCacheTimeMillis; + mfaAuthenticated = null; + } + + /** + * Constructor with MFA status provided. + * + * @param token the stored token + * @param tokenCacheTimeMillis the token cache time in milliseconds + * @param mfaAuthenticated the MFA authentication status + */ + public APIToken(final StoredToken token, final long tokenCacheTimeMillis, + final Boolean mfaAuthenticated) { + super(token); + cachefor = tokenCacheTimeMillis; + this.mfaAuthenticated = mfaAuthenticated; } public long getCachefor() { return cachefor; } + + /** + * Gets the MFA authentication status for this token. + * + * @return true if the user authenticated with MFA, false if password only, + * null if unknown or not applicable + */ + public Boolean getMfaAuthenticated() { + return mfaAuthenticated; + } @Override public int hashCode() { final int prime = 31; int result = super.hashCode(); result = prime * result + (int) (cachefor ^ (cachefor >>> 32)); + result = prime * result + ((mfaAuthenticated == null) ? 0 : mfaAuthenticated.hashCode()); return result; } @@ -38,6 +72,13 @@ public boolean equals(Object obj) { if (cachefor != other.cachefor) { return false; } + if (mfaAuthenticated == null) { + if (other.mfaAuthenticated != null) { + return false; + } + } else if (!mfaAuthenticated.equals(other.mfaAuthenticated)) { + return false; + } return true; } diff --git a/src/main/java/us/kbase/auth2/service/api/MfaStatus.java b/src/main/java/us/kbase/auth2/service/api/MfaStatus.java deleted file mode 100644 index ae4c42c7..00000000 --- a/src/main/java/us/kbase/auth2/service/api/MfaStatus.java +++ /dev/null @@ -1,89 +0,0 @@ -package us.kbase.auth2.service.api; - -import static us.kbase.auth2.service.common.ServiceCommon.getToken; - -import java.util.Map; -import java.util.Set; - -import javax.inject.Inject; -import javax.ws.rs.GET; -import javax.ws.rs.HeaderParam; -import javax.ws.rs.Path; -import javax.ws.rs.Produces; -import javax.ws.rs.core.MediaType; - -import com.google.common.collect.ImmutableMap; - -import us.kbase.auth2.lib.Authentication; -import us.kbase.auth2.lib.exceptions.DisabledUserException; -import us.kbase.auth2.lib.exceptions.InvalidTokenException; -import us.kbase.auth2.lib.exceptions.NoTokenProvidedException; -import us.kbase.auth2.lib.identity.RemoteIdentity; -import us.kbase.auth2.lib.storage.exceptions.AuthStorageException; -import us.kbase.auth2.lib.user.AuthUser; - -/** - * API endpoint for checking multi-factor authentication status of user tokens. - * - * This endpoint allows clients to determine if a user's current session was - * authenticated using multi-factor authentication via supported identity providers. - */ -@Path(APIPaths.API_V2_MFA_STATUS) -public class MfaStatus { - - @Inject - private Authentication auth; - - /** - * Returns the MFA authentication status for the current user token. - * - * Checks if the user authenticated using multi-factor authentication through - * supported identity providers (currently ORCID with OpenID Connect). - * - * @param token the user's authentication token in the Authorization header - * @return JSON object containing MFA status information - * @throws InvalidTokenException if the token is invalid or expired - * @throws AuthStorageException if there's a database access error - * @throws NoTokenProvidedException if no token is provided - * @throws DisabledUserException if the user account is disabled - */ - @GET - @Produces(MediaType.APPLICATION_JSON) - public Map getMfaStatus( - @HeaderParam("Authorization") final String token) - throws InvalidTokenException, AuthStorageException, NoTokenProvidedException, - DisabledUserException { - - final AuthUser user = auth.getUser(getToken(token)); - - // Check for ORCID identities with MFA information - Boolean orcidMfaUsed = null; - String providerName = null; - - final Set identities = user.getIdentities(); - for (final RemoteIdentity identity : identities) { - if ("OrcID".equals(identity.getRemoteID().getProviderName())) { - final Boolean mfaStatus = identity.getDetails().isMfaAuthenticated(); - if (mfaStatus != null) { - orcidMfaUsed = mfaStatus; - providerName = identity.getRemoteID().getProviderName(); - break; // Use the first ORCID identity with MFA information - } - } - } - - if (orcidMfaUsed != null) { - return ImmutableMap.of( - "provider", providerName, - "mfa_used", orcidMfaUsed, - "status", orcidMfaUsed ? "mfa_authenticated" : "password_only" - ); - } else { - return ImmutableMap.of( - "provider", "none", - "mfa_used", (Object) null, - "status", "no_mfa_info_available" - ); - } - } -} \ No newline at end of file diff --git a/src/main/java/us/kbase/auth2/service/api/Token.java b/src/main/java/us/kbase/auth2/service/api/Token.java index 234a1131..d4425434 100644 --- a/src/main/java/us/kbase/auth2/service/api/Token.java +++ b/src/main/java/us/kbase/auth2/service/api/Token.java @@ -29,6 +29,7 @@ import us.kbase.auth2.lib.exceptions.NoTokenProvidedException; import us.kbase.auth2.lib.exceptions.UnauthorizedException; import us.kbase.auth2.lib.storage.exceptions.AuthStorageException; +import us.kbase.auth2.lib.token.IncomingToken; import us.kbase.auth2.lib.token.StoredToken; import us.kbase.auth2.lib.token.TokenName; import us.kbase.auth2.lib.token.TokenType; @@ -55,8 +56,9 @@ public Token(final Authentication auth, final UserAgentParser userAgentParser) { @Produces(MediaType.APPLICATION_JSON) public APIToken viewToken(@HeaderParam(APIConstants.HEADER_TOKEN) final String token) throws NoTokenProvidedException, InvalidTokenException, AuthStorageException { - final StoredToken ht = auth.getToken(getToken(token)); - return new APIToken(ht, auth.getSuggestedTokenCacheTime()); + final IncomingToken it = getToken(token); + final StoredToken ht = auth.getToken(it); + return new APIToken(ht, auth.getSuggestedTokenCacheTime(), auth.getMfaStatus(it)); } private static class CreateToken extends IncomingJSON { diff --git a/src/test/java/us/kbase/test/auth2/service/api/APITokenTest.java b/src/test/java/us/kbase/test/auth2/service/api/APITokenTest.java index 9748f0a6..f89e4d82 100644 --- a/src/test/java/us/kbase/test/auth2/service/api/APITokenTest.java +++ b/src/test/java/us/kbase/test/auth2/service/api/APITokenTest.java @@ -39,6 +39,7 @@ TokenType.AGENT, id, new UserName("foo")) assertThat("incorrect id", t.getId(), is(id.toString())); assertThat("incorrect cache time", t.getCachefor(), is(20000L)); + assertThat("incorrect mfa status", t.getMfaAuthenticated(), is((Boolean) null)); } diff --git a/src/test/java/us/kbase/test/auth2/service/api/MfaStatusTest.java b/src/test/java/us/kbase/test/auth2/service/api/MfaStatusTest.java deleted file mode 100644 index 1a8a70ef..00000000 --- a/src/test/java/us/kbase/test/auth2/service/api/MfaStatusTest.java +++ /dev/null @@ -1,322 +0,0 @@ -package us.kbase.test.auth2.service.api; - -import static org.hamcrest.CoreMatchers.is; -import static org.junit.Assert.assertThat; -import static us.kbase.test.auth2.TestCommon.inst; -import static us.kbase.test.auth2.service.ServiceTestUtils.failRequestJSON; - -import java.net.URI; -import java.time.Instant; -import java.util.Map; -import java.util.UUID; - -import javax.ws.rs.client.Client; -import javax.ws.rs.client.ClientBuilder; -import javax.ws.rs.client.WebTarget; -import javax.ws.rs.client.Invocation.Builder; -import javax.ws.rs.core.MediaType; -import javax.ws.rs.core.Response; -import javax.ws.rs.core.UriBuilder; - -import org.junit.AfterClass; -import org.junit.Before; -import org.junit.BeforeClass; -import org.junit.Test; - -import us.kbase.auth2.kbase.KBaseAuthConfig; -import us.kbase.auth2.lib.DisplayName; -import us.kbase.auth2.lib.EmailAddress; -import us.kbase.auth2.lib.PasswordHashAndSalt; -import us.kbase.auth2.lib.TokenCreationContext; -import us.kbase.auth2.lib.UserName; -import us.kbase.auth2.lib.exceptions.NoTokenProvidedException; -import us.kbase.auth2.lib.exceptions.InvalidTokenException; -import us.kbase.auth2.lib.identity.RemoteIdentity; -import us.kbase.auth2.lib.identity.RemoteIdentityDetails; -import us.kbase.auth2.lib.identity.RemoteIdentityID; -import us.kbase.auth2.lib.token.IncomingToken; -import us.kbase.auth2.lib.token.NewToken; -import us.kbase.auth2.lib.token.StoredToken; -import us.kbase.auth2.lib.token.TokenType; -import us.kbase.auth2.lib.user.LocalUser; -import us.kbase.test.auth2.MongoStorageTestManager; -import us.kbase.test.auth2.StandaloneAuthServer; -import us.kbase.test.auth2.TestCommon; -import us.kbase.test.auth2.StandaloneAuthServer.ServerThread; -import us.kbase.test.auth2.service.ServiceTestUtils; - -public class MfaStatusTest { - - private static final String DB_NAME = "test_mfa_status_api"; - private static final String COOKIE_NAME = "login-cookie"; - - private static final Client CLI = ClientBuilder.newClient(); - - private static MongoStorageTestManager manager = null; - private static StandaloneAuthServer server = null; - private static int port = -1; - private static String host = null; - - @BeforeClass - public static void beforeClass() throws Exception { - TestCommon.stfuLoggers(); - manager = new MongoStorageTestManager(DB_NAME); - final java.nio.file.Path cfgfile = ServiceTestUtils.generateTempConfigFile(manager, DB_NAME, COOKIE_NAME); - TestCommon.getenv().put("KB_DEPLOYMENT_CONFIG", cfgfile.toString()); - server = new StandaloneAuthServer(KBaseAuthConfig.class.getName()); - new ServerThread(server).start(); - System.out.println("Main thread waiting for server to start up"); - while (server.getPort() == null) { - Thread.sleep(1000); - } - port = server.getPort(); - host = "http://localhost:" + port; - } - - @AfterClass - public static void afterClass() throws Exception { - if (server != null) { - server.stop(); - } - if (manager != null) { - manager.destroy(); - } - } - - @Before - public void beforeTest() throws Exception { - ServiceTestUtils.resetServer(manager, host, COOKIE_NAME); - } - - @Test - public void getMfaStatusWithMfaTrue() throws Exception { - final NewToken nt = setUpUserWithMfa(true); - final URI target = UriBuilder.fromUri(host).path("/api/V2/mfastatus").build(); - - final WebTarget wt = CLI.target(target); - final Builder req = wt.request() - .header("authorization", nt.getToken()); - - final Response res = req.get(); - - assertThat("incorrect response code", res.getStatus(), is(200)); - - @SuppressWarnings("unchecked") - final Map response = res.readEntity(Map.class); - - assertThat("incorrect provider", response.get("provider"), is("OrcID")); - assertThat("incorrect mfa_used", response.get("mfa_used"), is(true)); - assertThat("incorrect status", response.get("status"), is("mfa_authenticated")); - } - - @Test - public void getMfaStatusWithMfaFalse() throws Exception { - final NewToken nt = setUpUserWithMfa(false); - final URI target = UriBuilder.fromUri(host).path("/api/V2/mfastatus").build(); - - final WebTarget wt = CLI.target(target); - final Builder req = wt.request() - .header("authorization", nt.getToken()); - - final Response res = req.get(); - - assertThat("incorrect response code", res.getStatus(), is(200)); - - @SuppressWarnings("unchecked") - final Map response = res.readEntity(Map.class); - - assertThat("incorrect provider", response.get("provider"), is("OrcID")); - assertThat("incorrect mfa_used", response.get("mfa_used"), is(false)); - assertThat("incorrect status", response.get("status"), is("password_only")); - } - - @Test - public void getMfaStatusWithMfaNull() throws Exception { - final NewToken nt = setUpUserWithMfa(null); - final URI target = UriBuilder.fromUri(host).path("/api/V2/mfastatus").build(); - - final WebTarget wt = CLI.target(target); - final Builder req = wt.request() - .header("authorization", nt.getToken()); - - final Response res = req.get(); - - assertThat("incorrect response code", res.getStatus(), is(200)); - - @SuppressWarnings("unchecked") - final Map response = res.readEntity(Map.class); - - assertThat("incorrect provider", response.get("provider"), is("none")); - assertThat("incorrect mfa_used", response.get("mfa_used"), is((Object) null)); - assertThat("incorrect status", response.get("status"), is("no_mfa_info_available")); - } - - @Test - public void getMfaStatusFailNoToken() throws Exception { - final URI target = UriBuilder.fromUri(host).path("/api/V2/mfastatus").build(); - - final WebTarget wt = CLI.target(target); - final Builder req = wt.request() - .header("accept", MediaType.APPLICATION_JSON); - - final Response res = req.get(); - - failRequestJSON(res, 400, "Bad Request", - new NoTokenProvidedException("No user token provided")); - } - - @Test - public void getMfaStatusFailBadToken() throws Exception { - final URI target = UriBuilder.fromUri(host).path("/api/V2/mfastatus").build(); - - final WebTarget wt = CLI.target(target); - final Builder req = wt.request() - .header("authorization", "invalidtoken") - .header("accept", MediaType.APPLICATION_JSON); - - final Response res = req.get(); - - failRequestJSON(res, 401, "Unauthorized", new InvalidTokenException()); - } - - private NewToken setUpUserWithMfa(final Boolean mfaAuthenticated) throws Exception { - final RemoteIdentity remoteId = new RemoteIdentity( - new RemoteIdentityID("OrcID", "testid123"), - new RemoteIdentityDetails("testuser", "Test User", "test@example.com", mfaAuthenticated)); - - manager.storage.createUser(us.kbase.auth2.lib.user.NewUser.getBuilder( - new UserName("testuser"), UUID.randomUUID(), new DisplayName("Test User"), inst(10000), remoteId) - .withEmailAddress(new EmailAddress("test@example.com")).build()); - - final NewToken nt = new NewToken(StoredToken.getBuilder( - TokenType.LOGIN, UUID.randomUUID(), new UserName("testuser")) - .withLifeTime(Instant.ofEpochMilli(10000), - Instant.ofEpochMilli(1000000000000000L)) - .build(), - "testtokenvalue"); - manager.storage.storeToken(nt.getStoredToken(), nt.getTokenHash()); - return nt; - } - - @Test - public void getMfaStatusWithNonOrcidProvider() throws Exception { - final RemoteIdentity remoteId = new RemoteIdentity( - new RemoteIdentityID("Google", "googleid123"), - new RemoteIdentityDetails("googleuser", "Google User", "google@example.com", true)); - - manager.storage.createUser(us.kbase.auth2.lib.user.NewUser.getBuilder( - new UserName("googleuser"), UUID.randomUUID(), new DisplayName("Google User"), inst(10000), remoteId) - .withEmailAddress(new EmailAddress("google@example.com")).build()); - - final NewToken nt = new NewToken(StoredToken.getBuilder( - TokenType.LOGIN, UUID.randomUUID(), new UserName("googleuser")) - .withLifeTime(Instant.ofEpochMilli(10000), - Instant.ofEpochMilli(1000000000000000L)) - .build(), - "googletokenvalue"); - manager.storage.storeToken(nt.getStoredToken(), nt.getTokenHash()); - - final URI target = UriBuilder.fromUri(host).path("/api/V2/mfastatus").build(); - - final WebTarget wt = CLI.target(target); - final Builder req = wt.request() - .header("authorization", nt.getToken()); - - final Response res = req.get(); - - assertThat("incorrect response code", res.getStatus(), is(200)); - - @SuppressWarnings("unchecked") - final Map response = res.readEntity(Map.class); - - assertThat("incorrect provider", response.get("provider"), is("none")); - assertThat("incorrect mfa_used", response.get("mfa_used"), is((Object) null)); - assertThat("incorrect status", response.get("status"), is("no_mfa_info_available")); - } - - @Test - public void getMfaStatusWithMultipleOrcidIdentities() throws Exception { - final RemoteIdentity orcidId1 = new RemoteIdentity( - new RemoteIdentityID("OrcID", "0000-0001-1234-5678"), - new RemoteIdentityDetails("orciduser1", "ORCID User 1", "orcid1@example.com", false)); - - final RemoteIdentity orcidId2 = new RemoteIdentity( - new RemoteIdentityID("OrcID", "0000-0001-1234-9999"), - new RemoteIdentityDetails("orciduser2", "ORCID User 2", "orcid2@example.com", true)); - - manager.storage.createUser(us.kbase.auth2.lib.user.NewUser.getBuilder( - new UserName("multiorciduser"), UUID.randomUUID(), new DisplayName("Multi ORCID User"), inst(10000), orcidId1) - .withEmailAddress(new EmailAddress("multi@example.com")).build()); - - // Link second ORCID identity - manager.storage.link(new UserName("multiorciduser"), orcidId2); - - final NewToken nt = new NewToken(StoredToken.getBuilder( - TokenType.LOGIN, UUID.randomUUID(), new UserName("multiorciduser")) - .withLifeTime(Instant.ofEpochMilli(10000), - Instant.ofEpochMilli(1000000000000000L)) - .build(), - "multiorcidtokenvalue"); - manager.storage.storeToken(nt.getStoredToken(), nt.getTokenHash()); - - final URI target = UriBuilder.fromUri(host).path("/api/V2/mfastatus").build(); - - final WebTarget wt = CLI.target(target); - final Builder req = wt.request() - .header("authorization", nt.getToken()); - - final Response res = req.get(); - - assertThat("incorrect response code", res.getStatus(), is(200)); - - @SuppressWarnings("unchecked") - final Map response = res.readEntity(Map.class); - - // Should return first ORCID identity with MFA info (false in this case) - assertThat("incorrect provider", response.get("provider"), is("OrcID")); - assertThat("incorrect mfa_used", response.get("mfa_used"), is(false)); - assertThat("incorrect status", response.get("status"), is("password_only")); - } - - @Test - public void getMfaStatusWithNoIdentities() throws Exception { - // This test is theoretical - in practice users always have at least one identity - // But we test the edge case for completeness - final RemoteIdentity tempId = new RemoteIdentity( - new RemoteIdentityID("TempProvider", "tempid123"), - new RemoteIdentityDetails("tempuser", "Temp User", "temp@example.com", null)); - - manager.storage.createUser(us.kbase.auth2.lib.user.NewUser.getBuilder( - new UserName("noidentuser"), UUID.randomUUID(), new DisplayName("No Ident User"), inst(10000), tempId) - .withEmailAddress(new EmailAddress("noident@example.com")).build()); - - // Manually remove all identities (simulating edge case) - // Note: This would be impossible in normal operation - manager.storage.unlink(new UserName("noidentuser"), tempId.getRemoteID()); - - final NewToken nt = new NewToken(StoredToken.getBuilder( - TokenType.LOGIN, UUID.randomUUID(), new UserName("noidentuser")) - .withLifeTime(Instant.ofEpochMilli(10000), - Instant.ofEpochMilli(1000000000000000L)) - .build(), - "noidenttokenvalue"); - manager.storage.storeToken(nt.getStoredToken(), nt.getTokenHash()); - - final URI target = UriBuilder.fromUri(host).path("/api/V2/mfastatus").build(); - - final WebTarget wt = CLI.target(target); - final Builder req = wt.request() - .header("authorization", nt.getToken()); - - final Response res = req.get(); - - assertThat("incorrect response code", res.getStatus(), is(200)); - - @SuppressWarnings("unchecked") - final Map response = res.readEntity(Map.class); - - assertThat("incorrect provider", response.get("provider"), is("none")); - assertThat("incorrect mfa_used", response.get("mfa_used"), is((Object) null)); - assertThat("incorrect status", response.get("status"), is("no_mfa_info_available")); - } -} \ No newline at end of file diff --git a/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java b/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java index 4bdb2a34..4f42fd8d 100644 --- a/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java +++ b/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java @@ -174,6 +174,7 @@ TokenType.AGENT, id, new UserName("foo")) .with("user", "foo") .with("custom", ImmutableMap.of("whee", "whoo")) .with("cachefor", 300000) + .with("mfaAuthenticated", null) .build(); assertThat("incorrect token", response, is(expected)); @@ -606,4 +607,157 @@ private void kbaseTokenSuccess( assertThat("incorrect response", response, is(expected)); } + + @Test + public void getTokenWithMfaTrue() throws Exception { + final UUID id = UUID.randomUUID(); + final IncomingToken it = new IncomingToken("mfatokenvalue"); + + // Create user with ORCID identity that used MFA + final us.kbase.auth2.lib.identity.RemoteIdentity orcidId = new us.kbase.auth2.lib.identity.RemoteIdentity( + new us.kbase.auth2.lib.identity.RemoteIdentityID("OrcID", "0000-0001-1234-5678"), + new us.kbase.auth2.lib.identity.RemoteIdentityDetails("orciduser", "ORCID User", "orcid@example.com", true)); + + manager.storage.createUser(us.kbase.auth2.lib.user.NewUser.getBuilder( + new UserName("mfauser"), id, new DisplayName("MFA User"), inst(10000), orcidId) + .withEmailAddress(new EmailAddress("mfa@example.com")).build()); + + manager.storage.storeToken(StoredToken.getBuilder( + TokenType.AGENT, id, new UserName("mfauser")) + .withLifeTime(Instant.ofEpochMilli(10000), Instant.ofEpochMilli(1000000000000000L)) + .withTokenName(new TokenName("mfatoken")) + .build(), it.getHashedToken().getTokenHash()); + + final URI target = UriBuilder.fromUri(host).path("/api/V2/token").build(); + + final WebTarget wt = CLI.target(target); + final Builder req = wt.request() + .header("authorization", it.getToken()); + + final Response res = req.get(); + + assertThat("incorrect response code", res.getStatus(), is(200)); + + @SuppressWarnings("unchecked") + final Map response = res.readEntity(Map.class); + + assertThat("incorrect MFA status", response.get("mfaAuthenticated"), is(true)); + assertThat("incorrect user", response.get("user"), is("mfauser")); + assertThat("incorrect token name", response.get("name"), is("mfatoken")); + } + + @Test + public void getTokenWithMfaFalse() throws Exception { + final UUID id = UUID.randomUUID(); + final IncomingToken it = new IncomingToken("nomfatokenvalue"); + + // Create user with ORCID identity that did NOT use MFA + final us.kbase.auth2.lib.identity.RemoteIdentity orcidId = new us.kbase.auth2.lib.identity.RemoteIdentity( + new us.kbase.auth2.lib.identity.RemoteIdentityID("OrcID", "0000-0001-1234-9999"), + new us.kbase.auth2.lib.identity.RemoteIdentityDetails("orciduser2", "ORCID User 2", "orcid2@example.com", false)); + + manager.storage.createUser(us.kbase.auth2.lib.user.NewUser.getBuilder( + new UserName("nomfauser"), id, new DisplayName("No MFA User"), inst(10000), orcidId) + .withEmailAddress(new EmailAddress("nomfa@example.com")).build()); + + manager.storage.storeToken(StoredToken.getBuilder( + TokenType.AGENT, id, new UserName("nomfauser")) + .withLifeTime(Instant.ofEpochMilli(10000), Instant.ofEpochMilli(1000000000000000L)) + .withTokenName(new TokenName("nomfatoken")) + .build(), it.getHashedToken().getTokenHash()); + + final URI target = UriBuilder.fromUri(host).path("/api/V2/token").build(); + + final WebTarget wt = CLI.target(target); + final Builder req = wt.request() + .header("authorization", it.getToken()); + + final Response res = req.get(); + + assertThat("incorrect response code", res.getStatus(), is(200)); + + @SuppressWarnings("unchecked") + final Map response = res.readEntity(Map.class); + + assertThat("incorrect MFA status", response.get("mfaAuthenticated"), is(false)); + assertThat("incorrect user", response.get("user"), is("nomfauser")); + assertThat("incorrect token name", response.get("name"), is("nomfatoken")); + } + + @Test + public void getTokenWithMfaNull() throws Exception { + final UUID id = UUID.randomUUID(); + final IncomingToken it = new IncomingToken("unknownmfatokenvalue"); + + // Create user with ORCID identity that has unknown MFA status + final us.kbase.auth2.lib.identity.RemoteIdentity orcidId = new us.kbase.auth2.lib.identity.RemoteIdentity( + new us.kbase.auth2.lib.identity.RemoteIdentityID("OrcID", "0000-0001-1234-0000"), + new us.kbase.auth2.lib.identity.RemoteIdentityDetails("orciduser3", "ORCID User 3", "orcid3@example.com", null)); + + manager.storage.createUser(us.kbase.auth2.lib.user.NewUser.getBuilder( + new UserName("unknownmfauser"), id, new DisplayName("Unknown MFA User"), inst(10000), orcidId) + .withEmailAddress(new EmailAddress("unknownmfa@example.com")).build()); + + manager.storage.storeToken(StoredToken.getBuilder( + TokenType.AGENT, id, new UserName("unknownmfauser")) + .withLifeTime(Instant.ofEpochMilli(10000), Instant.ofEpochMilli(1000000000000000L)) + .withTokenName(new TokenName("unknownmfatoken")) + .build(), it.getHashedToken().getTokenHash()); + + final URI target = UriBuilder.fromUri(host).path("/api/V2/token").build(); + + final WebTarget wt = CLI.target(target); + final Builder req = wt.request() + .header("authorization", it.getToken()); + + final Response res = req.get(); + + assertThat("incorrect response code", res.getStatus(), is(200)); + + @SuppressWarnings("unchecked") + final Map response = res.readEntity(Map.class); + + assertThat("incorrect MFA status", response.get("mfaAuthenticated"), is((Object) null)); + assertThat("incorrect user", response.get("user"), is("unknownmfauser")); + assertThat("incorrect token name", response.get("name"), is("unknownmfatoken")); + } + + @Test + public void getTokenWithNonOrcidProvider() throws Exception { + final UUID id = UUID.randomUUID(); + final IncomingToken it = new IncomingToken("googletokenvalue"); + + // Create user with non-ORCID identity (e.g., Google) + final us.kbase.auth2.lib.identity.RemoteIdentity googleId = new us.kbase.auth2.lib.identity.RemoteIdentity( + new us.kbase.auth2.lib.identity.RemoteIdentityID("Google", "googleid123"), + new us.kbase.auth2.lib.identity.RemoteIdentityDetails("googleuser", "Google User", "google@example.com", true)); + + manager.storage.createUser(us.kbase.auth2.lib.user.NewUser.getBuilder( + new UserName("googleuser"), id, new DisplayName("Google User"), inst(10000), googleId) + .withEmailAddress(new EmailAddress("google@example.com")).build()); + + manager.storage.storeToken(StoredToken.getBuilder( + TokenType.AGENT, id, new UserName("googleuser")) + .withLifeTime(Instant.ofEpochMilli(10000), Instant.ofEpochMilli(1000000000000000L)) + .withTokenName(new TokenName("googletoken")) + .build(), it.getHashedToken().getTokenHash()); + + final URI target = UriBuilder.fromUri(host).path("/api/V2/token").build(); + + final WebTarget wt = CLI.target(target); + final Builder req = wt.request() + .header("authorization", it.getToken()); + + final Response res = req.get(); + + assertThat("incorrect response code", res.getStatus(), is(200)); + + @SuppressWarnings("unchecked") + final Map response = res.readEntity(Map.class); + + // Non-ORCID providers should return null for MFA status + assertThat("incorrect MFA status", response.get("mfaAuthenticated"), is((Object) null)); + assertThat("incorrect user", response.get("user"), is("googleuser")); + assertThat("incorrect token name", response.get("name"), is("googletoken")); + } } From e23c5dd088ad7ad7919faaeea6efc288101f8dd5 Mon Sep 17 00:00:00 2001 From: David Lyon <5115845+dauglyon@users.noreply.github.com> Date: Tue, 15 Jul 2025 12:23:08 -0700 Subject: [PATCH 03/25] fix build errors --- src/main/java/us/kbase/auth2/lib/Authentication.java | 2 +- .../us/kbase/auth2/lib/identity/RemoteIdentityDetails.java | 2 +- .../java/us/kbase/auth2/lib/storage/mongo/MongoStorage.java | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/us/kbase/auth2/lib/Authentication.java b/src/main/java/us/kbase/auth2/lib/Authentication.java index 0e8eafbe..d1a5d572 100644 --- a/src/main/java/us/kbase/auth2/lib/Authentication.java +++ b/src/main/java/us/kbase/auth2/lib/Authentication.java @@ -3157,7 +3157,7 @@ public Boolean getMfaStatus(final IncomingToken token) throws AuthStorageExcepti // Check for identities with MFA information from supported providers for (final us.kbase.auth2.lib.identity.RemoteIdentity identity : identities) { - final Boolean mfaStatus = identity.getDetails().isMfaAuthenticated(); + final Boolean mfaStatus = identity.getDetails().getMfaAuthenticated(); if (mfaStatus != null) { return mfaStatus; } diff --git a/src/main/java/us/kbase/auth2/lib/identity/RemoteIdentityDetails.java b/src/main/java/us/kbase/auth2/lib/identity/RemoteIdentityDetails.java index dbfd637a..a909f4f6 100644 --- a/src/main/java/us/kbase/auth2/lib/identity/RemoteIdentityDetails.java +++ b/src/main/java/us/kbase/auth2/lib/identity/RemoteIdentityDetails.java @@ -80,7 +80,7 @@ public String getEmail() { * @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 isMfaAuthenticated() { + public Boolean getMfaAuthenticated() { return mfaAuthenticated; } diff --git a/src/main/java/us/kbase/auth2/lib/storage/mongo/MongoStorage.java b/src/main/java/us/kbase/auth2/lib/storage/mongo/MongoStorage.java index ee730530..45b320aa 100644 --- a/src/main/java/us/kbase/auth2/lib/storage/mongo/MongoStorage.java +++ b/src/main/java/us/kbase/auth2/lib/storage/mongo/MongoStorage.java @@ -1597,7 +1597,7 @@ private void updateIdentity(final RemoteIdentity remoteID) 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_MFA, rid.isMfaAuthenticated())); + .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. @@ -1731,7 +1731,7 @@ private Document toDocument(final RemoteIdentity id) { .append(Fields.IDENTITIES_USER, rid.getUsername()) .append(Fields.IDENTITIES_NAME, rid.getFullname()) .append(Fields.IDENTITIES_EMAIL, rid.getEmail()) - .append(Fields.IDENTITIES_MFA, rid.isMfaAuthenticated()); + .append(Fields.IDENTITIES_MFA, rid.getMfaAuthenticated()); } @Override From 5255abfa0039ff424fdffcc2f57988b909c83275 Mon Sep 17 00:00:00 2001 From: David Lyon <5115845+dauglyon@users.noreply.github.com> Date: Tue, 15 Jul 2025 19:30:30 -0700 Subject: [PATCH 04/25] fix test failures from MFA implementation Update tests to handle MFA field additions and ORCID OpenID Connect changes: - Fix hash codes in RemoteIdentityTest for new MFA field - Update ORCID provider tests for openid scope instead of /authenticate - Add mfaAuthenticated field to API response expectations - Fix non-ORCID provider test to use null MFA status - Update identity ordering in user endpoint tests --- .../auth2/lib/identity/RemoteIdentityTest.java | 8 ++++---- .../providers/OrcIDIdentityProviderTest.java | 16 ++++++++-------- .../service/api/TestModeIntegrationTest.java | 1 + .../auth2/service/api/TokenEndpointTest.java | 2 +- .../test/auth2/service/api/UserEndpointTest.java | 10 +++++----- .../us/kbase/test/auth2/service/ui/MeTest.java | 10 +++++----- 6 files changed, 24 insertions(+), 23 deletions(-) diff --git a/src/test/java/us/kbase/test/auth2/lib/identity/RemoteIdentityTest.java b/src/test/java/us/kbase/test/auth2/lib/identity/RemoteIdentityTest.java index 7b8c5332..4eb308db 100644 --- a/src/test/java/us/kbase/test/auth2/lib/identity/RemoteIdentityTest.java +++ b/src/test/java/us/kbase/test/auth2/lib/identity/RemoteIdentityTest.java @@ -21,7 +21,7 @@ public void remoteDetailsWithAllFields() throws Exception { assertThat("incorrect fullname", dets.getFullname(), is("full")); assertThat("incorrect email", dets.getEmail(), is("email")); assertThat("incorrect mfa authenticated", dets.getMfaAuthenticated(), is((Boolean) null)); - assertThat("incorrect hashcode", dets.hashCode(), is(-1536596969)); + assertThat("incorrect hashcode", dets.hashCode(), is(-497844993)); assertThat("incorrect toString()", dets.toString(), is("RemoteIdentityDetails [username=user, fullname=full, email=email, mfaAuthenticated=null]")); } @@ -33,7 +33,7 @@ public void remoteDetailsWithEmptyFields() throws Exception { assertThat("incorrect fullname", dets.getFullname(), is((String) null)); assertThat("incorrect email", dets.getEmail(), is((String) null)); assertThat("incorrect mfa authenticated", dets.getMfaAuthenticated(), is((Boolean) null)); - assertThat("incorrect hashcode", dets.hashCode(), is(3629098)); + assertThat("incorrect hashcode", dets.hashCode(), is(4522828)); assertThat("incorrect toString()", dets.toString(), is("RemoteIdentityDetails [username=user, fullname=null, email=null, mfaAuthenticated=null]")); @@ -42,7 +42,7 @@ public void remoteDetailsWithEmptyFields() throws Exception { assertThat("incorrect fullname", dets2.getFullname(), is((String) null)); assertThat("incorrect email", dets2.getEmail(), is((String) null)); assertThat("incorrect mfa authenticated", dets2.getMfaAuthenticated(), is((Boolean) null)); - assertThat("incorrect hashcode", dets2.hashCode(), is(3629098)); + assertThat("incorrect hashcode", dets2.hashCode(), is(4522828)); assertThat("incorrect toString()", dets2.toString(), is("RemoteIdentityDetails [username=user, fullname=null, email=null, mfaAuthenticated=null]")); } @@ -115,7 +115,7 @@ public void identity() throws Exception { final RemoteIdentity ri = new RemoteIdentity(id, dets); assertThat("incorrect id", ri.getRemoteID(), is(id)); assertThat("incorrect details", ri.getDetails(), is(dets)); - assertThat("incorrect hashcode", ri.hashCode(), is(4039350)); + assertThat("incorrect hashcode", ri.hashCode(), is(124952370)); assertThat("incorrect toString()", ri.toString(), is("RemoteIdentity [remoteID=RemoteIdentityID [provider=p, id=i], " + "details=RemoteIdentityDetails [username=u, fullname=f, email=e, mfaAuthenticated=null]]")); diff --git a/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java b/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java index 0525869d..8cbc88c2 100644 --- a/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java +++ b/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java @@ -119,23 +119,23 @@ public void simpleOperationsWithConfigurator() throws Exception { assertThat("incorrect environments", oip.getEnvironments(), is(set("myenv"))); assertThat("incorrect login url", oip.getLoginURI("foo3", "pkce", false, null), is(new URI("https://ologin.com/oauth/authorize?" + - "scope=%2Fauthenticate" + + "scope=openid" + "&state=foo3&redirect_uri=https%3A%2F%2Fologinredir.com" + "&response_type=code&client_id=ofoo"))); assertThat("incorrect link url", oip.getLoginURI("foo4", "pkce", true, null), is(new URI("https://ologin.com/oauth/authorize?" + - "scope=%2Fauthenticate" + + "scope=openid" + "&state=foo4&redirect_uri=https%3A%2F%2Folinkredir.com" + "&response_type=code&client_id=ofoo"))); assertThat("incorrect login url", oip.getLoginURI("foo3", "pkce", false, "myenv"), is(new URI("https://ologin.com/oauth/authorize?" + - "scope=%2Fauthenticate" + + "scope=openid" + "&state=foo3&redirect_uri=https%3A%2F%2Fmyologinred.com" + "&response_type=code&client_id=ofoo"))); assertThat("incorrect link url", oip.getLoginURI("foo4", "pkce", true, "myenv"), is(new URI("https://ologin.com/oauth/authorize?" + - "scope=%2Fauthenticate" + + "scope=openid" + "&state=foo4&redirect_uri=https%3A%2F%2Fmyolinkred.com" + "&response_type=code&client_id=ofoo"))); } @@ -148,23 +148,23 @@ public void simpleOperationsWithoutConfigurator() throws Exception { assertThat("incorrect environments", oip.getEnvironments(), is(set("myenv"))); assertThat("incorrect login url", oip.getLoginURI("foo5", "pkce", false, null), is(new URI("https://ologin.com/oauth/authorize?" + - "scope=%2Fauthenticate" + + "scope=openid" + "&state=foo5&redirect_uri=https%3A%2F%2Fologinredir.com" + "&response_type=code&client_id=ofoo"))); assertThat("incorrect link url", oip.getLoginURI("foo6", "pkce", true, null), is(new URI("https://ologin.com/oauth/authorize?" + - "scope=%2Fauthenticate" + + "scope=openid" + "&state=foo6&redirect_uri=https%3A%2F%2Folinkredir.com" + "&response_type=code&client_id=ofoo"))); assertThat("incorrect login url", oip.getLoginURI("foo3", "pkce", false, "myenv"), is(new URI("https://ologin.com/oauth/authorize?" + - "scope=%2Fauthenticate" + + "scope=openid" + "&state=foo3&redirect_uri=https%3A%2F%2Fmyologinred.com" + "&response_type=code&client_id=ofoo"))); assertThat("incorrect link url", oip.getLoginURI("foo4", "pkce", true, "myenv"), is(new URI("https://ologin.com/oauth/authorize?" + - "scope=%2Fauthenticate" + + "scope=openid" + "&state=foo4&redirect_uri=https%3A%2F%2Fmyolinkred.com" + "&response_type=code&client_id=ofoo"))); diff --git a/src/test/java/us/kbase/test/auth2/service/api/TestModeIntegrationTest.java b/src/test/java/us/kbase/test/auth2/service/api/TestModeIntegrationTest.java index 748a9566..e1febb2a 100644 --- a/src/test/java/us/kbase/test/auth2/service/api/TestModeIntegrationTest.java +++ b/src/test/java/us/kbase/test/auth2/service/api/TestModeIntegrationTest.java @@ -209,6 +209,7 @@ public void createAndGetToken() { expected.put("user", "whee"); expected.put("custom", Collections.emptyMap()); expected.put("cachefor", 300000); + expected.put("mfaAuthenticated", null); assertThat("incorrect return", response, is(expected)); diff --git a/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java b/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java index 4f42fd8d..92eed257 100644 --- a/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java +++ b/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java @@ -730,7 +730,7 @@ public void getTokenWithNonOrcidProvider() throws Exception { // Create user with non-ORCID identity (e.g., Google) final us.kbase.auth2.lib.identity.RemoteIdentity googleId = new us.kbase.auth2.lib.identity.RemoteIdentity( new us.kbase.auth2.lib.identity.RemoteIdentityID("Google", "googleid123"), - new us.kbase.auth2.lib.identity.RemoteIdentityDetails("googleuser", "Google User", "google@example.com", true)); + new us.kbase.auth2.lib.identity.RemoteIdentityDetails("googleuser", "Google User", "google@example.com", null)); manager.storage.createUser(us.kbase.auth2.lib.user.NewUser.getBuilder( new UserName("googleuser"), id, new DisplayName("Google User"), inst(10000), googleId) diff --git a/src/test/java/us/kbase/test/auth2/service/api/UserEndpointTest.java b/src/test/java/us/kbase/test/auth2/service/api/UserEndpointTest.java index 08e79047..0ff18ddd 100644 --- a/src/test/java/us/kbase/test/auth2/service/api/UserEndpointTest.java +++ b/src/test/java/us/kbase/test/auth2/service/api/UserEndpointTest.java @@ -244,14 +244,14 @@ public void getMeMaximalInput() throws Exception { ImmutableMap.of("id", "Admin", "desc", "Administrator"), ImmutableMap.of("id", "DevToken", "desc", "Create developer tokens"))) .with("idents", Arrays.asList( - ImmutableMap.of( - "provider", "prov2", - "provusername", "user2", - "id", "57980b7a3440a4342567e060c3e47666"), ImmutableMap.of( "provider", "prov", "provusername", "user1", - "id", "c20a5e632833ab26d99906fc9cb07d6b"))) + "id", "c20a5e632833ab26d99906fc9cb07d6b"), + ImmutableMap.of( + "provider", "prov2", + "provusername", "user2", + "id", "57980b7a3440a4342567e060c3e47666"))) .with("policyids", Arrays.asList( ImmutableMap.of("id", "wubba", "agreedon", 50000), ImmutableMap.of("id", "wugga", "agreedon", 40000))) diff --git a/src/test/java/us/kbase/test/auth2/service/ui/MeTest.java b/src/test/java/us/kbase/test/auth2/service/ui/MeTest.java index 4cf85a6d..3c00823d 100644 --- a/src/test/java/us/kbase/test/auth2/service/ui/MeTest.java +++ b/src/test/java/us/kbase/test/auth2/service/ui/MeTest.java @@ -200,14 +200,14 @@ public void getMeMaximalInput() throws Exception { ImmutableMap.of("id", "Admin", "desc", "Administrator"), ImmutableMap.of("id", "DevToken", "desc", "Create developer tokens"))) .with("idents", Arrays.asList( - ImmutableMap.of( - "provider", "prov2", - "provusername", "user2", - "id", "57980b7a3440a4342567e060c3e47666"), ImmutableMap.of( "provider", "prov", "provusername", "user1", - "id", "c20a5e632833ab26d99906fc9cb07d6b"))) + "id", "c20a5e632833ab26d99906fc9cb07d6b"), + ImmutableMap.of( + "provider", "prov2", + "provusername", "user2", + "id", "57980b7a3440a4342567e060c3e47666"))) .with("policyids", Arrays.asList( ImmutableMap.of("id", "wubba", "agreedon", 50000), ImmutableMap.of("id", "wugga", "agreedon", 40000))) From 99f0573b374d81354e79090f5bf34743016a136f Mon Sep 17 00:00:00 2001 From: David Lyon <5115845+dauglyon@users.noreply.github.com> Date: Tue, 15 Jul 2025 19:49:51 -0700 Subject: [PATCH 05/25] reorder expected html for test --- .../auth2/service/ui/MeTest_getMeMaximalInput.testdata | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/test/resources/us/kbase/test/auth2/service/ui/MeTest_getMeMaximalInput.testdata b/src/test/resources/us/kbase/test/auth2/service/ui/MeTest_getMeMaximalInput.testdata index c218326a..00acc4ab 100644 --- a/src/test/resources/us/kbase/test/auth2/service/ui/MeTest_getMeMaximalInput.testdata +++ b/src/test/resources/us/kbase/test/auth2/service/ui/MeTest_getMeMaximalInput.testdata @@ -28,16 +28,16 @@ Email is only visible to you, software acting on your behalf, and system adminis

Identities:

-Provider: prov2
-User id: user2
-
- -
Provider: prov
User id: user1
+Provider: prov2
+User id: user2
+
+ +
\ No newline at end of file From 748c8a2f80f5e6638f85f55f823620be1ad9259d Mon Sep 17 00:00:00 2001 From: David Lyon <5115845+dauglyon@users.noreply.github.com> Date: Tue, 15 Jul 2025 20:23:44 -0700 Subject: [PATCH 06/25] Improve test coverage; improve error handling --- .../us/kbase/auth2/lib/Authentication.java | 29 +++++----- .../OrcIDIdentityProviderFactory.java | 3 -- .../auth2/lib/AuthenticationTokenTest.java | 7 +++ .../providers/OrcIDIdentityProviderTest.java | 54 +++++++++++++++++++ 4 files changed, 74 insertions(+), 19 deletions(-) diff --git a/src/main/java/us/kbase/auth2/lib/Authentication.java b/src/main/java/us/kbase/auth2/lib/Authentication.java index d1a5d572..da1fa380 100644 --- a/src/main/java/us/kbase/auth2/lib/Authentication.java +++ b/src/main/java/us/kbase/auth2/lib/Authentication.java @@ -3146,28 +3146,25 @@ public long getSuggestedTokenCacheTime() throws AuthStorageException { * @param token the token to check. * @return true if MFA was used, false if password only, null if unknown or not supported. * @throws AuthStorageException if an error occurred accessing the storage system. + * @throws InvalidTokenException if the token is invalid. */ - public Boolean getMfaStatus(final IncomingToken token) throws AuthStorageException { + public Boolean getMfaStatus(final IncomingToken token) + throws AuthStorageException, InvalidTokenException { if (token == null) { return null; } - try { - final AuthUser user = getUser(token); - final Set identities = user.getIdentities(); - - // Check for identities with MFA information from supported providers - for (final us.kbase.auth2.lib.identity.RemoteIdentity identity : identities) { - final Boolean mfaStatus = identity.getDetails().getMfaAuthenticated(); - if (mfaStatus != null) { - return mfaStatus; - } + final AuthUser user = getUser(token); + final Set identities = user.getIdentities(); + + // Check for identities with MFA information from supported providers + for (final us.kbase.auth2.lib.identity.RemoteIdentity identity : identities) { + final Boolean mfaStatus = identity.getDetails().getMfaAuthenticated(); + if (mfaStatus != null) { + return mfaStatus; } - - return null; // No MFA information available - } catch (Exception e) { - // For any errors, return null rather than failing the request - return null; } + + return null; // No MFA information available } /** Get the external configuration without providing any credentials. diff --git a/src/main/java/us/kbase/auth2/providers/OrcIDIdentityProviderFactory.java b/src/main/java/us/kbase/auth2/providers/OrcIDIdentityProviderFactory.java index ddc1808e..28d47bf6 100644 --- a/src/main/java/us/kbase/auth2/providers/OrcIDIdentityProviderFactory.java +++ b/src/main/java/us/kbase/auth2/providers/OrcIDIdentityProviderFactory.java @@ -284,9 +284,6 @@ private Boolean parseAmrClaim(final String idToken) { } catch (IOException e) { // JSON parsing failed - malformed payload return null; - } catch (Exception e) { - // Other unexpected errors - don't fail authentication - return null; } } } diff --git a/src/test/java/us/kbase/test/auth2/lib/AuthenticationTokenTest.java b/src/test/java/us/kbase/test/auth2/lib/AuthenticationTokenTest.java index faf6a615..d3fa15bb 100644 --- a/src/test/java/us/kbase/test/auth2/lib/AuthenticationTokenTest.java +++ b/src/test/java/us/kbase/test/auth2/lib/AuthenticationTokenTest.java @@ -1187,4 +1187,11 @@ public void deleteLoginOrLinkStateNull() throws Exception { } } + @Test + public void getMfaStatusNull() throws Exception { + final Authentication auth = initTestMocks().auth; + + assertThat("incorrect mfa status", auth.getMfaStatus(null), is((Boolean) null)); + } + } diff --git a/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java b/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java index 8cbc88c2..a9effb57 100644 --- a/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java +++ b/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java @@ -875,4 +875,58 @@ private String createJWTWithoutAmr(final String orcID) { .encodeToString(payload.getBytes()); return encodedHeader + "." + encodedPayload + ".signature"; } + + @Test + public void getIdentityWithInvalidJWTFormat() throws Exception { + final String authCode = "authcodeInvalidFormat"; + final String orcID = "0000-0001-8607-8067"; + + // JWT with only 2 parts instead of 3 + final String invalidJWT = "header.payload"; + + ms.expect(RequestMethod.POST, getTokenURL()) + .andRespond(withSuccess( + "{\"access_token\": \"token\", \"orcid\": \"" + orcID + "\", " + + "\"name\": \"My name\", \"id_token\": \"" + invalidJWT + "\"}", + MediaType.APPLICATION_JSON)); + + ms.expect(RequestMethod.GET, getEmailURL(orcID)) + .andRespond(withSuccess(toJSON( + map("email", Arrays.asList(map("email", "invalid@format.com")))), + MediaType.APPLICATION_JSON)); + + final Set ri = new OrcIDIdentityProviderFactory() + .configure(CFG).getIdentities(authCode, "pkceCodeVerifier", false, null); + + assertThat("incorrect return", ri, is(set(new RemoteIdentity( + new RemoteIdentityID("OrcID", orcID), + new RemoteIdentityDetails(orcID, "My name", "invalid@format.com", null))))); + } + + @Test + public void getIdentityWithBase64DecodingError() throws Exception { + final String authCode = "authcodeBase64Error"; + final String orcID = "0000-0001-8607-8067"; + + // JWT with invalid base64 in payload + final String invalidJWT = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.invalid_base64!@#$.signature"; + + ms.expect(RequestMethod.POST, getTokenURL()) + .andRespond(withSuccess( + "{\"access_token\": \"token\", \"orcid\": \"" + orcID + "\", " + + "\"name\": \"My name\", \"id_token\": \"" + invalidJWT + "\"}", + MediaType.APPLICATION_JSON)); + + ms.expect(RequestMethod.GET, getEmailURL(orcID)) + .andRespond(withSuccess(toJSON( + map("email", Arrays.asList(map("email", "base64error@test.com")))), + MediaType.APPLICATION_JSON)); + + final Set ri = new OrcIDIdentityProviderFactory() + .configure(CFG).getIdentities(authCode, "pkceCodeVerifier", false, null); + + assertThat("incorrect return", ri, is(set(new RemoteIdentity( + new RemoteIdentityID("OrcID", orcID), + new RemoteIdentityDetails(orcID, "My name", "base64error@test.com", null))))); + } } From 589d52a5efedca3f9a86cd115f76a30349b62a18 Mon Sep 17 00:00:00 2001 From: David Lyon <5115845+dauglyon@users.noreply.github.com> Date: Tue, 15 Jul 2025 20:39:43 -0700 Subject: [PATCH 07/25] fix exception errors --- .../us/kbase/auth2/lib/Authentication.java | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/main/java/us/kbase/auth2/lib/Authentication.java b/src/main/java/us/kbase/auth2/lib/Authentication.java index da1fa380..7ae1a3cc 100644 --- a/src/main/java/us/kbase/auth2/lib/Authentication.java +++ b/src/main/java/us/kbase/auth2/lib/Authentication.java @@ -3153,18 +3153,23 @@ public Boolean getMfaStatus(final IncomingToken token) if (token == null) { return null; } - final AuthUser user = getUser(token); - final Set identities = user.getIdentities(); - - // Check for identities with MFA information from supported providers - for (final us.kbase.auth2.lib.identity.RemoteIdentity identity : identities) { - final Boolean mfaStatus = identity.getDetails().getMfaAuthenticated(); - if (mfaStatus != null) { - return mfaStatus; + try { + final AuthUser user = getUser(token); + final Set identities = user.getIdentities(); + + // Check for identities with MFA information from supported providers + for (final us.kbase.auth2.lib.identity.RemoteIdentity identity : identities) { + final Boolean mfaStatus = identity.getDetails().getMfaAuthenticated(); + if (mfaStatus != null) { + return mfaStatus; + } } + + return null; // No MFA information available + } catch (DisabledUserException e) { + // Return null for disabled users + return null; } - - return null; // No MFA information available } /** Get the external configuration without providing any credentials. From 099ca8d9f880c212bfb124b728e7b325f22f7623 Mon Sep 17 00:00:00 2001 From: David Lyon <5115845+dauglyon@users.noreply.github.com> Date: Tue, 15 Jul 2025 20:51:40 -0700 Subject: [PATCH 08/25] Update ORCID provider tests to use MockServer --- .../providers/OrcIDIdentityProviderTest.java | 82 +++++++++++++++---- 1 file changed, 64 insertions(+), 18 deletions(-) diff --git a/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java b/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java index a9effb57..38cb657b 100644 --- a/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java +++ b/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java @@ -880,20 +880,43 @@ private String createJWTWithoutAmr(final String orcID) { public void getIdentityWithInvalidJWTFormat() throws Exception { final String authCode = "authcodeInvalidFormat"; final String orcID = "0000-0001-8607-8067"; + final String token = "atok"; // JWT with only 2 parts instead of 3 final String invalidJWT = "header.payload"; - ms.expect(RequestMethod.POST, getTokenURL()) - .andRespond(withSuccess( - "{\"access_token\": \"token\", \"orcid\": \"" + orcID + "\", " + - "\"name\": \"My name\", \"id_token\": \"" + invalidJWT + "\"}", - MediaType.APPLICATION_JSON)); + mockClientAndServer.when( + new HttpRequest() + .withMethod("POST") + .withPath("/oauth/token") + .withHeader(ACCEPT, APP_JSON) + .withBody(new ParameterBody( + new Parameter("code", authCode), + new Parameter("redirect_uri", "https://ologinredir.com"), + new Parameter("grant_type", "authorization_code"), + new Parameter("client_id", "ofoo"), + new Parameter("client_secret", "obar")))) + .respond( + new HttpResponse() + .withStatusCode(200) + .withHeader(new Header(CONTENT_TYPE, APP_JSON)) + .withBody("{\"access_token\": \"" + token + "\", \"orcid\": \"" + orcID + "\", " + + "\"name\": \"My name\", \"id_token\": \"" + invalidJWT + "\"}")); - ms.expect(RequestMethod.GET, getEmailURL(orcID)) - .andRespond(withSuccess(toJSON( - map("email", Arrays.asList(map("email", "invalid@format.com")))), - MediaType.APPLICATION_JSON)); + final Map email = new HashMap<>(); + email.put("email", Arrays.asList(TestCommon.map("email", "invalid@format.com"))); + + mockClientAndServer.when( + new HttpRequest() + .withMethod("GET") + .withPath("/v2.1/" + orcID + "/email") + .withHeader(ACCEPT, APP_JSON) + .withHeader("Authorization", "Bearer " + token)) + .respond( + new HttpResponse() + .withStatusCode(200) + .withHeader(new Header(CONTENT_TYPE, APP_JSON)) + .withBody(new ObjectMapper().writeValueAsString(email))); final Set ri = new OrcIDIdentityProviderFactory() .configure(CFG).getIdentities(authCode, "pkceCodeVerifier", false, null); @@ -907,20 +930,43 @@ public void getIdentityWithInvalidJWTFormat() throws Exception { public void getIdentityWithBase64DecodingError() throws Exception { final String authCode = "authcodeBase64Error"; final String orcID = "0000-0001-8607-8067"; + final String token = "atok2"; // JWT with invalid base64 in payload final String invalidJWT = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.invalid_base64!@#$.signature"; - ms.expect(RequestMethod.POST, getTokenURL()) - .andRespond(withSuccess( - "{\"access_token\": \"token\", \"orcid\": \"" + orcID + "\", " + - "\"name\": \"My name\", \"id_token\": \"" + invalidJWT + "\"}", - MediaType.APPLICATION_JSON)); + mockClientAndServer.when( + new HttpRequest() + .withMethod("POST") + .withPath("/oauth/token") + .withHeader(ACCEPT, APP_JSON) + .withBody(new ParameterBody( + new Parameter("code", authCode), + new Parameter("redirect_uri", "https://ologinredir.com"), + new Parameter("grant_type", "authorization_code"), + new Parameter("client_id", "ofoo"), + new Parameter("client_secret", "obar")))) + .respond( + new HttpResponse() + .withStatusCode(200) + .withHeader(new Header(CONTENT_TYPE, APP_JSON)) + .withBody("{\"access_token\": \"" + token + "\", \"orcid\": \"" + orcID + "\", " + + "\"name\": \"My name\", \"id_token\": \"" + invalidJWT + "\"}")); - ms.expect(RequestMethod.GET, getEmailURL(orcID)) - .andRespond(withSuccess(toJSON( - map("email", Arrays.asList(map("email", "base64error@test.com")))), - MediaType.APPLICATION_JSON)); + final Map email = new HashMap<>(); + email.put("email", Arrays.asList(TestCommon.map("email", "base64error@test.com"))); + + mockClientAndServer.when( + new HttpRequest() + .withMethod("GET") + .withPath("/v2.1/" + orcID + "/email") + .withHeader(ACCEPT, APP_JSON) + .withHeader("Authorization", "Bearer " + token)) + .respond( + new HttpResponse() + .withStatusCode(200) + .withHeader(new Header(CONTENT_TYPE, APP_JSON)) + .withBody(new ObjectMapper().writeValueAsString(email))); final Set ri = new OrcIDIdentityProviderFactory() .configure(CFG).getIdentities(authCode, "pkceCodeVerifier", false, null); From 4cbf2ecd7a4e01578d60f6c0a82791fcf38ab562 Mon Sep 17 00:00:00 2001 From: David Lyon <5115845+dauglyon@users.noreply.github.com> Date: Tue, 15 Jul 2025 21:04:04 -0700 Subject: [PATCH 09/25] remove duplicate tests --- .../providers/OrcIDIdentityProviderTest.java | 99 ------------------- 1 file changed, 99 deletions(-) diff --git a/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java b/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java index 38cb657b..c1b1363b 100644 --- a/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java +++ b/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java @@ -876,103 +876,4 @@ private String createJWTWithoutAmr(final String orcID) { return encodedHeader + "." + encodedPayload + ".signature"; } - @Test - public void getIdentityWithInvalidJWTFormat() throws Exception { - final String authCode = "authcodeInvalidFormat"; - final String orcID = "0000-0001-8607-8067"; - final String token = "atok"; - - // JWT with only 2 parts instead of 3 - final String invalidJWT = "header.payload"; - - mockClientAndServer.when( - new HttpRequest() - .withMethod("POST") - .withPath("/oauth/token") - .withHeader(ACCEPT, APP_JSON) - .withBody(new ParameterBody( - new Parameter("code", authCode), - new Parameter("redirect_uri", "https://ologinredir.com"), - new Parameter("grant_type", "authorization_code"), - new Parameter("client_id", "ofoo"), - new Parameter("client_secret", "obar")))) - .respond( - new HttpResponse() - .withStatusCode(200) - .withHeader(new Header(CONTENT_TYPE, APP_JSON)) - .withBody("{\"access_token\": \"" + token + "\", \"orcid\": \"" + orcID + "\", " + - "\"name\": \"My name\", \"id_token\": \"" + invalidJWT + "\"}")); - - final Map email = new HashMap<>(); - email.put("email", Arrays.asList(TestCommon.map("email", "invalid@format.com"))); - - mockClientAndServer.when( - new HttpRequest() - .withMethod("GET") - .withPath("/v2.1/" + orcID + "/email") - .withHeader(ACCEPT, APP_JSON) - .withHeader("Authorization", "Bearer " + token)) - .respond( - new HttpResponse() - .withStatusCode(200) - .withHeader(new Header(CONTENT_TYPE, APP_JSON)) - .withBody(new ObjectMapper().writeValueAsString(email))); - - final Set ri = new OrcIDIdentityProviderFactory() - .configure(CFG).getIdentities(authCode, "pkceCodeVerifier", false, null); - - assertThat("incorrect return", ri, is(set(new RemoteIdentity( - new RemoteIdentityID("OrcID", orcID), - new RemoteIdentityDetails(orcID, "My name", "invalid@format.com", null))))); - } - - @Test - public void getIdentityWithBase64DecodingError() throws Exception { - final String authCode = "authcodeBase64Error"; - final String orcID = "0000-0001-8607-8067"; - final String token = "atok2"; - - // JWT with invalid base64 in payload - final String invalidJWT = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.invalid_base64!@#$.signature"; - - mockClientAndServer.when( - new HttpRequest() - .withMethod("POST") - .withPath("/oauth/token") - .withHeader(ACCEPT, APP_JSON) - .withBody(new ParameterBody( - new Parameter("code", authCode), - new Parameter("redirect_uri", "https://ologinredir.com"), - new Parameter("grant_type", "authorization_code"), - new Parameter("client_id", "ofoo"), - new Parameter("client_secret", "obar")))) - .respond( - new HttpResponse() - .withStatusCode(200) - .withHeader(new Header(CONTENT_TYPE, APP_JSON)) - .withBody("{\"access_token\": \"" + token + "\", \"orcid\": \"" + orcID + "\", " + - "\"name\": \"My name\", \"id_token\": \"" + invalidJWT + "\"}")); - - final Map email = new HashMap<>(); - email.put("email", Arrays.asList(TestCommon.map("email", "base64error@test.com"))); - - mockClientAndServer.when( - new HttpRequest() - .withMethod("GET") - .withPath("/v2.1/" + orcID + "/email") - .withHeader(ACCEPT, APP_JSON) - .withHeader("Authorization", "Bearer " + token)) - .respond( - new HttpResponse() - .withStatusCode(200) - .withHeader(new Header(CONTENT_TYPE, APP_JSON)) - .withBody(new ObjectMapper().writeValueAsString(email))); - - final Set ri = new OrcIDIdentityProviderFactory() - .configure(CFG).getIdentities(authCode, "pkceCodeVerifier", false, null); - - assertThat("incorrect return", ri, is(set(new RemoteIdentity( - new RemoteIdentityID("OrcID", orcID), - new RemoteIdentityDetails(orcID, "My name", "base64error@test.com", null))))); - } } From ef094fdf7207fcf6c40b915ad9545cc00f93873b Mon Sep 17 00:00:00 2001 From: David Lyon <5115845+dauglyon@users.noreply.github.com> Date: Thu, 17 Jul 2025 09:37:39 -0700 Subject: [PATCH 10/25] fix TokenEndpointTest.getToken --- src/main/java/us/kbase/auth2/lib/Authentication.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/main/java/us/kbase/auth2/lib/Authentication.java b/src/main/java/us/kbase/auth2/lib/Authentication.java index 7ae1a3cc..6020fe14 100644 --- a/src/main/java/us/kbase/auth2/lib/Authentication.java +++ b/src/main/java/us/kbase/auth2/lib/Authentication.java @@ -3158,10 +3158,14 @@ public Boolean getMfaStatus(final IncomingToken token) final Set identities = user.getIdentities(); // Check for identities with MFA information from supported providers - for (final us.kbase.auth2.lib.identity.RemoteIdentity identity : identities) { - final Boolean mfaStatus = identity.getDetails().getMfaAuthenticated(); - if (mfaStatus != null) { - return mfaStatus; + if (identities != null) { + for (final us.kbase.auth2.lib.identity.RemoteIdentity identity : identities) { + if (identity != null && identity.getDetails() != null) { + final Boolean mfaStatus = identity.getDetails().getMfaAuthenticated(); + if (mfaStatus != null) { + return mfaStatus; + } + } } } From ac606a5b394514e3459e7ae3121c8199685b729a Mon Sep 17 00:00:00 2001 From: David Lyon <5115845+dauglyon@users.noreply.github.com> Date: Thu, 17 Jul 2025 10:05:00 -0700 Subject: [PATCH 11/25] fix TokenEndpointTest.getToken --- .../java/us/kbase/auth2/lib/Authentication.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/main/java/us/kbase/auth2/lib/Authentication.java b/src/main/java/us/kbase/auth2/lib/Authentication.java index 6020fe14..65b2b0b9 100644 --- a/src/main/java/us/kbase/auth2/lib/Authentication.java +++ b/src/main/java/us/kbase/auth2/lib/Authentication.java @@ -3153,6 +3153,19 @@ public Boolean getMfaStatus(final IncomingToken token) if (token == null) { return null; } + + // Get the stored token to find the username + final StoredToken storedToken = getToken(token); + final UserName userName = storedToken.getUserName(); + + // Check if the user exists before trying to get user details + try { + storage.getUser(userName); + } catch (NoSuchUserException e) { + // User doesn't exist, return null for MFA status + return null; + } + try { final AuthUser user = getUser(token); final Set identities = user.getIdentities(); From 9dfc0a768f2ba2eafa9440cf0a0f207ba2f7b8df Mon Sep 17 00:00:00 2001 From: David Lyon <5115845+dauglyon@users.noreply.github.com> Date: Thu, 17 Jul 2025 12:31:11 -0700 Subject: [PATCH 12/25] Store MFA authentication status directly on tokens Move MFA status from being computed from user identities to being stored as a boolean field on tokens themselves. This provides per-token MFA tracking and eliminates the need for complex identity lookups. --- .../us/kbase/auth2/lib/Authentication.java | 73 +++++-------------- .../kbase/auth2/lib/storage/mongo/Fields.java | 2 + .../auth2/lib/storage/mongo/MongoStorage.java | 4 +- .../us/kbase/auth2/lib/token/StoredToken.java | 35 ++++++++- .../us/kbase/auth2/service/api/APIToken.java | 23 +----- .../us/kbase/auth2/service/api/Token.java | 2 +- .../auth2/service/api/TokenEndpointTest.java | 54 ++++++++++++++ 7 files changed, 113 insertions(+), 80 deletions(-) diff --git a/src/main/java/us/kbase/auth2/lib/Authentication.java b/src/main/java/us/kbase/auth2/lib/Authentication.java index 65b2b0b9..e72090e0 100644 --- a/src/main/java/us/kbase/auth2/lib/Authentication.java +++ b/src/main/java/us/kbase/auth2/lib/Authentication.java @@ -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 {}", @@ -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 + .build(), randGen.getToken()); storage.storeToken(nt.getStoredToken(), nt.getTokenHash()); logInfo("User {} created {} token {}", au.getUserName().getName(), tokenType, id); @@ -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()); @@ -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 getIdentity( @@ -3142,52 +3153,6 @@ public long getSuggestedTokenCacheTime() throws AuthStorageException { return cfg.getAppConfig().getTokenLifetimeMS(TokenLifetimeType.EXT_CACHE); } - /** Get MFA status for a token by checking the token user's identities. - * @param token the token to check. - * @return true if MFA was used, false if password only, null if unknown or not supported. - * @throws AuthStorageException if an error occurred accessing the storage system. - * @throws InvalidTokenException if the token is invalid. - */ - public Boolean getMfaStatus(final IncomingToken token) - throws AuthStorageException, InvalidTokenException { - if (token == null) { - return null; - } - - // Get the stored token to find the username - final StoredToken storedToken = getToken(token); - final UserName userName = storedToken.getUserName(); - - // Check if the user exists before trying to get user details - try { - storage.getUser(userName); - } catch (NoSuchUserException e) { - // User doesn't exist, return null for MFA status - return null; - } - - try { - final AuthUser user = getUser(token); - final Set identities = user.getIdentities(); - - // Check for identities with MFA information from supported providers - if (identities != null) { - for (final us.kbase.auth2.lib.identity.RemoteIdentity identity : identities) { - if (identity != null && identity.getDetails() != null) { - final Boolean mfaStatus = identity.getDetails().getMfaAuthenticated(); - if (mfaStatus != null) { - return mfaStatus; - } - } - } - } - - return null; // No MFA information available - } catch (DisabledUserException e) { - // Return null for disabled users - return null; - } - } /** Get the external configuration without providing any credentials. * diff --git a/src/main/java/us/kbase/auth2/lib/storage/mongo/Fields.java b/src/main/java/us/kbase/auth2/lib/storage/mongo/Fields.java index 48fcc8ce..74ff8607 100644 --- a/src/main/java/us/kbase/auth2/lib/storage/mongo/Fields.java +++ b/src/main/java/us/kbase/auth2/lib/storage/mongo/Fields.java @@ -137,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 diff --git a/src/main/java/us/kbase/auth2/lib/storage/mongo/MongoStorage.java b/src/main/java/us/kbase/auth2/lib/storage/mongo/MongoStorage.java index 45b320aa..1d8e53b0 100644 --- a/src/main/java/us/kbase/auth2/lib/storage/mongo/MongoStorage.java +++ b/src/main/java/us/kbase/auth2/lib/storage/mongo/MongoStorage.java @@ -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) { @@ -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(); } diff --git a/src/main/java/us/kbase/auth2/lib/token/StoredToken.java b/src/main/java/us/kbase/auth2/lib/token/StoredToken.java index cdedf822..4793b655 100644 --- a/src/main/java/us/kbase/auth2/lib/token/StoredToken.java +++ b/src/main/java/us/kbase/auth2/lib/token/StoredToken.java @@ -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, @@ -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 @@ -43,6 +45,7 @@ private StoredToken( this.expirationDate = expirationDate; this.creationDate = creationDate; this.id = id; + this.mfaAuthenticated = mfaAuthenticated; } /** Get the type of the token. @@ -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() { @@ -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; } @@ -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; } @@ -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. */ @@ -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"); @@ -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 diff --git a/src/main/java/us/kbase/auth2/service/api/APIToken.java b/src/main/java/us/kbase/auth2/service/api/APIToken.java index 394c8236..67a4d932 100644 --- a/src/main/java/us/kbase/auth2/service/api/APIToken.java +++ b/src/main/java/us/kbase/auth2/service/api/APIToken.java @@ -10,31 +10,10 @@ public class APIToken extends ExternalToken { private final long cachefor; private final Boolean mfaAuthenticated; - /** - * Constructor without MFA status calculation. - * MFA status will be set to null. - * - * @param token the stored token - * @param tokenCacheTimeMillis the token cache time in milliseconds - */ public APIToken(final StoredToken token, final long tokenCacheTimeMillis) { super(token); cachefor = tokenCacheTimeMillis; - mfaAuthenticated = null; - } - - /** - * Constructor with MFA status provided. - * - * @param token the stored token - * @param tokenCacheTimeMillis the token cache time in milliseconds - * @param mfaAuthenticated the MFA authentication status - */ - public APIToken(final StoredToken token, final long tokenCacheTimeMillis, - final Boolean mfaAuthenticated) { - super(token); - cachefor = tokenCacheTimeMillis; - this.mfaAuthenticated = mfaAuthenticated; + mfaAuthenticated = token.getMfaAuthenticated(); } public long getCachefor() { diff --git a/src/main/java/us/kbase/auth2/service/api/Token.java b/src/main/java/us/kbase/auth2/service/api/Token.java index d4425434..c9ac0e6d 100644 --- a/src/main/java/us/kbase/auth2/service/api/Token.java +++ b/src/main/java/us/kbase/auth2/service/api/Token.java @@ -58,7 +58,7 @@ public APIToken viewToken(@HeaderParam(APIConstants.HEADER_TOKEN) final String t throws NoTokenProvidedException, InvalidTokenException, AuthStorageException { final IncomingToken it = getToken(token); final StoredToken ht = auth.getToken(it); - return new APIToken(ht, auth.getSuggestedTokenCacheTime(), auth.getMfaStatus(it)); + return new APIToken(ht, auth.getSuggestedTokenCacheTime()); } private static class CreateToken extends IncomingJSON { diff --git a/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java b/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java index 92eed257..b5ddeb7a 100644 --- a/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java +++ b/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java @@ -180,6 +180,60 @@ TokenType.AGENT, id, new UserName("foo")) assertThat("incorrect token", response, is(expected)); } + @Test + public void getTokenMfaIsolation() throws Exception { + // Create user + final UserName userName = new UserName("testuser"); + final UUID userUuid = UUID.randomUUID(); + manager.storage.createLocalUser(LocalUser.getLocalUserBuilder( + userName, userUuid, new DisplayName("Test User"), inst(10000)) + .withEmailAddress(new EmailAddress("test@example.com")).build(), + new PasswordHashAndSalt("password".getBytes(), "salt".getBytes())); + + // Create two tokens with different MFA status + final UUID token1Id = UUID.randomUUID(); + final UUID token2Id = UUID.randomUUID(); + + final IncomingToken token1 = new IncomingToken("token1hash"); + final IncomingToken token2 = new IncomingToken("token2hash"); + + // Token 1 with MFA=true + manager.storage.storeToken(StoredToken.getBuilder( + TokenType.LOGIN, token1Id, userName) + .withLifeTime(Instant.ofEpochMilli(10000), Instant.ofEpochMilli(1000000000000000L)) + .withMfaAuthenticated(true) + .build(), token1.getHashedToken().getTokenHash()); + + // Token 2 with MFA=false + manager.storage.storeToken(StoredToken.getBuilder( + TokenType.LOGIN, token2Id, userName) + .withLifeTime(Instant.ofEpochMilli(10000), Instant.ofEpochMilli(1000000000000000L)) + .withMfaAuthenticated(false) + .build(), token2.getHashedToken().getTokenHash()); + + // Test token1 returns MFA=true + final URI target1 = UriBuilder.fromUri(host).path("/api/V2/token").build(); + final WebTarget wt1 = CLI.target(target1); + final Builder req1 = wt1.request().header("authorization", token1.getToken()); + final Response res1 = req1.get(); + + assertThat("incorrect response code for token1", res1.getStatus(), is(200)); + @SuppressWarnings("unchecked") + final Map response1 = res1.readEntity(Map.class); + assertThat("token1 should have MFA=true", response1.get("mfaAuthenticated"), is(true)); + + // Test token2 returns MFA=false + final URI target2 = UriBuilder.fromUri(host).path("/api/V2/token").build(); + final WebTarget wt2 = CLI.target(target2); + final Builder req2 = wt2.request().header("authorization", token2.getToken()); + final Response res2 = req2.get(); + + assertThat("incorrect response code for token2", res2.getStatus(), is(200)); + @SuppressWarnings("unchecked") + final Map response2 = res2.readEntity(Map.class); + assertThat("token2 should have MFA=false", response2.get("mfaAuthenticated"), is(false)); + } + @Test public void getTokenFailNoToken() throws Exception { final URI target = UriBuilder.fromUri(host).path("/api/V2/token").build(); From 53ed1762eef42b4db102ace423eb3b5bc16eb12c Mon Sep 17 00:00:00 2001 From: David Lyon <5115845+dauglyon@users.noreply.github.com> Date: Thu, 17 Jul 2025 14:15:40 -0700 Subject: [PATCH 13/25] update tests --- .../auth2/lib/AuthenticationTokenTest.java | 6 -- .../auth2/service/api/TokenEndpointTest.java | 83 +++++++++---------- 2 files changed, 41 insertions(+), 48 deletions(-) diff --git a/src/test/java/us/kbase/test/auth2/lib/AuthenticationTokenTest.java b/src/test/java/us/kbase/test/auth2/lib/AuthenticationTokenTest.java index d3fa15bb..1d93fbae 100644 --- a/src/test/java/us/kbase/test/auth2/lib/AuthenticationTokenTest.java +++ b/src/test/java/us/kbase/test/auth2/lib/AuthenticationTokenTest.java @@ -1187,11 +1187,5 @@ public void deleteLoginOrLinkStateNull() throws Exception { } } - @Test - public void getMfaStatusNull() throws Exception { - final Authentication auth = initTestMocks().auth; - - assertThat("incorrect mfa status", auth.getMfaStatus(null), is((Boolean) null)); - } } diff --git a/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java b/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java index b5ddeb7a..a08a006c 100644 --- a/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java +++ b/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java @@ -666,20 +666,20 @@ private void kbaseTokenSuccess( public void getTokenWithMfaTrue() throws Exception { final UUID id = UUID.randomUUID(); final IncomingToken it = new IncomingToken("mfatokenvalue"); + final UserName userName = new UserName("mfauser"); - // Create user with ORCID identity that used MFA - final us.kbase.auth2.lib.identity.RemoteIdentity orcidId = new us.kbase.auth2.lib.identity.RemoteIdentity( - new us.kbase.auth2.lib.identity.RemoteIdentityID("OrcID", "0000-0001-1234-5678"), - new us.kbase.auth2.lib.identity.RemoteIdentityDetails("orciduser", "ORCID User", "orcid@example.com", true)); - - manager.storage.createUser(us.kbase.auth2.lib.user.NewUser.getBuilder( - new UserName("mfauser"), id, new DisplayName("MFA User"), inst(10000), orcidId) - .withEmailAddress(new EmailAddress("mfa@example.com")).build()); + // Create simple local user + manager.storage.createLocalUser(LocalUser.getLocalUserBuilder( + userName, id, new DisplayName("MFA User"), inst(10000)) + .withEmailAddress(new EmailAddress("mfa@example.com")).build(), + new PasswordHashAndSalt("password".getBytes(), "salt".getBytes())); + // Create token with MFA=true manager.storage.storeToken(StoredToken.getBuilder( - TokenType.AGENT, id, new UserName("mfauser")) + TokenType.AGENT, id, userName) .withLifeTime(Instant.ofEpochMilli(10000), Instant.ofEpochMilli(1000000000000000L)) .withTokenName(new TokenName("mfatoken")) + .withMfaAuthenticated(true) .build(), it.getHashedToken().getTokenHash()); final URI target = UriBuilder.fromUri(host).path("/api/V2/token").build(); @@ -704,20 +704,20 @@ TokenType.AGENT, id, new UserName("mfauser")) public void getTokenWithMfaFalse() throws Exception { final UUID id = UUID.randomUUID(); final IncomingToken it = new IncomingToken("nomfatokenvalue"); + final UserName userName = new UserName("nomfauser"); - // Create user with ORCID identity that did NOT use MFA - final us.kbase.auth2.lib.identity.RemoteIdentity orcidId = new us.kbase.auth2.lib.identity.RemoteIdentity( - new us.kbase.auth2.lib.identity.RemoteIdentityID("OrcID", "0000-0001-1234-9999"), - new us.kbase.auth2.lib.identity.RemoteIdentityDetails("orciduser2", "ORCID User 2", "orcid2@example.com", false)); - - manager.storage.createUser(us.kbase.auth2.lib.user.NewUser.getBuilder( - new UserName("nomfauser"), id, new DisplayName("No MFA User"), inst(10000), orcidId) - .withEmailAddress(new EmailAddress("nomfa@example.com")).build()); + // Create simple local user + manager.storage.createLocalUser(LocalUser.getLocalUserBuilder( + userName, id, new DisplayName("No MFA User"), inst(10000)) + .withEmailAddress(new EmailAddress("nomfa@example.com")).build(), + new PasswordHashAndSalt("password".getBytes(), "salt".getBytes())); + // Create token with MFA=false manager.storage.storeToken(StoredToken.getBuilder( - TokenType.AGENT, id, new UserName("nomfauser")) + TokenType.AGENT, id, userName) .withLifeTime(Instant.ofEpochMilli(10000), Instant.ofEpochMilli(1000000000000000L)) .withTokenName(new TokenName("nomfatoken")) + .withMfaAuthenticated(false) .build(), it.getHashedToken().getTokenHash()); final URI target = UriBuilder.fromUri(host).path("/api/V2/token").build(); @@ -742,20 +742,20 @@ TokenType.AGENT, id, new UserName("nomfauser")) public void getTokenWithMfaNull() throws Exception { final UUID id = UUID.randomUUID(); final IncomingToken it = new IncomingToken("unknownmfatokenvalue"); + final UserName userName = new UserName("unknownmfauser"); - // Create user with ORCID identity that has unknown MFA status - final us.kbase.auth2.lib.identity.RemoteIdentity orcidId = new us.kbase.auth2.lib.identity.RemoteIdentity( - new us.kbase.auth2.lib.identity.RemoteIdentityID("OrcID", "0000-0001-1234-0000"), - new us.kbase.auth2.lib.identity.RemoteIdentityDetails("orciduser3", "ORCID User 3", "orcid3@example.com", null)); - - manager.storage.createUser(us.kbase.auth2.lib.user.NewUser.getBuilder( - new UserName("unknownmfauser"), id, new DisplayName("Unknown MFA User"), inst(10000), orcidId) - .withEmailAddress(new EmailAddress("unknownmfa@example.com")).build()); + // Create simple local user + manager.storage.createLocalUser(LocalUser.getLocalUserBuilder( + userName, id, new DisplayName("Unknown MFA User"), inst(10000)) + .withEmailAddress(new EmailAddress("unknownmfa@example.com")).build(), + new PasswordHashAndSalt("password".getBytes(), "salt".getBytes())); + // Create token with MFA=null (unknown) manager.storage.storeToken(StoredToken.getBuilder( - TokenType.AGENT, id, new UserName("unknownmfauser")) + TokenType.AGENT, id, userName) .withLifeTime(Instant.ofEpochMilli(10000), Instant.ofEpochMilli(1000000000000000L)) .withTokenName(new TokenName("unknownmfatoken")) + .withMfaAuthenticated(null) .build(), it.getHashedToken().getTokenHash()); final URI target = UriBuilder.fromUri(host).path("/api/V2/token").build(); @@ -777,23 +777,22 @@ TokenType.AGENT, id, new UserName("unknownmfauser")) } @Test - public void getTokenWithNonOrcidProvider() throws Exception { + public void getTokenWithNoMfaSet() throws Exception { final UUID id = UUID.randomUUID(); - final IncomingToken it = new IncomingToken("googletokenvalue"); + final IncomingToken it = new IncomingToken("nomfasettokenvalue"); + final UserName userName = new UserName("nomfasetuser"); - // Create user with non-ORCID identity (e.g., Google) - final us.kbase.auth2.lib.identity.RemoteIdentity googleId = new us.kbase.auth2.lib.identity.RemoteIdentity( - new us.kbase.auth2.lib.identity.RemoteIdentityID("Google", "googleid123"), - new us.kbase.auth2.lib.identity.RemoteIdentityDetails("googleuser", "Google User", "google@example.com", null)); - - manager.storage.createUser(us.kbase.auth2.lib.user.NewUser.getBuilder( - new UserName("googleuser"), id, new DisplayName("Google User"), inst(10000), googleId) - .withEmailAddress(new EmailAddress("google@example.com")).build()); + // Create simple local user + manager.storage.createLocalUser(LocalUser.getLocalUserBuilder( + userName, id, new DisplayName("No MFA Set User"), inst(10000)) + .withEmailAddress(new EmailAddress("nomfaset@example.com")).build(), + new PasswordHashAndSalt("password".getBytes(), "salt".getBytes())); + // Create token without explicitly setting MFA (should default to null) manager.storage.storeToken(StoredToken.getBuilder( - TokenType.AGENT, id, new UserName("googleuser")) + TokenType.AGENT, id, userName) .withLifeTime(Instant.ofEpochMilli(10000), Instant.ofEpochMilli(1000000000000000L)) - .withTokenName(new TokenName("googletoken")) + .withTokenName(new TokenName("nomfasettoken")) .build(), it.getHashedToken().getTokenHash()); final URI target = UriBuilder.fromUri(host).path("/api/V2/token").build(); @@ -809,9 +808,9 @@ TokenType.AGENT, id, new UserName("googleuser")) @SuppressWarnings("unchecked") final Map response = res.readEntity(Map.class); - // Non-ORCID providers should return null for MFA status + // Should return null when MFA not explicitly set assertThat("incorrect MFA status", response.get("mfaAuthenticated"), is((Object) null)); - assertThat("incorrect user", response.get("user"), is("googleuser")); - assertThat("incorrect token name", response.get("name"), is("googletoken")); + assertThat("incorrect user", response.get("user"), is("nomfasetuser")); + assertThat("incorrect token name", response.get("name"), is("nomfasettoken")); } } From ae74a21a1c90d1cdf7087aaef12f23649acda6b3 Mon Sep 17 00:00:00 2001 From: David Lyon <5115845+dauglyon@users.noreply.github.com> Date: Thu, 17 Jul 2025 14:36:31 -0700 Subject: [PATCH 14/25] fix tests --- src/main/java/us/kbase/auth2/lib/Authentication.java | 2 +- .../test/auth2/service/api/TokenEndpointTest.java | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/us/kbase/auth2/lib/Authentication.java b/src/main/java/us/kbase/auth2/lib/Authentication.java index e72090e0..91a38348 100644 --- a/src/main/java/us/kbase/auth2/lib/Authentication.java +++ b/src/main/java/us/kbase/auth2/lib/Authentication.java @@ -745,7 +745,7 @@ public void forceResetAllPasswords(final IncomingToken token) private NewToken login(final UserName userName, final TokenCreationContext tokenCtx) throws AuthStorageException { - return login(userName, tokenCtx, false); + return login(userName, tokenCtx, null); } private NewToken login(final UserName userName, final TokenCreationContext tokenCtx, diff --git a/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java b/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java index a08a006c..be1eef0e 100644 --- a/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java +++ b/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java @@ -188,7 +188,7 @@ public void getTokenMfaIsolation() throws Exception { manager.storage.createLocalUser(LocalUser.getLocalUserBuilder( userName, userUuid, new DisplayName("Test User"), inst(10000)) .withEmailAddress(new EmailAddress("test@example.com")).build(), - new PasswordHashAndSalt("password".getBytes(), "salt".getBytes())); + new PasswordHashAndSalt("passwordhash1234".getBytes(), "salt".getBytes())); // Create two tokens with different MFA status final UUID token1Id = UUID.randomUUID(); @@ -672,7 +672,7 @@ public void getTokenWithMfaTrue() throws Exception { manager.storage.createLocalUser(LocalUser.getLocalUserBuilder( userName, id, new DisplayName("MFA User"), inst(10000)) .withEmailAddress(new EmailAddress("mfa@example.com")).build(), - new PasswordHashAndSalt("password".getBytes(), "salt".getBytes())); + new PasswordHashAndSalt("passwordhash1234".getBytes(), "salt".getBytes())); // Create token with MFA=true manager.storage.storeToken(StoredToken.getBuilder( @@ -710,7 +710,7 @@ public void getTokenWithMfaFalse() throws Exception { manager.storage.createLocalUser(LocalUser.getLocalUserBuilder( userName, id, new DisplayName("No MFA User"), inst(10000)) .withEmailAddress(new EmailAddress("nomfa@example.com")).build(), - new PasswordHashAndSalt("password".getBytes(), "salt".getBytes())); + new PasswordHashAndSalt("passwordhash1234".getBytes(), "salt".getBytes())); // Create token with MFA=false manager.storage.storeToken(StoredToken.getBuilder( @@ -748,7 +748,7 @@ public void getTokenWithMfaNull() throws Exception { manager.storage.createLocalUser(LocalUser.getLocalUserBuilder( userName, id, new DisplayName("Unknown MFA User"), inst(10000)) .withEmailAddress(new EmailAddress("unknownmfa@example.com")).build(), - new PasswordHashAndSalt("password".getBytes(), "salt".getBytes())); + new PasswordHashAndSalt("passwordhash1234".getBytes(), "salt".getBytes())); // Create token with MFA=null (unknown) manager.storage.storeToken(StoredToken.getBuilder( @@ -786,7 +786,7 @@ public void getTokenWithNoMfaSet() throws Exception { manager.storage.createLocalUser(LocalUser.getLocalUserBuilder( userName, id, new DisplayName("No MFA Set User"), inst(10000)) .withEmailAddress(new EmailAddress("nomfaset@example.com")).build(), - new PasswordHashAndSalt("password".getBytes(), "salt".getBytes())); + new PasswordHashAndSalt("passwordhash1234".getBytes(), "salt".getBytes())); // Create token without explicitly setting MFA (should default to null) manager.storage.storeToken(StoredToken.getBuilder( From 935f19302a47ee16144418092aa7ffae01aa76d7 Mon Sep 17 00:00:00 2001 From: David Lyon <5115845+dauglyon@users.noreply.github.com> Date: Thu, 17 Jul 2025 15:17:14 -0700 Subject: [PATCH 15/25] slightly better documentation --- RELEASE_NOTES.md | 6 ++++++ src/main/java/us/kbase/auth2/service/api/APIToken.java | 6 ++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index ba235ddd..aa16f8c6 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -1,5 +1,11 @@ # Authentication Service MKII release notes +## 0.8.0 + +* Added MFA (Multi-Factor Authentication) status tracking for tokens +* The `/api/V2/token` endpoint now returns an `mfaAuthenticated` field +* ORCID provider updated to use OpenID Connect scope for MFA detection + ## 0.7.2 * Fixed a bug where usernames with underscores would not be matched in username searches if an diff --git a/src/main/java/us/kbase/auth2/service/api/APIToken.java b/src/main/java/us/kbase/auth2/service/api/APIToken.java index 67a4d932..be6ff3ee 100644 --- a/src/main/java/us/kbase/auth2/service/api/APIToken.java +++ b/src/main/java/us/kbase/auth2/service/api/APIToken.java @@ -23,8 +23,10 @@ public long getCachefor() { /** * Gets the MFA authentication status for this token. * - * @return true if the user authenticated with MFA, false if password only, - * null if unknown or not applicable + * @return the MFA authentication status: + * true - User authenticated with MFA during token creation + * false - User explicitly chose not to use MFA when available + * null - MFA status unknown or not applicable to authentication method */ public Boolean getMfaAuthenticated() { return mfaAuthenticated; From 7bbf759866ca363bd9a35ddf31dfdef3da846a1a Mon Sep 17 00:00:00 2001 From: David Lyon <5115845+dauglyon@users.noreply.github.com> Date: Thu, 17 Jul 2025 15:21:59 -0700 Subject: [PATCH 16/25] update token field name --- src/main/java/us/kbase/auth2/lib/storage/mongo/Fields.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/us/kbase/auth2/lib/storage/mongo/Fields.java b/src/main/java/us/kbase/auth2/lib/storage/mongo/Fields.java index 74ff8607..15b73991 100644 --- a/src/main/java/us/kbase/auth2/lib/storage/mongo/Fields.java +++ b/src/main/java/us/kbase/auth2/lib/storage/mongo/Fields.java @@ -138,7 +138,7 @@ public class Fields { /** 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"; + public static final String TOKEN_MFA_AUTHENTICATED = "mfa"; /* ************************ * temporary session data fields From 3d9e93e18826eb39c3fc76c8da6a8d0d706471f8 Mon Sep 17 00:00:00 2001 From: David Lyon <5115845+dauglyon@users.noreply.github.com> Date: Thu, 17 Jul 2025 15:53:09 -0700 Subject: [PATCH 17/25] consolidate test logic --- .../mongo/MongoStorageUserCreateGetTest.java | 43 ++---- .../auth2/service/api/TokenEndpointTest.java | 143 ++++-------------- 2 files changed, 40 insertions(+), 146 deletions(-) diff --git a/src/test/java/us/kbase/test/auth2/lib/storage/mongo/MongoStorageUserCreateGetTest.java b/src/test/java/us/kbase/test/auth2/lib/storage/mongo/MongoStorageUserCreateGetTest.java index d10bfcad..65a4d4ba 100644 --- a/src/test/java/us/kbase/test/auth2/lib/storage/mongo/MongoStorageUserCreateGetTest.java +++ b/src/test/java/us/kbase/test/auth2/lib/storage/mongo/MongoStorageUserCreateGetTest.java @@ -608,47 +608,32 @@ public void getUserAndUpdateRemoteId() throws Exception { @Test public void createUserWithMfaTrue() throws Exception { - storage.createUser(NewUser.getBuilder( - new UserName("mfauser"), UID, new DisplayName("MFA User"), NOW, REMOTE_MFA_TRUE) - .withEmailAddress(new EmailAddress("mfa@example.com")) - .build()); - - final AuthUser u = storage.getUser(new UserName("mfauser")); - - assertThat("incorrect identities", u.getIdentities(), is(set(REMOTE_MFA_TRUE))); - assertThat("incorrect username", u.getUserName(), is(new UserName("mfauser"))); - assertThat("incorrect display name", u.getDisplayName(), is(new DisplayName("MFA User"))); - assertThat("incorrect email", u.getEmail(), is(new EmailAddress("mfa@example.com"))); + testCreateUserWithMfaAndVerifyStorage("mfauser", "MFA User", "mfa@example.com", REMOTE_MFA_TRUE); } @Test public void createUserWithMfaFalse() throws Exception { - storage.createUser(NewUser.getBuilder( - new UserName("nomfauser"), UID, new DisplayName("No MFA User"), NOW, REMOTE_MFA_FALSE) - .withEmailAddress(new EmailAddress("nomfa@example.com")) - .build()); - - final AuthUser u = storage.getUser(new UserName("nomfauser")); - - assertThat("incorrect identities", u.getIdentities(), is(set(REMOTE_MFA_FALSE))); - assertThat("incorrect username", u.getUserName(), is(new UserName("nomfauser"))); - assertThat("incorrect display name", u.getDisplayName(), is(new DisplayName("No MFA User"))); - assertThat("incorrect email", u.getEmail(), is(new EmailAddress("nomfa@example.com"))); + testCreateUserWithMfaAndVerifyStorage("nomfauser", "No MFA User", "nomfa@example.com", REMOTE_MFA_FALSE); } @Test public void createUserWithMfaNull() throws Exception { + testCreateUserWithMfaAndVerifyStorage("unknownmfauser", "Unknown MFA User", "unknownmfa@example.com", REMOTE_MFA_NULL); + } + + private void testCreateUserWithMfaAndVerifyStorage(final String userName, final String displayName, + final String email, final RemoteIdentity remoteIdentity) throws Exception { storage.createUser(NewUser.getBuilder( - new UserName("unknownmfauser"), UID, new DisplayName("Unknown MFA User"), NOW, REMOTE_MFA_NULL) - .withEmailAddress(new EmailAddress("unknownmfa@example.com")) + new UserName(userName), UID, new DisplayName(displayName), NOW, remoteIdentity) + .withEmailAddress(new EmailAddress(email)) .build()); - final AuthUser u = storage.getUser(new UserName("unknownmfauser")); + final AuthUser u = storage.getUser(new UserName(userName)); - assertThat("incorrect identities", u.getIdentities(), is(set(REMOTE_MFA_NULL))); - assertThat("incorrect username", u.getUserName(), is(new UserName("unknownmfauser"))); - assertThat("incorrect display name", u.getDisplayName(), is(new DisplayName("Unknown MFA User"))); - assertThat("incorrect email", u.getEmail(), is(new EmailAddress("unknownmfa@example.com"))); + assertThat("incorrect identities", u.getIdentities(), is(set(remoteIdentity))); + assertThat("incorrect username", u.getUserName(), is(new UserName(userName))); + assertThat("incorrect display name", u.getDisplayName(), is(new DisplayName(displayName))); + assertThat("incorrect email", u.getEmail(), is(new EmailAddress(email))); } @Test diff --git a/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java b/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java index be1eef0e..0d52d47b 100644 --- a/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java +++ b/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java @@ -664,136 +664,46 @@ private void kbaseTokenSuccess( @Test public void getTokenWithMfaTrue() throws Exception { - final UUID id = UUID.randomUUID(); - final IncomingToken it = new IncomingToken("mfatokenvalue"); - final UserName userName = new UserName("mfauser"); - - // Create simple local user - manager.storage.createLocalUser(LocalUser.getLocalUserBuilder( - userName, id, new DisplayName("MFA User"), inst(10000)) - .withEmailAddress(new EmailAddress("mfa@example.com")).build(), - new PasswordHashAndSalt("passwordhash1234".getBytes(), "salt".getBytes())); - - // Create token with MFA=true - manager.storage.storeToken(StoredToken.getBuilder( - TokenType.AGENT, id, userName) - .withLifeTime(Instant.ofEpochMilli(10000), Instant.ofEpochMilli(1000000000000000L)) - .withTokenName(new TokenName("mfatoken")) - .withMfaAuthenticated(true) - .build(), it.getHashedToken().getTokenHash()); - - final URI target = UriBuilder.fromUri(host).path("/api/V2/token").build(); - - final WebTarget wt = CLI.target(target); - final Builder req = wt.request() - .header("authorization", it.getToken()); - - final Response res = req.get(); - - assertThat("incorrect response code", res.getStatus(), is(200)); - - @SuppressWarnings("unchecked") - final Map response = res.readEntity(Map.class); - - assertThat("incorrect MFA status", response.get("mfaAuthenticated"), is(true)); - assertThat("incorrect user", response.get("user"), is("mfauser")); - assertThat("incorrect token name", response.get("name"), is("mfatoken")); + testTokenEndpointReturnsMfaAuthenticatedField("mfauser", "MFA User", "mfa@example.com", + "mfatoken", "mfatokenvalue", true); } @Test public void getTokenWithMfaFalse() throws Exception { - final UUID id = UUID.randomUUID(); - final IncomingToken it = new IncomingToken("nomfatokenvalue"); - final UserName userName = new UserName("nomfauser"); - - // Create simple local user - manager.storage.createLocalUser(LocalUser.getLocalUserBuilder( - userName, id, new DisplayName("No MFA User"), inst(10000)) - .withEmailAddress(new EmailAddress("nomfa@example.com")).build(), - new PasswordHashAndSalt("passwordhash1234".getBytes(), "salt".getBytes())); - - // Create token with MFA=false - manager.storage.storeToken(StoredToken.getBuilder( - TokenType.AGENT, id, userName) - .withLifeTime(Instant.ofEpochMilli(10000), Instant.ofEpochMilli(1000000000000000L)) - .withTokenName(new TokenName("nomfatoken")) - .withMfaAuthenticated(false) - .build(), it.getHashedToken().getTokenHash()); - - final URI target = UriBuilder.fromUri(host).path("/api/V2/token").build(); - - final WebTarget wt = CLI.target(target); - final Builder req = wt.request() - .header("authorization", it.getToken()); - - final Response res = req.get(); - - assertThat("incorrect response code", res.getStatus(), is(200)); - - @SuppressWarnings("unchecked") - final Map response = res.readEntity(Map.class); - - assertThat("incorrect MFA status", response.get("mfaAuthenticated"), is(false)); - assertThat("incorrect user", response.get("user"), is("nomfauser")); - assertThat("incorrect token name", response.get("name"), is("nomfatoken")); + testTokenEndpointReturnsMfaAuthenticatedField("nomfauser", "No MFA User", "nomfa@example.com", + "nomfatoken", "nomfatokenvalue", false); } @Test public void getTokenWithMfaNull() throws Exception { - final UUID id = UUID.randomUUID(); - final IncomingToken it = new IncomingToken("unknownmfatokenvalue"); - final UserName userName = new UserName("unknownmfauser"); - - // Create simple local user - manager.storage.createLocalUser(LocalUser.getLocalUserBuilder( - userName, id, new DisplayName("Unknown MFA User"), inst(10000)) - .withEmailAddress(new EmailAddress("unknownmfa@example.com")).build(), - new PasswordHashAndSalt("passwordhash1234".getBytes(), "salt".getBytes())); - - // Create token with MFA=null (unknown) - manager.storage.storeToken(StoredToken.getBuilder( - TokenType.AGENT, id, userName) - .withLifeTime(Instant.ofEpochMilli(10000), Instant.ofEpochMilli(1000000000000000L)) - .withTokenName(new TokenName("unknownmfatoken")) - .withMfaAuthenticated(null) - .build(), it.getHashedToken().getTokenHash()); - - final URI target = UriBuilder.fromUri(host).path("/api/V2/token").build(); - - final WebTarget wt = CLI.target(target); - final Builder req = wt.request() - .header("authorization", it.getToken()); - - final Response res = req.get(); - - assertThat("incorrect response code", res.getStatus(), is(200)); - - @SuppressWarnings("unchecked") - final Map response = res.readEntity(Map.class); - - assertThat("incorrect MFA status", response.get("mfaAuthenticated"), is((Object) null)); - assertThat("incorrect user", response.get("user"), is("unknownmfauser")); - assertThat("incorrect token name", response.get("name"), is("unknownmfatoken")); + testTokenEndpointReturnsMfaAuthenticatedField("unknownmfauser", "Unknown MFA User", "unknownmfa@example.com", + "unknownmfatoken", "unknownmfatokenvalue", null); } - @Test - public void getTokenWithNoMfaSet() throws Exception { + private void testTokenEndpointReturnsMfaAuthenticatedField(final String userName, final String displayName, + final String email, final String tokenName, final String tokenValue, + final Boolean mfaStatus) throws Exception { final UUID id = UUID.randomUUID(); - final IncomingToken it = new IncomingToken("nomfasettokenvalue"); - final UserName userName = new UserName("nomfasetuser"); + final IncomingToken it = new IncomingToken(tokenValue); + final UserName user = new UserName(userName); // Create simple local user manager.storage.createLocalUser(LocalUser.getLocalUserBuilder( - userName, id, new DisplayName("No MFA Set User"), inst(10000)) - .withEmailAddress(new EmailAddress("nomfaset@example.com")).build(), + user, id, new DisplayName(displayName), inst(10000)) + .withEmailAddress(new EmailAddress(email)).build(), new PasswordHashAndSalt("passwordhash1234".getBytes(), "salt".getBytes())); - // Create token without explicitly setting MFA (should default to null) - manager.storage.storeToken(StoredToken.getBuilder( - TokenType.AGENT, id, userName) + // Create token with specified MFA status + StoredToken.OptionalsStep tokenBuilder = StoredToken.getBuilder( + TokenType.AGENT, id, user) .withLifeTime(Instant.ofEpochMilli(10000), Instant.ofEpochMilli(1000000000000000L)) - .withTokenName(new TokenName("nomfasettoken")) - .build(), it.getHashedToken().getTokenHash()); + .withTokenName(new TokenName(tokenName)); + + if (mfaStatus != null) { + tokenBuilder = tokenBuilder.withMfaAuthenticated(mfaStatus); + } + + manager.storage.storeToken(tokenBuilder.build(), it.getHashedToken().getTokenHash()); final URI target = UriBuilder.fromUri(host).path("/api/V2/token").build(); @@ -808,9 +718,8 @@ userName, id, new DisplayName("No MFA Set User"), inst(10000)) @SuppressWarnings("unchecked") final Map response = res.readEntity(Map.class); - // Should return null when MFA not explicitly set - assertThat("incorrect MFA status", response.get("mfaAuthenticated"), is((Object) null)); - assertThat("incorrect user", response.get("user"), is("nomfasetuser")); - assertThat("incorrect token name", response.get("name"), is("nomfasettoken")); + assertThat("incorrect MFA status", response.get("mfaAuthenticated"), is((Object) mfaStatus)); + assertThat("incorrect user", response.get("user"), is(userName)); + assertThat("incorrect token name", response.get("name"), is(tokenName)); } } From 8d85ad9d87022ad76cb23d49d5362ea7816283d9 Mon Sep 17 00:00:00 2001 From: David Lyon <5115845+dauglyon@users.noreply.github.com> Date: Fri, 18 Jul 2025 13:49:52 -0700 Subject: [PATCH 18/25] Change mfa value to enum, from boolean --- .../us/kbase/auth2/lib/Authentication.java | 13 +++---- .../kbase/auth2/lib/identity/MfaStatus.java | 11 ++++++ .../lib/identity/RemoteIdentityDetails.java | 25 +++++-------- .../auth2/lib/storage/mongo/MongoStorage.java | 11 +++--- .../us/kbase/auth2/lib/token/StoredToken.java | 27 +++++++------- .../OrcIDIdentityProviderFactory.java | 21 +++++------ .../us/kbase/auth2/service/api/APIToken.java | 18 ++++------ .../lib/identity/RemoteIdentityTest.java | 35 ++++++++++--------- .../mongo/MongoStorageUserCreateGetTest.java | 7 ++-- .../providers/OrcIDIdentityProviderTest.java | 9 ++--- .../test/auth2/service/api/APITokenTest.java | 3 +- .../service/api/TestModeIntegrationTest.java | 3 +- .../auth2/service/api/TokenEndpointTest.java | 25 +++++++------ 13 files changed, 105 insertions(+), 103 deletions(-) create mode 100644 src/main/java/us/kbase/auth2/lib/identity/MfaStatus.java diff --git a/src/main/java/us/kbase/auth2/lib/Authentication.java b/src/main/java/us/kbase/auth2/lib/Authentication.java index 91a38348..b814e2c5 100644 --- a/src/main/java/us/kbase/auth2/lib/Authentication.java +++ b/src/main/java/us/kbase/auth2/lib/Authentication.java @@ -77,6 +77,7 @@ import us.kbase.auth2.lib.exceptions.UserExistsException; import us.kbase.auth2.lib.identity.IdentityProvider; import us.kbase.auth2.lib.identity.RemoteIdentity; +import us.kbase.auth2.lib.identity.MfaStatus; import us.kbase.auth2.lib.identity.RemoteIdentityID; import us.kbase.auth2.lib.storage.AuthStorage; import us.kbase.auth2.lib.storage.exceptions.AuthStorageException; @@ -745,11 +746,11 @@ public void forceResetAllPasswords(final IncomingToken token) private NewToken login(final UserName userName, final TokenCreationContext tokenCtx) throws AuthStorageException { - return login(userName, tokenCtx, null); + return login(userName, tokenCtx, MfaStatus.UNKNOWN); } private NewToken login(final UserName userName, final TokenCreationContext tokenCtx, - final Boolean mfaAuthenticated) throws AuthStorageException { + final MfaStatus mfaAuthenticated) throws AuthStorageException { final NewToken nt = new NewToken(StoredToken.getBuilder( TokenType.LOGIN, randGen.randomUUID(), userName) .withLifeTime(clock.instant(), @@ -912,7 +913,7 @@ public NewToken createToken( .withLifeTime(clock.instant(), life) .withContext(tokenCtx) .withTokenName(tokenName) - .withMfaAuthenticated(null) // Agent/Dev/Serv tokens don't have MFA status + .withMfaAuthenticated(MfaStatus.UNKNOWN) // Agent/Dev/Serv tokens don't have MFA status .build(), randGen.getToken()); storage.storeToken(nt.getStoredToken(), nt.getTokenHash()); @@ -2051,7 +2052,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 + .withMfaAuthenticated(MfaStatus.UNKNOWN) // Test mode tokens don't have MFA status .build(), randGen.getToken()); storage.testModeStoreToken(nt.getStoredToken(), nt.getTokenHash()); @@ -2348,8 +2349,8 @@ public NewToken login( linked, u.get().getUserName().getName()); } } - final Boolean mfaStatus = ri.get().getDetails() != null ? - ri.get().getDetails().getMfaAuthenticated() : null; + final MfaStatus mfaStatus = ri.get().getDetails() != null ? + ri.get().getDetails().getMfaAuthenticated() : MfaStatus.UNKNOWN; return login(u.get().getUserName(), tokenCtx, mfaStatus); } diff --git a/src/main/java/us/kbase/auth2/lib/identity/MfaStatus.java b/src/main/java/us/kbase/auth2/lib/identity/MfaStatus.java new file mode 100644 index 00000000..3d6df71a --- /dev/null +++ b/src/main/java/us/kbase/auth2/lib/identity/MfaStatus.java @@ -0,0 +1,11 @@ +package us.kbase.auth2.lib.identity; + +/** An enumeration representing the multi-factor authentication status of a user's login. */ +public enum MfaStatus { + /** User authenticated with MFA during token creation. */ + USED, + /** User explicitly chose not to use MFA when available. */ + NOT_USED, + /** MFA status unknown or not applicable to authentication method. */ + UNKNOWN; +} \ No newline at end of file diff --git a/src/main/java/us/kbase/auth2/lib/identity/RemoteIdentityDetails.java b/src/main/java/us/kbase/auth2/lib/identity/RemoteIdentityDetails.java index a909f4f6..d6fdc667 100644 --- a/src/main/java/us/kbase/auth2/lib/identity/RemoteIdentityDetails.java +++ b/src/main/java/us/kbase/auth2/lib/identity/RemoteIdentityDetails.java @@ -10,7 +10,7 @@ public class RemoteIdentityDetails { private final String username; private final String fullname; private final String email; - private final Boolean mfaAuthenticated; + private final MfaStatus mfaAuthenticated; /** Create a new set of details. * @param username the user name of the identity. @@ -21,22 +21,20 @@ public RemoteIdentityDetails( final String username, final String fullname, final String email) { - this(username, fullname, email, null); + this(username, fullname, email, MfaStatus.UNKNOWN); } /** 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. + * @param mfaAuthenticated the multi-factor authentication status. */ public RemoteIdentityDetails( final String username, final String fullname, final String email, - final Boolean mfaAuthenticated) { + final MfaStatus mfaAuthenticated) { super(); if (username == null || username.trim().isEmpty()) { throw new IllegalArgumentException( @@ -76,11 +74,10 @@ 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. + /** Get the multi-factor authentication status. + * @return the MFA status. */ - public Boolean getMfaAuthenticated() { + public MfaStatus getMfaAuthenticated() { return mfaAuthenticated; } @@ -90,7 +87,7 @@ public int hashCode() { 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 + mfaAuthenticated.hashCode(); result = prime * result + ((username == null) ? 0 : username.hashCode()); return result; } @@ -121,11 +118,7 @@ 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)) { + if (!mfaAuthenticated.equals(other.mfaAuthenticated)) { return false; } if (username == null) { diff --git a/src/main/java/us/kbase/auth2/lib/storage/mongo/MongoStorage.java b/src/main/java/us/kbase/auth2/lib/storage/mongo/MongoStorage.java index 1d8e53b0..533c4352 100644 --- a/src/main/java/us/kbase/auth2/lib/storage/mongo/MongoStorage.java +++ b/src/main/java/us/kbase/auth2/lib/storage/mongo/MongoStorage.java @@ -90,6 +90,7 @@ import us.kbase.auth2.lib.exceptions.UserExistsException; import us.kbase.auth2.lib.identity.RemoteIdentity; import us.kbase.auth2.lib.identity.RemoteIdentityDetails; +import us.kbase.auth2.lib.identity.MfaStatus; import us.kbase.auth2.lib.identity.RemoteIdentityID; import us.kbase.auth2.lib.storage.AuthStorage; import us.kbase.auth2.lib.storage.exceptions.AuthStorageException; @@ -872,7 +873,7 @@ private void storeToken(final String collection, final StoredToken token, final .append(Fields.TOKEN_IP, ctx.getIpAddress().isPresent() ? ctx.getIpAddress().get().getHostAddress() : null) .append(Fields.TOKEN_CUSTOM_CONTEXT, toCustomContextList(ctx.getCustomContext())) - .append(Fields.TOKEN_MFA_AUTHENTICATED, token.getMfaAuthenticated()); + .append(Fields.TOKEN_MFA_AUTHENTICATED, token.getMfaAuthenticated().name()); try { db.getCollection(collection).insertOne(td); } catch (MongoWriteException mwe) { @@ -970,7 +971,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)) + .withMfaAuthenticated(MfaStatus.valueOf(t.getString(Fields.TOKEN_MFA_AUTHENTICATED))) .build(); } @@ -1599,7 +1600,7 @@ private void updateIdentity(final RemoteIdentity remoteID) 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_MFA, rid.getMfaAuthenticated())); + .append(pre + Fields.IDENTITIES_MFA, rid.getMfaAuthenticated().name())); try { // id might have been unlinked, so we just assume // the update worked. If it was just unlinked we don't care. @@ -1733,7 +1734,7 @@ private Document toDocument(final RemoteIdentity id) { .append(Fields.IDENTITIES_USER, rid.getUsername()) .append(Fields.IDENTITIES_NAME, rid.getFullname()) .append(Fields.IDENTITIES_EMAIL, rid.getEmail()) - .append(Fields.IDENTITIES_MFA, rid.getMfaAuthenticated()); + .append(Fields.IDENTITIES_MFA, rid.getMfaAuthenticated().name()); } @Override @@ -1794,7 +1795,7 @@ private Set toIdentities(final List ids) { i.getString(Fields.IDENTITIES_USER), i.getString(Fields.IDENTITIES_NAME), i.getString(Fields.IDENTITIES_EMAIL), - i.getBoolean(Fields.IDENTITIES_MFA)); + MfaStatus.valueOf(i.getString(Fields.IDENTITIES_MFA))); ret.add(new RemoteIdentity(rid, det)); } return ret; diff --git a/src/main/java/us/kbase/auth2/lib/token/StoredToken.java b/src/main/java/us/kbase/auth2/lib/token/StoredToken.java index 4793b655..b07cdaa8 100644 --- a/src/main/java/us/kbase/auth2/lib/token/StoredToken.java +++ b/src/main/java/us/kbase/auth2/lib/token/StoredToken.java @@ -8,6 +8,7 @@ import us.kbase.auth2.lib.TokenCreationContext; import us.kbase.auth2.lib.UserName; +import us.kbase.auth2.lib.identity.MfaStatus; /** A token associated with a user stored in the authentication storage system. * @@ -23,7 +24,7 @@ public class StoredToken { private final UserName userName; private final Instant creationDate; private final Instant expirationDate; - private final Boolean mfaAuthenticated; + private final MfaStatus mfaAuthenticated; private StoredToken( final UUID id, @@ -33,7 +34,7 @@ private StoredToken( final TokenCreationContext context, final Instant creationDate, final Instant expirationDate, - final Boolean mfaAuthenticated) { + final MfaStatus 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 @@ -97,10 +98,10 @@ 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. + /** Get the multi-factor authentication status for this token. + * @return the MFA status. */ - public Boolean getMfaAuthenticated() { + public MfaStatus getMfaAuthenticated() { return mfaAuthenticated; } @@ -115,7 +116,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()); + result = prime * result + mfaAuthenticated.hashCode(); return result; } @@ -176,11 +177,7 @@ 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)) { + if (!mfaAuthenticated.equals(other.mfaAuthenticated)) { return false; } return true; @@ -243,10 +240,10 @@ 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. + * @param mfaAuthenticated the MFA status. * @return this builder. */ - OptionalsStep withMfaAuthenticated(Boolean mfaAuthenticated); + OptionalsStep withMfaAuthenticated(MfaStatus mfaAuthenticated); /** Build the token. * @return a new StoredToken. @@ -263,7 +260,7 @@ private static class Builder implements LifeStep, OptionalsStep { private final UserName userName; private Instant creationDate; private Instant expirationDate; - private Boolean mfaAuthenticated; + private MfaStatus mfaAuthenticated = MfaStatus.UNKNOWN; private Builder(final TokenType type, final UUID id, final UserName userName) { requireNonNull(type, "type"); @@ -295,7 +292,7 @@ public OptionalsStep withContext(final TokenCreationContext context) { } @Override - public OptionalsStep withMfaAuthenticated(final Boolean mfaAuthenticated) { + public OptionalsStep withMfaAuthenticated(final MfaStatus mfaAuthenticated) { this.mfaAuthenticated = mfaAuthenticated; return this; } diff --git a/src/main/java/us/kbase/auth2/providers/OrcIDIdentityProviderFactory.java b/src/main/java/us/kbase/auth2/providers/OrcIDIdentityProviderFactory.java index 28d47bf6..72375e83 100644 --- a/src/main/java/us/kbase/auth2/providers/OrcIDIdentityProviderFactory.java +++ b/src/main/java/us/kbase/auth2/providers/OrcIDIdentityProviderFactory.java @@ -34,6 +34,7 @@ import us.kbase.auth2.lib.identity.IdentityProviderFactory; import us.kbase.auth2.lib.identity.RemoteIdentity; import us.kbase.auth2.lib.identity.RemoteIdentityDetails; +import us.kbase.auth2.lib.identity.MfaStatus; import us.kbase.auth2.lib.identity.RemoteIdentityID; /** A factory for a OrcID identity provider. @@ -214,7 +215,7 @@ private static class OrcIDAccessTokenResponse { private final String fullName; private final String orcID; private final String idToken; - private final Boolean mfaAuthenticated; + private final MfaStatus mfaAuthenticated; private OrcIDAccessTokenResponse( final String accessToken, @@ -242,11 +243,11 @@ private OrcIDAccessTokenResponse( * to determine if multi-factor authentication was used. * * @param idToken the JWT ID token from ORCID - * @return true if MFA was used, false if password only, null if unknown or parsing failed + * @return MfaStatus indicating whether MFA was used */ - private Boolean parseAmrClaim(final String idToken) { + private MfaStatus parseAmrClaim(final String idToken) { if (idToken == null || idToken.trim().isEmpty()) { - return null; + return MfaStatus.UNKNOWN; } try { @@ -254,7 +255,7 @@ private Boolean parseAmrClaim(final String idToken) { final String[] parts = idToken.split("\\."); if (parts.length != 3) { // Invalid JWT format - return null; + return MfaStatus.UNKNOWN; } // Decode the payload (second part) - URL-safe base64 @@ -269,21 +270,21 @@ private Boolean parseAmrClaim(final String idToken) { // OpenID Connect spec: AMR should be an array of strings @SuppressWarnings("unchecked") final List amrList = (List) amrClaim; - return amrList.contains("mfa"); + return amrList.contains("mfa") ? MfaStatus.USED : MfaStatus.NOT_USED; } else if (amrClaim instanceof String) { // ORCID may return single string - handle as fallback - return "mfa".equals(amrClaim); + return "mfa".equals(amrClaim) ? MfaStatus.USED : MfaStatus.NOT_USED; } // AMR claim present but in unexpected format - return null; + return MfaStatus.UNKNOWN; } catch (IllegalArgumentException e) { // Base64 decoding failed - invalid JWT - return null; + return MfaStatus.UNKNOWN; } catch (IOException e) { // JSON parsing failed - malformed payload - return null; + return MfaStatus.UNKNOWN; } } } diff --git a/src/main/java/us/kbase/auth2/service/api/APIToken.java b/src/main/java/us/kbase/auth2/service/api/APIToken.java index be6ff3ee..1b51740a 100644 --- a/src/main/java/us/kbase/auth2/service/api/APIToken.java +++ b/src/main/java/us/kbase/auth2/service/api/APIToken.java @@ -1,6 +1,7 @@ package us.kbase.auth2.service.api; import us.kbase.auth2.lib.token.StoredToken; +import us.kbase.auth2.lib.identity.MfaStatus; import us.kbase.auth2.service.common.ExternalToken; public class APIToken extends ExternalToken { @@ -8,7 +9,7 @@ public class APIToken extends ExternalToken { //TODO JAVADOC or swagger private final long cachefor; - private final Boolean mfaAuthenticated; + private final MfaStatus mfaAuthenticated; public APIToken(final StoredToken token, final long tokenCacheTimeMillis) { super(token); @@ -23,12 +24,9 @@ public long getCachefor() { /** * Gets the MFA authentication status for this token. * - * @return the MFA authentication status: - * true - User authenticated with MFA during token creation - * false - User explicitly chose not to use MFA when available - * null - MFA status unknown or not applicable to authentication method + * @return the MFA authentication status. */ - public Boolean getMfaAuthenticated() { + public MfaStatus getMfaAuthenticated() { return mfaAuthenticated; } @@ -37,7 +35,7 @@ public int hashCode() { final int prime = 31; int result = super.hashCode(); result = prime * result + (int) (cachefor ^ (cachefor >>> 32)); - result = prime * result + ((mfaAuthenticated == null) ? 0 : mfaAuthenticated.hashCode()); + result = prime * result + mfaAuthenticated.hashCode(); return result; } @@ -53,11 +51,7 @@ public boolean equals(Object obj) { if (cachefor != other.cachefor) { return false; } - if (mfaAuthenticated == null) { - if (other.mfaAuthenticated != null) { - return false; - } - } else if (!mfaAuthenticated.equals(other.mfaAuthenticated)) { + if (!mfaAuthenticated.equals(other.mfaAuthenticated)) { return false; } return true; diff --git a/src/test/java/us/kbase/test/auth2/lib/identity/RemoteIdentityTest.java b/src/test/java/us/kbase/test/auth2/lib/identity/RemoteIdentityTest.java index 4eb308db..7336c56f 100644 --- a/src/test/java/us/kbase/test/auth2/lib/identity/RemoteIdentityTest.java +++ b/src/test/java/us/kbase/test/auth2/lib/identity/RemoteIdentityTest.java @@ -9,6 +9,7 @@ import nl.jqno.equalsverifier.EqualsVerifier; import us.kbase.auth2.lib.identity.RemoteIdentity; import us.kbase.auth2.lib.identity.RemoteIdentityDetails; +import us.kbase.auth2.lib.identity.MfaStatus; import us.kbase.auth2.lib.identity.RemoteIdentityID; @@ -20,10 +21,10 @@ public void remoteDetailsWithAllFields() throws Exception { assertThat("incorrect username", dets.getUsername(), is("user")); assertThat("incorrect fullname", dets.getFullname(), is("full")); assertThat("incorrect email", dets.getEmail(), is("email")); - assertThat("incorrect mfa authenticated", dets.getMfaAuthenticated(), is((Boolean) null)); + assertThat("incorrect mfa authenticated", dets.getMfaAuthenticated(), is(MfaStatus.UNKNOWN)); assertThat("incorrect hashcode", dets.hashCode(), is(-497844993)); assertThat("incorrect toString()", dets.toString(), - is("RemoteIdentityDetails [username=user, fullname=full, email=email, mfaAuthenticated=null]")); + is("RemoteIdentityDetails [username=user, fullname=full, email=email, mfaAuthenticated=UNKNOWN]")); } @Test @@ -32,19 +33,19 @@ public void remoteDetailsWithEmptyFields() throws Exception { assertThat("incorrect username", dets.getUsername(), is("user")); assertThat("incorrect fullname", dets.getFullname(), is((String) null)); assertThat("incorrect email", dets.getEmail(), is((String) null)); - assertThat("incorrect mfa authenticated", dets.getMfaAuthenticated(), is((Boolean) null)); + assertThat("incorrect mfa authenticated", dets.getMfaAuthenticated(), is(MfaStatus.UNKNOWN)); assertThat("incorrect hashcode", dets.hashCode(), is(4522828)); assertThat("incorrect toString()", dets.toString(), - is("RemoteIdentityDetails [username=user, fullname=null, email=null, mfaAuthenticated=null]")); + is("RemoteIdentityDetails [username=user, fullname=null, email=null, mfaAuthenticated=UNKNOWN]")); final RemoteIdentityDetails dets2 = new RemoteIdentityDetails("user", null, null); assertThat("incorrect username", dets2.getUsername(), is("user")); assertThat("incorrect fullname", dets2.getFullname(), is((String) null)); assertThat("incorrect email", dets2.getEmail(), is((String) null)); - assertThat("incorrect mfa authenticated", dets2.getMfaAuthenticated(), is((Boolean) null)); + assertThat("incorrect mfa authenticated", dets2.getMfaAuthenticated(), is(MfaStatus.UNKNOWN)); assertThat("incorrect hashcode", dets2.hashCode(), is(4522828)); assertThat("incorrect toString()", dets2.toString(), - is("RemoteIdentityDetails [username=user, fullname=null, email=null, mfaAuthenticated=null]")); + is("RemoteIdentityDetails [username=user, fullname=null, email=null, mfaAuthenticated=UNKNOWN]")); } @Test @@ -118,7 +119,7 @@ public void identity() throws Exception { assertThat("incorrect hashcode", ri.hashCode(), is(124952370)); assertThat("incorrect toString()", ri.toString(), is("RemoteIdentity [remoteID=RemoteIdentityID [provider=p, id=i], " + - "details=RemoteIdentityDetails [username=u, fullname=f, email=e, mfaAuthenticated=null]]")); + "details=RemoteIdentityDetails [username=u, fullname=f, email=e, mfaAuthenticated=UNKNOWN]]")); } @Test @@ -146,41 +147,41 @@ private void failCreateIdentity( @Test public void remoteDetailsWithMfaTrue() throws Exception { - final RemoteIdentityDetails dets = new RemoteIdentityDetails("user", "full", "email", true); + final RemoteIdentityDetails dets = new RemoteIdentityDetails("user", "full", "email", MfaStatus.USED); assertThat("incorrect username", dets.getUsername(), is("user")); assertThat("incorrect fullname", dets.getFullname(), is("full")); assertThat("incorrect email", dets.getEmail(), is("email")); - assertThat("incorrect mfa authenticated", dets.getMfaAuthenticated(), is(true)); + assertThat("incorrect mfa authenticated", dets.getMfaAuthenticated(), is(MfaStatus.USED)); assertThat("incorrect toString()", dets.toString(), - is("RemoteIdentityDetails [username=user, fullname=full, email=email, mfaAuthenticated=true]")); + is("RemoteIdentityDetails [username=user, fullname=full, email=email, mfaAuthenticated=USED]")); } @Test public void remoteDetailsWithMfaFalse() throws Exception { - final RemoteIdentityDetails dets = new RemoteIdentityDetails("user", "full", "email", false); + final RemoteIdentityDetails dets = new RemoteIdentityDetails("user", "full", "email", MfaStatus.NOT_USED); assertThat("incorrect username", dets.getUsername(), is("user")); assertThat("incorrect fullname", dets.getFullname(), is("full")); assertThat("incorrect email", dets.getEmail(), is("email")); - assertThat("incorrect mfa authenticated", dets.getMfaAuthenticated(), is(false)); + assertThat("incorrect mfa authenticated", dets.getMfaAuthenticated(), is(MfaStatus.NOT_USED)); assertThat("incorrect toString()", dets.toString(), - is("RemoteIdentityDetails [username=user, fullname=full, email=email, mfaAuthenticated=false]")); + is("RemoteIdentityDetails [username=user, fullname=full, email=email, mfaAuthenticated=NOT_USED]")); } @Test public void remoteDetailsWithMfaNull() throws Exception { - final RemoteIdentityDetails dets = new RemoteIdentityDetails("user", "full", "email", null); + final RemoteIdentityDetails dets = new RemoteIdentityDetails("user", "full", "email", MfaStatus.UNKNOWN); assertThat("incorrect username", dets.getUsername(), is("user")); assertThat("incorrect fullname", dets.getFullname(), is("full")); assertThat("incorrect email", dets.getEmail(), is("email")); - assertThat("incorrect mfa authenticated", dets.getMfaAuthenticated(), is((Boolean) null)); + assertThat("incorrect mfa authenticated", dets.getMfaAuthenticated(), is(MfaStatus.UNKNOWN)); assertThat("incorrect toString()", dets.toString(), - is("RemoteIdentityDetails [username=user, fullname=full, email=email, mfaAuthenticated=null]")); + is("RemoteIdentityDetails [username=user, fullname=full, email=email, mfaAuthenticated=UNKNOWN]")); } @Test public void remoteDetailsMfaFailWithNullUser() throws Exception { try { - new RemoteIdentityDetails(null, "full", "email", true); + new RemoteIdentityDetails(null, "full", "email", MfaStatus.USED); fail("created bad details with mfa"); } catch (IllegalArgumentException e) { assertThat("incorrect exception msg", e.getMessage(), diff --git a/src/test/java/us/kbase/test/auth2/lib/storage/mongo/MongoStorageUserCreateGetTest.java b/src/test/java/us/kbase/test/auth2/lib/storage/mongo/MongoStorageUserCreateGetTest.java index 65a4d4ba..977d3fe9 100644 --- a/src/test/java/us/kbase/test/auth2/lib/storage/mongo/MongoStorageUserCreateGetTest.java +++ b/src/test/java/us/kbase/test/auth2/lib/storage/mongo/MongoStorageUserCreateGetTest.java @@ -31,6 +31,7 @@ import us.kbase.auth2.lib.exceptions.NoSuchUserException; import us.kbase.auth2.lib.exceptions.UserExistsException; import us.kbase.auth2.lib.identity.RemoteIdentityDetails; +import us.kbase.auth2.lib.identity.MfaStatus; import us.kbase.auth2.lib.identity.RemoteIdentityID; import us.kbase.auth2.lib.identity.RemoteIdentity; import us.kbase.auth2.lib.storage.exceptions.AuthStorageException; @@ -59,15 +60,15 @@ public class MongoStorageUserCreateGetTest extends MongoStorageTester { private static final RemoteIdentity REMOTE_MFA_TRUE = new RemoteIdentity( new RemoteIdentityID("orcid", "0000-0001-1234-5678"), - new RemoteIdentityDetails("orciduser", "ORCID User", "orcid@example.com", true)); + new RemoteIdentityDetails("orciduser", "ORCID User", "orcid@example.com", MfaStatus.USED)); private static final RemoteIdentity REMOTE_MFA_FALSE = new RemoteIdentity( new RemoteIdentityID("orcid", "0000-0001-1234-9999"), - new RemoteIdentityDetails("orciduser2", "ORCID User 2", "orcid2@example.com", false)); + new RemoteIdentityDetails("orciduser2", "ORCID User 2", "orcid2@example.com", MfaStatus.NOT_USED)); private static final RemoteIdentity REMOTE_MFA_NULL = new RemoteIdentity( new RemoteIdentityID("orcid", "0000-0001-1234-0000"), - new RemoteIdentityDetails("orciduser3", "ORCID User 3", "orcid3@example.com", null)); + new RemoteIdentityDetails("orciduser3", "ORCID User 3", "orcid3@example.com", MfaStatus.UNKNOWN)); @Test public void createGetLocalUserMinimal() throws Exception { diff --git a/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java b/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java index c1b1363b..ec130de5 100644 --- a/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java +++ b/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java @@ -35,6 +35,7 @@ import us.kbase.auth2.lib.identity.IdentityProviderConfig; import us.kbase.auth2.lib.identity.RemoteIdentity; import us.kbase.auth2.lib.identity.RemoteIdentityDetails; +import us.kbase.auth2.lib.identity.MfaStatus; import us.kbase.auth2.lib.identity.RemoteIdentityID; import us.kbase.auth2.lib.identity.IdentityProviderConfig.IdentityProviderConfigurationException; import us.kbase.auth2.providers.OrcIDIdentityProviderFactory; @@ -510,7 +511,7 @@ public void getIdentityWithMfaTrue() throws Exception { assertThat("incorrect number of idents", rids.size(), is(1)); final Set expected = new HashSet<>(); expected.add(new RemoteIdentity(new RemoteIdentityID(ORCID, orcID), - new RemoteIdentityDetails(orcID, "My name", "mfa@test.com", true))); + new RemoteIdentityDetails(orcID, "My name", "mfa@test.com", MfaStatus.USED))); assertThat("incorrect ident set", rids, is(expected)); } @@ -530,7 +531,7 @@ public void getIdentityWithMfaFalse() throws Exception { assertThat("incorrect number of idents", rids.size(), is(1)); final Set expected = new HashSet<>(); expected.add(new RemoteIdentity(new RemoteIdentityID(ORCID, orcID), - new RemoteIdentityDetails(orcID, "My name", "nomfa@test.com", false))); + new RemoteIdentityDetails(orcID, "My name", "nomfa@test.com", MfaStatus.NOT_USED))); assertThat("incorrect ident set", rids, is(expected)); } @@ -589,7 +590,7 @@ public void getIdentityWithStringAmr() throws Exception { assertThat("incorrect number of idents", rids.size(), is(1)); final Set expected = new HashSet<>(); expected.add(new RemoteIdentity(new RemoteIdentityID(ORCID, orcID), - new RemoteIdentityDetails(orcID, "My name", "stringmfa@test.com", true))); + new RemoteIdentityDetails(orcID, "My name", "stringmfa@test.com", MfaStatus.USED))); assertThat("incorrect ident set", rids, is(expected)); } @@ -832,7 +833,7 @@ public void getIdentityWithEmptyAmrJWT() throws Exception { assertThat("incorrect number of idents", rids.size(), is(1)); final Set expected = new HashSet<>(); expected.add(new RemoteIdentity(new RemoteIdentityID(ORCID, orcID), - new RemoteIdentityDetails(orcID, "My name", "emptyamr@test.com", false))); + new RemoteIdentityDetails(orcID, "My name", "emptyamr@test.com", MfaStatus.NOT_USED))); assertThat("incorrect ident set", rids, is(expected)); } diff --git a/src/test/java/us/kbase/test/auth2/service/api/APITokenTest.java b/src/test/java/us/kbase/test/auth2/service/api/APITokenTest.java index f89e4d82..ba011126 100644 --- a/src/test/java/us/kbase/test/auth2/service/api/APITokenTest.java +++ b/src/test/java/us/kbase/test/auth2/service/api/APITokenTest.java @@ -10,6 +10,7 @@ import nl.jqno.equalsverifier.EqualsVerifier; import us.kbase.auth2.lib.UserName; +import us.kbase.auth2.lib.identity.MfaStatus; import us.kbase.auth2.lib.token.NewToken; import us.kbase.auth2.lib.token.StoredToken; import us.kbase.auth2.lib.token.TokenType; @@ -39,7 +40,7 @@ TokenType.AGENT, id, new UserName("foo")) assertThat("incorrect id", t.getId(), is(id.toString())); assertThat("incorrect cache time", t.getCachefor(), is(20000L)); - assertThat("incorrect mfa status", t.getMfaAuthenticated(), is((Boolean) null)); + assertThat("incorrect mfa status", t.getMfaAuthenticated(), is(MfaStatus.UNKNOWN)); } diff --git a/src/test/java/us/kbase/test/auth2/service/api/TestModeIntegrationTest.java b/src/test/java/us/kbase/test/auth2/service/api/TestModeIntegrationTest.java index e1febb2a..16ddce1a 100644 --- a/src/test/java/us/kbase/test/auth2/service/api/TestModeIntegrationTest.java +++ b/src/test/java/us/kbase/test/auth2/service/api/TestModeIntegrationTest.java @@ -31,6 +31,7 @@ import com.google.common.collect.ImmutableMap; import us.kbase.auth2.kbase.KBaseAuthConfig; +import us.kbase.auth2.lib.identity.MfaStatus; import us.kbase.test.auth2.MapBuilder; import us.kbase.test.auth2.MongoStorageTestManager; import us.kbase.test.auth2.StandaloneAuthServer; @@ -209,7 +210,7 @@ public void createAndGetToken() { expected.put("user", "whee"); expected.put("custom", Collections.emptyMap()); expected.put("cachefor", 300000); - expected.put("mfaAuthenticated", null); + expected.put("mfaAuthenticated", MfaStatus.UNKNOWN); assertThat("incorrect return", response, is(expected)); diff --git a/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java b/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java index 0d52d47b..9266a7f2 100644 --- a/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java +++ b/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java @@ -35,6 +35,7 @@ import us.kbase.auth2.lib.PasswordHashAndSalt; import us.kbase.auth2.lib.TokenCreationContext; import us.kbase.auth2.lib.UserName; +import us.kbase.auth2.lib.identity.MfaStatus; import us.kbase.auth2.lib.exceptions.AuthException; import us.kbase.auth2.lib.exceptions.ErrorType; import us.kbase.auth2.lib.exceptions.IllegalParameterException; @@ -174,7 +175,7 @@ TokenType.AGENT, id, new UserName("foo")) .with("user", "foo") .with("custom", ImmutableMap.of("whee", "whoo")) .with("cachefor", 300000) - .with("mfaAuthenticated", null) + .with("mfaAuthenticated", MfaStatus.UNKNOWN) .build(); assertThat("incorrect token", response, is(expected)); @@ -201,14 +202,14 @@ userName, userUuid, new DisplayName("Test User"), inst(10000)) manager.storage.storeToken(StoredToken.getBuilder( TokenType.LOGIN, token1Id, userName) .withLifeTime(Instant.ofEpochMilli(10000), Instant.ofEpochMilli(1000000000000000L)) - .withMfaAuthenticated(true) + .withMfaAuthenticated(MfaStatus.USED) .build(), token1.getHashedToken().getTokenHash()); // Token 2 with MFA=false manager.storage.storeToken(StoredToken.getBuilder( TokenType.LOGIN, token2Id, userName) .withLifeTime(Instant.ofEpochMilli(10000), Instant.ofEpochMilli(1000000000000000L)) - .withMfaAuthenticated(false) + .withMfaAuthenticated(MfaStatus.NOT_USED) .build(), token2.getHashedToken().getTokenHash()); // Test token1 returns MFA=true @@ -220,7 +221,7 @@ userName, userUuid, new DisplayName("Test User"), inst(10000)) assertThat("incorrect response code for token1", res1.getStatus(), is(200)); @SuppressWarnings("unchecked") final Map response1 = res1.readEntity(Map.class); - assertThat("token1 should have MFA=true", response1.get("mfaAuthenticated"), is(true)); + assertThat("token1 should have MFA=true", response1.get("mfaAuthenticated"), is(MfaStatus.USED)); // Test token2 returns MFA=false final URI target2 = UriBuilder.fromUri(host).path("/api/V2/token").build(); @@ -231,7 +232,7 @@ userName, userUuid, new DisplayName("Test User"), inst(10000)) assertThat("incorrect response code for token2", res2.getStatus(), is(200)); @SuppressWarnings("unchecked") final Map response2 = res2.readEntity(Map.class); - assertThat("token2 should have MFA=false", response2.get("mfaAuthenticated"), is(false)); + assertThat("token2 should have MFA=false", response2.get("mfaAuthenticated"), is(MfaStatus.NOT_USED)); } @Test @@ -665,24 +666,24 @@ private void kbaseTokenSuccess( @Test public void getTokenWithMfaTrue() throws Exception { testTokenEndpointReturnsMfaAuthenticatedField("mfauser", "MFA User", "mfa@example.com", - "mfatoken", "mfatokenvalue", true); + "mfatoken", "mfatokenvalue", MfaStatus.USED); } @Test public void getTokenWithMfaFalse() throws Exception { testTokenEndpointReturnsMfaAuthenticatedField("nomfauser", "No MFA User", "nomfa@example.com", - "nomfatoken", "nomfatokenvalue", false); + "nomfatoken", "nomfatokenvalue", MfaStatus.NOT_USED); } @Test public void getTokenWithMfaNull() throws Exception { testTokenEndpointReturnsMfaAuthenticatedField("unknownmfauser", "Unknown MFA User", "unknownmfa@example.com", - "unknownmfatoken", "unknownmfatokenvalue", null); + "unknownmfatoken", "unknownmfatokenvalue", MfaStatus.UNKNOWN); } private void testTokenEndpointReturnsMfaAuthenticatedField(final String userName, final String displayName, final String email, final String tokenName, final String tokenValue, - final Boolean mfaStatus) throws Exception { + final MfaStatus mfaStatus) throws Exception { final UUID id = UUID.randomUUID(); final IncomingToken it = new IncomingToken(tokenValue); final UserName user = new UserName(userName); @@ -699,9 +700,7 @@ user, id, new DisplayName(displayName), inst(10000)) .withLifeTime(Instant.ofEpochMilli(10000), Instant.ofEpochMilli(1000000000000000L)) .withTokenName(new TokenName(tokenName)); - if (mfaStatus != null) { - tokenBuilder = tokenBuilder.withMfaAuthenticated(mfaStatus); - } + tokenBuilder = tokenBuilder.withMfaAuthenticated(mfaStatus); manager.storage.storeToken(tokenBuilder.build(), it.getHashedToken().getTokenHash()); @@ -718,7 +717,7 @@ user, id, new DisplayName(displayName), inst(10000)) @SuppressWarnings("unchecked") final Map response = res.readEntity(Map.class); - assertThat("incorrect MFA status", response.get("mfaAuthenticated"), is((Object) mfaStatus)); + assertThat("incorrect MFA status", response.get("mfaAuthenticated"), is(mfaStatus)); assertThat("incorrect user", response.get("user"), is(userName)); assertThat("incorrect token name", response.get("name"), is(tokenName)); } From 2db78148b1fa52e742dcd50dd3dc24b20071c3c5 Mon Sep 17 00:00:00 2001 From: David Lyon <5115845+dauglyon@users.noreply.github.com> Date: Fri, 18 Jul 2025 14:10:28 -0700 Subject: [PATCH 19/25] rename all uses MfaAuthenticated -> mfa --- RELEASE_NOTES.md | 2 +- .../us/kbase/auth2/lib/Authentication.java | 6 ++--- .../lib/identity/RemoteIdentityDetails.java | 20 +++++++------- .../kbase/auth2/lib/storage/mongo/Fields.java | 2 +- .../auth2/lib/storage/mongo/MongoStorage.java | 8 +++--- .../us/kbase/auth2/lib/token/StoredToken.java | 26 +++++++++---------- .../OrcIDIdentityProviderFactory.java | 6 ++--- .../us/kbase/auth2/service/api/APIToken.java | 12 ++++----- .../lib/identity/RemoteIdentityTest.java | 26 +++++++++---------- .../test/auth2/service/api/APITokenTest.java | 2 +- .../service/api/TestModeIntegrationTest.java | 2 +- .../auth2/service/api/TokenEndpointTest.java | 8 +++--- 12 files changed, 60 insertions(+), 60 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index aa16f8c6..2a265428 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -3,7 +3,7 @@ ## 0.8.0 * Added MFA (Multi-Factor Authentication) status tracking for tokens -* The `/api/V2/token` endpoint now returns an `mfaAuthenticated` field +* The `/api/V2/token` endpoint now returns an `mfa` field * ORCID provider updated to use OpenID Connect scope for MFA detection ## 0.7.2 diff --git a/src/main/java/us/kbase/auth2/lib/Authentication.java b/src/main/java/us/kbase/auth2/lib/Authentication.java index b814e2c5..535f21a8 100644 --- a/src/main/java/us/kbase/auth2/lib/Authentication.java +++ b/src/main/java/us/kbase/auth2/lib/Authentication.java @@ -750,13 +750,13 @@ private NewToken login(final UserName userName, final TokenCreationContext token } private NewToken login(final UserName userName, final TokenCreationContext tokenCtx, - final MfaStatus mfaAuthenticated) throws AuthStorageException { + final MfaStatus mfa) throws AuthStorageException { final NewToken nt = new NewToken(StoredToken.getBuilder( TokenType.LOGIN, randGen.randomUUID(), userName) .withLifeTime(clock.instant(), cfg.getAppConfig().getTokenLifetimeMS(TokenLifetimeType.LOGIN)) .withContext(tokenCtx) - .withMfaAuthenticated(mfaAuthenticated) + .withMfa(mfa) .build(), randGen.getToken()); storage.storeToken(nt.getStoredToken(), nt.getTokenHash()); @@ -2350,7 +2350,7 @@ public NewToken login( } } final MfaStatus mfaStatus = ri.get().getDetails() != null ? - ri.get().getDetails().getMfaAuthenticated() : MfaStatus.UNKNOWN; + ri.get().getDetails().getMfa() : MfaStatus.UNKNOWN; return login(u.get().getUserName(), tokenCtx, mfaStatus); } diff --git a/src/main/java/us/kbase/auth2/lib/identity/RemoteIdentityDetails.java b/src/main/java/us/kbase/auth2/lib/identity/RemoteIdentityDetails.java index d6fdc667..96382c8c 100644 --- a/src/main/java/us/kbase/auth2/lib/identity/RemoteIdentityDetails.java +++ b/src/main/java/us/kbase/auth2/lib/identity/RemoteIdentityDetails.java @@ -10,7 +10,7 @@ public class RemoteIdentityDetails { private final String username; private final String fullname; private final String email; - private final MfaStatus mfaAuthenticated; + private final MfaStatus mfa; /** Create a new set of details. * @param username the user name of the identity. @@ -28,13 +28,13 @@ public RemoteIdentityDetails( * @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 the multi-factor authentication status. + * @param mfa the multi-factor authentication status. */ public RemoteIdentityDetails( final String username, final String fullname, final String email, - final MfaStatus mfaAuthenticated) { + final MfaStatus mfa) { super(); if (username == null || username.trim().isEmpty()) { throw new IllegalArgumentException( @@ -51,7 +51,7 @@ public RemoteIdentityDetails( } else { this.email = email.trim(); } - this.mfaAuthenticated = mfaAuthenticated; + this.mfa = mfa; } /** Get the user name for the identity. @@ -77,8 +77,8 @@ public String getEmail() { /** Get the multi-factor authentication status. * @return the MFA status. */ - public MfaStatus getMfaAuthenticated() { - return mfaAuthenticated; + public MfaStatus getMfa() { + return mfa; } @Override @@ -87,7 +87,7 @@ public int hashCode() { int result = 1; result = prime * result + ((email == null) ? 0 : email.hashCode()); result = prime * result + ((fullname == null) ? 0 : fullname.hashCode()); - result = prime * result + mfaAuthenticated.hashCode(); + result = prime * result + mfa.hashCode(); result = prime * result + ((username == null) ? 0 : username.hashCode()); return result; } @@ -118,7 +118,7 @@ public boolean equals(Object obj) { } else if (!fullname.equals(other.fullname)) { return false; } - if (!mfaAuthenticated.equals(other.mfaAuthenticated)) { + if (!mfa.equals(other.mfa)) { return false; } if (username == null) { @@ -140,8 +140,8 @@ public String toString() { builder.append(fullname); builder.append(", email="); builder.append(email); - builder.append(", mfaAuthenticated="); - builder.append(mfaAuthenticated); + builder.append(", mfa="); + builder.append(mfa); builder.append("]"); return builder.toString(); } diff --git a/src/main/java/us/kbase/auth2/lib/storage/mongo/Fields.java b/src/main/java/us/kbase/auth2/lib/storage/mongo/Fields.java index 15b73991..0b20822d 100644 --- a/src/main/java/us/kbase/auth2/lib/storage/mongo/Fields.java +++ b/src/main/java/us/kbase/auth2/lib/storage/mongo/Fields.java @@ -138,7 +138,7 @@ public class Fields { /** 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 = "mfa"; + public static final String TOKEN_MFA = "mfa"; /* ************************ * temporary session data fields diff --git a/src/main/java/us/kbase/auth2/lib/storage/mongo/MongoStorage.java b/src/main/java/us/kbase/auth2/lib/storage/mongo/MongoStorage.java index 533c4352..848b38be 100644 --- a/src/main/java/us/kbase/auth2/lib/storage/mongo/MongoStorage.java +++ b/src/main/java/us/kbase/auth2/lib/storage/mongo/MongoStorage.java @@ -873,7 +873,7 @@ private void storeToken(final String collection, final StoredToken token, final .append(Fields.TOKEN_IP, ctx.getIpAddress().isPresent() ? ctx.getIpAddress().get().getHostAddress() : null) .append(Fields.TOKEN_CUSTOM_CONTEXT, toCustomContextList(ctx.getCustomContext())) - .append(Fields.TOKEN_MFA_AUTHENTICATED, token.getMfaAuthenticated().name()); + .append(Fields.TOKEN_MFA, token.getMfa().name()); try { db.getCollection(collection).insertOne(td); } catch (MongoWriteException mwe) { @@ -971,7 +971,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(MfaStatus.valueOf(t.getString(Fields.TOKEN_MFA_AUTHENTICATED))) + .withMfa(MfaStatus.valueOf(t.getString(Fields.TOKEN_MFA))) .build(); } @@ -1600,7 +1600,7 @@ private void updateIdentity(final RemoteIdentity remoteID) 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_MFA, rid.getMfaAuthenticated().name())); + .append(pre + Fields.IDENTITIES_MFA, rid.getMfa().name())); try { // id might have been unlinked, so we just assume // the update worked. If it was just unlinked we don't care. @@ -1734,7 +1734,7 @@ private Document toDocument(final RemoteIdentity id) { .append(Fields.IDENTITIES_USER, rid.getUsername()) .append(Fields.IDENTITIES_NAME, rid.getFullname()) .append(Fields.IDENTITIES_EMAIL, rid.getEmail()) - .append(Fields.IDENTITIES_MFA, rid.getMfaAuthenticated().name()); + .append(Fields.IDENTITIES_MFA, rid.getMfa().name()); } @Override diff --git a/src/main/java/us/kbase/auth2/lib/token/StoredToken.java b/src/main/java/us/kbase/auth2/lib/token/StoredToken.java index b07cdaa8..ef2447bb 100644 --- a/src/main/java/us/kbase/auth2/lib/token/StoredToken.java +++ b/src/main/java/us/kbase/auth2/lib/token/StoredToken.java @@ -24,7 +24,7 @@ public class StoredToken { private final UserName userName; private final Instant creationDate; private final Instant expirationDate; - private final MfaStatus mfaAuthenticated; + private final MfaStatus mfa; private StoredToken( final UUID id, @@ -34,7 +34,7 @@ private StoredToken( final TokenCreationContext context, final Instant creationDate, final Instant expirationDate, - final MfaStatus mfaAuthenticated) { + final MfaStatus mfa) { // 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 @@ -46,7 +46,7 @@ private StoredToken( this.expirationDate = expirationDate; this.creationDate = creationDate; this.id = id; - this.mfaAuthenticated = mfaAuthenticated; + this.mfa = mfa; } /** Get the type of the token. @@ -101,8 +101,8 @@ public Instant getExpirationDate() { /** Get the multi-factor authentication status for this token. * @return the MFA status. */ - public MfaStatus getMfaAuthenticated() { - return mfaAuthenticated; + public MfaStatus getMfa() { + return mfa; } @Override @@ -116,7 +116,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.hashCode(); + result = prime * result + mfa.hashCode(); return result; } @@ -177,7 +177,7 @@ public boolean equals(Object obj) { } else if (!userName.equals(other.userName)) { return false; } - if (!mfaAuthenticated.equals(other.mfaAuthenticated)) { + if (!mfa.equals(other.mfa)) { return false; } return true; @@ -240,10 +240,10 @@ public interface OptionalsStep { OptionalsStep withContext(TokenCreationContext context); /** Specify whether the token was created with multi-factor authentication. - * @param mfaAuthenticated the MFA status. + * @param mfa the MFA status. * @return this builder. */ - OptionalsStep withMfaAuthenticated(MfaStatus mfaAuthenticated); + OptionalsStep withMfa(MfaStatus mfa); /** Build the token. * @return a new StoredToken. @@ -260,7 +260,7 @@ private static class Builder implements LifeStep, OptionalsStep { private final UserName userName; private Instant creationDate; private Instant expirationDate; - private MfaStatus mfaAuthenticated = MfaStatus.UNKNOWN; + private MfaStatus mfa = MfaStatus.UNKNOWN; private Builder(final TokenType type, final UUID id, final UserName userName) { requireNonNull(type, "type"); @@ -292,15 +292,15 @@ public OptionalsStep withContext(final TokenCreationContext context) { } @Override - public OptionalsStep withMfaAuthenticated(final MfaStatus mfaAuthenticated) { - this.mfaAuthenticated = mfaAuthenticated; + public OptionalsStep withMfa(final MfaStatus mfa) { + this.mfa = mfa; return this; } @Override public StoredToken build() { return new StoredToken(id, type, tokenName, userName, context, - creationDate, expirationDate, mfaAuthenticated); + creationDate, expirationDate, mfa); } @Override diff --git a/src/main/java/us/kbase/auth2/providers/OrcIDIdentityProviderFactory.java b/src/main/java/us/kbase/auth2/providers/OrcIDIdentityProviderFactory.java index 72375e83..268d4bad 100644 --- a/src/main/java/us/kbase/auth2/providers/OrcIDIdentityProviderFactory.java +++ b/src/main/java/us/kbase/auth2/providers/OrcIDIdentityProviderFactory.java @@ -177,7 +177,7 @@ private RemoteIdentity getIdentity(final OrcIDAccessTokenResponse accessToken) accessToken.orcID, accessToken.fullName, email, - accessToken.mfaAuthenticated)); + accessToken.mfa)); } private Map orcIDGetRequest( @@ -215,7 +215,7 @@ private static class OrcIDAccessTokenResponse { private final String fullName; private final String orcID; private final String idToken; - private final MfaStatus mfaAuthenticated; + private final MfaStatus mfa; private OrcIDAccessTokenResponse( final String accessToken, @@ -235,7 +235,7 @@ private OrcIDAccessTokenResponse( this.fullName = fullName == null ? null : fullName.trim(); this.orcID = orcID.trim(); this.idToken = idToken == null ? null : idToken.trim(); - this.mfaAuthenticated = parseAmrClaim(this.idToken); + this.mfa = parseAmrClaim(this.idToken); } /** diff --git a/src/main/java/us/kbase/auth2/service/api/APIToken.java b/src/main/java/us/kbase/auth2/service/api/APIToken.java index 1b51740a..2f72014d 100644 --- a/src/main/java/us/kbase/auth2/service/api/APIToken.java +++ b/src/main/java/us/kbase/auth2/service/api/APIToken.java @@ -9,12 +9,12 @@ public class APIToken extends ExternalToken { //TODO JAVADOC or swagger private final long cachefor; - private final MfaStatus mfaAuthenticated; + private final MfaStatus mfa; public APIToken(final StoredToken token, final long tokenCacheTimeMillis) { super(token); cachefor = tokenCacheTimeMillis; - mfaAuthenticated = token.getMfaAuthenticated(); + mfa = token.getMfa(); } public long getCachefor() { @@ -26,8 +26,8 @@ public long getCachefor() { * * @return the MFA authentication status. */ - public MfaStatus getMfaAuthenticated() { - return mfaAuthenticated; + public MfaStatus getMfa() { + return mfa; } @Override @@ -35,7 +35,7 @@ public int hashCode() { final int prime = 31; int result = super.hashCode(); result = prime * result + (int) (cachefor ^ (cachefor >>> 32)); - result = prime * result + mfaAuthenticated.hashCode(); + result = prime * result + mfa.hashCode(); return result; } @@ -51,7 +51,7 @@ public boolean equals(Object obj) { if (cachefor != other.cachefor) { return false; } - if (!mfaAuthenticated.equals(other.mfaAuthenticated)) { + if (!mfa.equals(other.mfa)) { return false; } return true; diff --git a/src/test/java/us/kbase/test/auth2/lib/identity/RemoteIdentityTest.java b/src/test/java/us/kbase/test/auth2/lib/identity/RemoteIdentityTest.java index 7336c56f..88a5be99 100644 --- a/src/test/java/us/kbase/test/auth2/lib/identity/RemoteIdentityTest.java +++ b/src/test/java/us/kbase/test/auth2/lib/identity/RemoteIdentityTest.java @@ -21,10 +21,10 @@ public void remoteDetailsWithAllFields() throws Exception { assertThat("incorrect username", dets.getUsername(), is("user")); assertThat("incorrect fullname", dets.getFullname(), is("full")); assertThat("incorrect email", dets.getEmail(), is("email")); - assertThat("incorrect mfa authenticated", dets.getMfaAuthenticated(), is(MfaStatus.UNKNOWN)); + assertThat("incorrect mfa authenticated", dets.getMfa(), is(MfaStatus.UNKNOWN)); assertThat("incorrect hashcode", dets.hashCode(), is(-497844993)); assertThat("incorrect toString()", dets.toString(), - is("RemoteIdentityDetails [username=user, fullname=full, email=email, mfaAuthenticated=UNKNOWN]")); + is("RemoteIdentityDetails [username=user, fullname=full, email=email, mfa=UNKNOWN]")); } @Test @@ -33,19 +33,19 @@ public void remoteDetailsWithEmptyFields() throws Exception { assertThat("incorrect username", dets.getUsername(), is("user")); assertThat("incorrect fullname", dets.getFullname(), is((String) null)); assertThat("incorrect email", dets.getEmail(), is((String) null)); - assertThat("incorrect mfa authenticated", dets.getMfaAuthenticated(), is(MfaStatus.UNKNOWN)); + assertThat("incorrect mfa authenticated", dets.getMfa(), is(MfaStatus.UNKNOWN)); assertThat("incorrect hashcode", dets.hashCode(), is(4522828)); assertThat("incorrect toString()", dets.toString(), - is("RemoteIdentityDetails [username=user, fullname=null, email=null, mfaAuthenticated=UNKNOWN]")); + is("RemoteIdentityDetails [username=user, fullname=null, email=null, mfa=UNKNOWN]")); final RemoteIdentityDetails dets2 = new RemoteIdentityDetails("user", null, null); assertThat("incorrect username", dets2.getUsername(), is("user")); assertThat("incorrect fullname", dets2.getFullname(), is((String) null)); assertThat("incorrect email", dets2.getEmail(), is((String) null)); - assertThat("incorrect mfa authenticated", dets2.getMfaAuthenticated(), is(MfaStatus.UNKNOWN)); + assertThat("incorrect mfa authenticated", dets2.getMfa(), is(MfaStatus.UNKNOWN)); assertThat("incorrect hashcode", dets2.hashCode(), is(4522828)); assertThat("incorrect toString()", dets2.toString(), - is("RemoteIdentityDetails [username=user, fullname=null, email=null, mfaAuthenticated=UNKNOWN]")); + is("RemoteIdentityDetails [username=user, fullname=null, email=null, mfa=UNKNOWN]")); } @Test @@ -119,7 +119,7 @@ public void identity() throws Exception { assertThat("incorrect hashcode", ri.hashCode(), is(124952370)); assertThat("incorrect toString()", ri.toString(), is("RemoteIdentity [remoteID=RemoteIdentityID [provider=p, id=i], " + - "details=RemoteIdentityDetails [username=u, fullname=f, email=e, mfaAuthenticated=UNKNOWN]]")); + "details=RemoteIdentityDetails [username=u, fullname=f, email=e, mfa=UNKNOWN]]")); } @Test @@ -151,9 +151,9 @@ public void remoteDetailsWithMfaTrue() throws Exception { assertThat("incorrect username", dets.getUsername(), is("user")); assertThat("incorrect fullname", dets.getFullname(), is("full")); assertThat("incorrect email", dets.getEmail(), is("email")); - assertThat("incorrect mfa authenticated", dets.getMfaAuthenticated(), is(MfaStatus.USED)); + assertThat("incorrect mfa authenticated", dets.getMfa(), is(MfaStatus.USED)); assertThat("incorrect toString()", dets.toString(), - is("RemoteIdentityDetails [username=user, fullname=full, email=email, mfaAuthenticated=USED]")); + is("RemoteIdentityDetails [username=user, fullname=full, email=email, mfa=USED]")); } @Test @@ -162,9 +162,9 @@ public void remoteDetailsWithMfaFalse() throws Exception { assertThat("incorrect username", dets.getUsername(), is("user")); assertThat("incorrect fullname", dets.getFullname(), is("full")); assertThat("incorrect email", dets.getEmail(), is("email")); - assertThat("incorrect mfa authenticated", dets.getMfaAuthenticated(), is(MfaStatus.NOT_USED)); + assertThat("incorrect mfa authenticated", dets.getMfa(), is(MfaStatus.NOT_USED)); assertThat("incorrect toString()", dets.toString(), - is("RemoteIdentityDetails [username=user, fullname=full, email=email, mfaAuthenticated=NOT_USED]")); + is("RemoteIdentityDetails [username=user, fullname=full, email=email, mfa=NOT_USED]")); } @Test @@ -173,9 +173,9 @@ public void remoteDetailsWithMfaNull() throws Exception { assertThat("incorrect username", dets.getUsername(), is("user")); assertThat("incorrect fullname", dets.getFullname(), is("full")); assertThat("incorrect email", dets.getEmail(), is("email")); - assertThat("incorrect mfa authenticated", dets.getMfaAuthenticated(), is(MfaStatus.UNKNOWN)); + assertThat("incorrect mfa authenticated", dets.getMfa(), is(MfaStatus.UNKNOWN)); assertThat("incorrect toString()", dets.toString(), - is("RemoteIdentityDetails [username=user, fullname=full, email=email, mfaAuthenticated=UNKNOWN]")); + is("RemoteIdentityDetails [username=user, fullname=full, email=email, mfa=UNKNOWN]")); } @Test diff --git a/src/test/java/us/kbase/test/auth2/service/api/APITokenTest.java b/src/test/java/us/kbase/test/auth2/service/api/APITokenTest.java index ba011126..5b78cb44 100644 --- a/src/test/java/us/kbase/test/auth2/service/api/APITokenTest.java +++ b/src/test/java/us/kbase/test/auth2/service/api/APITokenTest.java @@ -40,7 +40,7 @@ TokenType.AGENT, id, new UserName("foo")) assertThat("incorrect id", t.getId(), is(id.toString())); assertThat("incorrect cache time", t.getCachefor(), is(20000L)); - assertThat("incorrect mfa status", t.getMfaAuthenticated(), is(MfaStatus.UNKNOWN)); + assertThat("incorrect mfa status", t.getMfa(), is(MfaStatus.UNKNOWN)); } diff --git a/src/test/java/us/kbase/test/auth2/service/api/TestModeIntegrationTest.java b/src/test/java/us/kbase/test/auth2/service/api/TestModeIntegrationTest.java index 16ddce1a..3b2018c5 100644 --- a/src/test/java/us/kbase/test/auth2/service/api/TestModeIntegrationTest.java +++ b/src/test/java/us/kbase/test/auth2/service/api/TestModeIntegrationTest.java @@ -210,7 +210,7 @@ public void createAndGetToken() { expected.put("user", "whee"); expected.put("custom", Collections.emptyMap()); expected.put("cachefor", 300000); - expected.put("mfaAuthenticated", MfaStatus.UNKNOWN); + expected.put("mfa", MfaStatus.UNKNOWN); assertThat("incorrect return", response, is(expected)); diff --git a/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java b/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java index 9266a7f2..6488656e 100644 --- a/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java +++ b/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java @@ -175,7 +175,7 @@ TokenType.AGENT, id, new UserName("foo")) .with("user", "foo") .with("custom", ImmutableMap.of("whee", "whoo")) .with("cachefor", 300000) - .with("mfaAuthenticated", MfaStatus.UNKNOWN) + .with("mfa", MfaStatus.UNKNOWN) .build(); assertThat("incorrect token", response, is(expected)); @@ -221,7 +221,7 @@ userName, userUuid, new DisplayName("Test User"), inst(10000)) assertThat("incorrect response code for token1", res1.getStatus(), is(200)); @SuppressWarnings("unchecked") final Map response1 = res1.readEntity(Map.class); - assertThat("token1 should have MFA=true", response1.get("mfaAuthenticated"), is(MfaStatus.USED)); + assertThat("token1 should have MFA=true", response1.get("mfa"), is(MfaStatus.USED)); // Test token2 returns MFA=false final URI target2 = UriBuilder.fromUri(host).path("/api/V2/token").build(); @@ -232,7 +232,7 @@ userName, userUuid, new DisplayName("Test User"), inst(10000)) assertThat("incorrect response code for token2", res2.getStatus(), is(200)); @SuppressWarnings("unchecked") final Map response2 = res2.readEntity(Map.class); - assertThat("token2 should have MFA=false", response2.get("mfaAuthenticated"), is(MfaStatus.NOT_USED)); + assertThat("token2 should have MFA=false", response2.get("mfa"), is(MfaStatus.NOT_USED)); } @Test @@ -717,7 +717,7 @@ user, id, new DisplayName(displayName), inst(10000)) @SuppressWarnings("unchecked") final Map response = res.readEntity(Map.class); - assertThat("incorrect MFA status", response.get("mfaAuthenticated"), is(mfaStatus)); + assertThat("incorrect MFA status", response.get("mfa"), is(mfaStatus)); assertThat("incorrect user", response.get("user"), is(userName)); assertThat("incorrect token name", response.get("name"), is(tokenName)); } From 328704cf12a315a8c541575876207ac4b51fecae Mon Sep 17 00:00:00 2001 From: David Lyon <5115845+dauglyon@users.noreply.github.com> Date: Fri, 18 Jul 2025 14:20:42 -0700 Subject: [PATCH 20/25] fix tests --- .../auth2/providers/OrcIDIdentityProviderTest.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java b/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java index ec130de5..b793a3e0 100644 --- a/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java +++ b/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java @@ -550,7 +550,7 @@ public void getIdentityWithNoIdToken() throws Exception { assertThat("incorrect number of idents", rids.size(), is(1)); final Set expected = new HashSet<>(); expected.add(new RemoteIdentity(new RemoteIdentityID(ORCID, orcID), - new RemoteIdentityDetails(orcID, "My name", "noid@test.com", null))); + new RemoteIdentityDetails(orcID, "My name", "noid@test.com", MfaStatus.UNKNOWN))); assertThat("incorrect ident set", rids, is(expected)); } @@ -570,7 +570,7 @@ public void getIdentityWithInvalidJWT() throws Exception { assertThat("incorrect number of idents", rids.size(), is(1)); final Set expected = new HashSet<>(); expected.add(new RemoteIdentity(new RemoteIdentityID(ORCID, orcID), - new RemoteIdentityDetails(orcID, "My name", "invalid@test.com", null))); + new RemoteIdentityDetails(orcID, "My name", "invalid@test.com", MfaStatus.UNKNOWN))); assertThat("incorrect ident set", rids, is(expected)); } @@ -767,7 +767,7 @@ public void getIdentityWithMalformedJWT() throws Exception { assertThat("incorrect number of idents", rids.size(), is(1)); final Set expected = new HashSet<>(); expected.add(new RemoteIdentity(new RemoteIdentityID(ORCID, orcID), - new RemoteIdentityDetails(orcID, "My name", "malformed@test.com", null))); + new RemoteIdentityDetails(orcID, "My name", "malformed@test.com", MfaStatus.UNKNOWN))); assertThat("incorrect ident set", rids, is(expected)); } @@ -788,7 +788,7 @@ public void getIdentityWithInvalidBase64JWT() throws Exception { assertThat("incorrect number of idents", rids.size(), is(1)); final Set expected = new HashSet<>(); expected.add(new RemoteIdentity(new RemoteIdentityID(ORCID, orcID), - new RemoteIdentityDetails(orcID, "My name", "invalidb64@test.com", null))); + new RemoteIdentityDetails(orcID, "My name", "invalidb64@test.com", MfaStatus.UNKNOWN))); assertThat("incorrect ident set", rids, is(expected)); } @@ -813,7 +813,7 @@ public void getIdentityWithMalformedJSONJWT() throws Exception { assertThat("incorrect number of idents", rids.size(), is(1)); final Set expected = new HashSet<>(); expected.add(new RemoteIdentity(new RemoteIdentityID(ORCID, orcID), - new RemoteIdentityDetails(orcID, "My name", "malformedjson@test.com", null))); + new RemoteIdentityDetails(orcID, "My name", "malformedjson@test.com", MfaStatus.UNKNOWN))); assertThat("incorrect ident set", rids, is(expected)); } @@ -853,7 +853,7 @@ public void getIdentityWithNoAmrClaimJWT() throws Exception { assertThat("incorrect number of idents", rids.size(), is(1)); final Set expected = new HashSet<>(); expected.add(new RemoteIdentity(new RemoteIdentityID(ORCID, orcID), - new RemoteIdentityDetails(orcID, "My name", "noamr@test.com", null))); + new RemoteIdentityDetails(orcID, "My name", "noamr@test.com", MfaStatus.UNKNOWN))); assertThat("incorrect ident set", rids, is(expected)); } From 90ce1aff20a21ce8dcc0f02bf4a966fb1c78350c Mon Sep 17 00:00:00 2001 From: David Lyon <5115845+dauglyon@users.noreply.github.com> Date: Fri, 18 Jul 2025 14:25:56 -0700 Subject: [PATCH 21/25] rename all uses MfaAuthenticated -> mfa --- src/main/java/us/kbase/auth2/lib/Authentication.java | 2 -- .../us/kbase/test/auth2/service/api/TokenEndpointTest.java | 6 +++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/main/java/us/kbase/auth2/lib/Authentication.java b/src/main/java/us/kbase/auth2/lib/Authentication.java index 535f21a8..87e00c3e 100644 --- a/src/main/java/us/kbase/auth2/lib/Authentication.java +++ b/src/main/java/us/kbase/auth2/lib/Authentication.java @@ -913,7 +913,6 @@ public NewToken createToken( .withLifeTime(clock.instant(), life) .withContext(tokenCtx) .withTokenName(tokenName) - .withMfaAuthenticated(MfaStatus.UNKNOWN) // Agent/Dev/Serv tokens don't have MFA status .build(), randGen.getToken()); storage.storeToken(nt.getStoredToken(), nt.getTokenHash()); @@ -2052,7 +2051,6 @@ public NewToken testModeCreateToken( final NewToken nt = new NewToken(StoredToken.getBuilder(tokenType, id, userName) .withLifeTime(clock.instant(), TEST_MODE_DATA_LIFETIME_MS) .withNullableTokenName(tokenName) - .withMfaAuthenticated(MfaStatus.UNKNOWN) // Test mode tokens don't have MFA status .build(), randGen.getToken()); storage.testModeStoreToken(nt.getStoredToken(), nt.getTokenHash()); diff --git a/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java b/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java index 6488656e..29ae165b 100644 --- a/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java +++ b/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java @@ -202,14 +202,14 @@ userName, userUuid, new DisplayName("Test User"), inst(10000)) manager.storage.storeToken(StoredToken.getBuilder( TokenType.LOGIN, token1Id, userName) .withLifeTime(Instant.ofEpochMilli(10000), Instant.ofEpochMilli(1000000000000000L)) - .withMfaAuthenticated(MfaStatus.USED) + .withMfa(MfaStatus.USED) .build(), token1.getHashedToken().getTokenHash()); // Token 2 with MFA=false manager.storage.storeToken(StoredToken.getBuilder( TokenType.LOGIN, token2Id, userName) .withLifeTime(Instant.ofEpochMilli(10000), Instant.ofEpochMilli(1000000000000000L)) - .withMfaAuthenticated(MfaStatus.NOT_USED) + .withMfa(MfaStatus.NOT_USED) .build(), token2.getHashedToken().getTokenHash()); // Test token1 returns MFA=true @@ -700,7 +700,7 @@ user, id, new DisplayName(displayName), inst(10000)) .withLifeTime(Instant.ofEpochMilli(10000), Instant.ofEpochMilli(1000000000000000L)) .withTokenName(new TokenName(tokenName)); - tokenBuilder = tokenBuilder.withMfaAuthenticated(mfaStatus); + tokenBuilder = tokenBuilder.withMfa(mfaStatus); manager.storage.storeToken(tokenBuilder.build(), it.getHashedToken().getTokenHash()); From 58a2aec90c1041b91842a74d7656f3cca20ca979 Mon Sep 17 00:00:00 2001 From: David Lyon <5115845+dauglyon@users.noreply.github.com> Date: Fri, 18 Jul 2025 14:43:27 -0700 Subject: [PATCH 22/25] rename jwt field for clarity --- RELEASE_NOTES.md | 2 +- .../OrcIDIdentityProviderFactory.java | 16 +++--- .../providers/OrcIDIdentityProviderTest.java | 52 +++++++++---------- 3 files changed, 35 insertions(+), 35 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 2a265428..b9a136eb 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -1,6 +1,6 @@ # Authentication Service MKII release notes -## 0.8.0 +## 0.7.3 * Added MFA (Multi-Factor Authentication) status tracking for tokens * The `/api/V2/token` endpoint now returns an `mfa` field diff --git a/src/main/java/us/kbase/auth2/providers/OrcIDIdentityProviderFactory.java b/src/main/java/us/kbase/auth2/providers/OrcIDIdentityProviderFactory.java index 268d4bad..179db0c4 100644 --- a/src/main/java/us/kbase/auth2/providers/OrcIDIdentityProviderFactory.java +++ b/src/main/java/us/kbase/auth2/providers/OrcIDIdentityProviderFactory.java @@ -214,14 +214,14 @@ private static class OrcIDAccessTokenResponse { private final String accessToken; private final String fullName; private final String orcID; - private final String idToken; + private final String jwt; private final MfaStatus mfa; private OrcIDAccessTokenResponse( final String accessToken, final String fullName, final String orcID, - final String idToken) + final String jwt) throws IdentityRetrievalException { if (accessToken == null || accessToken.trim().isEmpty()) { throw new IdentityRetrievalException( @@ -234,25 +234,25 @@ private OrcIDAccessTokenResponse( this.accessToken = accessToken.trim(); this.fullName = fullName == null ? null : fullName.trim(); this.orcID = orcID.trim(); - this.idToken = idToken == null ? null : idToken.trim(); - this.mfa = parseAmrClaim(this.idToken); + this.jwt = jwt == null ? null : jwt.trim(); + this.mfa = parseAmrClaim(this.jwt); } /** * Parses the Authentication Method Reference (AMR) claim from an OpenID Connect ID token * to determine if multi-factor authentication was used. * - * @param idToken the JWT ID token from ORCID + * @param jwt the JWT ID token from ORCID * @return MfaStatus indicating whether MFA was used */ - private MfaStatus parseAmrClaim(final String idToken) { - if (idToken == null || idToken.trim().isEmpty()) { + private MfaStatus parseAmrClaim(final String jwt) { + if (jwt == null || jwt.trim().isEmpty()) { return MfaStatus.UNKNOWN; } try { // JWT format: header.payload.signature - final String[] parts = idToken.split("\\."); + final String[] parts = jwt.split("\\."); if (parts.length != 3) { // Invalid JWT format return MfaStatus.UNKNOWN; diff --git a/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java b/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java index b793a3e0..81ebc0d6 100644 --- a/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java +++ b/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java @@ -501,10 +501,10 @@ public void getIdentityWithMfaTrue() throws Exception { final IdentityProviderConfig idconfig = getTestIDConfig(); final IdentityProvider idp = new OrcIDIdentityProvider(idconfig); final String orcID = "0000-0001-1234-5678"; - final String idToken = createValidJWTWithMfa(orcID, true); + final String jwt = createValidJWTWithMfa(orcID, true); - setUpCallAuthTokenWithIdToken(authCode, "footoken3", "https://ologinredir.com", - idconfig.getClientID(), idconfig.getClientSecret(), " My name ", orcID, idToken); + setUpCallAuthTokenWithJWT(authCode, "footoken3", "https://ologinredir.com", + idconfig.getClientID(), idconfig.getClientSecret(), " My name ", orcID, jwt); setupCallID("footoken3", orcID, APP_JSON, 200, MAPPER.writeValueAsString( map("email", Arrays.asList(map("email", "mfa@test.com"))))); final Set rids = idp.getIdentities(authCode, "pkce", false, null); @@ -521,10 +521,10 @@ public void getIdentityWithMfaFalse() throws Exception { final IdentityProviderConfig idconfig = getTestIDConfig(); final IdentityProvider idp = new OrcIDIdentityProvider(idconfig); final String orcID = "0000-0001-1234-5678"; - final String idToken = createValidJWTWithMfa(orcID, false); + final String jwt = createValidJWTWithMfa(orcID, false); - setUpCallAuthTokenWithIdToken(authCode, "footoken4", "https://ologinredir.com", - idconfig.getClientID(), idconfig.getClientSecret(), " My name ", orcID, idToken); + setUpCallAuthTokenWithJWT(authCode, "footoken4", "https://ologinredir.com", + idconfig.getClientID(), idconfig.getClientSecret(), " My name ", orcID, jwt); setupCallID("footoken4", orcID, APP_JSON, 200, MAPPER.writeValueAsString( map("email", Arrays.asList(map("email", "nomfa@test.com"))))); final Set rids = idp.getIdentities(authCode, "pkce", false, null); @@ -536,8 +536,8 @@ public void getIdentityWithMfaFalse() throws Exception { } @Test - public void getIdentityWithNoIdToken() throws Exception { - final String authCode = "authcodeNoIdToken"; + public void getIdentityWithNoJWT() throws Exception { + final String authCode = "authcodeNoJWT"; final IdentityProviderConfig idconfig = getTestIDConfig(); final IdentityProvider idp = new OrcIDIdentityProvider(idconfig); final String orcID = "0000-0001-1234-5678"; @@ -560,10 +560,10 @@ public void getIdentityWithInvalidJWT() throws Exception { final IdentityProviderConfig idconfig = getTestIDConfig(); final IdentityProvider idp = new OrcIDIdentityProvider(idconfig); final String orcID = "0000-0001-1234-5678"; - final String invalidIdToken = "invalid.jwt.token"; + final String invalidJWT = "invalid.jwt.token"; - setUpCallAuthTokenWithIdToken(authCode, "footoken6", "https://ologinredir.com", - idconfig.getClientID(), idconfig.getClientSecret(), " My name ", orcID, invalidIdToken); + setUpCallAuthTokenWithJWT(authCode, "footoken6", "https://ologinredir.com", + idconfig.getClientID(), idconfig.getClientSecret(), " My name ", orcID, invalidJWT); setupCallID("footoken6", orcID, APP_JSON, 200, MAPPER.writeValueAsString( map("email", Arrays.asList(map("email", "invalid@test.com"))))); final Set rids = idp.getIdentities(authCode, "pkce", false, null); @@ -580,10 +580,10 @@ public void getIdentityWithStringAmr() throws Exception { final IdentityProviderConfig idconfig = getTestIDConfig(); final IdentityProvider idp = new OrcIDIdentityProvider(idconfig); final String orcID = "0000-0001-1234-5678"; - final String idToken = createJWTWithStringAmr(orcID, "mfa"); + final String jwt = createJWTWithStringAmr(orcID, "mfa"); - setUpCallAuthTokenWithIdToken(authCode, "footoken7", "https://ologinredir.com", - idconfig.getClientID(), idconfig.getClientSecret(), " My name ", orcID, idToken); + setUpCallAuthTokenWithJWT(authCode, "footoken7", "https://ologinredir.com", + idconfig.getClientID(), idconfig.getClientSecret(), " My name ", orcID, jwt); setupCallID("footoken7", orcID, APP_JSON, 200, MAPPER.writeValueAsString( map("email", Arrays.asList(map("email", "stringmfa@test.com"))))); final Set rids = idp.getIdentities(authCode, "pkce", false, null); @@ -692,7 +692,7 @@ private Map map(final Object... entries) { return ret; } - private void setUpCallAuthTokenWithIdToken( + private void setUpCallAuthTokenWithJWT( final String authCode, final String authtoken, final String redirect, @@ -700,7 +700,7 @@ private void setUpCallAuthTokenWithIdToken( final String clientSecret, final String name, final String orcID, - final String idToken) + final String jwt) throws Exception { mockClientAndServer.when( new HttpRequest() @@ -723,7 +723,7 @@ private void setUpCallAuthTokenWithIdToken( "access_token", authtoken, "name", name, "orcid", orcID, - "id_token", idToken + "id_token", jwt ))) ); } @@ -759,7 +759,7 @@ public void getIdentityWithMalformedJWT() throws Exception { // Test JWT with only 2 parts final String invalidJWT = "header.payload"; - setUpCallAuthTokenWithIdToken(authCode, "footoken8", "https://ologinredir.com", + setUpCallAuthTokenWithJWT(authCode, "footoken8", "https://ologinredir.com", idconfig.getClientID(), idconfig.getClientSecret(), " My name ", orcID, invalidJWT); setupCallID("footoken8", orcID, APP_JSON, 200, MAPPER.writeValueAsString( map("email", Arrays.asList(map("email", "malformed@test.com"))))); @@ -780,7 +780,7 @@ public void getIdentityWithInvalidBase64JWT() throws Exception { // JWT with invalid base64 in payload final String invalidJWT = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.invalid==base64!!.signature"; - setUpCallAuthTokenWithIdToken(authCode, "footoken9", "https://ologinredir.com", + setUpCallAuthTokenWithJWT(authCode, "footoken9", "https://ologinredir.com", idconfig.getClientID(), idconfig.getClientSecret(), " My name ", orcID, invalidJWT); setupCallID("footoken9", orcID, APP_JSON, 200, MAPPER.writeValueAsString( map("email", Arrays.asList(map("email", "invalidb64@test.com"))))); @@ -805,7 +805,7 @@ public void getIdentityWithMalformedJSONJWT() throws Exception { .encodeToString(invalidJSON.getBytes()); final String invalidJWT = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9." + encodedPayload + ".signature"; - setUpCallAuthTokenWithIdToken(authCode, "footoken10", "https://ologinredir.com", + setUpCallAuthTokenWithJWT(authCode, "footoken10", "https://ologinredir.com", idconfig.getClientID(), idconfig.getClientSecret(), " My name ", orcID, invalidJWT); setupCallID("footoken10", orcID, APP_JSON, 200, MAPPER.writeValueAsString( map("email", Arrays.asList(map("email", "malformedjson@test.com"))))); @@ -823,10 +823,10 @@ public void getIdentityWithEmptyAmrJWT() throws Exception { final IdentityProviderConfig idconfig = getTestIDConfig(); final IdentityProvider idp = new OrcIDIdentityProvider(idconfig); final String orcID = "0000-0001-1234-5678"; - final String idToken = createJWTWithEmptyAmr(orcID); + final String jwt = createJWTWithEmptyAmr(orcID); - setUpCallAuthTokenWithIdToken(authCode, "footoken11", "https://ologinredir.com", - idconfig.getClientID(), idconfig.getClientSecret(), " My name ", orcID, idToken); + setUpCallAuthTokenWithJWT(authCode, "footoken11", "https://ologinredir.com", + idconfig.getClientID(), idconfig.getClientSecret(), " My name ", orcID, jwt); setupCallID("footoken11", orcID, APP_JSON, 200, MAPPER.writeValueAsString( map("email", Arrays.asList(map("email", "emptyamr@test.com"))))); final Set rids = idp.getIdentities(authCode, "pkce", false, null); @@ -843,10 +843,10 @@ public void getIdentityWithNoAmrClaimJWT() throws Exception { final IdentityProviderConfig idconfig = getTestIDConfig(); final IdentityProvider idp = new OrcIDIdentityProvider(idconfig); final String orcID = "0000-0001-1234-5678"; - final String idToken = createJWTWithoutAmr(orcID); + final String jwt = createJWTWithoutAmr(orcID); - setUpCallAuthTokenWithIdToken(authCode, "footoken12", "https://ologinredir.com", - idconfig.getClientID(), idconfig.getClientSecret(), " My name ", orcID, idToken); + setUpCallAuthTokenWithJWT(authCode, "footoken12", "https://ologinredir.com", + idconfig.getClientID(), idconfig.getClientSecret(), " My name ", orcID, jwt); setupCallID("footoken12", orcID, APP_JSON, 200, MAPPER.writeValueAsString( map("email", Arrays.asList(map("email", "noamr@test.com"))))); final Set rids = idp.getIdentities(authCode, "pkce", false, null); From c7667049aa9934961ac314a7aafda1fa26ca84c2 Mon Sep 17 00:00:00 2001 From: David Lyon <5115845+dauglyon@users.noreply.github.com> Date: Fri, 18 Jul 2025 14:50:16 -0700 Subject: [PATCH 23/25] fix tests, equalities --- .../kbase/auth2/lib/identity/RemoteIdentityDetails.java | 8 ++++++-- src/main/java/us/kbase/auth2/lib/token/StoredToken.java | 8 ++++++-- src/main/java/us/kbase/auth2/service/api/APIToken.java | 8 ++++++-- .../test/auth2/service/api/TestModeIntegrationTest.java | 2 +- .../kbase/test/auth2/service/api/TokenEndpointTest.java | 6 +++--- 5 files changed, 22 insertions(+), 10 deletions(-) diff --git a/src/main/java/us/kbase/auth2/lib/identity/RemoteIdentityDetails.java b/src/main/java/us/kbase/auth2/lib/identity/RemoteIdentityDetails.java index 96382c8c..8062456c 100644 --- a/src/main/java/us/kbase/auth2/lib/identity/RemoteIdentityDetails.java +++ b/src/main/java/us/kbase/auth2/lib/identity/RemoteIdentityDetails.java @@ -87,7 +87,7 @@ public int hashCode() { int result = 1; result = prime * result + ((email == null) ? 0 : email.hashCode()); result = prime * result + ((fullname == null) ? 0 : fullname.hashCode()); - result = prime * result + mfa.hashCode(); + result = prime * result + ((mfa == null) ? 0 : mfa.hashCode()); result = prime * result + ((username == null) ? 0 : username.hashCode()); return result; } @@ -118,7 +118,11 @@ public boolean equals(Object obj) { } else if (!fullname.equals(other.fullname)) { return false; } - if (!mfa.equals(other.mfa)) { + if (mfa == null) { + if (other.mfa != null) { + return false; + } + } else if (!mfa.equals(other.mfa)) { return false; } if (username == null) { diff --git a/src/main/java/us/kbase/auth2/lib/token/StoredToken.java b/src/main/java/us/kbase/auth2/lib/token/StoredToken.java index ef2447bb..6ab1438a 100644 --- a/src/main/java/us/kbase/auth2/lib/token/StoredToken.java +++ b/src/main/java/us/kbase/auth2/lib/token/StoredToken.java @@ -116,7 +116,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 + mfa.hashCode(); + result = prime * result + ((mfa == null) ? 0 : mfa.hashCode()); return result; } @@ -177,7 +177,11 @@ public boolean equals(Object obj) { } else if (!userName.equals(other.userName)) { return false; } - if (!mfa.equals(other.mfa)) { + if (mfa == null) { + if (other.mfa != null) { + return false; + } + } else if (!mfa.equals(other.mfa)) { return false; } return true; diff --git a/src/main/java/us/kbase/auth2/service/api/APIToken.java b/src/main/java/us/kbase/auth2/service/api/APIToken.java index 2f72014d..8afb3c21 100644 --- a/src/main/java/us/kbase/auth2/service/api/APIToken.java +++ b/src/main/java/us/kbase/auth2/service/api/APIToken.java @@ -35,7 +35,7 @@ public int hashCode() { final int prime = 31; int result = super.hashCode(); result = prime * result + (int) (cachefor ^ (cachefor >>> 32)); - result = prime * result + mfa.hashCode(); + result = prime * result + ((mfa == null) ? 0 : mfa.hashCode()); return result; } @@ -51,7 +51,11 @@ public boolean equals(Object obj) { if (cachefor != other.cachefor) { return false; } - if (!mfa.equals(other.mfa)) { + if (mfa == null) { + if (other.mfa != null) { + return false; + } + } else if (!mfa.equals(other.mfa)) { return false; } return true; diff --git a/src/test/java/us/kbase/test/auth2/service/api/TestModeIntegrationTest.java b/src/test/java/us/kbase/test/auth2/service/api/TestModeIntegrationTest.java index 3b2018c5..e0705214 100644 --- a/src/test/java/us/kbase/test/auth2/service/api/TestModeIntegrationTest.java +++ b/src/test/java/us/kbase/test/auth2/service/api/TestModeIntegrationTest.java @@ -210,7 +210,7 @@ public void createAndGetToken() { expected.put("user", "whee"); expected.put("custom", Collections.emptyMap()); expected.put("cachefor", 300000); - expected.put("mfa", MfaStatus.UNKNOWN); + expected.put("mfa", MfaStatus.UNKNOWN.toString()); assertThat("incorrect return", response, is(expected)); diff --git a/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java b/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java index 29ae165b..1609d5a4 100644 --- a/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java +++ b/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java @@ -175,7 +175,7 @@ TokenType.AGENT, id, new UserName("foo")) .with("user", "foo") .with("custom", ImmutableMap.of("whee", "whoo")) .with("cachefor", 300000) - .with("mfa", MfaStatus.UNKNOWN) + .with("mfa", MfaStatus.UNKNOWN.toString()) .build(); assertThat("incorrect token", response, is(expected)); @@ -221,7 +221,7 @@ userName, userUuid, new DisplayName("Test User"), inst(10000)) assertThat("incorrect response code for token1", res1.getStatus(), is(200)); @SuppressWarnings("unchecked") final Map response1 = res1.readEntity(Map.class); - assertThat("token1 should have MFA=true", response1.get("mfa"), is(MfaStatus.USED)); + assertThat("token1 should have MFA=true", response1.get("mfa"), is(MfaStatus.USED.toString())); // Test token2 returns MFA=false final URI target2 = UriBuilder.fromUri(host).path("/api/V2/token").build(); @@ -717,7 +717,7 @@ user, id, new DisplayName(displayName), inst(10000)) @SuppressWarnings("unchecked") final Map response = res.readEntity(Map.class); - assertThat("incorrect MFA status", response.get("mfa"), is(mfaStatus)); + assertThat("incorrect MFA status", response.get("mfa"), is(mfaStatus.toString())); assertThat("incorrect user", response.get("user"), is(userName)); assertThat("incorrect token name", response.get("name"), is(tokenName)); } From 37d2ef60ffb7c0a88e2f62d40f4b31f620cfb6d7 Mon Sep 17 00:00:00 2001 From: David Lyon <5115845+dauglyon@users.noreply.github.com> Date: Fri, 18 Jul 2025 15:12:17 -0700 Subject: [PATCH 24/25] throw errors for jwt parsing issues --- .../us/kbase/auth2/lib/Authentication.java | 3 +- .../OrcIDIdentityProviderFactory.java | 13 +++-- .../providers/OrcIDIdentityProviderTest.java | 57 +++++++++++-------- 3 files changed, 42 insertions(+), 31 deletions(-) diff --git a/src/main/java/us/kbase/auth2/lib/Authentication.java b/src/main/java/us/kbase/auth2/lib/Authentication.java index 87e00c3e..7ee458ea 100644 --- a/src/main/java/us/kbase/auth2/lib/Authentication.java +++ b/src/main/java/us/kbase/auth2/lib/Authentication.java @@ -2347,8 +2347,7 @@ public NewToken login( linked, u.get().getUserName().getName()); } } - final MfaStatus mfaStatus = ri.get().getDetails() != null ? - ri.get().getDetails().getMfa() : MfaStatus.UNKNOWN; + final MfaStatus mfaStatus = ri.get().getDetails().getMfa(); return login(u.get().getUserName(), tokenCtx, mfaStatus); } diff --git a/src/main/java/us/kbase/auth2/providers/OrcIDIdentityProviderFactory.java b/src/main/java/us/kbase/auth2/providers/OrcIDIdentityProviderFactory.java index 179db0c4..0b3a4bf4 100644 --- a/src/main/java/us/kbase/auth2/providers/OrcIDIdentityProviderFactory.java +++ b/src/main/java/us/kbase/auth2/providers/OrcIDIdentityProviderFactory.java @@ -27,6 +27,8 @@ import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; +import org.slf4j.LoggerFactory; + import us.kbase.auth2.lib.exceptions.IdentityRetrievalException; import us.kbase.auth2.lib.exceptions.NoSuchEnvironmentException; import us.kbase.auth2.lib.identity.IdentityProvider; @@ -245,8 +247,9 @@ private OrcIDAccessTokenResponse( * @param jwt the JWT ID token from ORCID * @return MfaStatus indicating whether MFA was used */ - private MfaStatus parseAmrClaim(final String jwt) { + private MfaStatus parseAmrClaim(final String jwt) throws IdentityRetrievalException { if (jwt == null || jwt.trim().isEmpty()) { + LoggerFactory.getLogger(getClass()).info("No JWT token provided by ORCID, MFA status unknown"); return MfaStatus.UNKNOWN; } @@ -255,7 +258,7 @@ private MfaStatus parseAmrClaim(final String jwt) { final String[] parts = jwt.split("\\."); if (parts.length != 3) { // Invalid JWT format - return MfaStatus.UNKNOWN; + throw new IdentityRetrievalException("Invalid JWT format from ORCID: expected 3 parts, got " + parts.length); } // Decode the payload (second part) - URL-safe base64 @@ -277,14 +280,14 @@ private MfaStatus parseAmrClaim(final String jwt) { } // AMR claim present but in unexpected format - return MfaStatus.UNKNOWN; + throw new IdentityRetrievalException("AMR claim from ORCID in unexpected format: " + amrClaim); } catch (IllegalArgumentException e) { // Base64 decoding failed - invalid JWT - return MfaStatus.UNKNOWN; + throw new IdentityRetrievalException("Unable to decode JWT from ORCID: " + e.getMessage(), e); } catch (IOException e) { // JSON parsing failed - malformed payload - return MfaStatus.UNKNOWN; + throw new IdentityRetrievalException("Unable to parse JWT payload from ORCID: " + e.getMessage(), e); } } } diff --git a/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java b/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java index 81ebc0d6..d4728e2a 100644 --- a/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java +++ b/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java @@ -1,5 +1,6 @@ package us.kbase.test.auth2.providers; +import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; @@ -566,12 +567,14 @@ public void getIdentityWithInvalidJWT() throws Exception { idconfig.getClientID(), idconfig.getClientSecret(), " My name ", orcID, invalidJWT); setupCallID("footoken6", orcID, APP_JSON, 200, MAPPER.writeValueAsString( map("email", Arrays.asList(map("email", "invalid@test.com"))))); - final Set rids = idp.getIdentities(authCode, "pkce", false, null); - assertThat("incorrect number of idents", rids.size(), is(1)); - final Set expected = new HashSet<>(); - expected.add(new RemoteIdentity(new RemoteIdentityID(ORCID, orcID), - new RemoteIdentityDetails(orcID, "My name", "invalid@test.com", MfaStatus.UNKNOWN))); - assertThat("incorrect ident set", rids, is(expected)); + + try { + idp.getIdentities(authCode, "pkce", false, null); + fail("Expected IdentityRetrievalException"); + } catch (IdentityRetrievalException e) { + assertThat("incorrect exception message", e.getMessage(), + containsString("Unable to decode JWT from ORCID")); + } } @Test @@ -763,12 +766,14 @@ public void getIdentityWithMalformedJWT() throws Exception { idconfig.getClientID(), idconfig.getClientSecret(), " My name ", orcID, invalidJWT); setupCallID("footoken8", orcID, APP_JSON, 200, MAPPER.writeValueAsString( map("email", Arrays.asList(map("email", "malformed@test.com"))))); - final Set rids = idp.getIdentities(authCode, "pkce", false, null); - assertThat("incorrect number of idents", rids.size(), is(1)); - final Set expected = new HashSet<>(); - expected.add(new RemoteIdentity(new RemoteIdentityID(ORCID, orcID), - new RemoteIdentityDetails(orcID, "My name", "malformed@test.com", MfaStatus.UNKNOWN))); - assertThat("incorrect ident set", rids, is(expected)); + + try { + idp.getIdentities(authCode, "pkce", false, null); + fail("Expected IdentityRetrievalException"); + } catch (IdentityRetrievalException e) { + assertThat("incorrect exception message", e.getMessage(), + containsString("Invalid JWT format from ORCID: expected 3 parts, got 2")); + } } @Test @@ -784,12 +789,14 @@ public void getIdentityWithInvalidBase64JWT() throws Exception { idconfig.getClientID(), idconfig.getClientSecret(), " My name ", orcID, invalidJWT); setupCallID("footoken9", orcID, APP_JSON, 200, MAPPER.writeValueAsString( map("email", Arrays.asList(map("email", "invalidb64@test.com"))))); - final Set rids = idp.getIdentities(authCode, "pkce", false, null); - assertThat("incorrect number of idents", rids.size(), is(1)); - final Set expected = new HashSet<>(); - expected.add(new RemoteIdentity(new RemoteIdentityID(ORCID, orcID), - new RemoteIdentityDetails(orcID, "My name", "invalidb64@test.com", MfaStatus.UNKNOWN))); - assertThat("incorrect ident set", rids, is(expected)); + + try { + idp.getIdentities(authCode, "pkce", false, null); + fail("Expected IdentityRetrievalException"); + } catch (IdentityRetrievalException e) { + assertThat("incorrect exception message", e.getMessage(), + containsString("Unable to decode JWT from ORCID")); + } } @Test @@ -809,12 +816,14 @@ public void getIdentityWithMalformedJSONJWT() throws Exception { idconfig.getClientID(), idconfig.getClientSecret(), " My name ", orcID, invalidJWT); setupCallID("footoken10", orcID, APP_JSON, 200, MAPPER.writeValueAsString( map("email", Arrays.asList(map("email", "malformedjson@test.com"))))); - final Set rids = idp.getIdentities(authCode, "pkce", false, null); - assertThat("incorrect number of idents", rids.size(), is(1)); - final Set expected = new HashSet<>(); - expected.add(new RemoteIdentity(new RemoteIdentityID(ORCID, orcID), - new RemoteIdentityDetails(orcID, "My name", "malformedjson@test.com", MfaStatus.UNKNOWN))); - assertThat("incorrect ident set", rids, is(expected)); + + try { + idp.getIdentities(authCode, "pkce", false, null); + fail("Expected IdentityRetrievalException"); + } catch (IdentityRetrievalException e) { + assertThat("incorrect exception message", e.getMessage(), + containsString("Unable to parse JWT payload from ORCID")); + } } @Test From 62d606698da64c21c168817f4f2929cf7e821a07 Mon Sep 17 00:00:00 2001 From: David Lyon <5115845+dauglyon@users.noreply.github.com> Date: Fri, 18 Jul 2025 15:35:16 -0700 Subject: [PATCH 25/25] add check for null/missing AMR claim, fix tests --- .../auth2/providers/OrcIDIdentityProviderFactory.java | 5 ++++- .../test/auth2/lib/identity/RemoteIdentityTest.java | 8 ++++---- .../auth2/providers/OrcIDIdentityProviderTest.java | 2 +- .../test/auth2/service/api/TokenEndpointTest.java | 2 +- .../kbase/test/auth2/service/api/UserEndpointTest.java | 10 +++++----- 5 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/main/java/us/kbase/auth2/providers/OrcIDIdentityProviderFactory.java b/src/main/java/us/kbase/auth2/providers/OrcIDIdentityProviderFactory.java index 0b3a4bf4..a485eee5 100644 --- a/src/main/java/us/kbase/auth2/providers/OrcIDIdentityProviderFactory.java +++ b/src/main/java/us/kbase/auth2/providers/OrcIDIdentityProviderFactory.java @@ -269,7 +269,10 @@ private MfaStatus parseAmrClaim(final String jwt) throws IdentityRetrievalExcept final Map claims = MAPPER.readValue(payload, Map.class); final Object amrClaim = claims.get("amr"); - if (amrClaim instanceof List) { + if (amrClaim == null) { + // No AMR claim present - MFA status unknown + return MfaStatus.UNKNOWN; + } else if (amrClaim instanceof List) { // OpenID Connect spec: AMR should be an array of strings @SuppressWarnings("unchecked") final List amrList = (List) amrClaim; diff --git a/src/test/java/us/kbase/test/auth2/lib/identity/RemoteIdentityTest.java b/src/test/java/us/kbase/test/auth2/lib/identity/RemoteIdentityTest.java index 88a5be99..2707294c 100644 --- a/src/test/java/us/kbase/test/auth2/lib/identity/RemoteIdentityTest.java +++ b/src/test/java/us/kbase/test/auth2/lib/identity/RemoteIdentityTest.java @@ -22,7 +22,7 @@ public void remoteDetailsWithAllFields() throws Exception { assertThat("incorrect fullname", dets.getFullname(), is("full")); assertThat("incorrect email", dets.getEmail(), is("email")); assertThat("incorrect mfa authenticated", dets.getMfa(), is(MfaStatus.UNKNOWN)); - assertThat("incorrect hashcode", dets.hashCode(), is(-497844993)); + assertThat("incorrect hashcode", dets.hashCode(), is(1166981462)); assertThat("incorrect toString()", dets.toString(), is("RemoteIdentityDetails [username=user, fullname=full, email=email, mfa=UNKNOWN]")); } @@ -34,7 +34,7 @@ public void remoteDetailsWithEmptyFields() throws Exception { assertThat("incorrect fullname", dets.getFullname(), is((String) null)); assertThat("incorrect email", dets.getEmail(), is((String) null)); assertThat("incorrect mfa authenticated", dets.getMfa(), is(MfaStatus.UNKNOWN)); - assertThat("incorrect hashcode", dets.hashCode(), is(4522828)); + assertThat("incorrect hashcode", dets.hashCode(), is(1669349283)); assertThat("incorrect toString()", dets.toString(), is("RemoteIdentityDetails [username=user, fullname=null, email=null, mfa=UNKNOWN]")); @@ -43,7 +43,7 @@ public void remoteDetailsWithEmptyFields() throws Exception { assertThat("incorrect fullname", dets2.getFullname(), is((String) null)); assertThat("incorrect email", dets2.getEmail(), is((String) null)); assertThat("incorrect mfa authenticated", dets2.getMfa(), is(MfaStatus.UNKNOWN)); - assertThat("incorrect hashcode", dets2.hashCode(), is(4522828)); + assertThat("incorrect hashcode", dets2.hashCode(), is(1669349283)); assertThat("incorrect toString()", dets2.toString(), is("RemoteIdentityDetails [username=user, fullname=null, email=null, mfa=UNKNOWN]")); } @@ -116,7 +116,7 @@ public void identity() throws Exception { final RemoteIdentity ri = new RemoteIdentity(id, dets); assertThat("incorrect id", ri.getRemoteID(), is(id)); assertThat("incorrect details", ri.getDetails(), is(dets)); - assertThat("incorrect hashcode", ri.hashCode(), is(124952370)); + assertThat("incorrect hashcode", ri.hashCode(), is(194964923)); assertThat("incorrect toString()", ri.toString(), is("RemoteIdentity [remoteID=RemoteIdentityID [provider=p, id=i], " + "details=RemoteIdentityDetails [username=u, fullname=f, email=e, mfa=UNKNOWN]]")); diff --git a/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java b/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java index d4728e2a..7d0656a6 100644 --- a/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java +++ b/src/test/java/us/kbase/test/auth2/providers/OrcIDIdentityProviderTest.java @@ -573,7 +573,7 @@ public void getIdentityWithInvalidJWT() throws Exception { fail("Expected IdentityRetrievalException"); } catch (IdentityRetrievalException e) { assertThat("incorrect exception message", e.getMessage(), - containsString("Unable to decode JWT from ORCID")); + containsString("Unable to parse JWT payload from ORCID")); } } diff --git a/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java b/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java index 1609d5a4..f1fd2c5d 100644 --- a/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java +++ b/src/test/java/us/kbase/test/auth2/service/api/TokenEndpointTest.java @@ -232,7 +232,7 @@ userName, userUuid, new DisplayName("Test User"), inst(10000)) assertThat("incorrect response code for token2", res2.getStatus(), is(200)); @SuppressWarnings("unchecked") final Map response2 = res2.readEntity(Map.class); - assertThat("token2 should have MFA=false", response2.get("mfa"), is(MfaStatus.NOT_USED)); + assertThat("token2 should have MFA=false", response2.get("mfa"), is(MfaStatus.NOT_USED.toString())); } @Test diff --git a/src/test/java/us/kbase/test/auth2/service/api/UserEndpointTest.java b/src/test/java/us/kbase/test/auth2/service/api/UserEndpointTest.java index 0ff18ddd..08e79047 100644 --- a/src/test/java/us/kbase/test/auth2/service/api/UserEndpointTest.java +++ b/src/test/java/us/kbase/test/auth2/service/api/UserEndpointTest.java @@ -244,14 +244,14 @@ public void getMeMaximalInput() throws Exception { ImmutableMap.of("id", "Admin", "desc", "Administrator"), ImmutableMap.of("id", "DevToken", "desc", "Create developer tokens"))) .with("idents", Arrays.asList( - ImmutableMap.of( - "provider", "prov", - "provusername", "user1", - "id", "c20a5e632833ab26d99906fc9cb07d6b"), ImmutableMap.of( "provider", "prov2", "provusername", "user2", - "id", "57980b7a3440a4342567e060c3e47666"))) + "id", "57980b7a3440a4342567e060c3e47666"), + ImmutableMap.of( + "provider", "prov", + "provusername", "user1", + "id", "c20a5e632833ab26d99906fc9cb07d6b"))) .with("policyids", Arrays.asList( ImmutableMap.of("id", "wubba", "agreedon", 50000), ImmutableMap.of("id", "wugga", "agreedon", 40000)))