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

[MNOE-909]Removed trackable for impersonation requests #648

Open
wants to merge 2 commits into
base: 4.0
Choose a base branch
from

Conversation

iseessel
Copy link
Contributor

@iseessel iseessel commented Feb 7, 2018

Devise makes it easy to skip trackable. Just need to set request.env["devise.skip_trackable"] = true, BEFORE authentication happens. Tested locally and works.

It is appropriate to put it in the impersonate_controller, rather than the general auth-controller because we specifically want to do this on impersonating.

If we wanted to skip trackable other times, we could consider moving it into sessions_controller.rb and having an if params[:skip_trackable].

adamaziz15
adamaziz15 previously approved these changes Feb 7, 2018
Copy link
Contributor

@adamaziz15 adamaziz15 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@@ -39,6 +40,10 @@ def destroy

private

def skip_trackable
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is not needed, the function is already defined in the ApplicationController

Copy link
Contributor Author

@iseessel iseessel Feb 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ouranos
Ah I see.

I'm a little confused as to why this is in the application_controller. Trackables are only updated when the user signs in, correct? I see several before_filter :authenticate_user!, does it have to do with that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry this was added in #578 and hasn't made to the 4.0 branch yet.
The method is in the ApplicationController so it can be called from any controller that might requires it.

Added more details in JIRA

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

Successfully merging this pull request may close these issues.

3 participants