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

Identity gets corrupted and associated data goes missing. #17673

Open
geoidesic opened this issue Apr 22, 2024 · 0 comments
Open

Identity gets corrupted and associated data goes missing. #17673

geoidesic opened this issue Apr 22, 2024 · 0 comments
Labels
Milestone

Comments

@geoidesic
Copy link

geoidesic commented Apr 22, 2024

Description

Hullo all. v4.x I'm getting a strange error where the $identity becomes corrupted. I haven't been able to figure out why. It happens after a particular error gets thrown but it's not an error that should affect the identity: This is after a successful login where the identity->role->permissions are filled, for some reason the permissions become empty. Any ideas?

2024-03-16 18:16:41 error: [TypeError] Argument 1 passed to Cake\Collection\Collection::__construct() must be iterable, null given, called in /var/www/html/mnr-be/src/Service/PermissionCheckerService.php on line 14 in /var/www/html/mnr-be/vendor/cakephp/cakephp/src/Collection/Collection.php on line 38
Stack Trace:
- /var/www/html/mnr-be/src/Service/PermissionCheckerService.php:14
- /var/www/html/mnr-be/src/Policy/RequestPolicy.php:79
- /var/www/html/mnr-be/vendor/cakephp/authorization/src/AuthorizationService.php:92
- /var/www/html/mnr-be/vendor/cakephp/authorization/src/AuthorizationService.php:65
- /var/www/html/mnr-be/vendor/cakephp/authorization/src/Middleware/RequestAuthorizationMiddleware.php:101
- /var/www/html/mnr-be/vendor/cakephp/cakephp/src/Http/Runner.php:82
- /var/www/html/mnr-be/vendor/cakephp/authorization/src/Middleware/AuthorizationMiddleware.php:129
- /var/www/html/mnr-be/vendor/cakephp/cakephp/src/Http/Runner.php:82
- /var/www/html/mnr-be/vendor/cakephp/authentication/src/Middleware/AuthenticationMiddleware.php:124
- /var/www/html/mnr-be/vendor/cakephp/cakephp/src/Http/Runner.php:82
- /var/www/html/mnr-be/vendor/cakephp/cakephp/src/Http/Runner.php:86
- /var/www/html/mnr-be/vendor/cakephp/cakephp/src/Http/Middleware/BodyParserMiddleware.php:172
- /var/www/html/mnr-be/vendor/cakephp/cakephp/src/Http/Runner.php:82
- /var/www/html/mnr-be/plugins/Cors/src/Middleware/CorsMiddleware.php:27
- /var/www/html/mnr-be/vendor/cakephp/cakephp/src/Http/Runner.php:82
- /var/www/html/mnr-be/vendor/cakephp/cakephp/src/Http/Runner.php:67
- /var/www/html/mnr-be/vendor/cakephp/cakephp/src/Routing/Middleware/RoutingMiddleware.php:192
- /var/www/html/mnr-be/vendor/cakephp/cakephp/src/Http/Runner.php:82
- /var/www/html/mnr-be/vendor/cakephp/cakephp/src/Routing/Middleware/AssetMiddleware.php:68
- /var/www/html/mnr-be/vendor/cakephp/cakephp/src/Http/Runner.php:82
- /var/www/html/mnr-be/vendor/cakephp/cakephp/src/Error/Middleware/ErrorHandlerMiddleware.php:131
- /var/www/html/mnr-be/vendor/cakephp/cakephp/src/Http/Runner.php:82
- /var/www/html/mnr-be/vendor/cakephp/cakephp/src/Http/Runner.php:67
- /var/www/html/mnr-be/vendor/cakephp/cakephp/src/Http/Server.php:90
- /var/www/html/mnr-be/webroot/index.php:43

Request URL: /xero/invoice
Referer URL: https://fe.mnr.localhost/
Client IP: 192.168.65.1
This is making debugging the original error challenging because it basically keeps breaking my session. (edited) 

2 replies
Last reply 1 month agoView thread

