From 435b4c30aefefee2767ab9c041a1c689f6991ee9 Mon Sep 17 00:00:00 2001 From: Andrej Petras Date: Mon, 22 Jan 2024 18:00:06 +0100 Subject: [PATCH] feat: switch rest-context token parser --- pom.xml | 2 +- .../permission/common/models/TokenConfig.java | 22 +++--- .../common/services/ClaimService.java | 2 +- .../common/services/TokenService.java | 63 ++++----------- .../permission/common/utils/TokenUtil.java | 59 -------------- .../onecx-permission-internal-openapi.yaml | 3 + src/main/resources/application.properties | 2 +- .../common/utils/TokenUtilTest.java | 79 ------------------- ...missionRestControllerConfigIssuerTest.java | 72 ----------------- ...sionRestControllerConfigPublicKeyTest.java | 72 ----------------- .../controllers/RoleRestControllerTest.java | 23 +++++- .../onecx/permission/test/AbstractTest.java | 2 +- 12 files changed, 54 insertions(+), 347 deletions(-) delete mode 100644 src/main/java/io/github/onecx/permission/common/utils/TokenUtil.java delete mode 100644 src/test/java/io/github/onecx/permission/common/utils/TokenUtilTest.java delete mode 100644 src/test/java/io/github/onecx/permission/rs/external/v1/PermissionRestControllerConfigIssuerTest.java delete mode 100644 src/test/java/io/github/onecx/permission/rs/external/v1/PermissionRestControllerConfigPublicKeyTest.java diff --git a/pom.xml b/pom.xml index c42d3f8..dc0b029 100644 --- a/pom.xml +++ b/pom.xml @@ -7,7 +7,7 @@ io.github.onecx onecx-quarkus3-parent - 0.25.0 + 0.26.0 onecx-permission-svc diff --git a/src/main/java/io/github/onecx/permission/common/models/TokenConfig.java b/src/main/java/io/github/onecx/permission/common/models/TokenConfig.java index 6930ea2..277726d 100644 --- a/src/main/java/io/github/onecx/permission/common/models/TokenConfig.java +++ b/src/main/java/io/github/onecx/permission/common/models/TokenConfig.java @@ -8,22 +8,22 @@ import io.smallrye.config.WithName; @StaticInitSafe -@ConfigMapping(prefix = "onecx.permission") +@ConfigMapping(prefix = "onecx.permission.token") public interface TokenConfig { - @WithName("token.verified") - boolean tokenVerified(); + @WithName("verified") + boolean verified(); - @WithName("token.issuer.public-key-location.suffix") - String tokenPublicKeyLocationSuffix(); + @WithName("issuer.public-key-location.suffix") + String publicKeyLocationSuffix(); - @WithName("token.issuer.public-key-location.enabled") - boolean tokenPublicKeyEnabled(); + @WithName("issuer.public-key-location.enabled") + boolean publicKeyEnabled(); - @WithName("token.claim.separator") - Optional tokenClaimSeparator(); + @WithName("claim.separator") + Optional claimSeparator(); - @WithName("token.claim.path") + @WithName("claim.path") @WithDefault("realm_access/roles") - String tokenClaimPath(); + String claimPath(); } diff --git a/src/main/java/io/github/onecx/permission/common/services/ClaimService.java b/src/main/java/io/github/onecx/permission/common/services/ClaimService.java index 35567ef..19eea53 100644 --- a/src/main/java/io/github/onecx/permission/common/services/ClaimService.java +++ b/src/main/java/io/github/onecx/permission/common/services/ClaimService.java @@ -22,7 +22,7 @@ public class ClaimService { @PostConstruct @SuppressWarnings("java:S2696") public void init() { - claimPath = splitClaimPath(config.tokenClaimPath()); + claimPath = splitClaimPath(config.claimPath()); } public String[] getClaimPath() { diff --git a/src/main/java/io/github/onecx/permission/common/services/TokenService.java b/src/main/java/io/github/onecx/permission/common/services/TokenService.java index ba42a16..5407950 100644 --- a/src/main/java/io/github/onecx/permission/common/services/TokenService.java +++ b/src/main/java/io/github/onecx/permission/common/services/TokenService.java @@ -1,78 +1,45 @@ package io.github.onecx.permission.common.services; -import static io.github.onecx.permission.common.utils.TokenUtil.findClaimWithRoles; - import java.util.List; import jakarta.enterprise.context.ApplicationScoped; import jakarta.inject.Inject; -import org.jose4j.jws.JsonWebSignature; -import org.jose4j.jwt.JwtClaims; -import org.jose4j.jwt.MalformedClaimException; -import org.jose4j.jwt.consumer.InvalidJwtException; -import org.jose4j.jwx.JsonWebStructure; -import org.jose4j.lang.JoseException; +import org.tkit.quarkus.rs.context.token.TokenClaimUtility; +import org.tkit.quarkus.rs.context.token.TokenParserRequest; +import org.tkit.quarkus.rs.context.token.TokenParserService; import io.github.onecx.permission.common.models.TokenConfig; -import io.smallrye.jwt.auth.principal.JWTAuthContextInfo; -import io.smallrye.jwt.auth.principal.JWTParser; -import io.smallrye.jwt.auth.principal.ParseException; import lombok.extern.slf4j.Slf4j; @Slf4j @ApplicationScoped public class TokenService { - @Inject - JWTAuthContextInfo authContextInfo; - @Inject TokenConfig config; @Inject - JWTParser parser; + ClaimService claimService; @Inject - ClaimService claimService; + TokenParserService tokenParserService; public List getTokenRoles(String tokenData) { - try { - return getRoles(tokenData); - } catch (Exception ex) { - throw new TokenException("Error parsing principal token", ex); - } - } - - private List getRoles(String tokenData) - throws JoseException, InvalidJwtException, MalformedClaimException, ParseException { - - var claimPath = claimService.getClaimPath(); - - if (config.tokenVerified()) { - var info = authContextInfo; - // get public key location from issuer URL - if (config.tokenPublicKeyEnabled()) { - var jws = (JsonWebSignature) JsonWebStructure.fromCompactSerialization(tokenData); - var jwtClaims = JwtClaims.parse(jws.getUnverifiedPayload()); - var publicKeyLocation = jwtClaims.getIssuer() + config.tokenPublicKeyLocationSuffix(); - info = new JWTAuthContextInfo(authContextInfo); - info.setPublicKeyLocation(publicKeyLocation); - } - - var token = parser.parse(tokenData, info); - var first = token.getClaim(claimPath[0]); - - return findClaimWithRoles(config, first, claimPath); + try { - } else { + var request = new TokenParserRequest(tokenData) + .verify(config.verified()) + .issuerEnabled(config.publicKeyEnabled()) + .issuerSuffix(config.publicKeyLocationSuffix()); - var jws = (JsonWebSignature) JsonWebStructure.fromCompactSerialization(tokenData); + var permissionToken = tokenParserService.parseToken(request); + var path = claimService.getClaimPath(); + return TokenClaimUtility.findClaimStringList(permissionToken, path, config.claimSeparator().orElse(" ")); - var jwtClaims = JwtClaims.parse(jws.getUnverifiedPayload()); - var first = jwtClaims.getClaimValue(claimPath[0]); - return findClaimWithRoles(config, first, claimPath); + } catch (Exception ex) { + throw new TokenException("Error parsing permission token", ex); } } diff --git a/src/main/java/io/github/onecx/permission/common/utils/TokenUtil.java b/src/main/java/io/github/onecx/permission/common/utils/TokenUtil.java deleted file mode 100644 index a298caa..0000000 --- a/src/main/java/io/github/onecx/permission/common/utils/TokenUtil.java +++ /dev/null @@ -1,59 +0,0 @@ -package io.github.onecx.permission.common.utils; - -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.List; - -import jakarta.json.JsonArray; -import jakarta.json.JsonObject; -import jakarta.json.JsonValue; - -import io.github.onecx.permission.common.models.TokenConfig; -import io.smallrye.jwt.JsonUtils; - -public final class TokenUtil { - - private TokenUtil() { - } - - public static List findClaimWithRoles(TokenConfig config, Object value, String[] path) { - JsonValue first = JsonUtils.wrapValue(value); - JsonValue claimValue = findClaimValue(first, path, 1); - - if (claimValue instanceof JsonArray jsonArray) { - return convertJsonArrayToList(jsonArray); - } else if (claimValue != null) { - if (claimValue.toString().isBlank()) { - return Collections.emptyList(); - } - return Arrays.asList(claimValue.toString().split(config.tokenClaimSeparator().orElse(" "))); - } else { - return Collections.emptyList(); - } - } - - static List convertJsonArrayToList(JsonArray claimValue) { - List list = new ArrayList<>(claimValue.size()); - for (int i = 0; i < claimValue.size(); i++) { - String claimValueStr = claimValue.getString(i); - if (claimValueStr.isBlank()) { - continue; - } - list.add(claimValue.getString(i)); - } - return list; - } - - private static JsonValue findClaimValue(JsonValue json, String[] pathArray, int step) { - if (json == null) { - return null; - } - if (step < pathArray.length && (json instanceof JsonObject)) { - JsonValue claimValue = json.asJsonObject().get(pathArray[step].replace("\"", "")); - return findClaimValue(claimValue, pathArray, step + 1); - } - return json; - } - -} diff --git a/src/main/openapi/onecx-permission-internal-openapi.yaml b/src/main/openapi/onecx-permission-internal-openapi.yaml index e2599e5..00c0125 100644 --- a/src/main/openapi/onecx-permission-internal-openapi.yaml +++ b/src/main/openapi/onecx-permission-internal-openapi.yaml @@ -485,6 +485,7 @@ components: type: object required: - description + - modificationCount properties: modificationCount: format: int32 @@ -699,6 +700,8 @@ components: type: string UpdateRoleRequest: type: object + required: + - modificationCount properties: modificationCount: format: int32 diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 17e8343..be6abc5 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -5,6 +5,7 @@ quarkus.datasource.jdbc.min-size=10 quarkus.hibernate-orm.database.generation=validate quarkus.hibernate-orm.multitenant=DISCRIMINATOR +quarkus.hibernate-orm.jdbc.timezone=UTC quarkus.liquibase.migrate-at-start=true quarkus.liquibase.validate-on-migrate=true @@ -41,7 +42,6 @@ quarkus.test.integration-test-profile=test %test.tkit.rs.context.tenant-id.mock.enabled=true %test.tkit.rs.context.tenant-id.mock.default-tenant=default %test.tkit.rs.context.tenant-id.mock.claim-org-id=orgId -%test.tkit.rs.context.tenant-id.mock.token-header-param=apm-principal-token %test.tkit.rs.context.tenant-id.mock.data.org1=100 %test.tkit.rs.context.tenant-id.mock.data.org2=200 %test.tkit.rs.context.tenant-id.mock.data.i100=i100 diff --git a/src/test/java/io/github/onecx/permission/common/utils/TokenUtilTest.java b/src/test/java/io/github/onecx/permission/common/utils/TokenUtilTest.java deleted file mode 100644 index 0c31c29..0000000 --- a/src/test/java/io/github/onecx/permission/common/utils/TokenUtilTest.java +++ /dev/null @@ -1,79 +0,0 @@ -package io.github.onecx.permission.common.utils; - -import static org.assertj.core.api.Assertions.assertThat; - -import java.util.Optional; - -import jakarta.json.Json; -import jakarta.json.JsonValue; - -import org.junit.jupiter.api.Test; - -import io.github.onecx.permission.common.models.TokenConfig; -import io.github.onecx.permission.test.AbstractTest; -import io.quarkus.test.junit.QuarkusTest; - -@QuarkusTest -class TokenUtilTest extends AbstractTest { - - @Test - void tokenUtilityTest() { - - var config = new TokenConfig() { - @Override - public boolean tokenVerified() { - return false; - } - - @Override - public String tokenPublicKeyLocationSuffix() { - return null; - } - - @Override - public boolean tokenPublicKeyEnabled() { - return false; - } - - @Override - public Optional tokenClaimSeparator() { - return Optional.empty(); - } - - @Override - public String tokenClaimPath() { - return null; - } - }; - - var tmp = TokenUtil.findClaimWithRoles(config, null, new String[] { "test" }); - assertThat(tmp).isNotNull().isEmpty(); - - var value = Json.createValue(32); - tmp = TokenUtil.findClaimWithRoles(config, value, new String[] { "test1", "test2" }); - assertThat(tmp).isNotNull().containsExactly("32"); - - JsonValue emptyValue = new JsonValue() { - @Override - public ValueType getValueType() { - return null; - } - - @Override - public String toString() { - return " "; - } - }; - - tmp = TokenUtil.findClaimWithRoles(config, emptyValue, new String[] { "test1", "test2" }); - assertThat(tmp).isNotNull().isEmpty(); - - var list = Json.createArrayBuilder(); - list.add("s1"); - list.add(" "); - list.add(""); - - tmp = TokenUtil.findClaimWithRoles(config, list.build(), new String[] { "test1", "test2" }); - assertThat(tmp).isNotNull().containsExactly("s1"); - } -} diff --git a/src/test/java/io/github/onecx/permission/rs/external/v1/PermissionRestControllerConfigIssuerTest.java b/src/test/java/io/github/onecx/permission/rs/external/v1/PermissionRestControllerConfigIssuerTest.java deleted file mode 100644 index 0448e0b..0000000 --- a/src/test/java/io/github/onecx/permission/rs/external/v1/PermissionRestControllerConfigIssuerTest.java +++ /dev/null @@ -1,72 +0,0 @@ -package io.github.onecx.permission.rs.external.v1; - -import static io.restassured.RestAssured.given; -import static jakarta.ws.rs.core.MediaType.APPLICATION_JSON; -import static org.assertj.core.api.Assertions.assertThat; -import static org.jboss.resteasy.reactive.RestResponse.Status.OK; - -import jakarta.inject.Inject; - -import org.eclipse.microprofile.config.Config; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.mockito.Mockito; -import org.tkit.quarkus.test.WithDBData; - -import gen.io.github.onecx.permission.rs.external.v1.model.ApplicationPermissionsDTOV1; -import gen.io.github.onecx.permission.rs.external.v1.model.PermissionRequestDTOV1; -import io.github.onecx.permission.common.models.TokenConfig; -import io.github.onecx.permission.common.services.ClaimService; -import io.github.onecx.permission.rs.external.v1.controllers.PermissionRestController; -import io.github.onecx.permission.test.AbstractTest; -import io.quarkus.test.InjectMock; -import io.quarkus.test.common.http.TestHTTPEndpoint; -import io.quarkus.test.junit.QuarkusTest; -import io.quarkus.test.keycloak.client.KeycloakTestClient; -import io.smallrye.config.SmallRyeConfig; - -@QuarkusTest -@TestHTTPEndpoint(PermissionRestController.class) -@WithDBData(value = "data/test-v1.xml", deleteBeforeInsert = true, deleteAfterTest = true, rinseAndRepeat = true) -class PermissionRestControllerConfigIssuerTest extends AbstractTest { - - @InjectMock - TokenConfig tokenConfig; - - @InjectMock - ClaimService claimService; - - @Inject - Config config; - - @BeforeEach - void beforeEach() { - Mockito.when(claimService.getClaimPath()).thenReturn(new String[] { "groups" }); - var tmp = config.unwrap(SmallRyeConfig.class).getConfigMapping(TokenConfig.class); - Mockito.when(tokenConfig.tokenClaimSeparator()).thenReturn(tmp.tokenClaimSeparator()); - Mockito.when(tokenConfig.tokenClaimPath()).thenReturn(tmp.tokenClaimPath()); - Mockito.when(tokenConfig.tokenVerified()).thenReturn(true); - Mockito.when(tokenConfig.tokenPublicKeyLocationSuffix()).thenReturn(tmp.tokenPublicKeyLocationSuffix()); - Mockito.when(tokenConfig.tokenPublicKeyEnabled()).thenReturn(true); - } - - @Test - void skipTokenVerified() { - - KeycloakTestClient keycloakClient = new KeycloakTestClient(); - var accessToken = keycloakClient.getAccessToken("bob"); - - var dto = given() - .contentType(APPLICATION_JSON) - .body(new PermissionRequestDTOV1().token(accessToken)) - .post("/application/app1") - .then() - .statusCode(OK.getStatusCode()) - .extract() - .body().as(ApplicationPermissionsDTOV1.class); - - assertThat(dto).isNotNull(); - assertThat(dto.getPermissions()).isNotNull().hasSize(1); - assertThat(dto.getPermissions().get("o1")).isNotNull().hasSize(1).containsExactly("a3"); - } -} diff --git a/src/test/java/io/github/onecx/permission/rs/external/v1/PermissionRestControllerConfigPublicKeyTest.java b/src/test/java/io/github/onecx/permission/rs/external/v1/PermissionRestControllerConfigPublicKeyTest.java deleted file mode 100644 index 2942522..0000000 --- a/src/test/java/io/github/onecx/permission/rs/external/v1/PermissionRestControllerConfigPublicKeyTest.java +++ /dev/null @@ -1,72 +0,0 @@ -package io.github.onecx.permission.rs.external.v1; - -import static io.restassured.RestAssured.given; -import static jakarta.ws.rs.core.MediaType.APPLICATION_JSON; -import static org.assertj.core.api.Assertions.assertThat; -import static org.jboss.resteasy.reactive.RestResponse.Status.OK; - -import jakarta.inject.Inject; - -import org.eclipse.microprofile.config.Config; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.mockito.Mockito; -import org.tkit.quarkus.test.WithDBData; - -import gen.io.github.onecx.permission.rs.external.v1.model.ApplicationPermissionsDTOV1; -import gen.io.github.onecx.permission.rs.external.v1.model.PermissionRequestDTOV1; -import io.github.onecx.permission.common.models.TokenConfig; -import io.github.onecx.permission.common.services.ClaimService; -import io.github.onecx.permission.rs.external.v1.controllers.PermissionRestController; -import io.github.onecx.permission.test.AbstractTest; -import io.quarkus.test.InjectMock; -import io.quarkus.test.common.http.TestHTTPEndpoint; -import io.quarkus.test.junit.QuarkusTest; -import io.quarkus.test.keycloak.client.KeycloakTestClient; -import io.smallrye.config.SmallRyeConfig; - -@QuarkusTest -@TestHTTPEndpoint(PermissionRestController.class) -@WithDBData(value = "data/test-v1.xml", deleteBeforeInsert = true, deleteAfterTest = true, rinseAndRepeat = true) -class PermissionRestControllerConfigPublicKeyTest extends AbstractTest { - - @InjectMock - TokenConfig tokenConfig; - - @InjectMock - ClaimService claimService; - - @Inject - Config config; - - @BeforeEach - void beforeEach() { - Mockito.when(claimService.getClaimPath()).thenReturn(new String[] { "groups" }); - var tmp = config.unwrap(SmallRyeConfig.class).getConfigMapping(TokenConfig.class); - Mockito.when(tokenConfig.tokenClaimSeparator()).thenReturn(tmp.tokenClaimSeparator()); - Mockito.when(tokenConfig.tokenClaimPath()).thenReturn(tmp.tokenClaimPath()); - Mockito.when(tokenConfig.tokenVerified()).thenReturn(true); - Mockito.when(tokenConfig.tokenPublicKeyLocationSuffix()).thenReturn(tmp.tokenPublicKeyLocationSuffix()); - Mockito.when(tokenConfig.tokenPublicKeyEnabled()).thenReturn(false); - } - - @Test - void skipTokenVerified() { - - KeycloakTestClient keycloakClient = new KeycloakTestClient(); - var accessToken = keycloakClient.getAccessToken("bob"); - - var dto = given() - .contentType(APPLICATION_JSON) - .body(new PermissionRequestDTOV1().token(accessToken)) - .post("/application/app1") - .then() - .statusCode(OK.getStatusCode()) - .extract() - .body().as(ApplicationPermissionsDTOV1.class); - - assertThat(dto).isNotNull(); - assertThat(dto.getPermissions()).isNotNull().hasSize(1); - assertThat(dto.getPermissions().get("o1")).isNotNull().hasSize(1).containsExactly("a3"); - } -} diff --git a/src/test/java/io/github/onecx/permission/rs/internal/controllers/RoleRestControllerTest.java b/src/test/java/io/github/onecx/permission/rs/internal/controllers/RoleRestControllerTest.java index 332bac0..9d9d562 100644 --- a/src/test/java/io/github/onecx/permission/rs/internal/controllers/RoleRestControllerTest.java +++ b/src/test/java/io/github/onecx/permission/rs/internal/controllers/RoleRestControllerTest.java @@ -194,9 +194,19 @@ void searchRolesTest() { @Test void updateRoleTest() { + // download Role + var dto = given().contentType(APPLICATION_JSON) + .when() + .get("r11") + .then().statusCode(OK.getStatusCode()) + .contentType(APPLICATION_JSON) + .extract() + .body().as(RoleDTO.class); + // update none existing Role var requestDto = new UpdateRoleRequestDTO(); requestDto.setName("test01"); + requestDto.setModificationCount(dto.getModificationCount()); requestDto.setDescription("description-update"); given() @@ -216,8 +226,7 @@ void updateRoleTest() { .statusCode(OK.getStatusCode()); // download Role - var dto = given().contentType(APPLICATION_JSON) - .body(requestDto) + dto = given().contentType(APPLICATION_JSON) .when() .get("r11") .then().statusCode(OK.getStatusCode()) @@ -233,7 +242,17 @@ void updateRoleTest() { @Test void updateRoleWithExistingNameTest() { + // download Role + var d = given().contentType(APPLICATION_JSON) + .when() + .get("r11") + .then().statusCode(OK.getStatusCode()) + .contentType(APPLICATION_JSON) + .extract() + .body().as(RoleDTO.class); + var dto = new UpdateRoleRequestDTO(); + dto.setModificationCount(d.getModificationCount()); dto.setName("n3"); dto.setDescription("description"); diff --git a/src/test/java/io/github/onecx/permission/test/AbstractTest.java b/src/test/java/io/github/onecx/permission/test/AbstractTest.java index cbc2070..0247d5c 100644 --- a/src/test/java/io/github/onecx/permission/test/AbstractTest.java +++ b/src/test/java/io/github/onecx/permission/test/AbstractTest.java @@ -31,7 +31,7 @@ public class AbstractTest { protected static final String APM_HEADER_PARAM = ConfigProvider.getConfig() - .getValue("%test.tkit.rs.context.tenant-id.mock.token-header-param", String.class); + .getValue("tkit.rs.context.token.header-param", String.class); protected static final String CLAIMS_ORG_ID = ConfigProvider.getConfig() .getValue("%test.tkit.rs.context.tenant-id.mock.claim-org-id", String.class);