Skip to content

Refuse APPLICATION_JSON to avoid data loss#390

Closed
symphony-enrico wants to merge 1 commit intoapache:developfrom
symphony-enrico:develop
Closed

Refuse APPLICATION_JSON to avoid data loss#390
symphony-enrico wants to merge 1 commit intoapache:developfrom
symphony-enrico:develop

Conversation

@symphony-enrico
Copy link
Copy Markdown
Contributor

If APPLICATION_JSON is accepted when posting a SCIM entity, the custom JacksonXmlBindJsonProvider is not used. This is a serious problem, because the REST API appears to work without any errors, but in reality all data relating to the use of a schema extension is lost. This behavior can confuse the user and be very dangerous, it is better to reject the application/json format

If APPLICATION_JSON is accepted when posting a SCIM entity, the custom JacksonXmlBindJsonProvider is not used. This is a serious problem, because the REST API appears to work without any errors, but in reality all data relating to the use of a schema extension is lost. This behavior can confuse the user and be very dangerous, it is better to reject the application/json format
@symphony-enrico
Copy link
Copy Markdown
Contributor Author

@BacemSymphony

@bdemers
Copy link
Copy Markdown
Member

bdemers commented Oct 11, 2023

Do you want to try tweaking the JSON provider?

https://github.com/apache/directory-scimple/blob/develop/scim-server/src/main/java/org/apache/directory/scim/server/rest/ScimJacksonXmlBindJsonProvider.java#L37

My preference would be to try to make sure everything works with application/json. But if that doesn't work we could merged this for the short term. (as you mentioned, it's better to NOT loose data)

Per spec:

Service providers MUST support the "Accept" header "Accept: application/scim+json" and SHOULD support the header "Accept: application/json", both of which specify JSON documents conforming to [RFC7159].

@symphony-enrico
Copy link
Copy Markdown
Contributor Author

Do you want to try tweaking the JSON provider?

https://github.com/apache/directory-scimple/blob/develop/scim-server/src/main/java/org/apache/directory/scim/server/rest/ScimJacksonXmlBindJsonProvider.java#L37

My preference would be to try to make sure everything works with application/json. But if that doesn't work we could merged this for the short term. (as you mentioned, it's better to NOT loose data)

Per spec:

Service providers MUST support the "Accept" header "Accept: application/scim+json" and SHOULD support the header "Accept: application/json", both of which specify JSON documents conforming to [RFC7159].

My concern is that if the framework is integrated in a project that implements other endpoints (not related to scim protocol), using scim custom mapper for all endpoints (standard ones in application/json) it could be a problem.

I don't know if it is possible to bind the custom mapper only for scim related endpoints... or at least to have a configuration option to bind custom object mapper to json or not according needs... WDYT ?

@bdemers
Copy link
Copy Markdown
Member

bdemers commented Oct 11, 2023

I think that is a valid point, but you should be able to define multiple JAX-RS apps in a project.

bdemers added a commit that referenced this pull request Nov 3, 2023
Previously application/json requests may have lost any custom extension data.

This change adds applicaiton/json to the list of supproted media types (as recommended by the SCIM specs), but restricts it's types to ONLY classes known by SCIMple, and allows other MessageBodyReader/Writer to handle other requests

Fixes: #390
@bdemers bdemers closed this in #404 Nov 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants