Review all routes, improve their scopes and tests

There were too many root level paths. Reduced a lot by grouping routes
under the same scopes, which also makes it all more organized.

app/assets/javascripts/app/my/meetings.js.coffee

@@ -1,4 +1,4 @@
-# TODO: What is done here is almost duplicated at spaces/recordings.js.coffee, find a way to merge
+# TODO: What is done here is almost duplicated at spaces/meetings.js.coffee, find a way to merge
 # them together.
 
 $ ->

app/assets/javascripts/app/spaces/meetings.js.coffee

@@ -1,4 +1,4 @@
-# TODO: What is done here is almost duplicated at my/recordings.js.coffee, find a way to merge
+# TODO: What is done here is almost duplicated at my/meetings.js.coffee, find a way to merge
 # them together.
 
 $ ->

app/controllers/my_controller.rb

@@ -66,31 +66,6 @@ def activity
       .paginate(:page => params[:page], :per_page => @contents_per_page.to_i)
   end
 
-  # Renders a json with the webconference rooms accessible to the current user
-  # Response example:
-  #
-  # [
-  #   { "bigbluebutton_room":
-  #     { "name":"Admins Room", "join_path":"/bigbluebutton/servers/default-server/rooms/admins-room/join?mobile=1",
-  #       "owner":{ "type":"User", "id":"1" } }
-  #   }
-  # ]
-  #
-  # The attribute "owner" will follow one of the examples below:
-  # "owner":null
-  # "owner":{ "type":"User", "id":1 }
-  # "owner":{ "type":"Space", "id":1, "name":"Space's name", "public":true }
-  #
-  # Note: this route exists so the mobile client can get the rooms available for the user
-  def rooms
-    array = current_user.accessible_rooms || []
-    mapped_array = array.map{ |r|
-      link = join_bigbluebutton_room_path(r, :mobile => '1')
-      { :bigbluebutton_room => { :name => r.name, :join_path => link, :owner => owner_hash(r.owner) } }
-    }
-    render :json => mapped_array
-  end
-
   # List of meetings for the current user's web conference room.
   def meetings
     @room = current_user.bigbluebutton_room

app/controllers/sessions_controller.rb

@@ -14,8 +14,9 @@ class SessionsController < Devise::SessionsController
   # If there's no auth enabled, go to the frontpage and not the login page
   before_filter only: [:new, :create] do
 
-    # when going to /admin we allow access
-    if !request.path.match(/^\/admin$/)
+    # when going through the admin login page we allow access
+    matcher = /^#{Regexp.escape(admin_login_path)}$/
+    if !request.path.match(matcher)
 
       site = Site.current
       if !site.local_auth_enabled? && !site.ldap_enabled?

app/mailers/views/participant_confirmation_mailer/confirmation_email.html.haml

@@ -2,9 +2,9 @@
   %p= t('.hello').html_safe
   %p= t('.message', email: @email, event: @event).html_safe
   %p
-    - url = participant_confirmation_url(token: @pc.to_param, host: Site.current.domain)
+    - url = participant_confirmation_events_url(token: @pc.to_param, host: Site.current.domain)
     = t('.confirm', url: url).html_safe
   %p
-    - url = cancel_participant_confirmation_url(token: @pc.to_param, host: Site.current.domain)
+    - url = cancel_participant_confirmation_events_url(token: @pc.to_param, host: Site.current.domain)
     = t('.cancel', url: url).html_safe
   %p= t('.footer').html_safe

app/views/my/meetings.html.haml

@@ -22,7 +22,7 @@
       -# only update the recording db if the user has permission to
       - if can?(:fetch_recordings, @room)
 
-        -# TODO: This block below is almost duplicated at spaces/recordings.html.haml, find a way to merge
+        -# TODO: This block below is almost duplicated at spaces/meetings.html.haml, find a way to merge
         -# them together.
 
         -# a form to trigger the method that will fetch the recordings in the webconf server

app/views/spaces/meetings.html.haml

@@ -21,7 +21,7 @@
       -# only update the recording db if the user has permission to
       - if can?(:fetch_recordings, @webconf_room)
 
-        -# TODO: This block below is almost duplicated at my/recordings.html.haml, find a way to merge
+        -# TODO: This block below is almost duplicated at my/meetings.html.haml, find a way to merge
         -# them together.
 
         -# a form to trigger the method that will fetch the recordings in the webconf server

config/routes.rb

@@ -20,10 +20,6 @@
 Mconf::Application.routes.draw do
   root to: 'frontpage#show'
 
-  constraints CanAccessResque do
-    mount Resque::Server, at: 'manage/resque'
-  end
-
   # devise
   controllers = { sessions: "sessions", registrations: "registrations",
                   passwords: "passwords", confirmations: "confirmations" }
@@ -35,7 +31,7 @@
     get "register", to: "registrations#new"
 
     # so admins can log in even if local auth is disabled
-    get "admin", to: "sessions#new"
+    get '/manage/login', to: 'sessions#new', as: 'admin_login'
   end
 
   # conference routes
@@ -68,20 +64,54 @@
     to: 'custom_bigbluebutton_rooms#invite_userid',
     as: "join_webconf"
 
+  # note: this block *has* to be before `resources :users`, otherwise some
+  # routes here won't work well
+  scope 'users' do
+    get 'pending', to: 'my#approval_pending', as: 'my_approval_pending'
 
-  # shibboleth controller
-  get '/secure', to: 'shibboleth#login', as: "shibboleth"
-  get '/secure/info', to: 'shibboleth#info', as: "shibboleth_info"
-  post '/secure/associate', to: 'shibboleth#create_association', as: "shibboleth_create_association"
+    # login via Shibboleth
+    scope 'shibboleth' do
+      get '/', to: 'shibboleth#login', as: "shibboleth"
+      get 'info', to: 'shibboleth#info', as: "shibboleth_info"
+      post 'associate', to: 'shibboleth#create_association', as: "shibboleth_create_association"
+    end
 
-  # to crop images
-  get "logo_images/crop", to: 'logo_images#crop'
+    # login via certificate
+    get 'certificate', to: 'certificate_authentication#login', as: 'certificate_login'
+  end
+  # to keep it compatible with previous versions (i.e. if apache is configured for '/secure')
+  get 'secure', to: redirect('/users/shibboleth')
 
-  # tags
-  get "tags/select", to: 'tags#select'
+  resources :users, except: [:index] do
+    collection do
+      get :fellows
+      get :select
+      get :current
+    end
+
+    member do
+      delete :disable
+      post :enable
+      post :approve
+      post :disapprove
+      post :confirm
+    end
+
+    resource :profile, only: [:show, :edit, :update] do
+      post :update_logo
+    end
+  end
 
-  resources :spaces do
 
+  # routes specific for the current user
+  scope 'home' do
+    get '/', to: 'my#home', as: 'my_home'
+    get 'activity', to: 'my#activity', as: 'my_activity'
+    get 'meetings', to: 'my#meetings', as: 'my_meetings'
+    get 'recordings/:id/edit', to: 'my#edit_recording', as: 'edit_my_recording'
+  end
+
+  resources :spaces do
     collection do
       get :select
     end
@@ -126,69 +156,47 @@
     delete 'attachments', to: 'attachments#delete_collection'
   end
 
-  resources :users, except: [:index] do
+  scope 'manage' do
+    resource :site, only: [:show, :edit, :update]
+
+    ['users', 'spaces', 'recordings'].each do |resource|
+      get resource, to: "manage##{resource}", as: "manage_#{resource}"
+    end
+
+    get '/', to: redirect('/manage/site'), as: 'manage'
+  end
+
+  constraints CanAccessResque do
+    mount Resque::Server, at: 'manage/resque'
+  end
+
+  resources :events do
     collection do
-      get :fellows
       get :select
-      get :current
+      get 'participants/confirmations/:token', to: 'participant_confirmations#confirm', as: 'participant_confirmation'
+      get 'participants/confirmations/:token/cancel', to: 'participant_confirmations#destroy', as: 'cancel_participant_confirmation'
     end
 
     member do
-      delete :disable
-      post :enable
-      post :approve
-      post :disapprove
-      post :confirm
+      post :send_invitation
+      get  :invite
     end
 
