diff --git a/Gemfile b/Gemfile index 483ac3252..416ab1c2d 100644 --- a/Gemfile +++ b/Gemfile @@ -25,7 +25,7 @@ gem 'newrelic_rpm', '~> 9.11' gem 'pdfkit', '~> 0.8.7.3', require: false gem 'pg', '~> 1.5', '>= 1.5.6' gem 'pghero', '~> 3.5' -gem 'rack-attack', '~> 6.7', require: false +gem 'rack-attack', '~> 6.7' gem 'rack-cors', '~> 2.0', '>= 2.0.2' gem 'rack-timeout', '~> 0.7.0' gem 'rack-mini-profiler', '~> 3.3', '>= 3.3.1' diff --git a/app/controllers/api/beta/base_controller.rb b/app/controllers/api/beta/base_controller.rb index 37699d18a..55f25567d 100644 --- a/app/controllers/api/beta/base_controller.rb +++ b/app/controllers/api/beta/base_controller.rb @@ -29,9 +29,7 @@ def present(hash) private def authenticate_user! - if doorkeeper_token - @current_user = User.find(doorkeeper_token.resource_owner_id) - end + @current_user = AccessTokenAuthentication.user_for(doorkeeper_token) return if @current_user diff --git a/app/controllers/api/v8/apidocs_controller.rb b/app/controllers/api/v8/apidocs_controller.rb index f8b984310..1a1980051 100644 --- a/app/controllers/api/v8/apidocs_controller.rb +++ b/app/controllers/api/v8/apidocs_controller.rb @@ -188,6 +188,7 @@ class ApidocsController < ActionController::Base Api::V8::Core::SubmissionsController, Api::V8::Core::Submissions::FeedbackController, Api::V8::Core::Submissions::ReviewsController, + Api::V8::Application::UserAccessTokensController, AvailablePoint, AwardedPoint, Course, diff --git a/app/controllers/api/v8/application/user_access_tokens_controller.rb b/app/controllers/api/v8/application/user_access_tokens_controller.rb new file mode 100644 index 000000000..ecdb53aa6 --- /dev/null +++ b/app/controllers/api/v8/application/user_access_tokens_controller.rb @@ -0,0 +1,110 @@ +# frozen_string_literal: true + +module Api + module V8 + module Application + # Issues a login token for a user to a trusted backend after a sign-in handoff verified + # elsewhere. Callable only by allow-listed client_credentials apps with the issuer scope. + class UserAccessTokensController < Api::V8::BaseController + include Swagger::Blocks + + TOKEN_LIFETIME = 1.day + + swagger_path '/api/v8/application/user_access_tokens' do + operation :post do + key :description, "Issues an access token for the given user, owned by the configured target application. Requires a client_credentials token of an allow-listed application with the #{AccessTokenAuthentication::ISSUER_SCOPE} scope. Administrators, teachers and assistants are refused. The token cannot change the user's email or password or delete the account. It expires after #{TOKEN_LIFETIME.to_i} seconds." + key :operationId, 'issueUserAccessToken' + key :produces, ['application/json'] + key :tags, ['user'] + parameter do + key :name, :user_id + key :in, :body + key :description, "The user's id" + key :required, true + schema do + key :'$ref', :UserAccessTokenInput + end + end + response 200 do + key :description, 'The issued access token' + schema do + key :'$ref', :UserAccessToken + end + end + response 400, '$ref': '#/responses/error' + response 401, '$ref': '#/responses/error' + response 403, '$ref': '#/responses/error' + response 404, '$ref': '#/responses/error' + end + end + + swagger_schema :UserAccessTokenInput do + key :required, [:user_id] + property :user_id, type: :integer, example: 1 + end + + swagger_schema :UserAccessToken do + key :required, %i[access_token token_type created_at expires_in] + property :access_token, type: :string + property :token_type, type: :string, example: 'bearer' + property :created_at, type: :integer, description: 'Unix timestamp' + property :expires_in, type: :integer, description: 'Seconds from created_at', example: 86_400 + end + + skip_before_action :authenticate_user! + skip_authorization_check + + def create + return respond_unauthorized unless doorkeeper_token&.accessible? + return respond_forbidden unless authorized_issuer? + + return respond_with_error('user_id must be an integer', 400) unless params[:user_id].to_s.match?(/\A\d+\z/) + + user = User.find_by(id: params[:user_id].to_i) + return respond_not_found('User not found') unless user + return respond_forbidden('Tokens are not issued for administrators, teachers or assistants') if user.privileged? + + target_application = Doorkeeper::Application.find_by(uid: Rails.configuration.x.user_access_token_target_application_uid) + return respond_with_error('Target application is not configured', 500) unless target_application + + token = Doorkeeper::AccessToken.create_for( + application: target_application, + resource_owner: user, + scopes: "public #{AccessTokenAuthentication::HANDOFF_SCOPE}", + expires_in: TOKEN_LIFETIME.to_i + ) + Rails.logger.info("Issued user access token: issuer application uid=#{doorkeeper_token.application.uid} user_id=#{user.id}") + + response.headers['Cache-Control'] = 'no-store' + response.headers['Pragma'] = 'no-cache' + + render json: { + access_token: token.plaintext_token, + token_type: 'bearer', + created_at: token.created_at.to_i, + expires_in: token.expires_in + } + end + + private + # Their credentials are served publicly by the credentials endpoint. + def public_client_names + Array(SiteSetting.value('valid_clients')).filter_map { |client| client['name'] } + end + + def authorized_issuer? + issuer_uids = Rails.configuration.x.user_access_token_issuer_application_uids + # resource_owner_id is set on user tokens, which never qualify even from an allowed application. + return false unless AccessTokenAuthentication.issuer_token?(doorkeeper_token) + + application = doorkeeper_token.application + # The target is a frontend client, so its credentials must not be usable to issue tokens. + issuer_uids.include?(application&.uid) && + application.confidential? && + application.uid != Rails.configuration.x.user_access_token_target_application_uid && + public_client_names.exclude?(application.name) + end + end + end + end +end diff --git a/app/controllers/api/v8/base_controller.rb b/app/controllers/api/v8/base_controller.rb index c9b362808..d4a602b87 100644 --- a/app/controllers/api/v8/base_controller.rb +++ b/app/controllers/api/v8/base_controller.rb @@ -43,8 +43,9 @@ def present(hash) def authenticate_user! return @current_user if @current_user if doorkeeper_token - @current_user ||= User.find_by(id: doorkeeper_token.resource_owner_id) - raise 'Invalid token' unless @current_user + @current_user = AccessTokenAuthentication.user_for(doorkeeper_token) + return respond_unauthorized('Invalid token') unless @current_user + @auth_source = :handoff_token if AccessTokenAuthentication.handoff?(doorkeeper_token) elsif Rails.configuration.x.accept_courses_mooc_fi_tokens @current_user = CoursesMoocFiAuthentication.user_for(request) @auth_source = :courses_mooc_fi_token if @current_user diff --git a/app/controllers/api/v8/users_controller.rb b/app/controllers/api/v8/users_controller.rb index 16d110f7d..d45f13966 100644 --- a/app/controllers/api/v8/users_controller.rb +++ b/app/controllers/api/v8/users_controller.rb @@ -306,6 +306,7 @@ def update_email if new_email.blank? @user.errors.add(:email, 'needed') elsif @user.email.casecmp(new_email) != 0 + authorize! :update_credentials, @user @user.email = new_email @user.email_verified = false UserMailer.email_confirmation(@user, params[:origin], params[:language]).deliver_now @@ -327,6 +328,7 @@ def set_password def maybe_update_password if params[:old_password].present? && params[:password].present? + authorize! :update_credentials, @user if @user.managed_externally? unless @user.update_password_via_courses_mooc_fi(params[:old_password], params[:password]) @user.errors.add(:password, 'could not be changed. Please check your current password and try again.') diff --git a/app/controllers/auths_controller.rb b/app/controllers/auths_controller.rb index e79611689..dcc82b9f2 100644 --- a/app/controllers/auths_controller.rb +++ b/app/controllers/auths_controller.rb @@ -10,10 +10,9 @@ class AuthsController < ApplicationController def show if params[:username].present? && params[:session_id].present? - user = User.find_by(login: params[:username]) - user ||= User.find_by('lower(email) = ?', params[:username].downcase) + user = User.find_by_login_or_email(params[:username]) # Allows using oauth2 tokens of the new api for authenticating - res = if user && Doorkeeper::AccessToken.find_by(resource_owner_id: user.id, token: params[:session_id]) + res = if user && AccessTokenAuthentication.user_for(Doorkeeper::AccessToken.by_token(params[:session_id])) == user OK_MESSAGE elsif user && find_session_by_id(params[:session_id])&.belongs_to?(user) OK_MESSAGE @@ -23,8 +22,7 @@ def show return render plain: res end - user = User.find_by(login: params[:username]) - user ||= User.find_by('lower(email) = ?', params[:username].downcase) + user = User.find_by_login_or_email(params[:username]) msg = if user && params[:password].present? && user.has_password?(params[:password]) OK_MESSAGE else diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 518a28c73..506076634 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -100,7 +100,7 @@ def update def confirm_email language = params[:language] - VerificationToken.unscoped.email.find_by!(user_id: params[:user_id], token: params[:id]) + VerificationToken.unscoped.email.find_by!(user_id: params[:user_id], token: params[:token]) redirect_path = root_url redirect_path = "https://course.elementsofai.com/#{language && language != "en" ? "#{language}/" : ''}email-verification" if params[:origin] && params[:origin].start_with?('elements_of_ai') redirect_path = "https://buildingai.elementsofai.com/#{language && language != "en" ? "#{language}/" : ''}email-verified" if params[:origin] && params[:origin].start_with?('building_ai') @@ -119,7 +119,7 @@ def send_verification_email def verify_destroying_user @user = authenticate_current_user_destroy - VerificationToken.delete_user.find_by!(user: @user, token: params[:id]) + VerificationToken.delete_user.find_by!(user: @user, token: params[:token]) end def destroy_user @@ -134,7 +134,7 @@ def destroy_user redirect_to verify_destroying_user_url, alert: failed_authentication_alert(status) return end - VerificationToken.delete_user.find_by!(user: user, token: params[:id]) + VerificationToken.delete_user.find_by!(user: user, token: params[:token]) sign_out if current_user == user email = user.email username = user.login diff --git a/app/helpers/sessions_helper.rb b/app/helpers/sessions_helper.rb index 5fe3b2f08..e21022fb7 100644 --- a/app/helpers/sessions_helper.rb +++ b/app/helpers/sessions_helper.rb @@ -34,7 +34,7 @@ def user_from_session end def user_from_basic_auth - if request&.authorization + if request && ActionController::HttpAuthentication::Basic.has_basic_credentials?(request) username, password = ActionController::HttpAuthentication::Basic.user_name_and_password(request) User.authenticate(username, password) if username && password end diff --git a/app/models/ability.rb b/app/models/ability.rb index 812fc015b..b10ffca36 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -6,9 +6,12 @@ class Ability include CanCan::Ability - # +auth_source+ is :courses_mooc_fi_token when the user authenticated with a courses.mooc.fi - # access token; account changes are then denied to everyone, administrators included. + # +auth_source+ restricts account changes for tokens that are not the user's own login: + # :courses_mooc_fi_token denies all, administrators included; :handoff_token allows profile + # edits but no +:update_credentials+ (email, password) and no account deletion. def initialize(user, auth_source: nil) + alias_action :update_credentials, to: :update + if user.administrator? can :manage, :all can :create, Course @@ -291,10 +294,13 @@ def initialize(user, auth_source: nil) end end - return unless auth_source == :courses_mooc_fi_token - - # The token is scoped to exercise services; it must not take over or delete the account. - cannot :update, User - cannot :destroy, User + case auth_source + when :courses_mooc_fi_token + # The token is scoped to exercise services; it must not take over or delete the account. + cannot :update, User + cannot :destroy, User + when :handoff_token + cannot %i[update_credentials destroy], User + end end end diff --git a/app/models/user.rb b/app/models/user.rb index 781abaa59..8dbebe43f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -167,6 +167,14 @@ def has_password?(submitted_password) result end + # The user a typed username names, matched against login exactly and email case-insensitively. + def self.find_by_login_or_email(username) + username = username&.strip + return nil if username.blank? + + find_by(login: username) || find_by('lower(email) = ?', username.downcase) + end + # The user when the password is accepted, nil otherwise. Use .authenticate_with_status when the # outcome is shown to the user, so that a delegation failure is not reported as a wrong password. def self.authenticate(login, submitted_password) @@ -179,10 +187,7 @@ def self.authenticate(login, submitted_password) # cannot be delegated at all and only an admin can repair it. A found user is returned whatever # the status, so sign in only on :accepted. def self.authenticate_with_status(login, submitted_password) - return [nil, :rejected] unless login - login = login.strip - user = find_by(login: login) - user ||= find_by('lower(email) = ?', login.downcase) + user = find_by_login_or_email(login) return [nil, :rejected] if user.nil? return [user, user.courses_mooc_fi_authentication_status(submitted_password)] if user.managed_externally? @@ -480,6 +485,11 @@ def <=>(other) login.downcase <=> other.login.downcase end + # Administrators, teachers and assistants, whose accounts handoff tokens must never act as. + def privileged? + administrator? || teacherships.exists? || assistantships.exists? + end + def teacher?(organization) organizations.include? organization end diff --git a/app/services/access_token_authentication.rb b/app/services/access_token_authentication.rb new file mode 100644 index 000000000..6b1ee8776 --- /dev/null +++ b/app/services/access_token_authentication.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +# Resolves the user behind a Doorkeeper access token, for every endpoint that accepts one. +module AccessTokenAuthentication + # Marks tokens issued by Api::V8::Application::UserAccessTokensController. + HANDOFF_SCOPE = 'handoff' + # Lets a client_credentials token call Api::V8::Application::UserAccessTokensController. + ISSUER_SCOPE = 'issue_user_access_tokens' + + # The token's owner, or nil when the token is revoked or expired, has no owner, or is a handoff + # token of a user who became privileged after it was issued. + def self.user_for(token) + return nil unless token&.accessible? + + user = User.find_by(id: token.resource_owner_id) + user unless user.nil? || (handoff?(token) && user.privileged?) + end + + # Whether the token lets its client call UserAccessTokensController; the client still has to be + # an allow-listed issuer. + def self.issuer_token?(token) + token.resource_owner_id.nil? && token.acceptable?([ISSUER_SCOPE]) + end + + def self.handoff?(token) + token.includes_scope?(HANDOFF_SCOPE) + end +end diff --git a/app/views/password_reset_keys/show.html.erb b/app/views/password_reset_keys/show.html.erb index e59066d8d..673a0b902 100644 --- a/app/views/password_reset_keys/show.html.erb +++ b/app/views/password_reset_keys/show.html.erb @@ -2,7 +2,7 @@
Resetting password for <%= @user.email %>
-<%= form_tag reset_password_path(key: @key.token), method: :delete do %> +<%= form_tag reset_password_path(@key.token), method: :delete do %> <%= labeled_field('New password: ', password_field_tag(:password)) %> <%= labeled_field('Confirm new password: ', password_field_tag(:password_confirmation)) %> <%= submit_tag 'Set password' %> diff --git a/config/application.rb b/config/application.rb index 1b3f4a4e7..4a703f644 100644 --- a/config/application.rb +++ b/config/application.rb @@ -27,7 +27,7 @@ class Application < Rails::Application # config.i18n.default_locale = :de # Configure sensitive parameters which will be filtered from the log file. - config.filter_parameters += %i[password session_id api_password submission_file return_file test_output stdout stderr vm_log] + config.filter_parameters += %i[password session_id api_password submission_file return_file test_output stdout stderr vm_log client_secret access_token refresh_token bearer_token id_token id_token_hint code_verifier] + [/\A(token|code)\z/] # Do not swallow errors in after_commit/after_rollback callbacks. # config.eager_load_paths << Rails.root.join('lib') @@ -35,6 +35,8 @@ class Application < Rails::Application config.relative_url_root = SiteSetting.value('base_path') config.x.accept_courses_mooc_fi_tokens = ENV['ACCEPT_COURSES_MOOC_FI_TOKENS'] == 'true' + config.x.user_access_token_issuer_application_uids = ENV.fetch('USER_ACCESS_TOKEN_ISSUER_APPLICATION_UIDS', '').split(',').map(&:strip).reject(&:empty?) + config.x.user_access_token_target_application_uid = ENV['USER_ACCESS_TOKEN_TARGET_APPLICATION_UID'].presence config.middleware.insert_before 0, Rack::Cors, debug: true, logger: (-> { Rails.logger }) do allow do diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index 314a2356c..4f92a4d1c 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -37,6 +37,11 @@ # access_token_expires_in 2.hours access_token_expires_in nil + # Issuer tokens are re-fetched by their clients; every other token stays non-expiring. + custom_access_token_expires_in do |context| + 1.hour.to_i if context.grant_type == Doorkeeper::OAuth::CLIENT_CREDENTIALS && context.scopes&.exists?(AccessTokenAuthentication::ISSUER_SCOPE) + end + # Assign a custom TTL for implicit grants. # custom_access_token_expires_in do |oauth_client| # oauth_client.application.additional_settings.implicit_oauth_expiration @@ -66,6 +71,7 @@ optional_scopes %I[ internal_beta_all openid + issue_user_access_tokens ] # Change the way client credentials are retrieved from the request object. diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index e2ed47366..5a568b4b3 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -5,7 +5,7 @@ LOGGER = Logger.new('log/rack-attack.log') ActiveSupport::Notifications.subscribe('rack.attack') do |_name, _start, _finish, _request_id, req| req = req[:request] - msg = [req.env['rack.attack.match_type'], req.ip, req.request_method, req.fullpath, ('"' + req.user_agent.to_s + '"')].join(' ') + msg = [req.env['rack.attack.match_type'], req.ip, req.request_method, SecretPathFilter.filter(req.path), ('"' + req.user_agent.to_s + '"')].join(' ') if %i[throttle blocklist].include?(req.env['rack.attack.match_type']) LOGGER.error(msg) else @@ -14,10 +14,128 @@ end end +# Not Rails.cache: Readthis increments with a separate read and write, so concurrent requests +# would slip past every limit. +Rack::Attack.cache.store = Redis.new(url: ENV.fetch('REDIS_URL')) if ENV['REDIS_URL'] + # RACK Request: https://github.com/rack/rack/blob/main/lib/rack/request.rb class Rack::Attack + LOGIN_PATHS = %w[/sessions /oauth/token].freeze + ACCOUNT_CREATION_PATHS = %w[/user /api/v8/users].freeze + PASSWORD_RESET_PATHS = %w[/password_reset_keys /api/v8/users/password_reset].freeze + + class Request + # The path as Rails routes it, so that /oauth/token/, //oauth/token and /oauth/token.json + # cannot dodge the matches below. + def normalized_path + @normalized_path ||= begin + path = ::Rack::Utils.unescape_path(self.path).b.squeeze('/') + path = path.delete_suffix('/') unless path == '/' + path.sub(%r{\.[^/.]+\z}, '') + end + end + + # The parameters as the Rails controller sees them. A body Rails cannot parse fails the + # request there, so only the query string counts then. + def controller_params + @controller_params ||= begin + action_dispatch_request.request_parameters.merge(query_params) + rescue StandardError + query_params + end + end + + def login_attempt? + LOGIN_PATHS.include?(normalized_path) && post? + end + + def account_creation? + ACCOUNT_CREATION_PATHS.include?(normalized_path) && post? + end + + def api_v7_submission? + post? && normalized_path.match?(%r{\A(/org/\d+)?/exercises/\d+/submissions\z}) + end + + def api_v8_submission? + post? && normalized_path.match?(%r{\A/api/v8/core/exercises/\d+/submissions\z}) + end + + def password_reset_request? + PASSWORD_RESET_PATHS.include?(normalized_path) && post? + end + + # Normalized as the password reset controllers look the address up. + def password_reset_email + controller_params['email'].to_s.strip.downcase.presence + end + + def token_issue? + normalized_path == Rack::Attack.issue_token_path && post? + end + + def shared_address_password_check? + auth_password_check? || basic_auth_username.present? + end + + def auth_password_check? + normalized_path == '/auth' && controller_params['password'].present? + end + + # Only SessionsHelper reads Basic credentials as a user's password; Doorkeeper reads them as + # client credentials and the API controllers ignore them. + def basic_auth_username + return @basic_auth_username if defined?(@basic_auth_username) + + @basic_auth_username = unless normalized_path.start_with?('/oauth/', '/api/') + request = action_dispatch_request + ActionController::HttpAuthentication::Basic.user_name_and_password(request).first if ActionController::HttpAuthentication::Basic.has_basic_credentials?(request) + end + end + + # The issuer application and target user of a token issue request. Nil unless the request + # carries a usable issuer token, so that anonymous requests cannot use up a user's handoffs. + def token_issue_bucket + return @token_issue_bucket if defined?(@token_issue_bucket) + + user_id = controller_params['user_id'].to_s if token_issue? + token = access_token if user_id&.match?(/\A\d+\z/) + @token_issue_bucket = ("#{token.application_id}:#{user_id.to_i}" if token && AccessTokenAuthentication.issuer_token?(token)) + end + + # Read from the header or parameters as Doorkeeper reads it. + def access_token_string + return @access_token_string if defined?(@access_token_string) + + @access_token_string = begin + Doorkeeper::OAuth::Token.from_request(action_dispatch_request, *Doorkeeper.config.access_token_methods) + rescue StandardError + nil + end + end + + private + def access_token + Doorkeeper::AccessToken.by_token(access_token_string) if access_token_string.present? + end + + def action_dispatch_request + @action_dispatch_request ||= ActionDispatch::Request.new(env) + end + + def query_params + action_dispatch_request.query_parameters + rescue StandardError + {} + end + end + + def self.issue_token_path + @issue_token_path ||= Rails.application.routes.url_helpers.api_v8_application_user_access_tokens_path + end + def self.user_identifier_discriminator(access_token) - db_token = Doorkeeper::AccessToken.find_by(token: access_token) + db_token = Doorkeeper::AccessToken.by_token(access_token) return access_token unless db_token db_token.resource_owner_id end @@ -25,12 +143,11 @@ def self.user_identifier_discriminator(access_token) # This safelist is needed because courses.mooc.fi backend auths user # rather then the user and his IP. # RACK Request adds the "HTTP_" infront of the header key. + # Exempt from the safelist so the issue endpoint stays throttled for the mooc.fi backend too. safelist('mark courses.mooc.fi backend requests as safe') do |request| - if ENV['RACK_ATTACK_SAFE_API_KEY'] && request.has_header?('HTTP_RATELIMIT_PROTECTION_SAFE_API_KEY') - ENV['RACK_ATTACK_SAFE_API_KEY'] == request.get_header('HTTP_RATELIMIT_PROTECTION_SAFE_API_KEY') - else - false - end + ENV['RACK_ATTACK_SAFE_API_KEY'].present? && + ActiveSupport::SecurityUtils.secure_compare(request.get_header('HTTP_RATELIMIT_PROTECTION_SAFE_API_KEY').to_s, ENV['RACK_ATTACK_SAFE_API_KEY']) && + request.normalized_path != issue_token_path end # Return 503 Service Unavailable for throttles @@ -41,73 +158,66 @@ def self.user_identifier_discriminator(access_token) ] end - # Limit the number of logins to 20 attempts per minute - throttle('login attempts per minute', limit: 20, period: 1.minute) do |req| - req.ip if req.path == '/sessions' || - req.path == '/oauth/token' && - req.post? + throttle('login attempts per minute', limit: 100, period: 1.minute) do |req| + req.ip if req.login_attempt? + end + + throttle('login attempts per hour', limit: 600, period: 1.hour) do |req| + req.ip if req.login_attempt? end - # Limit the number of logins to 100 attempts per hour - throttle('login attempts per hour', limit: 100, period: 1.hour) do |req| - req.ip if req.path == '/sessions' || - req.path == '/oauth/token' && - req.post? + throttle('login attempts per day', limit: 3000, period: 1.day) do |req| + req.ip if req.login_attempt? end - # Limit the number of logins to 500 attempts per day - throttle('login attempts per day', limit: 500, period: 1.day) do |req| - req.ip if req.path == '/sessions' || - req.path == '/oauth/token' && - req.post? + # Kept apart from the login buckets so that clients sending Basic credentials on every request + # cannot lock their classroom's shared address out of logging in. + throttle('basic auth and /auth password checks per ip per minute', limit: 300, period: 1.minute) do |req| + req.ip if req.shared_address_password_check? end - # Limit the number of account creations to 15 accounts per minute - throttle('account creations per minute', limit: 15, period: 1.minute) do |req| - req.ip if req.path == '/user' || - req.path == 'api/v8/users' && - req.post? + throttle('basic auth and /auth password checks per ip per hour', limit: 3000, period: 1.hour) do |req| + req.ip if req.shared_address_password_check? end - # Limit the number of account creations to 1000 per day - throttle('account creations per day', limit: 1000, period: 1.day) do |req| - req.ip if req.path == '/user' || - req.path == 'api/v8/users' && - req.post? + throttle('account creations per minute', limit: 60, period: 1.minute) do |req| + req.ip if req.account_creation? end - # Limit the number of submissions per ip to 15 per 10 minutes - throttle('submissions per ip per 10 minutes', limit: 30, period: 10.minutes) do |req| - req.ip if req.path =~ %r{^/exercises/\d+/submissions$} || - req.path =~ %r{^/org/\d+/exercises/\d+/submissions$} || - req.path =~ %r{^/api/v8/core/exercises/\d+/submissions$} && - req.post? + throttle('account creations per day', limit: 3000, period: 1.day) do |req| + req.ip if req.account_creation? end - # Limit the number of submissions per ip to 200 per day to api v7 endpoints - throttle('submissions per ip per day to api v7', limit: 200, period: 1.day) do |req| - req.ip if req.path =~ %r{^/exercises/\d+/submissions$} || - req.path =~ %r{^/org/\d+/exercises/\d+/submissions$} && - req.post? + throttle('submissions per ip per 10 minutes', limit: 600, period: 10.minutes) do |req| + req.ip if req.api_v7_submission? || req.api_v8_submission? + end + + throttle('submissions per ip per day to api v7', limit: 5000, period: 1.day) do |req| + req.ip if req.api_v7_submission? end - # Limit the number of submissions per user to 250 per day to the api v8 endpoint throttle('submissions per user per day to api v8', limit: 250, period: 1.day) do |req| - if req.path =~ %r{^/api/v8/core/exercises/\d+/submissions$} && req.post? - token = req.params['access_token'] - discriminator = user_identifier_discriminator(token) - discriminator - end + user_identifier_discriminator(req.access_token_string) if req.api_v8_submission? && req.access_token_string.present? + end + + # Every call comes from the mooc.fi backend, so this per-IP limit is effectively global. + throttle('user access tokens per ip per minute', limit: 120, period: 1.minute) do |req| + req.ip if req.token_issue? + end + + throttle('user access tokens per user per 10 minutes', limit: 10, period: 10.minutes) do |req| + req.token_issue_bucket + end + + throttle('user access tokens per user per day', limit: 50, period: 1.day) do |req| + req.token_issue_bucket end - # Limit the number of password resets per ip to 10 per hour throttle('password resets per ip per hour', limit: 10, period: 1.hour) do |req| - req.ip if req.path == '/password_reset_keys' && - req.post? + req.ip if req.password_reset_request? end - # Limit the number of password resets per email to 5 per day throttle('password resets per email per day', limit: 5, period: 1.day) do |req| - req.params['email'] if req.path == '/password_reset_keys' && req.post? + req.password_reset_email if req.password_reset_request? end end diff --git a/config/initializers/secret_path_filter.rb b/config/initializers/secret_path_filter.rb new file mode 100644 index 000000000..f29a78c3a --- /dev/null +++ b/config/initializers/secret_path_filter.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +# Emailed links carry their token in the URL path, which Rails and Rack::Attack would log verbatim. +module SecretPathFilter + TOKEN_SEGMENT = %r{(/reset_password/+|/users/+[^/]+/+(?:verify|destroy)/+)[^/?#]+} + + def self.filter(path) + path.gsub(TOKEN_SEGMENT, '\1[FILTERED]') + end +end + +ActionDispatch::Request.prepend(Module.new do + def filtered_path + SecretPathFilter.filter(super) + end +end) + +Rails.application.config.filter_redirect << SecretPathFilter::TOKEN_SEGMENT diff --git a/config/routes.rb b/config/routes.rb index 3dc83e7a7..202b74cef 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -153,6 +153,10 @@ resources :application, param: :name, only: [] do resources :credentials, module: :application, only: :index end + + namespace :application do + resources :user_access_tokens, only: :create + end end end @@ -280,14 +284,14 @@ end end - get '/users/:user_id/verify/:id', to: 'users#confirm_email', as: 'confirm_email' + get '/users/:user_id/verify/:token', to: 'users#confirm_email', as: 'confirm_email' post '/users/:user_id/send_verification_email', to: 'users#send_verification_email', as: 'send_verification_email' resources :users, only: [:index] post '/users/:user_id/send_destroy_email', to: 'users#send_destroy_email', as: 'send_destroy_email' - get '/users/:user_id/destroy/:id', to: 'users#verify_destroying_user', as: 'verify_destroying_user' - delete '/users/:user_id/destroy/:id', to: 'users#destroy_user', as: 'destroy_user' + get '/users/:user_id/destroy/:token', to: 'users#verify_destroying_user', as: 'verify_destroying_user' + delete '/users/:user_id/destroy/:token', to: 'users#destroy_user', as: 'destroy_user' resources :certificates, only: %i[show create] diff --git a/spec/controllers/api/v8/application/user_access_tokens_controller_spec.rb b/spec/controllers/api/v8/application/user_access_tokens_controller_spec.rb new file mode 100644 index 000000000..6bab3b316 --- /dev/null +++ b/spec/controllers/api/v8/application/user_access_tokens_controller_spec.rb @@ -0,0 +1,144 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Api::V8::Application::UserAccessTokensController, type: :controller do + let!(:user) { FactoryBot.create(:user) } + let!(:admin) { FactoryBot.create(:admin) } + let!(:issuer_app) { Doorkeeper::Application.create!(name: 'mooc.fi backend', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob') } + let!(:other_app) { Doorkeeper::Application.create!(name: 'other backend', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob') } + let!(:target_app) { Doorkeeper::Application.create!(name: 'mooc.fi frontend', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob') } + let(:issuer_token) { Doorkeeper::AccessToken.create!(application: issuer_app, scopes: 'issue_user_access_tokens') } + + around :each do |example| + config = Rails.configuration.x + original = [config.user_access_token_issuer_application_uids, config.user_access_token_target_application_uid] + example.run + ensure + config.user_access_token_issuer_application_uids, config.user_access_token_target_application_uid = original + end + + before :each do + Rails.configuration.x.user_access_token_issuer_application_uids = [issuer_app.uid] + Rails.configuration.x.user_access_token_target_application_uid = target_app.uid + end + + def post_with(token, user_id) + request.headers['Authorization'] = "Bearer #{token.token}" if token + post :create, params: { user_id: user_id }, format: :json + end + + it 'issues a one-day handoff token for the user owned by the target application' do + post_with(issuer_token, user.id) + + expect(response).to have_http_status(200) + body = JSON.parse(response.body) + expect(body['token_type']).to eq('bearer') + issued = Doorkeeper::AccessToken.find_by(token: body['access_token']) + expect(issued.resource_owner_id).to eq(user.id) + expect(issued.application).to eq(target_app) + expect(issued.scopes.to_a).to eq(%w[public handoff]) + expect(body['created_at']).to eq(issued.created_at.to_i) + expect(body['expires_in']).to eq(86_400) + expect(issued.expires_in).to eq(86_400) + expect(response.headers['Cache-Control']).to eq('no-store') + end + + it 'rejects an application whose credentials are public' do + public_app = Doorkeeper::Application.create!(name: SiteSetting.value('valid_clients').first['name'], redirect_uri: 'urn:ietf:wg:oauth:2.0:oob') + Rails.configuration.x.user_access_token_issuer_application_uids = [public_app.uid] + token = Doorkeeper::AccessToken.create!(application: public_app, scopes: 'issue_user_access_tokens') + post_with(token, user.id) + + expect(response).to have_http_status(403) + end + + it 'rejects a non-confidential application' do + issuer_app.update!(confidential: false) + post_with(issuer_token, user.id) + + expect(response).to have_http_status(403) + end + + it 'requires a token' do + post_with(nil, user.id) + + expect(response).to have_http_status(401) + end + + it 'rejects an application that is not allow-listed' do + token = Doorkeeper::AccessToken.create!(application: other_app, scopes: 'issue_user_access_tokens') + post_with(token, user.id) + + expect(response).to have_http_status(403) + end + + it 'rejects a token without the scope' do + token = Doorkeeper::AccessToken.create!(application: issuer_app, scopes: 'public') + post_with(token, user.id) + + expect(response).to have_http_status(403) + end + + it 'rejects a user token even from the allow-listed application' do + token = Doorkeeper::AccessToken.create!(application: issuer_app, resource_owner_id: user.id, scopes: 'issue_user_access_tokens') + post_with(token, user.id) + + expect(response).to have_http_status(403) + end + + it 'rejects a revoked token' do + issuer_token.revoke + post_with(issuer_token, user.id) + + expect(response).to have_http_status(401) + end + + it 'rejects an expired token' do + token = Doorkeeper::AccessToken.create!(application: issuer_app, scopes: 'issue_user_access_tokens', expires_in: 60, created_at: 2.minutes.ago) + post_with(token, user.id) + + expect(response).to have_http_status(401) + end + + it 'reads a zero-padded user_id as decimal' do + other_user = FactoryBot.create(:user) + post_with(issuer_token, "0#{other_user.id}") + + expect(response).to have_http_status(200) + issued = Doorkeeper::AccessToken.find_by(token: JSON.parse(response.body)['access_token']) + expect(issued.resource_owner_id).to eq(other_user.id) + end + + it 'returns 404 for an unknown user' do + post_with(issuer_token, User.maximum(:id) + 1) + + expect(response).to have_http_status(404) + end + + it 'refuses to issue tokens for administrators' do + expect do + post_with(issuer_token, admin.id) + end.not_to change { Doorkeeper::AccessToken.where(resource_owner_id: admin.id).count } + + expect(response).to have_http_status(403) + end + + it 'refuses to issue tokens for teachers' do + Teachership.create!(user: user, organization: FactoryBot.create(:organization)) + expect do + post_with(issuer_token, user.id) + end.not_to change { Doorkeeper::AccessToken.where(resource_owner_id: user.id).count } + + expect(response).to have_http_status(403) + end + + it 'refuses to issue tokens for assistants' do + Assistantship.create!(user: user, course: FactoryBot.create(:course)) + expect do + post_with(issuer_token, user.id) + end.not_to change { Doorkeeper::AccessToken.where(resource_owner_id: user.id).count } + + expect(response).to have_http_status(403) + end +end diff --git a/spec/controllers/api/v8/base_controller_spec.rb b/spec/controllers/api/v8/base_controller_spec.rb index 4d3404e23..bfc2aad6a 100644 --- a/spec/controllers/api/v8/base_controller_spec.rb +++ b/spec/controllers/api/v8/base_controller_spec.rb @@ -28,7 +28,7 @@ def index context 'when logged in' do let(:user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } it { expect(subject.id).to be user.id } end end @@ -38,9 +38,68 @@ def index allow(controller).to receive(:doorkeeper_token) { token } end context 'with a valid token having invalid user ID' do - let(:token) { double resource_owner_id: -1, acceptable?: true } + let(:token) { double resource_owner_id: -1, accessible?: true, includes_scope?: false } it 'request is not successful' do - expect { get :index }.to raise_error(RuntimeError) + get :index + expect(response).to have_http_status(401) + end + end + context 'with a revoked token' do + let(:user) { FactoryBot.create(:user) } + let(:application) { Doorkeeper::Application.create!(name: 'client', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob') } + let(:token) { Doorkeeper::AccessToken.create!(application: application, resource_owner_id: user.id, scopes: 'public').tap(&:revoke) } + it 'request is not successful' do + request.accept = 'application/json' + get :index + expect(response).to have_http_status(401) + expect(JSON.parse(response.body)).to eq('error' => 'Invalid token') + end + end + context 'with an expired token' do + let(:user) { FactoryBot.create(:user) } + let(:application) { Doorkeeper::Application.create!(name: 'client', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob') } + let(:token) { Doorkeeper::AccessToken.create!(application: application, resource_owner_id: user.id, scopes: 'public', expires_in: 60, created_at: 2.minutes.ago) } + it 'request is not successful' do + get :index + expect(response).to have_http_status(401) + end + end + context 'with a handoff token' do + let(:user) { FactoryBot.create(:user) } + let(:application) { Doorkeeper::Application.create!(name: 'client', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob') } + let(:token) { Doorkeeper::AccessToken.create!(application: application, resource_owner_id: user.id, scopes: 'public handoff') } + + it 'authenticates an unprivileged user' do + get :index + expect(response).to have_http_status(200) + end + + it 'is refused once the user has become an administrator' do + user.update!(administrator: true) + get :index + expect(response).to have_http_status(401) + end + + it 'is refused once the user has become a teacher' do + Teachership.create!(user: user, organization: FactoryBot.create(:organization)) + get :index + expect(response).to have_http_status(401) + end + + it 'is refused once the user has become an assistant' do + Assistantship.create!(user: user, course: FactoryBot.create(:course)) + get :index + expect(response).to have_http_status(401) + end + end + context 'with a privileged user\'s ordinary login token' do + let(:user) { FactoryBot.create(:admin) } + let(:application) { Doorkeeper::Application.create!(name: 'client', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob') } + let(:token) { Doorkeeper::AccessToken.create!(application: application, resource_owner_id: user.id, scopes: 'public') } + + it 'authenticates' do + get :index + expect(response).to have_http_status(200) end end end diff --git a/spec/controllers/api/v8/core/courses/reviews_controller_spec.rb b/spec/controllers/api/v8/core/courses/reviews_controller_spec.rb index 44ec90c85..09b28b1b9 100644 --- a/spec/controllers/api/v8/core/courses/reviews_controller_spec.rb +++ b/spec/controllers/api/v8/core/courses/reviews_controller_spec.rb @@ -18,7 +18,7 @@ describe "GET reviews for a course for current user's submissions" do describe 'as admin' do let(:user) { FactoryBot.create(:admin) } - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } describe 'when course id given' do it "shows only user's submission's review information" do @@ -46,7 +46,7 @@ describe 'as user' do let(:user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } describe 'when course id given' do it "shows only user's submission's review information" do @@ -100,7 +100,7 @@ describe 'Update review info' do describe 'As an admin' do let(:user) { FactoryBot.create(:admin) } - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } before :each do controller.current_user = user end @@ -144,7 +144,7 @@ describe 'As an user' do describe 'when trying to edit review' do let(:user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } it 'should deny access' do FactoryBot.create(:review, submission: submission) exercise.reload diff --git a/spec/controllers/api/v8/core/courses/unlocks_controller_spec.rb b/spec/controllers/api/v8/core/courses/unlocks_controller_spec.rb index 19c924c8e..ceef485e2 100644 --- a/spec/controllers/api/v8/core/courses/unlocks_controller_spec.rb +++ b/spec/controllers/api/v8/core/courses/unlocks_controller_spec.rb @@ -14,7 +14,7 @@ describe 'POST unlock exercises' do describe 'as an authenticated user' do - let!(:token) { double resource_owner_id: user.id, acceptable?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } it 'should allow unlocking exercises' do post :create, params: { organization_slug: organization.id, course_id: course.id } diff --git a/spec/controllers/api/v8/core/courses_controller_spec.rb b/spec/controllers/api/v8/core/courses_controller_spec.rb index 15c420e34..618e1fc0c 100644 --- a/spec/controllers/api/v8/core/courses_controller_spec.rb +++ b/spec/controllers/api/v8/core/courses_controller_spec.rb @@ -11,7 +11,7 @@ describe 'GET course details show' do describe 'in JSON format with valid token' do - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } let(:course) { FactoryBot.create(:course, name: 'Course1') } before :each do course.exercises << FactoryBot.create(:returnable_exercise, name: 'Exercise1', course: course) diff --git a/spec/controllers/api/v8/core/exercises/solutions_controller_spec.rb b/spec/controllers/api/v8/core/exercises/solutions_controller_spec.rb index 6ed9749da..67a49c8e7 100644 --- a/spec/controllers/api/v8/core/exercises/solutions_controller_spec.rb +++ b/spec/controllers/api/v8/core/exercises/solutions_controller_spec.rb @@ -20,7 +20,7 @@ describe 'Downloading an exercise solution zip' do describe 'as an admin' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } it 'should succeed if the exercise exists' do repo = clone_course_repo(course) @@ -34,7 +34,7 @@ end describe 'as a student' do - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } describe 'if the course exists' do it 'should succeed if I have already solved it' do diff --git a/spec/controllers/api/v8/core/exercises/submissions_controller_spec.rb b/spec/controllers/api/v8/core/exercises/submissions_controller_spec.rb index 8b150b0f5..498176469 100644 --- a/spec/controllers/api/v8/core/exercises/submissions_controller_spec.rb +++ b/spec/controllers/api/v8/core/exercises/submissions_controller_spec.rb @@ -29,7 +29,7 @@ def create_submission(file) describe 'Creating a submission' do describe 'as an authenticated user' do - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } it 'should accept submissions when the deadline is open' do exercise.deadline_spec = ['1.1.2100'].to_json diff --git a/spec/controllers/api/v8/core/exercises_controller_spec.rb b/spec/controllers/api/v8/core/exercises_controller_spec.rb index aa27b15a6..0809b3e6d 100644 --- a/spec/controllers/api/v8/core/exercises_controller_spec.rb +++ b/spec/controllers/api/v8/core/exercises_controller_spec.rb @@ -45,7 +45,7 @@ end describe 'When logged in' do - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } describe 'and correct exercise id is given' do it 'should return correct data' do submission = FactoryBot.create(:submission, course: course, user: user, exercise: exercise) diff --git a/spec/controllers/api/v8/core/organizations/courses_controller_spec.rb b/spec/controllers/api/v8/core/organizations/courses_controller_spec.rb index 62f96a8e5..e7a1d4690 100644 --- a/spec/controllers/api/v8/core/organizations/courses_controller_spec.rb +++ b/spec/controllers/api/v8/core/organizations/courses_controller_spec.rb @@ -16,7 +16,7 @@ describe 'GET courses' do describe 'as admin' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } it 'shows a list of collections of course urls' do get :index, params: { organization_slug: organization.slug } json = JSON.parse response.body @@ -31,7 +31,7 @@ end end describe 'as user' do - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } it 'shows a list of collections of course urls' do get :index, params: { organization_slug: organization.slug } json = JSON.parse response.body diff --git a/spec/controllers/api/v8/core/submissions/feedback_controller_spec.rb b/spec/controllers/api/v8/core/submissions/feedback_controller_spec.rb index f2d91ff1b..e8148c324 100644 --- a/spec/controllers/api/v8/core/submissions/feedback_controller_spec.rb +++ b/spec/controllers/api/v8/core/submissions/feedback_controller_spec.rb @@ -16,7 +16,7 @@ describe 'POST create' do describe 'As a student' do - let(:token) { double resource_owner_id: student.id, acceptable?: true } + let(:token) { double resource_owner_id: student.id, accessible?: true, includes_scope?: false } it 'can give feedback' do # pending 'Failing for some reason' diff --git a/spec/controllers/api/v8/core/submissions/reviews_controller_spec.rb b/spec/controllers/api/v8/core/submissions/reviews_controller_spec.rb index 2553cdf8b..84a117ab5 100644 --- a/spec/controllers/api/v8/core/submissions/reviews_controller_spec.rb +++ b/spec/controllers/api/v8/core/submissions/reviews_controller_spec.rb @@ -18,7 +18,7 @@ describe 'POST create' do describe 'As an admin' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } it 'can make new review' do expect do @@ -29,7 +29,7 @@ end describe 'As a teacher' do - let(:token) { double resource_owner_id: teacher.id, acceptable?: true } + let(:token) { double resource_owner_id: teacher.id, accessible?: true, includes_scope?: false } it 'can make new review' do expect do @@ -40,7 +40,7 @@ end describe 'As a student' do - let(:token) { double resource_owner_id: student.id, acceptable?: true } + let(:token) { double resource_owner_id: student.id, accessible?: true, includes_scope?: false } it "can't make new review" do expect do diff --git a/spec/controllers/api/v8/core/submissions_controller_spec.rb b/spec/controllers/api/v8/core/submissions_controller_spec.rb index 439ed7743..355cf76fc 100644 --- a/spec/controllers/api/v8/core/submissions_controller_spec.rb +++ b/spec/controllers/api/v8/core/submissions_controller_spec.rb @@ -27,7 +27,7 @@ describe 'Downloading a submission as zip' do describe 'as an admin' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } it "should allow to download everyone's submissions" do # pending("test that submission zip's content is correct") @@ -40,7 +40,7 @@ before :each do Teachership.create(user: teacher, organization: organization) end - let(:token) { double resource_owner_id: teacher.id, acceptable?: true } + let(:token) { double resource_owner_id: teacher.id, accessible?: true, includes_scope?: false } it "should allow to download own organization's submissions" do # pending("test that submission zip's content is correct") @@ -63,7 +63,7 @@ before :each do Assistantship.create(user: assistant, course: course) end - let(:token) { double resource_owner_id: assistant.id, acceptable?: true } + let(:token) { double resource_owner_id: assistant.id, accessible?: true, includes_scope?: false } it "should allow to download own course's submissions" do # pending("test that submission zip's content is correct") @@ -82,7 +82,7 @@ end end describe 'as a student' do - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } it 'should allow to download own submissions' do # pending("test that submission zip's content is correct") diff --git a/spec/controllers/api/v8/courses/all_courses_with_this_template_spec.rb b/spec/controllers/api/v8/courses/all_courses_with_this_template_spec.rb index 587604361..8f8c284db 100644 --- a/spec/controllers/api/v8/courses/all_courses_with_this_template_spec.rb +++ b/spec/controllers/api/v8/courses/all_courses_with_this_template_spec.rb @@ -34,7 +34,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: admin.id, acceptable?: true } + let!(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } it 'should show all courses with same template' do two_courses_by_template_id @@ -62,7 +62,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: teacher.id, acceptable?: true } + let!(:token) { double resource_owner_id: teacher.id, accessible?: true, includes_scope?: false } it 'should show all courses with same template' do two_courses_by_template_id @@ -90,7 +90,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: assistant.id, acceptable?: true } + let!(:token) { double resource_owner_id: assistant.id, accessible?: true, includes_scope?: false } it 'should show all courses with same template' do two_courses_by_template_id @@ -118,7 +118,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: user.id, acceptable?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } it 'should show all courses with same template' do two_courses_by_template_id diff --git a/spec/controllers/api/v8/courses/exercises/points_controller_spec.rb b/spec/controllers/api/v8/courses/exercises/points_controller_spec.rb index 3428b940b..fca26eb5c 100644 --- a/spec/controllers/api/v8/courses/exercises/points_controller_spec.rb +++ b/spec/controllers/api/v8/courses/exercises/points_controller_spec.rb @@ -20,7 +20,7 @@ end describe 'As an admin' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } describe 'when searching for all users awarded points' do describe 'using course name' do it 'should return all users awarded points of the exercise' do @@ -46,7 +46,7 @@ end describe 'As any user' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } describe 'when searching awarded points' do describe 'and no points are found' do it 'should return an empty array' do diff --git a/spec/controllers/api/v8/courses/exercises/users/points_controller_spec.rb b/spec/controllers/api/v8/courses/exercises/users/points_controller_spec.rb index ce18f8de7..3c4365f60 100644 --- a/spec/controllers/api/v8/courses/exercises/users/points_controller_spec.rb +++ b/spec/controllers/api/v8/courses/exercises/users/points_controller_spec.rb @@ -30,7 +30,7 @@ end describe 'As any user' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } describe 'when searching for users awarded points by user id' do describe 'and using course id' do it 'should return only correct users awarded points' do diff --git a/spec/controllers/api/v8/courses/exercises_controller_spec.rb b/spec/controllers/api/v8/courses/exercises_controller_spec.rb index 508148238..5023667b8 100644 --- a/spec/controllers/api/v8/courses/exercises_controller_spec.rb +++ b/spec/controllers/api/v8/courses/exercises_controller_spec.rb @@ -15,7 +15,7 @@ end describe 'As an admin' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } describe 'when course id is given' do it 'should return successful response' do get :index, params: { course_id: course.id } @@ -33,7 +33,7 @@ end describe 'As a student' do - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } describe 'when course id is given' do it 'should return successful response' do get :index, params: { course_id: course.id } @@ -51,7 +51,7 @@ end describe 'As a student without unlock rows' do - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } def unlocked_flags get :index, params: { course_id: course.id } @@ -81,7 +81,7 @@ def unlocked_flags end describe 'As any user' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } describe 'when course id could not be found' do it 'should return error' do get :index, params: { course_id: '123' } diff --git a/spec/controllers/api/v8/courses/points_controller_spec.rb b/spec/controllers/api/v8/courses/points_controller_spec.rb index db947acce..ed0f1d8f4 100644 --- a/spec/controllers/api/v8/courses/points_controller_spec.rb +++ b/spec/controllers/api/v8/courses/points_controller_spec.rb @@ -20,7 +20,7 @@ describe 'GET points for a course by id' do describe 'as admin' do let(:current_user) { FactoryBot.create(:admin) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when course ID given' do it "shows only user's point information" do @@ -47,7 +47,7 @@ describe 'as user' do let(:current_user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when course ID given' do it "shows only user's point information" do diff --git a/spec/controllers/api/v8/courses/submissions/last_hour_controller_spec.rb b/spec/controllers/api/v8/courses/submissions/last_hour_controller_spec.rb index 649c23d3e..d3c72899d 100644 --- a/spec/controllers/api/v8/courses/submissions/last_hour_controller_spec.rb +++ b/spec/controllers/api/v8/courses/submissions/last_hour_controller_spec.rb @@ -18,7 +18,7 @@ describe 'index' do context 'as a regular user' do - let!(:token) { double resource_owner_id: regular_user.id, acceptable?: true } + let!(:token) { double resource_owner_id: regular_user.id, accessible?: true, includes_scope?: false } it 'should not allow access' do get :index, params: { course_id: course.id } @@ -27,7 +27,7 @@ end context 'as an admin' do - let!(:token) { double resource_owner_id: admin_user.id, acceptable?: true } + let!(:token) { double resource_owner_id: admin_user.id, accessible?: true, includes_scope?: false } it 'gives submission ids for the last hour' do get :index, params: { course_id: course.id } diff --git a/spec/controllers/api/v8/courses/submissions_controller_spec.rb b/spec/controllers/api/v8/courses/submissions_controller_spec.rb index 10b627e33..91f5773e0 100644 --- a/spec/controllers/api/v8/courses/submissions_controller_spec.rb +++ b/spec/controllers/api/v8/courses/submissions_controller_spec.rb @@ -37,7 +37,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: admin.id, acceptable?: true } + let!(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } it 'should show all of the submissions' do two_subs_by_id @@ -61,7 +61,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: teacher.id, acceptable?: true } + let!(:token) { double resource_owner_id: teacher.id, accessible?: true, includes_scope?: false } it 'should show all of the submissions in my organizations' do two_subs_by_id @@ -89,7 +89,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: assistant.id, acceptable?: true } + let!(:token) { double resource_owner_id: assistant.id, accessible?: true, includes_scope?: false } it 'should show all of the submissions in my courses' do two_subs_by_id @@ -117,7 +117,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: user.id, acceptable?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } it 'should show my own submissions' do all_own_subs(course_id: course.id) diff --git a/spec/controllers/api/v8/courses/users/points_controller_spec.rb b/spec/controllers/api/v8/courses/users/points_controller_spec.rb index f9966ee51..db70785be 100644 --- a/spec/controllers/api/v8/courses/users/points_controller_spec.rb +++ b/spec/controllers/api/v8/courses/users/points_controller_spec.rb @@ -21,7 +21,7 @@ describe "GET current user's points" do describe 'as admin' do let(:current_user) { FactoryBot.create(:admin) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when course ID given' do it "shows only user's point information" do @@ -48,7 +48,7 @@ describe 'as user' do let(:current_user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when course ID given' do it "shows only user's point information" do @@ -99,7 +99,7 @@ describe "GET user's points" do describe 'as admin' do let(:current_user) { FactoryBot.create(:admin) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when course ID given' do it "shows only user's point information" do @@ -126,7 +126,7 @@ describe 'as user' do let(:current_user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when course ID given' do it "shows only user's point information" do @@ -153,7 +153,7 @@ describe 'as another user' do let(:current_user) { user2 } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when course ID given' do it "shows only user's point information" do diff --git a/spec/controllers/api/v8/courses/users/submissions_controller_spec.rb b/spec/controllers/api/v8/courses/users/submissions_controller_spec.rb index 9558e0608..4fc9f0352 100644 --- a/spec/controllers/api/v8/courses/users/submissions_controller_spec.rb +++ b/spec/controllers/api/v8/courses/users/submissions_controller_spec.rb @@ -38,7 +38,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: admin.id, acceptable?: true } + let!(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } it "should show given user's submissions" do users_own_subs(course_id: course.id, user_id: user.id) @@ -62,7 +62,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: teacher.id, acceptable?: true } + let!(:token) { double resource_owner_id: teacher.id, accessible?: true, includes_scope?: false } it "should show given user's submissions in my organizations" do users_own_subs(course_id: course.id, user_id: user.id) @@ -90,7 +90,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: assistant.id, acceptable?: true } + let!(:token) { double resource_owner_id: assistant.id, accessible?: true, includes_scope?: false } it "should show given user's submissions in my courses" do users_own_subs(course_id: course.id, user_id: user.id) @@ -118,7 +118,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: user.id, acceptable?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } it 'should show my own submissions' do users_own_subs(course_id: course.id, user_id: user.id) @@ -163,7 +163,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: user.id, acceptable?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } it 'should show my own submissions' do users_own_subs(course_id: course.id, user_id: 'current') diff --git a/spec/controllers/api/v8/courses_controller_spec.rb b/spec/controllers/api/v8/courses_controller_spec.rb index a497c1f06..8ad6d32af 100644 --- a/spec/controllers/api/v8/courses_controller_spec.rb +++ b/spec/controllers/api/v8/courses_controller_spec.rb @@ -15,7 +15,7 @@ describe 'GET course by id' do describe 'as admin' do let(:current_user) { admin } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when course ID given' do it 'shows course information' do @@ -45,7 +45,7 @@ describe 'as user' do let(:current_user) { user } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when course ID given' do it 'shows course information' do diff --git a/spec/controllers/api/v8/courses_mooc_fi_token_auth_spec.rb b/spec/controllers/api/v8/courses_mooc_fi_token_auth_spec.rb index 7b17cf17c..9a893ccb0 100644 --- a/spec/controllers/api/v8/courses_mooc_fi_token_auth_spec.rb +++ b/spec/controllers/api/v8/courses_mooc_fi_token_auth_spec.rb @@ -52,7 +52,7 @@ def introspection_returns(value) it 'prefers a native Doorkeeper token' do user = FactoryBot.create(:user) - allow(controller).to receive(:doorkeeper_token).and_return(double(resource_owner_id: user.id, acceptable?: true)) + allow(controller).to receive(:doorkeeper_token).and_return(double(resource_owner_id: user.id, accessible?: true, includes_scope?: false)) expect(CoursesMoocFiTokenIntrospector).not_to receive(:introspect) get :index expect(current_user).to eq(user) diff --git a/spec/controllers/api/v8/organizations/courses/exercises/points_controller_spec.rb b/spec/controllers/api/v8/organizations/courses/exercises/points_controller_spec.rb index 7b2b7e1fa..7c1b8fd64 100644 --- a/spec/controllers/api/v8/organizations/courses/exercises/points_controller_spec.rb +++ b/spec/controllers/api/v8/organizations/courses/exercises/points_controller_spec.rb @@ -22,7 +22,7 @@ end describe 'As an admin' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } describe 'when searching for all users awarded points' do describe 'using course name' do it 'should return all users awarded points of the exercise' do @@ -48,7 +48,7 @@ end describe 'As any user' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } describe 'when searching awarded points' do describe 'and no points are found' do it 'should return an empty array' do diff --git a/spec/controllers/api/v8/organizations/courses/exercises/users/points_controller_spec.rb b/spec/controllers/api/v8/organizations/courses/exercises/users/points_controller_spec.rb index 41d7eca96..04ed394d2 100644 --- a/spec/controllers/api/v8/organizations/courses/exercises/users/points_controller_spec.rb +++ b/spec/controllers/api/v8/organizations/courses/exercises/users/points_controller_spec.rb @@ -31,7 +31,7 @@ end describe 'As any user' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } describe 'when searching for users awarded points by user id' do describe 'and using course id' do it 'should return only correct users awarded points' do diff --git a/spec/controllers/api/v8/organizations/courses/exercises_controller_spec.rb b/spec/controllers/api/v8/organizations/courses/exercises_controller_spec.rb index faacf1cca..2229d891f 100644 --- a/spec/controllers/api/v8/organizations/courses/exercises_controller_spec.rb +++ b/spec/controllers/api/v8/organizations/courses/exercises_controller_spec.rb @@ -20,7 +20,7 @@ end describe 'As an admin' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } describe 'when course name is given' do it 'should return successful response' do get :index, params: { course_name: course_name, organization_slug: organization.slug } @@ -38,7 +38,7 @@ end describe 'As a student' do - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } describe 'when course name is given' do it 'should return successful response' do get :index, params: { course_name: course_name, organization_slug: organization.slug } @@ -56,7 +56,7 @@ end describe 'As any user' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } describe 'when course name could not be found' do it 'should return error' do get :index, params: { course_name: 'null', organization_slug: organization.slug } @@ -67,7 +67,7 @@ describe 'Downloading an exercise zip' do describe 'as an unauthenticated user' do - let(:token) { double resource_owner_id: Guest.new.id, acceptable?: true } + let(:token) { double resource_owner_id: Guest.new.id, accessible?: true, includes_scope?: false } it 'should succeed if the course name and slug are correct', driver: :rack_test do repo = clone_course_repo(course) diff --git a/spec/controllers/api/v8/organizations/courses/points_controller_spec.rb b/spec/controllers/api/v8/organizations/courses/points_controller_spec.rb index 340a07f58..0437adfa9 100644 --- a/spec/controllers/api/v8/organizations/courses/points_controller_spec.rb +++ b/spec/controllers/api/v8/organizations/courses/points_controller_spec.rb @@ -21,7 +21,7 @@ describe 'GET points for a course' do describe 'as admin' do let(:current_user) { FactoryBot.create(:admin) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when course name given' do it "shows only user's point information" do @@ -48,7 +48,7 @@ describe 'as user' do let(:current_user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when course name given' do it "shows only user's point information" do diff --git a/spec/controllers/api/v8/organizations/courses/submissions_controller_spec.rb b/spec/controllers/api/v8/organizations/courses/submissions_controller_spec.rb index 51c576cae..6ff237e0d 100644 --- a/spec/controllers/api/v8/organizations/courses/submissions_controller_spec.rb +++ b/spec/controllers/api/v8/organizations/courses/submissions_controller_spec.rb @@ -39,7 +39,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: admin.id, acceptable?: true } + let!(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } it 'should show all of the submissions' do two_subs_by_name @@ -63,7 +63,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: teacher.id, acceptable?: true } + let!(:token) { double resource_owner_id: teacher.id, accessible?: true, includes_scope?: false } it 'should show all of the submissions in my organizations' do two_subs_by_name @@ -91,7 +91,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: assistant.id, acceptable?: true } + let!(:token) { double resource_owner_id: assistant.id, accessible?: true, includes_scope?: false } it 'should show all of the submissions in my courses' do two_subs_by_name @@ -119,7 +119,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: user.id, acceptable?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } it 'should show my own submissions' do all_own_subs(organization_slug: organization.slug, course_name: course_name) diff --git a/spec/controllers/api/v8/organizations/courses/users/points_controller_spec.rb b/spec/controllers/api/v8/organizations/courses/users/points_controller_spec.rb index bd5256b66..e4ec94ee1 100644 --- a/spec/controllers/api/v8/organizations/courses/users/points_controller_spec.rb +++ b/spec/controllers/api/v8/organizations/courses/users/points_controller_spec.rb @@ -22,7 +22,7 @@ describe "GET current user's points" do describe 'as admin' do let(:current_user) { FactoryBot.create(:admin) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when course name given' do it "shows only user's point information" do @@ -49,7 +49,7 @@ describe 'as user' do let(:current_user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when course name given' do it "shows only user's point information" do @@ -100,7 +100,7 @@ describe "GET user's points" do describe 'as admin' do let(:current_user) { FactoryBot.create(:admin) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when course name given' do it "shows only user's point information" do @@ -127,7 +127,7 @@ describe 'as user' do let(:current_user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when course name given' do it "shows only user's point information" do @@ -154,7 +154,7 @@ describe 'as another user' do let(:current_user) { user2 } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when course name given' do it "shows only user's point information" do diff --git a/spec/controllers/api/v8/organizations/courses/users/submissions_controller_spec.rb b/spec/controllers/api/v8/organizations/courses/users/submissions_controller_spec.rb index f2f9bed6c..9fda51ddf 100644 --- a/spec/controllers/api/v8/organizations/courses/users/submissions_controller_spec.rb +++ b/spec/controllers/api/v8/organizations/courses/users/submissions_controller_spec.rb @@ -38,7 +38,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: admin.id, acceptable?: true } + let!(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } it "should show given user's submissions" do users_own_subs(organization_slug: organization.slug, course_name: course_name, user_id: user.id) @@ -62,7 +62,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: teacher.id, acceptable?: true } + let!(:token) { double resource_owner_id: teacher.id, accessible?: true, includes_scope?: false } it "should show given user's submissions in my organizations" do users_own_subs(organization_slug: organization.slug, course_name: course_name, user_id: user.id) @@ -90,7 +90,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: assistant.id, acceptable?: true } + let!(:token) { double resource_owner_id: assistant.id, accessible?: true, includes_scope?: false } it "should show given user's submissions in my courses" do users_own_subs(organization_slug: organization.slug, course_name: course_name, user_id: user.id) @@ -118,7 +118,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: user.id, acceptable?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } it 'should show my own submissions' do users_own_subs(organization_slug: organization.slug, course_name: course_name, user_id: user.id) @@ -163,7 +163,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: user.id, acceptable?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } it 'should show my own submissions' do users_own_subs(organization_slug: organization.slug, course_name: course_name, user_id: 'current') diff --git a/spec/controllers/api/v8/organizations/courses_controller_spec.rb b/spec/controllers/api/v8/organizations/courses_controller_spec.rb index c573445c3..32571d48e 100644 --- a/spec/controllers/api/v8/organizations/courses_controller_spec.rb +++ b/spec/controllers/api/v8/organizations/courses_controller_spec.rb @@ -17,7 +17,7 @@ describe 'GET course by name' do describe 'when logged as admin' do let(:current_user) { admin } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when organization id and course name given' do it 'shows course information' do @@ -60,7 +60,7 @@ describe 'when logged as user' do let(:current_user) { user } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when organization id and course name given' do it 'shows course information' do diff --git a/spec/controllers/api/v8/users_controller_spec.rb b/spec/controllers/api/v8/users_controller_spec.rb index 66fc3b696..d83c97832 100644 --- a/spec/controllers/api/v8/users_controller_spec.rb +++ b/spec/controllers/api/v8/users_controller_spec.rb @@ -12,7 +12,7 @@ end describe "GET current user's info with an access token" do - let!(:token) { double resource_owner_id: user.id, acceptable?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } it 'user sees own info' do get :show, params: { id: 'current' } @@ -30,7 +30,7 @@ end context 'as an administrator' do - let!(:token) { double resource_owner_id: admin.id, acceptable?: true } + let!(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } it 'user sees own info' do get :show, params: { id: 'current' } @@ -40,7 +40,7 @@ end describe "GET user's info with user id" do describe 'using user token' do - let!(:token) { double resource_owner_id: user.id, acceptable?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } it 'user sees own info' do get :show, params: { id: user.id } @@ -57,7 +57,7 @@ end end describe 'using admin token' do - let!(:token) { double resource_owner_id: admin.id, acceptable?: true } + let!(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } it "admin can see everyone's infos" do get :show, params: { id: user.id } @@ -123,7 +123,7 @@ end describe 'PUT update password' do - let!(:token) { double resource_owner_id: user.id, acceptable?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } before :each do user.password = 'oldpassword' @@ -334,4 +334,63 @@ def authenticate_with_token_as(token_user) expect(user.reload.password_managed_by_courses_mooc_fi).to eq(false) end end + + describe 'with a handoff token' do + let(:application) { Doorkeeper::Application.create!(name: 'mooc.fi frontend', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob') } + let(:token) { Doorkeeper::AccessToken.create!(application: application, resource_owner_id: user.id, scopes: 'public handoff') } + + before :each do + allow(ExtraField).to receive(:by_kind).with(:user).and_return([UserField.new(name: 'first_name'), UserField.new(name: 'last_name')]) + user.password = 'oldpassword' + user.save! + end + + it 'updates the name when the rest of the profile is sent back unchanged' do + put :update, params: { id: user.id, user: { email: user.email.upcase }, user_field: { first_name: 'Ada', last_name: 'Lovelace' } } + expect(response).to have_http_status(200) + expect(UserFieldValue.where(user: user).pluck(:field_name, :value).to_h).to eq('first_name' => 'Ada', 'last_name' => 'Lovelace') + end + + it 'refuses to change the email' do + put :update, params: { id: 'current', user: { email: 'taken-over@example.com' }, user_field: { first_name: 'Ada' } } + expect(response).to have_http_status(403) + expect(user.reload.email).not_to eq('taken-over@example.com') + expect(UserFieldValue.where(user: user, field_name: 'first_name')).to be_empty + end + + it 'refuses to change the password' do + put :update, params: { id: 'current', user: { email: user.email }, old_password: 'oldpassword', password: 'newpassword', password_repeat: 'newpassword' } + expect(response).to have_http_status(403) + expect(user.reload).to have_password('oldpassword') + end + + it 'refuses to delete the user' do + delete :destroy, params: { id: user.id } + expect(response).to have_http_status(403) + expect(User.find_by(id: user.id)).not_to be_nil + end + + context 'when the token is an ordinary login token' do + let(:token) { Doorkeeper::AccessToken.create!(application: application, resource_owner_id: user.id, scopes: 'public') } + + it 'changes the email' do + put :update, params: { id: 'current', user: { email: 'new@example.com' } } + expect(response).to have_http_status(200) + expect(user.reload.email).to eq('new@example.com') + end + + it 'changes the password' do + allow_any_instance_of(User).to receive(:post_new_user_to_courses_mooc_fi).and_return(true) + put :update, params: { id: 'current', user: { email: user.email }, old_password: 'oldpassword', password: 'newpassword', password_repeat: 'newpassword' } + expect(response).to have_http_status(200) + expect(user.reload).to have_password('newpassword') + end + + it 'deletes the user' do + delete :destroy, params: { id: user.id } + expect(response).to have_http_status(200) + expect(User.find_by(id: user.id)).to be_nil + end + end + end end diff --git a/spec/controllers/auths_controller_spec.rb b/spec/controllers/auths_controller_spec.rb index 94e9b50df..8f902f0cf 100644 --- a/spec/controllers/auths_controller_spec.rb +++ b/spec/controllers/auths_controller_spec.rb @@ -43,4 +43,16 @@ expect(response).to be_successful expect(response.body).to eq('OK') end + + it 'accepts an access token only while it is not revoked' do + application = Doorkeeper::Application.create!(name: 'client', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob') + token = Doorkeeper::AccessToken.create!(application: application, resource_owner_id: @user.id, scopes: 'public') + + post :show, params: { username: 'foo', session_id: token.token, format: :text } + expect(response.body).to eq('OK') + + token.revoke + post :show, params: { username: 'foo', session_id: token.token, format: :text } + expect(response.body).to eq('FAIL') + end end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 0a7971bcf..4ea7df95f 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -252,7 +252,7 @@ end def do_destroy_user(password) - delete :destroy_user, params: { user_id: user.id, id: verification_token.token, im_sure: '1', + delete :destroy_user, params: { user_id: user.id, token: verification_token.token, im_sure: '1', user: { password: password } } end diff --git a/spec/requests/handoff_token_consumers_spec.rb b/spec/requests/handoff_token_consumers_spec.rb new file mode 100644 index 000000000..ab787f1b1 --- /dev/null +++ b/spec/requests/handoff_token_consumers_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Handoff tokens of users who became privileged', type: :request do + let(:user) { FactoryBot.create(:user) } + let(:application) { Doorkeeper::Application.create!(name: 'client', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob') } + let(:token) { Doorkeeper::AccessToken.create!(application: application, resource_owner_id: user.id, scopes: 'public handoff') } + + before { user.update!(administrator: true) } + + it 'are accepted by the beta API before the user became privileged' do + user.update!(administrator: false) + get '/api/beta/participant', headers: { 'Authorization' => "Bearer #{token.token}" } + + expect(response).to have_http_status(200) + end + + it 'are refused by the beta API' do + get '/api/beta/participant', headers: { 'Authorization' => "Bearer #{token.token}" } + + expect(response).to have_http_status(401) + end + + it 'are refused by /auth' do + get '/auth.text', params: { username: user.login, session_id: token.token } + + expect(response.body).to eq('FAIL') + end +end diff --git a/spec/requests/issuer_token_hardening_spec.rb b/spec/requests/issuer_token_hardening_spec.rb new file mode 100644 index 000000000..27d27440e --- /dev/null +++ b/spec/requests/issuer_token_hardening_spec.rb @@ -0,0 +1,119 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Issuer token hardening', type: :request do + let!(:issuer_app) { Doorkeeper::Application.create!(name: 'mooc.fi backend', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob', scopes: 'issue_user_access_tokens') } + + describe 'client_credentials token' do + def request_token + post '/oauth/token', params: { grant_type: 'client_credentials', client_id: issuer_app.uid, client_secret: issuer_app.plaintext_secret, scope: 'issue_user_access_tokens' } + end + + it 'expires in one hour' do + request_token + + expect(response).to have_http_status(200) + expect(JSON.parse(response.body)['expires_in']).to eq(3600) + end + + it 'leaves client_credentials tokens without the issuer scope non-expiring' do + issuer_app.update!(scopes: 'public issue_user_access_tokens') + post '/oauth/token', params: { grant_type: 'client_credentials', client_id: issuer_app.uid, client_secret: issuer_app.plaintext_secret, scope: 'public' } + + expect(response).to have_http_status(200) + expect(JSON.parse(response.body)).not_to have_key('expires_in') + end + + it 'leaves password-grant tokens non-expiring' do + user = FactoryBot.create(:user, password: 'hunter2hunter2') + post '/oauth/token', params: { grant_type: 'password', client_id: issuer_app.uid, client_secret: issuer_app.plaintext_secret, username: user.login, password: 'hunter2hunter2' } + + expect(response).to have_http_status(200) + expect(JSON.parse(response.body)).not_to have_key('expires_in') + end + end + + describe 'parameter filtering' do + let(:filter) { ActiveSupport::ParameterFilter.new(Rails.application.config.filter_parameters) } + + it 'hides credentials and tokens' do + params = { client_secret: 's', access_token: 'a', refresh_token: 'r', token: 't', code: 'c', bearer_token: 'b', id_token: 'i', id_token_hint: 'h', code_verifier: 'v' } + + expect(filter.filter(params).values.uniq).to eq(['[FILTERED]']) + end + + it 'keeps similarly named diagnostics visible' do + params = { exit_code: 1, return_code: 2, token_count: 3 } + + expect(filter.filter(params)).to eq(params) + end + end + + describe 'rate limiting' do + let(:app) { Rack::Attack.new(->(_env) { [200, {}, ['ok']] }) } + let(:safe_headers) { { 'HTTP_RATELIMIT_PROTECTION_SAFE_API_KEY' => 'safe-key' } } + + around :each do |example| + Rack::Attack.enabled = true + Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new + ENV['RACK_ATTACK_SAFE_API_KEY'] = 'safe-key' + example.run + ensure + Rack::Attack.enabled = false + ENV.delete('RACK_ATTACK_SAFE_API_KEY') + end + + def status_after(times, path, headers = {}) + status = nil + times.times { status, = app.call(Rack::MockRequest.env_for(path, { method: 'POST', 'REMOTE_ADDR' => '203.0.113.7' }.merge(headers))) } + status + end + + it 'throttles the issue endpoint even with the safe API key' do + expect(status_after(120, '/api/v8/application/user_access_tokens', safe_headers)).to eq(200) + expect(status_after(1, '/api/v8/application/user_access_tokens', safe_headers)).to eq(503) + end + + it 'does not throttle safelisted oauth token requests' do + expect(status_after(25, '/oauth/token', safe_headers)).to eq(200) + end + + it 'throttles unsafelisted oauth token requests' do + expect(status_after(100, '/oauth/token')).to eq(200) + expect(status_after(1, '/oauth/token')).to eq(503) + end + end + describe 'Rack::Attack in the real stack' do + around :each do |example| + Rack::Attack.enabled = true + Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new + example.run + ensure + Rack::Attack.enabled = false + end + + def hit_token_endpoint(times, env) + times.times { post '/oauth/token', env: env } + end + + it 'is in the middleware stack exactly once' do + expect(Rails.application.middleware.count { |m| m.klass == Rack::Attack }).to eq(1) + end + + it 'throttles by client address' do + hit_token_endpoint(101, 'REMOTE_ADDR' => '203.0.113.7') + + expect(response).to have_http_status(503) + end + + it 'buckets by the forwarded client address behind a private-range proxy' do + proxied = { 'REMOTE_ADDR' => '10.0.0.1' } + hit_token_endpoint(101, proxied.merge('HTTP_X_FORWARDED_FOR' => '203.0.113.9')) + expect(response).to have_http_status(503) + + hit_token_endpoint(1, proxied.merge('HTTP_X_FORWARDED_FOR' => '203.0.113.10')) + expect(response).not_to have_http_status(503) + end + end +end diff --git a/spec/requests/oauth_token_info_spec.rb b/spec/requests/oauth_token_info_spec.rb new file mode 100644 index 000000000..b60663eb3 --- /dev/null +++ b/spec/requests/oauth_token_info_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require 'spec_helper' + +# courses.mooc.fi reads the issuing application of forwarded TMC tokens from this endpoint. +describe 'GET /oauth/token/info', type: :request do + let(:user) { FactoryBot.create(:user) } + let(:application) { Doorkeeper::Application.create!(name: 'mooc.fi frontend', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob') } + + def token_info(bearer) + get '/oauth/token/info', headers: { 'Authorization' => "Bearer #{bearer}" } + end + + it 'reports the issuing application of a handoff token' do + token = Doorkeeper::AccessToken.create!(application: application, resource_owner_id: user.id, scopes: 'public handoff', expires_in: 86_400) + token_info(token.token) + + expect(response).to have_http_status(200) + body = JSON.parse(response.body) + expect(body['application']).to eq('uid' => application.uid) + expect(body['resource_owner_id']).to eq(user.id) + expect(body['scope']).to eq(%w[public handoff]) + end + + it 'refuses a revoked token' do + token = Doorkeeper::AccessToken.create!(application: application, resource_owner_id: user.id, scopes: 'public') + token.revoke + token_info(token.token) + + expect(response).to have_http_status(401) + end + + context 'with a courses.mooc.fi token' do + around :each do |example| + original = Rails.configuration.x.accept_courses_mooc_fi_tokens + Rails.configuration.x.accept_courses_mooc_fi_tokens = true + example.run + ensure + Rails.configuration.x.accept_courses_mooc_fi_tokens = original + end + + it 'reports no application' do + bearer = 'sp331-access-token' + user.update_column(:courses_mooc_fi_user_id, '11111111-2222-3333-4444-555555555555') + result = CoursesMoocFiTokenIntrospector::Result.new(sub: user.courses_mooc_fi_user_id, upstream_id: nil, expires_at: Time.now + 3600) + allow(CoursesMoocFiTokenIntrospector).to receive(:introspect).with(bearer).and_return(result) + + token_info(bearer) + + expect(response).to have_http_status(401) + expect(response.body).not_to include('uid') + end + end +end diff --git a/spec/requests/rack_attack_throttles_spec.rb b/spec/requests/rack_attack_throttles_spec.rb new file mode 100644 index 000000000..3f0a65450 --- /dev/null +++ b/spec/requests/rack_attack_throttles_spec.rb @@ -0,0 +1,148 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Rack::Attack throttles', type: :request do + let(:app) { Rack::Attack.new(->(_env) { [200, {}, ['ok']] }) } + let(:safe_headers) { { 'HTTP_RATELIMIT_PROTECTION_SAFE_API_KEY' => 'safe-key' } } + + around :each do |example| + Rack::Attack.enabled = true + Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new + ENV['RACK_ATTACK_SAFE_API_KEY'] = 'safe-key' + example.run + ensure + Rack::Attack.enabled = false + ENV.delete('RACK_ATTACK_SAFE_API_KEY') + end + + def call(path, method: 'POST', ip: '203.0.113.7', **options) + env = Rack::MockRequest.env_for(path, { method: method, 'REMOTE_ADDR' => ip }.merge(options)) + # env_for would read the start of //x as a host name. + env['PATH_INFO'] = path.split('?').first + status, = app.call(env) + status + end + + def status_after(times, path, **options) + status = nil + times.times { status = call(path, **options) } + status + end + + def basic(username, password = 'guess') + { 'HTTP_AUTHORIZATION' => ActionController::HttpAuthentication::Basic.encode_credentials(username, password) } + end + + describe 'path variants' do + { + '/oauth/token' => 100, + '/sessions' => 100, + '/api/v8/users' => 60, + '/password_reset_keys' => 10, + '/api/v8/users/password_reset' => 10, + '/api/v8/application/user_access_tokens' => 120 + }.each do |path, limit| + ["#{path}/", "/#{path}", "#{path}.json"].each do |variant| + it "throttles #{variant} together with #{path}" do + expect(status_after(limit, path)).to eq(200) + expect(call(variant)).to eq(503) + end + end + end + + %w[/api/v8/application/user_access_tokens/ //api/v8/application/user_access_tokens /api/v8/application/user_access_tokens.json].each do |variant| + it "keeps #{variant} out of the safelist" do + expect(status_after(120, variant, **safe_headers)).to eq(200) + expect(call(variant, **safe_headers)).to eq(503) + end + end + end + + describe 'Basic auth and /auth password checks per IP' do + it 'throttles Basic auth on web URLs' do + 300.times { |i| call('/courses', method: 'GET', **basic("user#{i}")) } + + expect(call('/courses', method: 'GET', **basic('fresh-user'))).to eq(503) + end + + it 'throttles /auth password checks' do + 300.times { |i| call('/auth', method: 'GET', params: { username: "user#{i}", password: 'x' }) } + + expect(call('/auth.text', params: { username: 'fresh-user', password: 'x' })).to eq(503) + end + + it 'counts query parameters when a JSON body is empty' do + 300.times { |i| call("/auth?username=user#{i}&password=x", method: 'GET', 'CONTENT_TYPE' => 'application/json') } + + expect(call('/auth?username=fresh-user&password=x', method: 'GET', 'CONTENT_TYPE' => 'application/json')).to eq(503) + end + + it 'ignores client credentials sent as Basic auth to Doorkeeper' do + expect(status_after(301, '/oauth/introspect', **basic('client-id'))).to eq(200) + end + + it 'ignores /auth session checks' do + expect(status_after(301, '/auth.text', method: 'GET', params: { username: 'victim', session_id: 'abc' })).to eq(200) + end + + it 'leaves the login bucket of the address alone' do + 300.times { |i| call('/courses', method: 'GET', **basic("user#{i}")) } + + expect(call('/sessions', params: { session: { login: 'fresh-user', password: 'x' } })).to eq(200) + end + end + + describe 'user access tokens per target user' do + let(:path) { '/api/v8/application/user_access_tokens' } + let(:issuer_app) { Doorkeeper::Application.create!(name: 'mooc.fi backend', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob', scopes: 'issue_user_access_tokens') } + let(:issuer_token) { Doorkeeper::AccessToken.create!(application: issuer_app, scopes: 'issue_user_access_tokens') } + let(:bearer) { { 'HTTP_AUTHORIZATION' => "Bearer #{issuer_token.token}" } } + + it 'throttles one user without affecting others' do + 10.times { |i| call(path, ip: "198.51.100.#{i}", **safe_headers, **bearer, params: { user_id: '42' }) } + + expect(call(path, ip: '198.51.100.99', **bearer, params: { user_id: '042' })).to eq(503) + expect(call(path, ip: '198.51.100.99', **bearer, params: { user_id: '43' })).to eq(200) + end + + it 'reads user_id from a JSON body' do + body = { user_id: 42 }.to_json + 10.times { |i| call(path, ip: "198.51.100.#{i}", input: body, 'CONTENT_TYPE' => 'application/json', **bearer) } + + expect(call(path, ip: '198.51.100.99', input: body, 'CONTENT_TYPE' => 'application/json', **bearer)).to eq(503) + end + + it 'does not let requests without a usable issuer token use up the budget' do + user_token = Doorkeeper::AccessToken.create!(application: issuer_app, resource_owner_id: 1, scopes: 'issue_user_access_tokens') + 10.times { |i| call(path, ip: "198.51.100.#{i}", params: { user_id: '42' }) } + 10.times { |i| call(path, ip: "198.51.100.#{i + 20}", 'HTTP_AUTHORIZATION' => 'Bearer forged', params: { user_id: '42' }) } + 10.times { |i| call(path, ip: "198.51.100.#{i + 40}", 'HTTP_AUTHORIZATION' => "Bearer #{user_token.token}", params: { user_id: '42' }) } + + expect(call(path, ip: '198.51.100.99', **bearer, params: { user_id: '42' })).to eq(200) + end + end + + describe 'password resets per email' do + it 'normalizes the address as the controller does' do + %w[Victim@x.fi victim@X.fi].each { |email| call('/password_reset_keys', ip: '198.51.100.1', params: { email: email }) } + call('/password_reset_keys', ip: '198.51.100.2', params: { email: ' victim@x.fi' }) + call('/password_reset_keys', ip: '198.51.100.3', input: { email: 'VICTIM@x.fi' }.to_json, 'CONTENT_TYPE' => 'application/json') + call('/password_reset_keys', ip: '198.51.100.4', params: { email: 'victim@x.fi' }) + + expect(call('/password_reset_keys', ip: '198.51.100.5', params: { email: 'victim@x.fi' })).to eq(503) + expect(call('/api/v8/users/password_reset', ip: '198.51.100.6', params: { email: 'victim@x.fi' })).to eq(503) + end + end + + describe 'submissions per user to api v8' do + let(:user) { FactoryBot.create(:user) } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public') } + + it 'reads the token from the Authorization header' do + 250.times { |i| call('/api/v8/core/exercises/1/submissions', ip: "198.51.100.#{i % 200}", 'HTTP_AUTHORIZATION' => "Bearer #{token.token}") } + + expect(call('/api/v8/core/exercises/1/submissions', ip: '203.0.113.99', params: { access_token: token.token })).to eq(503) + end + end +end diff --git a/spec/requests/secret_path_logging_spec.rb b/spec/requests/secret_path_logging_spec.rb new file mode 100644 index 000000000..e9fad6c29 --- /dev/null +++ b/spec/requests/secret_path_logging_spec.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Tokens in emailed link paths', type: :request do + let(:user) { FactoryBot.create(:user, password: 'secret123') } + + def instrumented(event) + payloads = [] + subscriber = ActiveSupport::Notifications.subscribe(event) { |*, payload| payloads << payload } + yield + payloads + ensure + ActiveSupport::Notifications.unsubscribe(subscriber) + end + + def logged_path_and_params(&block) + payload = instrumented('process_action.action_controller', &block).last + [payload[:path], payload[:request].filtered_parameters.to_s] + end + + it 'keeps the email verification token out of the request log and still verifies' do + token = VerificationToken.create!(user: user, type: :email) + path, params = logged_path_and_params { get "/users/#{user.id}/verify/#{token.token}" } + + expect(path).to eq("/users/#{user.id}/verify/[FILTERED]") + expect(params).not_to include(token.token) + expect(user.reload.email_verified).to be(true) + end + + it 'keeps the password reset token out of the request log' do + key = ActionToken.generate_password_reset_key_for(user) + path, params = logged_path_and_params { get "/reset_password/#{key.token}" } + + expect(path).to eq('/reset_password/[FILTERED]') + expect(params).not_to include(key.token) + expect(response).to have_http_status(200) + expect(response.body).not_to include("key=#{key.token}") + end + + it 'keeps the account deletion token out of redirect logs' do + token = VerificationToken.create!(user: user, type: :delete_user) + post '/sessions', params: { session: { login: user.login, password: 'secret123' } } + + payloads = instrumented('redirect_to.action_controller') do + delete "/users/#{user.id}/destroy/#{token.token}", params: { im_sure: '1', user: { password: 'wrong' } } + end + + expect(response).to redirect_to("http://www.example.com/users/#{user.id}/destroy/#{token.token}") + expect(payloads.map { |payload| payload[:location] }).to eq(['[FILTERED]']) + end + + it 'filters the token from Rack::Attack log lines' do + expect(SecretPathFilter.filter('//users/1/destroy/abc.json')).to eq('//users/1/destroy/[FILTERED]') + expect(SecretPathFilter.filter('/reset_password')).to eq('/reset_password') + end +end