diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 01f31093f..0b36aa814 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -130,6 +130,24 @@ jobs: - name: Run spec_lib tests run: docker compose run web bin/spec_lib + spec_requests: + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v2 + with: + submodules: false + - name: Setup git submodules + run: | + sed -i 's/git@github.com:/https:\/\/github.com\//' .gitmodules + git submodule update --init --recursive + - name: Docker compose build + run: docker compose build + - name: Setup database & test env + run: docker compose run web bin/rake db:create db:schema:load RAILS_ENV=test + - name: Run spec_requests tests + run: docker compose run web bin/spec_requests + spec_integration: runs-on: ubuntu-latest steps: diff --git a/app/background_tasks/access_token_cleanup_task.rb b/app/background_tasks/access_token_cleanup_task.rb new file mode 100644 index 000000000..b3501084a --- /dev/null +++ b/app/background_tasks/access_token_cleanup_task.rb @@ -0,0 +1,29 @@ +# typed: strict +# frozen_string_literal: true + +# Deletes Doorkeeper access tokens that expired or were revoked over RETENTION ago. Handoff and +# issuer tokens expire daily and hourly, so their rows would otherwise pile up. +class AccessTokenCleanupTask + extend T::Sig + + # Until a row is deleted, presenting its token gets "Invalid token" instead of being taken for a + # courses.mooc.fi token. + RETENTION = T.let(30.days, ActiveSupport::Duration) + BATCH_SIZE = 1000 + + sig { void } + def run + cutoff = RETENTION.ago + tokens = Doorkeeper::AccessToken + stale = tokens.where(revoked_at: ...cutoff) + .or(tokens.where.not(expires_in: nil).where("created_at + expires_in * INTERVAL '1 second' < ?", cutoff)) + deleted = 0 + stale.in_batches(of: BATCH_SIZE) { |batch| deleted += batch.delete_all } + Rails.logger.info("Deleted #{deleted} stale access tokens") if deleted.positive? + end + + sig { returns(Integer) } + def wait_delay + 1.hour.to_i + end +end diff --git a/app/controllers/api/beta/base_controller.rb b/app/controllers/api/beta/base_controller.rb index a229ffffd..01941fff3 100644 --- a/app/controllers/api/beta/base_controller.rb +++ b/app/controllers/api/beta/base_controller.rb @@ -36,7 +36,7 @@ def present(hash) sig { void } def authenticate_user! if (token = doorkeeper_token) - @current_user = T.let(User.find_by(id: token.resource_owner_id), T.nilable(User)) + @current_user = T.let(AccessTokenAuthentication.user_for(token), T.nilable(User)) end return if @current_user diff --git a/app/controllers/api/v8/apidocs_controller.rb b/app/controllers/api/v8/apidocs_controller.rb index 8758e309e..4de82f798 100644 --- a/app/controllers/api/v8/apidocs_controller.rb +++ b/app/controllers/api/v8/apidocs_controller.rb @@ -190,6 +190,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..50d3a5301 --- /dev/null +++ b/app/controllers/api/v8/application/user_access_tokens_controller.rb @@ -0,0 +1,137 @@ +# typed: strict +# 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 + extend T::Sig + include Swagger::Blocks + + TOKEN_LIFETIME = T.let(1.day, ActiveSupport::Duration) + + 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 + + sig { void } + def create + issuer_token = doorkeeper_token + return respond_unauthorized unless issuer_token&.accessible? + + issuer_uid = issuer_token.application&.uid + unless authorized_issuer?(issuer_token) + log_refusal('not an allowed issuer', issuer_uid) + return respond_forbidden + end + + return respond_with_error('user_id must be an integer', 400) unless params[:user_id].to_s.match?(/\A\d+\z/) + + user_id = params[:user_id].to_i + user = User.find_by(id: user_id) + unless user + log_refusal('unknown user', issuer_uid, user_id) + return respond_not_found('User not found') + end + if user.privileged? + log_refusal('privileged user', issuer_uid, user_id) + return respond_forbidden('Tokens are not issued for administrators, teachers or assistants') + end + + target_application = Doorkeeper::Application.find_by(uid: AccessTokenAuthentication.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=#{issuer_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 + sig { params(reason: String, issuer_uid: T.nilable(String), user_id: T.nilable(Integer)).void } + def log_refusal(reason, issuer_uid, user_id = nil) + Rails.logger.warn("Refused to issue user access token (#{reason}): issuer application uid=#{issuer_uid} user_id=#{user_id}") + end + + # Their credentials are served publicly by the credentials endpoint. + sig { returns(T::Array[String]) } + def public_client_names + clients = SiteSetting.value('valid_clients') + return [] unless clients.is_a?(Array) + + clients.filter_map { |client| client['name'].to_s if client.is_a?(Hash) } + end + + sig { params(issuer_token: Doorkeeper::AccessToken).returns(T::Boolean) } + def authorized_issuer?(issuer_token) + # resource_owner_id is set on user tokens, which never qualify even from an allowed application. + return false unless AccessTokenAuthentication.issuer_token?(issuer_token) + + application = issuer_token.application + return false unless application + + # The target is a frontend client, so its credentials must not be usable to issue tokens. + AccessTokenAuthentication.issuer_application_uids.include?(application.uid) && + application.confidential? && + application.uid != AccessTokenAuthentication.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 6914fc65a..679b450e4 100644 --- a/app/controllers/api/v8/base_controller.rb +++ b/app/controllers/api/v8/base_controller.rb @@ -50,8 +50,11 @@ def present(hash) def authenticate_user! return if @current_user if (token = doorkeeper_token) - @current_user = T.let(User.find_by(id: token.resource_owner_id), T.nilable(User)) - raise 'Invalid token' unless @current_user + @current_user = T.let(AccessTokenAuthentication.user_for(token), T.nilable(User)) + return respond_unauthorized('Invalid token') unless @current_user + @auth_source = T.let(:handoff_token, T.nilable(Symbol)) if AccessTokenAuthentication.handoff?(token) + elsif presented_doorkeeper_token + return respond_unauthorized('Invalid token') elsif Rails.configuration.x.accept_courses_mooc_fi_tokens @current_user = CoursesMoocFiAuthentication.user_for(request) @auth_source = T.let(:courses_mooc_fi_token, T.nilable(Symbol)) if @current_user diff --git a/app/controllers/api/v8/users_controller.rb b/app/controllers/api/v8/users_controller.rb index 40d491526..b1f9d6c82 100644 --- a/app/controllers/api/v8/users_controller.rb +++ b/app/controllers/api/v8/users_controller.rb @@ -325,6 +325,7 @@ def update_email(user) 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 @@ -348,6 +349,7 @@ def set_password(user) sig { params(user: User).void } def maybe_update_password(user) 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/application_controller.rb b/app/controllers/application_controller.rb index 7726ecd8a..a3a942da2 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -95,10 +95,17 @@ def current_ability # The request's Doorkeeper access token, or nil when it is missing, revoked or expired. sig { returns(T.nilable(Doorkeeper::AccessToken)) } def doorkeeper_token - token = super + token = presented_doorkeeper_token token if token&.accessible? end + # The Doorkeeper access token the request presents, also when it is revoked or expired; nil when + # the request presents none or a token TMC did not issue. Authenticate with doorkeeper_token. + sig { returns(T.nilable(Doorkeeper::AccessToken)) } + def presented_doorkeeper_token + @presented_doorkeeper_token ||= T.let(Doorkeeper.authenticate(request), T.nilable(Doorkeeper::AccessToken)) + end + sig { void } def set_default_url_options Rails.application.routes.default_url_options[:host] = request.host_with_port diff --git a/app/controllers/auths_controller.rb b/app/controllers/auths_controller.rb index 7f05338b3..df9bfd9e0 100644 --- a/app/controllers/auths_controller.rb +++ b/app/controllers/auths_controller.rb @@ -13,10 +13,9 @@ class AuthsController < ApplicationController sig { void } 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.by_login_or_email(params[:username].to_s) # 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])&.accessible? + res = if user && AccessTokenAuthentication.user_for(Doorkeeper::AccessToken.by_token(params[:session_id].to_s)) == user OK_MESSAGE elsif user && find_session_by_id(params[:session_id])&.belongs_to?(user) OK_MESSAGE diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 08717d482..9cefb0d7e 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -109,7 +109,7 @@ def update sig { void } 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') @@ -130,7 +130,7 @@ def send_verification_email sig { void } 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 sig { void } @@ -146,7 +146,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/models/ability.rb b/app/models/ability.rb index 58a87ff5d..1c4729572 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -8,10 +8,13 @@ class Ability extend T::Sig 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. sig { params(user: User, auth_source: T.nilable(Symbol)).void } def initialize(user, auth_source: nil) + alias_action :update_credentials, to: :update + if user.administrator? can :manage, :all can :create, Course @@ -294,10 +297,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/assistantship.rb b/app/models/assistantship.rb index 5f49e2e45..ddaf17d75 100644 --- a/app/models/assistantship.rb +++ b/app/models/assistantship.rb @@ -2,6 +2,8 @@ # frozen_string_literal: true class Assistantship < ApplicationRecord + extend T::Sig + belongs_to :user belongs_to :course @@ -9,4 +11,12 @@ class Assistantship < ApplicationRecord presence: { message: 'does not exist' }, uniqueness: { scope: :course, message: 'is already an assistant for this course' } validates :course, presence: true + + after_save :revoke_handoff_tokens, if: :saved_change_to_user_id? + + private + sig { void } + def revoke_handoff_tokens + AccessTokenAuthentication.revoke_handoff_tokens(T.must(user)) + end end diff --git a/app/models/teachership.rb b/app/models/teachership.rb index ce17b67c9..187362994 100644 --- a/app/models/teachership.rb +++ b/app/models/teachership.rb @@ -2,6 +2,8 @@ # frozen_string_literal: true class Teachership < ApplicationRecord + extend T::Sig + belongs_to :user belongs_to :organization @@ -9,4 +11,12 @@ class Teachership < ApplicationRecord presence: { message: 'does not exist' }, uniqueness: { scope: :organization, message: 'is already in this organization' } validates :organization, presence: true + + after_save :revoke_handoff_tokens, if: :saved_change_to_user_id? + + private + sig { void } + def revoke_handoff_tokens + AccessTokenAuthentication.revoke_handoff_tokens(T.must(user)) + end end diff --git a/app/models/user.rb b/app/models/user.rb index e8430f1d5..ba6d5f6b4 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -75,6 +75,7 @@ class User < ApplicationRecord sig { returns(T.nilable(String)) } attr_accessor :password before_save :encrypt_password + after_save :revoke_handoff_tokens, if: :saved_change_to_administrator? sig { params(course: Course).returns(ActiveRecord::Relation) } def self.course_students(course) @@ -188,6 +189,15 @@ def has_password?(submitted_password) result end + # The user a typed username names, matched against login exactly and email case-insensitively. + sig { params(username: T.nilable(String)).returns(T.nilable(User)) } + def self.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. sig { params(login: T.nilable(String), submitted_password: T.nilable(String)).returns(T.nilable(User)) } @@ -202,10 +212,7 @@ def self.authenticate(login, submitted_password) # the status, so sign in only on :accepted. sig { params(login: T.nilable(String), submitted_password: T.nilable(String)).returns([T.nilable(User), Symbol]) } 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 = 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? @@ -515,6 +522,12 @@ def <=>(other) login.downcase <=> other.login.downcase end + # Administrators, teachers and assistants, whose accounts handoff tokens must never act as. + sig { returns(T::Boolean) } + def privileged? + administrator? || teacherships.exists? || assistantships.exists? + end + sig { params(organization: T.nilable(Organization)).returns(T::Boolean) } def teacher?(organization) organizations.include? organization @@ -650,6 +663,11 @@ def processing_submissions_count_for_exercise(exercise_name, course_id) end private + sig { void } + def revoke_handoff_tokens + AccessTokenAuthentication.revoke_handoff_tokens(self) if administrator? + end + # Unchecked because specs stub Faraday.new with plain doubles. sig { returns(Faraday::Connection).checked(:never) } def courses_mooc_fi_connection diff --git a/app/services/access_token_authentication.rb b/app/services/access_token_authentication.rb new file mode 100644 index 000000000..5e3bc717b --- /dev/null +++ b/app/services/access_token_authentication.rb @@ -0,0 +1,57 @@ +# typed: strict +# frozen_string_literal: true + +# Resolves the user behind a Doorkeeper access token, for every endpoint that accepts one. +module AccessTokenAuthentication + extend T::Sig + + # 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 missing, revoked or expired, has no owner, or is a + # handoff token of a user who became privileged after it was issued. + sig { params(token: T.nilable(Doorkeeper::AccessToken)).returns(T.nilable(User)) } + 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 + + # Revokes the user's handoff tokens, for when the user becomes privileged: endpoints such as + # /oauth/token/info only check revocation, not user_for. + sig { params(user: User).void } + def self.revoke_handoff_tokens(user) + Doorkeeper::AccessToken + .where(resource_owner_id: user.id, revoked_at: nil) + .where("? = ANY(string_to_array(scopes, ' '))", HANDOFF_SCOPE) + .update_all(revoked_at: Time.current) + end + + # Whether the token lets its client call UserAccessTokensController; the client still has to be + # an allow-listed issuer. + sig { params(token: Doorkeeper::AccessToken).returns(T::Boolean) } + def self.issuer_token?(token) + token.resource_owner_id.nil? && token.acceptable?([ISSUER_SCOPE]) + end + + # Whether UserAccessTokensController issued the token. + sig { params(token: Doorkeeper::AccessToken).returns(T::Boolean) } + def self.handoff?(token) + token.includes_scope?(HANDOFF_SCOPE) + end + + # Uids of the Doorkeeper applications allowed to issue tokens. + sig { returns(T::Array[String]) } + def self.issuer_application_uids + T.let(Rails.configuration.x.user_access_token_issuer_application_uids, T::Array[String]) + end + + # Uid of the Doorkeeper application that issued tokens belong to. + sig { returns(T.nilable(String)) } + def self.target_application_uid + T.let(Rails.configuration.x.user_access_token_target_application_uid, T.nilable(String)) + 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/bin/spec_requests b/bin/spec_requests new file mode 100755 index 000000000..fa40e087a --- /dev/null +++ b/bin/spec_requests @@ -0,0 +1,4 @@ +#!/bin/sh + +RSPEC_PATTERN="spec/requests/**/*.rb,spec/services/**/*.rb,spec/background_tasks/**/*.rb" +bundle exec rake spec SPEC_OPTS="--pattern $RSPEC_PATTERN --format documentation" diff --git a/config/application.rb b/config/application.rb index ac0809804..90b6d1a9e 100644 --- a/config/application.rb +++ b/config/application.rb @@ -33,7 +33,7 @@ class Application < Rails::Application # 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 - token access_token refresh_token client_secret secret _key crypt otp salt] + token access_token refresh_token client_secret secret _key crypt otp salt code_verifier] # Exact, so that other keys containing "code" stay readable. config.filter_parameters << /\Acode\z/ @@ -47,6 +47,8 @@ class Application < Rails::Application config.sass.silence_deprecations = %w[import global-builtin color-functions if-function abs-percent] 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: false, logger: (-> { Rails.logger }) do T.bind(self, Rack::Cors) diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index c93bb667b..75ef3ea70 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -38,6 +38,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 @@ -67,6 +72,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 fdfc38b5d..465be8dad 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -6,7 +6,7 @@ LOGGER = T.let(Logger.new('log/rack-attack.log'), Logger) 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.path, ('"' + 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 @@ -19,23 +19,71 @@ class Rack::Attack extend T::Sig - # The token is a raw request param, so it can be any shape. - sig { params(access_token: T.untyped).returns(T.untyped) } + # The path as Rails routes it, so that /oauth/token/, //oauth/token and /oauth/token.json + # cannot dodge the matches below. + sig { params(req: Rack::Attack::Request).returns(String) } + def self.normalized_path(req) + path = ::Rack::Utils.unescape_path(req.path).b.squeeze('/') + path = path.delete_suffix('/') unless path == '/' + path.sub(%r{\.[^/.]+\z}, '') + end + + sig { returns(String) } + def self.issue_token_path + Rails.application.routes.url_helpers.api_v8_application_user_access_tokens_path + end + + sig { params(req: Rack::Attack::Request).returns(T::Boolean) } + def self.token_issue?(req) + return false unless req.post? + + normalized_path(req) == issue_token_path + end + + # The access token as Doorkeeper reads it, from the Authorization header or the parameters. + sig { params(req: Rack::Attack::Request).returns(T.nilable(String)) } + def self.access_token_string(req) + request = ActionDispatch::Request.new(req.env) + Doorkeeper.config.access_token_methods.each do |method| + token = Doorkeeper::OAuth::Token.from_request(request, method) + return token if token.present? + end + nil + rescue ActionDispatch::Http::Parameters::ParseError + nil + end + + sig { params(access_token: String).returns(T.any(String, Integer)) } def self.user_identifier_discriminator(access_token) - db_token = Doorkeeper::AccessToken.find_by(token: access_token) - return access_token unless db_token - db_token.resource_owner_id + Doorkeeper::AccessToken.by_token(access_token)&.resource_owner_id || access_token + 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. + # Requests whose user_id is not a plain integer, oversized JSON bodies included, share one bucket + # per application: Rails may still read a user from them. + sig { params(req: Rack::Attack::Request).returns(T.nilable(String)) } + def self.token_issue_bucket(req) + return unless token_issue?(req) + + token_string = access_token_string(req) + token = token_string && Doorkeeper::AccessToken.by_token(token_string) + return unless token && AccessTokenAuthentication.issuer_token?(token) + + user_id = rails_param(req, 'user_id') + target = user_id&.match?(/\A\d+\z/) ? user_id.to_i : 'unrecognized user_id' + "#{token.application_id}:#{target}" end # 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. + # The token 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 + safe_api_key = ENV.fetch('RACK_ATTACK_SAFE_API_KEY', nil) + safe_api_key.present? && + ActiveSupport::SecurityUtils.secure_compare(request.get_header('HTTP_RATELIMIT_PROTECTION_SAFE_API_KEY').to_s, safe_api_key) && + !token_issue?(request) end # Return 503 Service Unavailable for throttles @@ -48,22 +96,22 @@ def self.user_identifier_discriminator(access_token) # 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.ip if normalized_path(req) == '/sessions' || + (normalized_path(req) == '/oauth/token' && req.post?) 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.ip if normalized_path(req) == '/sessions' || + (normalized_path(req) == '/oauth/token' && req.post?) 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.ip if normalized_path(req) == '/sessions' || + (normalized_path(req) == '/oauth/token' && req.post?) end @@ -75,80 +123,108 @@ def self.user_identifier_discriminator(access_token) # 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.match?(API_ACCOUNT_CREATION_PATH) && + req.ip if normalized_path(req) == '/user' || + (normalized_path(req).match?(API_ACCOUNT_CREATION_PATH) && req.post?) 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.match?(API_ACCOUNT_CREATION_PATH) && + req.ip if normalized_path(req) == '/user' || + (normalized_path(req).match?(API_ACCOUNT_CREATION_PATH) && req.post?) 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.ip if normalized_path(req) =~ %r{^/exercises/\d+/submissions$} || + normalized_path(req) =~ %r{^/org/\d+/exercises/\d+/submissions$} || + (normalized_path(req) =~ %r{^/api/v8/core/exercises/\d+/submissions$} && req.post?) 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.ip if normalized_path(req) =~ %r{^/exercises/\d+/submissions$} || + (normalized_path(req) =~ %r{^/org/\d+/exercises/\d+/submissions$} && req.post?) 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 + if normalized_path(req) =~ %r{^/api/v8/core/exercises/\d+/submissions$} && req.post? + token = access_token_string(req) + user_identifier_discriminator(token) if token end 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 token_issue?(req) + end + + throttle('user access tokens per user per 10 minutes', limit: 10, period: 10.minutes) do |req| + token_issue_bucket(req) + end + + throttle('user access tokens per user per day', limit: 50, period: 1.day) do |req| + token_issue_bucket(req) + end + PASSWORD_RESET_PATH = T.let(%r{\A/(password_reset_keys|api/v8/users/password_reset)(\.\w+)?\z}, Regexp) # 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.match?(PASSWORD_RESET_PATH) && + req.ip if normalized_path(req).match?(PASSWORD_RESET_PATH) && req.post? end - # Reset requests carry only an email, so anything larger is not a real client. - PASSWORD_RESET_JSON_MAX_BYTES = 8192 + # Throttled requests carry a handful of short fields, so anything larger is not a real client. + JSON_PARAMS_MAX_BYTES = 8192 - # The email a password reset is requested for, picked the way Rails params do: the query string - # wins over the body, and every content type Rails parses as JSON is read as JSON. Oversized - # JSON bodies share one key so that padding cannot dodge the per-email limit. - sig { params(req: Rack::Attack::Request).returns(T.nilable(String)) } - def self.password_reset_email(req) - email = req.GET['email'] - email ||= Mime::Type.lookup(req.media_type.to_s).symbol == :json ? json_body_email(req) : req.POST['email'] - email.to_s.strip.downcase.presence + # A request parameter, picked the way Rails params do: the query string wins over the body, and + # every content type Rails parses as JSON is read as JSON. Oversized JSON bodies give one shared + # value so that padding cannot dodge a per-value limit. + sig { params(req: Rack::Attack::Request, name: String).returns(T.nilable(String)) } + def self.rails_param(req, name) + value = req.GET[name] + value ||= json_body?(req) ? json_body_param(req, name) : req.POST[name] + value&.to_s + end + + sig { params(req: Rack::Attack::Request).returns(T::Boolean) } + private_class_method def self.json_body?(req) + media_type = req.media_type + return false if media_type.blank? + + Mime::Type.lookup(media_type).symbol == :json + rescue Mime::Type::InvalidMimeType + false end - sig { params(req: Rack::Attack::Request).returns(T.untyped) } - private_class_method def self.json_body_email(req) + sig { params(req: Rack::Attack::Request, name: String).returns(T.nilable(String)) } + private_class_method def self.json_body_param(req, name) body = req.body - json = body.read(PASSWORD_RESET_JSON_MAX_BYTES + 1).to_s + json = body.read(JSON_PARAMS_MAX_BYTES + 1).to_s body.rewind - return 'oversized JSON body' if json.bytesize > PASSWORD_RESET_JSON_MAX_BYTES + return 'oversized JSON body' if json.bytesize > JSON_PARAMS_MAX_BYTES parsed = begin JSON.parse(json) rescue JSON::ParserError nil end - parsed['email'] if parsed.is_a?(Hash) + parsed[name]&.to_s if parsed.is_a?(Hash) + end + + # The email a password reset is requested for, normalized as the controllers look it up. + sig { params(req: Rack::Attack::Request).returns(T.nilable(String)) } + def self.password_reset_email(req) + rails_param(req, 'email').to_s.strip.downcase.presence 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| - password_reset_email(req) if req.path.match?(PASSWORD_RESET_PATH) && req.post? + password_reset_email(req) if normalized_path(req).match?(PASSWORD_RESET_PATH) && req.post? end end diff --git a/config/initializers/secret_path_filter.rb b/config/initializers/secret_path_filter.rb new file mode 100644 index 000000000..9503948bb --- /dev/null +++ b/config/initializers/secret_path_filter.rb @@ -0,0 +1,34 @@ +# typed: strict +# frozen_string_literal: true + +# Emailed links carry their token in the URL path, which Rails and Rack::Attack would log verbatim. +module SecretPathFilter + extend T::Sig + + TOKEN_SEGMENT = T.let(%r{(/reset_password/+|/users/+[^/]+/+(?:verify|destroy)/+)[^/?#]+}, Regexp) + + # +path+ with the token segments of emailed links replaced by [FILTERED]. + sig { params(path: String).returns(String) } + def self.filter(path) + path.gsub(TOKEN_SEGMENT, '\1[FILTERED]') + end + + # Prepended to ActionDispatch::Request, whose filtered_path the request log prints. + module RequestFilteredPath + extend T::Sig + extend T::Helpers + requires_ancestor { ActionDispatch::Request } + + sig { returns(String) } + def filtered_path + SecretPathFilter.filter(super) + end + end +end + +ActiveSupport.on_load(:action_dispatch_request) do + T.bind(self, T.class_of(ActionDispatch::Request)) + prepend SecretPathFilter::RequestFilteredPath +end + +Rails.application.config.filter_redirect << SecretPathFilter::TOKEN_SEGMENT diff --git a/config/routes.rb b/config/routes.rb index a3e99c8f9..12e706d69 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -154,6 +154,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 @@ -281,14 +285,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/sorbet/rbi/dsl/generated_path_helpers_module.rbi b/sorbet/rbi/dsl/generated_path_helpers_module.rbi index 8a5331db0..21d7316b0 100644 --- a/sorbet/rbi/dsl/generated_path_helpers_module.rbi +++ b/sorbet/rbi/dsl/generated_path_helpers_module.rbi @@ -27,6 +27,9 @@ module GeneratedPathHelpersModule sig { params(args: T.untyped).returns(String) } def api_v8_application_credentials_path(*args); end + sig { params(args: T.untyped).returns(String) } + def api_v8_application_user_access_tokens_path(*args); end + sig { params(args: T.untyped).returns(String) } def api_v8_core_course_path(*args); end diff --git a/sorbet/rbi/dsl/generated_url_helpers_module.rbi b/sorbet/rbi/dsl/generated_url_helpers_module.rbi index e7ab820b0..f0166195e 100644 --- a/sorbet/rbi/dsl/generated_url_helpers_module.rbi +++ b/sorbet/rbi/dsl/generated_url_helpers_module.rbi @@ -27,6 +27,9 @@ module GeneratedUrlHelpersModule sig { params(args: T.untyped).returns(String) } def api_v8_application_credentials_url(*args); end + sig { params(args: T.untyped).returns(String) } + def api_v8_application_user_access_tokens_url(*args); end + sig { params(args: T.untyped).returns(String) } def api_v8_core_course_review_url(*args); end diff --git a/spec/background_tasks/access_token_cleanup_task_spec.rb b/spec/background_tasks/access_token_cleanup_task_spec.rb new file mode 100644 index 000000000..f5866f809 --- /dev/null +++ b/spec/background_tasks/access_token_cleanup_task_spec.rb @@ -0,0 +1,26 @@ +# typed: false +# frozen_string_literal: true + +require 'spec_helper' + +describe AccessTokenCleanupTask do + let(:user) { FactoryBot.create(:user) } + + def create_token(**attributes) + Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public', **attributes) + end + + it 'deletes tokens that expired or were revoked over the retention period ago' do + long_expired = create_token(expires_in: 86_400, created_at: 32.days.ago) + long_revoked = create_token(revoked_at: 31.days.ago) + recently_expired = create_token(expires_in: 86_400, created_at: 29.days.ago) + recently_revoked = create_token(revoked_at: 29.days.ago) + valid = create_token(expires_in: 86_400) + non_expiring = create_token(created_at: 1.year.ago) + + described_class.new.run + + remaining = Doorkeeper::AccessToken.where(id: [long_expired, long_revoked, recently_expired, recently_revoked, valid, non_expiring].map(&:id)) + expect(remaining).to contain_exactly(recently_expired, recently_revoked, valid, non_expiring) + end +end 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..2ad9f7a82 --- /dev/null +++ b/spec/controllers/api/v8/application/user_access_tokens_controller_spec.rb @@ -0,0 +1,150 @@ +# typed: false +# 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 + allow(Rails.logger).to receive(:warn).and_call_original + 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 = response.parsed_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) + expect(Rails.logger).to have_received(:warn).with("Refused to issue user access token (not an allowed issuer): issuer application uid=#{other_app.uid} user_id=") + 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: response.parsed_body['access_token']) + expect(issued.resource_owner_id).to eq(other_user.id) + end + + it 'returns 404 for an unknown user' do + unknown_id = User.maximum(:id) + 1 + post_with(issuer_token, unknown_id) + + expect(response).to have_http_status(404) + expect(Rails.logger).to have_received(:warn).with("Refused to issue user access token (unknown user): issuer application uid=#{issuer_app.uid} user_id=#{unknown_id}") + 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) + expect(Rails.logger).to have_received(:warn).with("Refused to issue user access token (privileged user): issuer application uid=#{issuer_app.uid} user_id=#{admin.id}") + 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 5bbacb45c..701677a7f 100644 --- a/spec/controllers/api/v8/base_controller_spec.rb +++ b/spec/controllers/api/v8/base_controller_spec.rb @@ -29,7 +29,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) { Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public') } it { expect(subject.id).to be user.id } end end @@ -39,9 +39,48 @@ 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) { Doorkeeper::AccessToken.create!(resource_owner_id: -1, scopes: 'public') } 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 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 e31b46ada..8d10aa3cf 100644 --- a/spec/controllers/api/v8/core/courses/reviews_controller_spec.rb +++ b/spec/controllers/api/v8/core/courses/reviews_controller_spec.rb @@ -19,7 +19,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) { Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public') } describe 'when course id given' do it "shows only user's submission's review information" do @@ -47,7 +47,7 @@ describe 'as user' do let(:user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public') } describe 'when course id given' do it "shows only user's submission's review information" do @@ -101,7 +101,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) { Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public') } before :each do controller.current_user = user end @@ -145,7 +145,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) { Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public') } it 'should deny access' do other_review = 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 4955b57b3..f752d7a0a 100644 --- a/spec/controllers/api/v8/core/courses/unlocks_controller_spec.rb +++ b/spec/controllers/api/v8/core/courses/unlocks_controller_spec.rb @@ -15,7 +15,7 @@ describe 'POST unlock exercises' do describe 'as an authenticated user' do - let!(:token) { double resource_owner_id: user.id, acceptable?: true } + let!(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public') } 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 d35dc6492..43e479571 100644 --- a/spec/controllers/api/v8/core/courses_controller_spec.rb +++ b/spec/controllers/api/v8/core/courses_controller_spec.rb @@ -12,7 +12,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) { Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public') } 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/details_controller_spec.rb b/spec/controllers/api/v8/core/exercises/details_controller_spec.rb index fb08d2a1c..33a1cf6d7 100644 --- a/spec/controllers/api/v8/core/exercises/details_controller_spec.rb +++ b/spec/controllers/api/v8/core/exercises/details_controller_spec.rb @@ -34,14 +34,14 @@ def query_count(&) hidden_exercise = FactoryBot.create(:exercise, course: visible_exercise.course, hidden: true) teacher = FactoryBot.create(:user) Teachership.create!(user: teacher, organization: visible_exercise.course.organization) - allow(controller).to receive(:doorkeeper_token).and_return(double(resource_owner_id: teacher.id, acceptable?: true)) + allow(controller).to receive(:doorkeeper_token).and_return(Doorkeeper::AccessToken.create!(resource_owner_id: teacher.id, scopes: 'public')) get :show, params: { ids: "#{visible_exercise.id},#{hidden_exercise.id}" } expect(response.parsed_body['exercises'].pluck('id')).to contain_exactly(visible_exercise.id, hidden_exercise.id) end it 'runs the same number of queries for 2 and 6 exercises' 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(Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public')) course = visible_exercise.course exercises = [visible_exercise] + FactoryBot.create_list(:exercise, 5, course: course) exercises.last.update!(hidden: true) 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 23298d744..0399f17bd 100644 --- a/spec/controllers/api/v8/core/exercises/solutions_controller_spec.rb +++ b/spec/controllers/api/v8/core/exercises/solutions_controller_spec.rb @@ -24,7 +24,7 @@ end describe 'Downloading an exercise solution zip' do describe 'as an admin' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: admin.id, scopes: 'public') } it 'should succeed if the exercise exists' do repo = clone_course_repo(course) @@ -38,7 +38,7 @@ end describe 'as a student' do - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public') } 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 df7519228..3d10fff79 100644 --- a/spec/controllers/api/v8/core/exercises/submissions_controller_spec.rb +++ b/spec/controllers/api/v8/core/exercises/submissions_controller_spec.rb @@ -30,7 +30,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) { Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public') } 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 ba0c89b53..83c92a7ec 100644 --- a/spec/controllers/api/v8/core/exercises_controller_spec.rb +++ b/spec/controllers/api/v8/core/exercises_controller_spec.rb @@ -53,7 +53,7 @@ end describe 'When logged in' do - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public') } 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 f337018ed..686408460 100644 --- a/spec/controllers/api/v8/core/organizations/courses_controller_spec.rb +++ b/spec/controllers/api/v8/core/organizations/courses_controller_spec.rb @@ -17,7 +17,7 @@ describe 'GET courses' do describe 'as admin' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: admin.id, scopes: 'public') } it 'shows a list of collections of course urls' do get :index, params: { organization_slug: organization.slug } json = response.parsed_body @@ -32,7 +32,7 @@ end end describe 'as user' do - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public') } it 'shows a list of collections of course urls' do get :index, params: { organization_slug: organization.slug } json = response.parsed_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 467c5da63..1e4652b08 100644 --- a/spec/controllers/api/v8/core/submissions/feedback_controller_spec.rb +++ b/spec/controllers/api/v8/core/submissions/feedback_controller_spec.rb @@ -17,7 +17,7 @@ describe 'POST create' do describe 'As a student' do - let(:token) { double resource_owner_id: student.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: student.id, scopes: 'public') } 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 20e5e53ab..7e0b524af 100644 --- a/spec/controllers/api/v8/core/submissions/reviews_controller_spec.rb +++ b/spec/controllers/api/v8/core/submissions/reviews_controller_spec.rb @@ -19,7 +19,7 @@ describe 'POST create' do describe 'As an admin' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: admin.id, scopes: 'public') } it 'can make new review' do expect do @@ -30,7 +30,7 @@ end describe 'As a teacher' do - let(:token) { double resource_owner_id: teacher.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: teacher.id, scopes: 'public') } it 'can make new review' do expect do @@ -41,7 +41,7 @@ end describe 'As a student' do - let(:token) { double resource_owner_id: student.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: student.id, scopes: 'public') } 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 6ec48baf7..1c1423be2 100644 --- a/spec/controllers/api/v8/core/submissions_controller_spec.rb +++ b/spec/controllers/api/v8/core/submissions_controller_spec.rb @@ -35,7 +35,7 @@ end describe 'Downloading a submission as zip' do describe 'as an admin' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: admin.id, scopes: 'public') } it "should allow to download everyone's submissions" do # pending("test that submission zip's content is correct") @@ -48,7 +48,7 @@ before :each do Teachership.create(user: teacher, organization: organization) end - let(:token) { double resource_owner_id: teacher.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: teacher.id, scopes: 'public') } it "should allow to download own organization's submissions" do # pending("test that submission zip's content is correct") @@ -71,7 +71,7 @@ before :each do Assistantship.create(user: assistant, course: course) end - let(:token) { double resource_owner_id: assistant.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: assistant.id, scopes: 'public') } it "should allow to download own course's submissions" do # pending("test that submission zip's content is correct") @@ -90,7 +90,7 @@ end end describe 'as a student' do - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public') } it 'should allow to download own submissions' do # pending("test that submission zip's content is correct") @@ -120,7 +120,7 @@ end describe 'a submission whose exercise has been removed from the course' do - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public') } before :each do exercise.update_column(:name, 'removed-by-refresh') @@ -142,7 +142,7 @@ end describe 'a submission whose results are hidden' do - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public') } before :each do course.update!(hide_submission_results: true) diff --git a/spec/controllers/api/v8/courses/all_courses_with_this_template_controller_spec.rb b/spec/controllers/api/v8/courses/all_courses_with_this_template_controller_spec.rb index 5ec022f5b..0d5039e81 100644 --- a/spec/controllers/api/v8/courses/all_courses_with_this_template_controller_spec.rb +++ b/spec/controllers/api/v8/courses/all_courses_with_this_template_controller_spec.rb @@ -10,7 +10,7 @@ let(:user) { FactoryBot.create(:user) } before :each do - allow(controller).to receive(:doorkeeper_token).and_return(double(resource_owner_id: user.id, acceptable?: true)) + allow(controller).to receive(:doorkeeper_token).and_return(Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public')) end it 'lists only the courses the caller can read' do 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 72f5d07ca..0e094a4e1 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 @@ -35,7 +35,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: admin.id, acceptable?: true } + let!(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: admin.id, scopes: 'public') } it 'should show all courses with same template' do two_courses_by_template_id @@ -63,7 +63,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: teacher.id, acceptable?: true } + let!(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: teacher.id, scopes: 'public') } it 'should show all courses with same template' do two_courses_by_template_id @@ -91,7 +91,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: assistant.id, acceptable?: true } + let!(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: assistant.id, scopes: 'public') } it 'should show all courses with same template' do two_courses_by_template_id @@ -119,7 +119,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: user.id, acceptable?: true } + let!(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public') } 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 f4374abe0..e8b40a643 100644 --- a/spec/controllers/api/v8/courses/exercises/points_controller_spec.rb +++ b/spec/controllers/api/v8/courses/exercises/points_controller_spec.rb @@ -21,7 +21,7 @@ end describe 'As an admin' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: admin.id, scopes: 'public') } 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 @@ -36,7 +36,7 @@ end describe 'As a student' do - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public') } it "returns only the student's own points" do get :index, params: { course_id: course.id, exercise_name: exercise.name } @@ -46,7 +46,7 @@ describe 'As a teacher' do let(:teacher) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: teacher.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: teacher.id, scopes: 'public') } before { Teachership.create!(user: teacher, organization: organization) } @@ -68,7 +68,7 @@ end describe 'As any user' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: admin.id, scopes: 'public') } 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 3ca4d0c8a..f7b542b5e 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 @@ -31,7 +31,7 @@ end describe 'As any user' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: admin.id, scopes: 'public') } 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 728a8a218..78aec2689 100644 --- a/spec/controllers/api/v8/courses/exercises_controller_spec.rb +++ b/spec/controllers/api/v8/courses/exercises_controller_spec.rb @@ -16,7 +16,7 @@ end describe 'As an admin' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: admin.id, scopes: 'public') } describe 'when course id is given' do it 'should return successful response' do get :index, params: { course_id: course.id } @@ -34,7 +34,7 @@ end describe 'As a student' do - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public') } describe 'when course id is given' do it 'should return successful response' do get :index, params: { course_id: course.id } @@ -52,7 +52,7 @@ end describe 'As a student without unlock rows' do - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public') } def unlocked_flags get :index, params: { course_id: course.id } @@ -82,7 +82,7 @@ def unlocked_flags end describe 'As any user' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: admin.id, scopes: 'public') } 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 da2ec55ea..f6c2322b3 100644 --- a/spec/controllers/api/v8/courses/points_controller_spec.rb +++ b/spec/controllers/api/v8/courses/points_controller_spec.rb @@ -21,7 +21,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) { Doorkeeper::AccessToken.create!(resource_owner_id: current_user.id, scopes: 'public') } 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) { Doorkeeper::AccessToken.create!(resource_owner_id: current_user.id, scopes: 'public') } describe 'when course ID given' do it "shows only the caller's own points" do @@ -101,7 +101,7 @@ describe 'as user' do let(:current_user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: current_user.id, scopes: 'public') } it 'lists no points' do get :index, params: { course_id: course.id } @@ -123,7 +123,7 @@ describe 'as admin' do let(:current_user) { FactoryBot.create(:admin) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: current_user.id, scopes: 'public') } it 'lists the points' do get :index, params: { course_id: course.id } @@ -139,7 +139,7 @@ describe 'as user' do let(:current_user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: current_user.id, scopes: 'public') } it 'is forbidden' do get :index, params: { course_id: course.id } @@ -167,7 +167,7 @@ FactoryBot.create(:awarded_point, course: course, user: current_user, name: exercise.available_points.first.name) end let(:current_user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: current_user.id, scopes: 'public') } it "leaves out the exercise's points" do get :index, params: { course_id: course.id } @@ -186,7 +186,7 @@ describe 'as user' do let(:current_user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: current_user.id, scopes: 'public') } it "leaves out the hidden exercise's points" do get :index, params: { course_id: course.id } @@ -198,7 +198,7 @@ describe 'as admin' do let(:current_user) { FactoryBot.create(:admin) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: current_user.id, scopes: 'public') } it "includes the hidden exercise's points" do get :index, params: { course_id: course.id } 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 a2a731584..a93e1d273 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 @@ -19,7 +19,7 @@ describe 'index' do context 'as a regular user' do - let!(:token) { double resource_owner_id: regular_user.id, acceptable?: true } + let!(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: regular_user.id, scopes: 'public') } it 'should not allow access' do get :index, params: { course_id: course.id } @@ -28,7 +28,7 @@ end context 'as an admin' do - let!(:token) { double resource_owner_id: admin_user.id, acceptable?: true } + let!(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: admin_user.id, scopes: 'public') } 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 4aea70dd2..168d757dc 100644 --- a/spec/controllers/api/v8/courses/submissions_controller_spec.rb +++ b/spec/controllers/api/v8/courses/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) { Doorkeeper::AccessToken.create!(resource_owner_id: admin.id, scopes: 'public') } it 'should show all of the submissions' do two_subs_by_id @@ -62,7 +62,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: teacher.id, acceptable?: true } + let!(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: teacher.id, scopes: 'public') } it 'should show all of the submissions in my organizations' do two_subs_by_id @@ -90,7 +90,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: assistant.id, acceptable?: true } + let!(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: assistant.id, scopes: 'public') } it 'should show all of the submissions in my courses' do two_subs_by_id @@ -118,7 +118,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: user.id, acceptable?: true } + let!(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public') } 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 fb399c3d6..aa977baad 100644 --- a/spec/controllers/api/v8/courses/users/points_controller_spec.rb +++ b/spec/controllers/api/v8/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) { Doorkeeper::AccessToken.create!(resource_owner_id: current_user.id, scopes: 'public') } describe 'when course ID 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) { Doorkeeper::AccessToken.create!(resource_owner_id: current_user.id, scopes: 'public') } describe 'when course ID 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) { Doorkeeper::AccessToken.create!(resource_owner_id: current_user.id, scopes: 'public') } describe 'when course ID 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) { Doorkeeper::AccessToken.create!(resource_owner_id: current_user.id, scopes: 'public') } describe 'when course ID given' do it "lists none of another user's points" 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) { Doorkeeper::AccessToken.create!(resource_owner_id: current_user.id, scopes: 'public') } describe 'when course ID given' do it "lists none of another user's points" 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 a53cb5982..2a480fc19 100644 --- a/spec/controllers/api/v8/courses/users/submissions_controller_spec.rb +++ b/spec/controllers/api/v8/courses/users/submissions_controller_spec.rb @@ -62,7 +62,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: admin.id, acceptable?: true } + let!(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: admin.id, scopes: 'public') } it "should show given user's submissions" do users_own_subs(course_id: course.id, user_id: user.id) @@ -86,7 +86,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: teacher.id, acceptable?: true } + let!(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: teacher.id, scopes: 'public') } it "should show given user's submissions in my organizations" do users_own_subs(course_id: course.id, user_id: user.id) @@ -114,7 +114,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: assistant.id, acceptable?: true } + let!(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: assistant.id, scopes: 'public') } it "should show given user's submissions in my courses" do users_own_subs(course_id: course.id, user_id: user.id) @@ -142,7 +142,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: user.id, acceptable?: true } + let!(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public') } it 'should show my own submissions' do users_own_subs(course_id: course.id, user_id: user.id) @@ -187,7 +187,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: user.id, acceptable?: true } + let!(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public') } 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 744e8b099..ce33c8d04 100644 --- a/spec/controllers/api/v8/courses_controller_spec.rb +++ b/spec/controllers/api/v8/courses_controller_spec.rb @@ -16,7 +16,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) { Doorkeeper::AccessToken.create!(resource_owner_id: current_user.id, scopes: 'public') } describe 'when course ID given' do it 'shows course information' do @@ -46,7 +46,7 @@ describe 'as user' do let(:current_user) { user } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: current_user.id, scopes: 'public') } 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 7c1999c86..dffe15c6d 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 @@ -53,7 +53,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(Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public')) expect(CoursesMoocFiTokenIntrospector).not_to receive(:introspect) get :index expect(current_user).to eq(user) diff --git a/spec/controllers/api/v8/exercises/model_solutions_controller_spec.rb b/spec/controllers/api/v8/exercises/model_solutions_controller_spec.rb index dd79b20a0..908128fe9 100644 --- a/spec/controllers/api/v8/exercises/model_solutions_controller_spec.rb +++ b/spec/controllers/api/v8/exercises/model_solutions_controller_spec.rb @@ -14,7 +14,7 @@ end def sign_in(as) - allow(controller).to receive(:doorkeeper_token).and_return(double(resource_owner_id: as.id, acceptable?: true)) + allow(controller).to receive(:doorkeeper_token).and_return(Doorkeeper::AccessToken.create!(resource_owner_id: as.id, scopes: 'public')) end def complete_exercises(count) diff --git a/spec/controllers/api/v8/exercises/users/submissions_controller_spec.rb b/spec/controllers/api/v8/exercises/users/submissions_controller_spec.rb index e0c40aac7..a05da945d 100644 --- a/spec/controllers/api/v8/exercises/users/submissions_controller_spec.rb +++ b/spec/controllers/api/v8/exercises/users/submissions_controller_spec.rb @@ -8,7 +8,7 @@ let(:exercise) { FactoryBot.create(:exercise) } def sign_in(as) - allow(controller).to receive(:doorkeeper_token).and_return(as && double(resource_owner_id: as.id, acceptable?: true)) + allow(controller).to receive(:doorkeeper_token).and_return(as && Doorkeeper::AccessToken.create!(resource_owner_id: as.id, scopes: 'public')) end def submission(owner, **attributes) 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 f280a98c6..ccc2504b4 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 @@ -23,7 +23,7 @@ end describe 'As an admin' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: admin.id, scopes: 'public') } 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 @@ -49,7 +49,7 @@ end describe 'As any user' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: admin.id, scopes: 'public') } 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 77cee0372..972135ffe 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 @@ -32,7 +32,7 @@ end describe 'As any user' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: admin.id, scopes: 'public') } 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 3afbd8d19..b30862c77 100644 --- a/spec/controllers/api/v8/organizations/courses/exercises_controller_spec.rb +++ b/spec/controllers/api/v8/organizations/courses/exercises_controller_spec.rb @@ -28,7 +28,7 @@ FileUtils.rm_rf(repo_path) end describe 'As an admin' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: admin.id, scopes: 'public') } describe 'when course name is given' do it 'should return successful response' do get :index, params: { course_name: course_name, organization_slug: organization.slug } @@ -46,7 +46,7 @@ end describe 'As a student' do - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public') } describe 'when course name is given' do it 'should return successful response' do get :index, params: { course_name: course_name, organization_slug: organization.slug } @@ -64,7 +64,7 @@ end describe 'As any user' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: admin.id, scopes: 'public') } describe 'when course name could not be found' do it 'should return error' do get :index, params: { course_name: 'null', organization_slug: organization.slug } @@ -75,7 +75,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) { Doorkeeper::AccessToken.create!(resource_owner_id: Guest.new.id, scopes: 'public') } 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 a0c8acf20..c05a32680 100644 --- a/spec/controllers/api/v8/organizations/courses/points_controller_spec.rb +++ b/spec/controllers/api/v8/organizations/courses/points_controller_spec.rb @@ -22,7 +22,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) { Doorkeeper::AccessToken.create!(resource_owner_id: current_user.id, scopes: 'public') } 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) { Doorkeeper::AccessToken.create!(resource_owner_id: current_user.id, scopes: 'public') } describe 'when course name given' do it "shows only the caller's own points" do diff --git a/spec/controllers/api/v8/organizations/courses/studyright_eligibility_controller_spec.rb b/spec/controllers/api/v8/organizations/courses/studyright_eligibility_controller_spec.rb index b820a8ae6..787cfc70c 100644 --- a/spec/controllers/api/v8/organizations/courses/studyright_eligibility_controller_spec.rb +++ b/spec/controllers/api/v8/organizations/courses/studyright_eligibility_controller_spec.rb @@ -7,7 +7,7 @@ let(:organization) { FactoryBot.create(:organization, slug: 'mooc') } let(:course) { FactoryBot.create(:course, organization: organization, name: 'mooc-2019-ohjelmointi') } let(:admin) { FactoryBot.create(:admin) } - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: admin.id, scopes: 'public') } before :each do allow(controller).to receive(:doorkeeper_token) { token } 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 16db2c966..0ad13fcec 100644 --- a/spec/controllers/api/v8/organizations/courses/submissions_controller_spec.rb +++ b/spec/controllers/api/v8/organizations/courses/submissions_controller_spec.rb @@ -40,7 +40,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: admin.id, acceptable?: true } + let!(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: admin.id, scopes: 'public') } it 'should show all of the submissions' do two_subs_by_name @@ -64,7 +64,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: teacher.id, acceptable?: true } + let!(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: teacher.id, scopes: 'public') } it 'should show all of the submissions in my organizations' do two_subs_by_name @@ -92,7 +92,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: assistant.id, acceptable?: true } + let!(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: assistant.id, scopes: 'public') } it 'should show all of the submissions in my courses' do two_subs_by_name @@ -120,7 +120,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: user.id, acceptable?: true } + let!(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public') } 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 75ebc36f8..815c96e90 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 @@ -23,7 +23,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) { Doorkeeper::AccessToken.create!(resource_owner_id: current_user.id, scopes: 'public') } describe 'when course name given' do it "shows only user's point information" do @@ -50,7 +50,7 @@ describe 'as user' do let(:current_user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: current_user.id, scopes: 'public') } describe 'when course name given' do it "shows only user's point information" do @@ -101,7 +101,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) { Doorkeeper::AccessToken.create!(resource_owner_id: current_user.id, scopes: 'public') } describe 'when course name given' do it "shows only user's point information" do @@ -128,7 +128,7 @@ describe 'as user' do let(:current_user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: current_user.id, scopes: 'public') } describe 'when course name given' do it "lists none of another user's points" do @@ -155,7 +155,7 @@ describe 'as another user' do let(:current_user) { user2 } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: current_user.id, scopes: 'public') } describe 'when course name given' do it "lists none of another user's points" do @@ -202,7 +202,7 @@ describe "GET current user's points in exam mode" do let(:current_user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: current_user.id, scopes: 'public') } it 'lists no points' do course.update!(hide_submission_results: true) 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 7328cc8ee..47578fc51 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 @@ -39,7 +39,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: admin.id, acceptable?: true } + let!(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: admin.id, scopes: 'public') } it "should show given user's submissions" do users_own_subs(organization_slug: organization.slug, course_name: course_name, user_id: user.id) @@ -63,7 +63,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: teacher.id, acceptable?: true } + let!(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: teacher.id, scopes: 'public') } 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) @@ -91,7 +91,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: assistant.id, acceptable?: true } + let!(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: assistant.id, scopes: 'public') } 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) @@ -119,7 +119,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: user.id, acceptable?: true } + let!(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public') } it 'should show my own submissions' do users_own_subs(organization_slug: organization.slug, course_name: course_name, user_id: user.id) @@ -164,7 +164,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: user.id, acceptable?: true } + let!(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public') } 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 6d7fbaf49..b361676d6 100644 --- a/spec/controllers/api/v8/organizations/courses_controller_spec.rb +++ b/spec/controllers/api/v8/organizations/courses_controller_spec.rb @@ -18,7 +18,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) { Doorkeeper::AccessToken.create!(resource_owner_id: current_user.id, scopes: 'public') } describe 'when organization id and course name given' do it 'shows course information' do @@ -61,7 +61,7 @@ describe 'when logged as user' do let(:current_user) { user } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: current_user.id, scopes: 'public') } describe 'when organization id and course name given' do it 'shows course information' do diff --git a/spec/controllers/api/v8/users/request_deletion_controller_spec.rb b/spec/controllers/api/v8/users/request_deletion_controller_spec.rb index 8123aba17..e98cd5754 100644 --- a/spec/controllers/api/v8/users/request_deletion_controller_spec.rb +++ b/spec/controllers/api/v8/users/request_deletion_controller_spec.rb @@ -25,4 +25,15 @@ expect(response).to have_http_status(403) end + + it 'refuses a deletion request made with a handoff token' do + application = Doorkeeper::Application.create!(name: 'mooc.fi frontend', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob') + token = Doorkeeper::AccessToken.create!(application: application, resource_owner_id: user.id, scopes: 'public handoff') + request.headers['Authorization'] = "Bearer #{token.token}" + expect(UserMailer).not_to receive(:destroy_confirmation) + + post :create, params: { user_id: 'current' } + + expect(response).to have_http_status(403) + end end diff --git a/spec/controllers/api/v8/users/teacherships_controller_spec.rb b/spec/controllers/api/v8/users/teacherships_controller_spec.rb index 9915168da..614360c49 100644 --- a/spec/controllers/api/v8/users/teacherships_controller_spec.rb +++ b/spec/controllers/api/v8/users/teacherships_controller_spec.rb @@ -9,7 +9,7 @@ before do Teachership.create!(user: teacher, organization: organization) - allow(controller).to receive(:doorkeeper_token).and_return(double(resource_owner_id: teacher.id, acceptable?: true)) + allow(controller).to receive(:doorkeeper_token).and_return(Doorkeeper::AccessToken.create!(resource_owner_id: teacher.id, scopes: 'public')) end it "lists the user's organizations, with missing information as an empty string" do diff --git a/spec/controllers/api/v8/users_controller_spec.rb b/spec/controllers/api/v8/users_controller_spec.rb index 955555283..9d79aa298 100644 --- a/spec/controllers/api/v8/users_controller_spec.rb +++ b/spec/controllers/api/v8/users_controller_spec.rb @@ -13,7 +13,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) { Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public') } it 'user sees own info' do get :show, params: { id: 'current' } @@ -31,7 +31,7 @@ end context 'as an administrator' do - let!(:token) { double resource_owner_id: admin.id, acceptable?: true } + let!(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: admin.id, scopes: 'public') } it 'user sees own info' do get :show, params: { id: 'current' } @@ -41,7 +41,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) { Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public') } it 'user sees own info' do get :show, params: { id: user.id } @@ -58,7 +58,7 @@ end end describe 'using admin token' do - let!(:token) { double resource_owner_id: admin.id, acceptable?: true } + let!(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: admin.id, scopes: 'public') } it "admin can see everyone's infos" do get :show, params: { id: user.id } @@ -349,4 +349,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 4ba9b8996..cb086b5fa 100644 --- a/spec/controllers/auths_controller_spec.rb +++ b/spec/controllers/auths_controller_spec.rb @@ -46,6 +46,18 @@ 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 + it 'fails without a username' do get :show, params: { password: 'bar', format: :text } expect(response).to be_successful diff --git a/spec/controllers/submission_download_authorization_spec.rb b/spec/controllers/submission_download_authorization_spec.rb index b6f49dcac..d4a91b2b0 100644 --- a/spec/controllers/submission_download_authorization_spec.rb +++ b/spec/controllers/submission_download_authorization_spec.rb @@ -54,7 +54,7 @@ def download_as(user) include_context 'a failing paste submission' def download_as(user) - token = user && double(resource_owner_id: user.id, acceptable?: true) + token = user && Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public') allow(controller).to receive(:doorkeeper_token) { token } get :download, params: { id: submission.id } end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 1afbd0290..8e0831575 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -253,7 +253,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/api_v8_token_authentication_spec.rb b/spec/requests/api_v8_token_authentication_spec.rb new file mode 100644 index 000000000..bc7ae282c --- /dev/null +++ b/spec/requests/api_v8_token_authentication_spec.rb @@ -0,0 +1,67 @@ +# typed: false +# frozen_string_literal: true + +require 'spec_helper' + +describe 'API v8 bearer token authentication', 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(:revoked_token) { Doorkeeper::AccessToken.create!(application: application, resource_owner_id: user.id, scopes: 'public').tap(&:revoke) } + let(:expired_token) { Doorkeeper::AccessToken.create!(application: application, resource_owner_id: user.id, scopes: 'public', expires_in: 60, created_at: 2.minutes.ago) } + + def get_current_user(bearer) + get '/api/v8/users/current.json', headers: { 'Authorization' => "Bearer #{bearer}" } + end + + shared_examples 'refusing TMC tokens that are no longer valid' do + it 'refuses a revoked token' do + get_current_user(revoked_token.token) + + expect(response).to have_http_status(401) + expect(response.parsed_body).to eq('error' => 'Invalid token') + end + + it 'refuses an expired token' do + get_current_user(expired_token.token) + + expect(response).to have_http_status(401) + expect(response.parsed_body).to eq('error' => 'Invalid token') + end + end + + include_examples 'refusing TMC tokens that are no longer valid' + + context 'when courses.mooc.fi tokens are accepted' 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 + + before :each do + allow(CoursesMoocFiTokenIntrospector).to receive(:introspect).and_return(nil) + end + + include_examples 'refusing TMC tokens that are no longer valid' + + it 'does not send a revoked TMC token to courses.mooc.fi' do + get_current_user(revoked_token.token) + + expect(CoursesMoocFiTokenIntrospector).not_to have_received(:introspect) + end + + it 'still authenticates a courses.mooc.fi token' 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) + + get_current_user(bearer) + + expect(response).to have_http_status(200) + expect(response.parsed_body['id']).to eq(user.id) + end + end +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..c53c5a943 --- /dev/null +++ b/spec/requests/handoff_token_consumers_spec.rb @@ -0,0 +1,31 @@ +# typed: false +# 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..c4018b8de --- /dev/null +++ b/spec/requests/issuer_token_hardening_spec.rb @@ -0,0 +1,118 @@ +# typed: false +# 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(response.parsed_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(response.parsed_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(response.parsed_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 } + + expect(filter.filter(params)).to eq(params) + end + end + + describe 'rate limiting' do + include_context 'with Rack::Attack' + + let(:safe_headers) { { 'HTTP_RATELIMIT_PROTECTION_SAFE_API_KEY' => 'safe-key' } } + + around :each do |example| + ENV['RACK_ATTACK_SAFE_API_KEY'] = 'safe-key' + example.run + ensure + ENV.delete('RACK_ATTACK_SAFE_API_KEY') + end + + def status_after(times, path, env = {}) + times.times { post path, env: { 'REMOTE_ADDR' => '203.0.113.7' }.merge(env) } + response.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)).not_to eq(503) + 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)).not_to eq(503) + end + + it 'throttles unsafelisted oauth token requests' do + expect(status_after(20, '/oauth/token')).not_to eq(503) + expect(status_after(1, '/oauth/token')).to eq(503) + end + + context 'with an allow-listed issuer' do + 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') } + let(:user) { FactoryBot.create(:user) } + + around :each do |example| + config = Rails.configuration.x + original = [config.user_access_token_issuer_application_uids, config.user_access_token_target_application_uid] + config.user_access_token_issuer_application_uids = [issuer_app.uid] + config.user_access_token_target_application_uid = target_app.uid + example.run + ensure + config.user_access_token_issuer_application_uids, config.user_access_token_target_application_uid = original + end + + it 'counts padded JSON bodies towards the per-user limit' do + body = { user_id: user.id, padding: 'x' * 10_000 }.to_json + headers = { 'Authorization' => "Bearer #{issuer_token.token}", 'Content-Type' => 'application/json' } + statuses = Array.new(11) do |i| + post '/api/v8/application/user_access_tokens', params: body, headers: headers, env: { 'REMOTE_ADDR' => "198.51.100.#{i}" } + response.status + end + + expect(statuses).to eq(([200] * 10) + [503]) + end + end + + it 'buckets by the forwarded client address behind a private-range proxy' do + proxied = { 'REMOTE_ADDR' => '10.0.0.1' } + expect(status_after(21, '/oauth/token', proxied.merge('HTTP_X_FORWARDED_FOR' => '203.0.113.9'))).to eq(503) + expect(status_after(1, '/oauth/token', proxied.merge('HTTP_X_FORWARDED_FOR' => '203.0.113.10'))).not_to eq(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..b91fabcd9 --- /dev/null +++ b/spec/requests/oauth_token_info_spec.rb @@ -0,0 +1,85 @@ +# typed: false +# 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 = response.parsed_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 'when the owner of a handoff token becomes privileged' do + let!(:handoff_token) { Doorkeeper::AccessToken.create!(application: application, resource_owner_id: user.id, scopes: 'public handoff', expires_in: 86_400) } + let!(:login_token) { Doorkeeper::AccessToken.create!(application: application, resource_owner_id: user.id, scopes: 'public') } + let!(:other_users_handoff_token) { Doorkeeper::AccessToken.create!(application: application, resource_owner_id: FactoryBot.create(:user).id, scopes: 'public handoff', expires_in: 86_400) } + + { + 'an administrator' => ->(user) { user.update!(administrator: true) }, + 'a teacher' => ->(user) { Teachership.create!(user: user, organization: FactoryBot.create(:organization)) }, + 'an assistant' => ->(user) { Assistantship.create!(user: user, course: FactoryBot.create(:course)) } + }.each do |role, grant| + it "refuses the handoff token once the user is #{role}" do + grant.call(user) + + token_info(handoff_token.token) + expect(response).to have_http_status(401) + token_info(login_token.token) + expect(response).to have_http_status(200) + token_info(other_users_handoff_token.token) + expect(response).to have_http_status(200) + end + end + + it 'keeps the handoff token when an unrelated attribute changes' do + user.update!(email: 'renamed@example.com') + + token_info(handoff_token.token) + expect(response).to have_http_status(200) + end + 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..91ba83384 --- /dev/null +++ b/spec/requests/rack_attack_throttles_spec.rb @@ -0,0 +1,120 @@ +# typed: false +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Rack::Attack throttles', type: :request do + include_context 'with Rack::Attack' + + let(:safe_headers) { { 'HTTP_RATELIMIT_PROTECTION_SAFE_API_KEY' => 'safe-key' } } + + around :each do |example| + ENV['RACK_ATTACK_SAFE_API_KEY'] = 'safe-key' + example.run + ensure + ENV.delete('RACK_ATTACK_SAFE_API_KEY') + end + + def app + Rack::Attack.new(->(_env) { [200, {}, ['ok']] }) + 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 + + describe 'path variants' do + { + '/oauth/token' => 20, + '/sessions' => 20, + '/api/v8/users' => 15, + '/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 '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 'counts user_ids that are not integers in one bucket' do + 10.times { |i| call(path, ip: "198.51.100.#{i}", **bearer, params: { user_id: "42 #{i}" }) } + + expect(call(path, ip: '198.51.100.99', **bearer, params: { user_id: '42x' })).to eq(503) + expect(call(path, ip: '198.51.100.99', **bearer, params: { user_id: '42' })).to eq(200) + 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..b3279cd16 --- /dev/null +++ b/spec/requests/secret_path_logging_spec.rb @@ -0,0 +1,58 @@ +# typed: false +# 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.pluck(: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