Skip to content

UsernamePasswordAuthenticationTokenDeserializer doesn't deserialize details to correct type #7482

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.

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

Closed
buzzerrookie opened this issue Sep 27, 2019 · 10 comments · Fixed by #7660
Assignees
Labels
in: core An issue in spring-security-core type: breaks-passivity A change that breaks passivity with the previous release
Milestone

Comments

@buzzerrookie
Copy link
Contributor

buzzerrookie commented Sep 27, 2019

UsernamePasswordAuthenticationTokenDeserializer doesn't deserialize details to correct type

When using Spring Security and Spring Session with GenericJackson2JsonRedisSerializer, UsernamePasswordAuthenticationTokenDeserializer deserializes the details field of UsernamePasswordAuthenticationToken as a JsonNode, not the original Object such as WebAuthenticationDetails.

Actual Behavior

the details field of UsernamePasswordAuthenticationToken is deserialized as a com.fasterxml.jackson.databind.node.ObjectNode.

Expected Behavior

the details field of UsernamePasswordAuthenticationToken should be deserialized as a object of type @ class.

Configuration

I replace the default JdkSerializationRedisSerializer with GenericJackson2JsonRedisSerializer.

@EnableWebSecurity
@Configuration
public class WebSecurityConfig extends WebSecurityConfigurerAdapter  {

    // other beans and methods omitted

    @Bean
    public RedisSerializer<Object> springSessionDefaultRedisSerializer() {
        ObjectMapper mapper = new ObjectMapper();
        mapper.registerModules(new CoreJackson2Module(), new WebJackson2Module());
        return new GenericJackson2JsonRedisSerializer(mapper);
    }
}

Version

I'm using Spring Security 4.2.2, but I also find the same issue in master branch.

Sample

The relevant code is as follows, and I add some comments in it.

class UsernamePasswordAuthenticationTokenDeserializer extends JsonDeserializer<UsernamePasswordAuthenticationToken> {

    @Override
    public UsernamePasswordAuthenticationToken deserialize(JsonParser jp, DeserializationContext ctxt) throws IOException,
            JsonProcessingException {
        UsernamePasswordAuthenticationToken token = null;
        ObjectMapper mapper = (ObjectMapper) jp.getCodec();
        JsonNode jsonNode = mapper.readTree(jp);
        Boolean authenticated = readJsonNode(jsonNode, "authenticated").asBoolean();
        JsonNode principalNode = readJsonNode(jsonNode, "principal");
        Object principal = null;
        if(principalNode.isObject()) {
            principal = mapper.readValue(principalNode.toString(), new TypeReference<User>() {});
        } else {
            principal = principalNode.asText();
        }
        Object credentials = readJsonNode(jsonNode, "credentials").asText();
        List<GrantedAuthority> authorities = mapper.readValue(
                readJsonNode(jsonNode, "authorities").toString(), new TypeReference<List<GrantedAuthority>>() {
                });
        if (authenticated) {
            token = new UsernamePasswordAuthenticationToken(principal, credentials, authorities);
        } else {
            token = new UsernamePasswordAuthenticationToken(principal, credentials);
        }
        token.setDetails(readJsonNode(jsonNode, "details")); // it just makes details deserialized as a JsonNode
        return token;
    }

    private JsonNode readJsonNode(JsonNode jsonNode, String field) {
        return jsonNode.has(field) ? jsonNode.get(field) : MissingNode.getInstance();
    }
}

the following code works.

JsonNode detailsNode = readJsonNode(jsonNode, "details");
Object details = mapper.readValue(detailsNode.toString(), new TypeReference<Object>() {});
token.setDetails(details);
@eleftherias
Copy link
Contributor

@buzzerrookie Is there a reason why you are only registering these specific modules?

mapper.registerModules(new CoreJackson2Module(), new WebJackson2Module());

Please take a look at the Spring Security Jackson documentation.
You may also find this sample project from Spring Session useful, as it is also using a GenericJackson2JsonRedisSerializer.

@eleftherias eleftherias added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 28, 2019
@eleftherias eleftherias self-assigned this Nov 28, 2019
@buzzerrookie
Copy link
Contributor Author

@eleftherias
List<Module> modules = SecurityJackson2Modules.getModules(loader); mapper.registerModules(modules);
The code in the doc registers CoreJackson2Module, CasJackson2Module and WebJackson2Module. There's no need for CAS in my project, so I only register those two specific modules.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 28, 2019
@eleftherias
Copy link
Contributor

@buzzerrookie Can you please provide a minimal sample that reproduces this issue?
Currently I am not able to reproduce it.

@eleftherias eleftherias added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Nov 29, 2019
@buzzerrookie
Copy link
Contributor Author

@eleftherias I uploaded a minimal sample project as follows.
minimal-sample.zip
A user with username "test1" and password "123456" is configured, and you can use Basic Authentication to gain authentication.
When I visit "/test" path for the first time, it prints true, but then I visit "/test" path for the second time, it prints false.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 29, 2019
@RobbenPeng
Copy link

@eleftherias Hi,I also meet the same problem as @buzzerrookie does.And I have try to do as what you say, but it doesn't work. I am using <artifactId>spring-session-data-redis</artifactId> <version>2.2.0.RELEASE</version>,<artifactId>spring-boot-starter-security</artifactId> <version>2.2.0.RELEASE</version>.

@RobbenPeng
Copy link

RobbenPeng commented Dec 2, 2019

@eleftherias I do as https://github.com/spring-projects/spring-session/tree/2.2.0.RELEASE/spring-session-samples/spring-session-sample-boot-redis-json) . Then it throws an new exception.Caused by: com.fasterxml.jackson.databind.JsonMappingException: The class with com.robben.bbs.config.AuthUser and name of com.robben.bbs.config.AuthUser is not whitelisted. If you believe this class is safe to deserialize, please provide an explicit mapping using Jackson annotations or by providing a Mixin. If the serialization is only done by a trusted source, you can also enable default typing. See https://github.com/spring-projects/spring-security/issues/4370 for details (through reference chain: org.springframework.security.core.context.SecurityContextImpl["authentication"]) .

@eleftherias
Copy link
Contributor

@chokPeng You issue appears to be different.
The original issue does not throw an exception, but rather deserializes the object in an unexpected way.
For your issue, try making you custom class com.robben.bbs.config.AuthUser implement Serializable.
If you believe there is something broken in Spring Security please create a separate GitHub issue.

@buzzerrookie
Copy link
Contributor Author

@eleftherias Can you reproduce this issue with my sample project ?

@eleftherias
Copy link
Contributor

@buzzerrookie Yes, thank you for providing the sample.
I initially misunderstood the issue, but I can now see the problem with the deserialization.
I will leave some comments in your PR.

@eleftherias eleftherias added in: core An issue in spring-security-core type: enhancement A general enhancement and removed status: feedback-provided Feedback has been provided labels Dec 13, 2019
@buzzerrookie
Copy link
Contributor Author

buzzerrookie commented Dec 14, 2019

@eleftherias I have pushed another commit to branch gh-7482. Could you please have a look to see whether it is ok? Thank you.

eleftherias pushed a commit that referenced this issue Dec 18, 2019
Before this commit, the details field was set to a JsonNode, but now it is deserialized correctly.

Fixes gh-7482
@eleftherias eleftherias added the type: breaks-passivity A change that breaks passivity with the previous release label Dec 27, 2019
@eleftherias eleftherias added this to the 5.3.0.M1 milestone Dec 27, 2019
@eleftherias eleftherias removed the type: enhancement A general enhancement label Dec 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core An issue in spring-security-core type: breaks-passivity A change that breaks passivity with the previous release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants