ManageIQ / manageiq-ui-classic

Classic UI of ManageIQ
Apache License 2.0
50 stars 358 forks source link

Fix issue when using kerberos auth with Rails 7 #9295

Closed Fryguy closed 1 month ago

Fryguy commented 1 month ago

kerberos auth requires a wait_for_task under the covers. When we create this task, we store a backup copy of the incoming request params within the session object, so we can restore it later after the task we are waiting for has completed, and that session object is Marshal.dumped into memcached. In Rails 7, Rails changed the ActionController::Parameters object to include some more objects as context, such as the request, and the request internally has an IO object that cannot be dumped. Since we don't actually need the context and instead only need the parameters, we can use to_unsafe_h instead of deep_dup, to get a copy of those parameters in HashWithIndifferentAccess form. Later when wait_for_task reconstructs the parameters, they will be placed back into the request object, which will get them back into the ActionController::Parameters format.

@jrafanie Please review.

jrafanie commented 1 month ago

From DMs, we found that this change is what added more context around the request parameters, and we believe the request object is what is a problem when it tries to Marshal it. See: https://www.github.com/rails/rails/pull/41809/files#diff-99dab0dfb4d0cfa044997480d5aa1b100dc60347a10cedd6d8f7a0395f6a6efdR1194

jrafanie commented 1 month ago

I wish I knew how to test this properly.

Fryguy commented 1 month ago

Yeah I wanted to add specs, but I was having problems figure out how to add them. I'm hoping to do it in a followup. Note that I tested this manually using the FreeIPA demo server, and I could replicate the initial bug, and then it's fixed afterwards.

jrafanie commented 1 month ago

I'm fine with merging this as is. We can add a test later if we figure out a good way to reliably test it.

Fryguy commented 1 month ago

Backported to radjabov in commit 54d72d01a265cfa26066e9a8e4e07cb386f66905.

commit 54d72d01a265cfa26066e9a8e4e07cb386f66905
Author: Joe Rafaniello <jrafanie@users.noreply.github.com>
Date:   Tue Oct 22 17:08:22 2024 -0400

    Merge pull request #9295 from Fryguy/fix_marshal_dump

    Fix issue when using kerberos auth with Rails 7

    (cherry picked from commit fd3d3e9749f1a791f9404214abf564145c01d57f)