Skip to content

Commit

Permalink
fix(jans-config-api): avoid loss of attributes in agama endpoints (#2058
Browse files Browse the repository at this point in the history 
)

* fix: avoid data loss #1982

* docs: remove PUT json endpoint #1982

* chore: remove unnecessary check #1982
  • Loading branch information
@jgomer2001
jgomer2001 committed Aug 9, 2022
1 parent 2ffac4d commit 3c8f816
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 106 deletions.
31 changes: 2 additions & 29 deletions jans-config-api/docs/jans-config-api-swagger.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down
118 changes: 45 additions & 73 deletions jans-config-api/server/src/main/java/io/jans/configapi/rest/resource/auth/AgamaResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}

Expand All @@ -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
Expand All @@ -98,37 +98,37 @@ 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
updateFlowDetails(flow, null, true);
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);

String decodedFlowName = getURLDecodedValue(flowName);
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();
Expand All @@ -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 })
Expand All @@ -171,37 +173,33 @@ 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);

String decodedFlowName = getURLDecodedValue(flowName);
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
Expand All @@ -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);
Expand All @@ -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<Flow> 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) {
Expand Down Expand Up @@ -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);
}
}

Expand All @@ -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);
Expand Down Expand Up @@ -382,23 +361,16 @@ private Flow updateMetadata(Flow flow) {
return flow;
}

private List<Flow> getAgamaFlowDetails(List<Flow> 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;

}

}
8 changes: 4 additions & 4 deletions jans-config-api/server/src/main/java/io/jans/configapi/service/auth/AgamaFlowService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -162,7 +162,7 @@ public String validateFlowFields(Flow flow, List<String> 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)) {
Expand Down

0 comments on commit 3c8f816

Please sign in to comment.