Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix progressive enrolment of passkey for JIT-Provisioned federated user #128

Merged
merged 3 commits into from
Jan 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,8 @@ public AuthenticatorFlowStatus process(HttpServletRequest request, HttpServletRe
handleUnProvisionedFederatedUser(response);
return AuthenticatorFlowStatus.INCOMPLETE;
}
boolean enrolledPasskeysExist = hasUserSetPasskeys(mappedLocalUsername);
authenticatedUser.setUserName(mappedLocalUsername);
boolean enrolledPasskeysExist = hasUserSetPasskeys(authenticatedUser);
if (enrolledPasskeysExist) {
// If the user have already enrolled passkeys and if the user initiated a passkey enrollment request,
// then inform the user that passkeys already exist and disregard the enrollment request.
Expand Down Expand Up @@ -802,7 +803,7 @@ private String initiateFido2AuthenticationRequest(AuthenticatedUser user, String

//Initiate the usernameless authentication process when either the user is unidentified or the identified user
// lacks an enrolled passkey.
if (user == null || !hasUserSetPasskeys(user.getUsernameAsSubjectIdentifier(true, true))) {
if (user == null || !hasUserSetPasskeys(user)) {
return webAuthnService.startUsernamelessAuthentication(appID);
}

Expand Down Expand Up @@ -853,10 +854,10 @@ private void processFido2PasskeyEnrollmentResponse(String challengeResponse, Str
}
}

private boolean hasUserSetPasskeys(String username) throws AuthenticationFailedException {
private boolean hasUserSetPasskeys(AuthenticatedUser authenticatedUser) throws AuthenticationFailedException {

WebAuthnService webAuthnService = new WebAuthnService();
return webAuthnService.isFidoKeyRegistered(username);
return webAuthnService.isFidoKeyRegistered(authenticatedUser);
}

private String buildAbsoluteURL(String redirectUrl) throws URISyntaxException, URLBuilderException {
Expand Down Expand Up @@ -1156,18 +1157,16 @@ private String getMappedLocalUsername(AuthenticatedUser authenticatedUser, Authe
throws AuthenticationFailedException {

if (!authenticatedUser.isFederatedUser()) {
return authenticatedUser.getUsernameAsSubjectIdentifier(true, true);
}
// If the user is federated, we need to check whether the user is already jit provisioned to the organization.
String federatedUsername = FederatedAuthenticatorUtil.getLoggedInFederatedUser(context);
if (StringUtils.isBlank(federatedUsername)) {
throw new AuthenticationFailedException("No federated user found");
return authenticatedUser.getUsernameAsSubjectIdentifier(false, false);
}
// Have to set idpName to the context since
// the FederatedAuthenticatorUtil.getLocalUsernameAssociatedWithFederatedUser is expecting it.
context.setProperty("idpName", authenticatedUser.getFederatedIdPName());
Copy link
Contributor

Choose a reason for hiding this comment

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

can have a constant for idpName

String associatedLocalUsername =
FederatedAuthenticatorUtil.getLocalUsernameAssociatedWithFederatedUser(MultitenantUtils.
getTenantAwareUsername(federatedUsername), context);
getTenantAwareUsername(authenticatedUser.toString()), context);
if (StringUtils.isNotBlank(associatedLocalUsername)) {
return FrameworkUtils.preprocessUsername(associatedLocalUsername, context);
return associatedLocalUsername;
}
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@ public String startAuthentication(String username, String tenantDomain, String s
user.setUserName(username);
user.setTenantDomain(tenantDomain);
user.setUserStoreDomain(storeDomain);
if (userStorage.getFIDO2RegistrationsByUsername(user.toString()).isEmpty()) {
if (userStorage.getFIDO2RegistrationsByUser(user).isEmpty()) {
if (log.isDebugEnabled()) {
log.debug("No registered device found for user :" + user.toString());
}
Expand Down Expand Up @@ -644,7 +644,7 @@ public AuthenticatedUser finishUsernamelessAuthentication(String responseJson)
userStorage.updateFIDO2SignatureCount(result);

new SuccessfulAuthenticationResult(request, response,
userStorage.getFIDO2RegistrationsByUsername(result.getUsername()), null);
userStorage.getFIDO2RegistrationsByUser(authenticatedUser), null);
} catch (FIDO2AuthenticatorServerException e) {
throw new AuthenticationFailedException("Error in usernameless authentication flow.", e);
}
Expand Down Expand Up @@ -672,7 +672,7 @@ public Collection<CredentialRegistration> getDeviceMetaData(String username) {
public Collection<FIDO2CredentialRegistration> getFIDO2DeviceMetaData(String username) throws
FIDO2AuthenticatorServerException {

return userStorage.getFIDO2RegistrationsByUsername(User.getUserFromUserName(username).toString());
return userStorage.getFIDO2RegistrationsByUsername(username);
}

@Deprecated
Expand Down Expand Up @@ -1278,4 +1278,12 @@ public boolean isFidoKeyRegistered (String username) throws AuthenticationFailed
throw new AuthenticationFailedException(e.getMessage());
}
}

public boolean isFidoKeyRegistered (AuthenticatedUser authenticatedUser) throws AuthenticationFailedException {
try {
Copy link
Contributor

Choose a reason for hiding this comment

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

can add a line after method signature

return !userStorage.getFIDO2RegistrationsByUser(authenticatedUser).isEmpty();
} catch (FIDO2AuthenticatorServerException e) {
throw new AuthenticationFailedException(e.getMessage());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,19 @@ public Collection<FIDO2CredentialRegistration> getFIDO2RegistrationsByUsername(S
FIDO2AuthenticatorServerException {

User user = User.getUserFromUserName(username);
return getFIDO2RegistrationsByUser(user);
}

/**
* Retrieve FIDO2 registration details.
*
* @param user User.
* @return A collection of FIDO2 registrations available for a user.
* @throws FIDO2AuthenticatorServerException
*/
public Collection<FIDO2CredentialRegistration> getFIDO2RegistrationsByUser(User user) throws
FIDO2AuthenticatorServerException {

List<FIDO2CredentialRegistration> credentialRegistrations = new ArrayList<>();

if (log.isDebugEnabled()) {
Expand Down Expand Up @@ -475,7 +488,7 @@ public Collection<FIDO2CredentialRegistration> getFIDO2RegistrationsByUsername(S
}
} catch (SQLException | IOException e) {
throw new FIDO2AuthenticatorServerException("Server error occurred while retrieving FIDO2 device " +
"registration for username: " + username, e);
"registration for username: " + user.getLoggableMaskedUserId(), e);
} finally {
IdentityDatabaseUtil.closeAllConnections(connection, resultSet, preparedStatement);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ public void setUp() throws UserStoreException, IOException, FIDO2AuthenticatorSe
when(FIDO2DeviceStoreDAO.getInstance()).thenReturn(userStorageMock);
mockStatic(IdentityUtil.class);
when(IdentityUtil.getProperty("FIDO.UserResponseTimeout")).thenReturn("300");
when(IdentityUtil.addDomainToName(anyString(), anyString())).thenCallRealMethod();
mockStatic(JacksonCodecs.class);
when(JacksonCodecs.json()).thenReturn(objectMapperMock);
webAuthnService = new WebAuthnService();
Expand All @@ -240,7 +241,7 @@ public void setUp() throws UserStoreException, IOException, FIDO2AuthenticatorSe

mockStatic(User.class);
user = new User();
user.setUserName(TENANT_QUALIFIED_USERNAME);
user.setUserName(USERNAME);
user.setTenantDomain(TENANT_DOMAIN);
user.setUserStoreDomain(USER_STORE_DOMAIN);
when(User.getUserFromUserName(TENANT_QUALIFIED_USERNAME)).thenReturn(user);
Expand Down Expand Up @@ -285,6 +286,7 @@ public void setUp() throws UserStoreException, IOException, FIDO2AuthenticatorSe
List<FIDO2CredentialRegistration> credentialRegistrations = new ArrayList<>();
credentialRegistrations.add(fido2CredentialRegistration);
when(userStorageMock.getFIDO2RegistrationsByUsername(anyString())).thenReturn(credentialRegistrations);
when(userStorageMock.getFIDO2RegistrationsByUser(any(User.class))).thenReturn(credentialRegistrations);
FIDO2CredentialRegistration fido2CredentialRegistration = mock(FIDO2CredentialRegistration.class);
when(userStorageMock.getFIDO2RegistrationByUsernameAndCredentialId(anyString(), any(ByteArray.class)))
.thenReturn(Optional.of(fido2CredentialRegistration));
Expand Down