- Issue created by @teknocat
- 🇨🇦Canada teknocat
The permissions provided by this module should also account for the fact that the core taxonomy module now provides a permission to reset any. As such, maybe this should actually be removed altogether from this module?
- 🇩🇪Germany FeyP
Thank you very much for filing this issue, it's really appreciated.
Are you sure that you get this error on an unpatched core 9.5.3? The core issue for that change was recently committed to 10.1.x, but I can't find the requirement change in 9.5.x and 10.0.x and I can't reproduce the error on various real world installations of 9.5.x and 10.0.x. Are you maybe using a patch from that issue in your 9.5.3 (see the related issue I just added)? Or maybe you are using another contributed or custom module that modifies this route? Any candidates?
Of course we still need to fix this since it will be released in core 10.1.x and I will look into preparing a patch, if no one else beats me to it, that you could then probably also use on 9.5.x, if you're currently using a patch from that core issue, but given that 10.1.x has not been released yet, I don't consider it that urgent.
W/r/t to the part of the proposed resolution where your suggestion is to not throw an exception when the verification fails: If the requirements don't match our expectations, we can't guarantee that our access control handler returns correct access results. If we just ignore this, it may result in an access bypass. That's why we throw the exception so that it is immediately clear that the module might not work as expected. So this is a safe guard against potential security vulnerabilities introduced through unexpected changes in core or use of other incompatible contrib or custom code. This has actually happened to this module in the past (before I joined as a co-maintainer), so it's not theoretical. Just logging a notice will not work to prevent a security advisory. That's why I don't want to remove this exception. In my opinion, the correct way to deal with it is just to file an issue for any incompatibility encountered and if it's in core or contrib, we'll fix the code to work with the new requirements. Having a user file an issue, like you did, was exactly what I hoped for in such a case, so in my opinion the exception worked exactly as intended.
- 🇨🇦Canada teknocat
Ah yes, indeed I was using the patch from that related core issue.
If I remove that patch, then I can use the latest version of this module without issue and that provides the reset permissions and it all works.
So options for anyone until the release of 10.1.x and the appropriate update to this module to go along with that are either:
a) Use the core patch and use a workaround to continue using version 3.3 of this module, or
b) Don't use the core patch and use the latest version of this module - Status changed to Needs review
over 1 year ago 12:50pm 1 May 2023 - last update
over 1 year ago 11 pass - last update
over 1 year ago 11 pass - last update
over 1 year ago 11 pass - 🇯🇴Jordan Rajab Natshah Jordan
Thank you, Patrick
The quick fix patch from #6 is working with
4.0.0-beta1
and Drupal 10.1.0-alpha1 - Status changed to Fixed
over 1 year ago 12:02am 2 May 2023 Automatically closed - issue fixed for 2 weeks with no activity.
- Status changed to Fixed
about 1 year ago 4:21pm 28 September 2023 - 🇮🇳India rajesh.vishwakarma
Facing issue with Drupal 10.1.4
I recently updated my core 9.5.8 to -> 10.1.4
During the update, I received this as a fatal error and stopped the update. So I disabled taxonomy_access_fix module and finished the update. After the successful update, I enabled taxonomy_access_fix module but got the same issue. It is also showing with drush cr - 🇩🇪Germany Lutfiyeturan Stuttgart
I also still have this issue with Drupal 10.1.5
Appears when i do drush crUnexpected requirements of /admin/structure/taxonomy/manage/{taxonomy_vocabulary}/reset route. This might be due to an unexpected change in Drupal Core or due to a conflict with another contributed or custom module in use.
- 🇩🇪Germany axroth Stuttgart
also still having this issue on 9.5.11 when updating the module to ^4.
- 🇩🇪Germany FeyP
Thanks for letting us know. Looking at the code in core I don't see any recent changes that we didn't address and I haven't had any issues myself on a bunch of systems.
Most likely this is either a core patch that you're using that modifies the route, another contributed module that alters this route or some custom code you use. Could you please check your systems for any candidates? If it's a contrib module, we might be able to accommodate with a patch.
- 🇩🇪Germany axroth Stuttgart
I could fix this only by deleting the cache tables in db. drush cr, did not solve it.
> another contributed module that alters this route
that route is not mentioned at all in custom or contrib code, that's why I researched and found this ticket where others report the same issue. I don't see an obvious issue in the modules code as well.
- 🇩🇪Germany FeyP
I could fix this only by deleting the cache tables in db. drush cr, did not solve it.
That's interesting. So it's not related to the code at all, but some kind of caching issue? I'll see if I can find something in that direction. Might be a few days until I can take a look though.