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.

Sign up for GitHub

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

Jump to bottom

Execution order issue with load_and_authorize_resource? #531

Open
andrewculver opened this issue Sep 7, 2023 · 4 comments · May be fixed by #730
Open

Execution order issue with load_and_authorize_resource? #531

andrewculver opened this issue Sep 7, 2023 · 4 comments · May be fixed by #730
Assignees
@jagthedrummer

Comments

@andrewculver
Copy link
Contributor

andrewculver commented Sep 7, 2023 •
edited

Reported by @seattlecyclist:

We are having issues where account_load_and_authorize_resource is generating errors cause it is “skipping” authenticate_user!.
By skipping what I really mean is that this method is injecting methods into the rails action chain BEFORE the authenticate_user! via prepend: true (other usages too).
We execute code in before we authenticate a user cause account_load_and_authorize_resource uses prepend actions
@jagthedrummer
Copy link
Contributor

@seattlecyclist, do you have some reproduction steps you can share on this?

@aaricpittman
Copy link
Contributor

@jagthedrummer if you set authenticate_user! as a before_action in your ApplicationController and then call account_load_and_authorize_resource in a sub controller, the load_and_authorize_resource methods will get called before authenticate_user! because they're prepended.

@jagthedrummer
Copy link
Contributor

@aaricpittmanj / @seattlecyclist apologies for the delay on this. I think I understand what you're saying about the order that things would (should?) happen, given that we're using prepend, but I'm having trouble constructing any sort of failing reproduction scenario.

I've added this to my main ApplicationController:

before_action :authenticate_user!

And everything in my app still works. I can navigate and create various resources, and the test suite is passing (other than a failure in a test that's expecting us not to have tried to authenticate a user on a non-authenticated route).

Can you help me understand more about what problems this is causing and how you trigger the problem?

@jagthedrummer jagthedrummer mentioned this issue Dec 13, 2023
Closed
jagthedrummer added a commit that referenced this issue Jan 2, 2024
@jagthedrummer
Don't use prepend_before_action when loading/authorizing resources
54f32ea 
Using `prepend_before_action` causes problems because developers lose the ability to control the order in which callbacks happen.

Maybe fixes #531? (Hard to be sure since I don't know how to reproduce whatever problem people are running into.)

Supersedes #716
@jagthedrummer jagthedrummer linked a pull request Jan 2, 2024 that will close this issue
Open
@jagthedrummer
Copy link
Contributor

@seattlecyclist @aaricpittman Can you guys give the branch from this PR a shot? #730

I removed usage of prepend_before_filter but since I don't have any reproduction steps for whatever problems y'all are running into I don't have any way to know if I've fixed your particular problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jagthedrummer jagthedrummer

Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@aaricpittman @jagthedrummer @andrewculver