Skip to content

Commit

Permalink
Merge pull request #69 from Onlineberatung/OB-4625-fix-cve
Browse files Browse the repository at this point in the history
Ob 4625 fix CVE
  • Loading branch information
tkuzynow authored Sep 25, 2023
2 parents 23442bb + 1872914 commit 7de2312
Show file tree
Hide file tree
Showing 18 changed files with 100 additions and 78 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/dockerImage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ jobs:
- name: Setup JVM
uses: actions/setup-java@v1
with:
java-version: 11.0.10
java-version: 17
java-package: jdk
architecture: x64

Expand Down Expand Up @@ -182,4 +182,4 @@ jobs:
value: ${{ env.DOCKER_IMAGE_TAG }}
custom-actions: |
- text: View CI
url: "${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}"
url: "${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}"
2 changes: 1 addition & 1 deletion .github/workflows/feature-branch.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
- name: Setup JVM
uses: actions/setup-java@v1
with:
java-version: 11.0.10
java-version: 17
java-package: jdk
architecture: x64

Expand Down
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM quay.io/keycloak/keycloak:20.0.0
FROM quay.io/keycloak/keycloak:22.0.3
COPY keycloak-otp-config-spi-1.0-SNAPSHOT-keycloak.jar /opt/keycloak/providers/keycloak-otp-config-spi-1.0-SNAPSHOT-keycloak.jar

ENTRYPOINT /opt/keycloak/bin/kc.sh start --auto-build
36 changes: 33 additions & 3 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@

<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<maven.compiler.source>11</maven.compiler.source>
<maven.compiler.target>11</maven.compiler.target>
<keycloak.version>20.0.0</keycloak.version>
<maven.compiler.source>17</maven.compiler.source>
<maven.compiler.target>17</maven.compiler.target>
<keycloak.version>22.0.3</keycloak.version>
</properties>

<dependencies>
Expand All @@ -21,11 +21,23 @@
<artifactId>openapi-generator-maven-plugin</artifactId>
<version>6.2.0</version>
<scope>provided</scope>
<exclusions>
<exclusion>
<groupId>org.yaml</groupId>
<artifactId>snakeyaml</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
<version>2.14.0-rc2</version>
<exclusions>
<exclusion>
<groupId>org.yaml</groupId>
<artifactId>snakeyaml</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>org.keycloak</groupId>
Expand All @@ -44,6 +56,12 @@
<artifactId>keycloak-server-spi-private</artifactId>
<version>${keycloak.version}</version>
<scope>provided</scope>
<exclusions>
<exclusion>
<groupId>org.yaml</groupId>
<artifactId>snakeyaml</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>org.keycloak</groupId>
Expand All @@ -57,6 +75,12 @@
<groupId>com.openshift</groupId>
<artifactId>openshift-restclient-java</artifactId>
<version>9.0.5.Final</version>
<exclusions>
<exclusion>
<groupId>org.yaml</groupId>
<artifactId>snakeyaml</artifactId>
</exclusion>
</exclusions>
</dependency>

<dependency>
Expand All @@ -78,6 +102,12 @@
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.yaml</groupId>
<artifactId>snakeyaml</artifactId>
<version>2.0</version>
</dependency>

</dependencies>
<build>
<plugins>
Expand Down
16 changes: 11 additions & 5 deletions src/main/java/de/onlineberatung/RealmOtpResourceProvider.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,23 @@
import de.onlineberatung.otp.Otp;
import de.onlineberatung.otp.OtpMailSender;
import de.onlineberatung.otp.OtpService;
import jakarta.ws.rs.Consumes;
import jakarta.ws.rs.DELETE;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.POST;
import jakarta.ws.rs.PUT;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.PathParam;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.core.MediaType;
import jakarta.ws.rs.core.Response;
import jakarta.ws.rs.core.Response.Status;
import org.jboss.logging.Logger;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RealmModel;
import org.keycloak.models.UserModel;
import org.keycloak.services.resource.RealmResourceProvider;

import javax.ws.rs.*;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.Response.Status;

import static java.util.Objects.isNull;
import static java.util.Objects.nonNull;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

import static java.util.Objects.isNull;

import jakarta.ws.rs.ForbiddenException;
import jakarta.ws.rs.NotAuthorizedException;
import java.util.Objects;
import javax.ws.rs.ForbiddenException;
import javax.ws.rs.NotAuthorizedException;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RoleModel;
import org.keycloak.services.managers.AppAuthManager.BearerTokenAuthenticator;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@
import de.onlineberatung.otp.OtpMailSender;
import de.onlineberatung.otp.OtpService;
import de.onlineberatung.keycloak_otp_config_spi.keycloakextension.generated.web.model.OtpType;
import jakarta.ws.rs.core.MediaType;
import jakarta.ws.rs.core.Response;
import jakarta.ws.rs.core.Response.Status;
import java.util.Collections;
import java.util.List;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.Response.Status;
import org.jboss.logging.Logger;
import org.keycloak.authentication.AuthenticationFlowContext;
import org.keycloak.authentication.AuthenticationFlowError;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@