-    resource :profile, only: [:show, :edit, :update] do
-      post :update_logo
-    end
+    resources :participants, except: [:show, :edit]
   end
 
-  # Routes specific for the current user
-  get '/home', to: 'my#home', as: 'my_home'
-  get '/activity', to: 'my#activity', as: 'my_activity'
-  get '/rooms', to: 'my#rooms', as: 'my_rooms'
-  get '/meetings', to: 'my#meetings', as: 'my_meetings'
-  get '/recordings/:id/edit', to: 'my#edit_recording', as: 'edit_my_recording'
-  get '/pending', to: 'my#approval_pending', as: 'my_approval_pending'
-
-  # Login via certificate
-  get '/certificate_login', to: 'certificate_authentication#login', as: 'certificate_login'
+  resource :language, only: [:create], controller: :session_locales, as: :session_locale
 
   resources :feedback, only: [:new, :create] do
     get :webconf, on: :collection
   end
 
   resources :permissions, only: [:update, :destroy]
 
-  # The unique Site is created in db/seeds and can only be edited
-  resource :site, only: [:show, :edit, :update]
-
-  # Management routes
-  get "/manage", to: redirect('/site'), as: "manage"
-  ['users', 'spaces', 'recordings'].each do |resource|
-    get "/manage/#{resource}", to: "manage##{resource}", as: "manage_#{resource}"
-  end
-
-  # Locale controller, to change languages
-  resource :language, only: [:create], controller: :session_locales, as: :session_locale
+  get "tags/select", to: 'tags#select'
 
-  # Events
-  # Note: we load the routes even if the events are disabled in the site
-  resources :events do
-    collection do
-      get :select
-    end
-    resources :participants, except: [:show, :edit]
-    member do
-      post :send_invitation
-      get  :invite
-    end
-  end
-  get 'participant_confirmations/:token', to: 'participant_confirmations#confirm', as: 'participant_confirmation'
-  get 'participant_confirmations/:token/cancel', to: 'participant_confirmations#destroy', as: 'cancel_participant_confirmation'
+  # to crop images
+  get "logos/crop", to: 'logo_images#crop', as: 'logo_images_crop'
 
   # To treat errors on pages that don't fall on any other controller
   match ':status', to: 'errors#on_error', constraints: { status: /\d{3}/ }, via: :all

config/webserver/apache2_shibboleth.example

@@ -1,7 +1,7 @@
 # Add this to your application's configuration file, right before the closing
 # </VirtualHost> tag
 
-<LocationMatch "^/secure(/associate)?$">
+<LocationMatch "^/users/shibboleth(/associate)?$">
   AllowOverride all
   AuthType shibboleth
   ShibRequireSession On

lib/mconf/redirect_controller_module.rb

@@ -36,13 +36,14 @@ def request_is_redirectable?(request)
   # the type of the request or anything else.
   def path_is_redirectable?(path)
     # Paths to which users should never be redirected back to.
-    ignored_paths = [ "/login", "/users/login", "/users",
+    ignored_paths = [ "/login", "/users/login", "/users", "/logout",
                       "/register", "/users/registration",
                       "/users/registration/signup", "/users/registration/cancel",
                       "/users/password", "/users/password/new",
                       "/users/confirmation/new", "/users/confirmation",
-                      "/secure", "/secure/info", "/secure/associate", "/feedback/webconf",
-                      "/pending", "/#{configatron.conf.scope}/rooms/.*/join", "/#{configatron.conf.scope}/rooms/.*/end"]
+                      "/users/shibboleth", "/users/shibboleth/info", "/users/shibboleth/associate", "/secure",
+                      "/users/pending", "/feedback/webconf",
+                      "/#{configatron.conf.scope}/rooms/.*/join", "/#{configatron.conf.scope}/rooms/.*/end"]
     ignored_paths.select{ |ignored| path.match("^"+ignored+"$") }.empty?
   end
 

spec/controllers/my_controller_spec.rb

@@ -52,7 +52,7 @@
       it { should render_template(:approval_pending) }
     end
 
