Lockdown

I apologize, I don't know how I missed this post. Looking at it now.

Which version of Lockdown are you using? There was an issue with the links with older versions of Lockdown. If you are using the latest version and still see the links, can you actually access the page?

I am using version 0.5.20.

Huy, you mentioned:

"When I logged in as a Manager I still can't see links that supposed to see since he has permission."

After you log in, please log the session[:access_rights] info and post it here. I want to see what's in there.

thanks,
andy

This is what I get from the session[:access_rights] after login:

http://127.0.0.1:3000/users -> /login
sessions/create
sessions/new
sessions/destroy
users/edit
users/update
users/show
afl_leagues/update_player_identifiers
afl_leagues/show
afl_leagues/update_team_identifiers
afl_leagues/edit
afl_leagues/index
afl_leagues/create
afl_leagues/new
afl_leagues/update
afl_leagues/destroy
afl_matches/process_quarter_event
afl_matches/adjust_players
afl_matches/show
afl_matches/edit
afl_matches/index
afl_matches/record_goal
afl_matches/create
afl_matches/new
afl_matches/update
afl_matches/time_with_datetime_fallback
afl_matches/local_time
afl_matches/utc_time
afl_matches/days_in_month
afl_matches/destroy
afl_players/show
afl_players/edit
afl_players/index
afl_players/create
afl_players/new
afl_players/update
afl_players/destroy
afl_rounds/show
afl_rounds/edit
afl_rounds/create
afl_rounds/new
afl_rounds/update
afl_rounds/destroy
afl_seasons/show
afl_seasons/edit
afl_seasons/create
afl_seasons/new
afl_seasons/update
afl_seasons/destroy
afl_teams/show
afl_teams/edit
afl_teams/create
afl_teams/new
afl_teams/update
afl_teams/destroy
afl1up_games/finalise
afl1up_games/show
afl1up_games/edit
afl1up_games/index
afl1up_games/close
afl1up_games/create
afl1up_games/new
afl1up_games/update
afl1up_games/activate
afl1up_games/manage
afl1up_games/destroy
fixed_odds_pools/process_winners
fixed_odds_pools/edit_market_parameters
fixed_odds_pools/show
fixed_odds_pools/edit
fixed_odds_pools/create
fixed_odds_pools/update_market_parameters
fixed_odds_pools/new
fixed_odds_pools/update
fixed_odds_pools/destroy
possible_outcomes/show
possible_outcomes/edit
possible_outcomes/index
possible_outcomes/create
possible_outcomes/new
possible_outcomes/update
possible_outcomes/destroy
sms_messages/show
sms_messages/edit
sms_messages/index
sms_messages/create
sms_messages/process_messages
sms_messages/new
sms_messages/update
sms_messages/destroy
reports/index
reports/sport_report
reports/financial_report
reports/pup_report
reports/product_report
reports/overall_report
users/update_password
users/show
users/edit
users/index
users/change_password
users/create
users/new
users/update
bank_withdrawal_requests/show
bank_withdrawal_requests/edit
bank_withdrawal_requests/index
bank_withdrawal_requests/mark_as_processed
bank_withdrawal_requests/create
bank_withdrawal_requests/mark_as_failed
bank_withdrawal_requests/new
bank_withdrawal_requests/update
bank_withdrawal_requests/export_pending_requests

Everything looks fine but the links still disappear.

Huy

Huy,

That looks right, could you show me your link_to call?

Sorry I don't have the easy answer yet...

thanks,
andy

<tr>
            <td><%= link_to 'Playup Players', playup_players_path %></td>
            <td>|</td>
            <td><%= link_to 'Submit SMS Entry', new_sms_message_path %></td>
            <td>|</td>
            <td><%= link_to 'Afl1upGames', afl1up_games_path %></td>
            <td>|</td>
            <td><%= link_to 'AflMatches', afl_matches_path %></td>
            <td>|</td>
            <td><%= link_to 'Leagues', afl_leagues_path %></td>
            <td>|</td>
            <td><%= link_to 'Outgoing SMSs', outgoing_sms_messages_path %></td>
            <td>|</td>
            <td><%= link_to 'Reporting', reports_path %></td>
            <td>|</td>
                  <td><%= link_to 'Background Tasks', background_tasks_path %></td>
            <td>|</td>
            <td><%= link_to 'Export Bank Withdrawal Requests', export_files_path %></td>
            <td>|</td>
            <td><%= link_to 'Logout', logout_path %></td>
        </tr>

Huy,

Lockdown hasn't been tested with JRuby and everything looks fine from a 'normal' setup, so I'm guessing the issue must be with JRuby. There was another person on the forum working with JRuby a while back, not sure what the outcome was. So, it looks like work needs to be done to play nice with JRuby. I don't have the time now to setup this environment and dig into this issue.

I hope you have the time to dig into this issue and contribute any corrections needed back to the project. It would be greatly appreciated.

thanks,
andy

Andy,

I work with Huy and have been digging further into the issue. The root cause seems to be that when deployed to tomcat urls have an additional path element i.e. the "context" e.g. "/myapp/controller/action" rather than just "controller/action".

A minor tweak seems (not fully tested) to have fixed the actual security i.e. can execute actions that user is supposed to and get an unauthorized error for those that the user shouldn't: in controller.rb::path_allowed? changed "req == ar" => "req.ends_with?(ar)"

However there is still an issue with the links, it seems primarily because the LockDown.format_controller_action method won't add "index" to the path if there are "/" in it.

From examining the code I'm not confident that I understand the rationale behind the format_controller_action. If possible could you briefly explain so that I might be able to devise a solution.

Cheers,
James

James,

I will look at this tonight. I just wanted to say thanks and I'm really excited about getting Lockdown working in your environment.

