JanssenProject / jans

An open source enterprise digital identity platform for CIAM or workforce... Janssen is a distribution of standards-based, developer friendly, components that are engineered to work together in any cloud. #OAuth #OpenID #FIDO
https://docs.jans.io
Apache License 2.0
457 stars 74 forks source link

feat(jans-config-api): improvements to existing agama endpoints and create PUT for code only #1982

Closed jgomer2001 closed 2 years ago

jgomer2001 commented 2 years ago

Retrieval

Currently GETting all flows (/jans-config-api/api/v1/agama) returns all attributes; it should return fewer as mentioned here. Also, let's make single flow retrieval behave the same, ie. not outputing all attributes.

In some cases, one may like to have source in the output as well. Let's include it if a given request parameter is present, like /jans-config-api/api/v1/agama?includeSource. Applies for both GET endpoints

jgomer2001 commented 2 years ago

Creation

  1. POST to /jans-config-api/api/v1/agama has a typo when there is an unexpected attribute:

{"code":"400","message":"Value of these feilds should be null -> (transHash)."}

should be fields. The same goes for PUT

  1. POST to /jans-config-api/api/v1/agama/{qname} returns the error as a string in case of malformed source file, like this:

{"code":"400","message":"{\"error\":\"mismatched input 'hi' expecting 'Flow'\",\"symbol\":\"[@0,0:2='mua',<45>,1:0]\",\"line\":1,\"column\":0,\"message\":\"Syntax error: mismatched input 'mua' expecting 'Flow'\\nSymbol: [@0,0:2='mua',<45>,1:0]\\nLine: 1\\nColumn: 1\"}"}

but it should contain the SyntaxException as straight json, e.g:

{
    ...
    "error":"mismatched input 'hi' expecting 'Flow'",
    "symbol":"[@0,0:2='mua',<45>,1:0]",
    "line":1,
    "column":0,
    "message": "..."
}
  1. POST to /jans-config-api/api/v1/agama/{qname} returns the full json representation of the flow created. It should return an empty response or only the fields used for retrieval of single flow

  2. Small request for flow creation: can you set timestamp inside flow metadata equal to System.currentTimemillis please?

jgomer2001 commented 2 years ago

Modification

  1. An endpoint is missing. We need to be able to update a flow passing the source code only - no json - as mentioned here. Can you add a new PUT handler that consumes text/plain ?

  2. Regarding the existing PUT endpoint, I couldn't make it work. Locally I have an existing flow with qname re.move

Sending this payload: { "qname": "re.move" }

gives {"code":"400","message":"Required feilds missing -> (source). "} , however source is not required for an update.

Using this payload:

{
"qname": "re.move",
"source": "Flow re.move\n\tBasepath \"remove\"\nFinish true"
}

throws internal server error. Here are the logs:

configapi.log.txt

pujavs commented 2 years ago

Testing: Feedback implementation testing evidence Agama_endpoint_improvement_test_issue#1982.xlsx

Note for:

jgomer2001 commented 2 years ago

@pujavs

  1. Regarding PUT /jans-config-api/api/v1/agama/source/{qname}

Increasing jansRevision by 1 at every call here was a good decision, thanks. However you are also updating metadata/timestamp. This is a creation timestamp so there is no need to alter this property

If the flow has agFlowTrans and/or jansCustomMessage attributes populated in ldap already, they are wiped after calling this operation - not the case of jansScrError though. These three attributes are populated (and managed) internally by the engine; no tool should touch them.

Desired behavior is to alter source and jansRevision only

  1. Regarding PATCH

Every time this endpoint is called, revision and timestamp are refreshed automatically. This is not desired. If the caller wants to explicitly supply values for them, they can be updated.

Some properties are lost as in the previous case (PUT)

Desired behavior is to alter only the data sent in the patch

  1. About runSyntaxCheck invocation

can you call setStackTrace​(empty array) on the exception caught?. Sometimes, the whole stacktrace is serialized to json and it adds a lot of clutter

pujavs commented 2 years ago

@jgomer2001

agFlowTrans and/or jansCustomMessage attributes are not returned by the GET method as these are not part of Flow schema.

PUT: input is Flow object and directly updates the Flow object as received. Even though i manually check and exclude these attributes they should be retuned by the FETCH. PATCH: Same goes for PATCH. The existing Flow data is fetched from DB and only the attributed as received in JsonPatch operation is patched. Kindly advice how to fetch the values of these fields which are not part of Flow.java

DB schema: error when adding agFlowTransandjansCustomMessage` attributes. image image image

GET : does not return these agFlowTrans and jansCustomMessage attributes image