Kevin Pfeifer
  [10:05 AM](https://cakesf.slack.com/archives/C053DPNGT/p1710669930003589)
the entity is retrieved from the session.
so if you do any updates/adjustments of the entity in the DB you also need to set/update the entity via the Authentication component to have the up2date value everywhere you expect it to (edited) 
[10:07](https://cakesf.slack.com/archives/C053DPNGT/p1710670022920879)
something like $this->Authentication->setIdentity($entity);


dereuromark
  [10:41 AM](https://cakesf.slack.com/archives/C053DPNGT/p1710672119177979)
Is cake actually storing the identity as object (serialized) in session here? That would be quite the antipattern imo. Should be an array instead.


Kevin Pfeifer
  [10:52 AM](https://cakesf.slack.com/archives/C053DPNGT/p1710672723241909)
it indeed is a serialized entity in the session
[10:54](https://cakesf.slack.com/archives/C053DPNGT/p1710672853888999)
well the FileEngine (which i use for sessions) does the serialization


dereuromark
  [11:52 AM](https://cakesf.slack.com/archives/C053DPNGT/p1710676336449149)
very bad idea :slightly_smiling_face:
breaks everyones session on the smallest possible change for no good reason


Kevin Pfeifer
  [12:06 PM](https://cakesf.slack.com/archives/C053DPNGT/p1710677218796519)
why? I don’t understand


dereuromark
  [12:13 PM](https://cakesf.slack.com/archives/C053DPNGT/p1710677602176519)
storing serialized objects is always an anti pattern if the data could be stored using to/from array instead as pure data.


Kevin Pfeifer
  [12:13 PM](https://cakesf.slack.com/archives/C053DPNGT/p1710677627898469)
ah, you mean this old topic of yours :stuck_out_tongue:


dereuromark
  [12:14 PM](https://cakesf.slack.com/archives/C053DPNGT/p1710677671630849)
its not my topic. its common sense.


Kevin Pfeifer
  [12:15 PM](https://cakesf.slack.com/archives/C053DPNGT/p1710677701459139)
well I just mean you yourself did serializing in the DB till recently in your queue plugin


dereuromark
  [12:15 PM](https://cakesf.slack.com/archives/C053DPNGT/p1710677735582599)
No, I did not. Used json for years. And for that reason. It sure was time to get rid of legacy stuff, that's true.


Kevin Pfeifer
  [12:17 PM](https://cakesf.slack.com/archives/C053DPNGT/p1710677839407819)
anyway… the session value is read and taken as is as you can see here
https://github.com/cakephp/authentication/blob/3.x/src/Authenticator/SessionAuthenticator.php#L61-L83


ADmad
  [4:30 PM](https://cakesf.slack.com/archives/C053DPNGT/p1710693038028089)
That fact that the entity object is stored in the session has nothing to do with Noel's problem. The problem is the single pass middlewares in which the changes to the request instance don't bubble up the queue.
The ErrorHandlerMiddleware is the first in the queue and hence the request instance it has is an older one and doesn't hold the auth identity which is added later by the AuthMiddleware. That's why he sees the "corruption" only when an exception occurs.

[Update: I tried moving the ErrorHandlerMiddleware to the end of the middleware queue but I get the same problem – Noel]
1 reply
1 month agoView thread


Kevin Pfeifer
  [5:00 PM](https://cakesf.slack.com/archives/C053DPNGT/p1710694810620609)
his stacktrace clearly goes beyond the authenticationmiddleware which means the identity is fetched and set in the request object.
Yes, if an exception is being thrown and not caught before the default ErrorHandler then the request object won’t have the identity in there but I feel this is not his problem.


dereuromark
  [6:18 PM](https://cakesf.slack.com/archives/C053DPNGT/p1710699523383489)
Yeah, they are not directly linked, those things
But we should definitely clean that up too, e.g. providing a flag that checks all objects are transferred to their serialized value as json, string or array.


[6:19](https://cakesf.slack.com/archives/C053DPNGT/p1710699554787739)
Bool false for bc

--

Something else I noticed. If I do the request from a browser, this corruption happens. If I do it from POSTMAN, it doesn't happen. I also noticed that it was happening when the memory gets overrun on a request. I had a debug logging library that was trying to output very large nested arrays and was foreach looping over them, and each was being called about 450 times on a given request. I think specifically I was trying to log the exception object within CakeException.

CakePHP Version

4.4.0

PHP Version

7.4

@markstory markstory added this to the 5.0.8 milestone Apr 22, 2024
@markstory markstory modified the milestones: 5.0.8, 5.0.9 May 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants