Merge pull request #1410 from ChrisBr/force_login_833

[api] Force login for all API calls

src/api/app/controllers/about_controller.rb

@@ -1,6 +1,7 @@
 class AboutController < ApplicationController
   validate_action :index => {:method => :get, :response => :about}
   skip_before_action :extract_user
+  skip_before_action :require_login
 
   def index
     @api_revision = CONFIG['version'].to_s

src/api/app/controllers/application_controller.rb

@@ -263,6 +263,14 @@ def check_for_anonymous_user
     false
   end
 
+  def require_login
+    # we allow anonymous user only for rare special operations (if configured) but we require
+    # a valid account for all other operations.
+    # For this rare special operations we simply skip the require login before filter!
+    # At the moment these operations are the /public, /trigger and /about controller actions.
+    be_not_nobody!
+  end
+
   def check_extracted_user
     unless @http_user
       if @login.blank?

src/api/app/controllers/comments_controller.rb

@@ -1,5 +1,4 @@
 class CommentsController < ApplicationController
-  before_filter :require_login, except: [:show_comments]
   before_filter :find_obj, only: [:show_comments, :create]
 
   def show_comments

src/api/app/controllers/configurations_controller.rb

@@ -2,7 +2,6 @@
 
 class ConfigurationsController < ApplicationController
   # Site-specific configuration is insensitive information, no login needed therefore
-  skip_before_filter :extract_user, :only => [:show]
   before_filter :require_admin, :only => [:update]
   skip_filter :validate_params, :only => [:update] # we use an array for archs here
 

src/api/app/controllers/distributions_controller.rb

@@ -1,6 +1,5 @@
 class DistributionsController < ApplicationController
   # Distribution list is insensitive information, no login needed therefore
-  skip_before_filter :extract_user, :only => [:index, :show, :include_remotes]
   before_filter :require_admin, :except => [:index, :show, :include_remotes]
 
   validate_action :index => {:method => :get, :response => :distributions}

src/api/app/controllers/group_controller.rb

@@ -5,7 +5,6 @@ class GroupController < ApplicationController
   validate_action :groupinfo => { :method => :put, :request => :group, :response => :status }
   validate_action :groupinfo => { :method => :delete, :response => :status }
 
-  before_action :require_login, except: [:index, :show]
   # raise an exception if authorize has not yet been called.
   after_action :verify_authorized, :except => [:index, :show]
 

src/api/app/controllers/issue_trackers_controller.rb

@@ -1,5 +1,4 @@
 class IssueTrackersController < ApplicationController
-  skip_before_filter :extract_user, :only => [:index, :show]
   before_filter :require_admin, :only => [:create, :update, :destroy]
 
   validate_action :index => {:method => :get, :response => :issue_trackers}

src/api/app/controllers/issues_controller.rb

@@ -1,5 +1,4 @@
 class IssuesController < ApplicationController
-  skip_before_filter :extract_user, :only => [:index, :show]
   before_filter :require_admin, :only => [:create, :update, :destroy]
 
   def show

src/api/app/mixins/forbids_anonymous_users.rb

@@ -3,16 +3,6 @@ class AnonymousUser < APIException
     setup 401
   end
 
-  #
-  # The following depends on ApplicationController:check_for_anonymous_user
-  # conditionally loading the anonymous user!
-  #
-  def require_login
-    # we may allow anonymous GET operations (if configured) but we require
-    # a valid account on other opertations
-    be_not_nobody! unless request.get?
-  end
-
   def be_not_nobody!
     if !User.current || User.current.is_nobody?
       raise AnonymousUser.new 'Anonymous user is not allowed here - please login'

src/api/test/functional/configurations_controller_test.rb

@@ -6,10 +6,6 @@ def setup
   end
 
   def test_show_and_update_configuration
-    reset_auth
-    get '/public/configuration' # required for anonymous remote webui access
-    assert_response :success
-
     login_tom
     get '/public/configuration.json' # is done by webui from OBS 2.4
     assert_response :success

src/api/test/functional/distributions_controller_test.rb

@@ -8,6 +8,7 @@ def setup
   end
 
   def test_should_show_distribution
+    login_tom
     get distribution_path(id: distributions(:two).to_param)
     assert_response :success
     # the default XML renderer just s***s
@@ -72,10 +73,6 @@ def test_the_old_interface_works
     put "/distributions", data
     assert_response 200
 
-    reset_auth
-    get "/distributions"
-    assert_response :success
-
     login_tom
     get "/distributions"
     assert_response :success
@@ -126,6 +123,7 @@ def test_remotes_work
   end
 
   def test_we_survive_remote_instances_timeouts
+    login_tom
     stub_request(:get, "http://localhost:#{CONFIG['source_port']}/distributions.xml").to_timeout
     get "/distributions/include_remotes"
     assert_response :success

src/api/test/functional/issue_controller_test.rb

@@ -9,14 +9,6 @@ def setup
   end
 
   def test_get_issues
-    # bugs are public atm. Secret stuff should not get imported.
-    get '/issue_trackers'
-    assert_response :success
-    get '/issue_trackers/bnc'
-    assert_response :success
-    get '/issue_trackers/bnc/issues/123456'
-    assert_response :success
-
     # as user
     login_Iggy
     get '/issue_trackers'

src/api/test/functional/issue_trackers_controller_test.rb

@@ -7,6 +7,7 @@ def setup
 
   def test_should_get_index
     # Get all issue trackers
+    login_king
     get '/issue_trackers'
     assert_response :success
     assert_not_nil assigns(:issue_trackers)

src/api/test/functional/read_permission_test.rb

@@ -50,9 +50,9 @@ def test_basic_read_tests
 
     # anonymous access with user-agent set
     get "/source/SourceprotectedProject", nil, { 'HTTP_USER_AGENT' => 'osc-something' }
-    assert_response 200
+    assert_response 401
     get "/source/SourceprotectedProject/_meta", nil, { 'HTTP_USER_AGENT' => 'osc-something' }
-    assert_response 200
+    assert_response 401
     get "/source/SourceprotectedProject/pack",  nil, { 'HTTP_USER_AGENT' => 'osc-something' }
     assert_response 401
 
@@ -82,7 +82,7 @@ def test_basic_repository_tests
 
     # anonymous access with user-agent set
     get "/build/SourceprotectedProject/repo/i586/pack",  nil, { 'HTTP_USER_AGENT' => 'osc-something' }
-    assert_response 200
+    assert_response 401
 
     srcrpm="package-1.0-1.src.rpm"
 

src/api/test/functional/source_controller_test.rb

@@ -23,6 +23,11 @@ def setup
     reset_auth
   end
 
+  def test_get_projectlist_requires_login
+    get '/source'
+    assert_response 401
+  end
+
   def test_get_projectlist
     login_tom
     get '/source'