From 8163bb663205e6dce41a4eac472eb000d2ad35d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miquel=20Sabat=C3=A9?= Date: Fri, 7 Aug 2015 13:54:45 +0200 Subject: [PATCH 01/11] Fixed the Portus user for the RegisterClient class Up until now, the portus "user" was handled in a special way, by adding some code that worked around the whole authentication mechanism. However, we recently discovered that this was basically broken. The solution to this situation is to effectively create the "portus" user, as any other user in the system. Because of this, I've added a new secret, which is the password of the "portus" user, and I have added a rake task to help with the migration. Moreover, the seeds file already takes care of that. With this commit, the `manifest` method from the RegisterClient works again, and the authentication mechanism has been greatly simplified. There are still some small issues that need to be addressed. This will be done in later commits. --- Gemfile.lock | 3 --- Vagrantfile | 3 ++- app/controllers/api/v2/tokens_controller.rb | 17 ++--------------- app/models/registry_client.rb | 2 ++ app/models/user.rb | 4 +--- compose-setup.sh | 4 ++-- config/secrets.yml | 3 +++ db/seeds.rb | 16 ++++++++++++++++ lib/tasks/.keep | 0 lib/tasks/portus_user.rake | 11 +++++++++++ spec/models/user_spec.rb | 1 - 11 files changed, 39 insertions(+), 25 deletions(-) delete mode 100644 lib/tasks/.keep create mode 100644 lib/tasks/portus_user.rake diff --git a/Gemfile.lock b/Gemfile.lock index 3621384c6..4b27a5a4a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -362,6 +362,3 @@ DEPENDENCIES webmock wirb wirble - -BUNDLED WITH - 1.10.5 diff --git a/Vagrantfile b/Vagrantfile index fdb77b0bb..9ed63c73e 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -78,9 +78,10 @@ systemctl start mysql cd /vagrant bundle config build.nokogiri --use-system-libraries -bundle install +bundle install --retry=3 bundle exec rake db:create bundle exec rake db:migrate +bundle exec rake portus:create sudo gem install passenger -v 5.0.7 passenger-install-apache2-module.ruby2.1 -a diff --git a/app/controllers/api/v2/tokens_controller.rb b/app/controllers/api/v2/tokens_controller.rb index 827c9c538..60e94b9ef 100644 --- a/app/controllers/api/v2/tokens_controller.rb +++ b/app/controllers/api/v2/tokens_controller.rb @@ -1,17 +1,5 @@ class Api::V2::TokensController < Api::BaseController - before_action :authenticate - - def authenticate - if params["account"] == "portus" - totp = ROTP::TOTP.new(Rails.application.config.otp_secret) - - authenticate_with_http_basic do |u, p| - raise WrongPortusOTP if u != "portus" || p != totp.now - end - else - authenticate_user! - end - end + before_action :authenticate_user! def show registry = Registry.find_by(hostname: params["service"]) @@ -32,8 +20,7 @@ def show private def authorize_scopes(registry) - # The 'portus' user can do anything - return unless params[:scope] && params["account"] != "portus" + return unless params[:scope] auth_scope, scopes = scope_handler(registry, params[:scope]) scopes.each do |scope| diff --git a/app/models/registry_client.rb b/app/models/registry_client.rb index e50e0d0cd..d3a20f3db 100644 --- a/app/models/registry_client.rb +++ b/app/models/registry_client.rb @@ -30,6 +30,8 @@ def manifest(repository, tag = "latest") def get_request(path, request_auth_token = true) uri = URI.join(@base_url, path) + Rails.logger.info uri + Rails.logger.info "Token -> #{@token} <-" req = Net::HTTP::Get.new(uri) req["Authorization"] = "Bearer #{@token}" if @token diff --git a/app/models/user.rb b/app/models/user.rb index e5acbb326..2e0d0b072 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -4,9 +4,7 @@ class User < ActiveRecord::Base validates :username, presence: true, uniqueness: true, format: { with: /\A[a-z0-9]{4,30}\Z/, - message: 'Accepted format: "\A[a-z0-9]{4,30}\Z"' }, - exclusion: { in: %w(portus), - message: "%{value} is reserved." } + message: 'Accepted format: "\A[a-z0-9]{4,30}\Z"' } validate :private_namespace_available, on: :create diff --git a/compose-setup.sh b/compose-setup.sh index 3f4873815..2e077c341 100755 --- a/compose-setup.sh +++ b/compose-setup.sh @@ -8,7 +8,7 @@ setup_database() { TIMEOUT=90 COUNT=0 printf "Portus: configuring database..." - docker-compose run --rm web rake db:create db:migrate &> /dev/null + docker-compose run --rm web rake db:create db:migrate portus:create &> /dev/null while [ $? -ne 0 ]; do printf " [FAIL]\n" @@ -22,7 +22,7 @@ setup_database() { COUNT=$((COUNT+5)) printf "Portus: configuring database..." - docker-compose run --rm web rake db:create db:migrate &> /dev/null + docker-compose run --rm web rake db:create db:migrate portus:create &> /dev/null done printf " [SUCCESS]\n" set -e diff --git a/config/secrets.yml b/config/secrets.yml index 99f313b16..623966a2d 100644 --- a/config/secrets.yml +++ b/config/secrets.yml @@ -14,11 +14,13 @@ development: secret_key_base: 8bc8ccc710eafd73d43cd59ac8881aadc89f7a6ab55f1ac11c97fb436a3931cc78c38e735e664958d9e793725f3d52178f4e2c376c346edbaca3936aebf66e27 encryption_private_key_path: 'vagrant/conf/ca_bundle/server.key' machine_fqdn: 'portus.test.lan' + portus_password: 'portus1234' test: secret_key_base: 03423ada1c1d3dce1638664c17ad9debe3401fa51ae332ddfe9bc04de70466cf2213c619911c181534d2ba77836c0da50ce7e9748aad7c2e5c40e5b8ddb1d997 encryption_private_key_path: 'vagrant/conf/ca_bundle/server.key' machine_fqdn: 'portus.test.lan' + portus_password: 'portus1234' # Do not keep production secrets in the repository, # instead read values from the environment. @@ -26,3 +28,4 @@ production: secret_key_base: <%= ENV["SECRET_KEY_BASE"] %> encryption_private_key_path: 'vagrant/conf/ca_bundle/server.key' machine_fqdn: 'portus.test.lan' + portus_password: <%= ENV["PORTUS_PASSWORD"] %> diff --git a/db/seeds.rb b/db/seeds.rb index 659224372..8dd49eacb 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -1,3 +1,19 @@ +# Adding the Portus user. + +if User.count > 0 + Rails.logger.fatal "The DB is not empty! Only seed for kick-starting your DB" + exit(-1) +end + +Rails.logger.info "Adding the \"portus\" user" +User.create!( + username: "portus", + password: Rails.application.secrets.portus_password, + email: "portus@portus.com", + admin: true +) + +# Adding a user and a registry for integration tests. if ENV["INTEGRATION_TESTS"] Rails.logger.info "Adding user username31" if User.find_by_username("username31") diff --git a/lib/tasks/.keep b/lib/tasks/.keep deleted file mode 100644 index e69de29bb..000000000 diff --git a/lib/tasks/portus_user.rake b/lib/tasks/portus_user.rake new file mode 100644 index 000000000..32981b8c3 --- /dev/null +++ b/lib/tasks/portus_user.rake @@ -0,0 +1,11 @@ +namespace :portus do + desc 'It tries to create the "portus" user on an existing DB' + task :create => :environment do + User.create!( + username: "portus", + password: Rails.application.secrets.portus_password, + email: "portus@portus.com", + admin: true + ) + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 5afb1a082..06c4d8727 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -7,7 +7,6 @@ it { should validate_uniqueness_of(:email) } it { should validate_uniqueness_of(:username) } it { should allow_value("test1", "1test").for(:username) } - it { should_not allow_value("portus", "foo", "1Test", "another_test").for(:username) } it "should block user creation when the private namespace is not available" do name = "coolname" From b5cdbd47a19e8f1d0f152b2974ccc08bc783a4f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miquel=20Sabat=C3=A9?= Date: Fri, 7 Aug 2015 14:55:01 +0200 Subject: [PATCH 02/11] Added documentation for some of the classes dealing with authentication --- app/controllers/api/base_controller.rb | 16 ------ app/controllers/api/v2/tokens_controller.rb | 26 +++++++++ app/models/registry_client.rb | 61 ++++++++++++++++----- 3 files changed, 74 insertions(+), 29 deletions(-) diff --git a/app/controllers/api/base_controller.rb b/app/controllers/api/base_controller.rb index 8cb8d86c9..72ca6776a 100644 --- a/app/controllers/api/base_controller.rb +++ b/app/controllers/api/base_controller.rb @@ -18,20 +18,4 @@ class WrongPortusOTP < StandardError; end def deny_access head :unauthorized end - - def scope_handler(registry, scope_string) - type = scope_string.split(":", 3)[0] - - case type - when "repository" - auth_scope = Namespace::AuthScope.new(registry, scope_string) - else - logger.error "Scope not handled: #{type}" - raise ScopeNotHandled - end - - scopes = scope_string.split(":", 3)[2].split(",") - - [auth_scope, scopes] - end end diff --git a/app/controllers/api/v2/tokens_controller.rb b/app/controllers/api/v2/tokens_controller.rb index 60e94b9ef..8b52546da 100644 --- a/app/controllers/api/v2/tokens_controller.rb +++ b/app/controllers/api/v2/tokens_controller.rb @@ -1,6 +1,11 @@ +# TokensController is used to deliver the token that the docker client should +# use in order to perform operation into the registry. This is the last step in +# the authentication process for Portus' point of view. class Api::V2::TokensController < Api::BaseController before_action :authenticate_user! + # Returns the token that the docker client should use in order to perform + # operation into the private registry. def show registry = Registry.find_by(hostname: params["service"]) raise RegistryNotHandled if registry.nil? @@ -19,6 +24,9 @@ def show private + # If there was a scope specified in the request parameters, try to authorize + # the given scopes. That is, it "filters" the scopes that can be requested + # depending of the issuer of the request and its permissions. def authorize_scopes(registry) return unless params[:scope] @@ -34,4 +42,22 @@ def authorize_scopes(registry) auth_scope end + + # From the given scope string, try to fetch a scope handler class for it. + # Scope handlers are defined in "app/models/*/auth_scope.rb" files. + def scope_handler(registry, scope_string) + type = scope_string.split(":", 3)[0] + + case type + when "repository" + auth_scope = Namespace::AuthScope.new(registry, scope_string) + else + logger.error "Scope not handled: #{type}" + raise ScopeNotHandled + end + + scopes = scope_string.split(":", 3)[2].split(",") + + [auth_scope, scopes] + end end diff --git a/app/models/registry_client.rb b/app/models/registry_client.rb index d3a20f3db..50cf57929 100644 --- a/app/models/registry_client.rb +++ b/app/models/registry_client.rb @@ -1,7 +1,21 @@ +# RegistryClient is a a layer between Portus and the Registry. Given a set of +# credentials, it's able to call to any endpoint in the registry API. Moreover, +# it also implements some handy methods on top of some of these endpoints (e.g. +# the `manifest` method for the Manifest API endpoints). class RegistryClient + # As specified in the token specification of distribution, the client will + # get a 401 on the first attempt of logging in, but in there should be the + # "WWW-Authenticate" header. This exception will be raised when there's no + # authentication token bearer. class NoBearerRealmException < RuntimeError; end + + # Raised when the authorization token could not be fetched. class AuthorizationError < RuntimeError; end - class ManifestNotFoundError < RuntimeError; end + + # Used when a resource was not found for the given endpoint. + class NotFoundError < RuntimeError; end + + # Raised if this client does not have the credentials to perform an API call. class CredentialsMissingError < RuntimeError; end def initialize(host, use_ssl = true, username = nil, password = nil) @@ -12,43 +26,62 @@ def initialize(host, use_ssl = true, username = nil, password = nil) @password = password end - def credentials? - @username && @password - end - + # Retrieves the manifest for the required repository:tag. If everything goes + # well, it will return a parsed response from the registry, otherwise it will + # raise either ManifestNotFoundError or a RuntimeError. def manifest(repository, tag = "latest") res = get_request("#{repository}/manifests/#{tag}") if res.code.to_i == 200 JSON.parse(res.body) elsif res.code.to_i == 404 - raise ManifestNotFoundError, "Cannot find manifest for #{repository}:#{tag}" + raise NotFoundError, "Cannot find manifest for #{repository}:#{tag}" else - raise "Something went wrong while fetching manifest for #{repository}:#{tag}:" \ - "[#{res.code}] - #{res.body}" + raise "Something went wrong while fetching manifest for " \ + "#{repository}:#{tag}:[#{res.code}] - #{res.body}" end end + # This is the general method to perform a GET request to an endpoint provided + # by the registry. The first parameter is the URI of the endpoint itself. The + # `request_auth_token` parameter means that if this method gets a 401 when + # calling the given path, it should get an authorization token automatically + # and try again. def get_request(path, request_auth_token = true) uri = URI.join(@base_url, path) - Rails.logger.info uri - Rails.logger.info "Token -> #{@token} <-" req = Net::HTTP::Get.new(uri) + + # This only happens if the auth token has already been set by a previous + # call. req["Authorization"] = "Bearer #{@token}" if @token res = get_response_token(uri, req) if res.code.to_i == 401 - # Note that request_auth_token will raise an exception on error. + # This can mean that this is the first time that the client is calling + # the registry API, and that, therefore, it might need to request the + # authorization token first. if request_auth_token + # Note that request_auth_token will raise an exception on error. request_auth_token(res) + + # Recursive call, but this time we make sure that we don't enter here + # again. If this call fails, then there's something *really* wrong with + # the given credentials. return get_request(path, false) end - else - res end + res end private + # Returns true if this client has the credentials set. + def credentials? + @username && @password + end + + # This method should be called after getting a 401. In this case, the + # registry has sent the proper "WWW-Authenticate" header value that will + # allow us the request a new authorization token for this client. def request_auth_token(unhauthorized_response) bearer_realm, query = parse_unhauthorized_response(unhauthorized_response) @@ -66,6 +99,8 @@ def request_auth_token(unhauthorized_response) end end + # For the given 401 response, try to extract the token and the parameters + # that this client should use in order to request an authorization token. def parse_unhauthorized_response(res) auth_args = res.to_hash["www-authenticate"].first.split(",").each_with_object({}) do |i, h| key, val = i.split("=") From 0ad7754997c4355f71b9df2e7bf90594f53fb237 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miquel=20Sabat=C3=A9?= Date: Fri, 7 Aug 2015 18:07:27 +0200 Subject: [PATCH 03/11] Added the `catalog` method This method returns all the repositories (with their respective tags) that can be found through the new Catalog API from Docker's Distribution v2. --- app/controllers/api/v2/tokens_controller.rb | 7 +++- app/models/namespace/auth_scope.rb | 2 +- app/models/registry/auth_scope.rb | 45 +++++++++++++++++++++ app/models/registry_client.rb | 35 ++++++++++++++++ app/policies/registry_policy.rb | 18 +++++++++ config/application.rb | 3 -- 6 files changed, 104 insertions(+), 6 deletions(-) create mode 100644 app/models/registry/auth_scope.rb create mode 100644 app/policies/registry_policy.rb diff --git a/app/controllers/api/v2/tokens_controller.rb b/app/controllers/api/v2/tokens_controller.rb index 8b52546da..37c9da95c 100644 --- a/app/controllers/api/v2/tokens_controller.rb +++ b/app/controllers/api/v2/tokens_controller.rb @@ -31,6 +31,7 @@ def authorize_scopes(registry) return unless params[:scope] auth_scope, scopes = scope_handler(registry, params[:scope]) + scopes.each do |scope| begin authorize auth_scope.resource, "#{scope}?".to_sym @@ -51,13 +52,15 @@ def scope_handler(registry, scope_string) case type when "repository" auth_scope = Namespace::AuthScope.new(registry, scope_string) + scopes = scope_string.split(":", 3)[2].split(",") + when "registry" + auth_scope = Registry::AuthScope.new(registry, scope_string) + scopes = auth_scope.scopes else logger.error "Scope not handled: #{type}" raise ScopeNotHandled end - scopes = scope_string.split(":", 3)[2].split(",") - [auth_scope, scopes] end end diff --git a/app/models/namespace/auth_scope.rb b/app/models/namespace/auth_scope.rb index 7bd2ed59c..1e0b1cfa2 100644 --- a/app/models/namespace/auth_scope.rb +++ b/app/models/namespace/auth_scope.rb @@ -5,7 +5,7 @@ class ResourceIsNotFound < StandardError; end def initialize(registry, scope_string) @scope_string = scope_string - @registry = registry + @registry = registry parse_scope_string! end diff --git a/app/models/registry/auth_scope.rb b/app/models/registry/auth_scope.rb new file mode 100644 index 000000000..4b926e289 --- /dev/null +++ b/app/models/registry/auth_scope.rb @@ -0,0 +1,45 @@ +# Registry::AuthScope parses the scope string so it can be used afterwards for +# the "registry" type. +class Registry::AuthScope + # The given Registry was not found. + class ResourceIsNotFound < StandardError; end + + attr_accessor :resource, :actions, :resource_type, :resource_name + + def initialize(registry, scope_string) + @scope_string = scope_string + @registry = registry + parse_scope_string! + end + + # Returns the registry required by this scope. + def resource + reg = Registry.find_by(hostname: @registry.hostname) + if reg.nil? + Rails.logger.warn "Could not find registry #{@registry.hostname}" + raise ResourceIsNotFound + end + reg + end + + # Returns an array containing the scopes available for this registry object. + def scopes + return catalog? ? ["all"] : [] + end + + private + + # Returns true if the given scope string corresponds to the /v2/_catalog + # endpoint. + def catalog? + @resource_name == "catalog" && @actions[0] == "*" + end + + # Parses the @scope_string variable into the needed attributes. + def parse_scope_string! + parts = @scope_string.split(":", 3) + @resource_type = parts[0] + @resource_name = parts[1] + @actions = parts[2].split(",") + end +end diff --git a/app/models/registry_client.rb b/app/models/registry_client.rb index 50cf57929..b31d23daf 100644 --- a/app/models/registry_client.rb +++ b/app/models/registry_client.rb @@ -41,6 +41,25 @@ def manifest(repository, tag = "latest") end end + # Fetches all the repositories available in the registry, with all their + # corresponding tags. If something goes wrong while fetching the repos from + # the catalog (e.g. authorization error), it will raise an exception. + # + # Returns an array of hashes which contain two keys: + # - name: a string containing the name of the repository. + # - tags: an array containing the available tags for the repository. + def catalog + res = get_request("_catalog") + if res.code.to_i == 200 + catalog = JSON.parse(res.body) + add_tags(catalog["repositories"]) + elsif res.code.to_i == 404 + raise NotFoundError, "Could not find the catalog endpoint!" + else + raise "Something went wrong while fetching the catalog" + end + end + # This is the general method to perform a GET request to an endpoint provided # by the registry. The first parameter is the URI of the endpoint itself. The # `request_auth_token` parameter means that if this method gets a 401 when @@ -133,4 +152,20 @@ def get_response_token(uri, req) http.request(req) end end + + # Adds the available tags for each of the given repositories. If there is a + # problem while fetching a repository's tag, it will return an empty array. + # Otherwise it will return an array with the results as specified in the + # documentation of the `catalog` method. + def add_tags(repositories) + return [] if repositories.nil? + + result = [] + repositories.each do |repo| + res = get_request("#{repo}/tags/list") + return [] if res.code.to_i != 200 + result << JSON.parse(res.body) + end + result + end end diff --git a/app/policies/registry_policy.rb b/app/policies/registry_policy.rb new file mode 100644 index 000000000..38c37ba70 --- /dev/null +++ b/app/policies/registry_policy.rb @@ -0,0 +1,18 @@ +# RegistryPolicy implements the authorization policy for methods inside the +# "registry" type. +class RegistryPolicy + attr_reader :user + + # Note that the "registry" parameter is not used by this class. + def initialize(user, registry) + raise Pundit::NotAuthorizedError, "must be logged in" unless user + @user = user + end + + # This method defines the permissions for the following scope string + # "registry:catalog:*". Only the "portus" special user is allowed to perform + # this call. + def all? + user.username == "portus" + end +end diff --git a/config/application.rb b/config/application.rb index b3a0d8ad5..22a3083f7 100644 --- a/config/application.rb +++ b/config/application.rb @@ -22,9 +22,6 @@ class Application < Rails::Application # Do not swallow errors in after_commit/after_rollback callbacks. config.active_record.raise_in_transactional_callbacks = true - config.active_record.observers = :user_observer, :registry_observer - - config.otp_secret = ROTP::Base32.random_base32 end end From 2e1c76b399a9c5760d54cb1c0ad3648203fb1a05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miquel=20Sabat=C3=A9?= Date: Mon, 10 Aug 2015 15:39:18 +0200 Subject: [PATCH 04/11] Fix the creation of admins and the listing of users after introducing portus The portus user has to be handled transparently behind the scenes. This means that it must not be listed in the users list for admins, and it has to be hidden when dealing with authentication. Moreover, the admin will have a notice in the dashboard if the Portus user is not there. This should never happen since the portus user is installed always by seeding it. This would mean that there's something really wrong with the DB (some manual update gone wrong). --- app/controllers/admin/dashboard_controller.rb | 1 + app/controllers/auth/registrations_controller.rb | 7 +++++-- app/controllers/auth/sessions_controller.rb | 2 +- app/models/user.rb | 5 +++-- app/views/admin/dashboard/index.html.slim | 12 ++++++++++++ 5 files changed, 22 insertions(+), 5 deletions(-) diff --git a/app/controllers/admin/dashboard_controller.rb b/app/controllers/admin/dashboard_controller.rb index b3b8bf64d..e2f3f7452 100644 --- a/app/controllers/admin/dashboard_controller.rb +++ b/app/controllers/admin/dashboard_controller.rb @@ -3,5 +3,6 @@ def index @recent_activities = PublicActivity::Activity .order("created_at DESC") .limit(20) + @portus_exists = User.where(username: "portus").any? end end diff --git a/app/controllers/auth/registrations_controller.rb b/app/controllers/auth/registrations_controller.rb index d035d6040..887c2bd73 100644 --- a/app/controllers/auth/registrations_controller.rb +++ b/app/controllers/auth/registrations_controller.rb @@ -75,12 +75,12 @@ def destroy end def check_admin - @admin = User.exists?(admin: true) + @admin = User.admins.any? end def configure_sign_up_params devise_parameter_sanitizer.for(:sign_up) << :email - return if User.exists?(admin: true) + return if User.admins.any? devise_parameter_sanitizer.for(:sign_up) << :admin end @@ -99,6 +99,9 @@ def password_update? # 1. A user can disable himself unless it's the last admin on the system. # 2. The admin user is the only one that can disable other users. def can_disable?(user) + # The "portus" user can never be disabled. + return false if user.username == "portus" + if current_user == user # An admin cannot disable himself if he's the only admin in the system. current_user.admin? && User.admins.count == 1 diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb index 8efe148a3..de34ba80e 100644 --- a/app/controllers/auth/sessions_controller.rb +++ b/app/controllers/auth/sessions_controller.rb @@ -4,7 +4,7 @@ class Auth::SessionsController < Devise::SessionsController # Re-implementing. The logic is: if there is already a user that can log in, # work as usual. Otherwise, redirect always to the signup page. def new - if User.any? + if User.admins.any? super else # For some reason if we get here from the root path, we'll get a flashy diff --git a/app/models/user.rb b/app/models/user.rb index 2e0d0b072..bd688e98e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -12,8 +12,9 @@ class User < ActiveRecord::Base has_many :teams, through: :team_users has_many :stars - scope :enabled, -> { where enabled: true } - scope :admins, -> { where enabled: true, admin: true } + scope :not_portus, -> { where.not username: "portus" } + scope :enabled, -> { not_portus.where enabled: true } + scope :admins, -> { not_portus.where enabled: true, admin: true } def private_namespace_available return unless Namespace.exists?(name: username) diff --git a/app/views/admin/dashboard/index.html.slim b/app/views/admin/dashboard/index.html.slim index b1df07956..9fb89dc92 100644 --- a/app/views/admin/dashboard/index.html.slim +++ b/app/views/admin/dashboard/index.html.slim @@ -1,3 +1,15 @@ +- unless @portus_exists + .alert.alert-danger.fade.in.text-left.alert-dismissible + button.close data-dismiss="alert" type="button" + span aria-hidden="true" × + span.sr-only Close + .row + .alert-icon.pull-left + i.fa.fa-exclamation-circle.fa-3x + .pull-left + p + | The Portus user does not exist! + .row .col-md-3.col-xs-6 .panel-overview From c05112e1733aefd696a1f42225b26d69a213063d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miquel=20Sabat=C3=A9?= Date: Tue, 11 Aug 2015 10:23:16 +0200 Subject: [PATCH 05/11] Extended and improved the test suite regarding the Registry client --- .rubocop.yml | 2 +- app/controllers/auth/sessions_controller.rb | 2 +- app/models/registry/auth_scope.rb | 7 +- app/policies/registry_policy.rb | 3 +- app/views/admin/dashboard/index.html.slim | 8 +- app/views/shared/_notifications.html.slim | 2 +- lib/tasks/portus_user.rake | 2 +- spec/api/v2/token_spec.rb | 77 ++++++----- spec/classes/registry_client_spec.rb | 54 +++++++- spec/features/admin/dashboard_spec.rb | 30 ++++ spec/features/auth/login_feature_spec.rb | 29 ++-- spec/features/auth/signup_feature_spec.rb | 10 ++ spec/models/registry/auth_scope_spec.rb | 45 ++++++ spec/spec_helper.rb | 3 + spec/support/helpers.rb | 13 ++ .../registry/get_missing_catalog_endpoint.yml | 37 +++++ .../registry/get_missing_image_manifest.yml | 6 +- .../registry/get_registry_catalog.yml | 129 ++++++++++++++++++ 18 files changed, 394 insertions(+), 65 deletions(-) create mode 100644 spec/features/admin/dashboard_spec.rb create mode 100644 spec/models/registry/auth_scope_spec.rb create mode 100644 spec/vcr_cassettes/registry/get_missing_catalog_endpoint.yml create mode 100644 spec/vcr_cassettes/registry/get_registry_catalog.yml diff --git a/.rubocop.yml b/.rubocop.yml index ecd23a785..a03415720 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -45,4 +45,4 @@ AllCops: - db/migrate/* - bin/* - vendor/**/* - + - tmp/**/* diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb index de34ba80e..8efe148a3 100644 --- a/app/controllers/auth/sessions_controller.rb +++ b/app/controllers/auth/sessions_controller.rb @@ -4,7 +4,7 @@ class Auth::SessionsController < Devise::SessionsController # Re-implementing. The logic is: if there is already a user that can log in, # work as usual. Otherwise, redirect always to the signup page. def new - if User.admins.any? + if User.any? super else # For some reason if we get here from the root path, we'll get a flashy diff --git a/app/models/registry/auth_scope.rb b/app/models/registry/auth_scope.rb index 4b926e289..11f35fe49 100644 --- a/app/models/registry/auth_scope.rb +++ b/app/models/registry/auth_scope.rb @@ -23,8 +23,10 @@ def resource end # Returns an array containing the scopes available for this registry object. + # It returns a ["*"] if it's an invalid scope, because this will not be + # implemented as a method. def scopes - return catalog? ? ["all"] : [] + catalog? ? ["all"] : ["*"] end private @@ -40,6 +42,7 @@ def parse_scope_string! parts = @scope_string.split(":", 3) @resource_type = parts[0] @resource_name = parts[1] - @actions = parts[2].split(",") + @actions = parts[2] + @actions = @actions.split(",") if @actions end end diff --git a/app/policies/registry_policy.rb b/app/policies/registry_policy.rb index 38c37ba70..b2afcde63 100644 --- a/app/policies/registry_policy.rb +++ b/app/policies/registry_policy.rb @@ -3,8 +3,7 @@ class RegistryPolicy attr_reader :user - # Note that the "registry" parameter is not used by this class. - def initialize(user, registry) + def initialize(user, _) raise Pundit::NotAuthorizedError, "must be logged in" unless user @user = user end diff --git a/app/views/admin/dashboard/index.html.slim b/app/views/admin/dashboard/index.html.slim index 9fb89dc92..158003677 100644 --- a/app/views/admin/dashboard/index.html.slim +++ b/app/views/admin/dashboard/index.html.slim @@ -11,7 +11,7 @@ | The Portus user does not exist! .row - .col-md-3.col-xs-6 + .col-md-3.col-xs-6.registries .panel-overview .panel-heading = link_to 'Registry', admin_registries_path @@ -22,7 +22,7 @@ .col-xs-8 span.fa-3x= Registry.count - .col-md-3.col-xs-6 + .col-md-3.col-xs-6.users .panel-overview .panel-heading = link_to 'Users', admin_users_path @@ -33,7 +33,7 @@ .col-xs-8 span.fa-3x= User.enabled.count - .col-md-3.col-xs-6 + .col-md-3.col-xs-6.teams .panel-overview .panel-heading = link_to 'Teams', admin_teams_path @@ -46,7 +46,7 @@ .clearfix.visible-xs-block - .col-md-3.col-xs-6 + .col-md-3.col-xs-6.namespaces .panel-overview .panel-heading = link_to 'Namespaces', admin_namespaces_path diff --git a/app/views/shared/_notifications.html.slim b/app/views/shared/_notifications.html.slim index a4ce90bbd..9b05ab2ae 100644 --- a/app/views/shared/_notifications.html.slim +++ b/app/views/shared/_notifications.html.slim @@ -1,4 +1,4 @@ -- if current_user && !current_page?(url_for new_admin_registry_path) && Registry.count == 0 +- if current_user && !current_page?(url_for new_admin_registry_path) && !Registry.any? .alert.alert-danger.fade.in.text-left.alert-dismissible button.close data-dismiss="alert" type="button" span aria-hidden="true" × diff --git a/lib/tasks/portus_user.rake b/lib/tasks/portus_user.rake index 32981b8c3..42a381866 100644 --- a/lib/tasks/portus_user.rake +++ b/lib/tasks/portus_user.rake @@ -1,6 +1,6 @@ namespace :portus do desc 'It tries to create the "portus" user on an existing DB' - task :create => :environment do + task create: :environment do User.create!( username: "portus", password: Rails.application.secrets.portus_password, diff --git a/spec/api/v2/token_spec.rb b/spec/api/v2/token_spec.rb index d769673cd..34f52d281 100644 --- a/spec/api/v2/token_spec.rb +++ b/spec/api/v2/token_spec.rb @@ -52,40 +52,6 @@ end end - context "as the special portus user" do - - it "allows access when the one time password is valid" do - totp = ROTP::TOTP.new(Rails.application.config.otp_secret) - auth_header = { - "HTTP_AUTHORIZATION" => auth_mech.encode_credentials("portus", totp.now) - } - - get v2_token_url, { - service: registry.hostname, - account: "portus", - scope: "repository:foo/me:push" }, - auth_header - expect(response.status).to eq 200 - end - - it "blocks access when the time based OTP is not valid" do - auth_header = {} - - Timecop.travel(30.seconds.ago) do - totp = ROTP::TOTP.new(Rails.application.config.otp_secret) - auth_header["HTTP_AUTHORIZATION"] = auth_mech.encode_credentials( - "portus", totp.now) - end - - get v2_token_url, { - service: registry.hostname, - account: "portus", - scope: "repository:foo/me:push" }, - auth_header - expect(response.status).to eq 401 - end - end - context "as valid user" do let(:valid_request) do { @@ -158,6 +124,49 @@ end end + context "registry scope" do + let(:valid_request) do + { + service: registry.hostname, + account: "portus", + scope: "registry:catalog:*" + } + end + + let(:valid_portus_auth_header) do + { "HTTP_AUTHORIZATION" => auth_mech.encode_credentials("portus", Rails.application.secrets.portus_password) } + end + + before do + User.create!( + username: "portus", + password: Rails.application.secrets.portus_password, + email: "portus@portus.com", + admin: true + ) + end + + it "allows portus to access the Catalog API" do + get v2_token_url, valid_request, valid_portus_auth_header + expect(response.status).to eq 200 + + token = JSON.parse(response.body)["token"] + payload = JWT.decode(token, nil, false, leeway: 2)[0] + expect(payload["sub"]).to eq "portus" + expect(payload["aud"]).to eq registry.hostname + expect(payload["access"][0]["type"]).to eq "registry" + expect(payload["access"][0]["name"]).to eq "catalog" + expect(payload["access"][0]["actions"][0]).to eq "*" + end + + it "forbids unhandled methods from the registry type" do + wr = valid_request + wr[:scope] = "registry:catalog:lala" + get v2_token_url, wr, valid_portus_auth_header + expect(response.status).to eq 401 + end + end + context "unknown scope" do it "denies access" do get v2_token_url, diff --git a/spec/classes/registry_client_spec.rb b/spec/classes/registry_client_spec.rb index ee032c142..c6f3be404 100644 --- a/spec/classes/registry_client_spec.rb +++ b/spec/classes/registry_client_spec.rb @@ -157,7 +157,7 @@ expect do registry.manifest(repository, "2.0.0") - end.to raise_error(RegistryClient::ManifestNotFoundError) + end.to raise_error(RegistryClient::NotFoundError) end end @@ -183,6 +183,58 @@ WebMock.allow_net_connect! end end + end + + context "fetching Catalog from registry" do + it "returns the available catalog" do + create(:registry) + create(:admin, username: "portus") + + VCR.use_cassette("registry/get_registry_catalog", record: :none) do + registry = RegistryClient.new( + registry_server, + false, + "portus", + Rails.application.secrets.portus_password) + + expect(registry.catalog).to be_empty + end + end + + it "fails if this version of registry does not implement /v2/_catalog" do + VCR.use_cassette("registry/get_missing_catalog_endpoint", record: :none) do + registry = RegistryClient.new( + registry_server, + false, + username, + password) + + expect do + registry.catalog + end.to raise_error(RegistryClient::NotFoundError) + end + end + it "raises an exception when the return code is different from 200 or 401" do + registry = RegistryClient.new( + registry_server, + false, + username, + password) + + begin + VCR.turned_off do + WebMock.disable_net_connect! + stub_request(:get, "http://#{registry_server}/v2/_catalog") + .to_return(body: "BOOM", status: 500) + + expect do + registry.catalog + end.to raise_error(RuntimeError) + end + ensure + WebMock.allow_net_connect! + end + end end end diff --git a/spec/features/admin/dashboard_spec.rb b/spec/features/admin/dashboard_spec.rb new file mode 100644 index 000000000..ce7dcab4d --- /dev/null +++ b/spec/features/admin/dashboard_spec.rb @@ -0,0 +1,30 @@ +require "rails_helper" +require "pp" + +feature "Admin - Dashboard" do + let!(:registry) { create(:registry) } + let!(:admin) { create(:admin) } + let!(:user) { create(:user) } + + before do + login_as admin + visit admin_dashboard_index_path + end + + scenario "The dashboard does not count the Portus user", js: true do + create_portus_user! + visit admin_dashboard_index_path + + # 3 users: admin, user and the one created by the registry. + expect(find(".users span").text).to eq "3" + end + + scenario "Warn the admin that the portus user does not exist", js: true do + expect(page).to have_content("The Portus user does not exist!") + + create_portus_user! + visit admin_dashboard_index_path + + expect(page).to_not have_content("The Portus user does not exist!") + end +end diff --git a/spec/features/auth/login_feature_spec.rb b/spec/features/auth/login_feature_spec.rb index 3a2700483..9967c3e11 100644 --- a/spec/features/auth/login_feature_spec.rb +++ b/spec/features/auth/login_feature_spec.rb @@ -1,53 +1,52 @@ require "rails_helper" feature "Login feature" do - let!(:user) { create(:user) } before do - visit new_user_session_url + visit new_user_session_path end - scenario "It does not show any flash when accessing for the first time" do - visit root_url + scenario "It does not show any flash when accessing for the first time", js: true do + visit root_path expect(page).to_not have_content("You need to sign in or sign up before continuing.") end - scenario "Existing user is able using his login and password to login into Portus" do + scenario "Existing user is able using his login and password to login into Portus", js: true do expect(page).to_not have_content("Invalid username or password") # We don't use Capybara's `login_as` method on purpose, because we are # testing the UI for logging in. fill_in "user_username", with: user.username fill_in "user_password", with: user.password - click_button "Login" + find("button").click expect(page).to have_content("Activities") expect(page).to_not have_content("Signed in") end - scenario "Wrong password results in an error message" do + scenario "Wrong password results in an error message", js: true do fill_in "user_username", with: "foo" fill_in "user_password", with: "bar" - click_button "Login" - expect(current_url).to eq new_user_session_url + find("button").click + expect(current_path).to eq new_user_session_path expect(page).to have_content("Invalid username or password") end - scenario "When guest is trying to access dashboard - he should be redirected to login page" do - visit root_url + scenario "When guest is trying to access dashboard - he should be redirected to login page", js: true do + visit root_path expect(page).to have_content("Login") - expect(current_url).to eq root_url + expect(current_path).to eq root_path end - scenario "A disabled user cannot login" do + scenario "A disabled user cannot login", js: true do user.update_attributes(enabled: false) fill_in "user_username", with: user.username fill_in "user_password", with: user.password - click_button "Login" + find("button").click expect(page).to have_content(user.inactive_message) expect(page).to have_content("Login") - expect(current_url).to eq new_user_session_url + expect(current_path).to eq new_user_session_path end end diff --git a/spec/features/auth/signup_feature_spec.rb b/spec/features/auth/signup_feature_spec.rb index 9154317be..2edb538c4 100644 --- a/spec/features/auth/signup_feature_spec.rb +++ b/spec/features/auth/signup_feature_spec.rb @@ -24,6 +24,16 @@ expect(page).to have_css("#user_admin") end + scenario "The portus user does not interfere with regular admin creation" do + User.delete_all + create_portus_user! + + visit new_user_registration_url + + expect(page).to have_content("Create admin") + expect(page).to have_css("#user_admin") + end + scenario "As a guest I am able to signup" do expect(page).to_not have_content("Create admin") fill_in "user_username", with: user.username diff --git a/spec/models/registry/auth_scope_spec.rb b/spec/models/registry/auth_scope_spec.rb new file mode 100644 index 000000000..2500a547f --- /dev/null +++ b/spec/models/registry/auth_scope_spec.rb @@ -0,0 +1,45 @@ +require "rails_helper" + +describe Registry::AuthScope, type: :model do + describe "registries" do + it "raises an exception if there's no registry" do + reg = Registry.new(hostname: "host", name: "name") + scope = Registry::AuthScope.new(reg, "scope:string:lala") + expect do + scope.resource + end.to raise_error(Registry::AuthScope::ResourceIsNotFound) + end + + it "returns the current registry" do + reg = create(:registry) + scope = Registry::AuthScope.new(reg, "scope:string:lala") + res = scope.resource + expect(reg.hostname).to eq res.hostname + expect(reg.name).to eq res.name + end + end + + describe "scopes" do + let!(:registry) { create(:registry) } + + it "returns an invalid scope for incomplete scope strings" do + ["scope", "scope:lala"].each do |sc| + scope = Registry::AuthScope.new(registry, sc) + expect(scope.scopes).to match_array(["*"]) + end + end + + it "returns an invalid scope for an unknown scope string" do + # The resource type is guaranteed to equal "registry" by the caller. + ["registry:cata:aa", "registry:catalog:lala", "registry:lala:*"].each do |sc| + scope = Registry::AuthScope.new(registry, sc) + expect(scope.scopes).to match_array(["*"]) + end + end + + it 'returns "all" for a valid scope' do + scope = Registry::AuthScope.new(registry, "registry:catalog:*") + expect(scope.scopes).to match_array(["all"]) + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 530f60bc4..ef432ce3f 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -16,6 +16,9 @@ # So code coverage reports can be submitted to codeclimate.com c.ignore_hosts "codeclimate.com" + + # To debug when a VCR goes wrong. + # c.debug_logger = $stdout end RSpec.configure do |config| diff --git a/spec/support/helpers.rb b/spec/support/helpers.rb index 005076cc2..53b507ab9 100644 --- a/spec/support/helpers.rb +++ b/spec/support/helpers.rb @@ -17,6 +17,19 @@ def focused_element_id def disabled?(selector) page.evaluate_script("$('#{selector}').attr('disabled')") == "disabled" end + + # Creates the Portus user. The Portus user cannot be created with neither the + # "user" factory nor the "admin" one. This is because in the application this + # same user is created in a special way (directly, without associating a + # namespace to it, etc.). + def create_portus_user! + User.create!( + username: "portus", + password: Rails.application.secrets.portus_password, + email: "portus@portus.com", + admin: true + ) + end end RSpec.configure { |config| config.include Helpers, type: :feature } diff --git a/spec/vcr_cassettes/registry/get_missing_catalog_endpoint.yml b/spec/vcr_cassettes/registry/get_missing_catalog_endpoint.yml new file mode 100644 index 000000000..10dad3df9 --- /dev/null +++ b/spec/vcr_cassettes/registry/get_missing_catalog_endpoint.yml @@ -0,0 +1,37 @@ +--- +http_interactions: +- request: + method: get + uri: http://registry.test.lan/v2/_catalog + body: + encoding: US-ASCII + string: '' + headers: + Accept-Encoding: + - gzip;q=1.0,deflate;q=0.6,identity;q=0.3 + Accept: + - "*/*" + User-Agent: + - Ruby + Host: + - registry.test.lan + response: + status: + code: 404 + message: Not Found + headers: + Content-Type: + - application/json; charset=utf-8 + Docker-Distribution-Api-Version: + - registry/2.0 + Date: + - Mon, 10 Aug 2015 15:40:24 GMT + Content-Length: + - '19' + body: + encoding: UTF-8 + string: | + 404 page not found + http_version: + recorded_at: Mon, 10 Aug 2015 15:40:24 GMT +recorded_with: VCR 2.9.3 diff --git a/spec/vcr_cassettes/registry/get_missing_image_manifest.yml b/spec/vcr_cassettes/registry/get_missing_image_manifest.yml index 6b009c867..a4c64d69e 100644 --- a/spec/vcr_cassettes/registry/get_missing_image_manifest.yml +++ b/spec/vcr_cassettes/registry/get_missing_image_manifest.yml @@ -34,7 +34,7 @@ http_interactions: encoding: UTF-8 string: | {"errors":[{"code":"UNAUTHORIZED","message":"access to the requested resource is not authorized","detail":[{"Type":"repository","Name":"foo/busybox","Action":"pull"}]}]} - http_version: + http_version: recorded_at: Mon, 20 Apr 2015 10:11:01 GMT - request: method: get @@ -90,7 +90,7 @@ http_interactions: body: encoding: UTF-8 string: '{"token":"eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsImtpZCI6IlBUV1Q6Rk5KRTo3VFc3OlVMSTc6RFpRQTpKSkpJOlJESlE6Mk03NjpIRDZHOlpSU0M6VlBJRjpPNUJVIn0.eyJpc3MiOiJwb3J0dXMudGVzdC5sYW4iLCJzdWIiOiJmbGF2aW8iLCJhdWQiOiJyZWdpc3RyeS50ZXN0LmxhbiIsImV4cCI6MTQyOTUyNDk2MSwibmJmIjoxNDI5NTI0NjYxLCJpYXQiOjE0Mjk1MjQ2NjEsImp0aSI6Ijl4ak16VEJ1YnFZRDJKM1JzcHlLWVJkZWFGcWVEUllRRlk0R1BlajdabSIsImFjY2VzcyI6W3sidHlwZSI6InJlcG9zaXRvcnkiLCJuYW1lIjoiZm9vL2J1c3lib3giLCJhY3Rpb25zIjpbInB1bGwiXX1dfQ.o5oDGAeMSsmjVBaCK0JAfBheH7lPlR0HvjCq5pV-i6pDvcZ3V5thMBLBxaxxMJ0oX2WB3U9tjjQs7nSnYaGu-4-WFHBOYnDYHIB0tAQUBryelPc_bfwbp85Jkg4IDXhxDvOjJUkmZOcLhRW1saQSs4Y55rpHDKSb8W2R4JfZCxIBD0FlFh5Wl2ypj3zddVqO__SN4vSv9s70xLJFI-4Dk4XWU5mV0m4rHfZjpG4bplmfdmeEww_bnnZ0AN6AaW3KR-wk_6jsQzK_HVE9BiB9GlZT7okvpjJQlgNM1k2MBg4A-Ecc2KdbqEokN9t5KOGAqN6UZKHE9gVbOsYQvR-xJeUOGB3xFQAF6INRZkyk8CTbsNTFcTOslRAMt_dTVcKeDnRheZY9aTNgI7WxthNu93-zt8O9jbOpfQphjylAI-gtKulbDPmKChhSJglX6rkKaEDhhJzGbnGYYLXRroFHhbnuKRh7uJrBYdDEVJqyk8hE6iQp2ON2ZEvQTjpYd_ZX8rMeFrraCaxcKm-zQdjqfx0co3kB8YdF6ujRnQdXssPqHF4IXvfE5DKY9Uo2brhqFmoCYaDta9uRRiy19MYwVCiMKpDYHs_NrygLVnbtr8ECJ9sol7x1yyH06Dw6boJET9DEIy4aH1bdkY5WPoKCM2qdbODC7MBgpY2sTJfzEZY"}' - http_version: + http_version: recorded_at: Mon, 20 Apr 2015 10:11:01 GMT - request: method: get @@ -126,6 +126,6 @@ http_interactions: encoding: UTF-8 string: | {"errors":[{"code":"MANIFEST_UNKNOWN","message":"manifest unknown","detail":"unknown manifest name=foo/busybox tag=2.0.0"}]} - http_version: + http_version: recorded_at: Mon, 20 Apr 2015 10:11:01 GMT recorded_with: VCR 2.9.3 diff --git a/spec/vcr_cassettes/registry/get_registry_catalog.yml b/spec/vcr_cassettes/registry/get_registry_catalog.yml new file mode 100644 index 000000000..f8dbb7b96 --- /dev/null +++ b/spec/vcr_cassettes/registry/get_registry_catalog.yml @@ -0,0 +1,129 @@ +--- +http_interactions: +- request: + method: get + uri: http://portus.test.lan/v2/_catalog + body: + encoding: US-ASCII + string: '' + headers: + Accept-Encoding: + - gzip;q=1.0,deflate;q=0.6,identity;q=0.3 + Accept: + - "*/*" + User-Agent: + - Ruby + Host: + - registry.test.lan + response: + status: + code: 401 + message: Unauthorized + headers: + Content-Type: + - application/json; charset=utf-8 + Docker-Distribution-Api-Version: + - registry/2.0 + Www-Authenticate: + - Bearer realm="http://portus.test.lan/v2/token",service="registry.test.lan",scope="registry:catalog:*" + Date: + - Mon, 10 Aug 2015 15:51:10 GMT + Content-Length: + - '161' + body: + encoding: UTF-8 + string: | + {"errors":[{"code":"UNAUTHORIZED","message":"access to the requested resource is not authorized","detail":[{"Type":"registry","Name":"catalog","Action":"*"}]}]} + http_version: + recorded_at: Mon, 10 Aug 2015 15:51:10 GMT +- request: + method: get + uri: http://portus.test.lan/v2/token?account=portus&scope=registry%3Acatalog%3A%2A&service=registry.test.lan + body: + encoding: US-ASCII + string: '' + headers: + Accept-Encoding: + - gzip;q=1.0,deflate;q=0.6,identity;q=0.3 + Accept: + - "*/*" + User-Agent: + - Ruby + Host: + - portus.test.lan + response: + status: + code: 200 + message: OK + headers: + Date: + - Mon, 10 Aug 2015 16:06:08 GMT + Server: + - Apache + Cache-Control: + - max-age=0, private, must-revalidate + X-Frame-Options: + - SAMEORIGIN + X-Xss-Protection: + - 1; mode=block + X-Content-Type-Options: + - nosniff + X-Runtime: + - '0.263900' + X-Request-Id: + - 72aaedfd-506a-410c-b018-5534b2b7aebb + Connection: + - close + X-Powered-By: + - Phusion Passenger 5.0.7 + Set-Cookie: + - _portus_session=YktlejJ3RnNKU1llVDlnVVE2SnlNYkJsMndyQ2dtRWsrR08reEVzeWRRcFE4ZDVOOTFQTDFkYWh1SG8rYjJ1bDBFNmN5WU5WdExjaVpFeXRsMG85V0E9PS0tSVo2NUlMTTZjVThpWjhQVlNWWk1pUT09--e650cb0b5377c519657640423ba49e7bbb55fa21; path=/; HttpOnly + Etag: + - W/"3e175b01d330eb782809fd66157395db" + Status: + - 200 OK + Transfer-Encoding: + - chunked + Content-Type: + - application/json; charset=utf-8 + body: + encoding: UTF-8 + string: '{"token":"eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsImtpZCI6IlBUV1Q6Rk5KRTo3VFc3OlVMSTc6RFpRQTpKSkpJOlJESlE6Mk03NjpIRDZHOlpSU0M6VlBJRjpPNUJVIn0.eyJpc3MiOiJwb3J0dXMudGVzdC5sYW4iLCJzdWIiOiJwb3J0dXMiLCJhdWQiOiJyZWdpc3RyeS50ZXN0LmxhbiIsImV4cCI6MTQzOTIyMzA2OCwibmJmIjoxNDM5MjIyNzYzLCJpYXQiOjE0MzkyMjI3NjMsImp0aSI6ImlFa3lMSnRrVmhSMzI2aloxdGFhRTR2RHBuWTJSblNWQzhZRWN4dDdGdCIsImFjY2VzcyI6W3sidHlwZSI6InJlZ2lzdHJ5IiwibmFtZSI6ImNhdGFsb2ciLCJhY3Rpb25zIjpbIioiXX1dfQ.K6iX7ESlYcgiLwRZkxRuj6rmfFOu6o_WwbDIeEAOZ6bcPGPuD--fi24Y6HyIWQ5eAppyJaq6kASjMwDzHFnYuIWDhVr4zcED0T4BbKsUNf8VWpPtuZRK1-oJnYrVmHPdWJYGY1RzbfvthCJkfoIRKURPzNwGScJDb2391NsqZfPwrEFix3uLQEsiuLsSJ-c_StkL1kTbTspNrmtn7cVywERbvZo_T1MxhnGqPIz0oWpgL5PchCg8eByMiUW5Q8dBYNDzqVgpSVemJn_a_f5ybevf_0jADLxbRLOg4S73JgZaC_20RCygEkPEObI79MHgrlKTlaquLwQ6F4fYNEdc6oO9_xNjRtZdS_apwtbu_TpiVWaeESvTgNbggihc8tpUUJ8Ga4VwvFtp8akw9mkyvNKGUkyx70cqr2eASdmdNgfZdP5_blhK928wU8kO8s0j13CcDGTmL1nS_3vcJ_e_BFrAdNPSCg99zMUz2k2nHp_begtilU6OfzmWqkhlrGFcsWf19YI79LlQTMWNzC_T_lAsdyTSFnfeU1dJeZXvNT7OIN5zEj32_3U9hRwjtM8cx0T8LfG4In6ZGlPwrPnreQlYoG20-hPfkMCKKCptI3Dr7ecT9N1C_84-IH9IXk-VPhtc2cLg-011U32HYt759-NZetBdMLEuDFW2NlnRAxk"}' + http_version: + recorded_at: Mon, 10 Aug 2015 16:06:08 GMT +- request: + method: get + uri: http://registry.test.lan/v2/_catalog + body: + encoding: US-ASCII + string: '' + headers: + Accept-Encoding: + - gzip;q=1.0,deflate;q=0.6,identity;q=0.3 + Accept: + - "*/*" + User-Agent: + - Ruby + Host: + - registry.test.lan + Authorization: + - Bearer eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsImtpZCI6IlBUV1Q6Rk5KRTo3VFc3OlVMSTc6RFpRQTpKSkpJOlJESlE6Mk03NjpIRDZHOlpSU0M6VlBJRjpPNUJVIn0.eyJpc3MiOiJwb3J0dXMudGVzdC5sYW4iLCJzdWIiOiJwb3J0dXMiLCJhdWQiOiJyZWdpc3RyeS50ZXN0LmxhbiIsImV4cCI6MTQzOTIyMzU4MiwibmJmIjoxNDM5MjIzMjc3LCJpYXQiOjE0MzkyMjMyNzcsImp0aSI6IjFORmhzY3JjajRrYTdacjF4VkJjM25aVzM3aWJWZFZ1UFplOUVKVE5XRyIsImFjY2VzcyI6W3sidHlwZSI6InJlZ2lzdHJ5IiwibmFtZSI6ImNhdGFsb2ciLCJhY3Rpb25zIjpbIioiXX1dfQ.gvfa38eANCchwKGdNDDJOGmkcBrNvkjIioH46v8N_kp5J06TGFOiXN4MFEzBDI0zGPrGCksrCJwU-vV_2FH8QYULBMGNGNtCrn9HocF9KlsXeRnCOo2yzj5v5zn87htu5clWNW924WWXEn7QU3fbUphwYt7aI_yTbj_3vy5Wxcg0aTZFVWstIzwbYPVyrcelNJpl9FT1A-5qh_UuDFDu8vSg9x4d_YGcACYXiu64E0Pf5lPLqCgD8v4J7C9IhIxu2B2y1STXQYpQ-6EuzBn5P-2WoiH9T7X1VZkwICroziNPHoSLTr7IszBXUcUA_6APkvZpRVZ5TzmF30HOdJeG8yOb5vCgTKZU6mhpXYwpCPXCGkHmBaIVfugzfWAKf51V0MdqMN1DREWmLl7LYKs0qFUi8Q1YTOJFSSPBsiXQWw1h1oMAqLzBTHqhcQX30W8gOHW0x2PD4lbc4eqUs0ThWF3p2hL3Dc4xRk0NV--ll_oeaptVz3JqGvrJQRTuW9yA_50J7j6EU0jTGFGDKhqY7mc2PjxObvZH_1X2t2cfkzkMjVc9KsTafcdlI9ukZ2Y8QwIZksuK_1-wlOiFMSaEVQHdCxuXODjDfy6y3cJQgMLEchdihDOhlkGV4dwltkex3wKXfOoJG23dNKMyy0yN6p-rK47PByyvPXCgcd_Nv6w + response: + status: + code: 200 + message: OK + headers: + Content-Length: + - '20' + Content-Type: + - application/json; charset=utf-8 + Docker-Distribution-Api-Version: + - registry/2.0 + Date: + - Mon, 10 Aug 2015 16:06:08 GMT + body: + string: | + {"repositories":[]} + http_version: + recorded_at: Mon, 10 Aug 2015 16:06:08 GMT +recorded_with: VCR 2.9.3 From d05dad9129ed8ccb32fb2ae7d764461cc96d9daf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miquel=20Sabat=C3=A9?= Date: Tue, 11 Aug 2015 10:55:27 +0200 Subject: [PATCH 06/11] spec: adding more tests for the `add_tags` method --- spec/classes/registry_client_spec.rb | 6 ++- .../registry/get_registry_catalog.yml | 37 ++++++++++++++++++- 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/spec/classes/registry_client_spec.rb b/spec/classes/registry_client_spec.rb index c6f3be404..ff288aa34 100644 --- a/spec/classes/registry_client_spec.rb +++ b/spec/classes/registry_client_spec.rb @@ -186,7 +186,7 @@ end context "fetching Catalog from registry" do - it "returns the available catalog" do + it "returns the available catalog", focus: true do create(:registry) create(:admin, username: "portus") @@ -197,7 +197,9 @@ "portus", Rails.application.secrets.portus_password) - expect(registry.catalog).to be_empty + catalog = registry.catalog + expect(catalog.length).to be 1 + expect(catalog[0]["busybox"]).to match_array(["latest"]) end end diff --git a/spec/vcr_cassettes/registry/get_registry_catalog.yml b/spec/vcr_cassettes/registry/get_registry_catalog.yml index f8dbb7b96..4ba094a73 100644 --- a/spec/vcr_cassettes/registry/get_registry_catalog.yml +++ b/spec/vcr_cassettes/registry/get_registry_catalog.yml @@ -123,7 +123,42 @@ http_interactions: - Mon, 10 Aug 2015 16:06:08 GMT body: string: | - {"repositories":[]} + {"repositories":["busybox"]} + http_version: + recorded_at: Mon, 10 Aug 2015 16:06:08 GMT +- request: + method: get + uri: http://registry.test.lan/v2/busybox/tags/list + body: + encoding: US-ASCII + string: '' + headers: + Accept-Encoding: + - gzip;q=1.0,deflate;q=0.6,identity;q=0.3 + Accept: + - "*/*" + User-Agent: + - Ruby + Host: + - registry.test.lan + Authorization: + - Bearer eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsImtpZCI6IlBUV1Q6Rk5KRTo3VFc3OlVMSTc6RFpRQTpKSkpJOlJESlE6Mk03NjpIRDZHOlpSU0M6VlBJRjpPNUJVIn0.eyJpc3MiOiJwb3J0dXMudGVzdC5sYW4iLCJzdWIiOiJwb3J0dXMiLCJhdWQiOiJyZWdpc3RyeS50ZXN0LmxhbiIsImV4cCI6MTQzOTIyMzU4MiwibmJmIjoxNDM5MjIzMjc3LCJpYXQiOjE0MzkyMjMyNzcsImp0aSI6IjFORmhzY3JjajRrYTdacjF4VkJjM25aVzM3aWJWZFZ1UFplOUVKVE5XRyIsImFjY2VzcyI6W3sidHlwZSI6InJlZ2lzdHJ5IiwibmFtZSI6ImNhdGFsb2ciLCJhY3Rpb25zIjpbIioiXX1dfQ.gvfa38eANCchwKGdNDDJOGmkcBrNvkjIioH46v8N_kp5J06TGFOiXN4MFEzBDI0zGPrGCksrCJwU-vV_2FH8QYULBMGNGNtCrn9HocF9KlsXeRnCOo2yzj5v5zn87htu5clWNW924WWXEn7QU3fbUphwYt7aI_yTbj_3vy5Wxcg0aTZFVWstIzwbYPVyrcelNJpl9FT1A-5qh_UuDFDu8vSg9x4d_YGcACYXiu64E0Pf5lPLqCgD8v4J7C9IhIxu2B2y1STXQYpQ-6EuzBn5P-2WoiH9T7X1VZkwICroziNPHoSLTr7IszBXUcUA_6APkvZpRVZ5TzmF30HOdJeG8yOb5vCgTKZU6mhpXYwpCPXCGkHmBaIVfugzfWAKf51V0MdqMN1DREWmLl7LYKs0qFUi8Q1YTOJFSSPBsiXQWw1h1oMAqLzBTHqhcQX30W8gOHW0x2PD4lbc4eqUs0ThWF3p2hL3Dc4xRk0NV--ll_oeaptVz3JqGvrJQRTuW9yA_50J7j6EU0jTGFGDKhqY7mc2PjxObvZH_1X2t2cfkzkMjVc9KsTafcdlI9ukZ2Y8QwIZksuK_1-wlOiFMSaEVQHdCxuXODjDfy6y3cJQgMLEchdihDOhlkGV4dwltkex3wKXfOoJG23dNKMyy0yN6p-rK47PByyvPXCgcd_Nv6w + response: + status: + code: 200 + message: OK + headers: + Content-Length: + - '23' + Content-Type: + - application/json; charset=utf-8 + Docker-Distribution-Api-Version: + - registry/2.0 + Date: + - Mon, 10 Aug 2015 16:06:08 GMT + body: + string: | + {"busybox":["latest"]} http_version: recorded_at: Mon, 10 Aug 2015 16:06:08 GMT recorded_with: VCR 2.9.3 From 15e92e6e8db371c57dd23c75ee77f89bde6812ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miquel=20Sabat=C3=A9?= Date: Tue, 11 Aug 2015 11:27:13 +0200 Subject: [PATCH 07/11] Use rake db:seed instead of portus:create when provisioning Vagrant and Docker --- Vagrantfile | 2 +- compose-setup.sh | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Vagrantfile b/Vagrantfile index 9ed63c73e..6574b88cb 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -81,7 +81,7 @@ bundle config build.nokogiri --use-system-libraries bundle install --retry=3 bundle exec rake db:create bundle exec rake db:migrate -bundle exec rake portus:create +bundle exec rake db:seed sudo gem install passenger -v 5.0.7 passenger-install-apache2-module.ruby2.1 -a diff --git a/compose-setup.sh b/compose-setup.sh index 2e077c341..b0f8397c9 100755 --- a/compose-setup.sh +++ b/compose-setup.sh @@ -8,7 +8,7 @@ setup_database() { TIMEOUT=90 COUNT=0 printf "Portus: configuring database..." - docker-compose run --rm web rake db:create db:migrate portus:create &> /dev/null + docker-compose run --rm web rake db:create db:migrate db:seed &> /dev/null while [ $? -ne 0 ]; do printf " [FAIL]\n" @@ -22,7 +22,7 @@ setup_database() { COUNT=$((COUNT+5)) printf "Portus: configuring database..." - docker-compose run --rm web rake db:create db:migrate portus:create &> /dev/null + docker-compose run --rm web rake db:create db:migrate db:seed &> /dev/null done printf " [SUCCESS]\n" set -e From 195f55ad3c09f885299588afbf46504080c87257 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miquel=20Sabat=C3=A9?= Date: Tue, 11 Aug 2015 13:47:48 +0200 Subject: [PATCH 08/11] Moved code from the namespace & registry auth_scope's to a common base class --- app/controllers/api/base_controller.rb | 4 +- app/controllers/api/v2/tokens_controller.rb | 10 ++-- app/models/namespace/auth_scope.rb | 33 ++++--------- app/models/registry/auth_scope.rb | 30 ++---------- config/application.rb | 1 + lib/portus/auth_scope.rb | 52 +++++++++++++++++++++ spec/classes/registry_client_spec.rb | 4 +- spec/models/registry/auth_scope_spec.rb | 6 +-- 8 files changed, 76 insertions(+), 64 deletions(-) create mode 100644 lib/portus/auth_scope.rb diff --git a/app/controllers/api/base_controller.rb b/app/controllers/api/base_controller.rb index 72ca6776a..056d97c29 100644 --- a/app/controllers/api/base_controller.rb +++ b/app/controllers/api/base_controller.rb @@ -1,17 +1,15 @@ class Api::BaseController < ActionController::Base class ScopeNotHandled < StandardError; end class RegistryNotHandled < StandardError; end - class WrongPortusOTP < StandardError; end include Pundit respond_to :json - rescue_from Namespace::AuthScope::ResourceIsNotFound, with: :deny_access rescue_from Pundit::NotAuthorizedError, with: :deny_access rescue_from ScopeNotHandled, with: :deny_access rescue_from RegistryNotHandled, with: :deny_access - rescue_from WrongPortusOTP, with: :deny_access + rescue_from Portus::AuthScope::ResourceNotFound, with: :deny_access protected diff --git a/app/controllers/api/v2/tokens_controller.rb b/app/controllers/api/v2/tokens_controller.rb index 37c9da95c..fea879380 100644 --- a/app/controllers/api/v2/tokens_controller.rb +++ b/app/controllers/api/v2/tokens_controller.rb @@ -31,6 +31,7 @@ def authorize_scopes(registry) return unless params[:scope] auth_scope, scopes = scope_handler(registry, params[:scope]) + raise Pundit::NotAuthorizedError, "No scopes to handle" if scopes.empty? scopes.each do |scope| begin @@ -47,20 +48,19 @@ def authorize_scopes(registry) # From the given scope string, try to fetch a scope handler class for it. # Scope handlers are defined in "app/models/*/auth_scope.rb" files. def scope_handler(registry, scope_string) - type = scope_string.split(":", 3)[0] + str = scope_string.split(":", 3) + raise ScopeNotHandled, "Wrong format for scope string" if str.length != 3 - case type + case str[0] when "repository" auth_scope = Namespace::AuthScope.new(registry, scope_string) - scopes = scope_string.split(":", 3)[2].split(",") when "registry" auth_scope = Registry::AuthScope.new(registry, scope_string) - scopes = auth_scope.scopes else logger.error "Scope not handled: #{type}" raise ScopeNotHandled end - [auth_scope, scopes] + [auth_scope, auth_scope.scopes] end end diff --git a/app/models/namespace/auth_scope.rb b/app/models/namespace/auth_scope.rb index 1e0b1cfa2..3c2c82b4b 100644 --- a/app/models/namespace/auth_scope.rb +++ b/app/models/namespace/auth_scope.rb @@ -1,14 +1,7 @@ -class Namespace::AuthScope - class ResourceIsNotFound < StandardError; end - +# Namespace::AuthScope parses the scope string for the "namespace" type. +class Namespace::AuthScope < Portus::AuthScope attr_accessor :resource, :actions, :resource_type, :resource_name - def initialize(registry, scope_string) - @scope_string = scope_string - @registry = registry - parse_scope_string! - end - def resource if @namespace_name.blank? found_resource = @registry.namespaces.find_by(global: true) @@ -17,26 +10,18 @@ def resource end if found_resource.nil? - Rails.logger.warn "Namespace::AuthScope - Cannot find namespace with name #{@namespace_name}" - raise ResourceIsNotFound + Rails.logger.warn "Cannot find namespace with name #{@namespace_name}" + raise ResourceNotFound end found_resource end - private + protected + # Re-implemented from Portus::AuthScope to deal with the name of the + # namespace. def parse_scope_string! - @resource_type = @scope_string.split(":")[0] - @resource_name = @scope_string.split(":")[1] - @namespace_name = requested_resource_namespace_name - @actions = requested_actions - end - - def requested_resource_namespace_name - @resource_name.split("/").first if @resource_name.include?("/") - end - - def requested_actions - @scope_string.split(":")[2].split(",") + super + @namespace_name = @resource_name.split("/").first end end diff --git a/app/models/registry/auth_scope.rb b/app/models/registry/auth_scope.rb index 11f35fe49..2a0e68e91 100644 --- a/app/models/registry/auth_scope.rb +++ b/app/models/registry/auth_scope.rb @@ -1,32 +1,17 @@ # Registry::AuthScope parses the scope string so it can be used afterwards for # the "registry" type. -class Registry::AuthScope - # The given Registry was not found. - class ResourceIsNotFound < StandardError; end - - attr_accessor :resource, :actions, :resource_type, :resource_name - - def initialize(registry, scope_string) - @scope_string = scope_string - @registry = registry - parse_scope_string! - end - - # Returns the registry required by this scope. +class Registry::AuthScope < Portus::AuthScope def resource reg = Registry.find_by(hostname: @registry.hostname) if reg.nil? Rails.logger.warn "Could not find registry #{@registry.hostname}" - raise ResourceIsNotFound + raise ResourceNotFound end reg end - # Returns an array containing the scopes available for this registry object. - # It returns a ["*"] if it's an invalid scope, because this will not be - # implemented as a method. def scopes - catalog? ? ["all"] : ["*"] + catalog? ? ["all"] : [] end private @@ -36,13 +21,4 @@ def scopes def catalog? @resource_name == "catalog" && @actions[0] == "*" end - - # Parses the @scope_string variable into the needed attributes. - def parse_scope_string! - parts = @scope_string.split(":", 3) - @resource_type = parts[0] - @resource_name = parts[1] - @actions = parts[2] - @actions = @actions.split(",") if @actions - end end diff --git a/config/application.rb b/config/application.rb index 22a3083f7..dd22493b5 100644 --- a/config/application.rb +++ b/config/application.rb @@ -23,5 +23,6 @@ class Application < Rails::Application # Do not swallow errors in after_commit/after_rollback callbacks. config.active_record.raise_in_transactional_callbacks = true config.active_record.observers = :user_observer, :registry_observer + config.autoload_paths << Rails.root.join("lib") end end diff --git a/lib/portus/auth_scope.rb b/lib/portus/auth_scope.rb new file mode 100644 index 000000000..5b50a0f0d --- /dev/null +++ b/lib/portus/auth_scope.rb @@ -0,0 +1,52 @@ +module Portus + # This is the base class for AuthScope classes. An AuthScope class retrieves + # the information that can be extracted from a Docker scope string for the + # represented resource. + # + # An AuthScope class is required at least provide an implementation for the + # `resource` method. + class AuthScope + # The given resource was not found. + class ResourceNotFound < StandardError; end + + attr_accessor :resource, :actions, :resource_type, :resource_name + + def initialize(registry, scope_string) + @scope_string = scope_string + @registry = registry + parse_scope_string! + end + + # Returns the resource matching the requirements of an initialized + # AuthScope. It raises the Portus::AuthScope::ResourceNotFound exception if + # the required resource could not be found. + def resource + # Implemented by subclasses. + end + + # Returns an array containing the scopes available for this scope. The + # items on this array must match the names of a boolean method inside this + # resource's policy. For example, if we have a subclass Klass::AuthScope + # and this method returns ["pull", "push"], then the corresponding policy + # class KlassPolicy must define the boolean methods "pull?" and "push?". + # + # By default this method will return all the actions defined in the last + # element of the scope string. + # + # On error, this method should return an empty array. + def scopes + @actions + end + + protected + + # Parses the @scope_string variable into the needed attributes. + def parse_scope_string! + parts = @scope_string.split(":", 3) + @resource_type = parts[0] + @resource_name = parts[1] + @actions = parts[2] + @actions = @actions.split(",") if @actions + end + end +end diff --git a/spec/classes/registry_client_spec.rb b/spec/classes/registry_client_spec.rb index ff288aa34..ebefc23ed 100644 --- a/spec/classes/registry_client_spec.rb +++ b/spec/classes/registry_client_spec.rb @@ -1,6 +1,6 @@ require "rails_helper" -describe RegistryClient do +describe RegistryClient, focus: true do let(:registry_server) { "registry.test.lan" } let(:username) { "flavio" } let(:password) { "this is a test" } @@ -186,7 +186,7 @@ end context "fetching Catalog from registry" do - it "returns the available catalog", focus: true do + it "returns the available catalog" do create(:registry) create(:admin, username: "portus") diff --git a/spec/models/registry/auth_scope_spec.rb b/spec/models/registry/auth_scope_spec.rb index 2500a547f..6eeb68933 100644 --- a/spec/models/registry/auth_scope_spec.rb +++ b/spec/models/registry/auth_scope_spec.rb @@ -7,7 +7,7 @@ scope = Registry::AuthScope.new(reg, "scope:string:lala") expect do scope.resource - end.to raise_error(Registry::AuthScope::ResourceIsNotFound) + end.to raise_error(Registry::AuthScope::ResourceNotFound) end it "returns the current registry" do @@ -25,7 +25,7 @@ it "returns an invalid scope for incomplete scope strings" do ["scope", "scope:lala"].each do |sc| scope = Registry::AuthScope.new(registry, sc) - expect(scope.scopes).to match_array(["*"]) + expect(scope.scopes).to match_array([]) end end @@ -33,7 +33,7 @@ # The resource type is guaranteed to equal "registry" by the caller. ["registry:cata:aa", "registry:catalog:lala", "registry:lala:*"].each do |sc| scope = Registry::AuthScope.new(registry, sc) - expect(scope.scopes).to match_array(["*"]) + expect(scope.scopes).to match_array([]) end end From 68e8c51e90b3e4e623b843bdd9d7fc6839d38ee2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miquel=20Sabat=C3=A9?= Date: Tue, 11 Aug 2015 13:52:35 +0200 Subject: [PATCH 09/11] Cleaning up --- Gemfile | 1 - Gemfile.lock | 5 +++-- app/models/registry_client.rb | 3 ++- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Gemfile b/Gemfile index c78da1b0e..ec2c9ae09 100644 --- a/Gemfile +++ b/Gemfile @@ -19,7 +19,6 @@ gem "gravatar_image_tag" gem "rails-observers" gem "public_activity" gem "active_record_union" -gem "rotp" gem "mysql2" gem "search_cop" diff --git a/Gemfile.lock b/Gemfile.lock index 4b27a5a4a..8c86191a8 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -208,7 +208,6 @@ GEM rake (10.4.2) responders (2.1.0) railties (>= 4.2.0, < 5) - rotp (2.1.1) rspec-core (3.3.0) rspec-support (~> 3.3.0) rspec-expectations (3.3.0) @@ -344,7 +343,6 @@ DEPENDENCIES rack-mini-profiler rails (~> 4.2.2) rails-observers - rotp rspec-rails rubocop sass-rails (>= 3.2) @@ -362,3 +360,6 @@ DEPENDENCIES webmock wirb wirble + +BUNDLED WITH + 1.10.5 diff --git a/app/models/registry_client.rb b/app/models/registry_client.rb index b31d23daf..a0690c6ef 100644 --- a/app/models/registry_client.rb +++ b/app/models/registry_client.rb @@ -56,7 +56,8 @@ def catalog elsif res.code.to_i == 404 raise NotFoundError, "Could not find the catalog endpoint!" else - raise "Something went wrong while fetching the catalog" + raise "Something went wrong while fetching the catalog " \ + "Response: [#{res.code}] - #{res.body}" end end From 580876d6a11851a4be90ae15eec4a594054a9cf6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miquel=20Sabat=C3=A9?= Date: Tue, 11 Aug 2015 14:03:14 +0200 Subject: [PATCH 10/11] Allow all admin users to access the Catalog API --- app/policies/registry_policy.rb | 4 ++-- spec/classes/registry_client_spec.rb | 2 +- spec/policies/registry_policy_spec.rb | 18 ++++++++++++++++++ 3 files changed, 21 insertions(+), 3 deletions(-) create mode 100644 spec/policies/registry_policy_spec.rb diff --git a/app/policies/registry_policy.rb b/app/policies/registry_policy.rb index b2afcde63..1f865076d 100644 --- a/app/policies/registry_policy.rb +++ b/app/policies/registry_policy.rb @@ -9,9 +9,9 @@ def initialize(user, _) end # This method defines the permissions for the following scope string - # "registry:catalog:*". Only the "portus" special user is allowed to perform + # "registry:catalog:*". Only the admin users are allowed to perform # this call. def all? - user.username == "portus" + @user.admin? end end diff --git a/spec/classes/registry_client_spec.rb b/spec/classes/registry_client_spec.rb index ebefc23ed..d07d96eb3 100644 --- a/spec/classes/registry_client_spec.rb +++ b/spec/classes/registry_client_spec.rb @@ -1,6 +1,6 @@ require "rails_helper" -describe RegistryClient, focus: true do +describe RegistryClient do let(:registry_server) { "registry.test.lan" } let(:username) { "flavio" } let(:password) { "this is a test" } diff --git a/spec/policies/registry_policy_spec.rb b/spec/policies/registry_policy_spec.rb new file mode 100644 index 000000000..60046457d --- /dev/null +++ b/spec/policies/registry_policy_spec.rb @@ -0,0 +1,18 @@ +require "rails_helper" + +describe RegistryPolicy do + subject { described_class } + + let(:user) { create(:user) } + let(:admin) { create(:admin) } + + permissions :all? do + it "allows access to admin users" do + expect(subject).to permit(admin, nil) + end + + it "does not allow access to regular users" do + expect(subject).to_not permit(user, nil) + end + end +end From b3ee18f367a9e70a018570a44a700d5b1a864983 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miquel=20Sabat=C3=A9?= Date: Tue, 11 Aug 2015 15:02:59 +0200 Subject: [PATCH 11/11] spec: added tests for the namespace auth_scope class --- app/controllers/api/v2/tokens_controller.rb | 2 +- app/models/namespace/auth_scope.rb | 2 ++ spec/api/v2/token_spec.rb | 9 +++++++ spec/models/namespace/auth_scope_spec.rb | 26 +++++++++++++++++++++ 4 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 spec/models/namespace/auth_scope_spec.rb diff --git a/app/controllers/api/v2/tokens_controller.rb b/app/controllers/api/v2/tokens_controller.rb index fea879380..0cd7f979d 100644 --- a/app/controllers/api/v2/tokens_controller.rb +++ b/app/controllers/api/v2/tokens_controller.rb @@ -57,7 +57,7 @@ def scope_handler(registry, scope_string) when "registry" auth_scope = Registry::AuthScope.new(registry, scope_string) else - logger.error "Scope not handled: #{type}" + logger.error "Scope not handled: #{str[0]}" raise ScopeNotHandled end diff --git a/app/models/namespace/auth_scope.rb b/app/models/namespace/auth_scope.rb index 3c2c82b4b..f8838ca91 100644 --- a/app/models/namespace/auth_scope.rb +++ b/app/models/namespace/auth_scope.rb @@ -22,6 +22,8 @@ def resource # namespace. def parse_scope_string! super + + return unless @resource_name.include?("/") @namespace_name = @resource_name.split("/").first end end diff --git a/spec/api/v2/token_spec.rb b/spec/api/v2/token_spec.rb index 34f52d281..0e9345ac9 100644 --- a/spec/api/v2/token_spec.rb +++ b/spec/api/v2/token_spec.rb @@ -176,6 +176,15 @@ end end + context "unknown type" do + it "denies access" do + get v2_token_url, + { service: registry.hostname, account: user.username, scope: "lala:busybox:fork" }, + valid_auth_header + expect(response.status).to eq 401 + end + end + context "unkwnow registry" do context "no scope requested" do it "respond with 401" do diff --git a/spec/models/namespace/auth_scope_spec.rb b/spec/models/namespace/auth_scope_spec.rb new file mode 100644 index 000000000..377a2311b --- /dev/null +++ b/spec/models/namespace/auth_scope_spec.rb @@ -0,0 +1,26 @@ +require "rails_helper" + +describe Namespace::AuthScope, type: :model do + let!(:registry) { create(:registry) } + + it "handles the global namespace" do + nm = Namespace.first + scope = Namespace::AuthScope.new(registry, "registry:busybox:pull") + expect(scope.resource.id).to eq nm.id + expect(scope.scopes).to match_array(["pull"]) + end + + it "handles user namespaces" do + nm = create(:namespace, name: "mssola", registry: registry) + scope = Namespace::AuthScope.new(registry, "registry:mssola/busybox:pull") + expect(scope.resource.id).to eq nm.id + expect(scope.scopes).to match_array(["pull"]) + end + + it "raises an exception when it's not found" do + scope = Namespace::AuthScope.new(registry, "registry:mssola/busybox:pull") + expect do + scope.resource + end.to raise_error(Portus::AuthScope::ResourceNotFound) + end +end