-    context "renders the page if the referer is /secure" do
+    context "renders the page if the referer is the shibboleth path" do
       before(:each) {
         request.env["HTTP_REFERER"] = shibboleth_url
         get :approval_pending

spec/controllers/sessions_controller_spec.rb

@@ -45,9 +45,9 @@
       end
     end
 
-    context "if the route is /admin" do
+    context "if the route is the admin login path" do
       before do
-        controller.request.stub(:path).and_return("/admin")
+        controller.request.stub(:path).and_return(admin_login_path)
       end
 
       context "renders the page even if there's no local auth enabled" do

spec/controllers/shibboleth_controller_spec.rb

@@ -395,7 +395,7 @@
       end
     end
 
-    context "if params has no known option, redirects to /secure with a warning" do
+    context "if params has no known option, redirects to the shibboleth path with a warning" do
       let(:user) { FactoryGirl.create(:user) }
       before {
         setup_shib(user.full_name, user.email, user.email)
@@ -424,26 +424,26 @@
         save_shib_to_session
       }
 
-      context "if there's no user info in the params, goes back to /secure with an error" do
+      context "if there's no user info in the params, goes back to the shibboleth path with an error" do
         before(:each) { post :create_association, :existent_account => true }
         it { should redirect_to(shibboleth_path) }
         it { should set_flash.to(I18n.t('shibboleth.create_association.invalid_credentials')) }
       end
 
-      context "if the user info in the params is wrong, goes back to /secure with an error" do
+      context "if the user info in the params is wrong, goes back to the shibboleth path with an error" do
         before(:each) { post :create_association, :existent_account => true, :user => { :so_wrong => 2  } }
         it { should redirect_to(shibboleth_path) }
         it { should set_flash.to(I18n.t('shibboleth.create_association.invalid_credentials')) }
       end
 
-      context "if the target user is not found goes back to /secure with an error" do
+      context "if the target user is not found goes back to the shibboleth path with an error" do
         before(:each) { post :create_association, :existent_account => true, :user => { :login => 'any' } }
         it { User.find_first_by_auth_conditions({ :login => 'any' }).should be_nil}
         it { should redirect_to(shibboleth_path) }
         it { should set_flash.to(I18n.t('shibboleth.create_association.invalid_credentials')) }
       end
 
-      context "if found the user but the password is wrong goes back to /secure with an error" do
+      context "if found the user but the password is wrong goes back to the shibboleth path with an error" do
         let(:user) { FactoryGirl.create(:user) }
         before(:each) { post :create_association, :existent_account => true, :user => { :login => user.username } }
         it("finds the user") {
@@ -453,7 +453,7 @@
         it { should set_flash.to(I18n.t('shibboleth.create_association.invalid_credentials')) }
       end
 
-      context "if the user is disabled goes back to /secure with an error" do
+      context "if the user is disabled goes back to the shibboleth path with an error" do
         let(:user) { FactoryGirl.create(:user, :disabled => true, :password => '12345') }
         before(:each) { post :create_association, :existent_account => true, :user => { :login => user.username, :password => '12345' } }
         it("does not find the disabled user") {
@@ -472,7 +472,7 @@
           save_shib_to_session
         }
 
-        context "goes back to /secure with a success message" do
+        context "goes back to the shibboleth path with a success message" do
           before(:each) { post :create_association, :existent_account => true, :user => { :login => user.username, :password => '12345' } }
           it("finds the user") {
             User.find_first_by_auth_conditions({ :login => user.username }).should_not be_nil

spec/features/redirect_back_spec.rb

@@ -133,7 +133,7 @@
     context "to a path that is not redirectable" do
       before {
         visit spaces_path
-        visit login_path(return_to: "/secure/info")
+        visit login_path(return_to: shibboleth_info_path)
       }
 
       it { current_path.should eq(spaces_path) }

spec/features/shibboleth/user_signs_in_spec.rb

@@ -374,7 +374,7 @@
 
     context "from the association page" do
       skip
-      # the user was in the shibboleth association page "/secure/associate"
+      # the user was in the shibboleth association page "/users/shibboleth/associate"
       # he user clicks to go to the login page
       # when the user clicks to sign in via shibboleth redirects the user to the association page
       # user clicks to create a new account