import de.onlineberatung.keycloak_otp_config_spi.keycloakextension.generated.web.model.Challenge;
import de.onlineberatung.keycloak_otp_config_spi.keycloakextension.generated.web.model.OtpType;
import jakarta.ws.rs.core.MediaType;
import jakarta.ws.rs.core.Response;
import jakarta.ws.rs.core.Response.Status;
import java.util.Collections;
import java.util.List;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.Response.Status;
import org.keycloak.authentication.AuthenticationFlowContext;
import org.keycloak.authentication.AuthenticationFlowError;
import org.keycloak.authentication.authenticators.directgrant.AbstractDirectGrantAuthenticator;
Expand Down Expand Up @@ -55,8 +55,7 @@ public boolean requiresUser() {
@Override
public boolean configuredFor(KeycloakSession keycloakSession, RealmModel realmModel,
UserModel userModel) {
return keycloakSession.userCredentialManager()
.isConfiguredFor(realmModel, userModel, OTPCredentialModel.TYPE);
return userModel.credentialManager().isConfiguredFor(OTPCredentialModel.TYPE);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@ public String generateSecret() {
}

public boolean is2FAConfigured(CredentialContext context) {
return context.getSession().userCredentialManager()
.isConfiguredFor(context.getRealm(), context.getUser(), OTPCredentialModel.TYPE);
return context.getUser().credentialManager().isConfiguredFor(OTPCredentialModel.TYPE);
}

public boolean validate(String otp, OTPCredentialModel credentialModel,
Expand All @@ -40,9 +39,9 @@ public boolean validate(String otp, OTPCredentialModel credentialModel,
}

public void deleteCredentials(CredentialContext context) {
var userCredentialManager = context.getSession().userCredentialManager();
var userCredentialManager = context.getUser().credentialManager();
var credentials = userCredentialManager.getStoredCredentialsByTypeStream(
context.getRealm(), context.getUser(), OTPCredentialModel.TYPE);
OTPCredentialModel.TYPE);
credentials.forEach(
credentialModel -> CredentialHelper.deleteOTPCredential(context.getSession(),
context.getRealm(), context.getUser(), credentialModel.getId()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import org.keycloak.credential.CredentialProvider;
import org.keycloak.credential.CredentialTypeMetadata;
import org.keycloak.credential.CredentialTypeMetadataContext;
import org.keycloak.credential.UserCredentialStore;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RealmModel;
import org.keycloak.models.UserCredentialModel;
Expand Down Expand Up @@ -39,16 +38,12 @@ public CredentialModel createCredential(RealmModel realm, UserModel user,
if (credentialModel.getCreatedDate() == null) {
credentialModel.setCreatedDate(clock.millis());
}
return getCredentialStore().createCredential(realm, user, credentialModel);
return user.credentialManager().createStoredCredential(credentialModel);
}

@Override
public boolean deleteCredential(RealmModel realm, UserModel user, String credentialId) {
return getCredentialStore().removeStoredCredential(realm, user, credentialId);
}

private UserCredentialStore getCredentialStore() {
return session.userCredentialManager();
return user.credentialManager().removeStoredCredentialById(credentialId);
}

@Override
Expand Down Expand Up @@ -79,7 +74,7 @@ public boolean isConfiguredFor(RealmModel realm, UserModel user, String credenti
if (!supportsCredentialType(credentialType)) {
return false;
}
return getCredentialStore().getStoredCredentialsByTypeStream(realm, user, credentialType)
return user.credentialManager().getStoredCredentialsByTypeStream(credentialType)
.findFirst().isPresent();
}

Expand All @@ -96,14 +91,13 @@ public boolean isValid(RealmModel realm, UserModel user, CredentialInput credent
if (challengeResponse == null) {
return false;
}
var credentialModel = getCredentialStore().getStoredCredentialById(realm, user,
credentialInput.getCredentialId());
var credentialModel = user.credentialManager()
.getStoredCredentialById(credentialInput.getCredentialId());
var otpCredentialModel = getCredentialFromModel(credentialModel);
return otpCredentialModel.getOtp().getCode().equals(challengeResponse);
}

public void updateCredential(RealmModel realm, UserModel user,
MailOtpCredentialModel credentialModel) {
getCredentialStore().updateCredential(realm, user, credentialModel);
public void updateCredential(UserModel user, MailOtpCredentialModel credentialModel) {
user.credentialManager().updateStoredCredential(credentialModel);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,22 @@ public MailOtpCredentialModel createCredential(Otp otp, CredentialContext contex
}

public void update(MailOtpCredentialModel credentialModel, CredentialContext context) {
credentialProvider.updateCredential(context.getRealm(), context.getUser(), credentialModel);
credentialProvider.updateCredential(context.getUser(), credentialModel);
}

public void incrementFailedAttempts(MailOtpCredentialModel credentialModel,
CredentialContext context, int currentAttempts) {
credentialModel.updateFailedVerifications(currentAttempts + 1);
credentialModel.updateInternalModel();
credentialProvider.updateCredential(context.getRealm(), context.getUser(), credentialModel);
credentialProvider.updateCredential(context.getUser(), credentialModel);
}

public void activate(MailOtpCredentialModel credentialModel, CredentialContext context) {
credentialModel.setActive();
credentialModel.updateFailedVerifications(0);
credentialModel.invalidateCode();
credentialModel.updateInternalModel();
credentialProvider.updateCredential(context.getRealm(), context.getUser(), credentialModel);
credentialProvider.updateCredential(context.getUser(), credentialModel);
}

public MailOtpCredentialModel getCredential(CredentialContext context) {
Expand All @@ -61,7 +61,7 @@ public void invalidate(MailOtpCredentialModel credentialModel, CredentialContext
credentialModel.updateFailedVerifications(0);
credentialModel.invalidateCode();
credentialModel.updateInternalModel();
credentialProvider.updateCredential(context.getRealm(), context.getUser(), credentialModel);
credentialProvider.updateCredential(context.getUser(), credentialModel);
}

public boolean is2FAConfigured(CredentialContext context) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package de.onlineberatung.log;

import jakarta.ws.rs.core.Cookie;
import jakarta.ws.rs.core.HttpHeaders;
import jakarta.ws.rs.core.UriInfo;
import java.util.Map;
import javax.ws.rs.core.Cookie;
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.UriInfo;
import org.jboss.logging.Logger;
import org.keycloak.common.util.StackUtil;
import org.keycloak.events.Event;
Expand Down Expand Up @@ -169,7 +169,7 @@ private void setKeycloakContext(StringBuilder sb) {
} else {
sb.append(", ");
}
sb.append(cookieEntry.getValue().toString());
sb.append(cookieEntry.getValue());
}
sb.append("]");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import org.keycloak.models.KeycloakContext;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RealmModel;
import org.keycloak.models.UserCredentialManager;
import org.keycloak.models.UserModel;
import org.keycloak.models.UserProvider;

Expand Down Expand Up @@ -74,9 +73,6 @@ public void setUp() {
when(session.users()).thenReturn(userProvider);
UserModel user = mock(UserModel.class);
when(userProvider.getUserByUsername(realm, "heinrich")).thenReturn(user);
var userCredentialManager = mock(UserCredentialManager.class);
when(session.userCredentialManager()).thenReturn(userCredentialManager);
when(userCredentialManager.isConfiguredFor(any(), any(), any())).thenReturn(false);
var appCredentialService = mock(AppOtpCredentialService.class);
resourceProvider = new RealmOtpResourceProvider(session, otpService, mailSender,
sessionAuthenticator, appCredentialService, credentialService);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,10 +230,10 @@ public void setupOtpMail_should_be_created_and_delete_app_if_2fa_via_app_is_alre
var otp = new Otp("1223", 1L, 2L, "[email protected]", 0);
var credentialModel = MailOtpCredentialModel.createOtpModel(otp, Clock.systemDefaultZone());
when(otpService.validate("1223", otp)).thenReturn(
ValidationResult.VALID);
ValidationResult.VALID);
when(mailCredentialService.getCredential(credentialContext)).thenReturn(credentialModel);
when(mailCredentialService.createCredential(otp, credentialContext)).thenReturn(
credentialModel);
credentialModel);

var response = resourceProvider.setupOtpMail("heinrich", mailSetup);

Expand All @@ -259,7 +259,7 @@ public void getOtpSetupInfo_should_return_type_app_if_app_2fa_is_configured() {
public void getOtpSetupInfo_should_return_type_mail_if_mail_2fa_is_configured() {
when(appCredentialService.generateSecret()).thenReturn("someSecret");
when(appCredentialService.generateQRCodeBase64("someSecret", credentialContext)).thenReturn(
"base64EncodedQRCode");
"base64EncodedQRCode");
when(mailCredentialService.is2FAConfigured(credentialContext)).thenReturn(true);

var response = resourceProvider.getOtpSetupInfo("heinrich");
Expand Down Expand Up @@ -296,9 +296,9 @@ public void setupOtp_should_be_created_and_delete_mail_otp_if_mail_otp_is_alread
otpSetup.setInitialCode("4711");
var credentialModel = mock(OTPCredentialModel.class);
when(appCredentialService.createModel("secretSecret", credentialContext)).thenReturn(
credentialModel);
credentialModel);
when((appCredentialService.validate("4711", credentialModel, credentialContext))).thenReturn(
true);
true);

var response = resourceProvider.setupOtp("heinrich", otpSetup);

Expand Down
Loading

0 comments on commit 7de2312

Please sign in to comment.