Closed (fixed)
Project:
Login Destination
Version:
7.x-1.x-dev
Component:
Code
Priority:
Critical
Category:
Task
Assigned:
Reporter:
Created:
11 Feb 2015 at 19:25 UTC
Updated:
5 Feb 2016 at 08:24 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
darrellduane commentedComment #2
joelpittetThere is no patch here so this is just "active" status. But I get this as well.
Comment #3
joelpittetBecause that isn't a valid path... maybe this will do the trick.
Comment #4
darrellduane commentedI believe there's more to this issue than just stopping this check if the path isn't ''. I'm not sure what '' means, but I do think its important to check the path as the code currently does, it just needs to check the path as a function of the roles that are selected for this redirect. Doing that will take some coding, I may be up for writing the code someday, perhaps someone else can do it sooner than I can.
Thanks for submitting a patch so that I can make the status "Needs work"!
Comment #5
joelpittetIf you want to make this more complicated than it is you have to create a pseudo menu link to like the way that
<front>is created in core, or use some third party module like menu_token...Did you test the patch to determine it needs work?
Comment #6
joelpittetTo be more specific you need to define a menu route in the menu_router table, likely have it callback to a 404 like
<nolink>or<separator>are done for special_menu_items module, which is done through themenu_linkstable.Comment #7
joelpittetI think it's overkill because it's used as a placeholder not an actual menu item, that's why I went with the easier approach here.
Comment #8
perennial.sky commentedHi DarrellDuane,
Adding patch that may solve your problem
but i don't think it is bug according to drupal, administration can access all pages
Comment #9
joelpittetLooks like some upstream changes are requiring this to be re-rolled.
Comment #10
perennial.sky commentedHere is the rerolled patch.
Comment #11
joelpittetAny thoughts on using && instead of nesting the checks?
Comment #12
ddrozdik commentedHey guys,
Good Idea, but I suppose we should to do more flexible validation, and we should check, that role selected in the rule, has access to entered path. Provided patch does not check this, and exist situation that after login/logout user will be redirected to a page with forbidden access for that role.
Comment #13
ddrozdik commentedI have prepared a patch, with validation by role. This patch validates entered path by selected role. If role does not have access to a path, then validation function return an error.
Comment #14
rsvelko commented@DmitryDrozdik requested that I reroll the last patch to fix the english language in the msgs.
Here is a new version with no code logic changes from #13, just language.
I attach a word diff screenshot so we see what I changed (new words are in green).
Comment #15
afi13 commentedLooks good for me.
Comment #17
ddrozdik commentedComment #19
njogz commentedI was still getting this error for valid aliased paths so added $destination = drupal_get_normal_path($destination); above
$item = menu_get_item($destination); which is on line 526. Seems to have sorted it out.
Comment #20
bill_redman commentedI am having the same issue of "Incorrect path. Please enter a valid path" even after installing 7.x-1.x-dev. I then tried what njogz did in #19 and the error then changed to Role "role name" does not have access to the path. In the first case, the path is, in fact, a valid path and in the second case, the Role does, in fact have access to the path.
Suggestions anyone?
Thanks.
Comment #21
ddrozdik commented@njogz and @Bill Redman please provide examples of configuration that I could reproduce this bug. I need information about used roles, used paths, and other list of settings.
Comment #22
njogz commented@ddrozdik I just tried the module on a clean install of Drupal 7.41 and got the error. The path I am redirecting to is a basic page with the URL alias set. See screenshots for settings.


Comment #23
njogz commented@Bill Brendan I also got the same error and still looking into it. Commenting out the foreach loop on line 537-545 removes the error and the redirection works. This probably creates a security vulnerability so maybe someone can suggest an alternative?
Comment #24
ddrozdik commented@njogz do not need close ticket if you still have a problem.
I will review this bug today, and will prepare a patch if it will work, we will add it to the module.
As I understood you set an alias as destination path in your example, am I right?
Comment #25
ddrozdik commented@njogz and @Bill Redman, I have found a problem in the previous patch. I have added needed changes. Please apply this patch and check it.
Comment #26
bill_redman commented@ddrozdik, sorry for not responding to your earlier request. Fortunately, @njogz did. Anyway, your modified patch has solved the problem. I was able to add my new rule successfully. Thank you.
Comment #27
ddrozdik commentedComment #29
ddrozdik commentedPatch has been committed. Thanks guys.