As a side note, I am almost done with a refactor of Lockdown. This project originated over 3 years ago when I was a Ruby nuby so there are definite architecture and code improvements coming soon.

I was planning on completing that this weekend so this is perfect timing. Please let me know of anything else that has confusing rationale. One of my goals with the refactor is to eliminate such confusing/unclear code.

Thanks again!

-andy

Thanks Andy,

Really appreciate your efforts in this matter. Since you asked, one area that seems unnecessarily busy is the Controller::Rails::InstanceMethods.authorized? method which calls path_allowed? 4 times. Debug output shows that in the majority of instances path_allowed? is passed the same string to check, which would suggest that, at the least, authorized? could check whether the url string has been altered before calling path_allowed? again OR that potentially the number of calls to path_allowed? could be reduced by delaying the call(s) until the url string has been manipulated.

Also, (although you're probably already aware) lockdown does not seem to hook into the "link_to_remote" method, which has the same semantics as the "link_to" method but is for ajax calls.

Thanks again for your efforts. I'll be looking forward to the refactor.

Cheers,
James

Andy,

I have created a bit of a hack that allows lockdown to work in a JRuby-in-war-on-Tomcat (and probably other app servers) environment. Relevant code from controller.rb and view.rb is below.

In the process I added a hook into the link_to_remote method so that lockdown will work with ajax links as well.

        def path_allowed?(url)
          req = Lockdown.format_controller_action(url)
          session[:access_rights] ||= Lockdown::System.public_access
          session[:access_rights].each do |ar|
            return true if *req.ends_with?(ar)*
          end
          false
        end

    module Rails
      include Lockdown::View::Core
      def self.included(base)
        base.send :alias_method, :link_to_remote_open,  :link_to_remote
        base.send :alias_method, :link_to_remote,  :link_to_remote_secured

        base.send :alias_method, :link_to_open,  :link_to
        base.send :alias_method, :link_to,  :link_to_secured

        base.send :alias_method, :button_to_open,  :button_to
        base.send :alias_method, :button_to,  :button_to_secured
      end

      def link_to_remote_secured(name, options = {}, html_options = nil)
        url = url_for(options[:url])
        if authorized? test_path(url, options) # the :method entry is in the options hash for link_to_remote
          return link_to_remote_open(name, options, html_options)
        end
        return "" 
      end

      def link_to_secured(name, options = {}, html_options = nil)
        # Don't want to go through the url_for twice
        url = url_for(options)
        if authorized? test_path(url, html_options)
          return link_to_open(name, url, html_options)
        end
        return "" 
      end

      def link_to_or_show(name, options = {}, html_options = nil)
        lnk = link_to(name, options, html_options)
        lnk.length == 0 ? name : lnk
      end

      def button_to_secured(name, options = {}, html_options = nil)
        url = url_for(options)
        if authorized? test_path(url, html_options)
          return button_to_open(name, url, html_options)
        end
        return "" 
      end

      private

      def test_path(url, html_options)
        app_context = request.env["RAILS_RELATIVE_URL_ROOT"]
        url = url.gsub(app_context, "") if app_context
        if html_options.is_a?(Hash) && html_options[:method] == :delete
          url += "/destroy" 
        elsif url.split("/").last =~ /\A\d+\z/
          url += "/show" 
        end
        url
      end
    end

        def path_allowed?(url)
          req = Lockdown.format_controller_action(url)
          session[:access_rights] ||= Lockdown::System.public_access
          session[:access_rights].each do |ar|
            return true if *req.ends_with?(ar)*
          end
          false
        end

Thanks James !

I'm on vacation now but am looking forward to finishing up the refactor when I return this weekend.

Thanks for helping out. I really do appreciate it.

To clarify ...
It will probably take me a week or so to finish the refactor as I'm also working on rspec tests at the same time.

James,

I haven't published it yet...still testing. But please feel free to clone the project from github and give it a test drive. In order to account for the subdir issue, there's a new option you can set in init.rb:

  # If deploying to a subdirectory, set that here. Defaults to nil
  #       options[:subdirectory] = "blog" 
  #       *Notice: Do not add leading or trailing slashes,
  #                Lockdown will handle this. 

I also approached the "index" issue a little differently which should make things a lot more solid than I had previously coded.

Thanks for working with me on this...

-andy

fyi, the next release will be "0.6.0" and I expect to do this later this week or early next week depending on your feedback.

Oh yeah, after you clone, just go to the directory and type "rake install_gem".

I'm adding tests this week to verify everything is working as expected. I'll also continue to refactor code as the need arises (of course). The major structural changes are done.

Hi Andrew, I work with Huy and James :)

def format_controller_action(url)
   new_url = url.split("/").delete_if{|p| p.to_i > 0 || p.length == 0}.join("/")
   new_url += "/index" unless new_url =~ /\//
   new_url
end

.. to be ...

def format_controller_action(url)
   new_url = url.gsub(/.*[0-9]+\//, "")
   new_url += "/index" unless new_url =~ /\//
   new_url
end

...which ensures nested urls like /foo/1234/bar/456/baz still get appended with index.
Perhaps this is what you did for 0.6.0 anyway... I haven't looked yet.

James has pointed out that my solution does not work for urls such as "foo/12345/show" ... as we end up with just "show"
I'll therefore modify the code to take a "nested" url and turn it into a "non-nested" url before the numbers are removed as per your original split.
eg, foo/1234/bar/5677/show becomes bar/5677/show and then just bar/show

def format_controller_action(url)
   parts = url.split("/")
   parts = parts[-3..-1] if parts.size >=3
   new_url = parts.delete_if{|p| p.to_i > 0 || p.length == 0}.join("/")
   new_url += "/index" unless new_url =~ /\//
   new_url
end