WIP webauthn - fix a security issue when anyone could register a new credential in existing user's name

This commit is contained in:
Dusan Jakub
2023-09-25 11:27:12 +02:00
parent 5a677721df
commit 5462333ed0
2 changed files with 19 additions and 7 deletions

View File

@@ -19,7 +19,7 @@ public record User(String login, String password, List<WebAuthnCredential> crede
List<WebAuthnCredential> newCredentials;
if (existing.isPresent()) {
// TODO need to decide if immutable or not
existing.get().counter++;
existing.get().counter = webAuthnCredential.counter;
newCredentials = credentials;
} else {
newCredentials = new ArrayList<>(credentials);

View File

@@ -55,11 +55,23 @@ public class MyWebAuthnSetup implements WebAuthnUserProvider {
@Override
public Uni<Void> updateOrStoreWebAuthnCredentials(Authenticator authenticator) {
WebAuthnCredential credential1 = new WebAuthnCredential(authenticator);
usersRepo.getUser(authenticator.getUserName())
.ifPresentOrElse(
user -> usersRepo.register(user.withAddedCredential(credential1)),
() -> usersRepo.register(new User(authenticator.getUserName(), null, List.of(credential1)))
);
return Uni.createFrom().nullItem();
var existingUser = usersRepo.getUser(authenticator.getUserName());
var existingCredential = existingUser.stream().flatMap(u -> u.credentials().stream())
.filter(c -> authenticator.getCredID().equals(c.credID)).findAny();
if (existingUser.isPresent() && existingCredential.isPresent()) {
// returning user and credential -> update counter
usersRepo.register(existingUser.get().withAddedCredential(existingCredential.get()));
return Uni.createFrom().nullItem();
} else if (existingUser.isEmpty()) {
// new user -> register
usersRepo.register(new User(authenticator.getUserName(), null, List.of(credential1)));
return Uni.createFrom().nullItem();
} else {
// returning (or duplicate) user with new credential -> reject,
// as we do not provide a means to register additional credentials yet
return Uni.createFrom().failure(new Throwable("Duplicate user"));
}
}
}