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.

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

POST duplicate entry not causing PK collision in Spring Data REST [DATAREST-1007] #1370

Open
spring-projects-issues opened this issue Feb 14, 2017 · 3 comments
Assignees
Labels
in: repository status: feedback-provided type: bug

Comments

@spring-projects-issues
Copy link

spring-projects-issues commented Feb 14, 2017

Jianzhao Li opened DATAREST-1007 and commented

As per Spring Data REST Documentation, POST method creates a new entity from the given request body. However, I found it could also update the existing entity. In some cases, this can be problematic. Here is an example:

DemoApplication.java

package com.example;

import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;

@SpringBootApplication
public class DemoApplication {

    public static void main(String[] args) {
        SpringApplication.run(DemoApplication.class, args);
    }
}

UserRepository.java

package com.example;

import org.springframework.data.repository.PagingAndSortingRepository;

public interface UserRepository extends PagingAndSortingRepository<User, String> {}

User.java

package com.example;

import javax.persistence.Entity;
import javax.persistence.Id;

@Entity
public class User {
    @Id
    private String username;
    private String password;
    public String getUsername() {
        return username;
    }
    public void setUsername(String username) {
        this.username = username;
    }
    public String getPassword() {
        return password;
    }
    public void setPassword(String password) {
        this.password = password;
    }
}

pom.xml (within project tag)

<modelVersion>4.0.0</modelVersion>

<groupId>com.example</groupId>
<artifactId>demo</artifactId>
<version>0.0.1-SNAPSHOT</version>
<packaging>jar</packaging>

<name>demo</name>
<description>Demo project for Spring Boot</description>

<parent>
    <groupId>org.springframework.boot</groupId>
    <artifactId>spring-boot-starter-parent</artifactId>
    <version>1.5.1.RELEASE</version>
    <relativePath/> <!-- lookup parent from repository -->
</parent>

<properties>
    <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
    <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
    <java.version>1.8</java.version>
</properties>

<dependencies>
    <dependency>
        <groupId>org.springframework.boot</groupId>
        <artifactId>spring-boot-starter-data-jpa</artifactId>
    </dependency>
    <dependency>
        <groupId>org.springframework.boot</groupId>
        <artifactId>spring-boot-starter-data-rest</artifactId>
    </dependency>

    <dependency>
        <groupId>com.h2database</groupId>
        <artifactId>h2</artifactId>
        <scope>runtime</scope>
    </dependency>
    <dependency>
        <groupId>org.springframework.boot</groupId>
        <artifactId>spring-boot-starter-test</artifactId>
        <scope>test</scope>
    </dependency>
</dependencies>

<build>
    <plugins>
        <plugin>
            <groupId>org.springframework.boot</groupId>
            <artifactId>spring-boot-maven-plugin</artifactId>
        </plugin>
    </plugins>
</build>
{cdemo}

application.properties: empty

URL: http://localhost:8080/users

Method: POST

JSON content:

{"username":"user","password":"password"}
I assumed the above POST request was getting HTTP 201 at the first time, and only for one time. However, I was able to send the above POST request many times, and got HTTP 201 all the time. In addition, I was also able to change the password in the database using POST request.

I believe that this is a security problem. For example, I might allow anonymous user registration through a POST request. But, with above situation, the existing user could be overwritten.

Question: How can I prevent a new entity being created from a POST request if an old entity has already existed with the same id? Or, did I miss interpret the Spring Data REST Documentation?
Supplementary explanation:

The cause of this issue is the design behind Spring Data REST. Because Spring Data REST is built upon Spring Data JPA, which was not used to directly expose to the "outside". So it "trusts" data that comes in. The method isNew in org.springframework.data.repository.core.support.AbstractEntityInformation shows how data is determined as new or not new.

```java 
public boolean isNew(T entity) {

    ID id = getId(entity);
    Class<ID> idType = getIdType();

    if (!idType.isPrimitive()) {
        return id == null;
    }

    if (id instanceof Number) {
        return ((Number) id).longValue() == 0L;
    }

    throw new IllegalArgumentException(String.format("Unsupported primitive id type %s!", idType));
}

The result of isNew method will eventually effects save method in org.springframework.data.jpa.repository.support.SimpleJpaRepository.

public <S extends T> S save(S entity) {

    if (entityInformation.isNew(entity)) {
        em.persist(entity);
        return entity;
    } else {
        return em.merge(entity);
    }
}

In the situation mentioned in this question, the username field, which is also the id of the user entity, would always contain data in order to create new users. Therefore, when it goes to isNew, id == null will always return false. Then save method will always perform a merge operation


Affects: 2.6 GA (Ingalls)

Reference URL: http://stackoverflow.com/questions/42216997/post-duplicate-entry-not-causing-pk-collision-in-spring-data-rest

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Feb 14, 2017

Oliver Drotbohm commented

I think we can cut this short: this is not related to Spring Data REST at all but caused by your (partially correct) identifier setup. As documented, Spring Data inspects the identifier property and only treats entities with a null, non-primitive value as new. As you hide this behind a manually handled property, the second call issues a merge(…) which eventually results in an update.

As documented, you can tweak this by implementing Persistable or rather switching to a dedicated auto-applied, synthetical identifier

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Feb 14, 2017

Jianzhao Li commented

I'm sorry. I didn't realize I can use @Version along with Persistable to solve this issue. Thank you very much!

@spring-projects-issues spring-projects-issues added status: waiting-for-feedback type: bug in: repository labels Dec 31, 2020
@spring-projects-issues
Copy link
Author

spring-projects-issues commented Jan 7, 2021

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder label Jan 7, 2021
@gregturn gregturn added status: feedback-provided and removed status: feedback-reminder status: waiting-for-feedback labels Jan 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: repository status: feedback-provided type: bug
Projects
None yet
Development

No branches or pull requests

3 participants