Forum OpenACS Development: Problem with return_url check in login.tcl

The code in login.tcl checks that the return_url is relative (no host name) or the host name matches [ad_url].

This is a problem if you have HTTPS login pages but redirect back to HTTP or if you are using host node mapping.

I modified the code to look this this:

if { [exists_and_not_null return_url] } {
    if {[util_complete_url_p $return_url] \
            && ![regexp "^[string trimright [subsite::get_url -absolute_p 1 -protocol http] /]" [ns_urldecode $return_url]] \
            && ![regexp "^[string trimright [subsite::get_url -absolute_p 1 -protocol https] /]" [ns_urldecode $return_url]]} {
      ad_returnredirect -message "only urls without a host name are permitted" \
"."
      ad_script_abort
    }
} else {
    set return_url [ad_pvt_home]
}

and this seems to work. (NOTE hack to trim extra / in subsite::get_url)

Collapse
Posted by Gustaf Neumann on
From the logic it looks good to me. three small comments:
a) the backslashes are not needed within the condition (since it is between curly brackes)
b) since all what's needed is the leading match, string match is better and faster than regexp
c) is the ns_urldecode $return_url really needed? the preceding util_complete_url_p does a regexp on the return_url without decoding.
Collapse
Posted by Dave Bauer on
Simplified.

if { [exists_and_not_null return_url] } { if {[util_complete_url_p $return_url] && ![string match "[string trimright [subsite::get_url -absolute_p 1 -protocol http] /]/*" $return_url] && ![string match "[string trimright [subsite::get_url -absolute_p 1 -protocol https] /]/*" $return_url]} { ad_returnredirect -message "only urls without a host name are permitted" "." ad_script_abort } } else { set return_url [ad_pvt_home] }