From 3c8f816b62b0efdfffc0e3f53d8371f4510d3ef6 Mon Sep 17 00:00:00 2001 From: Jose Date: Tue, 9 Aug 2022 06:51:06 -0500 Subject: [PATCH] fix(jans-config-api): avoid loss of attributes in agama endpoints (#2058) * fix: avoid data loss #1982 * docs: remove PUT json endpoint #1982 * chore: remove unnecessary check #1982 --- .../docs/jans-config-api-swagger.yaml | 31 +---- .../rest/resource/auth/AgamaResource.java | 118 +++++++----------- .../service/auth/AgamaFlowService.java | 8 +- 3 files changed, 51 insertions(+), 106 deletions(-) diff --git a/jans-config-api/docs/jans-config-api-swagger.yaml b/jans-config-api/docs/jans-config-api-swagger.yaml index de80ccdef1b..3ed95e36f37 100644 --- a/jans-config-api/docs/jans-config-api-swagger.yaml +++ b/jans-config-api/docs/jans-config-api-swagger.yaml @@ -2517,8 +2517,8 @@ paths: - oauth2: [https://jans.io/oauth/config/agama.readonly] post: - summary: Create a new agama flow from source file. - description: Create a new agama flow from source file. + summary: Create a new agama flow from source. + description: Create a new agama flow from source. operationId: post-agama-flow-from-source tags: - Configuration – Agama Flow @@ -2541,33 +2541,6 @@ paths: $ref: '#/components/responses/InternalServerError' security: - oauth2: [https://jans.io/oauth/config/agama.write] - - put: - summary: Updates an agama flow based on Qname. - description: Updates an agama based on Qname. - operationId: put-agama-flow - tags: - - Configuration – Agama Flow - requestBody: - content: - application/json: - schema: - $ref: '#/components/schemas/AgamaFlow' - responses: - '200': - description: OK - content: - application/json: - schema: - $ref: '#/components/schemas/AgamaFlow' - '401': - $ref: '#/components/responses/Unauthorized' - '404': - $ref: '#/components/responses/NotFound' - '500': - $ref: '#/components/responses/InternalServerError' - security: - - oauth2: [https://jans.io/oauth/config/agama.write] patch: summary: Partially modify a Agama Flow. diff --git a/jans-config-api/server/src/main/java/io/jans/configapi/rest/resource/auth/AgamaResource.java b/jans-config-api/server/src/main/java/io/jans/configapi/rest/resource/auth/AgamaResource.java index f6c8d363e6a..355c7c02a74 100644 --- a/jans-config-api/server/src/main/java/io/jans/configapi/rest/resource/auth/AgamaResource.java +++ b/jans-config-api/server/src/main/java/io/jans/configapi/rest/resource/auth/AgamaResource.java @@ -34,6 +34,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.stream.Collectors; import static java.nio.charset.StandardCharsets.UTF_8; import org.apache.commons.lang.StringUtils; @@ -68,8 +69,7 @@ public Response getFlows(@DefaultValue("") @QueryParam(value = ApiConstants.PATT flows = agamaFlowService.getAllAgamaFlows(limit); } - // filter values - getAgamaFlowDetails(flows, includeSource); + flows = flows.stream().map(f -> minimize(f, includeSource)).collect(Collectors.toList()); return Response.ok(flows).build(); } @@ -85,9 +85,9 @@ public Response getFlowByName(@PathParam(ApiConstants.QNAME) @NotNull String flo String decodedFlowName = getURLDecodedValue(flowName); logger.trace(" Agama Decoded flow name decodedFlowName:{}", decodedFlowName); - Flow flow = findFlow(decodedFlowName, true, includeSource); + Flow flow = findFlow(decodedFlowName, true); - return Response.ok(flow).build(); + return Response.ok(minimize(flow, includeSource)).build(); } @POST @@ -98,10 +98,10 @@ public Response createFlow(@Valid Flow flow) flow.getSource()); // check if flow with same name already exists - Flow existingFlow = findFlow(flow.getQname(), false, false); + Flow existingFlow = findFlow(flow.getQname(), false); logger.debug(" existingFlow:{}", existingFlow); if (existingFlow != null) { - thorwBadRequestException("Flow identified by name '" + flow.getQname() + "' already exist!"); + thorwBadRequestException("Flow identified by name '" + flow.getQname() + "' already exists!"); } // validate flow data @@ -109,15 +109,15 @@ public Response createFlow(@Valid Flow flow) validateAgamaFlowData(flow, true); agamaFlowService.addAgamaFlow(flow); - flow = findFlow(flow.getQname(), true, false); - return Response.status(Response.Status.CREATED).entity(flow).build(); + flow = findFlow(flow.getQname(), true); + return Response.status(Response.Status.CREATED).entity(minimize(flow, false)).build(); } @POST @Consumes(MediaType.TEXT_PLAIN) @Path(ApiConstants.QNAME_PATH) @ProtectedApi(scopes = { ApiAccessConstants.AGAMA_WRITE_ACCESS }) - public Response createFlowFromFile(@PathParam(ApiConstants.QNAME) @NotNull String flowName, @Valid String source) + public Response createFlowFromSource(@PathParam(ApiConstants.QNAME) @NotNull String flowName, @Valid String source) throws NoSuchMethodException, IllegalAccessException, InvocationTargetException { logger.debug(" Flow to be created flowName:{}, source:{}", flowName, source); @@ -125,10 +125,10 @@ public Response createFlowFromFile(@PathParam(ApiConstants.QNAME) @NotNull Strin logger.trace(" Agama Decoded flow name for create is:{}", decodedFlowName); // check if flow with same name already exists - Flow existingFlow = findFlow(decodedFlowName, false, false); + Flow existingFlow = findFlow(decodedFlowName, false); logger.debug(" existing-flow:{}", existingFlow); if (existingFlow != null) { - thorwBadRequestException("Flow identified by name '" + decodedFlowName + "' already exist!"); + thorwBadRequestException("Flow identified by name '" + decodedFlowName + "' already exists!"); } Flow flow = new Flow(); @@ -141,10 +141,12 @@ public Response createFlowFromFile(@PathParam(ApiConstants.QNAME) @NotNull Strin validateAgamaFlowData(flow, true); agamaFlowService.addAgamaFlow(flow); - flow = findFlow(flow.getQname(), true, false); - return Response.status(Response.Status.CREATED).entity(flow).build(); + flow = findFlow(flow.getQname(), true); + return Response.status(Response.Status.CREATED).entity(minimize(flow, false)).build(); } + //TODO: determine if this endpoint is really needed in practice + /* @PUT @Path(ApiConstants.QNAME_PATH) @ProtectedApi(scopes = { ApiAccessConstants.AGAMA_WRITE_ACCESS }) @@ -171,13 +173,13 @@ public Response updateFlow(@PathParam(ApiConstants.QNAME) @NotNull String flowNa flow = findFlow(decodedFlowName, true, false); return Response.status(Response.Status.OK).entity(flow).build(); - } + }*/ @PUT @Consumes(MediaType.TEXT_PLAIN) @Path(ApiConstants.SOURCE + ApiConstants.QNAME_PATH) @ProtectedApi(scopes = { ApiAccessConstants.AGAMA_WRITE_ACCESS }) - public Response updateFlowFromFile(@PathParam(ApiConstants.QNAME) @NotNull String flowName, @Valid String source) + public Response updateFlowSource(@PathParam(ApiConstants.QNAME) @NotNull String flowName, @Valid String source) throws NoSuchMethodException, IllegalAccessException, InvocationTargetException { logger.debug(" Flow to be updated flowName:{}, source:{}", flowName, source); @@ -185,23 +187,19 @@ public Response updateFlowFromFile(@PathParam(ApiConstants.QNAME) @NotNull Strin logger.trace(" Agama flow name for update is:{}", decodedFlowName); // check if flow with same name exists - Flow existingFlow = findFlow(decodedFlowName, true, false); + Flow existingFlow = findFlow(decodedFlowName, true); logger.debug(" Agama existingFlow:{}", existingFlow); - // Update source and revision - if (existingFlow != null) { - existingFlow.setSource(source); + existingFlow.setSource(source); + updateFlowDetails(existingFlow, existingFlow, false); - updateFlowDetails(existingFlow, existingFlow,false); - - // validate flow data - validateAgamaFlowData(existingFlow, false); - logger.debug("Update flow after validation"); - agamaFlowService.updateFlow(existingFlow); + // validate flow data + validateAgamaFlowData(existingFlow, false); + logger.debug("Update flow after validation"); + agamaFlowService.updateFlow(existingFlow); - existingFlow = findFlow(existingFlow.getQname(), true, false); - } - return Response.status(Response.Status.OK).entity(existingFlow).build(); + existingFlow = findFlow(existingFlow.getQname(), true); + return Response.status(Response.Status.OK).entity(minimize(existingFlow, false)).build(); } @PATCH @@ -220,7 +218,7 @@ public Response patchFlow(@PathParam(ApiConstants.QNAME) @NotNull String flowNam logger.debug(" Flow to be patched is name:{}", decodedFlowName); // check if flow exists - Flow existingFlow = findFlow(decodedFlowName, false, true); + Flow existingFlow = findFlow(decodedFlowName, false); logger.debug(" Flow to be patched:{}", existingFlow); existingFlow = Jackson.applyJsonPatch(jsonPatch, existingFlow); @@ -230,37 +228,28 @@ public Response patchFlow(@PathParam(ApiConstants.QNAME) @NotNull String flowNam validateAgamaFlowData(existingFlow, false); logger.debug("Updating flow after validation"); agamaFlowService.updateFlow(existingFlow); - return Response.ok(existingFlow).build(); + return Response.ok(minimize(existingFlow, false)).build(); } @DELETE @Path(ApiConstants.QNAME_PATH) @ProtectedApi(scopes = { ApiAccessConstants.AGAMA_DELETE_ACCESS }) - public Response deleteAttribute(@PathParam(ApiConstants.QNAME) @NotNull String flowName) { + public Response delete(@PathParam(ApiConstants.QNAME) @NotNull String flowName) { logger.debug(" Flow to delete - flowName:{}", flowName); String decodedFlowName = getURLDecodedValue(flowName); logger.trace(" Agama Decoded flow name is:{}", decodedFlowName); // check if flow exists - Flow flow = findFlow(decodedFlowName, true, false); + Flow flow = findFlow(decodedFlowName, true); - agamaFlowService.removeAgamaFlow(flow); + agamaFlowService.removeAgamaFlow(flowName); return Response.noContent().build(); } - private Flow findFlow(String flowName, boolean throwError, boolean includeSource) { + private Flow findFlow(String flowName, boolean throwError) { Flow flow = null; try { flow = agamaFlowService.getFlowByName(flowName); - - // filter values - if (flow != null) { - List flows = Arrays.asList(flow); - getAgamaFlowDetails(flows, includeSource); - if (flows != null && !flows.isEmpty()) { - flow = flows.get(0); - } - } } catch (EntryPersistenceException e) { logger.error("No flow found with the name:{} ", flowName); if (throwError) { @@ -297,20 +286,10 @@ private void validateSyntax(Flow flow) { // validate syntax try { Transpiler.runSyntaxCheck(flow.getQname(), flow.getSource()); - } catch (SyntaxException se) { - logger.error("Transpiler syntax check error", se); - se.setStackTrace(new StackTraceElement[0]); - try { - logger.debug("Throwing BadRequestException 400 :{} ", Jackson.asPrettyJson(se)); - thorwBadRequestException(se); - } catch (IOException io) { - logger.error("Agama Flow Transpiler syntax error parsing error", io); - thorwBadRequestException("Transpiler syntax check error" + se); - } - } catch (TranspilerException te) { - logger.error("Agama Flow transpiler exception", te); - te.setStackTrace(new StackTraceElement[0]); - thorwBadRequestException(te); + } catch (SyntaxException | TranspilerException e) { + logger.error("Transpiler exception", e); + e.setStackTrace(new StackTraceElement[0]); + thorwBadRequestException(e); } } @@ -324,7 +303,7 @@ private String getURLDecodedValue(String pathParam) { return pathParam; } - private Flow updateFlowDetails(Flow flow, Flow existingFlow, boolean updateMetadata) { + private Flow updateFlowDetails(Flow flow, Flow existingFlow, boolean updateMetadata) { logger.debug("Update Flow details - flow:{}, existingFlow:{}, updateMetadata:{}", flow, existingFlow, updateMetadata); updateRevision(flow, existingFlow); @@ -382,23 +361,16 @@ private Flow updateMetadata(Flow flow) { return flow; } - private List getAgamaFlowDetails(List flows, boolean includeSource) { + private Flow minimize(Flow flow, boolean includeSource) { - logger.debug("Flow data filter - flows:{}, includeSource:{}", flows, includeSource); - if (flows == null || flows.isEmpty()) { - return flows; + flow.setTranspiled(null); + flow.setTransHash(null); + flow.setCodeError(null); + if (!includeSource) { + flow.setSource(null); } - - for (Flow flow : flows) { - flow.setTranspiled(null); - flow.setTransHash(null); - - if (!includeSource) { - flow.setSource(null); - } - - } - return flows; + return flow; } + } diff --git a/jans-config-api/server/src/main/java/io/jans/configapi/service/auth/AgamaFlowService.java b/jans-config-api/server/src/main/java/io/jans/configapi/service/auth/AgamaFlowService.java index d299806a747..d1398b2c3bc 100644 --- a/jans-config-api/server/src/main/java/io/jans/configapi/service/auth/AgamaFlowService.java +++ b/jans-config-api/server/src/main/java/io/jans/configapi/service/auth/AgamaFlowService.java @@ -114,9 +114,9 @@ public void updateFlow(Flow flow) { persistenceEntryManager.merge(flow); } - public void removeAgamaFlow(Flow flow) { - logger.debug("Remove Agama Flow:{}", flow); - persistenceEntryManager.removeRecursively(flow.getDn(), Flow.class); + public void removeAgamaFlow(String flowName) { + logger.debug("Remove Agama Flow:{}", flowName); + persistenceEntryManager.removeRecursively(getAgamaFlowDn(flowName), Flow.class); } public String getAgamaFlowDn(String flowName) { @@ -162,7 +162,7 @@ public String validateFlowFields(Flow flow, List mandatoryAttributes, Li Object attributeValue = null; for (String attribute : mandatoryAttributes) { - logger.debug("Flow class objectPropertyMap:{} conatins attribute:{} ? :{} ", objectPropertyMap, attribute, + logger.debug("Flow class objectPropertyMap:{} contains attribute:{} ? :{} ", objectPropertyMap, attribute, keys.contains(attribute)); if (keys.contains(attribute)) {