From dc66e3c19be81ba1a731156bba20c3750e3de559 Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Wed, 18 Sep 2019 15:33:55 +0200 Subject: [PATCH 01/13] api/internal: delete User endpoints --- app/api/api.rb | 1 - app/api/internal/services.rb | 32 ++++++++----------- app/api/internal/users.rb | 60 ------------------------------------ 3 files changed, 12 insertions(+), 81 deletions(-) delete mode 100644 app/api/internal/users.rb diff --git a/app/api/api.rb b/app/api/api.rb index eb4f79245..20108679a 100644 --- a/app/api/api.rb +++ b/app/api/api.rb @@ -3,7 +3,6 @@ require_relative 'internal/applications' require_relative 'internal/metrics' require_relative 'internal/usagelimits' -require_relative 'internal/users' require_relative 'internal/events' require_relative 'internal/alert_limits' require_relative 'internal/application_keys' diff --git a/app/api/internal/services.rb b/app/api/internal/services.rb index 99c5f23b1..33f2e18b8 100644 --- a/app/api/internal/services.rb +++ b/app/api/internal/services.rb @@ -14,31 +14,23 @@ module API end post '/' do - begin - svc_attrs = api_params Service - service = Service.save!(svc_attrs) - [201, headers, {service: service.to_hash, status: :created}.to_json] - rescue ServiceRequiresDefaultUserPlan => e - respond_with_400 e - end + svc_attrs = api_params Service + service = Service.save!(svc_attrs) + [201, headers, {service: service.to_hash, status: :created}.to_json] end put '/:id' do - begin - svc_attrs = api_params Service - service = Service.load_by_id(params[:id]) - if service - svc_attrs.each do |attr, value| - service.send "#{attr}=", value - end - service.save! - else - service = Service.save!(svc_attrs) + svc_attrs = api_params Service + service = Service.load_by_id(params[:id]) + if service + svc_attrs.each do |attr, value| + service.send "#{attr}=", value end - {service: service.to_hash, status: :ok}.to_json - rescue ServiceRequiresDefaultUserPlan => e - respond_with_400 e + service.save! + else + service = Service.save!(svc_attrs) end + {service: service.to_hash, status: :ok}.to_json end put '/change_provider_key/:key' do diff --git a/app/api/internal/users.rb b/app/api/internal/users.rb deleted file mode 100644 index c765bdf11..000000000 --- a/app/api/internal/users.rb +++ /dev/null @@ -1,60 +0,0 @@ -module ThreeScale - module Backend - module API - internal_api '/services/:service_id/users' do - module UserHelper - def self.save(service_id, username, usr_attrs, method, headers) - usr_attrs[:service_id] = service_id - usr_attrs[:username] = username - begin - user = User.save! usr_attrs - rescue => e - [400, headers, { status: :error, error: e.message }.to_json] - else - post = method == :post - [post ? 201 : 200, headers, { status: post ? :created : :modified, user: user.to_hash }.to_json] - end - end - end - - get '/:username' do |service_id, username| - user = User.load(service_id, username) - if user - { status: :found, user: user.to_hash }.to_json - else - [404, headers, { status: :not_found, error: 'user not found' }.to_json] - end - end - - post '/:username' do |service_id, username| - user_attrs = api_params User - UserHelper.save(service_id, username, user_attrs, :post, headers) - end - - put '/:username' do |service_id, username| - user_attrs = api_params User - UserHelper.save(service_id, username, user_attrs, :put, headers) - end - - delete '/:username' do |service_id, username| - begin - User.delete! service_id, username - { status: :deleted }.to_json - rescue => e - [400, headers, { status: :error, error: e.message }.to_json] - end - end - - delete '' do |service_id| - begin - User.delete_all(service_id) - { status: :deleted}.to_json - rescue => e - [400, headers, { status: :error, error: e.message }.to_json] - end - end - - end - end - end -end From a1faf0d7cd37f2cd7de8d3f7b60a9504930f419a Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Wed, 18 Sep 2019 16:36:26 +0200 Subject: [PATCH 02/13] users: remove support for end-users --- docs/rfcs/error_responses.md | 2 - lib/3scale/backend.rb | 1 - lib/3scale/backend/alerts.rb | 5 +- lib/3scale/backend/errors.rb | 57 ----- lib/3scale/backend/listener.rb | 18 -- lib/3scale/backend/service.rb | 10 - lib/3scale/backend/stats/aggregator.rb | 2 +- lib/3scale/backend/stats/delete_job_def.rb | 9 +- lib/3scale/backend/stats/key_generator.rb | 29 +-- lib/3scale/backend/stats/keys.rb | 33 +-- .../backend/stats/partition_eraser_job.rb | 3 +- .../backend/stats/partition_generator_job.rb | 5 +- lib/3scale/backend/transaction.rb | 2 +- lib/3scale/backend/transactor.rb | 48 +---- lib/3scale/backend/transactor/report_job.rb | 17 -- lib/3scale/backend/transactor/status.rb | 70 ++---- lib/3scale/backend/transactor/usage_report.rb | 12 +- lib/3scale/backend/usage.rb | 7 - .../service_user_management_use_case.rb | 30 --- lib/3scale/backend/user.rb | 201 ------------------ lib/3scale/backend/validators/limits.rb | 14 +- 21 files changed, 41 insertions(+), 534 deletions(-) delete mode 100644 lib/3scale/backend/use_cases/service_user_management_use_case.rb delete mode 100644 lib/3scale/backend/user.rb diff --git a/docs/rfcs/error_responses.md b/docs/rfcs/error_responses.md index 45a2aab07..2bc4b9f60 100644 --- a/docs/rfcs/error_responses.md +++ b/docs/rfcs/error_responses.md @@ -190,8 +190,6 @@ ie. parameter X is missing or not matching criteria Y. One or more required parameters are missing. - `usage_value_invalid`: One or more usage values for a given metric are missing or invalid. - - `user_not_defined`: - The application requires a user_id parameter but it is missing or invalid. - `service_id_missing`: The service ID is missing or blank. diff --git a/lib/3scale/backend.rb b/lib/3scale/backend.rb index 4ddf25c11..2f9392ddf 100644 --- a/lib/3scale/backend.rb +++ b/lib/3scale/backend.rb @@ -47,7 +47,6 @@ require '3scale/backend/errors' require '3scale/backend/stats' require '3scale/backend/usage_limit' -require '3scale/backend/user' require '3scale/backend/alerts' require '3scale/backend/event_storage' require '3scale/backend/worker' diff --git a/lib/3scale/backend/alerts.rb b/lib/3scale/backend/alerts.rb index 4bb8ab818..04b8104a7 100644 --- a/lib/3scale/backend/alerts.rb +++ b/lib/3scale/backend/alerts.rb @@ -50,7 +50,7 @@ def key_current_id FIRST_ALERT_BIN = ALERT_BINS.first RALERT_BINS = ALERT_BINS.reverse.freeze - def utilization(app_usage_reports, user_usage_reports) + def utilization(app_usage_reports) max_utilization = -1.0 max_record = nil max = proc do |item| @@ -65,12 +65,11 @@ def utilization(app_usage_reports, user_usage_reports) end app_usage_reports.each(&max) - user_usage_reports.each(&max) if max_utilization == -1 ## case that all the limits have max_value==0 max_utilization = 0 - max_record = app_usage_reports.first || user_usage_reports.first + max_record = app_usage_reports.first end [max_utilization, max_record] diff --git a/lib/3scale/backend/errors.rb b/lib/3scale/backend/errors.rb index 1bf86b021..192eb3753 100644 --- a/lib/3scale/backend/errors.rb +++ b/lib/3scale/backend/errors.rb @@ -201,13 +201,6 @@ def initialize(metric_name, value) class UnsupportedApiVersion < Error end - # new errors for the user limiting - class UserNotDefined < Error - def initialize(id) - super %(application with id="#{id}" requires a user (user_id)) - end - end - class TransactionTimestampNotWithinRange < Error end @@ -223,68 +216,18 @@ def initialize(max_seconds) end end - class UserRequiresRegistration < Error - def initialize(service_id, user_id) - super %(user with user_id="#{user_id}" requires registration to use service with id="#{service_id}") - end - end - - class ServiceCannotUseUserId < Error - def initialize(service_id) - super %(service with service_id="#{service_id}" does not have access to end user plans, user_id is not allowed) - end - end - class ServiceLoadInconsistency < Error def initialize(service_id, other_service_id) super %(service.load_by_id with id="#{service_id}" loaded the service with id="#{other_service_id}") end end - # FIXME: this has to be about the only service-related error without - # a service id in the reported message. - class ServiceRequiresDefaultUserPlan < Error - def initialize - super 'Services without the need for registered users require a default user plan'.freeze - end - end - class ServiceIsDefaultService < Error def initialize(id = nil) super %(Service id="#{id}" is the default service, cannot be removed) end end - class ServiceRequiresRegisteredUser < Error - def initialize(id = nil) - super %(Service id="#{id}" requires users to be registered beforehand) - end - end - - class UserRequiresUsername < Error - def initialize - super 'User requires username'.freeze - end - end - - class UserRequiresServiceId < Error - def initialize - super 'User requires a service ID'.freeze - end - end - - class UserRequiresValidService < Error - def initialize - super 'User requires a valid service, the service does not exist'.freeze - end - end - - class UserRequiresDefinedPlan < Error - def initialize - super 'User requires a defined plan'.freeze - end - end - class InvalidProviderKeys < Error def initialize super 'Provider keys are not valid, must be not nil and different'.freeze diff --git a/lib/3scale/backend/listener.rb b/lib/3scale/backend/listener.rb index 8ea60d50f..290d24944 100644 --- a/lib/3scale/backend/listener.rb +++ b/lib/3scale/backend/listener.rb @@ -46,15 +46,6 @@ class Listener < Sinatra::Base ##~ @parameter_user_key_inline = @parameter_user_key.clone ##~ @parameter_user_key_inline["description_inline"] = true - ##~ @parameter_user_id = {"name" => "user_id", "dataType" => "string", "paramType" => "query"} - ##~ @parameter_user_id["description"] = "User id. String identifying an end user. Required only when the application is rate limiting end users. The End User plans feature is not available in all 3scale plans." - ##~ @parameter_user_id_inline = @parameter_user_id.clone - ##~ @parameter_user_id_inline["description_inline"] = true - ## - ##~ @parameter_user_id_oauth = {"name" => "user_id", "dataType" => "string", "paramType" => "query"} - ##~ @parameter_user_id_oauth["description"] = "User id. String identifying an end user. Used only when the application is rate limiting end users and the specified token is not associated to a user. The End User plans feature is not available in all 3scale plans." - ## - ##~ @parameter_referrer = {"name" => "referrer", "dataType" => "string", "required" => false, "paramType" => "query"} ##~ @parameter_referrer["description"] = "Referrer IP Address or Domain. Required only if referrer filtering is enabled. If special value '*' (wildcard) is passed, the referrer check is bypassed." ## @@ -99,7 +90,6 @@ class Listener < Sinatra::Base ##~ @parameter_transaction_app_id["parameters"] = [] ## ##~ @parameter_transaction_app_id["parameters"] << @parameter_app_id_inline - ##~ @parameter_transaction_app_id["parameters"] << @parameter_user_id_inline ##~ @parameter_transaction_app_id["parameters"] << @timestamp ##~ @parameter_transaction_app_id["parameters"] << @parameter_usage ##~ @parameter_transaction_app_id["parameters"] << @parameter_log @@ -109,7 +99,6 @@ class Listener < Sinatra::Base ##~ @parameter_transaction_api_key["parameters"] = [] ##~ @parameter_transaction_api_key["parameters"] << @parameter_user_key_inline - ##~ @parameter_transaction_api_key["parameters"] << @parameter_user_id_inline ##~ @parameter_transaction_api_key["parameters"] << @timestamp ##~ @parameter_transaction_api_key["parameters"] << @parameter_usage ##~ @parameter_transaction_api_key["parameters"] << @parameter_log @@ -119,7 +108,6 @@ class Listener < Sinatra::Base ##~ @parameter_transaction_oauth["parameters"] = [] ##~ @parameter_transaction_oauth["parameters"] << @parameter_client_id_inline - ##~ @parameter_transaction_oauth["parameters"] << @parameter_user_id_inline ##~ @parameter_transaction_oauth["parameters"] << @timestamp ##~ @parameter_transaction_oauth["parameters"] << @parameter_usage ##~ @parameter_transaction_oauth["parameters"] << @parameter_log @@ -228,7 +216,6 @@ def do_api_method(method_name) ##~ op.parameters.add @parameter_app_id ##~ op.parameters.add @parameter_app_key ##~ op.parameters.add @parameter_referrer - ##~ op.parameters.add @parameter_user_id ##~ op.parameters.add @parameter_usage_predicted ## ##~ a = sapi.apis.add @@ -244,7 +231,6 @@ def do_api_method(method_name) ##~ op.parameters.add @parameter_service_id ##~ op.parameters.add @parameter_user_key ##~ op.parameters.add @parameter_referrer - ##~ op.parameters.add @parameter_user_id ##~ op.parameters.add @parameter_usage_predicted ## get '/transactions/authorize.xml' do @@ -276,7 +262,6 @@ def do_api_method(method_name) ##~ op.parameters.add @parameter_client_id ##~ op.parameters.add @parameter_app_key_oauth ##~ op.parameters.add @parameter_referrer - ##~ op.parameters.add @parameter_user_id_oauth ##~ op.parameters.add @parameter_usage_predicted ##~ op.parameters.add @parameter_redirect_url ##~ op.parameters.add @parameter_redirect_uri @@ -309,7 +294,6 @@ def do_api_method(method_name) ##~ op.parameters.add @parameter_app_id ##~ op.parameters.add @parameter_app_key ##~ op.parameters.add @parameter_referrer - ##~ op.parameters.add @parameter_user_id ##~ op.parameters.add @parameter_usage ##~ op.parameters.add @parameter_log ## @@ -326,7 +310,6 @@ def do_api_method(method_name) ##~ op.parameters.add @parameter_service_id ##~ op.parameters.add @parameter_user_key ##~ op.parameters.add @parameter_referrer - ##~ op.parameters.add @parameter_user_id ##~ op.parameters.add @parameter_usage ##~ op.parameters.add @parameter_log ## @@ -353,7 +336,6 @@ def do_api_method(method_name) ##~ op.parameters.add @parameter_client_id ##~ op.parameters.add @parameter_app_key_oauth ##~ op.parameters.add @parameter_referrer - ##~ op.parameters.add @parameter_user_id_oauth ##~ op.parameters.add @parameter_usage ##~ op.parameters.add @parameter_log ##~ op.parameters.add @parameter_redirect_url diff --git a/lib/3scale/backend/service.rb b/lib/3scale/backend/service.rb index 153d90c1d..7758facd5 100644 --- a/lib/3scale/backend/service.rb +++ b/lib/3scale/backend/service.rb @@ -200,7 +200,6 @@ def user_registration_required? end def save! - validate_user_registration_required set_as_default_if_needed persist clear_cache @@ -272,15 +271,6 @@ def storage_key_by_provider(attribute) self.class.storage_key_by_provider provider_key, attribute end - def validate_user_registration_required - @user_registration_required = true if @user_registration_required.nil? - - if !user_registration_required? && - (default_user_plan_id.nil? || default_user_plan_name.nil?) - raise ServiceRequiresDefaultUserPlan - end - end - def set_as_default_if_needed if @default_service.nil? default_service_id = self.class.default_id(provider_key) diff --git a/lib/3scale/backend/stats/aggregator.rb b/lib/3scale/backend/stats/aggregator.rb index 057573d40..4c940bb5a 100644 --- a/lib/3scale/backend/stats/aggregator.rb +++ b/lib/3scale/backend/stats/aggregator.rb @@ -152,7 +152,7 @@ def update_alerts(applications) values: usage) max_utilization, max_record = Alerts.utilization( - status.application_usage_reports, status.user_usage_reports) + status.application_usage_reports) if max_utilization >= 0.0 Alerts.update_utilization(service_id, diff --git a/lib/3scale/backend/stats/delete_job_def.rb b/lib/3scale/backend/stats/delete_job_def.rb index 51a7ca692..e840da0fe 100644 --- a/lib/3scale/backend/stats/delete_job_def.rb +++ b/lib/3scale/backend/stats/delete_job_def.rb @@ -2,7 +2,7 @@ module ThreeScale module Backend module Stats class DeleteJobDef - ATTRIBUTES = %i[service_id applications metrics users from to context_info].freeze + ATTRIBUTES = %i[service_id applications metrics from to context_info].freeze private_constant :ATTRIBUTES attr_reader(*ATTRIBUTES) @@ -19,7 +19,7 @@ def initialize(params = {}) def run_async Resque.enqueue(PartitionGeneratorJob, Time.now.getutc.to_f, service_id, applications, - metrics, users, from, to, context_info) + metrics, from, to, context_info) end def to_json @@ -49,11 +49,6 @@ def validate raise_validation_error('metrics values') unless metrics.all? do |x| x.is_a?(String) || x.is_a?(Integer) end - # users is array - raise_validation_error('users field') unless users.is_a? Array - raise_validation_error('users values') unless users.all? do |x| - x.is_a?(String) || x.is_a?(Integer) - end end def raise_validation_error(msg) diff --git a/lib/3scale/backend/stats/key_generator.rb b/lib/3scale/backend/stats/key_generator.rb index 3225d5cf6..f4c70486f 100644 --- a/lib/3scale/backend/stats/key_generator.rb +++ b/lib/3scale/backend/stats/key_generator.rb @@ -2,13 +2,12 @@ module ThreeScale module Backend module Stats class KeyGenerator - attr_reader :service_id, :applications, :metrics, :users, :from, :to + attr_reader :service_id, :applications, :metrics, :from, :to - def initialize(service_id:, applications: [], metrics: [], users: [], from:, to:, **) + def initialize(service_id:, applications: [], metrics: [], from:, to:, **) @service_id = service_id @applications = applications @metrics = metrics - @users = users @from = from @to = to end @@ -16,10 +15,8 @@ def initialize(service_id:, applications: [], metrics: [], users: [], from:, to: def keys response_code_service_keys + response_code_application_keys + - response_code_user_keys + usage_service_keys + - usage_application_keys + - usage_user_keys + usage_application_keys end private @@ -53,16 +50,6 @@ def response_code_application_keys end end - def response_code_user_keys - periods(PeriodCommons::PERMANENT_EXPANDED_GRANULARITIES).flat_map do |period| - response_codes.flat_map do |response_code| - users.flat_map do |user| - Keys.user_response_code_value_key(service_id, user, response_code, period) - end - end - end - end - def usage_service_keys periods(PeriodCommons::PERMANENT_SERVICE_GRANULARITIES).flat_map do |period| metrics.flat_map do |metric| @@ -80,16 +67,6 @@ def usage_application_keys end end end - - def usage_user_keys - periods(PeriodCommons::PERMANENT_EXPANDED_GRANULARITIES).flat_map do |period| - users.flat_map do |user| - metrics.flat_map do |metric| - Keys.user_usage_value_key(service_id, user, metric, period) - end - end - end - end end end end diff --git a/lib/3scale/backend/stats/keys.rb b/lib/3scale/backend/stats/keys.rb index 377138668..abf4edc83 100644 --- a/lib/3scale/backend/stats/keys.rb +++ b/lib/3scale/backend/stats/keys.rb @@ -22,12 +22,6 @@ def applications_key_prefix(prefix) "#{prefix}/cinstances" end - # @note For backwards compatibility, the key is called uinstance. - # It will be eventually renamed to user. - def user_key_prefix(prefix, user_id) - "#{prefix}/uinstance:#{user_id}" - end - def metric_key_prefix(prefix, metric_id) "#{prefix}/metric:#{metric_id}" end @@ -51,14 +45,6 @@ def application_usage_value_key(service_id, app_id, metric_id, period) encode_key(counter_key(metric_key, period)) end - def user_usage_value_key(service_id, user_id, metric_id, period) - service_key = service_key_prefix(service_id) - user_key = user_key_prefix(service_key, user_id) - metric_key = metric_key_prefix(user_key, metric_id) - - encode_key(counter_key(metric_key, period)) - end - def service_response_code_value_key(service_id, response_code, period) service_key = service_key_prefix(service_id) response_code_key = response_code_key_prefix(service_key, response_code) @@ -66,7 +52,7 @@ def service_response_code_value_key(service_id, response_code, period) encode_key(counter_key(response_code_key, period)) end - def application_response_code_value_key(service_id, app_id, response_code, period) + def application_response_code_value_key(service_id, app_id, response_code, period) service_key = service_key_prefix(service_id) app_key = application_key_prefix(service_key, app_id) response_code_key = response_code_key_prefix(app_key, response_code) @@ -74,14 +60,6 @@ def application_response_code_value_key(service_id, app_id, response_code, perio encode_key(counter_key(response_code_key, period)) end - def user_response_code_value_key(service_id, user_id, response_code, period) - service_key = service_key_prefix(service_id) - user_key = user_key_prefix(service_key, user_id) - response_code_key = response_code_key_prefix(user_key, response_code) - - encode_key(counter_key(response_code_key, period)) - end - def counter_key(prefix, period) granularity = period.granularity key = "#{prefix}/#{granularity}" @@ -114,17 +92,10 @@ def transaction_keys(transaction, item, value) method = "#{item}_key_prefix".to_sym - keys = { + { service: public_send(method, service_key, value), application: public_send(method, application_key, value), } - - if transaction.user_id - user_key = user_key_prefix(service_key, transaction.user_id) - keys[:user] = public_send(method, user_key, value) - end - - keys end end diff --git a/lib/3scale/backend/stats/partition_eraser_job.rb b/lib/3scale/backend/stats/partition_eraser_job.rb index 41fa64ca2..92eb0040c 100644 --- a/lib/3scale/backend/stats/partition_eraser_job.rb +++ b/lib/3scale/backend/stats/partition_eraser_job.rb @@ -11,13 +11,12 @@ class << self include StorageHelpers include Configurable - def perform_logged(_enqueue_time, service_id, applications, metrics, users, + def perform_logged(_enqueue_time, service_id, applications, metrics, from, to, offset, length, context_info = {}) job = DeleteJobDef.new( service_id: service_id, applications: applications, metrics: metrics, - users: users, from: from, to: to ) diff --git a/lib/3scale/backend/stats/partition_generator_job.rb b/lib/3scale/backend/stats/partition_generator_job.rb index 84e2b8c8f..4d39115cb 100644 --- a/lib/3scale/backend/stats/partition_generator_job.rb +++ b/lib/3scale/backend/stats/partition_generator_job.rb @@ -10,13 +10,12 @@ class PartitionGeneratorJob < BackgroundJob class << self include Configurable - def perform_logged(_enqueue_time, service_id, applications, metrics, users, + def perform_logged(_enqueue_time, service_id, applications, metrics, from, to, context_info = {}) job = DeleteJobDef.new( service_id: service_id, applications: applications, metrics: metrics, - users: users, from: from, to: to ) @@ -26,7 +25,7 @@ def perform_logged(_enqueue_time, service_id, applications, metrics, users, # Generate partitions 0.step(stats_key_gen.keys.count, configuration.stats.delete_partition_batch_size).each do |idx| Resque.enqueue(PartitionEraserJob, Time.now.getutc.to_f, service_id, applications, - metrics, users, from, to, idx, + metrics, from, to, idx, configuration.stats.delete_partition_batch_size, context_info) end diff --git a/lib/3scale/backend/transaction.rb b/lib/3scale/backend/transaction.rb index f8c89a85e..dfefbee92 100644 --- a/lib/3scale/backend/transaction.rb +++ b/lib/3scale/backend/transaction.rb @@ -13,7 +13,7 @@ class Transaction DEADLINE_RANGE = (-REPORT_DEADLINE_PAST..REPORT_DEADLINE_FUTURE).freeze private_constant :DEADLINE_RANGE - ATTRIBUTES = [:service_id, :application_id, :user_id, :timestamp, + ATTRIBUTES = [:service_id, :application_id, :timestamp, :log, :usage, :response_code] private_constant :ATTRIBUTES diff --git a/lib/3scale/backend/transactor.rb b/lib/3scale/backend/transactor.rb index 57b0cf003..12e0928f3 100644 --- a/lib/3scale/backend/transactor.rb +++ b/lib/3scale/backend/transactor.rb @@ -58,34 +58,23 @@ def validate(oauth, provider_key, report_usage, params, extensions) # service_id cannot be taken from params since it might be missing there service_id = service.id - app_id, user_id = params[:app_id], params[:user_id] + app_id = params[:app_id] # TODO: make sure params are nil if they are empty up the call stack # Note: app_key is an exception, as it being empty is semantically # significant. params[:app_id] = nil if app_id && app_id.empty? - params[:user_id] = nil if user_id && user_id.empty? - - # Now OAuth tokens also identify users, so must check tokens anyway if - # at least one of app or user ids is missing. - # - # NB: so what happens if we call an OAuth method with user_key=K and - # user_id=U? It is effectively as if app_id was given and no token would - # need to be checked, but we do... That is not consistent. And madness. - # Each time I try to understand this I feel I'm becoming dumber... - # + if oauth - if user_id.nil? || app_id.nil? + if app_id.nil? access_token = params[:access_token] access_token = nil if access_token && access_token.empty? if access_token.nil? raise ApplicationNotFound.new nil if app_id.nil? else - app_id, user_id = get_token_ids(access_token, service_id, - app_id, user_id) + app_id = get_token_ids(access_token, service_id, app_id) # update params, since they are checked elsewhere params[:app_id] = app_id - params[:user_id] = user_id end end @@ -98,14 +87,10 @@ def validate(oauth, provider_key, report_usage, params, extensions) application = Application.load_by_id_or_user_key!(service_id, app_id, params[:user_key]) - - user = load_user!(application, service, user_id) now = Time.now.getutc usage_values = Usage.application_usage(application, now) - user_usage = Usage.user_usage(user, now) if user status_attrs = { service_id: service_id, - user_values: user_usage, application: application, oauth: oauth, usage: params[:usage], @@ -114,12 +99,10 @@ def validate(oauth, provider_key, report_usage, params, extensions) # hierarchy parameter adds information in the response needed # to derive which limits affect directly or indirectly the # metrics for which authorization is requested. - hierarchy: extensions[:hierarchy] == '1', - user: user, + hierarchy: extensions[:hierarchy] == '1' } application.load_metric_names - user.load_metric_names if user # returns a status object apply_validators(validators, status_attrs, params) @@ -155,7 +138,7 @@ def do_authorize(method, provider_key, params, extensions) def do_authrep(method, provider_key, params, extensions) status = begin validate(method == :oauth_authrep, provider_key, true, params, extensions) - rescue ApplicationNotFound, UserNotDefined => e + rescue ApplicationNotFound => e # we still want to track these notify_authorize(provider_key) raise e @@ -165,8 +148,7 @@ def do_authrep(method, provider_key, params, extensions) if (usage || params[:log]) && status.authorized? application_id = status.application.id - username = status.user.username unless status.user.nil? - report_enqueue(status.service_id, ({ 0 => {"app_id" => application_id, "usage" => usage, "user_id" => username, "log" => params[:log]}}), {}) + report_enqueue(status.service_id, ({ 0 => {"app_id" => application_id, "usage" => usage, "log" => params[:log]}}), {}) notify_authrep(provider_key, usage ? 1 : 0) else notify_authorize(provider_key) @@ -175,22 +157,6 @@ def do_authrep(method, provider_key, params, extensions) status end - def load_user!(application, service, user_id) - user = nil - - if not (user_id.nil? || user_id.empty? || !user_id.is_a?(String)) - ## user_id on the paramters - if application.user_required? - user = User.load_or_create!(service, user_id) - raise UserRequiresRegistration, service.id, user_id unless user - end - else - raise UserNotDefined, application.id if application.user_required? - end - - user - end - # This method applies the validators in the given order. If there is one # that fails, it stops there instead of applying all of them. # Returns a Status instance. diff --git a/lib/3scale/backend/transactor/report_job.rb b/lib/3scale/backend/transactor/report_job.rb index 9151b086a..be3e37a32 100644 --- a/lib/3scale/backend/transactor/report_job.rb +++ b/lib/3scale/backend/transactor/report_job.rb @@ -38,7 +38,6 @@ def parse_transactions(service_id, raw_transactions) group_by_application_id(service_id, raw_transactions) do |app_id, group| group.each do |raw_transaction| - check_end_users_allowed(raw_transaction, service_id) transaction = compose_transaction(service_id, app_id, raw_transaction) log = raw_transaction['log'] @@ -60,21 +59,6 @@ def group_by_application_id(service_id, transactions, &block) end.each(&block) end - def check_end_users_allowed(raw_transaction, service_id = nil) - user_id = raw_transaction['user_id'] - - if service_id && user_id && !user_id.empty? && !end_users_allowed?(service_id) - raise ServiceCannotUseUserId.new(service_id) - end - end - - def end_users_allowed?(service_id) - service = Service.load_by_id(service_id) - !(service && - service.user_registration_required? && - service.default_user_plan_id.nil?) - end - def compose_transaction(service_id, app_id, raw_transaction) usage = raw_transaction['usage'] @@ -85,7 +69,6 @@ def compose_transaction(service_id, app_id, raw_transaction) application_id: app_id, timestamp: raw_transaction['timestamp'], usage: metrics.process_usage(usage), - user_id: raw_transaction['user_id'], } end end diff --git a/lib/3scale/backend/transactor/status.rb b/lib/3scale/backend/transactor/status.rb index 25b5e5e94..2afeb1fe9 100644 --- a/lib/3scale/backend/transactor/status.rb +++ b/lib/3scale/backend/transactor/status.rb @@ -16,13 +16,11 @@ def initialize(attributes) @usage = attributes[:usage] @predicted_usage = attributes[:predicted_usage] @values = filter_values(attributes[:values] || {}) - @user = attributes[:user] - @user_values = filter_values(attributes[:user_values]) @timestamp = attributes[:timestamp] || Time.now.getutc @hierarchy_ext = attributes[:hierarchy] raise 'service_id not specified' if @service_id.nil? - raise ':application is required' if @application.nil? && @user.nil? + raise ':application is required' if @application.nil? @redirect_uri_field = REDIRECT_URI_FIELD @authorized = true @@ -33,8 +31,6 @@ def initialize(attributes) attr_reader :oauth attr_accessor :redirect_uri_field attr_accessor :values - attr_reader :user - attr_accessor :user_values def reject!(error) @authorized = false @@ -70,24 +66,12 @@ def plan_name @application.plan_name unless @application.nil? end - def user_plan_name - @user.plan_name unless @user.nil? - end - def application_usage_reports - @usage_report ||= load_usage_reports @application, :application - end - - def user_usage_reports - @user_usage_report ||= load_usage_reports @user, :user + @usage_report ||= load_usage_reports @application end - def value_for_usage_limit(usage_limit, type = :application) - if type==:application - values = @values[usage_limit.period] - else - values = @user_values[usage_limit.period] - end + def value_for_usage_limit(usage_limit) + values = @values[usage_limit.period] values && values[usage_limit.metric_id] || 0 end @@ -96,11 +80,6 @@ def value_for_application_usage_limit(usage_limit) values && values[usage_limit.metric_id] || 0 end - def value_for_user_usage_limit(usage_limit) - values = @user_values[usage_limit.period] - values && values[usage_limit.metric_id] || 0 - end - # provides a hierarchy hash with metrics as symbolic names def hierarchy @hierarchy ||= Metric.hierarchy service_id @@ -120,7 +99,6 @@ def to_xml(options = {}) if oauth add_application_section(xml) - add_user_section(xml) end hierarchy_reports = [] if @hierarchy_ext @@ -129,11 +107,6 @@ def to_xml(options = {}) add_reports_section(xml, application_usage_reports) hierarchy_reports.concat application_usage_reports if hierarchy_reports end - if !@user.nil? - add_plan_section(xml, 'user_plan'.freeze, user_plan_name) - add_reports_section(xml, user_usage_reports, true) - hierarchy_reports.concat user_usage_reports if hierarchy_reports - end if hierarchy_reports add_hierarchy_section(xml, hierarchy_reports) @@ -144,13 +117,12 @@ def to_xml(options = {}) private - # Returns the app usage reports and user usage reports needed to - # construct the limit headers. If the status does not have a 'usage', - # this method returns all the usage reports. Otherwise, it returns the - # reports associated with the metrics present in the 'usage' and their - # parents. + # Returns the app usage reports needed to construct the limit headers. + # If the status does not have a 'usage', this method returns all the + # usage reports. Otherwise, it returns the reports associated with the + # metrics present in the 'usage' and their parents. def reports_to_calculate_limit_headers - all_reports = application_usage_reports + user_usage_reports + all_reports = application_usage_reports return all_reports if (@usage.nil? || @usage.empty?) @@ -220,13 +192,7 @@ def add_application_section(xml) '' end - def add_user_section(xml) - if !@user.nil? - xml << "".freeze << @user.username << "".freeze - end - end - - def load_usage_reports(what, type) + def load_usage_reports(application) # We might have usage limits that apply to metrics that no longer # exist. In that case, the usage limit refers to a metric ID that no # longer has a name associated to it. When that happens, we do not @@ -236,23 +202,19 @@ def load_usage_reports(what, type) # for some of the usage limits. reports = [] - if !what.nil? - what.usage_limits.each do |usage_limit| - if what.metric_name(usage_limit.metric_id) - reports << UsageReport.new(self, usage_limit, type) + if !application.nil? + application.usage_limits.each do |usage_limit| + if application.metric_name(usage_limit.metric_id) + reports << UsageReport.new(self, usage_limit) end end end reports end - def add_reports_section(xml, reports, report_type_user = false) + def add_reports_section(xml, reports) unless reports.empty? - xml_node = if report_type_user - 'user_usage_reports>'.freeze - else - 'usage_reports>'.freeze - end + xml_node = 'usage_reports>'.freeze xml << '<'.freeze xml << xml_node reports.each do |report| diff --git a/lib/3scale/backend/transactor/usage_report.rb b/lib/3scale/backend/transactor/usage_report.rb index 8256c588c..c145a9512 100644 --- a/lib/3scale/backend/transactor/usage_report.rb +++ b/lib/3scale/backend/transactor/usage_report.rb @@ -5,20 +5,14 @@ class Status class UsageReport attr_reader :type, :period - def initialize(status, usage_limit, type) + def initialize(status, usage_limit) @status = status @usage_limit = usage_limit - @type = type @period = usage_limit.period.new(status.timestamp) end def metric_name - @metric_name ||= - if @type == :application - @status.application.metric_name(metric_id) - else - @status.user.metric_name(metric_id) - end + @metric_name ||= @status.application.metric_name(metric_id) end def metric_id @@ -30,7 +24,7 @@ def max_value end def current_value - @current_value ||= @status.value_for_usage_limit(@usage_limit, @type) + @current_value ||= @status.value_for_usage_limit(@usage_limit) end # Returns -1 if the period is eternity. Otherwise, returns the time diff --git a/lib/3scale/backend/usage.rb b/lib/3scale/backend/usage.rb index 92133d550..6f6bcdf7c 100644 --- a/lib/3scale/backend/usage.rb +++ b/lib/3scale/backend/usage.rb @@ -2,13 +2,6 @@ module ThreeScale module Backend class Usage class << self - def user_usage(user, timestamp) - usage(user, timestamp) do |metric_id, instance_period| - Stats::Keys.user_usage_value_key( - user.service_id, user.username, metric_id, instance_period) - end - end - def application_usage(application, timestamp) usage(application, timestamp) do |metric_id, instance_period| Stats::Keys.application_usage_value_key( diff --git a/lib/3scale/backend/use_cases/service_user_management_use_case.rb b/lib/3scale/backend/use_cases/service_user_management_use_case.rb deleted file mode 100644 index 3e228a9a9..000000000 --- a/lib/3scale/backend/use_cases/service_user_management_use_case.rb +++ /dev/null @@ -1,30 +0,0 @@ -module ThreeScale - module Backend - class ServiceUserManagementUseCase - - def initialize(service, username = nil) - @service = service - @username = username - end - - def add - storage.sadd(@service.storage_key("user_set"), @username) - end - - def exists? - storage.sismember @service.storage_key("user_set"), @username - end - - def delete - storage.srem @service.storage_key("user_set"), @username - end - - private - - def storage - Service.storage - end - - end - end -end diff --git a/lib/3scale/backend/user.rb b/lib/3scale/backend/user.rb deleted file mode 100644 index b9d6e110f..000000000 --- a/lib/3scale/backend/user.rb +++ /dev/null @@ -1,201 +0,0 @@ -require '3scale/backend/use_cases/service_user_management_use_case' - -module ThreeScale - module Backend - class User - include Storable - - attr_accessor :service_id, :username, :state, :plan_id, :plan_name - attr_writer :metric_names - - def self.attribute_names - %i[service_id username state plan_id plan_name - metric_names].freeze - end - - def self.exists?(service_id, username) - storage.exists(key(service_id, username)) - end - - def self.load(service_id, username) - key = self.key(service_id, username) - - values = storage.hmget(key, 'state', 'plan_id', 'plan_name') - state, plan_id, plan_name = values - - unless state.nil? - new({ - service_id: service_id, - username: username, - state: state.to_sym, - plan_id: plan_id, - plan_name: plan_name, - }) - end - end - - def self.load_or_create!(service, username) - key = Memoizer.build_key(self, :load_or_create!, service.id, username) - Memoizer.memoize_block(key) do - user = load(service.id, username) - - unless user - # the user does not exist yet, we need to create it for the case of - # the open loop - if service.user_registration_required? - raise ServiceRequiresRegisteredUser, service.id - end - - if service.default_user_plan_id.nil? or service.default_user_plan_name.nil? - raise ServiceRequiresDefaultUserPlan - end - - attributes = { - service_id: service.id, - username: username, - state: :active, - plan_id: service.default_user_plan_id, - plan_name: service.default_user_plan_name, - } - user = new(attributes) - user.save - end - - user - end - end - - def self.save!(attributes) - validate_attributes(attributes) - - # create the user object - user = new(attributes) - user.save - user - end - - def self.delete!(service_id, username) - service = Service.load_by_id(service_id) - raise UserRequiresValidService if service.nil? - delete_users_in_batch(service_id, [username]) - end - - def self.delete_all(service_id) - service = Service.load_by_id(service_id) - raise UserRequiresValidService if service.nil? - current_scan_cursor_position = "0" - finished = false - users_set_key = service_users_set_key(service_id) - while !finished - (current_scan_cursor_position, current_usernames) = - storage.sscan(users_set_key, current_scan_cursor_position, - { count: Storage::BATCH_SIZE }) - delete_users_in_batch(service_id, current_usernames) - finished = current_scan_cursor_position == "0" - end - end - - def self.key(service_id, username) - "service:#{service_id}/user:#{username}" - end - - def self.service_users_set_key(service_id) - "service/id:#{service_id}/user_set" - end - - def to_hash - { - service_id: service_id, - username: username, - state: state, - plan_id: plan_id, - plan_name: plan_name, - } - end - - def service - @service ||= Service.load_by_id service_id - end - - def save - save_attributes - - ServiceUserManagementUseCase.new(service, username).add.tap do - self.class.clear_cache(service_id, username) - end - end - - def active? - state == :active - end - - def key - self.class.key(service_id, username) - end - - def metric_names - @metric_names ||= {} - end - - def metric_name(metric_id) - metric_names[metric_id] ||= Metric.load_name(service_id, metric_id) - end - - # Sets @metric_names with the names of all the metrics for which there is - # a usage limit that applies to the user, and returns it. - def load_metric_names - metric_ids = usage_limits.map(&:metric_id) - @metric_names = Metric.load_all_names(service_id, metric_ids) - end - - def usage_limits - @usage_limits ||= UsageLimit.load_all(service_id, plan_id) - end - - private - - def self.clear_cache(service_id, user_id) - key = Memoizer.build_key(self, :load_or_create!, service_id, user_id) - Memoizer.clear key - end - - def self.delete_users_in_batch(service_id, usernames) - if usernames.size != 0 - service_usernames_keys = usernames.map { |username| self.key(service_id, username) } - storage.pipelined do - storage.del(service_usernames_keys) - storage.srem(self.service_users_set_key(service_id), usernames) - end - usernames.each do |username| - clear_cache(service_id, username) - end - end - end - - def self.validate_attributes(attributes) - raise UserRequiresUsername if attributes[:username].nil? - raise UserRequiresServiceId if attributes[:service_id].nil? - service = Service.load_by_id(attributes[:service_id]) - raise UserRequiresValidService if service.nil? - attributes[:plan_id] ||= service.default_user_plan_id - attributes[:plan_name] ||= service.default_user_plan_name - raise UserRequiresDefinedPlan if attributes[:plan_id].nil? || attributes[:plan_name].nil? - attributes[:state] ||= "active" - end - - # Username is used as a unique user ID - def user_id - username - end - - def save_attributes - storage.hset key, "state", state.to_s if state - storage.hset key, "plan_id", plan_id if plan_id - storage.hset key, "plan_name", plan_name if plan_name - storage.hset key, "username", username if username - storage.hset key, "service_id", service_id if service_id - end - - end - end -end diff --git a/lib/3scale/backend/validators/limits.rb b/lib/3scale/backend/validators/limits.rb index 8ffcf82d1..0470431e8 100644 --- a/lib/3scale/backend/validators/limits.rb +++ b/lib/3scale/backend/validators/limits.rb @@ -4,15 +4,7 @@ module Validators class Limits < Base def apply - limits_app_ok = status.application ? valid_limits_app? : true - - limits_ok = if limits_app_ok - status.user ? valid_limits_user? : true - else - false - end - - limits_ok ? succeed! : fail!(LimitsExceeded.new) + valid_limits_app? ? succeed! : fail!(LimitsExceeded.new) end private @@ -32,10 +24,6 @@ def valid_limits_app? valid_limits?(status.values, status.application.usage_limits) end - def valid_limits_user? - valid_limits?(status.user_values, status.user.usage_limits) - end - def valid_limits?(values, limits) processed_values = process(values, params[:usage]) limits.all? { |limit| limit.validate(processed_values) } From 6acb64383165ba6a1096c55c4808fcb21617811c Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Wed, 18 Sep 2019 20:53:24 +0200 Subject: [PATCH 03/13] oauth: remove support for end-users --- docs/oauth_tokens.md | 44 ++++----------- lib/3scale/backend/listener.rb | 9 +-- lib/3scale/backend/oauth/token.rb | 8 +-- lib/3scale/backend/oauth/token_storage.rb | 56 +++++++------------ lib/3scale/backend/oauth/token_value.rb | 43 +++----------- lib/3scale/backend/transactor.rb | 10 +--- .../backend/views/oauth_access_tokens.builder | 1 - .../views/oauth_app_id_by_token.builder | 5 -- 8 files changed, 46 insertions(+), 130 deletions(-) diff --git a/docs/oauth_tokens.md b/docs/oauth_tokens.md index 997b206e0..f50e2ae74 100644 --- a/docs/oauth_tokens.md +++ b/docs/oauth_tokens.md @@ -37,23 +37,12 @@ The Redis layout for mapping a token to a service is a single key-value: * `oauth_access_tokens/service:#{service_id}/#{token}` So it depends only on a service. This key is known as the `token_key`. The value -of this key *must* contain an application identifier and *optionally* a user -identifier. +of this key *must* contain an application identifier. -When a user identifier is NOT present, and the application identifier is -'app_id', the value has the form: +The application identifier is 'app_id' and the value has the form: * `app_id` -When a user identifier IS present and is 'user_id', the value has the form: - -* `user_len:7/user_id/app_id` - -Notice that in the latter case a prefix `user_len` and an integer have been -added, as well as separators `/` to encode the additional information. The -integer is the amount of characters that the `user_id` has. This has to be like -this because both `app_id` and `user_id` are basically free-form. - Appropriate checks are in place to parse the above possibilities. ## Creating and Deleting tokens @@ -72,17 +61,11 @@ Their format is: * `"oauth_access_tokens/service:#{service_id}/app:#{app_id}/"` -The remaining question is how do we know which users have tokens. We basically -walk the set of tokens and filter them according to the user we are interested -in. Because users can be totally made up and never used again (ie. not stored at -all because they might be temporary), we should not store relationships. - ### Redis steps When storing a new token, we add `token_key` in Redis and assign an optional TTL to it if the token expires after a certain amount of time. The value of such key -is the (application id, user id) referred, with the user specific prefix if -applicable. +is the application id referred. We also add the token to the `token_set_key` so that we can list the tokens for that application. We do the reverse steps when deleting. @@ -99,10 +82,6 @@ This is the feature that requires walking the token set. > in the endpoint (or a way to stream results) and we don't yet use Redis' scan > support anywhere in the codebase to avoid blocking the database. -When we list the tokens for a given service and application, we want to list -*all* of them, that is, including user-specific tokens. We also support listing -those that apply to a single user alone. - Additionally, we want to take the opportunity to make some janitorial work. Because tokens can expire, we check that they are still valid, and if not, do all the related housekeeping. @@ -111,20 +90,19 @@ When we are done, we return the tokens in an array. ### Redis steps -First thing we do is getting the members of the `token_set_key` set. We then -filter the tokens if we're interested in a specific user identifier. +First thing we do is getting the members of the `token_set_key` set. For each token we get, we build its `token_key` and get it from Redis. If the -result was not nil, it means we got a correct tuple of (application id, user id), -and thus create a new `OAuth::Token` object ready to be returned. +result was not nil, it means we got a correct application id, and thus create a +new `OAuth::Token` object ready to be returned. If the result *was* nil, it means that token is no longer valid (expired). In that case, we remove the token from the `token_set_key` set. # Authorization -Authorization is granted or denied based on the application id and user id that -the token points to. Obviously, no authorization will occur if there is no such -mapping. However, *all and any* application and user identifiers provided -through parameters will take precedence over identifiers found through the -token. (this, inexplicably, totally bypasses `user_key` at the moment). +Authorization is granted or denied based on the application id that the token +points to. Obviously, no authorization will occur if there is no such mapping. +However, *all and any* application identifiers provided through parameters will +take precedence over identifiers found through the token. (this, inexplicably, +totally bypasses `user_key` at the moment). diff --git a/lib/3scale/backend/listener.rb b/lib/3scale/backend/listener.rb index 290d24944..08dd68afd 100644 --- a/lib/3scale/backend/listener.rb +++ b/lib/3scale/backend/listener.rb @@ -433,9 +433,7 @@ def do_api_method(method_name) app_id = params[:app_id] raise ApplicationNotFound, app_id unless Application.exists?(service_id, app_id) - # Users do not need to exist, since they can be "created" on-demand. - OAuth::Token::Storage.create(params[:token], service_id, app_id, - params[:user_id], params[:ttl]) + OAuth::Token::Storage.create(params[:token], service_id, app_id, params[:ttl]) end delete '/services/:service_id/oauth_access_tokens/:token.xml' do @@ -460,7 +458,7 @@ def do_api_method(method_name) raise ApplicationNotFound, app_id unless Application.exists?(service_id, app_id) - @tokens = OAuth::Token::Storage.all_by_service_and_app service_id, app_id, params[:user_id] + @tokens = OAuth::Token::Storage.all_by_service_and_app service_id, app_id builder :oauth_access_tokens end @@ -470,8 +468,7 @@ def do_api_method(method_name) service_id = params[:service_id] ensure_authenticated!(params[:provider_key], params[:service_token], service_id) - @token_to_app_id, @token_to_user_id = - OAuth::Token::Storage.get_credentials(params[:token], service_id) + @token_to_app_id = OAuth::Token::Storage.get_credentials(params[:token], service_id) builder :oauth_app_id_by_token end diff --git a/lib/3scale/backend/oauth/token.rb b/lib/3scale/backend/oauth/token.rb index c0a5bc44c..2fcb4479e 100644 --- a/lib/3scale/backend/oauth/token.rb +++ b/lib/3scale/backend/oauth/token.rb @@ -3,20 +3,18 @@ module Backend module OAuth class Token attr_reader :service_id, :token, :key - attr_accessor :ttl, :app_id, :user_id + attr_accessor :ttl, :app_id - # user_id can be nil (most providers do not have Users) - def initialize(token, service_id, app_id, user_id, ttl) + def initialize(token, service_id, app_id, ttl) @token = token @service_id = service_id @app_id = app_id - @user_id = user_id @ttl = ttl @key = Key.for token, service_id end def value - Value.for app_id, user_id + Value.for app_id end def self.from_value(token, service_id, value, ttl) diff --git a/lib/3scale/backend/oauth/token_storage.rb b/lib/3scale/backend/oauth/token_storage.rb index 2b88decf0..9573b07f9 100644 --- a/lib/3scale/backend/oauth/token_storage.rb +++ b/lib/3scale/backend/oauth/token_storage.rb @@ -22,7 +22,7 @@ class << self include Backend::Logging include Backend::StorageHelpers - def create(token, service_id, app_id, user_id, ttl = nil) + def create(token, service_id, app_id, ttl = nil) raise AccessTokenFormatInvalid if token.nil? || token.empty? || !token.is_a?(String) || token.bytesize > MAXIMUM_TOKEN_SIZE @@ -32,7 +32,7 @@ def create(token, service_id, app_id, user_id, ttl = nil) key = Key.for token, service_id raise AccessTokenAlreadyExists.new(token) unless storage.get(key).nil? - value = Value.for(app_id, user_id) + value = Value.for app_id token_set = Key::Set.for(service_id, app_id) store_token token, token_set, key, value, ttl @@ -41,14 +41,13 @@ def create(token, service_id, app_id, user_id, ttl = nil) # Deletes a token # - # Returns the associated [app_id, user_id] or nil + # Returns the associated app_id or nil # def delete(token, service_id) key = Key.for token, service_id val = storage.get key if val - val = Value.from val - app_id = val.first + app_id = Value.from val token_set = Key::Set.for(service_id, app_id) existed, * = remove_a_token token_set, token, key @@ -63,36 +62,32 @@ def delete(token, service_id) val end - # Get a token's associated [app_id, user_id] + # Get a token's associated app_id def get_credentials(token, service_id) - ids = Value.from(storage.get(Key.for(token, service_id))) - raise AccessTokenInvalid.new token if ids.first.nil? - ids + app_id = Value.from(storage.get(Key.for(token, service_id))) + raise AccessTokenInvalid.new token if app_id.nil? + app_id end - # This is used to list tokens by service, app and possibly user. + # This is used to list tokens by service and app. # # Note: this deletes tokens that have not been found from the set of # tokens for the given app - those have to be expired tokens. - def all_by_service_and_app(service_id, app_id, user_id = nil) + def all_by_service_and_app(service_id, app_id) token_set = Key::Set.for(service_id, app_id) deltokens = [] tokens_n_values_flat(token_set, service_id) .select do |(token, _key, value, _ttl)| - app_id, uid = Value.from value + app_id = Value.from value if app_id.nil? deltokens << token false else - !user_id || uid == user_id + true end end .map do |(token, _key, value, ttl)| - if user_id - Token.new token, service_id, app_id, user_id, ttl - else - Token.from_value token, service_id, value, ttl - end + Token.from_value token, service_id, value, ttl end .force.tap do # delete expired tokens (nil values) from token set @@ -102,38 +97,25 @@ def all_by_service_and_app(service_id, app_id, user_id = nil) end end - # Remove tokens by app_id and optionally user_id. - # - # If user_id is nil or unspecified, this will remove all app tokens + # Remove tokens by app_id. # # Triggered by Application deletion. # - # TODO: we could expose the ability to delete all tokens for a given - # user_id, but we are currently not doing that. - # - def remove_tokens(service_id, app_id, user_id = nil) - filter = lambda do |(_t, _k, v, _ttl)| - user_id == Value.from(v).last - end if user_id - remove_tokens_by service_id, app_id, &filter + def remove_tokens(service_id, app_id) + remove_tokens_by service_id, app_id end private - # Remove all tokens or only those selected by a block + # Remove all tokens # # I thought of leaving this one public, but remove_*_tokens removed # my use cases for the time being. - def remove_tokens_by(service_id, app_id, &blk) + def remove_tokens_by(service_id, app_id) token_set = Key::Set.for(service_id, app_id) - # No block? Just remove everything and smile! - if blk.nil? - remove_whole_token_set(token_set, service_id) - else - remove_token_set_by(token_set, service_id, &blk) - end + remove_whole_token_set(token_set, service_id) end def remove_token_set_by(token_set, service_id, &blk) diff --git a/lib/3scale/backend/oauth/token_value.rb b/lib/3scale/backend/oauth/token_value.rb index c7fe8c4bc..f8fd6ffa0 100644 --- a/lib/3scale/backend/oauth/token_value.rb +++ b/lib/3scale/backend/oauth/token_value.rb @@ -1,50 +1,21 @@ # This module encodes values in Redis for our tokens -# -# The values need to provide both app and user ids, but the latter are optional. -# module ThreeScale module Backend module OAuth class Token module Value - class << self - TOKEN_HAS_USER_FIELD = "user_len".freeze - # for ':' and '/'; the size of the user's len is determined and - # added in a different place. - TOKEN_HAS_USER_CONSTSIZE = TOKEN_HAS_USER_FIELD.size + 1 + 1 - TOKEN_HAS_USER_RE = /#{Regexp.escape("#{TOKEN_HAS_USER_FIELD}")}:(?\d+)\// - private_constant :TOKEN_HAS_USER_FIELD, - :TOKEN_HAS_USER_CONSTSIZE, - :TOKEN_HAS_USER_RE + # Note: this module made more sense when it also supported end-users. + # Given how simple the module is, we could get rid of it in a future + # refactor. + class << self # this method is used when creating tokens - def for(app_id, user_id) - if user_id.nil? - return app_id if app_id !~ TOKEN_HAS_USER_RE - # unlikely that an app starts with "user_len:\d+/"... but we can - # support it by just using user_len:0//user_len:\d+/. - user_id = ''.freeze - end - "#{TOKEN_HAS_USER_FIELD}:#{user_id.size}/#{user_id}/#{app_id}" + def for(app_id) + app_id end - # values have the form "user_len:4/alex/app_id_goes_here" def from(value) - app_id = value - if app_id - md = TOKEN_HAS_USER_RE.match value - if md - # we assume no one has added rogue keys that declare bad sizes - uidx = TOKEN_HAS_USER_CONSTSIZE + md[:ulen].size - ulen = md[:ulen].to_i - split_idx = uidx + ulen - user_id = value[uidx...split_idx] if ulen > 0 - app_id = value[split_idx+1..-1] - end - else - user_id = nil - end - [app_id, user_id] + value end end end diff --git a/lib/3scale/backend/transactor.rb b/lib/3scale/backend/transactor.rb index 12e0928f3..11340b25d 100644 --- a/lib/3scale/backend/transactor.rb +++ b/lib/3scale/backend/transactor.rb @@ -108,10 +108,9 @@ def validate(oauth, provider_key, report_usage, params, extensions) apply_validators(validators, status_attrs, params) end - def get_token_ids(token, service_id, app_id, user_id) + def get_token_ids(token, service_id, app_id) begin - token_aid, token_uid = OAuth::Token::Storage. - get_credentials(token, service_id) + token_aid = OAuth::Token::Storage.get_credentials(token, service_id) rescue AccessTokenInvalid => e # Yep, well, er. Someone specified that it is OK to have an # invalid token if an app_id is specified. Somehow passing in @@ -123,11 +122,8 @@ def get_token_ids(token, service_id, app_id, user_id) if app_id.nil? app_id = token_aid end - if user_id.nil? - user_id = token_uid - end - [app_id, user_id] + app_id end def do_authorize(method, provider_key, params, extensions) diff --git a/lib/3scale/backend/views/oauth_access_tokens.builder b/lib/3scale/backend/views/oauth_access_tokens.builder index 51ad534cd..5d530b3fb 100644 --- a/lib/3scale/backend/views/oauth_access_tokens.builder +++ b/lib/3scale/backend/views/oauth_access_tokens.builder @@ -9,7 +9,6 @@ xml.oauth_access_tokens do # # Just enforce the ttl to be -1 when there is none. attrs = { :ttl => t.ttl || -1 } - attrs[:user_id] = t.user_id if t.user_id xml.oauth_access_token t.token, attrs end end diff --git a/lib/3scale/backend/views/oauth_app_id_by_token.builder b/lib/3scale/backend/views/oauth_app_id_by_token.builder index a5fda8b28..be5c9de41 100644 --- a/lib/3scale/backend/views/oauth_app_id_by_token.builder +++ b/lib/3scale/backend/views/oauth_app_id_by_token.builder @@ -2,8 +2,3 @@ xml.instruct! xml.application do xml.app_id @token_to_app_id end -if @token_to_user_id - xml.user do - xml.user_id @token_to_user_id - end -end From 0ce512f052f2170c9a9a5fe4608bd7533134b0b6 Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Wed, 18 Sep 2019 21:47:28 +0200 Subject: [PATCH 04/13] spec/use_cases: remove support for end-users --- .../service_user_management_use_case_spec.rb | 49 ------------------- 1 file changed, 49 deletions(-) delete mode 100644 spec/use_cases/service_user_management_use_case_spec.rb diff --git a/spec/use_cases/service_user_management_use_case_spec.rb b/spec/use_cases/service_user_management_use_case_spec.rb deleted file mode 100644 index 392b0fc09..000000000 --- a/spec/use_cases/service_user_management_use_case_spec.rb +++ /dev/null @@ -1,49 +0,0 @@ -require_relative '../spec_helper' - -module ThreeScale - module Backend - describe ServiceUserManagementUseCase do - let(:service){ Service.save! id: 7001, provider_key: 'random' } - let(:use_case){ ServiceUserManagementUseCase.new(service, 'foo') } - - describe '#add' do - it 'adds a User to a Service' do - use_case.add - - expect(ServiceUserManagementUseCase.new(service, 'foo').exists?).to be true - end - - it 'returns true when User has been added' do - expect(use_case.add).to be true - end - - it 'returns false when User was already added' do - ServiceUserManagementUseCase.new(service, 'foo').add - - expect(use_case.add).to be false - end - end - - describe '#delete' do - it 'deletes a User from a Service' do - ServiceUserManagementUseCase.new(service, 'foo').add - use_case.delete - - expect(ServiceUserManagementUseCase.new(service, 'foo').exists?).to be false - end - - it 'returns true when User has been deleted' do - ServiceUserManagementUseCase.new(service, 'foo').add - - expect(use_case.delete).to be true - end - - it "returns false when User wasn't added" do - expect(use_case.delete).to be false - end - end - - end - end -end - From 35c6703da046653e6904a86a51dc805fbc3867e3 Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Wed, 18 Sep 2019 21:49:01 +0200 Subject: [PATCH 05/13] spec/integration: remove support for end-users --- spec/integration/stats_partition_eraser_job_spec.rb | 6 ++---- spec/integration/stats_partition_generator_job_spec.rb | 2 -- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/spec/integration/stats_partition_eraser_job_spec.rb b/spec/integration/stats_partition_eraser_job_spec.rb index e01208ecf..8a0a73ef7 100644 --- a/spec/integration/stats_partition_eraser_job_spec.rb +++ b/spec/integration/stats_partition_eraser_job_spec.rb @@ -4,7 +4,6 @@ let(:service_id) { '123456' } let(:applications) { %w[1] } let(:metrics) { %w[10] } - let(:users) { %w[] } let(:from) { Time.new(2002, 11, 31).to_i } let(:to) { from } let(:offset) { 5 } @@ -14,7 +13,6 @@ service_id: service_id, applications: applications, metrics: metrics, - users: users, from: from, to: to } @@ -31,7 +29,7 @@ it 'partition keys are deleted' do without_resque_spec do Resque.enqueue(described_class, Time.now.getutc.to_f, service_id, applications, - metrics, users, from, to, offset, length, nil) + metrics, from, to, offset, length, nil) expect(Resque.size(stats_queue)).to eq 1 # Try to process the job. ThreeScale::Backend::Worker.work(one_off: true) @@ -44,7 +42,7 @@ it 'keys outside partition are not deleted' do without_resque_spec do Resque.enqueue(described_class, Time.now.getutc.to_f, service_id, applications, - metrics, users, from, to, offset, length, nil) + metrics, from, to, offset, length, nil) expect(Resque.size(stats_queue)).to eq 1 # Try to process the job. ThreeScale::Backend::Worker.work(one_off: true) diff --git a/spec/integration/stats_partition_generator_job_spec.rb b/spec/integration/stats_partition_generator_job_spec.rb index 8cf8da8c4..4401fae7b 100644 --- a/spec/integration/stats_partition_generator_job_spec.rb +++ b/spec/integration/stats_partition_generator_job_spec.rb @@ -4,7 +4,6 @@ let(:service_id) { '123456' } let(:applications) { %w[1] } let(:metrics) { %w[10] } - let(:users) { %w[100] } let(:from) { Time.new(2002, 10, 31).to_i } let(:to) { Time.new(2002, 11, 30).to_i } let(:job) do @@ -12,7 +11,6 @@ service_id: service_id, applications: applications, metrics: metrics, - users: users, from: from, to: to ) From 6c87770bfc9ea3b09f87d8f60a4b5788a0db13c9 Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Wed, 18 Sep 2019 21:53:01 +0200 Subject: [PATCH 06/13] spec/acceptance: remove support for end-users --- .../api/internal/services_api_spec.rb | 18 -- .../acceptance/api/internal/stats_api_spec.rb | 2 - .../acceptance/api/internal/users_api_spec.rb | 156 ------------------ 3 files changed, 176 deletions(-) delete mode 100644 spec/acceptance/api/internal/users_api_spec.rb diff --git a/spec/acceptance/api/internal/services_api_spec.rb b/spec/acceptance/api/internal/services_api_spec.rb index 52b675faf..e08d65b44 100644 --- a/spec/acceptance/api/internal/services_api_spec.rb +++ b/spec/acceptance/api/internal/services_api_spec.rb @@ -105,15 +105,6 @@ expect(svc.to_hash).to eq service.merge(user_registration_required: true) end - example 'Try creating a Service with invalid data' do - do_request(service: { user_registration_required: false, - default_user_plan_name: nil, - default_user_plan_id: nil }) - - expect(status).to eq 400 - expect(response_json['error']).to match /require a default user plan/ - end - example 'Try creating a Service without specifying the service parameter in the body' do do_request(service: nil) @@ -199,15 +190,6 @@ user_registration_required: true) end - example 'Try updating Service with invalid data' do - do_request(service: { user_registration_required: false, - default_user_plan_name: nil, - default_user_plan_id: nil }) - - expect(status).to eq 400 - expect(response_json['error']).to match /require a default user plan/ - end - context 'Create a service that has no state' do let(:state) { nil } diff --git a/spec/acceptance/api/internal/stats_api_spec.rb b/spec/acceptance/api/internal/stats_api_spec.rb index c1f1fcba0..e0d4a423b 100644 --- a/spec/acceptance/api/internal/stats_api_spec.rb +++ b/spec/acceptance/api/internal/stats_api_spec.rb @@ -9,7 +9,6 @@ let(:provider_key) { 'statsfoo' } let(:applications) { %w[1 2 3] } let(:metrics) { %w[10 20 30] } - let(:users) { %w[100 200 300] } let(:from) { Time.new(2002, 10, 31).to_i } let(:to) { Time.new(2003, 10, 31).to_i } let(:req_body) do @@ -17,7 +16,6 @@ deletejobdef: { applications: applications, metrics: metrics, - users: users, from: from, to: to } diff --git a/spec/acceptance/api/internal/users_api_spec.rb b/spec/acceptance/api/internal/users_api_spec.rb deleted file mode 100644 index 4daaca6dc..000000000 --- a/spec/acceptance/api/internal/users_api_spec.rb +++ /dev/null @@ -1,156 +0,0 @@ -require_relative '../../../spec_helpers/acceptance_spec_helper' - -resource 'Users (prefix: /services/:service_id/users)' do - header 'Accept', 'application/json' - header 'Content-Type', 'application/json' - - let(:service_id) { '7575' } - let(:username) { 'pancho' } - let(:state) { :active } - let(:plan_id) { '80210' } - let(:plan_name) { 'foobar' } - let(:service_id_non_existent) { service_id.to_i.succ.to_s } - let(:username_non_existent) { username + '_nonexistent' } - let(:user) do - { - service_id: service_id, - username: username, - state: state, - plan_id: plan_id, - plan_name: plan_name - } - end - - before do - ThreeScale::Backend::Service.save!(id: service_id, provider_key: 'a_provider') - - ThreeScale::Backend::User.delete!(service_id, username) rescue nil - ThreeScale::Backend::User.delete!(service_id, username_non_existent) rescue nil - ThreeScale::Backend::User.save!(service_id: service_id, - username: username, - state: state, - plan_id: plan_id, - plan_name: plan_name) - end - - get '/services/:service_id/users/:username' do - parameter :service_id, 'Service ID', required: true - parameter :username, 'User name', required: true - - example_request 'Get User' do - expect(status).to eq 200 - expect(response_json['user']['username']).to eq username - expect(response_json['user']['service_id']).to eq service_id - expect(response_json['user']['state']).to eq state.to_s - expect(response_json['user']['plan_id']).to eq plan_id - expect(response_json['user']['plan_name']).to eq plan_name - end - - example 'Try to get a User by non-existent username' do - do_request username: username_non_existent - expect([400, 404]).to include status - expect(response_json['error']).to match /not found/i - end - - example 'Try to get a User by non-existent service ID' do - do_request service_id: service_id_non_existent - expect([400, 404]).to include status - expect(response_json['error']).to match /not found/i - end - end - - post '/services/:service_id/users/:username' do - parameter :service_id, 'Service ID', required: true - parameter :username, 'User name', required: true - parameter :user, 'User attributes', required: true - - let(:raw_post) { params.to_json } - - example_request 'Create a User' do - expect(status).to eq 201 - expect(response_json['status']).to eq 'created' - - user = ThreeScale::Backend::User.load(service_id, username) - expect(user.username).to eq username - expect(user.service_id).to eq service_id - expect(user.state).to eq state - expect(user.plan_id).to eq plan_id - expect(user.plan_name).to eq plan_name - end - - example 'Create a User with extra params' do - do_request user: user.merge(some_param: 'some_value') - expect(status).to eq 201 - expect(response_json['status']).to eq 'created' - - user = ThreeScale::Backend::User.load(service_id, username) - expect(user).not_to be_nil - expect(user).not_to respond_to :some_param - expect(user.username).to eq username - expect(user.service_id).to eq service_id - expect(user.state).to eq state - expect(user.plan_id).to eq plan_id - expect(user.plan_name).to eq plan_name - end - end - - put '/services/:service_id/users/:username' do - parameter :service_id, 'Service ID', required: true - parameter :username, 'User name', required: true - parameter :user, 'User attributes', required: true - - let(:modified_plan_name) { plan_name + '_modified' } - let(:modified_user) { user.merge(plan_name: modified_plan_name) } - - let(:raw_post) { params.to_json } - - example 'Update User' do - do_request user: modified_user - expect(status).to eq 200 - expect(response_json['status']).to eq 'modified' - - user = ThreeScale::Backend::User.load(service_id, username) - expect(user.username).to eq username - expect(user.service_id).to eq service_id - expect(user.state).to eq state - expect(user.plan_id).to eq plan_id - expect(user.plan_name).to eq modified_plan_name - end - - example 'Update User with extra params' do - do_request user: modified_user.merge(some_param: 'some_value') - expect(status).to eq 200 - expect(response_json['status']).to eq 'modified' - - user = ThreeScale::Backend::User.load(service_id, username) - expect(user).not_to be_nil - expect(user).not_to respond_to :some_param - expect(user.username).to eq username - expect(user.service_id).to eq service_id - expect(user.state).to eq state - expect(user.plan_id).to eq plan_id - expect(user.plan_name).to eq modified_plan_name - end - end - - delete '/services/:service_id/users/:username' do - parameter :service_id, 'Service ID', required: true - parameter :username, 'User name', required: true - - example_request 'Deleting a User' do - expect(status).to eq 200 - expect(response_json['status']).to eq 'deleted' - expect(ThreeScale::Backend::User.load(service_id, username)).to be nil - end - end - - delete '/services/:service_id/users' do - parameter :service_id, 'Service ID', required: true - - example_request 'Deleting all Users of a Service' do - expect(status).to eq 200 - expect(response_json['status']).to eq 'deleted' - expect(ThreeScale::Backend::User.exists?(service_id, username)).to be false - end - end -end From 440fdf25fe0d4bc1bd1fce01853a27d1eb3a2405 Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Wed, 18 Sep 2019 22:28:11 +0200 Subject: [PATCH 07/13] spec/unit: remove support for end-users --- spec/unit/jobs/report_job_spec.rb | 2 +- spec/unit/oauth/token_spec.rb | 83 +++----------------- spec/unit/service_spec.rb | 5 -- spec/unit/stats/delete_job_def_spec.rb | 23 +----- spec/unit/stats/key_generator_spec.rb | 54 ------------- spec/unit/stats/keys_spec.rb | 16 ---- spec/unit/stats/partition_eraser_job_spec.rb | 3 +- 7 files changed, 12 insertions(+), 174 deletions(-) diff --git a/spec/unit/jobs/report_job_spec.rb b/spec/unit/jobs/report_job_spec.rb index de8e99032..48b97128b 100644 --- a/spec/unit/jobs/report_job_spec.rb +++ b/spec/unit/jobs/report_job_spec.rb @@ -32,7 +32,7 @@ module Transactor before do expect(ReportJob) .to receive(:parse_transactions) - .and_raise(Backend::ServiceRequiresRegisteredUser.new(service_id)) + .and_raise(Backend::UserKeyInvalid.new('some_user_key')) end it 'rescues the exception' do diff --git a/spec/unit/oauth/token_spec.rb b/spec/unit/oauth/token_spec.rb index 944b854a1..51bb693cd 100644 --- a/spec/unit/oauth/token_spec.rb +++ b/spec/unit/oauth/token_spec.rb @@ -4,83 +4,18 @@ module ThreeScale module Backend module OAuth describe Token do - # static parameters, not important here - service_id = '1001' - ttl = -1 - - # variable parameters - tokens = [ - 'SomEoAuTHtok3n', - 'va/lid/tok/en' - ] - app_ids = [ - '2001', - 'user_len:5/myapp' - ] - user_ids = ['alex', 'user_len:1/x', 'user_len:200/uid', nil] - - # build combinations of parameters - constructors_w_params = tokens.product(app_ids, user_ids) - .inject({:new => []}) do |acc, (token, app_id, user_id)| - acc[:new] << [ - Token.new(token, service_id, app_id, user_id, ttl), - token, app_id, user_id - ] - acc - end - # Add another constructor JUST to spec .from_value, because all - # parameter combinations have been added already for .new. We do not - # need to retest those with .from_value, just one and see if it works. - constructors_w_params[:from_value] = [[ - Token.from_value(tokens.first, service_id, Token::Value.for(app_ids.first, user_ids.first), ttl), - tokens.first, app_ids.first, user_ids.first - ]] - constructors_w_params.each do |constructor, subject_params| - subject_params.each do |subject, token, app_id, user_id| - describe ".#{constructor}" do - context "with token #{token.inspect}, app_id #{app_id.inspect}, " \ - "user_id #{user_id.inspect}" do - [:token, :service_id, :app_id, :user_id, :ttl].each do |attr| - let(attr) { binding.local_variable_get attr } - describe "##{attr}" do - it "returns the associated #{attr}" do - expect(subject.public_send attr).to eq(public_send attr) - end - end - end - - describe '#key' do - it 'provides the key used for storage' do - expect(subject.key).to eq(Token::Key.for token, service_id) - end - end - - describe '#value' do - it "returns the stored value for the associated app and " \ - "user id #{user_id.inspect}" do - expect(subject.value).to eq(Token::Value.for app_id, user_id) - end - end - end - end - end - end - describe Token::Value do - app_ids.product(user_ids) do |app_id, user_id| - describe '.for' do - it "provides an encoded string out of an app_id #{app_id.inspect} " \ - "and user_id #{user_id.inspect}" do - expect(described_class.for(app_id, user_id)).to be_a(String) - end + describe '.for' do + it 'returns the app_id' do + app_id = 'some_app_id' + expect(described_class.for(app_id)).to eq app_id end + end - describe '.from' do - it "returns the app_id #{app_id.inspect} and user_id " \ - "#{user_id.inspect} out of an encoded string" do - expect(described_class.from(described_class.for(app_id, user_id))). - to eq([app_id, user_id]) - end + describe '.from' do + it 'returns the app_id' do + app_id = 'some_app_id' + expect(described_class.from(app_id)).to eq app_id end end end diff --git a/spec/unit/service_spec.rb b/spec/unit/service_spec.rb index b1d87cdce..e89ea7684 100644 --- a/spec/unit/service_spec.rb +++ b/spec/unit/service_spec.rb @@ -345,11 +345,6 @@ module Backend service.save! expect(Memoizer.memoized?(Memoizer.build_key(Service, :default_id, 'foo'))).to be false end - - it 'validates user_registration_required field' do - service.user_registration_required = false - expect { service.save! }.to raise_error(ServiceRequiresDefaultUserPlan) - end end describe '.delete_by_id' do diff --git a/spec/unit/stats/delete_job_def_spec.rb b/spec/unit/stats/delete_job_def_spec.rb index ad8beac4a..0a268bf82 100644 --- a/spec/unit/stats/delete_job_def_spec.rb +++ b/spec/unit/stats/delete_job_def_spec.rb @@ -13,10 +13,6 @@ expect(job).to include(metrics: metrics) end - it 'has users' do - expect(job).to include(users: users) - end - it 'has from' do expect(job).to include(from: from) end @@ -36,7 +32,6 @@ let(:service_id) { 'some_service_id' } let(:applications) { %w[1 2 3] } let(:metrics) { %w[10 20 30] } - let(:users) { %w[100 200 300] } let(:from) { Time.new(2002, 10, 31).to_i } let(:to) { Time.new(2003, 10, 31).to_i } let(:params) do @@ -44,7 +39,6 @@ service_id: service_id, applications: applications, metrics: metrics, - users: users, from: from, to: to } @@ -121,21 +115,6 @@ let(:metrics) { ['3', [], '4'] } include_examples 'validation error' end - - context 'users field is nil' do - let(:users) { nil } - include_examples 'validation error' - end - - context 'users field is not array' do - let(:users) { 3 } - include_examples 'validation error' - end - - context 'users field constains element bad string' do - let(:users) { ['3', [], '4'] } - include_examples 'validation error' - end end context '#run_async' do @@ -148,7 +127,7 @@ expect(ThreeScale::Backend::Stats::PartitionGeneratorJob).to have_queued(anything, service_id, applications, - metrics, users, + metrics, from, to, nil) end end diff --git a/spec/unit/stats/key_generator_spec.rb b/spec/unit/stats/key_generator_spec.rb index f3c3829bb..dd4440acc 100644 --- a/spec/unit/stats/key_generator_spec.rb +++ b/spec/unit/stats/key_generator_spec.rb @@ -8,7 +8,6 @@ def periods(granularity, from, to) let(:service_id) { '123456' } let(:applications) { %w[] } let(:metrics) { %w[] } - let(:users) { %w[] } let(:from) { Time.new(2002, 10, 31) } let(:to) { Time.new(2002, 11, 30) } let(:job_params) do @@ -16,7 +15,6 @@ def periods(granularity, from, to) service_id: service_id, applications: applications, metrics: metrics, - users: users, from: from.to_i, to: to.to_i } @@ -38,16 +36,6 @@ def periods(granularity, from, to) end end end - let(:expected_keys_responsecode_user) do - codes = %w[200 2XX 403 404 4XX 500 503 5XX] - periods = %i[hour day week month year eternity] - code_app_period = codes.product(applications, periods) - code_app_period.flat_map do |code, app_id, gr| - periods(gr, from, to).flat_map do |period| - ThreeScale::Backend::Stats::Keys.application_response_code_value_key(service_id, app_id, code, period) - end - end - end let(:expected_keys_usage_service) do periods = %i[hour day week month eternity] metric_period = metrics.product(periods) @@ -66,16 +54,6 @@ def periods(granularity, from, to) end end end - let(:expected_keys_usage_user) do - periods = %i[hour day week month year eternity] - metric_user_period = metrics.product(users, periods) - metric_user_period.flat_map do |metric_id, user_id, gr| - periods(gr, from, to).flat_map do |period| - ThreeScale::Backend::Stats::Keys.user_usage_value_key(service_id, user_id, metric_id, period) - end - end - end - subject { described_class.new(job_params).keys } context 'responsecode_service keys' do @@ -92,14 +70,6 @@ def periods(granularity, from, to) end end - context 'responsecode_user keys' do - let(:users) { %w[1] } - - it 'include expected keys' do - is_expected.to include(*expected_keys_responsecode_user) - end - end - context 'usage service keys' do let(:metrics) { %w[1] } @@ -116,28 +86,4 @@ def periods(granularity, from, to) is_expected.to include(*expected_keys_usage_application) end end - - context 'usage user keys' do - let(:metrics) { %w[1] } - let(:users) { %w[10] } - - it 'include expected keys' do - is_expected.to include(*expected_keys_usage_user) - end - end - context 'usage user keys' do - let(:metrics) { %w[1] } - let(:users) { %w[10] } - let(:applications) { %w[100] } - it 'unexpected keys not found' do - expect(subject.count).to eq([ - expected_keys_responsecode_service.count, - expected_keys_responsecode_application.count, - expected_keys_responsecode_user.count, - expected_keys_usage_service.count, - expected_keys_usage_application.count, - expected_keys_usage_user.count - ].reduce(:+)) - end - end end diff --git a/spec/unit/stats/keys_spec.rb b/spec/unit/stats/keys_spec.rb index 10eeedd4f..e3700ab3c 100644 --- a/spec/unit/stats/keys_spec.rb +++ b/spec/unit/stats/keys_spec.rb @@ -10,15 +10,12 @@ def self.currify(m, *args) private_class_method :currify Application = Struct.new(:service_id, :id) - User = Struct.new(:service_id, :username) app = Application.new 1000, 10 - user = User.new 1000, 20 let(:app) { app } let(:service_id) { app.service_id } let(:application_id) { app.id } let(:metric_id) { 100 } - let(:user_id) { user.username } let(:time) { Time.utc(2014, 7, 29, 18, 25) } describe '.service_key_prefix' do @@ -39,15 +36,6 @@ def self.currify(m, *args) end end - describe '.user_key_prefix' do - let(:prefix) { "stats/{service:1000}"} - let(:result) { Keys.user_key_prefix(prefix, user_id) } - - it 'returns a composed key with user id' do - expect(result).to eq("stats/{service:1000}/uinstance:20") - end - end - describe '.metric_key_prefix' do let(:prefix) { "stats/{service:1000}/cinstance:10"} let(:result) { Keys.metric_key_prefix(prefix, metric_id) } @@ -94,10 +82,6 @@ def self.currify(m, *args) it_behaves_like 'usage keys', currify(:application_usage_value_key, app.service_id, app.id), "cinstance:#{app.id}" end - describe '.user_usage_value_key' do - it_behaves_like 'usage keys', currify(:user_usage_value_key, app.service_id, user.username), "uinstance:#{user.username}" - end - describe '.counter_key' do let(:prefix) { "stats/{service:1000}/cinstance:10/metric:100"} diff --git a/spec/unit/stats/partition_eraser_job_spec.rb b/spec/unit/stats/partition_eraser_job_spec.rb index 9e3633e48..7dbdee127 100644 --- a/spec/unit/stats/partition_eraser_job_spec.rb +++ b/spec/unit/stats/partition_eraser_job_spec.rb @@ -4,14 +4,13 @@ let(:service_id) { '123456' } let(:applications) { %w[] } let(:metrics) { %w[] } - let(:users) { %w[] } let(:from) { Time.new(2002, 10, 31).to_i } let(:to) { Time.new(2002, 11, 30).to_i } let(:offset) { 5 } let(:length) { 10 } subject do - described_class.perform_logged(nil, service_id, applications, metrics, users, from, to, + described_class.perform_logged(nil, service_id, applications, metrics, from, to, offset, length, nil) end From 06d1f3bf866880666152bfb0b39beccfba0a2035 Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Wed, 18 Sep 2019 22:34:16 +0200 Subject: [PATCH 08/13] test/unit: remove support for end-users --- test/unit/aggregator_storage_stats_test.rb | 119 ---------------- test/unit/application_test.rb | 8 +- test/unit/transactor/report_job_test.rb | 6 +- test/unit/transactor/status_test.rb | 16 --- test/unit/user_test.rb | 150 --------------------- test/unit/validators/limits_test.rb | 68 ---------- 6 files changed, 6 insertions(+), 361 deletions(-) delete mode 100644 test/unit/user_test.rb diff --git a/test/unit/aggregator_storage_stats_test.rb b/test/unit/aggregator_storage_stats_test.rb index d99380513..10bd20ffe 100644 --- a/test/unit/aggregator_storage_stats_test.rb +++ b/test/unit/aggregator_storage_stats_test.rb @@ -81,125 +81,6 @@ def bucket_storage assert_equal 0, bucket_storage.pending_buckets_size end - test 'applications with end user plans (user_id) get recorded properly' do - default_user_plan_id = next_id - default_user_plan_name = 'user plan mobile' - timestamp = default_transaction_timestamp - - service = Service.save!(provider_key: @provider_key, id: next_id) - Service.stubs(:load_by_id).returns(service) - service.stubs :user_add - - service.user_registration_required = false - service.default_user_plan_name = default_user_plan_name - service.default_user_plan_id = default_user_plan_id - service.save! - - application = Application.save(service_id: service.id, - id: next_id, - state: :active, - plan_id: @plan_id, - plan_name: @plan_name, - user_required: true) - - transaction = Transaction.new(service_id: service.id, - application_id: application.id, - timestamp: timestamp, - usage: { @metric_id => 5 }, - user_id: 'user_id_xyz') - - Stats::Aggregator.process([transaction]) - - assert_equal '5', @storage.get( - application_key(service.id, application.id, @metric_id, :hour, '2010050713')) - assert_equal '5', @storage.get( - application_key(service.id, application.id, @metric_id, :month, '20100501')) - assert_equal '5', @storage.get( - application_key(service.id, application.id, @metric_id, :eternity)) - - assert_equal '5', @storage.get(service_key(service.id, @metric_id, :hour, '2010050713')) - assert_equal '5', @storage.get(service_key(service.id, @metric_id, :month, '20100501')) - assert_equal '5', @storage.get(service_key(service.id, @metric_id, :eternity)) - - assert_equal '5', @storage.get( - end_user_key(service.id, 'user_id_xyz', @metric_id, :hour, '2010050713')) - assert_equal '5', @storage.get( - end_user_key(service.id, 'user_id_xyz', @metric_id, :month, '20100501')) - assert_equal '5', @storage.get( - end_user_key(service.id, 'user_id_xyz', @metric_id, :eternity)) - - transaction = Transaction.new(service_id: service.id, - application_id: application.id, - timestamp: timestamp, - usage: { @metric_id => 4 }, - user_id: 'another_user_id_xyz') - - Stats::Aggregator.process([transaction]) - - assert_equal '9', @storage.get( - application_key(service.id, application.id, @metric_id, :hour, '2010050713')) - assert_equal '9', @storage.get( - application_key(service.id, application.id, @metric_id, :month, '20100501')) - assert_equal '9', @storage.get( - application_key(service.id, application.id, @metric_id, :eternity)) - - assert_equal '9', @storage.get(service_key(service.id, @metric_id, :hour, '2010050713')) - assert_equal '9', @storage.get(service_key(service.id, @metric_id, :month, '20100501')) - assert_equal '9', @storage.get(service_key(service.id, @metric_id, :eternity)) - - assert_equal '4', @storage.get( - end_user_key(service.id, 'another_user_id_xyz', @metric_id, :hour, '2010050713')) - assert_equal '4', @storage.get( - end_user_key(service.id, 'another_user_id_xyz', @metric_id, :month, '20100501')) - assert_equal '4', @storage.get( - end_user_key(service.id, 'another_user_id_xyz', @metric_id, :eternity)) - end - - - test 'transactions with end_user plans (user_id) with response codes get properly aggregated' do - default_user_plan_id = next_id - default_user_plan_name = 'user plan mobile' - timestamp = default_transaction_timestamp - - service = Service.save!(provider_key: @provider_key, id: next_id) - Service.stubs(:load_by_id).returns(service) - service.stubs :user_add - - service.user_registration_required = false - service.default_user_plan_name = default_user_plan_name - service.default_user_plan_id = default_user_plan_id - service.save! - - application = Application.save(service_id: service.id, - id: next_id, - state: :active, - plan_id: @plan_id, - plan_name: @plan_name, - user_required: true) - - transaction = Transaction.new(service_id: service.id, - application_id: application.id, - timestamp: timestamp, - usage: { @metric_id => 1 }, - response_code: 200, - user_id: 'user_id_xyz') - - Stats::Aggregator.process([transaction]) - - assert_equal '1', @storage.get(end_user_response_code_key( - service.id, 'user_id_xyz', '2XX', :hour, '2010050713')) - assert_equal '1', @storage.get(end_user_response_code_key( - service.id, 'user_id_xyz', '2XX', :month, '20100501')) - assert_equal '1', @storage.get(end_user_response_code_key( - service.id, 'user_id_xyz', '2XX', :eternity)) - assert_equal '1', @storage.get(end_user_response_code_key( - service.id, 'user_id_xyz', '200', :hour, '2010050713')) - assert_equal '1', @storage.get(end_user_response_code_key( - service.id, 'user_id_xyz', '200', :month, '20100501')) - assert_equal '1', @storage.get(end_user_response_code_key( - service.id, 'user_id_xyz', '200', :eternity)) - end - test 'delete all buckets and keys' do timestamp = Time.now.utc - 1000 n_buckets = 5 diff --git a/test/unit/application_test.rb b/test/unit/application_test.rb index 2d6689912..7211c3dcb 100644 --- a/test/unit/application_test.rb +++ b/test/unit/application_test.rb @@ -249,7 +249,7 @@ def setup test 'extract_id! handles access_token' do Application.save(:service_id => '1001', :id => '2001', :state => :active) - OAuth::Token::Storage.create('token', '1001', '2001', nil) + OAuth::Token::Storage.create('token', '1001', '2001') assert_equal '2001', Application.extract_id!('1001', nil, nil, 'token') end @@ -257,7 +257,7 @@ def setup test 'extract_id! fails when access token is not mapped to an app_id' do Application.save(:service_id => '1001', :id => '2001', :state => :active) - OAuth::Token::Storage.create('token', '1001', '2001', nil) + OAuth::Token::Storage.create('token', '1001', '2001') assert_raise AccessTokenInvalid do Application.extract_id!('1001', nil, nil, 'fake-token') @@ -268,7 +268,7 @@ def setup test 'extract_id! fails when access token is mapped to an app_id that does not exist' do Application.save(:service_id => '1001', :id => '2001', :state => :active) - OAuth::Token::Storage.create('token', '1001', 'fake', nil) + OAuth::Token::Storage.create('token', '1001', 'fake') assert_raise ApplicationNotFound do Application.extract_id!('1001', nil, nil, 'token') @@ -281,7 +281,7 @@ def setup Application.save(:service_id => '1001', :id => '2001', :state => :active) Application.save(:service_id => '1001', :id => '3001', :state => :active) - assert OAuth::Token::Storage.create('token', '1001', '3001', nil) + assert OAuth::Token::Storage.create('token', '1001', '3001') assert_equal '3001', Application.extract_id!('1001', nil, nil, 'token') diff --git a/test/unit/transactor/report_job_test.rb b/test/unit/transactor/report_job_test.rb index 39803266c..74ea1b1c2 100644 --- a/test/unit/transactor/report_job_test.rb +++ b/test/unit/transactor/report_job_test.rb @@ -21,8 +21,7 @@ def setup with([{:service_id => @service_id, :application_id => @application_id, :timestamp => nil, - :usage => {@metric_id => 1}, - :user_id => nil}]) + :usage => {@metric_id => 1}}]) Transactor::ReportJob.perform( @service_id, {'0' => {'app_id' => @application_id, 'usage' => {'hits' => 1}}}, Time.now.getutc.to_f, @context_info) @@ -120,8 +119,7 @@ def setup with([{:service_id => @service_id, :application_id => @application_id, :timestamp => nil, - :usage => {@metric_id => 1}, - :user_id => nil}]) + :usage => {@metric_id => 1}}]) Transactor::ReportJob.perform( @service_id, {'0' => {'user_key' => user_key, 'usage' => {'hits' => 1}}}, Time.now.getutc.to_f, @context_info) diff --git a/test/unit/transactor/status_test.rb b/test/unit/transactor/status_test.rb index 50b199cbe..e24b2c690 100644 --- a/test/unit/transactor/status_test.rb +++ b/test/unit/transactor/status_test.rb @@ -249,21 +249,5 @@ def setup assert_equal @application.id, doc.at('status application id').content end - - test '#to_xml shows the username along with the application when OAuth and a User are used' do - usage = {:month => {@metric_id.to_s => 429}} - user = User.new(service_id: @service_id, - username: "testusername") - status = Transactor::Status.new(service_id: @service_id, - application: @application, - values: usage, - oauth: true, - user: user - ) - doc = Nokogiri::XML(status.to_xml) - - assert_equal "testusername", doc.at('status user id').content - assert_equal @application.id, doc.at('status application id').content - end end end diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb deleted file mode 100644 index dee002186..000000000 --- a/test/unit/user_test.rb +++ /dev/null @@ -1,150 +0,0 @@ -require File.expand_path(File.dirname(__FILE__) + '/../test_helper') - -class UserTest < Test::Unit::TestCase - include TestHelpers::Sequences - - def storage - @storage ||= Storage.instance(true) - end - - def setup - storage.flushdb - end - - def test_create_user_errors - service = Service.save! :provider_key => 'foo', :id => 7001001 - - assert_raise ServiceRequiresRegisteredUser do - User.load_or_create!(service, 'username1') - end - - assert_raise UserRequiresDefinedPlan do - User.save!(:username => 'username', :service_id => '7001001') - end - - assert_raise UserRequiresUsername do - User.save!(:service_id => '7001') - end - - assert_raise UserRequiresServiceId do - User.save!(:username => 'username') - end - - assert_raise UserRequiresValidService do - User.save!(:username => 'username', :service_id => '7001001001') - end - end - - def test_create_user_successful_service_require_registered_users - service = Service.save!(provider_key: 'foo', id: '7002') - User.save! username: 'username', service_id: '7002', plan_id: '1001', - plan_name: 'planname' - user = User.load(service.id, 'username') - - assert_equal true, user.active? - assert_equal 'username', user.username - assert_equal 'planname', user.plan_name - assert_equal '1001', user.plan_id - assert_equal '7002', user.service_id - - User.delete! service.id, user.username - - assert_raise ServiceRequiresRegisteredUser do - user = User.load_or_create!(service, 'username') - end - end - - def test_create_user_successful_service_not_require_registered_users - service = Service.save!(provider_key: 'foo', id: '7001', - user_registration_required: false, - default_user_plan_name: 'planname', - default_user_plan_id: '1001') - - names = %w(username0 username1 username2 username3 username4 username5) - names.each_with_index do |username, idx| - user = User.load_or_create!(service, username) - - assert_equal true, user.active? - assert_equal username, user.username - assert_equal service.default_user_plan_name, user.plan_name - assert_equal service.default_user_plan_id, user.plan_id - assert_equal service.id, user.service_id - end - end - - test '#metric_names returns loaded metric names' do - service_id = next_id - plan_id = next_id - metric_id = next_id - metric_name = 'hits' - service = Service.save!(provider_key: 'foo', id: service_id, - user_registration_required: false, - default_user_plan_name: 'user_plan_name', - default_user_plan_id: plan_id) - - user = User.load_or_create!(service, 'user_1') - - Metric.save(service_id: service_id, id: metric_id, name: metric_name) - UsageLimit.save(service_id: service_id, - plan_id: plan_id, - metric_id: metric_id, - minute: 10) - - # No metrics loaded - assert_empty user.metric_names - - user.metric_name(metric_id) - assert_equal({ metric_id => metric_name }, user.metric_names) - end - - test '#load_metric_names loads and returns the names of all the metrics for '\ - 'which there is a usage limit that applies to the app' do - service_id = next_id - plan_id = next_id - metrics = { next_id => 'metric1', next_id => 'metric2' } - service = Service.save!(provider_key: 'foo', id: service_id, - user_registration_required: false, - default_user_plan_name: 'user_plan_name', - default_user_plan_id: plan_id) - - user = User.load_or_create!(service, 'user_1') - - metrics.each do |metric_id, metric_name| - Metric.save(service_id: service_id, id: metric_id, name: metric_name) - UsageLimit.save(service_id: service_id, - plan_id: plan_id, - metric_id: metric_id, - minute: 10) - end - - assert_equal metrics, user.load_metric_names - end - - test '.delete_all removes all the Users of a Service' do - Service.save!(id: 7003, provider_key: 'test_provkey', default_service: false) - Service.save!(id: 7004, provider_key: 'test_provkey', default_service: false) - num_users = Storage::BATCH_SIZE + 1 - num_users.times do |i| - User.save! username: "username#{i+1}", service_id: 7003, plan_id: '1001', - plan_name: 'planname' - end - User.save! username: "username_differentservice", service_id: 7004, - plan_id: '1001',plan_name: 'planname' - - User.delete_all(7003) - - num_users.times do |i| - assert_equal User.exists?(7003, "username#{i+1}"), false - end - assert_equal User.exists?(7004, "username_differentservice"), true - end - - test '.delete_all does not crash when a Service does not have any user' do - Service.save!(id: 7003, provider_key: 'test_provkey', default_service: false) - - assert_nothing_raised do - User.delete_all(7003) - end - - end -end diff --git a/test/unit/validators/limits_test.rb b/test/unit/validators/limits_test.rb index a612de142..f957b2de0 100644 --- a/test/unit/validators/limits_test.rb +++ b/test/unit/validators/limits_test.rb @@ -158,73 +158,5 @@ def setup assert !Limits.apply(status, :usage => {'hits' => 0}) end - - test 'succeeds if there are app and user limits and none are exceeded' do - app_daily_limit = 5 - user_daily_limit = 10 - fixtures = limited_app_and_user!( - @service, @metric_id, app_daily_limit, user_daily_limit) - - status = Transactor::Status.new( - service_id: @service.id, - application: fixtures[:app], - user: fixtures[:user], - values: { day: { @metric_id => app_daily_limit - 1 } }, - user_values: { day: { @metric_id => user_daily_limit - 1 } }) - - assert Limits.apply(status, {}) - end - - test 'fails if there are app and user limits and only app limits are exceeded' do - app_daily_limit = 5 - user_daily_limit = 10 - fixtures = limited_app_and_user!( - @service, @metric_id, app_daily_limit, user_daily_limit) - - status = Transactor::Status.new( - service_id: @service.id, - application: fixtures[:app], - user: fixtures[:user], - values: { day: { @metric_id => app_daily_limit + 1 } }, - user_values: { day: { @metric_id => user_daily_limit - 1 } }) - - assert !Limits.apply(status, {}) - end - - test 'fails if there are app and user limits and only user limits are exceeded' do - app_daily_limit = 5 - user_daily_limit = 10 - fixtures = limited_app_and_user!( - @service, @metric_id, app_daily_limit, user_daily_limit) - - status = Transactor::Status.new( - service_id: @service.id, - application: fixtures[:app], - user: fixtures[:user], - values: { day: { @metric_id => app_daily_limit - 1 } }, - user_values: { day: { @metric_id => user_daily_limit + 1 } }) - - assert !Limits.apply(status, {}) - assert_equal 'limits_exceeded', status.rejection_reason_code - assert_equal 'usage limits are exceeded', status.rejection_reason_text - end - - test 'fails if there are app and user limits and both are exceeded' do - app_daily_limit = 5 - user_daily_limit = 10 - fixtures = limited_app_and_user!( - @service, @metric_id, app_daily_limit, user_daily_limit) - - status = Transactor::Status.new( - service_id: @service.id, - application: fixtures[:app], - user: fixtures[:user], - values: { day: { @metric_id => app_daily_limit + 1 } }, - user_values: { day: { @metric_id => user_daily_limit + 1 } }) - - assert !Limits.apply(status, {}) - assert_equal 'limits_exceeded', status.rejection_reason_code - assert_equal 'usage limits are exceeded', status.rejection_reason_text - end end end From cae2ed9d2d192a1db168cb2d7a4fe1c38c5f36bd Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Wed, 18 Sep 2019 22:35:49 +0200 Subject: [PATCH 09/13] test/test_helpers: remove support for end-users --- test/test_helpers/fixtures.rb | 38 ------------------------------- test/test_helpers/storage_keys.rb | 8 ------- 2 files changed, 46 deletions(-) diff --git a/test/test_helpers/fixtures.rb b/test/test_helpers/fixtures.rb index 977986427..9f57348ce 100644 --- a/test/test_helpers/fixtures.rb +++ b/test/test_helpers/fixtures.rb @@ -172,44 +172,6 @@ def setup_provider_without_default_service end end - # Given a service and a metric ID, creates a user and an app for that - # service and creates usage limits with different max for both. - # Returns a hash with 2 keys: :user, and :app. - # Side effect: modifies the given service so we can register users - def limited_app_and_user!(service, metric_id, app_daily_limit, user_daily_limit) - # Set up the service so we can register users - service.user_registration_required = false - service.default_user_plan_name = 'default_user_plan' - service.default_user_plan_id = next_id - service.save! - - # Set up the app and its limits - app_plan_id = next_id - UsageLimit.save(service_id: service.id, - plan_id: app_plan_id, - metric_id: metric_id, - day: app_daily_limit) - app = Application.save(service_id: service.id, - id: next_id, - state: :active, - plan_id: app_plan_id, - user_required: true) - - # Set up the user and its limits - user_plan_id = next_id - UsageLimit.save(service_id: service.id, - plan_id: user_plan_id, - metric_id: metric_id, - day: user_daily_limit) - user = User.new(service_id: service.id, - username: 'Bob', - state: :active, - plan_id: user_plan_id) - user.save - - { app: app, user: user } - end - # Sets a service with a metric hierarchy with the number of levels specified. # Generates only one metric on each level. # diff --git a/test/test_helpers/storage_keys.rb b/test/test_helpers/storage_keys.rb index 0c5e2e6df..4aad2bc8d 100644 --- a/test/test_helpers/storage_keys.rb +++ b/test/test_helpers/storage_keys.rb @@ -2,10 +2,6 @@ module TestHelpers module StorageKeys private - def end_user_key(service_id, user_id, metric_id, period, time = nil) - "stats/{service:#{service_id}}/uinstance:#{user_id}/metric:#{metric_id}/#{period_part(period, time)}" - end - def application_key(service_id, application_id, metric_id, period, time = nil) "stats/{service:#{service_id}}/cinstance:#{application_id}/metric:#{metric_id}/#{period_part(period, time)}" end @@ -22,10 +18,6 @@ def app_response_code_key(service_id, application_id, response_code, period, tim "stats/{service:#{service_id}}/cinstance:#{application_id}/response_code:#{response_code}/#{period_part(period, time)}" end - def end_user_response_code_key(service_id, user_id, response_code, period, time = nil) - "stats/{service:#{service_id}}/uinstance:#{user_id}/response_code:#{response_code}/#{period_part(period, time)}" - end - def period_part(period, time = nil) if period == :eternity 'eternity' From 3240d11a39ad353110e02d22b7988540d2c84a25 Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Wed, 18 Sep 2019 22:53:41 +0200 Subject: [PATCH 10/13] test/integration: remove support for end-users --- test/integration/authorize/basic_test.rb | 61 --- .../authorize/set_user_usage_test.rb | 178 --------- test/integration/authrep/basic_test.rb | 61 --- .../authrep/set_user_usage_test.rb | 191 --------- test/integration/authrep/user_id_test.rb | 367 ------------------ test/integration/background_reports_test.rb | 57 --- test/integration/oauth/access_token_test.rb | 265 +------------ .../oauth/application_keys_test.rb | 21 - test/integration/oauth/basic_test.rb | 61 --- .../basic_test_with_access_tokens_test.rb | 46 +-- test/integration/oauth/set_user_usage_test.rb | 179 --------- test/integration/report_test.rb | 37 -- 12 files changed, 12 insertions(+), 1512 deletions(-) delete mode 100644 test/integration/authorize/set_user_usage_test.rb delete mode 100644 test/integration/authrep/set_user_usage_test.rb delete mode 100644 test/integration/authrep/user_id_test.rb delete mode 100644 test/integration/oauth/set_user_usage_test.rb diff --git a/test/integration/authorize/basic_test.rb b/test/integration/authorize/basic_test.rb index bfa92a562..5ec033a25 100644 --- a/test/integration/authorize/basic_test.rb +++ b/test/integration/authorize/basic_test.rb @@ -659,65 +659,4 @@ def setup assertions.call false end end - - test 'authorizes if there are app and user limits and none are exceeded' do - app_daily_limit = 10 - user_daily_limit = 2 - fixtures = limited_app_and_user!( - @service, @metric_id, app_daily_limit, user_daily_limit) - - get '/transactions/authorize.xml', :provider_key => @provider_key, - :service_id => @service.id, - :app_id => fixtures[:app].id, - :user_id => fixtures[:user].username, - :usage => { 'hits' => 0 } - - assert_authorized - end - - test 'denies if there are app and user limits and only app limits are exceeded' do - app_daily_limit = 10 - hits_to_report = app_daily_limit + 1 - user_daily_limit = hits_to_report + 1 - fixtures = limited_app_and_user!( - @service, @metric_id, app_daily_limit, user_daily_limit) - - get '/transactions/authorize.xml', :provider_key => @provider_key, - :service_id => @service.id, - :app_id => fixtures[:app].id, - :user_id => fixtures[:user].username, - :usage => { 'hits' => hits_to_report } - - assert_not_authorized 'usage limits are exceeded' - end - - test 'denies auth if there are app and user limits and only user limits are exceeded' do - user_daily_limit = 10 - hits_to_report = user_daily_limit + 1 - app_daily_limit = hits_to_report + 1 - fixtures = limited_app_and_user!( - @service, @metric_id, app_daily_limit, user_daily_limit) - - get '/transactions/authorize.xml', :provider_key => @provider_key, - :service_id => @service.id, - :app_id => fixtures[:app].id, - :user_id => fixtures[:user].username, - :usage => { 'hits' => app_daily_limit + 1 } - - assert_not_authorized('usage limits are exceeded') - end - - test 'denies auth if there are app and user limits and both are exceeded' do - app_daily_limit = user_daily_limit = 1 - fixtures = limited_app_and_user!( - @service, @metric_id, app_daily_limit, user_daily_limit) - - get '/transactions/authorize.xml', :provider_key => @provider_key, - :service_id => @service.id, - :app_id => fixtures[:app].id, - :user_id => fixtures[:user].username, - :usage => { 'hits' => app_daily_limit + 1 } - - assert_not_authorized('usage limits are exceeded') - end end diff --git a/test/integration/authorize/set_user_usage_test.rb b/test/integration/authorize/set_user_usage_test.rb deleted file mode 100644 index 62f240391..000000000 --- a/test/integration/authorize/set_user_usage_test.rb +++ /dev/null @@ -1,178 +0,0 @@ -require File.expand_path(File.dirname(__FILE__) + '/../../test_helper') - -class AuthorizeSetUserUsageTest < Test::Unit::TestCase - include TestHelpers::AuthorizeAssertions - include TestHelpers::Fixtures - include TestHelpers::Integration - - def setup - @storage = Storage.instance(true) - @storage.flushdb - - Resque.reset! - Memoizer.reset! - - setup_provider_fixtures - - @default_user_plan_id = next_id - @default_user_plan_name = 'user plan mobile' - - @service.user_registration_required = false - @service.default_user_plan_name = @default_user_plan_name - @service.default_user_plan_id = @default_user_plan_id - @service.save! - - @application = Application.save(:service_id => @service.id, - :id => next_id, - :state => :active, - :plan_id => @plan_id, - :plan_name => @plan_name, - :user_required => true) - - @metric_id = next_id - - Metric.save(:service_id => @service.id, :id => @metric_id, :name => 'hits') - - UsageLimit.save(:service_id => @service.id, - :plan_id => @default_user_plan_id, - :metric_id => @metric_id, - :day => 100, :month => 1000, :eternity => 10000) - end - - test 'check behavior of set values on usage passed as parameter of authorize' do - Timecop.freeze(Time.utc(2011, 1, 1)) do - post '/transactions.xml', - :provider_key => @provider_key, - :transactions => {0 => {:app_id => @application.id, :user_id => 'user1', :usage => {'hits' => '#99'}}} - Resque.run! - - post '/transactions.xml', - :provider_key => @provider_key, - :transactions => {0 => {:app_id => @application.id, :user_id => 'user2', :usage => {'hits' => '#98'}}} - Resque.run! - - get '/transactions/authorize.xml', :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user1', - :usage => {'hits' => 2} - - assert_user_usage_report(Time.utc(2011, 1, 1, 13, 0, 0), 'hits', 'day', 99, 100) - assert_not_authorized 'usage limits are exceeded' - - get '/transactions/authorize.xml', :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user2', - :usage => {'hits' => 2} - - assert_user_usage_report(Time.utc(2011, 1, 1, 13, 0, 0), 'hits', 'day', 98, 100) - assert_authorized - end - - Timecop.freeze(Time.utc(2011, 1, 1)) do - post '/transactions.xml', - :provider_key => @provider_key, - :transactions => {0 => {:app_id => @application.id, :user_id => 'user1', :usage => {'hits' => 2}}, - 1 => {:app_id => @application.id, :user_id => 'user2', :usage => {'hits' => 2}}} - Resque.run! - - get '/transactions/authorize.xml', :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user1', - :usage => {'hits' => 2} - - assert_user_usage_report(Time.utc(2011, 1, 1, 13, 0, 0), 'hits', 'day', 101, 100) - assert_not_authorized 'usage limits are exceeded' - - get '/transactions/authorize.xml', :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user2', - :usage => {'hits' => 2} - - assert_user_usage_report(Time.utc(2011, 1, 1, 13, 0, 0), 'hits', 'day', 100, 100) - assert_not_authorized 'usage limits are exceeded' - - get '/transactions/authorize.xml', :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user2' - - assert_user_usage_report(Time.utc(2011, 1, 1, 13, 0, 0), 'hits', 'day', 100, 100) - assert_authorized - end - - Timecop.freeze(Time.utc(2011, 1, 1)) do - get '/transactions/authorize.xml', :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user1', - :usage => {'hits' => '#66'} - - assert_user_usage_report(Time.utc(2011, 1, 1, 13, 0, 0), 'hits', 'day', 101, 100) - assert_authorized - - get '/transactions/authorize.xml', :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user2', - :usage => {'hits' => '#100'} - - assert_user_usage_report(Time.utc(2011, 1, 1, 13, 0, 0), 'hits', 'day', 100, 100) - assert_authorized - end - end - - test 'check behaviour of set values with caching and authorize' do - Timecop.freeze(Time.utc(2011, 1, 1)) do - post '/transactions.xml', - :provider_key => @provider_key, - :transactions => {0 => {:app_id => @application.id, :user_id => 'user1', :usage => {'hits' => 80}}} - Resque.run! - - 10.times do |cont| - if cont.odd? - get '/transactions/authorize.xml', :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user1', - :usage => {'hits' => "##{80 + (cont)*2}"} - else - get '/transactions/authorize.xml', :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user1', - :usage => {'hits' => 2} - end - Resque.run! - - assert_user_usage_report(Time.utc(2011, 1, 1, 13, 0, 0), 'hits', 'day', 80 + (cont)*2, 100) - assert_authorized - - post '/transactions.xml', - :provider_key => @provider_key, - :transactions => {0 => {:app_id => @application.id, :user_id => 'user1', :usage => {'hits' => 2}}} - Resque.run! - end - - 10.times do |cont| - get '/transactions/authorize.xml', :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user1', - :usage => {'hits' => 2} - Resque.run! - - assert_user_usage_report(Time.utc(2011, 1, 1, 13, 0, 0), 'hits', 'day', 100, 100) - assert_not_authorized 'usage limits are exceeded' - end - - 10.times do |cont| - get '/transactions/authorize.xml', :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user1', - :usage => {'hits' => "##{100 + (cont+1)*-2}"} - - assert_user_usage_report(Time.utc(2011, 1, 1, 13, 0, 0), 'hits', 'day', 100 + (cont)*-2, 100) - assert_authorized - - post '/transactions.xml', - :provider_key => @provider_key, - :transactions => {0 => {:app_id => @application.id, :user_id => 'user1', :usage => {'hits' => "##{100 + (cont+1)*-2}"}}} - Resque.run! - end - end - end -end diff --git a/test/integration/authrep/basic_test.rb b/test/integration/authrep/basic_test.rb index 42c739edf..d54b5ee39 100644 --- a/test/integration/authrep/basic_test.rb +++ b/test/integration/authrep/basic_test.rb @@ -877,65 +877,4 @@ def setup assert_equal 409, last_response.status assert_nil last_response.header['3scale-rejection-reason'] end - - test_authrep 'authorizes if there are app and user limits and none are exceeded' do |e| - app_daily_limit = 10 - user_daily_limit = 2 - fixtures = limited_app_and_user!( - @service, @metric_id, app_daily_limit, user_daily_limit) - - get e, :provider_key => @provider_key, - :service_id => @service.id, - :app_id => fixtures[:app].id, - :user_id => fixtures[:user].username, - :usage => { 'hits' => 0 } - - assert_authorized - end - - test_authrep 'denies auth if there are app and user limits and only app limits are exceeded' do |e| - app_daily_limit = 10 - hits_to_report = app_daily_limit + 1 - user_daily_limit = hits_to_report + 1 - fixtures = limited_app_and_user!( - @service, @metric_id, app_daily_limit, user_daily_limit) - - get e, :provider_key => @provider_key, - :service_id => @service.id, - :app_id => fixtures[:app].id, - :user_id => fixtures[:user].username, - :usage => { 'hits' => hits_to_report } - - assert_not_authorized 'usage limits are exceeded' - end - - test_authrep 'denies auth if there are app and user limits and only user limits are exceeded' do |e| - user_daily_limit = 10 - hits_to_report = user_daily_limit + 1 - app_daily_limit = hits_to_report + 1 - fixtures = limited_app_and_user!( - @service, @metric_id, app_daily_limit, user_daily_limit) - - get e, :provider_key => @provider_key, - :service_id => @service.id, - :app_id => fixtures[:app].id, - :user_id => fixtures[:user].username, - :usage => { 'hits' => app_daily_limit + 1 } - - assert_not_authorized('usage limits are exceeded') - end - - test_authrep 'denies auth if there are app and user limits and both are exceeded' do |e| - app_daily_limit = user_daily_limit = 1 - fixtures = limited_app_and_user!( - @service, @metric_id, app_daily_limit, user_daily_limit) - - get e, :provider_key => @provider_key, - :service_id => @service.id, - :app_id => fixtures[:app].id, - :user_id => fixtures[:user].username, - :usage => { 'hits' => app_daily_limit + 1 } - - assert_not_authorized('usage limits are exceeded') - end end diff --git a/test/integration/authrep/set_user_usage_test.rb b/test/integration/authrep/set_user_usage_test.rb deleted file mode 100644 index 2d639e346..000000000 --- a/test/integration/authrep/set_user_usage_test.rb +++ /dev/null @@ -1,191 +0,0 @@ -require File.expand_path(File.dirname(__FILE__) + '/../../test_helper') - -class AuthrepSetUserUsageTest < Test::Unit::TestCase - include TestHelpers::AuthorizeAssertions - include TestHelpers::Fixtures - include TestHelpers::Integration - - include TestHelpers::AuthRep - - def setup - @storage = Storage.instance(true) - @storage.flushdb - - Resque.reset! - Memoizer.reset! - - setup_oauth_provider_fixtures - - @default_user_plan_id = next_id - @default_user_plan_name = 'user plan mobile' - - @service.user_registration_required = false - @service.default_user_plan_name = @default_user_plan_name - @service.default_user_plan_id = @default_user_plan_id - @service.save! - - @application = Application.save(:service_id => @service.id, - :id => next_id, - :state => :active, - :plan_id => @plan_id, - :plan_name => @plan_name, - :user_required => true) - - @metric_id = next_id - - Metric.save(:service_id => @service.id, :id => @metric_id, :name => 'hits') - - UsageLimit.save(:service_id => @service.id, - :plan_id => @default_user_plan_id, - :metric_id => @metric_id, - :day => 100, :month => 1000, :eternity => 10000) - end - - test_authrep 'check behavior of set values when passed as usage on authrep' do |e| - Timecop.freeze(Time.utc(2011, 1, 1)) do - post '/transactions.xml', - :provider_key => @provider_key, - :transactions => {0 => {:app_id => @application.id, :user_id => 'user1', :usage => {'hits' => '#97'}}, - 1 => {:app_id => @application.id, :user_id => 'user2', :usage => {'hits' => '#96'}}} - Resque.run! - - get e, :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user1', - :usage => {'hits' => 1} - Resque.run! - - get e, :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user1', - :usage => {'hits' => 1} - Resque.run! - - assert_user_usage_report(Time.utc(2011, 1, 1, 13, 0, 0), 'hits', 'day', 99, 100) - assert_authorized - end - - Timecop.freeze(Time.utc(2011, 1, 1)) do - get e, :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user1', - :usage => {'hits' => '#101'} - Resque.run! - - assert_user_usage_report(Time.utc(2011, 1, 1, 13, 0, 0), 'hits', 'day', 99, 100) - assert_not_authorized 'usage limits are exceeded' - end - - Timecop.freeze(Time.utc(2011, 1, 1)) do - post '/transactions.xml', - :provider_key => @provider_key, - :transactions => {0 => {:app_id => @application.id, :user_id => 'user1', :usage => {'hits' => 2}}, - 1 => {:app_id => @application.id, :user_id => 'user2', :usage => {'hits' => 2}}} - Resque.run! - - get e, :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user1', - :usage => {'hits' => 2} - Resque.run! - - assert_user_usage_report(Time.utc(2011, 1, 1, 13, 0, 0), 'hits', 'day', 101, 100) - assert_not_authorized 'usage limits are exceeded' - - get e, :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user2', - :usage => {'hits' => 2} - Resque.run! - - assert_user_usage_report(Time.utc(2011, 1, 1, 13, 0, 0), 'hits', 'day', 100, 100) - assert_authorized - end - - Timecop.freeze(Time.utc(2011, 1, 1)) do - get e, :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user1', - :usage => {'hits' => '#99'} - Resque.run! - - assert_user_usage_report(Time.utc(2011, 1, 1, 13, 0, 0), 'hits', 'day', 99, 100) - assert_authorized - - get e, :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user2', - :usage => {'hits' => '#98'} - Resque.run! - - assert_user_usage_report(Time.utc(2011, 1, 1, 13, 0, 0), 'hits', 'day', 98, 100) - assert_authorized - - get e, :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user2', - :usage => {'hits' => '#0'} - Resque.run! - - assert_user_usage_report(Time.utc(2011, 1, 1, 13, 0, 0), 'hits', 'day', 0, 100) - assert_authorized - end - end - - test_authrep 'check behaviour of set values with caching and authrep' do |e| - Timecop.freeze(Time.utc(2011, 1, 1)) do - post '/transactions.xml', - :provider_key => @provider_key, - :transactions => {0 => {:app_id => @application.id, :usage => {'hits' => 80}, :user_id => 'user1'}} - Resque.run! - - 10.times do |cont| - if cont.even? - get e, :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user1', - :usage => {'hits' => "##{80 + (cont+1)*2}"} - else - get e, :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user1', - :usage => {'hits' => '2'} - end - Resque.run! - - assert_user_usage_report(Time.utc(2011, 1, 1, 13, 0, 0), 'hits', 'day', 80 + (cont+1)*2, 100) - assert_authorized - end - - get e, :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user1' - Resque.run! - - assert_user_usage_report(Time.utc(2011, 1, 1, 13, 0, 0), 'hits', 'day', 100, 100) - assert_authorized - - 10.times do |cont| - get e, :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user1', - :usage => {'hits' => 2} - Resque.run! - - assert_user_usage_report(Time.utc(2011, 1, 1, 13, 0, 0), 'hits', 'day', 100, 100) - assert_not_authorized 'usage limits are exceeded' - end - - 10.times do |cont| - get e, :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user1', - :usage => {'hits' => "##{100 + (cont+1)*-2}"} - Resque.run! - - assert_user_usage_report(Time.utc(2011, 1, 1, 13, 0, 0), 'hits', 'day', 100 + (cont+1)*-2, 100) - assert_authorized - end - end - end -end diff --git a/test/integration/authrep/user_id_test.rb b/test/integration/authrep/user_id_test.rb deleted file mode 100644 index 381e50fa6..000000000 --- a/test/integration/authrep/user_id_test.rb +++ /dev/null @@ -1,367 +0,0 @@ -require File.expand_path(File.dirname(__FILE__) + '/../../test_helper') - -class AuthrepUserIdTest < Test::Unit::TestCase - include TestHelpers::AuthorizeAssertions - include TestHelpers::Fixtures - include TestHelpers::Integration - include TestHelpers::StorageKeys - - include TestHelpers::AuthRep - - def setup - @storage = Storage.instance(true) - @storage.flushdb - - Resque.reset! - Memoizer.reset! - - setup_oauth_provider_fixtures - - @default_user_plan_id = next_id - @default_user_plan_name = 'user plan mobile' - - @service.user_registration_required = false - @service.default_user_plan_name = @default_user_plan_name - @service.default_user_plan_id = @default_user_plan_id - @service.save! - - @application_with_users = Application.save(:service_id => @service.id, - :id => next_id, - :state => :active, - :plan_id => @plan_id, - :plan_name => @plan_name, - :user_required => true) - - @application_without_users = Application.save(:service_id => @service.id, - :id => next_id, - :state => :active, - :plan_id => @plan_id, - :plan_name => @plan_name) - - @hits = Metric.save(:service_id => @service.id, :id => next_id, :name => 'hits') - @foo = Metric.save(:service_id => @service.id, :id => next_id, :name => 'foo') - - @ul1 = UsageLimit.save(:service_id => @service.id, - :plan_id => @plan_id, - :metric_id => @hits.id, - :day => 100) - - UsageLimit.save(:service_id => @service.id, - :plan_id => @default_user_plan_id, - :metric_id => @hits.id, - :day => 20) - - UsageLimit.save(:service_id => @service.id, - :plan_id => @default_user_plan_id, - :metric_id => @foo.id, - :day => 10) - end - - test_authrep 'application without end user required does not return user usage reports' do |e| - get e, :provider_key => @provider_key, - :app_id => @application_without_users.id, - :usage => {'hits' => 3} - Resque.run! - - assert_equal 200, last_response.status - doc = Nokogiri::XML(last_response.body) - usage_reports = doc.at('usage_reports') - assert_not_nil usage_reports - day = usage_reports.at('usage_report[metric = "hits"][period = "day"]') - assert_not_nil day - assert_equal '3', day.at('current_value').content - usage_reports = doc.at('user_usage_reports') - assert_nil usage_reports - assert_nil doc.at('user_plan') - - last_doc = doc - - get e, :provider_key => @provider_key, - :app_id => @application_without_users.id, - :usage => {'hits' => 0}, - :user_id => 'random user id' - Resque.run! - - assert_equal 200, last_response.status - doc = Nokogiri::XML(last_response.body) - assert_equal last_doc.to_xml, doc.to_xml - end - - test_authrep 'user usage report depends on the user_id, whereas usage report does not' do |e| - get e, :provider_key => @provider_key, - :app_id => @application_with_users.id, - :usage => {'hits' => 3}, - :user_id => 'user1' - Resque.run! - - assert_equal 200, last_response.status - doc = Nokogiri::XML(last_response.body) - usage_reports = doc.at('usage_reports') - assert_not_nil usage_reports - day = usage_reports.at('usage_report[metric = "hits"][period = "day"]') - assert_not_nil day - assert_equal '3', day.at('current_value').content - usage_reports = doc.at('user_usage_reports') - assert_not_nil usage_reports - day = usage_reports.at('usage_report[metric = "hits"][period = "day"]') - assert_not_nil day - assert_equal '3', day.at('current_value').content - day = usage_reports.at('usage_report[metric = "foo"][period = "day"]') - assert_not_nil day - assert_equal '0', day.at('current_value').content - assert_equal @default_user_plan_name, doc.at('user_plan').content - - get e, :provider_key => @provider_key, - :app_id => @application_with_users.id, - :usage => {'hits' => 3}, - :user_id => 'user1' - Resque.run! - - assert_equal 200, last_response.status - doc = Nokogiri::XML(last_response.body) - usage_reports = doc.at('usage_reports') - assert_not_nil usage_reports - day = usage_reports.at('usage_report[metric = "hits"][period = "day"]') - assert_not_nil day - assert_equal '6', day.at('current_value').content - usage_reports = doc.at('user_usage_reports') - assert_not_nil usage_reports - day = usage_reports.at('usage_report[metric = "hits"][period = "day"]') - assert_not_nil day - assert_equal '6', day.at('current_value').content - day = usage_reports.at('usage_report[metric = "foo"][period = "day"]') - assert_not_nil day - assert_equal '0', day.at('current_value').content - assert_equal @default_user_plan_name, doc.at('user_plan').content - - get e, :provider_key => @provider_key, - :app_id => @application_with_users.id, - :usage => {'hits' => 3}, - :user_id => 'user2' - Resque.run! - - assert_equal 200, last_response.status - doc = Nokogiri::XML(last_response.body) - usage_reports = doc.at('usage_reports') - assert_not_nil usage_reports - day = usage_reports.at('usage_report[metric = "hits"][period = "day"]') - assert_not_nil day - assert_equal '9', day.at('current_value').content - usage_reports = doc.at('user_usage_reports') - assert_not_nil usage_reports - day = usage_reports.at('usage_report[metric = "hits"][period = "day"]') - assert_not_nil day - assert_equal '3', day.at('current_value').content - day = usage_reports.at('usage_report[metric = "foo"][period = "day"]') - assert_not_nil day - assert_equal '0', day.at('current_value').content - assert_equal @default_user_plan_name, doc.at('user_plan').content - - get e, :provider_key => @provider_key, - :app_id => @application_with_users.id, - :usage => {'foo' => 3}, - :user_id => 'user3' - Resque.run! - - assert_equal 200, last_response.status - doc = Nokogiri::XML(last_response.body) - usage_reports = doc.at('usage_reports') - assert_not_nil usage_reports - day = usage_reports.at('usage_report[metric = "hits"][period = "day"]') - assert_not_nil day - assert_equal '9', day.at('current_value').content - usage_reports = doc.at('user_usage_reports') - assert_not_nil usage_reports - day = usage_reports.at('usage_report[metric = "hits"][period = "day"]') - assert_not_nil day - assert_equal '0', day.at('current_value').content - day = usage_reports.at('usage_report[metric = "foo"][period = "day"]') - assert_not_nil day - assert_equal '3', day.at('current_value').content - assert_equal @default_user_plan_name, doc.at('user_plan').content - end - - test_authrep 'user usage reports are across service, independant of application' do |e| - @application_with_users2 = Application.save(:service_id => @service.id, - :id => next_id, - :state => :active, - :plan_id => @plan_id, - :plan_name => @plan_name, - :user_required => true) - - get e, :provider_key => @provider_key, - :app_id => @application_with_users.id, - :usage => {'hits' => 3}, - :user_id => 'user1' - Resque.run! - - assert_equal 200, last_response.status - doc = Nokogiri::XML(last_response.body) - usage_reports = doc.at('usage_reports') - assert_not_nil usage_reports - day = usage_reports.at('usage_report[metric = "hits"][period = "day"]') - assert_not_nil day - assert_equal '3', day.at('current_value').content - usage_reports = doc.at('user_usage_reports') - assert_not_nil usage_reports - day = usage_reports.at('usage_report[metric = "hits"][period = "day"]') - assert_not_nil day - assert_equal '3', day.at('current_value').content - day = usage_reports.at('usage_report[metric = "foo"][period = "day"]') - assert_not_nil day - assert_equal '0', day.at('current_value').content - assert_equal @default_user_plan_name, doc.at('user_plan').content - - get e, :provider_key => @provider_key, - :app_id => @application_with_users2.id, - :usage => {'hits' => 3}, - :user_id => 'user1' - Resque.run! - - assert_equal 200, last_response.status - doc = Nokogiri::XML(last_response.body) - usage_reports = doc.at('usage_reports') - assert_not_nil usage_reports - day = usage_reports.at('usage_report[metric = "hits"][period = "day"]') - assert_not_nil day - assert_equal '3', day.at('current_value').content - usage_reports = doc.at('user_usage_reports') - assert_not_nil usage_reports - day = usage_reports.at('usage_report[metric = "hits"][period = "day"]') - assert_not_nil day - assert_equal '6', day.at('current_value').content - day = usage_reports.at('usage_report[metric = "foo"][period = "day"]') - assert_not_nil day - assert_equal '0', day.at('current_value').content - assert_equal @default_user_plan_name, doc.at('user_plan').content - end - - test_authrep 'application with users required fails if user_id is not passed or not valid' do |e| - get e, :provider_key => @provider_key, - :app_id => @application_with_users.id, - :usage => {'hits' => 3} - Resque.run! - - assert_equal 403, last_response.status - doc = Nokogiri::XML(last_response.body) - assert_equal 'user_not_defined', doc.at('error')[:code] - - get e, :provider_key => @provider_key, - :app_id => @application_with_users.id, - :usage => {'hits' => 3}, - :user_id => '' - Resque.run! - - assert_equal 403, last_response.status - doc = Nokogiri::XML(last_response.body) - assert_equal 'user_not_defined', doc.at('error')[:code] - - ## watch out, ' ' could be a valid user_id - - get e, :provider_key => @provider_key, - :app_id => @application_with_users.id, - :usage => {'hits' => 3}, - :user_id => nil - Resque.run! - - assert_equal 403, last_response.status - doc = Nokogiri::XML(last_response.body) - assert_equal 'user_not_defined', doc.at('error')[:code] - - get e, :provider_key => @provider_key, - :app_id => @application_with_users.id, - :usage => {'hits' => 3}, - :user_id => {} - Resque.run! - - assert_equal 403, last_response.status - doc = Nokogiri::XML(last_response.body) - assert_equal 'user_not_defined', doc.at('error')[:code] - - get e, :provider_key => @provider_key, - :app_id => @application_with_users.id, - :usage => {'hits' => 3}, - :user_id => {'foo' => 'bar'} - Resque.run! - - assert_equal 403, last_response.status - doc = Nokogiri::XML(last_response.body) - assert_equal 'user_not_defined', doc.at('error')[:code] - - get e, :provider_key => @provider_key, - :app_id => @application_with_users.id, - :usage => {'hits' => 3}, - :user_id => ['bla', 'foo'] - Resque.run! - - assert_equal 403, last_response.status - doc = Nokogiri::XML(last_response.body) - assert_equal 'user_not_defined', doc.at('error')[:code] - - ## all number gets treated as string in the backend, it's ok - get e, :provider_key => @provider_key, - :app_id => @application_with_users.id, - :usage => {'hits' => 3}, - :user_id => 11111 - Resque.run! - - assert_equal 200, last_response.status - doc = Nokogiri::XML(last_response.body) - end - - test_authrep 'user plans does not need to have limits' do |e| - user = User.load_or_create!(@service, 'user1') - assert_equal @default_user_plan_id, user.plan_id - assert_equal @default_user_plan_name, user.plan_name - - new_user_plan_id = next_id - user.plan_id = new_user_plan_id - user.plan_name = 'new name of user plan' - user.save - - user = User.load_or_create!(@service, 'user1') - assert_equal new_user_plan_id, user.plan_id - assert_equal 'new name of user plan', user.plan_name - - get e, :provider_key => @provider_key, - :app_id => @application_with_users.id, - :usage => {'hits' => 3}, - :user_id => 'user1' - Resque.run! - - assert_equal 200, last_response.status - doc = Nokogiri::XML(last_response.body) - usage_reports = doc.at('usage_reports') - assert_not_nil usage_reports - day = usage_reports.at('usage_report[metric = "hits"][period = "day"]') - assert_not_nil day - assert_equal '3', day.at('current_value').content - usage_reports = doc.at('user_usage_reports') - assert_nil usage_reports - assert_equal 'new name of user plan', doc.at('user_plan').content - - ## but that does not affect other users - get e, :provider_key => @provider_key, - :app_id => @application_with_users.id, - :usage => {'hits' => 3}, - :user_id => 'user2' - Resque.run! - - assert_equal 200, last_response.status - doc = Nokogiri::XML(last_response.body) - usage_reports = doc.at('usage_reports') - assert_not_nil usage_reports - day = usage_reports.at('usage_report[metric = "hits"][period = "day"]') - assert_not_nil day - assert_equal '6', day.at('current_value').content - usage_reports = doc.at('user_usage_reports') - assert_not_nil usage_reports - day = usage_reports.at('usage_report[metric = "hits"][period = "day"]') - assert_not_nil day - assert_equal '3', day.at('current_value').content - day = usage_reports.at('usage_report[metric = "foo"][period = "day"]') - assert_not_nil day - assert_equal '0', day.at('current_value').content - assert_equal @default_user_plan_name, doc.at('user_plan').content - end -end diff --git a/test/integration/background_reports_test.rb b/test/integration/background_reports_test.rb index fd5b13424..6d7d3d629 100644 --- a/test/integration/background_reports_test.rb +++ b/test/integration/background_reports_test.rb @@ -58,61 +58,4 @@ def setup :metric_id => @metric_id_3, :day => 100) end - - test 'fails when sending user_id when service does not support user plans' do - post '/transactions.xml', - :provider_key => @provider_key, - :service_id => @service_1.id, - :transactions => {0 => {:app_id => @application_1.id, :usage => {'hits' => 3}}} - - assert_equal 202, last_response.status - Resque.run! - - get '/transactions/authorize.xml', :provider_key => @provider_key, - :app_id => @application_1.id, - :service_id => @service_1.id - - assert_equal 200, last_response.status - - doc = Nokogiri::XML(last_response.body) - usage_reports = doc.at('usage_reports') - assert_not_nil usage_reports - - day = usage_reports.at('usage_report[metric = "hits"][period = "day"]') - assert_not_nil day - assert_equal '3', day.at('current_value').content - - - # Report a transaction with a user ID. End user plans are not enabled for - # the service, so there will be an integration error and the hits reported - # will not be added. - - post '/transactions.xml', - :provider_key => @provider_key, - :service_id => @service_1.id, - :transactions => {0 => {:user_id => "user_id1", :app_id => @application_1.id, :usage => {'hits' => 3}}} - - assert_equal 202, last_response.status - - Resque.run! - - integration_error = ErrorStorage.list(@service_1.id).first - assert_equal ServiceCannotUseUserId.code, integration_error[:code] - - get '/transactions/authorize.xml', :provider_key => @provider_key, - :app_id => @application_1.id, - :service_id => @service_1.id - - assert_equal 200, last_response.status - - doc = Nokogiri::XML(last_response.body) - usage_reports = doc.at('usage_reports') - assert_not_nil usage_reports - - day = usage_reports.at('usage_report[metric = "hits"][period = "day"]') - assert_not_nil day - - # Hits were not counted - assert_equal '3', day.at('current_value').content - end end diff --git a/test/integration/oauth/access_token_test.rb b/test/integration/oauth/access_token_test.rb index a747159cc..4a970c388 100644 --- a/test/integration/oauth/access_token_test.rb +++ b/test/integration/oauth/access_token_test.rb @@ -24,7 +24,6 @@ def setup :plan_id => @plan_id, :plan_name => @plan_name) - @user = User.save!(service_id: @service.id, username: 'pantxo', plan_id: '1', plan_name: 'plan') end test 'CR(U)D oauth_access_token' do @@ -45,7 +44,6 @@ def setup assert_equal 1, node.count assert_equal 'VALID-TOKEN', node.content assert_equal '-1', node.attribute('ttl').value - assert node.attribute('user_id').nil? # Read an invalid app id get "/services/#{@service.id}/applications/#{@application.id.succ}/oauth_access_tokens.xml", @@ -73,164 +71,6 @@ def setup assert_error_resp_with_exc(ApplicationNotFound.new @application.id.succ) end - test 'CR(U)D oauth_access_token tied to a specified user' do - user_id = @user.username - other_id = @user.username + '_other' - user_token = 'USER-TOKEN' - other_token = 'OTHER-USER-TOKEN' - - # Create user token - post "/services/#{@service.id}/oauth_access_tokens.xml", :provider_key => @provider_key, - :app_id => @application.id, - :user_id => user_id, - :token => user_token, - :ttl => PERMANENT_TTL - assert_equal 200, last_response.status - - # Create user token for a different, made up user - post "/services/#{@service.id}/oauth_access_tokens.xml", :provider_key => @provider_key, - :app_id => @application.id, - :user_id => other_id, - :token => other_token, - :ttl => PERMANENT_TTL - assert_equal 200, last_response.status - - # Create unrelated token within the same app - post "/services/#{@service.id}/oauth_access_tokens.xml", :provider_key => @provider_key, - :app_id => @application.id, - :token => 'GLOBAL-TOKEN', - :ttl => PERMANENT_TTL - assert_equal 200, last_response.status - - # Read tokens for this user, should be 1 - get "/services/#{@service.id}/applications/#{@application.id}/oauth_access_tokens.xml", - :provider_key => @provider_key, - :user_id => user_id - - assert_equal 200, last_response.status - - assert_equal 1, xml.at('oauth_access_tokens').element_children.size - - node = xml.at('oauth_access_tokens/oauth_access_token') - - assert_equal user_token, node.content - assert_equal '-1', node.attribute('ttl').value - assert_equal user_id, node.attribute('user_id').value - - # Read tokens for the made up user, should be 1 - get "/services/#{@service.id}/applications/#{@application.id}/oauth_access_tokens.xml", - :provider_key => @provider_key, - :user_id => other_id - - assert_equal 200, last_response.status - - assert_equal 1, xml.at('oauth_access_tokens').element_children.size - - node = xml.at('oauth_access_tokens/oauth_access_token') - - assert_equal other_token, node.content - assert_equal '-1', node.attribute('ttl').value - assert_equal other_id, node.attribute('user_id').value - - # Read tokens for the whole app, should get 3 - get "/services/#{@service.id}/applications/#{@application.id}/oauth_access_tokens.xml", - :provider_key => @provider_key - - assert_equal 200, last_response.status - - assert_equal 3, xml.at('oauth_access_tokens').element_children.size - - # Read tokens for another user_id, should be 0 - get "/services/#{@service.id}/applications/#{@application.id}/oauth_access_tokens.xml", - :provider_key => @provider_key, - :user_id => "non-#{user_id}" - - assert_equal 200, last_response.status - assert xml.at('oauth_access_tokens').element_children.empty?, 'No tokens should be present' - - # Delete the global token - delete "/services/#{@service.id}/oauth_access_tokens/GLOBAL-TOKEN.xml", - :provider_key => @provider_key - - assert_equal 200, last_response.status - - # Read tokens for the whole app again, should be only 2 - get "/services/#{@service.id}/applications/#{@application.id}/oauth_access_tokens.xml", - :provider_key => @provider_key - - assert_equal 200, last_response.status - nodes = xml.at('oauth_access_tokens').element_children - assert_equal 2, nodes.size - - node1, node2 = if nodes.first.content == user_token - nodes - else - nodes.reverse - end - - assert_equal user_token, node1.content - assert_equal '-1', node1.attribute('ttl').value - assert_equal user_id, node1.attribute('user_id').value - - assert_equal other_token, node2.content - assert_equal '-1', node2.attribute('ttl').value - assert_equal other_id, node2.attribute('user_id').value - - # Read tokens for the user_id again, should be 1 - get "/services/#{@service.id}/applications/#{@application.id}/oauth_access_tokens.xml", - :provider_key => @provider_key, - :user_id => user_id - - assert_equal 200, last_response.status - - assert_equal 1, xml.at('oauth_access_tokens').element_children.size - - node = xml.at('oauth_access_tokens/oauth_access_token') - - assert_equal user_token, node.content - assert_equal '-1', node.attribute('ttl').value - assert_equal user_id, node.attribute('user_id').value - - # Delete the user token of a user succeeds - delete "/services/#{@service.id}/oauth_access_tokens/#{user_token}.xml", - :provider_key => @provider_key - - assert_equal 200, last_response.status - - # Delete the user token AGAIN - delete "/services/#{@service.id}/oauth_access_tokens/#{user_token}.xml", - :provider_key => @provider_key - - assert_error_resp_with_exc(AccessTokenInvalid.new(user_token)) - - # Delete the remaining user's token - delete "/services/#{@service.id}/oauth_access_tokens/#{other_token}.xml", - :provider_key => @provider_key - - assert_equal 200, last_response.status - - # Delete the token again - delete "/services/#{@service.id}/oauth_access_tokens/#{other_token}.xml", - :provider_key => @provider_key - - assert_error_resp_with_exc(AccessTokenInvalid.new(other_token)) - - # Read tokens for the user_id again, should be 0 - get "/services/#{@service.id}/applications/#{@application.id}/oauth_access_tokens.xml", - :provider_key => @provider_key, - :user_id => user_id - - assert_equal 200, last_response.status - assert xml.at('oauth_access_tokens').element_children.empty?, 'No tokens should be present' - - # Read tokens for the whole app, should be 0 - get "/services/#{@service.id}/applications/#{@application.id}/oauth_access_tokens.xml", - :provider_key => @provider_key - - assert_equal 200, last_response.status - assert xml.at('oauth_access_tokens').element_children.empty?, 'No tokens should be present' - end - test 'create and read oauth_access_token with TTL supplied' do post "/services/#{@service.id}/oauth_access_tokens.xml", :provider_key => @provider_key, :app_id => @application.id, @@ -511,14 +351,6 @@ def setup assert_equal 200, last_response.status - # try to create the same token for a different user - post "/services/#{@service.id}/oauth_access_tokens.xml", :provider_key => @provider_key, - :app_id => @application.id, - :user_id => @user.username, - :token => 'valid-token-666' - - assert_error_resp_with_exc(AccessTokenAlreadyExists.new('valid-token-666')) - post "/services/#{@service.id}/oauth_access_tokens.xml", :provider_key => @provider_key, :app_id => application2.id, :token => 'valid-token-666' @@ -716,50 +548,24 @@ def setup assert_equal '-1', node[0].attribute('ttl').value end - test 'Delete all tokens for a given service, app and user' do - application2, user2 = setup_app_with_user_tokens + test 'Delete all tokens for a given service and app' do + application = setup_app_with_tokens - # remove 1 user token - OAuth::Token::Storage.remove_tokens(@service.id, application2.id, @user.username) + OAuth::Token::Storage.remove_tokens(@service.id, application.id) - get "/services/#{@service.id}/applications/#{application2.id}/oauth_access_tokens.xml", + get "/services/#{@service.id}/applications/#{application.id}/oauth_access_tokens.xml", :provider_key => @provider_key assert_equal 200, last_response.status - - assert_equal 3, xml.at('oauth_access_tokens').element_children.size - - get "/services/#{@service.id}/applications/#{application2.id}/oauth_access_tokens.xml", - :provider_key => @provider_key, - :user_id => @user.username - - assert_equal 200, last_response.status - assert_equal 0, xml.at('oauth_access_tokens').element_children.size - - get "/services/#{@service.id}/applications/#{application2.id}/oauth_access_tokens.xml", - :provider_key => @provider_key, - :user_id => user2.username - - assert_equal 200, last_response.status - - assert_equal 2, xml.at('oauth_access_tokens').element_children.size - - # remove all remaining tokens for this app - OAuth::Token::Storage.remove_tokens(@service.id, application2.id) - - check_app_with_user_tokens_deleted application2, user2 do - assert_equal 200, last_response.status - assert_equal 0, xml.at('oauth_access_tokens').element_children.size - end end test 'Application.delete triggers OAuth token deletion' do - application2, user2 = setup_app_with_user_tokens + application = setup_app_with_tokens - Application.delete(@service.id, application2.id) + Application.delete(@service.id, application.id) - check_app_with_user_tokens_deleted application2, user2 do + check_app_tokens_deleted application do assert_equal 404, last_response.status end end @@ -879,78 +685,29 @@ def xml Nokogiri::XML(last_response.body) end - def setup_app_with_user_tokens + def setup_app_with_tokens application = Application.save(:service_id => @service.id, :id => next_id, :state => :active, :plan_id => @plan_id, :plan_name => @plan_name) - user = User.save!(service_id: @service.id, username: 'pantxa', plan_id: '1', plan_name: 'plan') - - post "/services/#{@service.id}/oauth_access_tokens.xml", :provider_key => @provider_key, - :app_id => application.id, - :user_id => @user.username, - :token => 'USER-TOKEN' - assert_equal 200, last_response.status - - post "/services/#{@service.id}/oauth_access_tokens.xml", :provider_key => @provider_key, - :app_id => application.id, - :user_id => user.username, - :token => "#{user.username.upcase}-TOKEN" - assert_equal 200, last_response.status - - post "/services/#{@service.id}/oauth_access_tokens.xml", :provider_key => @provider_key, - :app_id => application.id, - :user_id => user.username, - :token => "#{user.username.upcase}-TOKEN2" - assert_equal 200, last_response.status - post "/services/#{@service.id}/oauth_access_tokens.xml", :provider_key => @provider_key, :app_id => application.id, :token => 'GLOBAL-TOKEN' assert_equal 200, last_response.status - # we have 4 tokens, 1 global and 1 for one user, 2 for the other - get "/services/#{@service.id}/applications/#{application.id}/oauth_access_tokens.xml", - :provider_key => @provider_key, - :user_id => @user.username - - assert_equal 200, last_response.status - - assert_equal 1, xml.at('oauth_access_tokens').element_children.size - - get "/services/#{@service.id}/applications/#{application.id}/oauth_access_tokens.xml", - :provider_key => @provider_key, - :user_id => user.username - - assert_equal 200, last_response.status - - assert_equal 2, xml.at('oauth_access_tokens').element_children.size - get "/services/#{@service.id}/applications/#{application.id}/oauth_access_tokens.xml", :provider_key => @provider_key assert_equal 200, last_response.status - assert_equal 4, xml.at('oauth_access_tokens').element_children.size + assert_equal 1, xml.at('oauth_access_tokens').element_children.size - return application, user + application end - def check_app_with_user_tokens_deleted(application, user, &blk) - get "/services/#{@service.id}/applications/#{application.id}/oauth_access_tokens.xml", - :provider_key => @provider_key, - :user_id => @user.username - - blk.call - - get "/services/#{@service.id}/applications/#{application.id}/oauth_access_tokens.xml", - :provider_key => @provider_key, - :user_id => user.username - - blk.call - + def check_app_tokens_deleted(application, &blk) get "/services/#{@service.id}/applications/#{application.id}/oauth_access_tokens.xml", :provider_key => @provider_key diff --git a/test/integration/oauth/application_keys_test.rb b/test/integration/oauth/application_keys_test.rb index f723b5311..fa2ffb2aa 100644 --- a/test/integration/oauth/application_keys_test.rb +++ b/test/integration/oauth/application_keys_test.rb @@ -16,10 +16,6 @@ def setup :state => :active, :plan_id => @plan_id, :plan_name => @plan_name) - @user = User.save!(:username => 'some_user', - :service_id => @service.id, - :plan_id => '1', - :plan_name => 'some_plan') end test 'succeeds if no application key is defined nor passed' do @@ -86,21 +82,4 @@ def setup assert_not_authorized 'application key is missing' end - - test 'succeeds if app_id and user_id are specified' do - get '/transactions/oauth_authorize.xml', :provider_key => @provider_key, - :app_id => @application.id, - :user_id => @user.username - - assert_authorized - end - - test 'succeeds if app_id and user_id are specified while having an app_key' do - @application.create_key - get '/transactions/oauth_authorize.xml', :provider_key => @provider_key, - :app_id => @application.id, - :user_id => @user.username - - assert_authorized - end end diff --git a/test/integration/oauth/basic_test.rb b/test/integration/oauth/basic_test.rb index 4eb11970b..826c649ca 100644 --- a/test/integration/oauth/basic_test.rb +++ b/test/integration/oauth/basic_test.rb @@ -649,65 +649,4 @@ def setup assertions.call false end end - - test 'authorizes if there are app and user limits and none are exceeded' do - app_daily_limit = 10 - user_daily_limit = 2 - fixtures = limited_app_and_user!( - @service, @metric_id, app_daily_limit, user_daily_limit) - - get '/transactions/oauth_authorize.xml', :provider_key => @provider_key, - :service_id => @service.id, - :app_id => fixtures[:app].id, - :user_id => fixtures[:user].username, - :usage => { 'hits' => 0 } - - assert_authorized - end - - test 'denies auth if there are app and user limits and only app limits are exceeded' do - app_daily_limit = 10 - hits_to_report = app_daily_limit + 1 - user_daily_limit = hits_to_report + 1 - fixtures = limited_app_and_user!( - @service, @metric_id, app_daily_limit, user_daily_limit) - - get '/transactions/oauth_authorize.xml', :provider_key => @provider_key, - :service_id => @service.id, - :app_id => fixtures[:app].id, - :user_id => fixtures[:user].username, - :usage => { 'hits' => hits_to_report } - - assert_not_authorized 'usage limits are exceeded' - end - - test 'denies auth if there are app and user limits and only user limits are exceeded' do - user_daily_limit = 10 - hits_to_report = user_daily_limit + 1 - app_daily_limit = hits_to_report + 1 - fixtures = limited_app_and_user!( - @service, @metric_id, app_daily_limit, user_daily_limit) - - get '/transactions/oauth_authorize.xml', :provider_key => @provider_key, - :service_id => @service.id, - :app_id => fixtures[:app].id, - :user_id => fixtures[:user].username, - :usage => { 'hits' => app_daily_limit + 1 } - - assert_not_authorized('usage limits are exceeded') - end - - test 'denies auth if there are app and user limits and both are exceeded' do - app_daily_limit = user_daily_limit = 1 - fixtures = limited_app_and_user!( - @service, @metric_id, app_daily_limit, user_daily_limit) - - get '/transactions/oauth_authorize.xml', :provider_key => @provider_key, - :service_id => @service.id, - :app_id => fixtures[:app].id, - :user_id => fixtures[:user].username, - :usage => { 'hits' => app_daily_limit + 1 } - - assert_not_authorized('usage limits are exceeded') - end end diff --git a/test/integration/oauth/basic_test_with_access_tokens_test.rb b/test/integration/oauth/basic_test_with_access_tokens_test.rb index 6660d3bf7..442a143f8 100644 --- a/test/integration/oauth/basic_test_with_access_tokens_test.rb +++ b/test/integration/oauth/basic_test_with_access_tokens_test.rb @@ -21,30 +21,18 @@ def setup :plan_id => @plan_id, :plan_name => @plan_name) - @user = User.save!(service_id: @service_id, username: 'pantxo', plan_id: '1', plan_name: 'plan') - @metric_id = next_id Metric.save(:service_id => @service.id, :id => @metric_id, :name => 'hits') @access_token = 'valid-token' - @access_token_user = 'valid-user-token' post "/services/#{@service.id}/oauth_access_tokens.xml", :provider_key => @provider_key, :app_id => @application.id, :token => @access_token, :ttl => 60 assert_equal 200, last_response.status - assert_equal [@application.id, nil], + assert_equal @application.id, OAuth::Token::Storage.get_credentials(@access_token, @service.id) - - post "/services/#{@service.id}/oauth_access_tokens.xml", :provider_key => @provider_key, - :app_id => @application.id, - :token => @access_token_user, - :user_id => @user.username, - :ttl => 60 - assert_equal 200, last_response.status - assert_equal [@application.id, @user.username], - OAuth::Token::Storage.get_credentials(@access_token_user, @service.id) end test 'successful authorize responds with 200' do @@ -54,38 +42,6 @@ def setup assert_equal 200, last_response.status end - test 'authorization with a user token with no user specified succeeds' do - get '/transactions/oauth_authorize.xml', :provider_key => @provider_key, - :access_token => @access_token_user - - assert_equal 200, last_response.status - end - - test 'authorization with a user token with user specified succeeds' do - get '/transactions/oauth_authorize.xml', :provider_key => @provider_key, - :access_token => @access_token_user, - :user_id => @user.username - - assert_equal 200, last_response.status - end - - test 'authorization with a user token with a made up user specified succeeds' do - get '/transactions/oauth_authorize.xml', :provider_key => @provider_key, - :access_token => @access_token_user, - :user_id => 'invented_user' - - assert_equal 200, last_response.status - end - - test 'authorization with a global token with user specified succeeds' do - # specifying user_id should make no difference to the OAuth token code - get '/transactions/oauth_authorize.xml', :provider_key => @provider_key, - :access_token => @access_token, - :user_id => @user.username - - assert_equal 200, last_response.status - end - test 'successful authorize with no body responds with 200' do get '/transactions/oauth_authorize.xml', { :provider_key => @provider_key, diff --git a/test/integration/oauth/set_user_usage_test.rb b/test/integration/oauth/set_user_usage_test.rb deleted file mode 100644 index eebd0fe70..000000000 --- a/test/integration/oauth/set_user_usage_test.rb +++ /dev/null @@ -1,179 +0,0 @@ -require File.expand_path(File.dirname(__FILE__) + '/../../test_helper') - -class OauthSetUserUsageTest < Test::Unit::TestCase - include TestHelpers::AuthorizeAssertions - include TestHelpers::Fixtures - include TestHelpers::Integration - - def setup - @storage = Storage.instance(true) - @storage.flushdb - - Resque.reset! - Memoizer.reset! - - setup_oauth_provider_fixtures - - @default_user_plan_id = next_id - @default_user_plan_name = 'user plan mobile' - - @service.user_registration_required = false - @service.default_user_plan_name = @default_user_plan_name - @service.default_user_plan_id = @default_user_plan_id - @service.save! - - @application = Application.save(:service_id => @service.id, - :id => next_id, - :state => :active, - :plan_id => @plan_id, - :plan_name => @plan_name, - :user_required => true) - - @metric_id = next_id - - Metric.save(:service_id => @service.id, :id => @metric_id, :name => 'hits') - - UsageLimit.save(:service_id => @service.id, - :plan_id => @default_user_plan_id, - :metric_id => @metric_id, - :day => 100, :month => 1000, :eternity => 10000) - - end - - test 'check behavior of set values on usage passed as parameter of oauth_authorize' do - Timecop.freeze(Time.utc(2011, 1, 1)) do - post '/transactions.xml', - :provider_key => @provider_key, - :transactions => {0 => {:app_id => @application.id, :user_id => 'user1', :usage => {'hits' => '#99'}}} - Resque.run! - - post '/transactions.xml', - :provider_key => @provider_key, - :transactions => {0 => {:app_id => @application.id, :user_id => 'user2', :usage => {'hits' => '#98'}}} - Resque.run! - - get '/transactions/oauth_authorize.xml', :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user1', - :usage => {'hits' => 2} - - assert_user_usage_report(Time.utc(2011, 1, 1, 13, 0, 0), 'hits', 'day', 99, 100) - assert_not_authorized 'usage limits are exceeded' - - get '/transactions/oauth_authorize.xml', :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user2', - :usage => {'hits' => 2} - - assert_user_usage_report(Time.utc(2011, 1, 1, 13, 0, 0), 'hits', 'day', 98, 100) - assert_authorized - end - - Timecop.freeze(Time.utc(2011, 1, 1)) do - post '/transactions.xml', - :provider_key => @provider_key, - :transactions => {0 => {:app_id => @application.id, :user_id => 'user1', :usage => {'hits' => 2}}, - 1 => {:app_id => @application.id, :user_id => 'user2', :usage => {'hits' => 2}}} - Resque.run! - - get '/transactions/oauth_authorize.xml', :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user1', - :usage => {'hits' => 2} - - assert_user_usage_report(Time.utc(2011, 1, 1, 13, 0, 0), 'hits', 'day', 101, 100) - assert_not_authorized 'usage limits are exceeded' - - get '/transactions/oauth_authorize.xml', :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user2', - :usage => {'hits' => 2} - - assert_user_usage_report(Time.utc(2011, 1, 1, 13, 0, 0), 'hits', 'day', 100, 100) - assert_not_authorized 'usage limits are exceeded' - - get '/transactions/oauth_authorize.xml', :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user2' - - assert_user_usage_report(Time.utc(2011, 1, 1, 13, 0, 0), 'hits', 'day', 100, 100) - assert_authorized - end - - Timecop.freeze(Time.utc(2011, 1, 1)) do - get '/transactions/oauth_authorize.xml', :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user1', - :usage => {'hits' => '#66'} - - assert_user_usage_report(Time.utc(2011, 1, 1, 13, 0, 0), 'hits', 'day', 101, 100) - assert_authorized - - get '/transactions/oauth_authorize.xml', :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user2', - :usage => {'hits' => '#100'} - - assert_user_usage_report(Time.utc(2011, 1, 1, 13, 0, 0), 'hits', 'day', 100, 100) - assert_authorized - end - end - - test 'check behaviour of set values with caching and oauth_authorize' do - Timecop.freeze(Time.utc(2011, 1, 1)) do - post '/transactions.xml', - :provider_key => @provider_key, - :transactions => {0 => {:app_id => @application.id, :user_id => 'user1', :usage => {'hits' => 80}}} - Resque.run! - - 10.times do |cont| - if cont.odd? - get '/transactions/oauth_authorize.xml', :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user1', - :usage => {'hits' => "##{80 + (cont)*2}"} - else - get '/transactions/oauth_authorize.xml', :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user1', - :usage => {'hits' => 2} - end - Resque.run! - - assert_user_usage_report(Time.utc(2011, 1, 1, 13, 0, 0), 'hits', 'day', 80 + (cont)*2, 100) - assert_authorized - - post '/transactions.xml', - :provider_key => @provider_key, - :transactions => {0 => {:app_id => @application.id, :user_id => 'user1', :usage => {'hits' => 2}}} - Resque.run! - end - - 10.times do |cont| - get '/transactions/oauth_authorize.xml', :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user1', - :usage => {'hits' => 2} - Resque.run! - - assert_user_usage_report(Time.utc(2011, 1, 1, 13, 0, 0), 'hits', 'day', 100, 100) - assert_not_authorized 'usage limits are exceeded' - end - - 10.times do |cont| - get '/transactions/oauth_authorize.xml', :provider_key => @provider_key, - :app_id => @application.id, - :user_id => 'user1', - :usage => {'hits' => "##{100 + (cont+1)*-2}"} - - assert_user_usage_report(Time.utc(2011, 1, 1, 13, 0, 0), 'hits', 'day', 100 + (cont)*-2, 100) - assert_authorized - - post '/transactions.xml', - :provider_key => @provider_key, - :transactions => {0 => {:app_id => @application.id, :user_id => 'user1', :usage => {'hits' => "##{100 + (cont+1)*-2}"}}} - Resque.run! - end - end - end -end diff --git a/test/integration/report_test.rb b/test/integration/report_test.rb index 4801c4918..e80920629 100644 --- a/test/integration/report_test.rb +++ b/test/integration/report_test.rb @@ -678,43 +678,6 @@ def setup end end - test 'reporting user_id when not enabled makes failures go to error instead of failed jobs' do - Timecop.freeze(Time.utc(2010, 5, 12, 13, 33)) do - post '/transactions.xml', - :provider_key => @provider_key, - :transactions => {0 => {:app_id => @application.id, :usage => {'hits' => 1}, :user_id => 'user_id'}, - 1 => {:app_id => @application.id, :usage => {'hits' => 1}}, - 2 => {:app_id => @application.id, :usage => {'hits' => 1}}} - Resque.run! - - assert_equal 0, @storage.get(application_key(@service_id, - @application.id, - @metric_id, - :month, '20100501')).to_i - assert_equal 0, Resque.queues[:main].size - assert_equal 1, ErrorStorage.list(@service_id).count - - error = ErrorStorage.list(@service_id).last - assert_not_nil error - assert_equal 'service_cannot_use_user_id', error[:code] - assert_equal "service with service_id=\"#{@service_id}\" does not have access to end user plans, user_id is not allowed", error[:message] - - post '/transactions.xml', - :provider_key => @provider_key, - :transactions => {0 => {:app_id => @application.id, :usage => {'hits' => 1}}, - 1 => {:app_id => @application.id, :usage => {'hits' => 1}}, - 2 => {:app_id => @application.id, :usage => {'hits' => 1}}} - Resque.run! - - assert_equal 0, Resque.queues[:main].size - assert_equal 3, @storage.get(application_key(@service_id, - @application.id, - @metric_id, - :month, '20100501')).to_i - assert_equal 1, ErrorStorage.list(@service_id).count - end - end - test 'reporting a transaction with a past timestamp within the limits does not generate errors' do past_limit = Transaction.const_get(:REPORT_DEADLINE_PAST) current_time = Time.utc(2016, 2, 10) From 5d62fc0fae58bb1e9ab1c2cbf80841283b2ef510 Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Wed, 18 Sep 2019 22:56:46 +0200 Subject: [PATCH 11/13] docs/active_docs: remove end-users --- .../Service Management API (on-premises).json | 57 ------------------- docs/active_docs/Service Management API.json | 57 ------------------- 2 files changed, 114 deletions(-) diff --git a/docs/active_docs/Service Management API (on-premises).json b/docs/active_docs/Service Management API (on-premises).json index 8176830a1..2e6d66a2a 100644 --- a/docs/active_docs/Service Management API (on-premises).json +++ b/docs/active_docs/Service Management API (on-premises).json @@ -52,12 +52,6 @@ "paramType": "query", "description": "Referrer IP Address or Domain. Required only if referrer filtering is enabled. If special value '*' (wildcard) is passed, the referrer check is bypassed." }, - { - "name": "user_id", - "dataType": "string", - "paramType": "query", - "description": "User id. String identifying an end user. Required only when the application is rate limiting end users. The End User plans feature is not available in all 3scale plans." - }, { "name": "usage", "dataType": "hash", @@ -128,12 +122,6 @@ "paramType": "query", "description": "Referrer IP Address or Domain. Required only if referrer filtering is enabled. If special value '*' (wildcard) is passed, the referrer check is bypassed." }, - { - "name": "user_id", - "dataType": "string", - "paramType": "query", - "description": "User id. String identifying an end user. Required only when the application is rate limiting end users. The End User plans feature is not available in all 3scale plans." - }, { "name": "usage", "dataType": "hash", @@ -220,12 +208,6 @@ "paramType": "query", "description": "Referrer IP Address or Domain. Required only if referrer filtering is enabled. If special value '*' (wildcard) is passed, the referrer check is bypassed." }, - { - "name": "user_id", - "dataType": "string", - "paramType": "query", - "description": "User id. String identifying an end user. Used only when the application is rate limiting end users and the specified token is not associated to a user. The End User plans feature is not available in all 3scale plans." - }, { "name": "usage", "dataType": "hash", @@ -312,12 +294,6 @@ "paramType": "query", "description": "Referrer IP Address or Domain. Required only if referrer filtering is enabled. If special value '*' (wildcard) is passed, the referrer check is bypassed." }, - { - "name": "user_id", - "dataType": "string", - "paramType": "query", - "description": "User id. String identifying an end user. Required only when the application is rate limiting end users. The End User plans feature is not available in all 3scale plans." - }, { "name": "usage", "dataType": "hash", @@ -399,12 +375,6 @@ "paramType": "query", "description": "Referrer IP Address or Domain. Required only if referrer filtering is enabled. If special value '*' (wildcard) is passed, the referrer check is bypassed." }, - { - "name": "user_id", - "dataType": "string", - "paramType": "query", - "description": "User id. String identifying an end user. Required only when the application is rate limiting end users. The End User plans feature is not available in all 3scale plans." - }, { "name": "usage", "dataType": "hash", @@ -504,12 +474,6 @@ "paramType": "query", "description": "Referrer IP Address or Domain. Required only if referrer filtering is enabled. If special value '*' (wildcard) is passed, the referrer check is bypassed." }, - { - "name": "user_id", - "dataType": "string", - "paramType": "query", - "description": "User id. String identifying an end user. Used only when the application is rate limiting end users and the specified token is not associated to a user. The End User plans feature is not available in all 3scale plans." - }, { "name": "usage", "dataType": "hash", @@ -607,13 +571,6 @@ "description": "App Id (identifier of the application if the auth. pattern is App Id)", "description_inline": true }, - { - "name": "user_id", - "dataType": "string", - "paramType": "query", - "description": "User id. String identifying an end user. Required only when the application is rate limiting end users. The End User plans feature is not available in all 3scale plans.", - "description_inline": true - }, { "name": "timestamp", "dataType": "string", @@ -707,13 +664,6 @@ "description": "User Key (identifier and shared secret of the application if the auth. pattern is Api Key)", "description_inline": true }, - { - "name": "user_id", - "dataType": "string", - "paramType": "query", - "description": "User id. String identifying an end user. Required only when the application is rate limiting end users. The End User plans feature is not available in all 3scale plans.", - "description_inline": true - }, { "name": "timestamp", "dataType": "string", @@ -807,13 +757,6 @@ "description": "Client Id (identifier of the application if the auth. pattern is OAuth, note that client_id == app_id)", "description_inline": true }, - { - "name": "user_id", - "dataType": "string", - "paramType": "query", - "description": "User id. String identifying an end user. Required only when the application is rate limiting end users. The End User plans feature is not available in all 3scale plans.", - "description_inline": true - }, { "name": "timestamp", "dataType": "string", diff --git a/docs/active_docs/Service Management API.json b/docs/active_docs/Service Management API.json index 8176830a1..2e6d66a2a 100644 --- a/docs/active_docs/Service Management API.json +++ b/docs/active_docs/Service Management API.json @@ -52,12 +52,6 @@ "paramType": "query", "description": "Referrer IP Address or Domain. Required only if referrer filtering is enabled. If special value '*' (wildcard) is passed, the referrer check is bypassed." }, - { - "name": "user_id", - "dataType": "string", - "paramType": "query", - "description": "User id. String identifying an end user. Required only when the application is rate limiting end users. The End User plans feature is not available in all 3scale plans." - }, { "name": "usage", "dataType": "hash", @@ -128,12 +122,6 @@ "paramType": "query", "description": "Referrer IP Address or Domain. Required only if referrer filtering is enabled. If special value '*' (wildcard) is passed, the referrer check is bypassed." }, - { - "name": "user_id", - "dataType": "string", - "paramType": "query", - "description": "User id. String identifying an end user. Required only when the application is rate limiting end users. The End User plans feature is not available in all 3scale plans." - }, { "name": "usage", "dataType": "hash", @@ -220,12 +208,6 @@ "paramType": "query", "description": "Referrer IP Address or Domain. Required only if referrer filtering is enabled. If special value '*' (wildcard) is passed, the referrer check is bypassed." }, - { - "name": "user_id", - "dataType": "string", - "paramType": "query", - "description": "User id. String identifying an end user. Used only when the application is rate limiting end users and the specified token is not associated to a user. The End User plans feature is not available in all 3scale plans." - }, { "name": "usage", "dataType": "hash", @@ -312,12 +294,6 @@ "paramType": "query", "description": "Referrer IP Address or Domain. Required only if referrer filtering is enabled. If special value '*' (wildcard) is passed, the referrer check is bypassed." }, - { - "name": "user_id", - "dataType": "string", - "paramType": "query", - "description": "User id. String identifying an end user. Required only when the application is rate limiting end users. The End User plans feature is not available in all 3scale plans." - }, { "name": "usage", "dataType": "hash", @@ -399,12 +375,6 @@ "paramType": "query", "description": "Referrer IP Address or Domain. Required only if referrer filtering is enabled. If special value '*' (wildcard) is passed, the referrer check is bypassed." }, - { - "name": "user_id", - "dataType": "string", - "paramType": "query", - "description": "User id. String identifying an end user. Required only when the application is rate limiting end users. The End User plans feature is not available in all 3scale plans." - }, { "name": "usage", "dataType": "hash", @@ -504,12 +474,6 @@ "paramType": "query", "description": "Referrer IP Address or Domain. Required only if referrer filtering is enabled. If special value '*' (wildcard) is passed, the referrer check is bypassed." }, - { - "name": "user_id", - "dataType": "string", - "paramType": "query", - "description": "User id. String identifying an end user. Used only when the application is rate limiting end users and the specified token is not associated to a user. The End User plans feature is not available in all 3scale plans." - }, { "name": "usage", "dataType": "hash", @@ -607,13 +571,6 @@ "description": "App Id (identifier of the application if the auth. pattern is App Id)", "description_inline": true }, - { - "name": "user_id", - "dataType": "string", - "paramType": "query", - "description": "User id. String identifying an end user. Required only when the application is rate limiting end users. The End User plans feature is not available in all 3scale plans.", - "description_inline": true - }, { "name": "timestamp", "dataType": "string", @@ -707,13 +664,6 @@ "description": "User Key (identifier and shared secret of the application if the auth. pattern is Api Key)", "description_inline": true }, - { - "name": "user_id", - "dataType": "string", - "paramType": "query", - "description": "User id. String identifying an end user. Required only when the application is rate limiting end users. The End User plans feature is not available in all 3scale plans.", - "description_inline": true - }, { "name": "timestamp", "dataType": "string", @@ -807,13 +757,6 @@ "description": "Client Id (identifier of the application if the auth. pattern is OAuth, note that client_id == app_id)", "description_inline": true }, - { - "name": "user_id", - "dataType": "string", - "paramType": "query", - "description": "User id. String identifying an end user. Required only when the application is rate limiting end users. The End User plans feature is not available in all 3scale plans.", - "description_inline": true - }, { "name": "timestamp", "dataType": "string", From 4e2f52b31d7b4239affff5479a5a56e25e632251 Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Fri, 20 Sep 2019 11:20:48 +0200 Subject: [PATCH 12/13] listener: raise error when there is a user ID --- lib/3scale/backend/errors.rb | 6 ++++++ lib/3scale/backend/listener.rb | 17 +++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/lib/3scale/backend/errors.rb b/lib/3scale/backend/errors.rb index 192eb3753..09eeee833 100644 --- a/lib/3scale/backend/errors.rb +++ b/lib/3scale/backend/errors.rb @@ -333,5 +333,11 @@ def initialize(service_id, msg) super "Delete stats job context validation error. Service: #{service_id}. Error: #{msg}" end end + + class EndUsersNoLongerSupported < BadRequest + def initialize + super 'End-users are no longer supported, do not specify the user_id parameter'.freeze + end + end end end diff --git a/lib/3scale/backend/listener.rb b/lib/3scale/backend/listener.rb index 08dd68afd..2d91c04e2 100644 --- a/lib/3scale/backend/listener.rb +++ b/lib/3scale/backend/listener.rb @@ -164,6 +164,9 @@ def do_api_method(method_name) provider_key_from(params[:service_token], params[:service_id]) raise_provider_key_error(params) if blank?(provider_key) + + check_no_user_id + halt 403 unless valid_usage_params? # As params is passed to other methods, we need to overwrite the @@ -567,6 +570,20 @@ def check_transactions_validity(transactions) if transactions.any? { |_id, data| data.nil? } raise TransactionsHasNilTransaction end + + if transactions.any? { |_id, data| data.is_a?(Hash) && data[:user_id] } + raise EndUsersNoLongerSupported + end + end + + # In previous versions it was possible to authorize by end-user. + # Apisonator used the "user_id" param to do that. + # That's no longer supported, and we want to raise an error when we + # detect that param to let the user know that. + def check_no_user_id + if params && params[:user_id] + raise EndUsersNoLongerSupported + end end def application From 1bfa7038be96f90eccdbc05b0db67e251da43478 Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Fri, 20 Sep 2019 11:21:22 +0200 Subject: [PATCH 13/13] test/integration/listener: test error for user_id --- test/integration/listener_test.rb | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/test/integration/listener_test.rb b/test/integration/listener_test.rb index 46ee33389..57f76b74d 100644 --- a/test/integration/listener_test.rb +++ b/test/integration/listener_test.rb @@ -74,9 +74,33 @@ def test_malformed_hash_param assert_equal 'bad_request', node['code'] end + def test_unsupported_end_users_auth + get '/transactions/authorize.xml', provider_key: 'pk', user_id: '123' + check_end_users_not_supported_error(last_response) + + get '/transactions/authrep.xml', provider_key: 'pk', user_id: '123' + check_end_users_not_supported_error(last_response) + end + + def test_unsupported_end_users_report + post '/transactions.xml', + provider_key: 'pk', + service_id: '42', + transactions: { 0 => { user_id: '123', usage: { 'hits' => 1 } } } + + check_end_users_not_supported_error(last_response) + end + private def xml Nokogiri::XML(last_response.body) end + + def check_end_users_not_supported_error(last_response) + assert_equal 400, last_response.status + node = xml.at('error') + assert_equal 'End-users are no longer supported, do not specify the user_id parameter', node.content + assert_equal 'end_users_no_longer_supported', node['code'] + end end