Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(jans-config-api): avoid loss of attributes in agama endpoints #2058

Merged
merged 3 commits into from
Aug 9, 2022
Merged

fix(jans-config-api): avoid loss of attributes in agama endpoints #2058

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
58ce1a6
fix: avoid data loss #1982
jgomer2001 Aug 9, 2022
70cb50d
docs: remove PUT json endpoint #1982
jgomer2001 Aug 9, 2022
5126b2f
chore: remove unnecessary check #1982
jgomer2001 Aug 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 2 additions & 29 deletions jans-config-api/docs/jans-config-api-swagger.yaml
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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