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

creating the same superuser twice create inconsistent state in accounts #537

Open
fvigotti opened this issue Oct 24, 2023 · 3 comments
Open

Comments

@fvigotti
Copy link

v 0.4.0 ( from the official docker image docker.io/apachepulsar/pulsar-manager:v0.4.0 )

this endpoint is not idempotent :


CSRF_TOKEN=$(curl http://127.0.0.1:7750/pulsar-manager/csrf-token)
curl \
    -H "X-XSRF-TOKEN: $CSRF_TOKEN" \
    -H "Cookie: XSRF-TOKEN=$CSRF_TOKEN;" \
    -H 'Content-Type: application/json' \
    -X PUT http://127.0.0.1:7750/pulsar-manager/users/superuser \
    -d '{"name": "test", "password": "apachepulsar", "description": "test", "email": "[email protected]"}'

nor is safe to use , it allow two same-name username creation which then crash the login services which expect unique usernames :

r
2023-10-24 09:52:05.296  INFO 31 --- [pool-8-thread-1] o.a.p.m.s.impl.BrokerStatsServiceImpl    : Start clearing stats from broker
2023-10-24 09:52:14.577 ERROR 31 --- [http-nio-7750-exec-7] o.a.c.c.C.[.[.[/].[dispatcherServlet]    : Servlet.service() for servlet [dispatcherServlet] in context with path [] threw exception [Request processing failed; nested exception is org.mybatis.spring.MyBatisSystemException: nested exception is org.apache.ibatis.exceptions.TooManyResultsException: Expected one result (or null) to be returned by selectOne(), but found: 2] with root cause

org.apache.ibatis.exceptions.TooManyResultsException: Expected one result (or null) to be returned by selectOne(), but found: 2
        at org.apache.ibatis.session.defaults.DefaultSqlSession.selectOne(DefaultSqlSession.java:80) ~[mybatis-3.5.4.jar:3.5.4]
        at sun.reflect.GeneratedMethodAccessor122.invoke(Unknown Source) ~[na:na]
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[na:1.8.0_342]
        at java.lang.reflect.Method.invoke(Method.java:498) ~[na:1.8.0_342]
        at org.mybatis.spring.SqlSessionTemplate$SqlSessionInterceptor.invoke(SqlSessionTemplate.java:426) ~[mybatis-spring-2.0.4.jar:2.0.4]
        at com.sun.proxy.$Proxy203.selectOne(Unknown Source) ~[na:na]
        at org.mybatis.spring.SqlSessionTemplate.selectOne(SqlSessionTemplate.java:159) ~[mybatis-spring-2.0.4.jar:2.0.4]
        at org.apache.ibatis.binding.MapperMethod.execute(MapperMethod.java:87) ~[mybatis-3.5.4.jar:3.5.4]

changing username works again , but again if you use the curl twice the error come up again .. during automated setup with timeout and retry functions where a curl that setup the account could hit twice is painful to handle also this problem which require to reset the all users accounts to be able to login again with the same users ( mistakenly duplicated by the endpoint )

those two functions ( user creation and user login ) should be made consistent about "username" uniqueness

@fvigotti
Copy link
Author

in UsersController
there is this check on already exists user :

    @RequestMapping(value = "/users/user", method = RequestMethod.PUT)
...
 Optional<UserInfoEntity> optionalUserEntity =  usersRepository.findByUserName(userInfoEntity.getName());
        if (optionalUserEntity.isPresent()) {
            result.put("error", "User already exist, please check");
            return ResponseEntity.ok(result);
        }

which is missing in this endpoint :

    @RequestMapping(value = "/users/superuser", method = RequestMethod.PUT)

@coreybutler
Copy link

I ran into this on 0.3.0

@maranmaran
Copy link
Contributor

@coreybutler @fvigotti #564 should solve this

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

No branches or pull requests

3 participants