From 4e8ec50c0620164679ff70c5b05f6f518b3f7dc2 Mon Sep 17 00:00:00 2001 From: Henrik Nygren Date: Mon, 2 Nov 2026 13:41:02 +0200 Subject: [PATCH 01/18] Issue short-lived user access tokens to trusted backends --- app/controllers/api/v8/apidocs_controller.rb | 1 + .../user_access_tokens_controller.rb | 108 +++++++++++++++++ config/application.rb | 2 + config/initializers/doorkeeper.rb | 1 + config/initializers/rack_attack.rb | 5 + config/routes.rb | 4 + .../user_access_tokens_controller_spec.rb | 111 ++++++++++++++++++ 7 files changed, 232 insertions(+) create mode 100644 app/controllers/api/v8/application/user_access_tokens_controller.rb create mode 100644 spec/controllers/api/v8/application/user_access_tokens_controller_spec.rb diff --git a/app/controllers/api/v8/apidocs_controller.rb b/app/controllers/api/v8/apidocs_controller.rb index f8b984310..1a1980051 100644 --- a/app/controllers/api/v8/apidocs_controller.rb +++ b/app/controllers/api/v8/apidocs_controller.rb @@ -188,6 +188,7 @@ class ApidocsController < ActionController::Base Api::V8::Core::SubmissionsController, Api::V8::Core::Submissions::FeedbackController, Api::V8::Core::Submissions::ReviewsController, + Api::V8::Application::UserAccessTokensController, AvailablePoint, AwardedPoint, Course, diff --git a/app/controllers/api/v8/application/user_access_tokens_controller.rb b/app/controllers/api/v8/application/user_access_tokens_controller.rb new file mode 100644 index 000000000..780b0c1b3 --- /dev/null +++ b/app/controllers/api/v8/application/user_access_tokens_controller.rb @@ -0,0 +1,108 @@ +# frozen_string_literal: true + +module Api + module V8 + module Application + # Issues a login access token for a user to a trusted backend, e.g. for a sign-in handoff + # verified elsewhere. Only allow-listed client_credentials applications holding the + # issue_user_access_tokens scope may call it. + class UserAccessTokensController < Api::V8::BaseController + include Swagger::Blocks + + REQUIRED_SCOPE = 'issue_user_access_tokens' + TOKEN_LIFETIME = 1.day + + swagger_path '/api/v8/application/user_access_tokens' do + operation :post do + key :description, "Issues an access token for the given user, owned by the configured target application. Requires a client_credentials token of an allow-listed application with the #{REQUIRED_SCOPE} scope. Administrators, teachers and assistants are refused. The token expires after #{TOKEN_LIFETIME.to_i} seconds." + key :operationId, 'issueUserAccessToken' + key :produces, ['application/json'] + key :tags, ['user'] + parameter do + key :name, :user_id + key :in, :body + key :description, "The user's id" + key :required, true + schema do + key :'$ref', :UserAccessTokenInput + end + end + response 200 do + key :description, 'The issued access token' + schema do + key :'$ref', :UserAccessToken + end + end + response 400, '$ref': '#/responses/error' + response 401, '$ref': '#/responses/error' + response 403, '$ref': '#/responses/error' + response 404, '$ref': '#/responses/error' + end + end + + swagger_schema :UserAccessTokenInput do + key :required, [:user_id] + property :user_id, type: :integer, example: 1 + end + + swagger_schema :UserAccessToken do + key :required, %i[access_token token_type created_at expires_in] + property :access_token, type: :string + property :token_type, type: :string, example: 'bearer' + property :created_at, type: :integer, description: 'Unix timestamp' + property :expires_in, type: :integer, description: 'Seconds from created_at', example: 86_400 + end + + skip_before_action :authenticate_user! + skip_authorization_check + + def create + return respond_unauthorized unless doorkeeper_token + return respond_forbidden unless authorized_issuer? + + user_id = Integer(params[:user_id].to_s, exception: false) + return respond_with_error('user_id must be an integer', 400) unless user_id + + user = User.find_by(id: user_id) + return respond_not_found('User not found') unless user + return respond_forbidden('Tokens are not issued for administrators, teachers or assistants') if privileged?(user) + + target_application = Doorkeeper::Application.find_by(uid: Rails.configuration.x.user_access_token_target_application_uid) + return respond_with_error('Target application is not configured', 500) unless target_application + + token = Doorkeeper::AccessToken.create_for( + application: target_application, + resource_owner: user, + scopes: 'public', + expires_in: TOKEN_LIFETIME.to_i + ) + Rails.logger.info("Issued user access token: issuer application uid=#{doorkeeper_token.application.uid} user_id=#{user.id}") + + render json: { + access_token: token.plaintext_token, + token_type: 'bearer', + created_at: token.created_at.to_i, + expires_in: token.expires_in + } + end + + private + def privileged?(user) + user.administrator? || user.teacherships.exists? || user.assistantships.exists? + end + + def authorized_issuer? + issuer_uids = Rails.configuration.x.user_access_token_issuer_application_uids + application = doorkeeper_token.application + # resource_owner_id is set on user tokens, which never qualify even from an allowed application. + # The target is a frontend client, so its credentials must not be usable to issue tokens. + doorkeeper_token.resource_owner_id.nil? && + application.present? && + issuer_uids.include?(application.uid) && + application.uid != Rails.configuration.x.user_access_token_target_application_uid && + doorkeeper_token.acceptable?([REQUIRED_SCOPE]) + end + end + end + end +end diff --git a/config/application.rb b/config/application.rb index 1b3f4a4e7..58c2f2e66 100644 --- a/config/application.rb +++ b/config/application.rb @@ -35,6 +35,8 @@ class Application < Rails::Application config.relative_url_root = SiteSetting.value('base_path') config.x.accept_courses_mooc_fi_tokens = ENV['ACCEPT_COURSES_MOOC_FI_TOKENS'] == 'true' + config.x.user_access_token_issuer_application_uids = ENV.fetch('USER_ACCESS_TOKEN_ISSUER_APPLICATION_UIDS', '').split(',').map(&:strip).reject(&:empty?) + config.x.user_access_token_target_application_uid = ENV['USER_ACCESS_TOKEN_TARGET_APPLICATION_UID'].presence config.middleware.insert_before 0, Rack::Cors, debug: true, logger: (-> { Rails.logger }) do allow do diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index 314a2356c..d4898f0b9 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -66,6 +66,7 @@ optional_scopes %I[ internal_beta_all openid + issue_user_access_tokens ] # Change the way client credentials are retrieved from the request object. diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index e2ed47366..fb8d217a4 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -100,6 +100,11 @@ def self.user_identifier_discriminator(access_token) end end + # Limit the number of issued user access tokens per ip to 120 per minute + throttle('user access tokens per ip per minute', limit: 120, period: 1.minute) do |req| + req.ip if req.path == '/api/v8/application/user_access_tokens' && req.post? + end + # Limit the number of password resets per ip to 10 per hour throttle('password resets per ip per hour', limit: 10, period: 1.hour) do |req| req.ip if req.path == '/password_reset_keys' && diff --git a/config/routes.rb b/config/routes.rb index 3dc83e7a7..f12da49bd 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -153,6 +153,10 @@ resources :application, param: :name, only: [] do resources :credentials, module: :application, only: :index end + + namespace :application do + resources :user_access_tokens, only: :create + end end end 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..1384d4de1 --- /dev/null +++ b/spec/controllers/api/v8/application/user_access_tokens_controller_spec.rb @@ -0,0 +1,111 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Api::V8::Application::UserAccessTokensController, type: :controller do + let!(:user) { FactoryBot.create(:user) } + let!(:admin) { FactoryBot.create(:admin) } + let!(:issuer_app) { Doorkeeper::Application.create!(name: 'mooc.fi backend', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob') } + let!(:other_app) { Doorkeeper::Application.create!(name: 'other backend', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob') } + let!(:target_app) { Doorkeeper::Application.create!(name: 'mooc.fi frontend', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob') } + let(:issuer_token) { Doorkeeper::AccessToken.create!(application: issuer_app, scopes: 'issue_user_access_tokens') } + + around :each do |example| + config = Rails.configuration.x + original = [config.user_access_token_issuer_application_uids, config.user_access_token_target_application_uid] + example.run + ensure + config.user_access_token_issuer_application_uids, config.user_access_token_target_application_uid = original + end + + before :each do + Rails.configuration.x.user_access_token_issuer_application_uids = [issuer_app.uid] + Rails.configuration.x.user_access_token_target_application_uid = target_app.uid + end + + def post_with(token, user_id) + request.headers['Authorization'] = "Bearer #{token.token}" if token + post :create, params: { user_id: user_id }, format: :json + end + + it 'issues a public one-day token for the user owned by the target application' do + post_with(issuer_token, user.id) + + expect(response).to have_http_status(200) + body = JSON.parse(response.body) + expect(body['token_type']).to eq('bearer') + issued = Doorkeeper::AccessToken.find_by(token: body['access_token']) + expect(issued.resource_owner_id).to eq(user.id) + expect(issued.application).to eq(target_app) + expect(issued.scopes.to_a).to eq(['public']) + 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) + end + + it 'requires a token' do + post_with(nil, user.id) + + expect(response).to have_http_status(401) + end + + it 'rejects an application that is not allow-listed' do + token = Doorkeeper::AccessToken.create!(application: other_app, scopes: 'issue_user_access_tokens') + post_with(token, user.id) + + expect(response).to have_http_status(403) + end + + it 'rejects a token without the scope' do + token = Doorkeeper::AccessToken.create!(application: issuer_app, scopes: 'public') + post_with(token, user.id) + + expect(response).to have_http_status(403) + end + + it 'rejects a user token even from the allow-listed application' do + token = Doorkeeper::AccessToken.create!(application: issuer_app, resource_owner_id: user.id, scopes: 'issue_user_access_tokens') + post_with(token, user.id) + + expect(response).to have_http_status(403) + end + + it 'rejects a revoked token' do + issuer_token.revoke + post_with(issuer_token, user.id) + + expect(response).to have_http_status(403) + end + + it 'returns 404 for an unknown user' do + post_with(issuer_token, User.maximum(:id) + 1) + + expect(response).to have_http_status(404) + end + + it 'refuses to issue tokens for administrators' do + expect do + post_with(issuer_token, admin.id) + end.not_to change { Doorkeeper::AccessToken.where(resource_owner_id: admin.id).count } + + expect(response).to have_http_status(403) + end + + it 'refuses to issue tokens for teachers' do + Teachership.create!(user: user, organization: FactoryBot.create(:organization)) + expect do + post_with(issuer_token, user.id) + end.not_to change { Doorkeeper::AccessToken.where(resource_owner_id: user.id).count } + + expect(response).to have_http_status(403) + end + + it 'refuses to issue tokens for assistants' do + Assistantship.create!(user: user, course: FactoryBot.create(:course)) + expect do + post_with(issuer_token, user.id) + end.not_to change { Doorkeeper::AccessToken.where(resource_owner_id: user.id).count } + + expect(response).to have_http_status(403) + end +end From c8ad7f745a2fc6eed5fb59c2e3bf3560a8001b6f Mon Sep 17 00:00:00 2001 From: Henrik Nygren Date: Mon, 2 Nov 2026 13:41:02 +0200 Subject: [PATCH 02/18] Refuse revoked and expired tokens with 401 --- app/controllers/api/beta/base_controller.rb | 2 +- .../user_access_tokens_controller.rb | 7 +++-- app/controllers/api/v8/base_controller.rb | 4 +-- app/controllers/auths_controller.rb | 2 +- config/initializers/rack_attack.rb | 3 +-- .../user_access_tokens_controller_spec.rb | 18 ++++++++++++- .../api/v8/base_controller_spec.rb | 27 ++++++++++++++++--- .../core/courses/reviews_controller_spec.rb | 8 +++--- .../core/courses/unlocks_controller_spec.rb | 2 +- .../api/v8/core/courses_controller_spec.rb | 2 +- .../exercises/solutions_controller_spec.rb | 4 +-- .../exercises/submissions_controller_spec.rb | 2 +- .../api/v8/core/exercises_controller_spec.rb | 2 +- .../organizations/courses_controller_spec.rb | 4 +-- .../submissions/feedback_controller_spec.rb | 2 +- .../submissions/reviews_controller_spec.rb | 6 ++--- .../v8/core/submissions_controller_spec.rb | 8 +++--- .../all_courses_with_this_template_spec.rb | 8 +++--- .../exercises/points_controller_spec.rb | 4 +-- .../exercises/users/points_controller_spec.rb | 2 +- .../v8/courses/exercises_controller_spec.rb | 8 +++--- .../api/v8/courses/points_controller_spec.rb | 4 +-- .../submissions/last_hour_controller_spec.rb | 4 +-- .../v8/courses/submissions_controller_spec.rb | 8 +++--- .../courses/users/points_controller_spec.rb | 10 +++---- .../users/submissions_controller_spec.rb | 10 +++---- .../api/v8/courses_controller_spec.rb | 4 +-- .../api/v8/courses_mooc_fi_token_auth_spec.rb | 2 +- .../exercises/points_controller_spec.rb | 4 +-- .../exercises/users/points_controller_spec.rb | 2 +- .../courses/exercises_controller_spec.rb | 8 +++--- .../courses/points_controller_spec.rb | 4 +-- .../courses/submissions_controller_spec.rb | 8 +++--- .../courses/users/points_controller_spec.rb | 10 +++---- .../users/submissions_controller_spec.rb | 10 +++---- .../organizations/courses_controller_spec.rb | 4 +-- .../api/v8/users_controller_spec.rb | 10 +++---- spec/controllers/auths_controller_spec.rb | 12 +++++++++ 38 files changed, 143 insertions(+), 96 deletions(-) diff --git a/app/controllers/api/beta/base_controller.rb b/app/controllers/api/beta/base_controller.rb index 37699d18a..0d56455eb 100644 --- a/app/controllers/api/beta/base_controller.rb +++ b/app/controllers/api/beta/base_controller.rb @@ -29,7 +29,7 @@ def present(hash) private def authenticate_user! - if doorkeeper_token + if doorkeeper_token&.accessible? @current_user = User.find(doorkeeper_token.resource_owner_id) end diff --git a/app/controllers/api/v8/application/user_access_tokens_controller.rb b/app/controllers/api/v8/application/user_access_tokens_controller.rb index 780b0c1b3..d7e5143dc 100644 --- a/app/controllers/api/v8/application/user_access_tokens_controller.rb +++ b/app/controllers/api/v8/application/user_access_tokens_controller.rb @@ -57,10 +57,10 @@ class UserAccessTokensController < Api::V8::BaseController skip_authorization_check def create - return respond_unauthorized unless doorkeeper_token + return respond_unauthorized unless doorkeeper_token&.accessible? return respond_forbidden unless authorized_issuer? - user_id = Integer(params[:user_id].to_s, exception: false) + user_id = Integer(params[:user_id].to_s, 10, exception: false) return respond_with_error('user_id must be an integer', 400) unless user_id user = User.find_by(id: user_id) @@ -97,8 +97,7 @@ def authorized_issuer? # resource_owner_id is set on user tokens, which never qualify even from an allowed application. # The target is a frontend client, so its credentials must not be usable to issue tokens. doorkeeper_token.resource_owner_id.nil? && - application.present? && - issuer_uids.include?(application.uid) && + issuer_uids.include?(application&.uid) && application.uid != Rails.configuration.x.user_access_token_target_application_uid && doorkeeper_token.acceptable?([REQUIRED_SCOPE]) end diff --git a/app/controllers/api/v8/base_controller.rb b/app/controllers/api/v8/base_controller.rb index c9b362808..eaa6ac34c 100644 --- a/app/controllers/api/v8/base_controller.rb +++ b/app/controllers/api/v8/base_controller.rb @@ -43,8 +43,8 @@ def present(hash) def authenticate_user! return @current_user if @current_user if doorkeeper_token - @current_user ||= User.find_by(id: doorkeeper_token.resource_owner_id) - raise 'Invalid token' unless @current_user + @current_user = User.find_by(id: doorkeeper_token.resource_owner_id) if doorkeeper_token.accessible? + return respond_unauthorized('Invalid token') unless @current_user elsif Rails.configuration.x.accept_courses_mooc_fi_tokens @current_user = CoursesMoocFiAuthentication.user_for(request) @auth_source = :courses_mooc_fi_token if @current_user diff --git a/app/controllers/auths_controller.rb b/app/controllers/auths_controller.rb index e79611689..dc2d99086 100644 --- a/app/controllers/auths_controller.rb +++ b/app/controllers/auths_controller.rb @@ -13,7 +13,7 @@ def show user = User.find_by(login: params[:username]) user ||= User.find_by('lower(email) = ?', params[:username].downcase) # Allows using oauth2 tokens of the new api for authenticating - res = if user && Doorkeeper::AccessToken.find_by(resource_owner_id: user.id, token: params[:session_id]) + res = if user && Doorkeeper::AccessToken.find_by(resource_owner_id: user.id, token: params[:session_id])&.accessible? OK_MESSAGE elsif user && find_session_by_id(params[:session_id])&.belongs_to?(user) OK_MESSAGE diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index fb8d217a4..35b6e2874 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -100,9 +100,8 @@ def self.user_identifier_discriminator(access_token) end end - # Limit the number of issued user access tokens per ip to 120 per minute throttle('user access tokens per ip per minute', limit: 120, period: 1.minute) do |req| - req.ip if req.path == '/api/v8/application/user_access_tokens' && req.post? + req.ip if req.path.start_with?('/api/v8/application/user_access_tokens') && req.post? end # Limit the number of password resets per ip to 10 per hour 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 index 1384d4de1..9a58ca163 100644 --- a/spec/controllers/api/v8/application/user_access_tokens_controller_spec.rb +++ b/spec/controllers/api/v8/application/user_access_tokens_controller_spec.rb @@ -74,7 +74,23 @@ def post_with(token, user_id) issuer_token.revoke post_with(issuer_token, user.id) - expect(response).to have_http_status(403) + expect(response).to have_http_status(401) + end + + it 'rejects an expired token' do + token = Doorkeeper::AccessToken.create!(application: issuer_app, scopes: 'issue_user_access_tokens', expires_in: 60, created_at: 2.minutes.ago) + post_with(token, user.id) + + expect(response).to have_http_status(401) + end + + it 'reads a zero-padded user_id as decimal' do + other_user = FactoryBot.create(:user) + post_with(issuer_token, "0#{other_user.id}") + + expect(response).to have_http_status(200) + issued = Doorkeeper::AccessToken.find_by(token: JSON.parse(response.body)['access_token']) + expect(issued.resource_owner_id).to eq(other_user.id) end it 'returns 404 for an unknown user' do diff --git a/spec/controllers/api/v8/base_controller_spec.rb b/spec/controllers/api/v8/base_controller_spec.rb index 4d3404e23..8fb4b80b7 100644 --- a/spec/controllers/api/v8/base_controller_spec.rb +++ b/spec/controllers/api/v8/base_controller_spec.rb @@ -28,7 +28,7 @@ def index context 'when logged in' do let(:user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true } it { expect(subject.id).to be user.id } end end @@ -38,9 +38,30 @@ def index allow(controller).to receive(:doorkeeper_token) { token } end context 'with a valid token having invalid user ID' do - let(:token) { double resource_owner_id: -1, acceptable?: true } + let(:token) { double resource_owner_id: -1, accessible?: true } it 'request is not successful' do - expect { get :index }.to raise_error(RuntimeError) + get :index + expect(response).to have_http_status(401) + end + end + context 'with a revoked token' do + let(:user) { FactoryBot.create(:user) } + let(:application) { Doorkeeper::Application.create!(name: 'client', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob') } + let(:token) { Doorkeeper::AccessToken.create!(application: application, resource_owner_id: user.id, scopes: 'public').tap(&:revoke) } + it 'request is not successful' do + request.accept = 'application/json' + get :index + expect(response).to have_http_status(401) + expect(JSON.parse(response.body)).to eq('error' => 'Invalid token') + end + end + context 'with an expired token' do + let(:user) { FactoryBot.create(:user) } + let(:application) { Doorkeeper::Application.create!(name: 'client', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob') } + let(:token) { Doorkeeper::AccessToken.create!(application: application, resource_owner_id: user.id, scopes: 'public', expires_in: 60, created_at: 2.minutes.ago) } + it 'request is not successful' do + get :index + expect(response).to have_http_status(401) end end end diff --git a/spec/controllers/api/v8/core/courses/reviews_controller_spec.rb b/spec/controllers/api/v8/core/courses/reviews_controller_spec.rb index 44ec90c85..04dc78abf 100644 --- a/spec/controllers/api/v8/core/courses/reviews_controller_spec.rb +++ b/spec/controllers/api/v8/core/courses/reviews_controller_spec.rb @@ -18,7 +18,7 @@ describe "GET reviews for a course for current user's submissions" do describe 'as admin' do let(:user) { FactoryBot.create(:admin) } - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true } describe 'when course id given' do it "shows only user's submission's review information" do @@ -46,7 +46,7 @@ describe 'as user' do let(:user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true } describe 'when course id given' do it "shows only user's submission's review information" do @@ -100,7 +100,7 @@ describe 'Update review info' do describe 'As an admin' do let(:user) { FactoryBot.create(:admin) } - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true } before :each do controller.current_user = user end @@ -144,7 +144,7 @@ describe 'As an user' do describe 'when trying to edit review' do let(:user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true } it 'should deny access' do FactoryBot.create(:review, submission: submission) exercise.reload diff --git a/spec/controllers/api/v8/core/courses/unlocks_controller_spec.rb b/spec/controllers/api/v8/core/courses/unlocks_controller_spec.rb index 19c924c8e..4e5390e6b 100644 --- a/spec/controllers/api/v8/core/courses/unlocks_controller_spec.rb +++ b/spec/controllers/api/v8/core/courses/unlocks_controller_spec.rb @@ -14,7 +14,7 @@ describe 'POST unlock exercises' do describe 'as an authenticated user' do - let!(:token) { double resource_owner_id: user.id, acceptable?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true } it 'should allow unlocking exercises' do post :create, params: { organization_slug: organization.id, course_id: course.id } diff --git a/spec/controllers/api/v8/core/courses_controller_spec.rb b/spec/controllers/api/v8/core/courses_controller_spec.rb index 15c420e34..78492051f 100644 --- a/spec/controllers/api/v8/core/courses_controller_spec.rb +++ b/spec/controllers/api/v8/core/courses_controller_spec.rb @@ -11,7 +11,7 @@ describe 'GET course details show' do describe 'in JSON format with valid token' do - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true } let(:course) { FactoryBot.create(:course, name: 'Course1') } before :each do course.exercises << FactoryBot.create(:returnable_exercise, name: 'Exercise1', course: course) diff --git a/spec/controllers/api/v8/core/exercises/solutions_controller_spec.rb b/spec/controllers/api/v8/core/exercises/solutions_controller_spec.rb index 6ed9749da..22bc972ba 100644 --- a/spec/controllers/api/v8/core/exercises/solutions_controller_spec.rb +++ b/spec/controllers/api/v8/core/exercises/solutions_controller_spec.rb @@ -20,7 +20,7 @@ describe 'Downloading an exercise solution zip' do describe 'as an admin' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true } it 'should succeed if the exercise exists' do repo = clone_course_repo(course) @@ -34,7 +34,7 @@ end describe 'as a student' do - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true } describe 'if the course exists' do it 'should succeed if I have already solved it' do diff --git a/spec/controllers/api/v8/core/exercises/submissions_controller_spec.rb b/spec/controllers/api/v8/core/exercises/submissions_controller_spec.rb index 8b150b0f5..bb844672e 100644 --- a/spec/controllers/api/v8/core/exercises/submissions_controller_spec.rb +++ b/spec/controllers/api/v8/core/exercises/submissions_controller_spec.rb @@ -29,7 +29,7 @@ def create_submission(file) describe 'Creating a submission' do describe 'as an authenticated user' do - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true } it 'should accept submissions when the deadline is open' do exercise.deadline_spec = ['1.1.2100'].to_json diff --git a/spec/controllers/api/v8/core/exercises_controller_spec.rb b/spec/controllers/api/v8/core/exercises_controller_spec.rb index aa27b15a6..0d3b7b859 100644 --- a/spec/controllers/api/v8/core/exercises_controller_spec.rb +++ b/spec/controllers/api/v8/core/exercises_controller_spec.rb @@ -45,7 +45,7 @@ end describe 'When logged in' do - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true } describe 'and correct exercise id is given' do it 'should return correct data' do submission = FactoryBot.create(:submission, course: course, user: user, exercise: exercise) diff --git a/spec/controllers/api/v8/core/organizations/courses_controller_spec.rb b/spec/controllers/api/v8/core/organizations/courses_controller_spec.rb index 62f96a8e5..b18fc5bd1 100644 --- a/spec/controllers/api/v8/core/organizations/courses_controller_spec.rb +++ b/spec/controllers/api/v8/core/organizations/courses_controller_spec.rb @@ -16,7 +16,7 @@ describe 'GET courses' do describe 'as admin' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true } it 'shows a list of collections of course urls' do get :index, params: { organization_slug: organization.slug } json = JSON.parse response.body @@ -31,7 +31,7 @@ end end describe 'as user' do - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true } it 'shows a list of collections of course urls' do get :index, params: { organization_slug: organization.slug } json = JSON.parse response.body diff --git a/spec/controllers/api/v8/core/submissions/feedback_controller_spec.rb b/spec/controllers/api/v8/core/submissions/feedback_controller_spec.rb index f2d91ff1b..c81f5dc52 100644 --- a/spec/controllers/api/v8/core/submissions/feedback_controller_spec.rb +++ b/spec/controllers/api/v8/core/submissions/feedback_controller_spec.rb @@ -16,7 +16,7 @@ describe 'POST create' do describe 'As a student' do - let(:token) { double resource_owner_id: student.id, acceptable?: true } + let(:token) { double resource_owner_id: student.id, accessible?: true } it 'can give feedback' do # pending 'Failing for some reason' diff --git a/spec/controllers/api/v8/core/submissions/reviews_controller_spec.rb b/spec/controllers/api/v8/core/submissions/reviews_controller_spec.rb index 2553cdf8b..1d9e8a004 100644 --- a/spec/controllers/api/v8/core/submissions/reviews_controller_spec.rb +++ b/spec/controllers/api/v8/core/submissions/reviews_controller_spec.rb @@ -18,7 +18,7 @@ describe 'POST create' do describe 'As an admin' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true } it 'can make new review' do expect do @@ -29,7 +29,7 @@ end describe 'As a teacher' do - let(:token) { double resource_owner_id: teacher.id, acceptable?: true } + let(:token) { double resource_owner_id: teacher.id, accessible?: true } it 'can make new review' do expect do @@ -40,7 +40,7 @@ end describe 'As a student' do - let(:token) { double resource_owner_id: student.id, acceptable?: true } + let(:token) { double resource_owner_id: student.id, accessible?: true } it "can't make new review" do expect do diff --git a/spec/controllers/api/v8/core/submissions_controller_spec.rb b/spec/controllers/api/v8/core/submissions_controller_spec.rb index 439ed7743..9d9ebdc97 100644 --- a/spec/controllers/api/v8/core/submissions_controller_spec.rb +++ b/spec/controllers/api/v8/core/submissions_controller_spec.rb @@ -27,7 +27,7 @@ describe 'Downloading a submission as zip' do describe 'as an admin' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true } it "should allow to download everyone's submissions" do # pending("test that submission zip's content is correct") @@ -40,7 +40,7 @@ before :each do Teachership.create(user: teacher, organization: organization) end - let(:token) { double resource_owner_id: teacher.id, acceptable?: true } + let(:token) { double resource_owner_id: teacher.id, accessible?: true } it "should allow to download own organization's submissions" do # pending("test that submission zip's content is correct") @@ -63,7 +63,7 @@ before :each do Assistantship.create(user: assistant, course: course) end - let(:token) { double resource_owner_id: assistant.id, acceptable?: true } + let(:token) { double resource_owner_id: assistant.id, accessible?: true } it "should allow to download own course's submissions" do # pending("test that submission zip's content is correct") @@ -82,7 +82,7 @@ end end describe 'as a student' do - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true } it 'should allow to download own submissions' do # pending("test that submission zip's content is correct") diff --git a/spec/controllers/api/v8/courses/all_courses_with_this_template_spec.rb b/spec/controllers/api/v8/courses/all_courses_with_this_template_spec.rb index 587604361..2428e507e 100644 --- a/spec/controllers/api/v8/courses/all_courses_with_this_template_spec.rb +++ b/spec/controllers/api/v8/courses/all_courses_with_this_template_spec.rb @@ -34,7 +34,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: admin.id, acceptable?: true } + let!(:token) { double resource_owner_id: admin.id, accessible?: true } it 'should show all courses with same template' do two_courses_by_template_id @@ -62,7 +62,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: teacher.id, acceptable?: true } + let!(:token) { double resource_owner_id: teacher.id, accessible?: true } it 'should show all courses with same template' do two_courses_by_template_id @@ -90,7 +90,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: assistant.id, acceptable?: true } + let!(:token) { double resource_owner_id: assistant.id, accessible?: true } it 'should show all courses with same template' do two_courses_by_template_id @@ -118,7 +118,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: user.id, acceptable?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true } it 'should show all courses with same template' do two_courses_by_template_id diff --git a/spec/controllers/api/v8/courses/exercises/points_controller_spec.rb b/spec/controllers/api/v8/courses/exercises/points_controller_spec.rb index 3428b940b..57190cd0d 100644 --- a/spec/controllers/api/v8/courses/exercises/points_controller_spec.rb +++ b/spec/controllers/api/v8/courses/exercises/points_controller_spec.rb @@ -20,7 +20,7 @@ end describe 'As an admin' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true } describe 'when searching for all users awarded points' do describe 'using course name' do it 'should return all users awarded points of the exercise' do @@ -46,7 +46,7 @@ end describe 'As any user' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true } describe 'when searching awarded points' do describe 'and no points are found' do it 'should return an empty array' do diff --git a/spec/controllers/api/v8/courses/exercises/users/points_controller_spec.rb b/spec/controllers/api/v8/courses/exercises/users/points_controller_spec.rb index ce18f8de7..3478189b1 100644 --- a/spec/controllers/api/v8/courses/exercises/users/points_controller_spec.rb +++ b/spec/controllers/api/v8/courses/exercises/users/points_controller_spec.rb @@ -30,7 +30,7 @@ end describe 'As any user' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true } describe 'when searching for users awarded points by user id' do describe 'and using course id' do it 'should return only correct users awarded points' do diff --git a/spec/controllers/api/v8/courses/exercises_controller_spec.rb b/spec/controllers/api/v8/courses/exercises_controller_spec.rb index 508148238..6a4d6f0bf 100644 --- a/spec/controllers/api/v8/courses/exercises_controller_spec.rb +++ b/spec/controllers/api/v8/courses/exercises_controller_spec.rb @@ -15,7 +15,7 @@ end describe 'As an admin' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true } describe 'when course id is given' do it 'should return successful response' do get :index, params: { course_id: course.id } @@ -33,7 +33,7 @@ end describe 'As a student' do - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true } describe 'when course id is given' do it 'should return successful response' do get :index, params: { course_id: course.id } @@ -51,7 +51,7 @@ end describe 'As a student without unlock rows' do - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true } def unlocked_flags get :index, params: { course_id: course.id } @@ -81,7 +81,7 @@ def unlocked_flags end describe 'As any user' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true } describe 'when course id could not be found' do it 'should return error' do get :index, params: { course_id: '123' } diff --git a/spec/controllers/api/v8/courses/points_controller_spec.rb b/spec/controllers/api/v8/courses/points_controller_spec.rb index db947acce..c30f4e538 100644 --- a/spec/controllers/api/v8/courses/points_controller_spec.rb +++ b/spec/controllers/api/v8/courses/points_controller_spec.rb @@ -20,7 +20,7 @@ describe 'GET points for a course by id' do describe 'as admin' do let(:current_user) { FactoryBot.create(:admin) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true } describe 'when course ID given' do it "shows only user's point information" do @@ -47,7 +47,7 @@ describe 'as user' do let(:current_user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true } describe 'when course ID given' do it "shows only user's point information" do diff --git a/spec/controllers/api/v8/courses/submissions/last_hour_controller_spec.rb b/spec/controllers/api/v8/courses/submissions/last_hour_controller_spec.rb index 649c23d3e..a3d882f77 100644 --- a/spec/controllers/api/v8/courses/submissions/last_hour_controller_spec.rb +++ b/spec/controllers/api/v8/courses/submissions/last_hour_controller_spec.rb @@ -18,7 +18,7 @@ describe 'index' do context 'as a regular user' do - let!(:token) { double resource_owner_id: regular_user.id, acceptable?: true } + let!(:token) { double resource_owner_id: regular_user.id, accessible?: true } it 'should not allow access' do get :index, params: { course_id: course.id } @@ -27,7 +27,7 @@ end context 'as an admin' do - let!(:token) { double resource_owner_id: admin_user.id, acceptable?: true } + let!(:token) { double resource_owner_id: admin_user.id, accessible?: true } it 'gives submission ids for the last hour' do get :index, params: { course_id: course.id } diff --git a/spec/controllers/api/v8/courses/submissions_controller_spec.rb b/spec/controllers/api/v8/courses/submissions_controller_spec.rb index 10b627e33..202e2134a 100644 --- a/spec/controllers/api/v8/courses/submissions_controller_spec.rb +++ b/spec/controllers/api/v8/courses/submissions_controller_spec.rb @@ -37,7 +37,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: admin.id, acceptable?: true } + let!(:token) { double resource_owner_id: admin.id, accessible?: true } it 'should show all of the submissions' do two_subs_by_id @@ -61,7 +61,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: teacher.id, acceptable?: true } + let!(:token) { double resource_owner_id: teacher.id, accessible?: true } it 'should show all of the submissions in my organizations' do two_subs_by_id @@ -89,7 +89,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: assistant.id, acceptable?: true } + let!(:token) { double resource_owner_id: assistant.id, accessible?: true } it 'should show all of the submissions in my courses' do two_subs_by_id @@ -117,7 +117,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: user.id, acceptable?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true } it 'should show my own submissions' do all_own_subs(course_id: course.id) diff --git a/spec/controllers/api/v8/courses/users/points_controller_spec.rb b/spec/controllers/api/v8/courses/users/points_controller_spec.rb index f9966ee51..cabcdeac0 100644 --- a/spec/controllers/api/v8/courses/users/points_controller_spec.rb +++ b/spec/controllers/api/v8/courses/users/points_controller_spec.rb @@ -21,7 +21,7 @@ describe "GET current user's points" do describe 'as admin' do let(:current_user) { FactoryBot.create(:admin) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true } describe 'when course ID given' do it "shows only user's point information" do @@ -48,7 +48,7 @@ describe 'as user' do let(:current_user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true } describe 'when course ID given' do it "shows only user's point information" do @@ -99,7 +99,7 @@ describe "GET user's points" do describe 'as admin' do let(:current_user) { FactoryBot.create(:admin) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true } describe 'when course ID given' do it "shows only user's point information" do @@ -126,7 +126,7 @@ describe 'as user' do let(:current_user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true } describe 'when course ID given' do it "shows only user's point information" do @@ -153,7 +153,7 @@ describe 'as another user' do let(:current_user) { user2 } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true } describe 'when course ID given' do it "shows only user's point information" do diff --git a/spec/controllers/api/v8/courses/users/submissions_controller_spec.rb b/spec/controllers/api/v8/courses/users/submissions_controller_spec.rb index 9558e0608..05d6419de 100644 --- a/spec/controllers/api/v8/courses/users/submissions_controller_spec.rb +++ b/spec/controllers/api/v8/courses/users/submissions_controller_spec.rb @@ -38,7 +38,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: admin.id, acceptable?: true } + let!(:token) { double resource_owner_id: admin.id, accessible?: true } it "should show given user's submissions" do users_own_subs(course_id: course.id, user_id: user.id) @@ -62,7 +62,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: teacher.id, acceptable?: true } + let!(:token) { double resource_owner_id: teacher.id, accessible?: true } it "should show given user's submissions in my organizations" do users_own_subs(course_id: course.id, user_id: user.id) @@ -90,7 +90,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: assistant.id, acceptable?: true } + let!(:token) { double resource_owner_id: assistant.id, accessible?: true } it "should show given user's submissions in my courses" do users_own_subs(course_id: course.id, user_id: user.id) @@ -118,7 +118,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: user.id, acceptable?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true } it 'should show my own submissions' do users_own_subs(course_id: course.id, user_id: user.id) @@ -163,7 +163,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: user.id, acceptable?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true } it 'should show my own submissions' do users_own_subs(course_id: course.id, user_id: 'current') diff --git a/spec/controllers/api/v8/courses_controller_spec.rb b/spec/controllers/api/v8/courses_controller_spec.rb index a497c1f06..1d20e6fb7 100644 --- a/spec/controllers/api/v8/courses_controller_spec.rb +++ b/spec/controllers/api/v8/courses_controller_spec.rb @@ -15,7 +15,7 @@ describe 'GET course by id' do describe 'as admin' do let(:current_user) { admin } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true } describe 'when course ID given' do it 'shows course information' do @@ -45,7 +45,7 @@ describe 'as user' do let(:current_user) { user } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true } describe 'when course ID given' do it 'shows course information' do diff --git a/spec/controllers/api/v8/courses_mooc_fi_token_auth_spec.rb b/spec/controllers/api/v8/courses_mooc_fi_token_auth_spec.rb index 7b17cf17c..11442729d 100644 --- a/spec/controllers/api/v8/courses_mooc_fi_token_auth_spec.rb +++ b/spec/controllers/api/v8/courses_mooc_fi_token_auth_spec.rb @@ -52,7 +52,7 @@ def introspection_returns(value) it 'prefers a native Doorkeeper token' do user = FactoryBot.create(:user) - allow(controller).to receive(:doorkeeper_token).and_return(double(resource_owner_id: user.id, acceptable?: true)) + allow(controller).to receive(:doorkeeper_token).and_return(double(resource_owner_id: user.id, accessible?: true)) expect(CoursesMoocFiTokenIntrospector).not_to receive(:introspect) get :index expect(current_user).to eq(user) diff --git a/spec/controllers/api/v8/organizations/courses/exercises/points_controller_spec.rb b/spec/controllers/api/v8/organizations/courses/exercises/points_controller_spec.rb index 7b2b7e1fa..90cb5a8c9 100644 --- a/spec/controllers/api/v8/organizations/courses/exercises/points_controller_spec.rb +++ b/spec/controllers/api/v8/organizations/courses/exercises/points_controller_spec.rb @@ -22,7 +22,7 @@ end describe 'As an admin' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true } describe 'when searching for all users awarded points' do describe 'using course name' do it 'should return all users awarded points of the exercise' do @@ -48,7 +48,7 @@ end describe 'As any user' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true } describe 'when searching awarded points' do describe 'and no points are found' do it 'should return an empty array' do diff --git a/spec/controllers/api/v8/organizations/courses/exercises/users/points_controller_spec.rb b/spec/controllers/api/v8/organizations/courses/exercises/users/points_controller_spec.rb index 41d7eca96..e5207c9cb 100644 --- a/spec/controllers/api/v8/organizations/courses/exercises/users/points_controller_spec.rb +++ b/spec/controllers/api/v8/organizations/courses/exercises/users/points_controller_spec.rb @@ -31,7 +31,7 @@ end describe 'As any user' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true } describe 'when searching for users awarded points by user id' do describe 'and using course id' do it 'should return only correct users awarded points' do diff --git a/spec/controllers/api/v8/organizations/courses/exercises_controller_spec.rb b/spec/controllers/api/v8/organizations/courses/exercises_controller_spec.rb index faacf1cca..97dddd7a2 100644 --- a/spec/controllers/api/v8/organizations/courses/exercises_controller_spec.rb +++ b/spec/controllers/api/v8/organizations/courses/exercises_controller_spec.rb @@ -20,7 +20,7 @@ end describe 'As an admin' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true } describe 'when course name is given' do it 'should return successful response' do get :index, params: { course_name: course_name, organization_slug: organization.slug } @@ -38,7 +38,7 @@ end describe 'As a student' do - let(:token) { double resource_owner_id: user.id, acceptable?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true } describe 'when course name is given' do it 'should return successful response' do get :index, params: { course_name: course_name, organization_slug: organization.slug } @@ -56,7 +56,7 @@ end describe 'As any user' do - let(:token) { double resource_owner_id: admin.id, acceptable?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true } describe 'when course name could not be found' do it 'should return error' do get :index, params: { course_name: 'null', organization_slug: organization.slug } @@ -67,7 +67,7 @@ describe 'Downloading an exercise zip' do describe 'as an unauthenticated user' do - let(:token) { double resource_owner_id: Guest.new.id, acceptable?: true } + let(:token) { double resource_owner_id: Guest.new.id, accessible?: true } it 'should succeed if the course name and slug are correct', driver: :rack_test do repo = clone_course_repo(course) diff --git a/spec/controllers/api/v8/organizations/courses/points_controller_spec.rb b/spec/controllers/api/v8/organizations/courses/points_controller_spec.rb index 340a07f58..ccc122bbc 100644 --- a/spec/controllers/api/v8/organizations/courses/points_controller_spec.rb +++ b/spec/controllers/api/v8/organizations/courses/points_controller_spec.rb @@ -21,7 +21,7 @@ describe 'GET points for a course' do describe 'as admin' do let(:current_user) { FactoryBot.create(:admin) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true } describe 'when course name given' do it "shows only user's point information" do @@ -48,7 +48,7 @@ describe 'as user' do let(:current_user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true } describe 'when course name given' do it "shows only user's point information" do diff --git a/spec/controllers/api/v8/organizations/courses/submissions_controller_spec.rb b/spec/controllers/api/v8/organizations/courses/submissions_controller_spec.rb index 51c576cae..070098715 100644 --- a/spec/controllers/api/v8/organizations/courses/submissions_controller_spec.rb +++ b/spec/controllers/api/v8/organizations/courses/submissions_controller_spec.rb @@ -39,7 +39,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: admin.id, acceptable?: true } + let!(:token) { double resource_owner_id: admin.id, accessible?: true } it 'should show all of the submissions' do two_subs_by_name @@ -63,7 +63,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: teacher.id, acceptable?: true } + let!(:token) { double resource_owner_id: teacher.id, accessible?: true } it 'should show all of the submissions in my organizations' do two_subs_by_name @@ -91,7 +91,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: assistant.id, acceptable?: true } + let!(:token) { double resource_owner_id: assistant.id, accessible?: true } it 'should show all of the submissions in my courses' do two_subs_by_name @@ -119,7 +119,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: user.id, acceptable?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true } it 'should show my own submissions' do all_own_subs(organization_slug: organization.slug, course_name: course_name) diff --git a/spec/controllers/api/v8/organizations/courses/users/points_controller_spec.rb b/spec/controllers/api/v8/organizations/courses/users/points_controller_spec.rb index bd5256b66..ea0ed45b1 100644 --- a/spec/controllers/api/v8/organizations/courses/users/points_controller_spec.rb +++ b/spec/controllers/api/v8/organizations/courses/users/points_controller_spec.rb @@ -22,7 +22,7 @@ describe "GET current user's points" do describe 'as admin' do let(:current_user) { FactoryBot.create(:admin) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true } describe 'when course name given' do it "shows only user's point information" do @@ -49,7 +49,7 @@ describe 'as user' do let(:current_user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true } describe 'when course name given' do it "shows only user's point information" do @@ -100,7 +100,7 @@ describe "GET user's points" do describe 'as admin' do let(:current_user) { FactoryBot.create(:admin) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true } describe 'when course name given' do it "shows only user's point information" do @@ -127,7 +127,7 @@ describe 'as user' do let(:current_user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true } describe 'when course name given' do it "shows only user's point information" do @@ -154,7 +154,7 @@ describe 'as another user' do let(:current_user) { user2 } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true } describe 'when course name given' do it "shows only user's point information" do diff --git a/spec/controllers/api/v8/organizations/courses/users/submissions_controller_spec.rb b/spec/controllers/api/v8/organizations/courses/users/submissions_controller_spec.rb index f2f9bed6c..207d143ac 100644 --- a/spec/controllers/api/v8/organizations/courses/users/submissions_controller_spec.rb +++ b/spec/controllers/api/v8/organizations/courses/users/submissions_controller_spec.rb @@ -38,7 +38,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: admin.id, acceptable?: true } + let!(:token) { double resource_owner_id: admin.id, accessible?: true } it "should show given user's submissions" do users_own_subs(organization_slug: organization.slug, course_name: course_name, user_id: user.id) @@ -62,7 +62,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: teacher.id, acceptable?: true } + let!(:token) { double resource_owner_id: teacher.id, accessible?: true } it "should show given user's submissions in my organizations" do users_own_subs(organization_slug: organization.slug, course_name: course_name, user_id: user.id) @@ -90,7 +90,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: assistant.id, acceptable?: true } + let!(:token) { double resource_owner_id: assistant.id, accessible?: true } it "should show given user's submissions in my courses" do users_own_subs(organization_slug: organization.slug, course_name: course_name, user_id: user.id) @@ -118,7 +118,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: user.id, acceptable?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true } it 'should show my own submissions' do users_own_subs(organization_slug: organization.slug, course_name: course_name, user_id: user.id) @@ -163,7 +163,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: user.id, acceptable?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true } it 'should show my own submissions' do users_own_subs(organization_slug: organization.slug, course_name: course_name, user_id: 'current') diff --git a/spec/controllers/api/v8/organizations/courses_controller_spec.rb b/spec/controllers/api/v8/organizations/courses_controller_spec.rb index c573445c3..00e3f9dee 100644 --- a/spec/controllers/api/v8/organizations/courses_controller_spec.rb +++ b/spec/controllers/api/v8/organizations/courses_controller_spec.rb @@ -17,7 +17,7 @@ describe 'GET course by name' do describe 'when logged as admin' do let(:current_user) { admin } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true } describe 'when organization id and course name given' do it 'shows course information' do @@ -60,7 +60,7 @@ describe 'when logged as user' do let(:current_user) { user } - let(:token) { double resource_owner_id: current_user.id, acceptable?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true } describe 'when organization id and course name given' do it 'shows course information' do diff --git a/spec/controllers/api/v8/users_controller_spec.rb b/spec/controllers/api/v8/users_controller_spec.rb index 66fc3b696..98aa0ab07 100644 --- a/spec/controllers/api/v8/users_controller_spec.rb +++ b/spec/controllers/api/v8/users_controller_spec.rb @@ -12,7 +12,7 @@ end describe "GET current user's info with an access token" do - let!(:token) { double resource_owner_id: user.id, acceptable?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true } it 'user sees own info' do get :show, params: { id: 'current' } @@ -30,7 +30,7 @@ end context 'as an administrator' do - let!(:token) { double resource_owner_id: admin.id, acceptable?: true } + let!(:token) { double resource_owner_id: admin.id, accessible?: true } it 'user sees own info' do get :show, params: { id: 'current' } @@ -40,7 +40,7 @@ end describe "GET user's info with user id" do describe 'using user token' do - let!(:token) { double resource_owner_id: user.id, acceptable?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true } it 'user sees own info' do get :show, params: { id: user.id } @@ -57,7 +57,7 @@ end end describe 'using admin token' do - let!(:token) { double resource_owner_id: admin.id, acceptable?: true } + let!(:token) { double resource_owner_id: admin.id, accessible?: true } it "admin can see everyone's infos" do get :show, params: { id: user.id } @@ -123,7 +123,7 @@ end describe 'PUT update password' do - let!(:token) { double resource_owner_id: user.id, acceptable?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true } before :each do user.password = 'oldpassword' diff --git a/spec/controllers/auths_controller_spec.rb b/spec/controllers/auths_controller_spec.rb index 94e9b50df..8f902f0cf 100644 --- a/spec/controllers/auths_controller_spec.rb +++ b/spec/controllers/auths_controller_spec.rb @@ -43,4 +43,16 @@ expect(response).to be_successful expect(response.body).to eq('OK') end + + it 'accepts an access token only while it is not revoked' do + application = Doorkeeper::Application.create!(name: 'client', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob') + token = Doorkeeper::AccessToken.create!(application: application, resource_owner_id: @user.id, scopes: 'public') + + post :show, params: { username: 'foo', session_id: token.token, format: :text } + expect(response.body).to eq('OK') + + token.revoke + post :show, params: { username: 'foo', session_id: token.token, format: :text } + expect(response.body).to eq('FAIL') + end end From caa6ae91d217a917b9763a4b1ecd5b38314aee16 Mon Sep 17 00:00:00 2001 From: Henrik Nygren Date: Mon, 2 Nov 2026 14:00:09 +0200 Subject: [PATCH 03/18] Keep handoff tokens from changing credentials or deleting accounts --- .../user_access_tokens_controller.rb | 4 +- app/controllers/api/v8/base_controller.rb | 4 ++ app/controllers/api/v8/users_controller.rb | 7 ++ app/models/ability.rb | 22 ++++-- .../user_access_tokens_controller_spec.rb | 4 +- .../api/v8/base_controller_spec.rb | 4 +- .../core/courses/reviews_controller_spec.rb | 8 +-- .../core/courses/unlocks_controller_spec.rb | 2 +- .../api/v8/core/courses_controller_spec.rb | 2 +- .../exercises/solutions_controller_spec.rb | 4 +- .../exercises/submissions_controller_spec.rb | 2 +- .../api/v8/core/exercises_controller_spec.rb | 2 +- .../organizations/courses_controller_spec.rb | 4 +- .../submissions/feedback_controller_spec.rb | 2 +- .../submissions/reviews_controller_spec.rb | 6 +- .../v8/core/submissions_controller_spec.rb | 8 +-- .../all_courses_with_this_template_spec.rb | 8 +-- .../exercises/points_controller_spec.rb | 4 +- .../exercises/users/points_controller_spec.rb | 2 +- .../v8/courses/exercises_controller_spec.rb | 8 +-- .../api/v8/courses/points_controller_spec.rb | 4 +- .../submissions/last_hour_controller_spec.rb | 4 +- .../v8/courses/submissions_controller_spec.rb | 8 +-- .../courses/users/points_controller_spec.rb | 10 +-- .../users/submissions_controller_spec.rb | 10 +-- .../api/v8/courses_controller_spec.rb | 4 +- .../api/v8/courses_mooc_fi_token_auth_spec.rb | 2 +- .../exercises/points_controller_spec.rb | 4 +- .../exercises/users/points_controller_spec.rb | 2 +- .../courses/exercises_controller_spec.rb | 8 +-- .../courses/points_controller_spec.rb | 4 +- .../courses/submissions_controller_spec.rb | 8 +-- .../courses/users/points_controller_spec.rb | 10 +-- .../users/submissions_controller_spec.rb | 10 +-- .../organizations/courses_controller_spec.rb | 4 +- .../api/v8/users_controller_spec.rb | 69 +++++++++++++++++-- 36 files changed, 173 insertions(+), 95 deletions(-) diff --git a/app/controllers/api/v8/application/user_access_tokens_controller.rb b/app/controllers/api/v8/application/user_access_tokens_controller.rb index d7e5143dc..6ee261d41 100644 --- a/app/controllers/api/v8/application/user_access_tokens_controller.rb +++ b/app/controllers/api/v8/application/user_access_tokens_controller.rb @@ -14,7 +14,7 @@ class UserAccessTokensController < Api::V8::BaseController 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 #{REQUIRED_SCOPE} scope. Administrators, teachers and assistants are refused. The token expires after #{TOKEN_LIFETIME.to_i} seconds." + 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 #{REQUIRED_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'] @@ -73,7 +73,7 @@ def create token = Doorkeeper::AccessToken.create_for( application: target_application, resource_owner: user, - scopes: 'public', + scopes: "public #{HANDOFF_TOKEN_SCOPE}", expires_in: TOKEN_LIFETIME.to_i ) Rails.logger.info("Issued user access token: issuer application uid=#{doorkeeper_token.application.uid} user_id=#{user.id}") diff --git a/app/controllers/api/v8/base_controller.rb b/app/controllers/api/v8/base_controller.rb index eaa6ac34c..ac5f95f62 100644 --- a/app/controllers/api/v8/base_controller.rb +++ b/app/controllers/api/v8/base_controller.rb @@ -6,6 +6,9 @@ module Api module V8 class BaseController < ApplicationController + # Marks tokens issued by Application::UserAccessTokensController. + HANDOFF_TOKEN_SCOPE = 'handoff' + clear_respond_to respond_to :json @@ -45,6 +48,7 @@ def authenticate_user! if doorkeeper_token @current_user = User.find_by(id: doorkeeper_token.resource_owner_id) if doorkeeper_token.accessible? return respond_unauthorized('Invalid token') unless @current_user + @auth_source = :handoff_token if doorkeeper_token.includes_scope?(HANDOFF_TOKEN_SCOPE) elsif Rails.configuration.x.accept_courses_mooc_fi_tokens @current_user = CoursesMoocFiAuthentication.user_for(request) @auth_source = :courses_mooc_fi_token if @current_user diff --git a/app/controllers/api/v8/users_controller.rb b/app/controllers/api/v8/users_controller.rb index 16d110f7d..b43219dd8 100644 --- a/app/controllers/api/v8/users_controller.rb +++ b/app/controllers/api/v8/users_controller.rb @@ -189,6 +189,7 @@ def update @user = User.find_by!(id: params[:id]) unless params[:id] == 'current' @email_before = @user.email authorize! :update, @user + authorize! :update_credentials, @user if credentials_change_requested? set_user_fields set_extra_data(true) update_email @@ -350,6 +351,12 @@ def maybe_update_password end end + def credentials_change_requested? + new_email = params.dig(:user, :email) + email_changed = new_email.present? && @user.email.casecmp(new_email.to_s.strip) != 0 + email_changed || (params[:old_password].present? && params[:password].present?) + end + def set_user_fields return if params[:user_field].nil? changes = {} diff --git a/app/models/ability.rb b/app/models/ability.rb index 812fc015b..01470c70f 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -6,9 +6,13 @@ class Ability include CanCan::Ability - # +auth_source+ is :courses_mooc_fi_token when the user authenticated with a courses.mooc.fi - # access token; account changes are then denied to everyone, administrators included. + # +auth_source+ restricts account changes for tokens that are not the user's own login: + # :courses_mooc_fi_token denies them all, administrators included; :handoff_token (issued by + # Api::V8::Application::UserAccessTokensController) allows profile edits but no email or password + # change (+:update_credentials+) and no account deletion. def initialize(user, auth_source: nil) + alias_action :update_credentials, to: :update + if user.administrator? can :manage, :all can :create, Course @@ -291,10 +295,14 @@ 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 :update_credentials, User + cannot :destroy, User + end 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 index 9a58ca163..56ac70b4e 100644 --- a/spec/controllers/api/v8/application/user_access_tokens_controller_spec.rb +++ b/spec/controllers/api/v8/application/user_access_tokens_controller_spec.rb @@ -28,7 +28,7 @@ def post_with(token, user_id) post :create, params: { user_id: user_id }, format: :json end - it 'issues a public one-day token for the user owned by the target application' do + 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) @@ -37,7 +37,7 @@ def post_with(token, user_id) 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(['public']) + 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) diff --git a/spec/controllers/api/v8/base_controller_spec.rb b/spec/controllers/api/v8/base_controller_spec.rb index 8fb4b80b7..fe0b448bc 100644 --- a/spec/controllers/api/v8/base_controller_spec.rb +++ b/spec/controllers/api/v8/base_controller_spec.rb @@ -28,7 +28,7 @@ def index context 'when logged in' do let(:user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: user.id, accessible?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } it { expect(subject.id).to be user.id } end end @@ -38,7 +38,7 @@ 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, accessible?: true } + let(:token) { double resource_owner_id: -1, accessible?: true, includes_scope?: false } it 'request is not successful' do get :index expect(response).to have_http_status(401) 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 04dc78abf..09b28b1b9 100644 --- a/spec/controllers/api/v8/core/courses/reviews_controller_spec.rb +++ b/spec/controllers/api/v8/core/courses/reviews_controller_spec.rb @@ -18,7 +18,7 @@ describe "GET reviews for a course for current user's submissions" do describe 'as admin' do let(:user) { FactoryBot.create(:admin) } - let(:token) { double resource_owner_id: user.id, accessible?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } describe 'when course id given' do it "shows only user's submission's review information" do @@ -46,7 +46,7 @@ describe 'as user' do let(:user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: user.id, accessible?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } describe 'when course id given' do it "shows only user's submission's review information" do @@ -100,7 +100,7 @@ describe 'Update review info' do describe 'As an admin' do let(:user) { FactoryBot.create(:admin) } - let(:token) { double resource_owner_id: user.id, accessible?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } before :each do controller.current_user = user end @@ -144,7 +144,7 @@ describe 'As an user' do describe 'when trying to edit review' do let(:user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: user.id, accessible?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } it 'should deny access' do FactoryBot.create(:review, submission: submission) exercise.reload diff --git a/spec/controllers/api/v8/core/courses/unlocks_controller_spec.rb b/spec/controllers/api/v8/core/courses/unlocks_controller_spec.rb index 4e5390e6b..ceef485e2 100644 --- a/spec/controllers/api/v8/core/courses/unlocks_controller_spec.rb +++ b/spec/controllers/api/v8/core/courses/unlocks_controller_spec.rb @@ -14,7 +14,7 @@ describe 'POST unlock exercises' do describe 'as an authenticated user' do - let!(:token) { double resource_owner_id: user.id, accessible?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } it 'should allow unlocking exercises' do post :create, params: { organization_slug: organization.id, course_id: course.id } diff --git a/spec/controllers/api/v8/core/courses_controller_spec.rb b/spec/controllers/api/v8/core/courses_controller_spec.rb index 78492051f..618e1fc0c 100644 --- a/spec/controllers/api/v8/core/courses_controller_spec.rb +++ b/spec/controllers/api/v8/core/courses_controller_spec.rb @@ -11,7 +11,7 @@ describe 'GET course details show' do describe 'in JSON format with valid token' do - let(:token) { double resource_owner_id: user.id, accessible?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } let(:course) { FactoryBot.create(:course, name: 'Course1') } before :each do course.exercises << FactoryBot.create(:returnable_exercise, name: 'Exercise1', course: course) diff --git a/spec/controllers/api/v8/core/exercises/solutions_controller_spec.rb b/spec/controllers/api/v8/core/exercises/solutions_controller_spec.rb index 22bc972ba..67a49c8e7 100644 --- a/spec/controllers/api/v8/core/exercises/solutions_controller_spec.rb +++ b/spec/controllers/api/v8/core/exercises/solutions_controller_spec.rb @@ -20,7 +20,7 @@ describe 'Downloading an exercise solution zip' do describe 'as an admin' do - let(:token) { double resource_owner_id: admin.id, accessible?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } it 'should succeed if the exercise exists' do repo = clone_course_repo(course) @@ -34,7 +34,7 @@ end describe 'as a student' do - let(:token) { double resource_owner_id: user.id, accessible?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } describe 'if the course exists' do it 'should succeed if I have already solved it' do diff --git a/spec/controllers/api/v8/core/exercises/submissions_controller_spec.rb b/spec/controllers/api/v8/core/exercises/submissions_controller_spec.rb index bb844672e..498176469 100644 --- a/spec/controllers/api/v8/core/exercises/submissions_controller_spec.rb +++ b/spec/controllers/api/v8/core/exercises/submissions_controller_spec.rb @@ -29,7 +29,7 @@ def create_submission(file) describe 'Creating a submission' do describe 'as an authenticated user' do - let(:token) { double resource_owner_id: user.id, accessible?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } it 'should accept submissions when the deadline is open' do exercise.deadline_spec = ['1.1.2100'].to_json diff --git a/spec/controllers/api/v8/core/exercises_controller_spec.rb b/spec/controllers/api/v8/core/exercises_controller_spec.rb index 0d3b7b859..0809b3e6d 100644 --- a/spec/controllers/api/v8/core/exercises_controller_spec.rb +++ b/spec/controllers/api/v8/core/exercises_controller_spec.rb @@ -45,7 +45,7 @@ end describe 'When logged in' do - let(:token) { double resource_owner_id: user.id, accessible?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } describe 'and correct exercise id is given' do it 'should return correct data' do submission = FactoryBot.create(:submission, course: course, user: user, exercise: exercise) diff --git a/spec/controllers/api/v8/core/organizations/courses_controller_spec.rb b/spec/controllers/api/v8/core/organizations/courses_controller_spec.rb index b18fc5bd1..e7a1d4690 100644 --- a/spec/controllers/api/v8/core/organizations/courses_controller_spec.rb +++ b/spec/controllers/api/v8/core/organizations/courses_controller_spec.rb @@ -16,7 +16,7 @@ describe 'GET courses' do describe 'as admin' do - let(:token) { double resource_owner_id: admin.id, accessible?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } it 'shows a list of collections of course urls' do get :index, params: { organization_slug: organization.slug } json = JSON.parse response.body @@ -31,7 +31,7 @@ end end describe 'as user' do - let(:token) { double resource_owner_id: user.id, accessible?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } it 'shows a list of collections of course urls' do get :index, params: { organization_slug: organization.slug } json = JSON.parse response.body diff --git a/spec/controllers/api/v8/core/submissions/feedback_controller_spec.rb b/spec/controllers/api/v8/core/submissions/feedback_controller_spec.rb index c81f5dc52..e8148c324 100644 --- a/spec/controllers/api/v8/core/submissions/feedback_controller_spec.rb +++ b/spec/controllers/api/v8/core/submissions/feedback_controller_spec.rb @@ -16,7 +16,7 @@ describe 'POST create' do describe 'As a student' do - let(:token) { double resource_owner_id: student.id, accessible?: true } + let(:token) { double resource_owner_id: student.id, accessible?: true, includes_scope?: false } it 'can give feedback' do # pending 'Failing for some reason' diff --git a/spec/controllers/api/v8/core/submissions/reviews_controller_spec.rb b/spec/controllers/api/v8/core/submissions/reviews_controller_spec.rb index 1d9e8a004..84a117ab5 100644 --- a/spec/controllers/api/v8/core/submissions/reviews_controller_spec.rb +++ b/spec/controllers/api/v8/core/submissions/reviews_controller_spec.rb @@ -18,7 +18,7 @@ describe 'POST create' do describe 'As an admin' do - let(:token) { double resource_owner_id: admin.id, accessible?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } it 'can make new review' do expect do @@ -29,7 +29,7 @@ end describe 'As a teacher' do - let(:token) { double resource_owner_id: teacher.id, accessible?: true } + let(:token) { double resource_owner_id: teacher.id, accessible?: true, includes_scope?: false } it 'can make new review' do expect do @@ -40,7 +40,7 @@ end describe 'As a student' do - let(:token) { double resource_owner_id: student.id, accessible?: true } + let(:token) { double resource_owner_id: student.id, accessible?: true, includes_scope?: false } it "can't make new review" do expect do diff --git a/spec/controllers/api/v8/core/submissions_controller_spec.rb b/spec/controllers/api/v8/core/submissions_controller_spec.rb index 9d9ebdc97..355cf76fc 100644 --- a/spec/controllers/api/v8/core/submissions_controller_spec.rb +++ b/spec/controllers/api/v8/core/submissions_controller_spec.rb @@ -27,7 +27,7 @@ describe 'Downloading a submission as zip' do describe 'as an admin' do - let(:token) { double resource_owner_id: admin.id, accessible?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } it "should allow to download everyone's submissions" do # pending("test that submission zip's content is correct") @@ -40,7 +40,7 @@ before :each do Teachership.create(user: teacher, organization: organization) end - let(:token) { double resource_owner_id: teacher.id, accessible?: true } + let(:token) { double resource_owner_id: teacher.id, accessible?: true, includes_scope?: false } it "should allow to download own organization's submissions" do # pending("test that submission zip's content is correct") @@ -63,7 +63,7 @@ before :each do Assistantship.create(user: assistant, course: course) end - let(:token) { double resource_owner_id: assistant.id, accessible?: true } + let(:token) { double resource_owner_id: assistant.id, accessible?: true, includes_scope?: false } it "should allow to download own course's submissions" do # pending("test that submission zip's content is correct") @@ -82,7 +82,7 @@ end end describe 'as a student' do - let(:token) { double resource_owner_id: user.id, accessible?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } it 'should allow to download own submissions' do # pending("test that submission zip's content is correct") diff --git a/spec/controllers/api/v8/courses/all_courses_with_this_template_spec.rb b/spec/controllers/api/v8/courses/all_courses_with_this_template_spec.rb index 2428e507e..8f8c284db 100644 --- a/spec/controllers/api/v8/courses/all_courses_with_this_template_spec.rb +++ b/spec/controllers/api/v8/courses/all_courses_with_this_template_spec.rb @@ -34,7 +34,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: admin.id, accessible?: true } + let!(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } it 'should show all courses with same template' do two_courses_by_template_id @@ -62,7 +62,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: teacher.id, accessible?: true } + let!(:token) { double resource_owner_id: teacher.id, accessible?: true, includes_scope?: false } it 'should show all courses with same template' do two_courses_by_template_id @@ -90,7 +90,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: assistant.id, accessible?: true } + let!(:token) { double resource_owner_id: assistant.id, accessible?: true, includes_scope?: false } it 'should show all courses with same template' do two_courses_by_template_id @@ -118,7 +118,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: user.id, accessible?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } it 'should show all courses with same template' do two_courses_by_template_id diff --git a/spec/controllers/api/v8/courses/exercises/points_controller_spec.rb b/spec/controllers/api/v8/courses/exercises/points_controller_spec.rb index 57190cd0d..fca26eb5c 100644 --- a/spec/controllers/api/v8/courses/exercises/points_controller_spec.rb +++ b/spec/controllers/api/v8/courses/exercises/points_controller_spec.rb @@ -20,7 +20,7 @@ end describe 'As an admin' do - let(:token) { double resource_owner_id: admin.id, accessible?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } describe 'when searching for all users awarded points' do describe 'using course name' do it 'should return all users awarded points of the exercise' do @@ -46,7 +46,7 @@ end describe 'As any user' do - let(:token) { double resource_owner_id: admin.id, accessible?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } describe 'when searching awarded points' do describe 'and no points are found' do it 'should return an empty array' do diff --git a/spec/controllers/api/v8/courses/exercises/users/points_controller_spec.rb b/spec/controllers/api/v8/courses/exercises/users/points_controller_spec.rb index 3478189b1..3c4365f60 100644 --- a/spec/controllers/api/v8/courses/exercises/users/points_controller_spec.rb +++ b/spec/controllers/api/v8/courses/exercises/users/points_controller_spec.rb @@ -30,7 +30,7 @@ end describe 'As any user' do - let(:token) { double resource_owner_id: admin.id, accessible?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } describe 'when searching for users awarded points by user id' do describe 'and using course id' do it 'should return only correct users awarded points' do diff --git a/spec/controllers/api/v8/courses/exercises_controller_spec.rb b/spec/controllers/api/v8/courses/exercises_controller_spec.rb index 6a4d6f0bf..5023667b8 100644 --- a/spec/controllers/api/v8/courses/exercises_controller_spec.rb +++ b/spec/controllers/api/v8/courses/exercises_controller_spec.rb @@ -15,7 +15,7 @@ end describe 'As an admin' do - let(:token) { double resource_owner_id: admin.id, accessible?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } describe 'when course id is given' do it 'should return successful response' do get :index, params: { course_id: course.id } @@ -33,7 +33,7 @@ end describe 'As a student' do - let(:token) { double resource_owner_id: user.id, accessible?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } describe 'when course id is given' do it 'should return successful response' do get :index, params: { course_id: course.id } @@ -51,7 +51,7 @@ end describe 'As a student without unlock rows' do - let(:token) { double resource_owner_id: user.id, accessible?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } def unlocked_flags get :index, params: { course_id: course.id } @@ -81,7 +81,7 @@ def unlocked_flags end describe 'As any user' do - let(:token) { double resource_owner_id: admin.id, accessible?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } describe 'when course id could not be found' do it 'should return error' do get :index, params: { course_id: '123' } diff --git a/spec/controllers/api/v8/courses/points_controller_spec.rb b/spec/controllers/api/v8/courses/points_controller_spec.rb index c30f4e538..ed0f1d8f4 100644 --- a/spec/controllers/api/v8/courses/points_controller_spec.rb +++ b/spec/controllers/api/v8/courses/points_controller_spec.rb @@ -20,7 +20,7 @@ describe 'GET points for a course by id' do describe 'as admin' do let(:current_user) { FactoryBot.create(:admin) } - let(:token) { double resource_owner_id: current_user.id, accessible?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when course ID given' do it "shows only user's point information" do @@ -47,7 +47,7 @@ describe 'as user' do let(:current_user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: current_user.id, accessible?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when course ID given' do it "shows only user's point information" do diff --git a/spec/controllers/api/v8/courses/submissions/last_hour_controller_spec.rb b/spec/controllers/api/v8/courses/submissions/last_hour_controller_spec.rb index a3d882f77..d3c72899d 100644 --- a/spec/controllers/api/v8/courses/submissions/last_hour_controller_spec.rb +++ b/spec/controllers/api/v8/courses/submissions/last_hour_controller_spec.rb @@ -18,7 +18,7 @@ describe 'index' do context 'as a regular user' do - let!(:token) { double resource_owner_id: regular_user.id, accessible?: true } + let!(:token) { double resource_owner_id: regular_user.id, accessible?: true, includes_scope?: false } it 'should not allow access' do get :index, params: { course_id: course.id } @@ -27,7 +27,7 @@ end context 'as an admin' do - let!(:token) { double resource_owner_id: admin_user.id, accessible?: true } + let!(:token) { double resource_owner_id: admin_user.id, accessible?: true, includes_scope?: false } it 'gives submission ids for the last hour' do get :index, params: { course_id: course.id } diff --git a/spec/controllers/api/v8/courses/submissions_controller_spec.rb b/spec/controllers/api/v8/courses/submissions_controller_spec.rb index 202e2134a..91f5773e0 100644 --- a/spec/controllers/api/v8/courses/submissions_controller_spec.rb +++ b/spec/controllers/api/v8/courses/submissions_controller_spec.rb @@ -37,7 +37,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: admin.id, accessible?: true } + let!(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } it 'should show all of the submissions' do two_subs_by_id @@ -61,7 +61,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: teacher.id, accessible?: true } + let!(:token) { double resource_owner_id: teacher.id, accessible?: true, includes_scope?: false } it 'should show all of the submissions in my organizations' do two_subs_by_id @@ -89,7 +89,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: assistant.id, accessible?: true } + let!(:token) { double resource_owner_id: assistant.id, accessible?: true, includes_scope?: false } it 'should show all of the submissions in my courses' do two_subs_by_id @@ -117,7 +117,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: user.id, accessible?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } it 'should show my own submissions' do all_own_subs(course_id: course.id) diff --git a/spec/controllers/api/v8/courses/users/points_controller_spec.rb b/spec/controllers/api/v8/courses/users/points_controller_spec.rb index cabcdeac0..db70785be 100644 --- a/spec/controllers/api/v8/courses/users/points_controller_spec.rb +++ b/spec/controllers/api/v8/courses/users/points_controller_spec.rb @@ -21,7 +21,7 @@ describe "GET current user's points" do describe 'as admin' do let(:current_user) { FactoryBot.create(:admin) } - let(:token) { double resource_owner_id: current_user.id, accessible?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when course ID given' do it "shows only user's point information" do @@ -48,7 +48,7 @@ describe 'as user' do let(:current_user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: current_user.id, accessible?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when course ID given' do it "shows only user's point information" do @@ -99,7 +99,7 @@ describe "GET user's points" do describe 'as admin' do let(:current_user) { FactoryBot.create(:admin) } - let(:token) { double resource_owner_id: current_user.id, accessible?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when course ID given' do it "shows only user's point information" do @@ -126,7 +126,7 @@ describe 'as user' do let(:current_user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: current_user.id, accessible?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when course ID given' do it "shows only user's point information" do @@ -153,7 +153,7 @@ describe 'as another user' do let(:current_user) { user2 } - let(:token) { double resource_owner_id: current_user.id, accessible?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when course ID given' do it "shows only user's point information" do diff --git a/spec/controllers/api/v8/courses/users/submissions_controller_spec.rb b/spec/controllers/api/v8/courses/users/submissions_controller_spec.rb index 05d6419de..4fc9f0352 100644 --- a/spec/controllers/api/v8/courses/users/submissions_controller_spec.rb +++ b/spec/controllers/api/v8/courses/users/submissions_controller_spec.rb @@ -38,7 +38,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: admin.id, accessible?: true } + let!(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } it "should show given user's submissions" do users_own_subs(course_id: course.id, user_id: user.id) @@ -62,7 +62,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: teacher.id, accessible?: true } + let!(:token) { double resource_owner_id: teacher.id, accessible?: true, includes_scope?: false } it "should show given user's submissions in my organizations" do users_own_subs(course_id: course.id, user_id: user.id) @@ -90,7 +90,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: assistant.id, accessible?: true } + let!(:token) { double resource_owner_id: assistant.id, accessible?: true, includes_scope?: false } it "should show given user's submissions in my courses" do users_own_subs(course_id: course.id, user_id: user.id) @@ -118,7 +118,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: user.id, accessible?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } it 'should show my own submissions' do users_own_subs(course_id: course.id, user_id: user.id) @@ -163,7 +163,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: user.id, accessible?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } it 'should show my own submissions' do users_own_subs(course_id: course.id, user_id: 'current') diff --git a/spec/controllers/api/v8/courses_controller_spec.rb b/spec/controllers/api/v8/courses_controller_spec.rb index 1d20e6fb7..8ad6d32af 100644 --- a/spec/controllers/api/v8/courses_controller_spec.rb +++ b/spec/controllers/api/v8/courses_controller_spec.rb @@ -15,7 +15,7 @@ describe 'GET course by id' do describe 'as admin' do let(:current_user) { admin } - let(:token) { double resource_owner_id: current_user.id, accessible?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when course ID given' do it 'shows course information' do @@ -45,7 +45,7 @@ describe 'as user' do let(:current_user) { user } - let(:token) { double resource_owner_id: current_user.id, accessible?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when course ID given' do it 'shows course information' do diff --git a/spec/controllers/api/v8/courses_mooc_fi_token_auth_spec.rb b/spec/controllers/api/v8/courses_mooc_fi_token_auth_spec.rb index 11442729d..9a893ccb0 100644 --- a/spec/controllers/api/v8/courses_mooc_fi_token_auth_spec.rb +++ b/spec/controllers/api/v8/courses_mooc_fi_token_auth_spec.rb @@ -52,7 +52,7 @@ def introspection_returns(value) it 'prefers a native Doorkeeper token' do user = FactoryBot.create(:user) - allow(controller).to receive(:doorkeeper_token).and_return(double(resource_owner_id: user.id, accessible?: true)) + allow(controller).to receive(:doorkeeper_token).and_return(double(resource_owner_id: user.id, accessible?: true, includes_scope?: false)) expect(CoursesMoocFiTokenIntrospector).not_to receive(:introspect) get :index expect(current_user).to eq(user) diff --git a/spec/controllers/api/v8/organizations/courses/exercises/points_controller_spec.rb b/spec/controllers/api/v8/organizations/courses/exercises/points_controller_spec.rb index 90cb5a8c9..7c1b8fd64 100644 --- a/spec/controllers/api/v8/organizations/courses/exercises/points_controller_spec.rb +++ b/spec/controllers/api/v8/organizations/courses/exercises/points_controller_spec.rb @@ -22,7 +22,7 @@ end describe 'As an admin' do - let(:token) { double resource_owner_id: admin.id, accessible?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } describe 'when searching for all users awarded points' do describe 'using course name' do it 'should return all users awarded points of the exercise' do @@ -48,7 +48,7 @@ end describe 'As any user' do - let(:token) { double resource_owner_id: admin.id, accessible?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } describe 'when searching awarded points' do describe 'and no points are found' do it 'should return an empty array' do diff --git a/spec/controllers/api/v8/organizations/courses/exercises/users/points_controller_spec.rb b/spec/controllers/api/v8/organizations/courses/exercises/users/points_controller_spec.rb index e5207c9cb..04ed394d2 100644 --- a/spec/controllers/api/v8/organizations/courses/exercises/users/points_controller_spec.rb +++ b/spec/controllers/api/v8/organizations/courses/exercises/users/points_controller_spec.rb @@ -31,7 +31,7 @@ end describe 'As any user' do - let(:token) { double resource_owner_id: admin.id, accessible?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } describe 'when searching for users awarded points by user id' do describe 'and using course id' do it 'should return only correct users awarded points' do diff --git a/spec/controllers/api/v8/organizations/courses/exercises_controller_spec.rb b/spec/controllers/api/v8/organizations/courses/exercises_controller_spec.rb index 97dddd7a2..2229d891f 100644 --- a/spec/controllers/api/v8/organizations/courses/exercises_controller_spec.rb +++ b/spec/controllers/api/v8/organizations/courses/exercises_controller_spec.rb @@ -20,7 +20,7 @@ end describe 'As an admin' do - let(:token) { double resource_owner_id: admin.id, accessible?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } describe 'when course name is given' do it 'should return successful response' do get :index, params: { course_name: course_name, organization_slug: organization.slug } @@ -38,7 +38,7 @@ end describe 'As a student' do - let(:token) { double resource_owner_id: user.id, accessible?: true } + let(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } describe 'when course name is given' do it 'should return successful response' do get :index, params: { course_name: course_name, organization_slug: organization.slug } @@ -56,7 +56,7 @@ end describe 'As any user' do - let(:token) { double resource_owner_id: admin.id, accessible?: true } + let(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } describe 'when course name could not be found' do it 'should return error' do get :index, params: { course_name: 'null', organization_slug: organization.slug } @@ -67,7 +67,7 @@ describe 'Downloading an exercise zip' do describe 'as an unauthenticated user' do - let(:token) { double resource_owner_id: Guest.new.id, accessible?: true } + let(:token) { double resource_owner_id: Guest.new.id, accessible?: true, includes_scope?: false } it 'should succeed if the course name and slug are correct', driver: :rack_test do repo = clone_course_repo(course) diff --git a/spec/controllers/api/v8/organizations/courses/points_controller_spec.rb b/spec/controllers/api/v8/organizations/courses/points_controller_spec.rb index ccc122bbc..0437adfa9 100644 --- a/spec/controllers/api/v8/organizations/courses/points_controller_spec.rb +++ b/spec/controllers/api/v8/organizations/courses/points_controller_spec.rb @@ -21,7 +21,7 @@ describe 'GET points for a course' do describe 'as admin' do let(:current_user) { FactoryBot.create(:admin) } - let(:token) { double resource_owner_id: current_user.id, accessible?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when course name given' do it "shows only user's point information" do @@ -48,7 +48,7 @@ describe 'as user' do let(:current_user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: current_user.id, accessible?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when course name given' do it "shows only user's point information" do diff --git a/spec/controllers/api/v8/organizations/courses/submissions_controller_spec.rb b/spec/controllers/api/v8/organizations/courses/submissions_controller_spec.rb index 070098715..6ff237e0d 100644 --- a/spec/controllers/api/v8/organizations/courses/submissions_controller_spec.rb +++ b/spec/controllers/api/v8/organizations/courses/submissions_controller_spec.rb @@ -39,7 +39,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: admin.id, accessible?: true } + let!(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } it 'should show all of the submissions' do two_subs_by_name @@ -63,7 +63,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: teacher.id, accessible?: true } + let!(:token) { double resource_owner_id: teacher.id, accessible?: true, includes_scope?: false } it 'should show all of the submissions in my organizations' do two_subs_by_name @@ -91,7 +91,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: assistant.id, accessible?: true } + let!(:token) { double resource_owner_id: assistant.id, accessible?: true, includes_scope?: false } it 'should show all of the submissions in my courses' do two_subs_by_name @@ -119,7 +119,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: user.id, accessible?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } it 'should show my own submissions' do all_own_subs(organization_slug: organization.slug, course_name: course_name) diff --git a/spec/controllers/api/v8/organizations/courses/users/points_controller_spec.rb b/spec/controllers/api/v8/organizations/courses/users/points_controller_spec.rb index ea0ed45b1..e4ec94ee1 100644 --- a/spec/controllers/api/v8/organizations/courses/users/points_controller_spec.rb +++ b/spec/controllers/api/v8/organizations/courses/users/points_controller_spec.rb @@ -22,7 +22,7 @@ describe "GET current user's points" do describe 'as admin' do let(:current_user) { FactoryBot.create(:admin) } - let(:token) { double resource_owner_id: current_user.id, accessible?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when course name given' do it "shows only user's point information" do @@ -49,7 +49,7 @@ describe 'as user' do let(:current_user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: current_user.id, accessible?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when course name given' do it "shows only user's point information" do @@ -100,7 +100,7 @@ describe "GET user's points" do describe 'as admin' do let(:current_user) { FactoryBot.create(:admin) } - let(:token) { double resource_owner_id: current_user.id, accessible?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when course name given' do it "shows only user's point information" do @@ -127,7 +127,7 @@ describe 'as user' do let(:current_user) { FactoryBot.create(:user) } - let(:token) { double resource_owner_id: current_user.id, accessible?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when course name given' do it "shows only user's point information" do @@ -154,7 +154,7 @@ describe 'as another user' do let(:current_user) { user2 } - let(:token) { double resource_owner_id: current_user.id, accessible?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when course name given' do it "shows only user's point information" do diff --git a/spec/controllers/api/v8/organizations/courses/users/submissions_controller_spec.rb b/spec/controllers/api/v8/organizations/courses/users/submissions_controller_spec.rb index 207d143ac..9fda51ddf 100644 --- a/spec/controllers/api/v8/organizations/courses/users/submissions_controller_spec.rb +++ b/spec/controllers/api/v8/organizations/courses/users/submissions_controller_spec.rb @@ -38,7 +38,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: admin.id, accessible?: true } + let!(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } it "should show given user's submissions" do users_own_subs(organization_slug: organization.slug, course_name: course_name, user_id: user.id) @@ -62,7 +62,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: teacher.id, accessible?: true } + let!(:token) { double resource_owner_id: teacher.id, accessible?: true, includes_scope?: false } it "should show given user's submissions in my organizations" do users_own_subs(organization_slug: organization.slug, course_name: course_name, user_id: user.id) @@ -90,7 +90,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: assistant.id, accessible?: true } + let!(:token) { double resource_owner_id: assistant.id, accessible?: true, includes_scope?: false } it "should show given user's submissions in my courses" do users_own_subs(organization_slug: organization.slug, course_name: course_name, user_id: user.id) @@ -118,7 +118,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: user.id, accessible?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } it 'should show my own submissions' do users_own_subs(organization_slug: organization.slug, course_name: course_name, user_id: user.id) @@ -163,7 +163,7 @@ end describe 'when using access token' do - let!(:token) { double resource_owner_id: user.id, accessible?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } it 'should show my own submissions' do users_own_subs(organization_slug: organization.slug, course_name: course_name, user_id: 'current') diff --git a/spec/controllers/api/v8/organizations/courses_controller_spec.rb b/spec/controllers/api/v8/organizations/courses_controller_spec.rb index 00e3f9dee..32571d48e 100644 --- a/spec/controllers/api/v8/organizations/courses_controller_spec.rb +++ b/spec/controllers/api/v8/organizations/courses_controller_spec.rb @@ -17,7 +17,7 @@ describe 'GET course by name' do describe 'when logged as admin' do let(:current_user) { admin } - let(:token) { double resource_owner_id: current_user.id, accessible?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when organization id and course name given' do it 'shows course information' do @@ -60,7 +60,7 @@ describe 'when logged as user' do let(:current_user) { user } - let(:token) { double resource_owner_id: current_user.id, accessible?: true } + let(:token) { double resource_owner_id: current_user.id, accessible?: true, includes_scope?: false } describe 'when organization id and course name given' do it 'shows course information' do diff --git a/spec/controllers/api/v8/users_controller_spec.rb b/spec/controllers/api/v8/users_controller_spec.rb index 98aa0ab07..d83c97832 100644 --- a/spec/controllers/api/v8/users_controller_spec.rb +++ b/spec/controllers/api/v8/users_controller_spec.rb @@ -12,7 +12,7 @@ end describe "GET current user's info with an access token" do - let!(:token) { double resource_owner_id: user.id, accessible?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } it 'user sees own info' do get :show, params: { id: 'current' } @@ -30,7 +30,7 @@ end context 'as an administrator' do - let!(:token) { double resource_owner_id: admin.id, accessible?: true } + let!(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } it 'user sees own info' do get :show, params: { id: 'current' } @@ -40,7 +40,7 @@ end describe "GET user's info with user id" do describe 'using user token' do - let!(:token) { double resource_owner_id: user.id, accessible?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } it 'user sees own info' do get :show, params: { id: user.id } @@ -57,7 +57,7 @@ end end describe 'using admin token' do - let!(:token) { double resource_owner_id: admin.id, accessible?: true } + let!(:token) { double resource_owner_id: admin.id, accessible?: true, includes_scope?: false } it "admin can see everyone's infos" do get :show, params: { id: user.id } @@ -123,7 +123,7 @@ end describe 'PUT update password' do - let!(:token) { double resource_owner_id: user.id, accessible?: true } + let!(:token) { double resource_owner_id: user.id, accessible?: true, includes_scope?: false } before :each do user.password = 'oldpassword' @@ -334,4 +334,63 @@ def authenticate_with_token_as(token_user) expect(user.reload.password_managed_by_courses_mooc_fi).to eq(false) end end + + describe 'with a handoff token' do + let(:application) { Doorkeeper::Application.create!(name: 'mooc.fi frontend', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob') } + let(:token) { Doorkeeper::AccessToken.create!(application: application, resource_owner_id: user.id, scopes: 'public handoff') } + + before :each do + allow(ExtraField).to receive(:by_kind).with(:user).and_return([UserField.new(name: 'first_name'), UserField.new(name: 'last_name')]) + user.password = 'oldpassword' + user.save! + end + + it 'updates the name when the rest of the profile is sent back unchanged' do + put :update, params: { id: user.id, user: { email: user.email.upcase }, user_field: { first_name: 'Ada', last_name: 'Lovelace' } } + expect(response).to have_http_status(200) + expect(UserFieldValue.where(user: user).pluck(:field_name, :value).to_h).to eq('first_name' => 'Ada', 'last_name' => 'Lovelace') + end + + it 'refuses to change the email' do + put :update, params: { id: 'current', user: { email: 'taken-over@example.com' }, user_field: { first_name: 'Ada' } } + expect(response).to have_http_status(403) + expect(user.reload.email).not_to eq('taken-over@example.com') + expect(UserFieldValue.where(user: user, field_name: 'first_name')).to be_empty + end + + it 'refuses to change the password' do + put :update, params: { id: 'current', user: { email: user.email }, old_password: 'oldpassword', password: 'newpassword', password_repeat: 'newpassword' } + expect(response).to have_http_status(403) + expect(user.reload).to have_password('oldpassword') + end + + it 'refuses to delete the user' do + delete :destroy, params: { id: user.id } + expect(response).to have_http_status(403) + expect(User.find_by(id: user.id)).not_to be_nil + end + + context 'when the token is an ordinary login token' do + let(:token) { Doorkeeper::AccessToken.create!(application: application, resource_owner_id: user.id, scopes: 'public') } + + it 'changes the email' do + put :update, params: { id: 'current', user: { email: 'new@example.com' } } + expect(response).to have_http_status(200) + expect(user.reload.email).to eq('new@example.com') + end + + it 'changes the password' do + allow_any_instance_of(User).to receive(:post_new_user_to_courses_mooc_fi).and_return(true) + put :update, params: { id: 'current', user: { email: user.email }, old_password: 'oldpassword', password: 'newpassword', password_repeat: 'newpassword' } + expect(response).to have_http_status(200) + expect(user.reload).to have_password('newpassword') + end + + it 'deletes the user' do + delete :destroy, params: { id: user.id } + expect(response).to have_http_status(200) + expect(User.find_by(id: user.id)).to be_nil + end + end + end end From fa4c4e68f24e1d3a464500b135c23fa18d7b4ad5 Mon Sep 17 00:00:00 2001 From: Henrik Nygren Date: Mon, 2 Nov 2026 16:42:41 +0200 Subject: [PATCH 04/18] Filter client secrets and tokens from request logs --- config/application.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/application.rb b/config/application.rb index 58c2f2e66..18f1ad641 100644 --- a/config/application.rb +++ b/config/application.rb @@ -27,7 +27,7 @@ class Application < Rails::Application # config.i18n.default_locale = :de # Configure sensitive parameters which will be filtered from the log file. - config.filter_parameters += %i[password session_id api_password submission_file return_file test_output stdout stderr vm_log] + config.filter_parameters += %i[password session_id api_password submission_file return_file test_output stdout stderr vm_log client_secret access_token refresh_token] + [/\A(token|code)\z/] # Do not swallow errors in after_commit/after_rollback callbacks. # config.eager_load_paths << Rails.root.join('lib') From 1d1283843178aaae96530e0f1f6549c417ff326f Mon Sep 17 00:00:00 2001 From: Henrik Nygren Date: Mon, 2 Nov 2026 16:42:41 +0200 Subject: [PATCH 05/18] Expire issuer tokens, throttle issuing and refuse public credentials --- Gemfile | 2 +- .../user_access_tokens_controller.rb | 19 +-- app/controllers/api/v8/users_controller.rb | 5 +- app/models/ability.rb | 3 +- config/initializers/doorkeeper.rb | 5 + config/initializers/rack_attack.rb | 58 ++++----- .../user_access_tokens_controller_spec.rb | 9 ++ spec/requests/issuer_token_hardening_spec.rb | 119 ++++++++++++++++++ spec/requests/oauth_token_info_spec.rb | 54 ++++++++ 9 files changed, 227 insertions(+), 47 deletions(-) create mode 100644 spec/requests/issuer_token_hardening_spec.rb create mode 100644 spec/requests/oauth_token_info_spec.rb diff --git a/Gemfile b/Gemfile index 483ac3252..416ab1c2d 100644 --- a/Gemfile +++ b/Gemfile @@ -25,7 +25,7 @@ gem 'newrelic_rpm', '~> 9.11' gem 'pdfkit', '~> 0.8.7.3', require: false gem 'pg', '~> 1.5', '>= 1.5.6' gem 'pghero', '~> 3.5' -gem 'rack-attack', '~> 6.7', require: false +gem 'rack-attack', '~> 6.7' gem 'rack-cors', '~> 2.0', '>= 2.0.2' gem 'rack-timeout', '~> 0.7.0' gem 'rack-mini-profiler', '~> 3.3', '>= 3.3.1' diff --git a/app/controllers/api/v8/application/user_access_tokens_controller.rb b/app/controllers/api/v8/application/user_access_tokens_controller.rb index 6ee261d41..a18ea2df0 100644 --- a/app/controllers/api/v8/application/user_access_tokens_controller.rb +++ b/app/controllers/api/v8/application/user_access_tokens_controller.rb @@ -60,10 +60,9 @@ def create return respond_unauthorized unless doorkeeper_token&.accessible? return respond_forbidden unless authorized_issuer? - user_id = Integer(params[:user_id].to_s, 10, exception: false) - return respond_with_error('user_id must be an integer', 400) unless user_id + return respond_with_error('user_id must be an integer', 400) unless params[:user_id].to_s.match?(/\A\d+\z/) - user = User.find_by(id: user_id) + user = User.find_by(id: params[:user_id].to_i) return respond_not_found('User not found') unless user return respond_forbidden('Tokens are not issued for administrators, teachers or assistants') if privileged?(user) @@ -91,15 +90,21 @@ def privileged?(user) user.administrator? || user.teacherships.exists? || user.assistantships.exists? end + # Their credentials are served publicly by the credentials endpoint. + def public_client_names + Array(SiteSetting.value('valid_clients')).filter_map { |client| client['name'] } + end + def authorized_issuer? issuer_uids = Rails.configuration.x.user_access_token_issuer_application_uids - application = doorkeeper_token.application # resource_owner_id is set on user tokens, which never qualify even from an allowed application. + return false unless doorkeeper_token.resource_owner_id.nil? && doorkeeper_token.acceptable?([REQUIRED_SCOPE]) + + application = doorkeeper_token.application # The target is a frontend client, so its credentials must not be usable to issue tokens. - doorkeeper_token.resource_owner_id.nil? && - issuer_uids.include?(application&.uid) && + issuer_uids.include?(application&.uid) && application.uid != Rails.configuration.x.user_access_token_target_application_uid && - doorkeeper_token.acceptable?([REQUIRED_SCOPE]) + public_client_names.exclude?(application.name) end end end diff --git a/app/controllers/api/v8/users_controller.rb b/app/controllers/api/v8/users_controller.rb index b43219dd8..e5a3daf77 100644 --- a/app/controllers/api/v8/users_controller.rb +++ b/app/controllers/api/v8/users_controller.rb @@ -352,8 +352,9 @@ def maybe_update_password end def credentials_change_requested? - new_email = params.dig(:user, :email) - email_changed = new_email.present? && @user.email.casecmp(new_email.to_s.strip) != 0 + user_params = params[:user] + new_email = user_params[:email] if user_params.is_a?(ActionController::Parameters) + email_changed = new_email.present? && @user.email.to_s.casecmp(new_email.to_s.strip) != 0 email_changed || (params[:old_password].present? && params[:password].present?) end diff --git a/app/models/ability.rb b/app/models/ability.rb index 01470c70f..56f5d436f 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -301,8 +301,7 @@ def initialize(user, auth_source: nil) cannot :update, User cannot :destroy, User when :handoff_token - cannot :update_credentials, User - cannot :destroy, User + cannot %i[update_credentials destroy], User end end end diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index d4898f0b9..e493b66b4 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -37,6 +37,11 @@ # access_token_expires_in 2.hours access_token_expires_in nil + # Issuer tokens are re-fetched by their clients; every other token stays non-expiring. + custom_access_token_expires_in do |context| + 1.hour.to_i if context.grant_type == Doorkeeper::OAuth::CLIENT_CREDENTIALS && context.scopes&.exists?('issue_user_access_tokens') + end + # Assign a custom TTL for implicit grants. # custom_access_token_expires_in do |oauth_client| # oauth_client.application.additional_settings.implicit_oauth_expiration diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index 35b6e2874..90cd657b4 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -16,6 +16,8 @@ # RACK Request: https://github.com/rack/rack/blob/main/lib/rack/request.rb class Rack::Attack + ISSUE_TOKEN_PATH = '/api/v8/application/user_access_tokens' + def self.user_identifier_discriminator(access_token) db_token = Doorkeeper::AccessToken.find_by(token: access_token) return access_token unless db_token @@ -25,8 +27,11 @@ def self.user_identifier_discriminator(access_token) # This safelist is needed because courses.mooc.fi backend auths user # rather then the user and his IP. # RACK Request adds the "HTTP_" infront of the header key. + # Exempt from the safelist so the issue endpoint stays throttled for the mooc.fi backend too. safelist('mark courses.mooc.fi backend requests as safe') do |request| - if ENV['RACK_ATTACK_SAFE_API_KEY'] && request.has_header?('HTTP_RATELIMIT_PROTECTION_SAFE_API_KEY') + if request.path.start_with?(ISSUE_TOKEN_PATH) + false + elsif 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 @@ -41,53 +46,36 @@ def self.user_identifier_discriminator(access_token) ] end - # Limit the number of logins to 20 attempts per minute - throttle('login attempts per minute', limit: 20, period: 1.minute) do |req| - req.ip if req.path == '/sessions' || - req.path == '/oauth/token' && - req.post? + throttle('login attempts per minute', limit: 100, period: 1.minute) do |req| + req.ip if %w[/sessions /oauth/token].include?(req.path) && 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.post? + throttle('login attempts per hour', limit: 600, period: 1.hour) do |req| + req.ip if %w[/sessions /oauth/token].include?(req.path) && 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.post? + throttle('login attempts per day', limit: 3000, period: 1.day) do |req| + req.ip if %w[/sessions /oauth/token].include?(req.path) && req.post? end - # Limit the number of account creations to 15 accounts per minute - throttle('account creations per minute', limit: 15, period: 1.minute) do |req| - req.ip if req.path == '/user' || - req.path == 'api/v8/users' && - req.post? + throttle('account creations per minute', limit: 60, period: 1.minute) do |req| + req.ip if %w[/user /api/v8/users].include?(req.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 == 'api/v8/users' && - req.post? + throttle('account creations per day', limit: 3000, period: 1.day) do |req| + req.ip if %w[/user /api/v8/users].include?(req.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$} || + throttle('submissions per ip per 10 minutes', limit: 600, 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.path =~ %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$} && + throttle('submissions per ip per day to api v7', limit: 5000, period: 1.day) do |req| + req.ip if (req.path =~ %r{^/exercises/\d+/submissions$} || + req.path =~ %r{^/org/\d+/exercises/\d+/submissions$}) && req.post? end @@ -101,7 +89,7 @@ def self.user_identifier_discriminator(access_token) end throttle('user access tokens per ip per minute', limit: 120, period: 1.minute) do |req| - req.ip if req.path.start_with?('/api/v8/application/user_access_tokens') && req.post? + req.ip if req.path.start_with?(ISSUE_TOKEN_PATH) && req.post? end # Limit the number of password resets per ip to 10 per hour 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 index 56ac70b4e..6199e84e7 100644 --- a/spec/controllers/api/v8/application/user_access_tokens_controller_spec.rb +++ b/spec/controllers/api/v8/application/user_access_tokens_controller_spec.rb @@ -43,6 +43,15 @@ def post_with(token, user_id) expect(issued.expires_in).to eq(86_400) 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 'requires a token' do post_with(nil, user.id) diff --git a/spec/requests/issuer_token_hardening_spec.rb b/spec/requests/issuer_token_hardening_spec.rb new file mode 100644 index 000000000..3c9119bbd --- /dev/null +++ b/spec/requests/issuer_token_hardening_spec.rb @@ -0,0 +1,119 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Issuer token hardening', type: :request do + let!(:issuer_app) { Doorkeeper::Application.create!(name: 'mooc.fi backend', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob', scopes: 'issue_user_access_tokens') } + + describe 'client_credentials token' do + def request_token + post '/oauth/token', params: { grant_type: 'client_credentials', client_id: issuer_app.uid, client_secret: issuer_app.plaintext_secret, scope: 'issue_user_access_tokens' } + end + + it 'expires in one hour' do + request_token + + expect(response).to have_http_status(200) + expect(JSON.parse(response.body)['expires_in']).to eq(3600) + end + + it 'leaves client_credentials tokens without the issuer scope non-expiring' do + issuer_app.update!(scopes: 'public issue_user_access_tokens') + post '/oauth/token', params: { grant_type: 'client_credentials', client_id: issuer_app.uid, client_secret: issuer_app.plaintext_secret, scope: 'public' } + + expect(response).to have_http_status(200) + expect(JSON.parse(response.body)).not_to have_key('expires_in') + end + + it 'leaves password-grant tokens non-expiring' do + user = FactoryBot.create(:user, password: 'hunter2hunter2') + post '/oauth/token', params: { grant_type: 'password', client_id: issuer_app.uid, client_secret: issuer_app.plaintext_secret, username: user.login, password: 'hunter2hunter2' } + + expect(response).to have_http_status(200) + expect(JSON.parse(response.body)).not_to have_key('expires_in') + end + end + + describe 'parameter filtering' do + let(:filter) { ActiveSupport::ParameterFilter.new(Rails.application.config.filter_parameters) } + + it 'hides credentials and tokens' do + params = { client_secret: 's', access_token: 'a', refresh_token: 'r', token: 't', code: 'c' } + + expect(filter.filter(params).values.uniq).to eq(['[FILTERED]']) + end + + it 'keeps similarly named diagnostics visible' do + params = { exit_code: 1, return_code: 2, token_count: 3 } + + expect(filter.filter(params)).to eq(params) + end + end + + describe 'rate limiting' do + let(:app) { Rack::Attack.new(->(_env) { [200, {}, ['ok']] }) } + let(:safe_headers) { { 'HTTP_RATELIMIT_PROTECTION_SAFE_API_KEY' => 'safe-key' } } + + around :each do |example| + Rack::Attack.enabled = true + Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new + ENV['RACK_ATTACK_SAFE_API_KEY'] = 'safe-key' + example.run + ensure + Rack::Attack.enabled = false + ENV.delete('RACK_ATTACK_SAFE_API_KEY') + end + + def status_after(times, path, headers = {}) + status = nil + times.times { status, = app.call(Rack::MockRequest.env_for(path, { method: 'POST', 'REMOTE_ADDR' => '203.0.113.7' }.merge(headers))) } + status + end + + it 'throttles the issue endpoint even with the safe API key' do + expect(status_after(120, '/api/v8/application/user_access_tokens', safe_headers)).to eq(200) + expect(status_after(1, '/api/v8/application/user_access_tokens', safe_headers)).to eq(503) + end + + it 'does not throttle safelisted oauth token requests' do + expect(status_after(25, '/oauth/token', safe_headers)).to eq(200) + end + + it 'throttles unsafelisted oauth token requests' do + expect(status_after(100, '/oauth/token')).to eq(200) + expect(status_after(1, '/oauth/token')).to eq(503) + end + end + describe 'Rack::Attack in the real stack' do + around :each do |example| + Rack::Attack.enabled = true + Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new + example.run + ensure + Rack::Attack.enabled = false + end + + def hit_token_endpoint(times, env) + times.times { post '/oauth/token', env: env } + end + + it 'is in the middleware stack exactly once' do + expect(Rails.application.middleware.count { |m| m.klass == Rack::Attack }).to eq(1) + end + + it 'throttles by client address' do + hit_token_endpoint(101, 'REMOTE_ADDR' => '203.0.113.7') + + expect(response).to have_http_status(503) + end + + it 'buckets by the forwarded client address behind a private-range proxy' do + proxied = { 'REMOTE_ADDR' => '10.0.0.1' } + hit_token_endpoint(101, proxied.merge('HTTP_X_FORWARDED_FOR' => '203.0.113.9')) + expect(response).to have_http_status(503) + + hit_token_endpoint(1, proxied.merge('HTTP_X_FORWARDED_FOR' => '203.0.113.10')) + expect(response).not_to have_http_status(503) + end + end +end diff --git a/spec/requests/oauth_token_info_spec.rb b/spec/requests/oauth_token_info_spec.rb new file mode 100644 index 000000000..b60663eb3 --- /dev/null +++ b/spec/requests/oauth_token_info_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require 'spec_helper' + +# courses.mooc.fi reads the issuing application of forwarded TMC tokens from this endpoint. +describe 'GET /oauth/token/info', type: :request do + let(:user) { FactoryBot.create(:user) } + let(:application) { Doorkeeper::Application.create!(name: 'mooc.fi frontend', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob') } + + def token_info(bearer) + get '/oauth/token/info', headers: { 'Authorization' => "Bearer #{bearer}" } + end + + it 'reports the issuing application of a handoff token' do + token = Doorkeeper::AccessToken.create!(application: application, resource_owner_id: user.id, scopes: 'public handoff', expires_in: 86_400) + token_info(token.token) + + expect(response).to have_http_status(200) + body = JSON.parse(response.body) + expect(body['application']).to eq('uid' => application.uid) + expect(body['resource_owner_id']).to eq(user.id) + expect(body['scope']).to eq(%w[public handoff]) + end + + it 'refuses a revoked token' do + token = Doorkeeper::AccessToken.create!(application: application, resource_owner_id: user.id, scopes: 'public') + token.revoke + token_info(token.token) + + expect(response).to have_http_status(401) + end + + context 'with a courses.mooc.fi token' do + around :each do |example| + original = Rails.configuration.x.accept_courses_mooc_fi_tokens + Rails.configuration.x.accept_courses_mooc_fi_tokens = true + example.run + ensure + Rails.configuration.x.accept_courses_mooc_fi_tokens = original + end + + it 'reports no application' do + bearer = 'sp331-access-token' + user.update_column(:courses_mooc_fi_user_id, '11111111-2222-3333-4444-555555555555') + result = CoursesMoocFiTokenIntrospector::Result.new(sub: user.courses_mooc_fi_user_id, upstream_id: nil, expires_at: Time.now + 3600) + allow(CoursesMoocFiTokenIntrospector).to receive(:introspect).with(bearer).and_return(result) + + token_info(bearer) + + expect(response).to have_http_status(401) + expect(response.body).not_to include('uid') + end + end +end From 39d229ecab64b2c9322b097bb9f7c97ccb5eda94 Mon Sep 17 00:00:00 2001 From: Henrik Nygren Date: Tue, 3 Nov 2026 14:11:52 +0200 Subject: [PATCH 06/18] Refuse privileged users and non-confidential apps for handoff tokens --- .../user_access_tokens_controller.rb | 7 +--- app/controllers/api/v8/base_controller.rb | 9 +++-- app/models/user.rb | 5 +++ .../user_access_tokens_controller_spec.rb | 7 ++++ .../api/v8/base_controller_spec.rb | 38 +++++++++++++++++++ 5 files changed, 58 insertions(+), 8 deletions(-) diff --git a/app/controllers/api/v8/application/user_access_tokens_controller.rb b/app/controllers/api/v8/application/user_access_tokens_controller.rb index a18ea2df0..c3aeade4e 100644 --- a/app/controllers/api/v8/application/user_access_tokens_controller.rb +++ b/app/controllers/api/v8/application/user_access_tokens_controller.rb @@ -64,7 +64,7 @@ def create user = User.find_by(id: params[:user_id].to_i) return respond_not_found('User not found') unless user - return respond_forbidden('Tokens are not issued for administrators, teachers or assistants') if privileged?(user) + return respond_forbidden('Tokens are not issued for administrators, teachers or assistants') if user.privileged? target_application = Doorkeeper::Application.find_by(uid: Rails.configuration.x.user_access_token_target_application_uid) return respond_with_error('Target application is not configured', 500) unless target_application @@ -86,10 +86,6 @@ def create end private - def privileged?(user) - user.administrator? || user.teacherships.exists? || user.assistantships.exists? - end - # Their credentials are served publicly by the credentials endpoint. def public_client_names Array(SiteSetting.value('valid_clients')).filter_map { |client| client['name'] } @@ -103,6 +99,7 @@ def authorized_issuer? application = doorkeeper_token.application # The target is a frontend client, so its credentials must not be usable to issue tokens. issuer_uids.include?(application&.uid) && + application.confidential? && application.uid != Rails.configuration.x.user_access_token_target_application_uid && public_client_names.exclude?(application.name) end diff --git a/app/controllers/api/v8/base_controller.rb b/app/controllers/api/v8/base_controller.rb index ac5f95f62..3110fb8bd 100644 --- a/app/controllers/api/v8/base_controller.rb +++ b/app/controllers/api/v8/base_controller.rb @@ -46,9 +46,12 @@ def present(hash) def authenticate_user! return @current_user if @current_user if doorkeeper_token - @current_user = User.find_by(id: doorkeeper_token.resource_owner_id) if doorkeeper_token.accessible? - return respond_unauthorized('Invalid token') unless @current_user - @auth_source = :handoff_token if doorkeeper_token.includes_scope?(HANDOFF_TOKEN_SCOPE) + user = User.find_by(id: doorkeeper_token.resource_owner_id) if doorkeeper_token.accessible? + handoff = doorkeeper_token.includes_scope?(HANDOFF_TOKEN_SCOPE) + # The user may have gained privileges after the handoff token was issued. + return respond_unauthorized('Invalid token') if user.nil? || (handoff && user.privileged?) + @current_user = user + @auth_source = :handoff_token if handoff elsif Rails.configuration.x.accept_courses_mooc_fi_tokens @current_user = CoursesMoocFiAuthentication.user_for(request) @auth_source = :courses_mooc_fi_token if @current_user diff --git a/app/models/user.rb b/app/models/user.rb index 781abaa59..27ec0385d 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -480,6 +480,11 @@ def <=>(other) login.downcase <=> other.login.downcase end + # Administrators, teachers and assistants, whose accounts handoff tokens must never act as. + def privileged? + administrator? || teacherships.exists? || assistantships.exists? + end + def teacher?(organization) organizations.include? organization end diff --git a/spec/controllers/api/v8/application/user_access_tokens_controller_spec.rb b/spec/controllers/api/v8/application/user_access_tokens_controller_spec.rb index 6199e84e7..74e88340b 100644 --- a/spec/controllers/api/v8/application/user_access_tokens_controller_spec.rb +++ b/spec/controllers/api/v8/application/user_access_tokens_controller_spec.rb @@ -52,6 +52,13 @@ def 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) diff --git a/spec/controllers/api/v8/base_controller_spec.rb b/spec/controllers/api/v8/base_controller_spec.rb index fe0b448bc..bfc2aad6a 100644 --- a/spec/controllers/api/v8/base_controller_spec.rb +++ b/spec/controllers/api/v8/base_controller_spec.rb @@ -64,5 +64,43 @@ def 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 end From 9241fe210083877b92f4b17408c2d0ab77af16de Mon Sep 17 00:00:00 2001 From: Henrik Nygren Date: Tue, 3 Nov 2026 14:16:34 +0200 Subject: [PATCH 07/18] Throttle normalized paths and check handoff tokens everywhere --- app/controllers/api/beta/base_controller.rb | 4 +- .../user_access_tokens_controller.rb | 12 +- app/controllers/api/v8/base_controller.rb | 12 +- app/controllers/api/v8/users_controller.rb | 10 +- app/controllers/auths_controller.rb | 8 +- app/controllers/users_controller.rb | 6 +- app/helpers/sessions_helper.rb | 2 +- app/models/ability.rb | 5 +- app/models/user.rb | 13 +- app/services/access_token_authentication.rb | 28 +++ app/views/password_reset_keys/show.html.erb | 2 +- config/application.rb | 2 +- config/initializers/doorkeeper.rb | 2 +- config/initializers/rack_attack.rb | 186 ++++++++++++++---- config/initializers/secret_path_filter.rb | 18 ++ config/routes.rb | 6 +- spec/controllers/users_controller_spec.rb | 2 +- spec/requests/handoff_token_consumers_spec.rb | 30 +++ spec/requests/issuer_token_hardening_spec.rb | 2 +- spec/requests/rack_attack_throttles_spec.rb | 148 ++++++++++++++ spec/requests/secret_path_logging_spec.rb | 57 ++++++ 21 files changed, 470 insertions(+), 85 deletions(-) create mode 100644 app/services/access_token_authentication.rb create mode 100644 config/initializers/secret_path_filter.rb create mode 100644 spec/requests/handoff_token_consumers_spec.rb create mode 100644 spec/requests/rack_attack_throttles_spec.rb create mode 100644 spec/requests/secret_path_logging_spec.rb diff --git a/app/controllers/api/beta/base_controller.rb b/app/controllers/api/beta/base_controller.rb index 0d56455eb..55f25567d 100644 --- a/app/controllers/api/beta/base_controller.rb +++ b/app/controllers/api/beta/base_controller.rb @@ -29,9 +29,7 @@ def present(hash) private def authenticate_user! - if doorkeeper_token&.accessible? - @current_user = User.find(doorkeeper_token.resource_owner_id) - end + @current_user = AccessTokenAuthentication.user_for(doorkeeper_token) return if @current_user diff --git a/app/controllers/api/v8/application/user_access_tokens_controller.rb b/app/controllers/api/v8/application/user_access_tokens_controller.rb index c3aeade4e..b6c39aab2 100644 --- a/app/controllers/api/v8/application/user_access_tokens_controller.rb +++ b/app/controllers/api/v8/application/user_access_tokens_controller.rb @@ -3,18 +3,16 @@ module Api module V8 module Application - # Issues a login access token for a user to a trusted backend, e.g. for a sign-in handoff - # verified elsewhere. Only allow-listed client_credentials applications holding the - # issue_user_access_tokens scope may call it. + # Issues a login token for a user to a trusted backend after a sign-in handoff verified + # elsewhere. Callable only by allow-listed client_credentials apps with the issuer scope. class UserAccessTokensController < Api::V8::BaseController include Swagger::Blocks - REQUIRED_SCOPE = 'issue_user_access_tokens' TOKEN_LIFETIME = 1.day swagger_path '/api/v8/application/user_access_tokens' do operation :post do - key :description, "Issues an access token for the given user, owned by the configured target application. Requires a client_credentials token of an allow-listed application with the #{REQUIRED_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 :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'] @@ -72,7 +70,7 @@ def create token = Doorkeeper::AccessToken.create_for( application: target_application, resource_owner: user, - scopes: "public #{HANDOFF_TOKEN_SCOPE}", + scopes: "public #{AccessTokenAuthentication::HANDOFF_SCOPE}", expires_in: TOKEN_LIFETIME.to_i ) Rails.logger.info("Issued user access token: issuer application uid=#{doorkeeper_token.application.uid} user_id=#{user.id}") @@ -94,7 +92,7 @@ def public_client_names def authorized_issuer? issuer_uids = Rails.configuration.x.user_access_token_issuer_application_uids # resource_owner_id is set on user tokens, which never qualify even from an allowed application. - return false unless doorkeeper_token.resource_owner_id.nil? && doorkeeper_token.acceptable?([REQUIRED_SCOPE]) + return false unless AccessTokenAuthentication.issuer_token?(doorkeeper_token) application = doorkeeper_token.application # The target is a frontend client, so its credentials must not be usable to issue tokens. diff --git a/app/controllers/api/v8/base_controller.rb b/app/controllers/api/v8/base_controller.rb index 3110fb8bd..d4a602b87 100644 --- a/app/controllers/api/v8/base_controller.rb +++ b/app/controllers/api/v8/base_controller.rb @@ -6,9 +6,6 @@ module Api module V8 class BaseController < ApplicationController - # Marks tokens issued by Application::UserAccessTokensController. - HANDOFF_TOKEN_SCOPE = 'handoff' - clear_respond_to respond_to :json @@ -46,12 +43,9 @@ def present(hash) def authenticate_user! return @current_user if @current_user if doorkeeper_token - user = User.find_by(id: doorkeeper_token.resource_owner_id) if doorkeeper_token.accessible? - handoff = doorkeeper_token.includes_scope?(HANDOFF_TOKEN_SCOPE) - # The user may have gained privileges after the handoff token was issued. - return respond_unauthorized('Invalid token') if user.nil? || (handoff && user.privileged?) - @current_user = user - @auth_source = :handoff_token if handoff + @current_user = AccessTokenAuthentication.user_for(doorkeeper_token) + return respond_unauthorized('Invalid token') unless @current_user + @auth_source = :handoff_token if AccessTokenAuthentication.handoff?(doorkeeper_token) elsif Rails.configuration.x.accept_courses_mooc_fi_tokens @current_user = CoursesMoocFiAuthentication.user_for(request) @auth_source = :courses_mooc_fi_token if @current_user diff --git a/app/controllers/api/v8/users_controller.rb b/app/controllers/api/v8/users_controller.rb index e5a3daf77..d45f13966 100644 --- a/app/controllers/api/v8/users_controller.rb +++ b/app/controllers/api/v8/users_controller.rb @@ -189,7 +189,6 @@ def update @user = User.find_by!(id: params[:id]) unless params[:id] == 'current' @email_before = @user.email authorize! :update, @user - authorize! :update_credentials, @user if credentials_change_requested? set_user_fields set_extra_data(true) update_email @@ -307,6 +306,7 @@ def update_email if new_email.blank? @user.errors.add(:email, 'needed') elsif @user.email.casecmp(new_email) != 0 + authorize! :update_credentials, @user @user.email = new_email @user.email_verified = false UserMailer.email_confirmation(@user, params[:origin], params[:language]).deliver_now @@ -328,6 +328,7 @@ def set_password def maybe_update_password if params[:old_password].present? && params[:password].present? + authorize! :update_credentials, @user if @user.managed_externally? unless @user.update_password_via_courses_mooc_fi(params[:old_password], params[:password]) @user.errors.add(:password, 'could not be changed. Please check your current password and try again.') @@ -351,13 +352,6 @@ def maybe_update_password end end - def credentials_change_requested? - user_params = params[:user] - new_email = user_params[:email] if user_params.is_a?(ActionController::Parameters) - email_changed = new_email.present? && @user.email.to_s.casecmp(new_email.to_s.strip) != 0 - email_changed || (params[:old_password].present? && params[:password].present?) - end - def set_user_fields return if params[:user_field].nil? changes = {} diff --git a/app/controllers/auths_controller.rb b/app/controllers/auths_controller.rb index dc2d99086..dcc82b9f2 100644 --- a/app/controllers/auths_controller.rb +++ b/app/controllers/auths_controller.rb @@ -10,10 +10,9 @@ class AuthsController < ApplicationController def show if params[:username].present? && params[:session_id].present? - user = User.find_by(login: params[:username]) - user ||= User.find_by('lower(email) = ?', params[:username].downcase) + user = User.find_by_login_or_email(params[:username]) # Allows using oauth2 tokens of the new api for authenticating - res = if user && Doorkeeper::AccessToken.find_by(resource_owner_id: user.id, token: params[:session_id])&.accessible? + res = if user && AccessTokenAuthentication.user_for(Doorkeeper::AccessToken.by_token(params[:session_id])) == user OK_MESSAGE elsif user && find_session_by_id(params[:session_id])&.belongs_to?(user) OK_MESSAGE @@ -23,8 +22,7 @@ def show return render plain: res end - user = User.find_by(login: params[:username]) - user ||= User.find_by('lower(email) = ?', params[:username].downcase) + user = User.find_by_login_or_email(params[:username]) msg = if user && params[:password].present? && user.has_password?(params[:password]) OK_MESSAGE else diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 518a28c73..506076634 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -100,7 +100,7 @@ def update def confirm_email language = params[:language] - VerificationToken.unscoped.email.find_by!(user_id: params[:user_id], token: params[:id]) + VerificationToken.unscoped.email.find_by!(user_id: params[:user_id], token: params[:token]) redirect_path = root_url redirect_path = "https://course.elementsofai.com/#{language && language != "en" ? "#{language}/" : ''}email-verification" if params[:origin] && params[:origin].start_with?('elements_of_ai') redirect_path = "https://buildingai.elementsofai.com/#{language && language != "en" ? "#{language}/" : ''}email-verified" if params[:origin] && params[:origin].start_with?('building_ai') @@ -119,7 +119,7 @@ def send_verification_email def verify_destroying_user @user = authenticate_current_user_destroy - VerificationToken.delete_user.find_by!(user: @user, token: params[:id]) + VerificationToken.delete_user.find_by!(user: @user, token: params[:token]) end def destroy_user @@ -134,7 +134,7 @@ def destroy_user redirect_to verify_destroying_user_url, alert: failed_authentication_alert(status) return end - VerificationToken.delete_user.find_by!(user: user, token: params[:id]) + VerificationToken.delete_user.find_by!(user: user, token: params[:token]) sign_out if current_user == user email = user.email username = user.login diff --git a/app/helpers/sessions_helper.rb b/app/helpers/sessions_helper.rb index 5fe3b2f08..e21022fb7 100644 --- a/app/helpers/sessions_helper.rb +++ b/app/helpers/sessions_helper.rb @@ -34,7 +34,7 @@ def user_from_session end def user_from_basic_auth - if request&.authorization + if request && ActionController::HttpAuthentication::Basic.has_basic_credentials?(request) username, password = ActionController::HttpAuthentication::Basic.user_name_and_password(request) User.authenticate(username, password) if username && password end diff --git a/app/models/ability.rb b/app/models/ability.rb index 56f5d436f..b10ffca36 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -7,9 +7,8 @@ class Ability include CanCan::Ability # +auth_source+ restricts account changes for tokens that are not the user's own login: - # :courses_mooc_fi_token denies them all, administrators included; :handoff_token (issued by - # Api::V8::Application::UserAccessTokensController) allows profile edits but no email or password - # change (+:update_credentials+) and no account deletion. + # :courses_mooc_fi_token denies all, administrators included; :handoff_token allows profile + # edits but no +:update_credentials+ (email, password) and no account deletion. def initialize(user, auth_source: nil) alias_action :update_credentials, to: :update diff --git a/app/models/user.rb b/app/models/user.rb index 27ec0385d..8dbebe43f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -167,6 +167,14 @@ def has_password?(submitted_password) result end + # The user a typed username names, matched against login exactly and email case-insensitively. + def self.find_by_login_or_email(username) + username = username&.strip + return nil if username.blank? + + find_by(login: username) || find_by('lower(email) = ?', username.downcase) + end + # The user when the password is accepted, nil otherwise. Use .authenticate_with_status when the # outcome is shown to the user, so that a delegation failure is not reported as a wrong password. def self.authenticate(login, submitted_password) @@ -179,10 +187,7 @@ def self.authenticate(login, submitted_password) # cannot be delegated at all and only an admin can repair it. A found user is returned whatever # the status, so sign in only on :accepted. def self.authenticate_with_status(login, submitted_password) - return [nil, :rejected] unless login - login = login.strip - user = find_by(login: login) - user ||= find_by('lower(email) = ?', login.downcase) + user = find_by_login_or_email(login) return [nil, :rejected] if user.nil? return [user, user.courses_mooc_fi_authentication_status(submitted_password)] if user.managed_externally? diff --git a/app/services/access_token_authentication.rb b/app/services/access_token_authentication.rb new file mode 100644 index 000000000..6b1ee8776 --- /dev/null +++ b/app/services/access_token_authentication.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +# Resolves the user behind a Doorkeeper access token, for every endpoint that accepts one. +module AccessTokenAuthentication + # Marks tokens issued by Api::V8::Application::UserAccessTokensController. + HANDOFF_SCOPE = 'handoff' + # Lets a client_credentials token call Api::V8::Application::UserAccessTokensController. + ISSUER_SCOPE = 'issue_user_access_tokens' + + # The token's owner, or nil when the token is revoked or expired, has no owner, or is a handoff + # token of a user who became privileged after it was issued. + def self.user_for(token) + return nil unless token&.accessible? + + user = User.find_by(id: token.resource_owner_id) + user unless user.nil? || (handoff?(token) && user.privileged?) + end + + # Whether the token lets its client call UserAccessTokensController; the client still has to be + # an allow-listed issuer. + def self.issuer_token?(token) + token.resource_owner_id.nil? && token.acceptable?([ISSUER_SCOPE]) + end + + def self.handoff?(token) + token.includes_scope?(HANDOFF_SCOPE) + end +end diff --git a/app/views/password_reset_keys/show.html.erb b/app/views/password_reset_keys/show.html.erb index e59066d8d..673a0b902 100644 --- a/app/views/password_reset_keys/show.html.erb +++ b/app/views/password_reset_keys/show.html.erb @@ -2,7 +2,7 @@

Resetting password for <%= @user.email %>

-<%= form_tag reset_password_path(key: @key.token), method: :delete do %> +<%= form_tag reset_password_path(@key.token), method: :delete do %> <%= labeled_field('New password: ', password_field_tag(:password)) %> <%= labeled_field('Confirm new password: ', password_field_tag(:password_confirmation)) %> <%= submit_tag 'Set password' %> diff --git a/config/application.rb b/config/application.rb index 18f1ad641..4a703f644 100644 --- a/config/application.rb +++ b/config/application.rb @@ -27,7 +27,7 @@ class Application < Rails::Application # config.i18n.default_locale = :de # Configure sensitive parameters which will be filtered from the log file. - config.filter_parameters += %i[password session_id api_password submission_file return_file test_output stdout stderr vm_log client_secret access_token refresh_token] + [/\A(token|code)\z/] + config.filter_parameters += %i[password session_id api_password submission_file return_file test_output stdout stderr vm_log client_secret access_token refresh_token bearer_token id_token id_token_hint code_verifier] + [/\A(token|code)\z/] # Do not swallow errors in after_commit/after_rollback callbacks. # config.eager_load_paths << Rails.root.join('lib') diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index e493b66b4..4f92a4d1c 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -39,7 +39,7 @@ # 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?('issue_user_access_tokens') + 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. diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index 90cd657b4..5a568b4b3 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -5,7 +5,7 @@ LOGGER = Logger.new('log/rack-attack.log') ActiveSupport::Notifications.subscribe('rack.attack') do |_name, _start, _finish, _request_id, req| req = req[:request] - msg = [req.env['rack.attack.match_type'], req.ip, req.request_method, req.fullpath, ('"' + req.user_agent.to_s + '"')].join(' ') + msg = [req.env['rack.attack.match_type'], req.ip, req.request_method, SecretPathFilter.filter(req.path), ('"' + req.user_agent.to_s + '"')].join(' ') if %i[throttle blocklist].include?(req.env['rack.attack.match_type']) LOGGER.error(msg) else @@ -14,12 +14,128 @@ end end +# Not Rails.cache: Readthis increments with a separate read and write, so concurrent requests +# would slip past every limit. +Rack::Attack.cache.store = Redis.new(url: ENV.fetch('REDIS_URL')) if ENV['REDIS_URL'] + # RACK Request: https://github.com/rack/rack/blob/main/lib/rack/request.rb class Rack::Attack - ISSUE_TOKEN_PATH = '/api/v8/application/user_access_tokens' + LOGIN_PATHS = %w[/sessions /oauth/token].freeze + ACCOUNT_CREATION_PATHS = %w[/user /api/v8/users].freeze + PASSWORD_RESET_PATHS = %w[/password_reset_keys /api/v8/users/password_reset].freeze + + class Request + # The path as Rails routes it, so that /oauth/token/, //oauth/token and /oauth/token.json + # cannot dodge the matches below. + def normalized_path + @normalized_path ||= begin + path = ::Rack::Utils.unescape_path(self.path).b.squeeze('/') + path = path.delete_suffix('/') unless path == '/' + path.sub(%r{\.[^/.]+\z}, '') + end + end + + # The parameters as the Rails controller sees them. A body Rails cannot parse fails the + # request there, so only the query string counts then. + def controller_params + @controller_params ||= begin + action_dispatch_request.request_parameters.merge(query_params) + rescue StandardError + query_params + end + end + + def login_attempt? + LOGIN_PATHS.include?(normalized_path) && post? + end + + def account_creation? + ACCOUNT_CREATION_PATHS.include?(normalized_path) && post? + end + + def api_v7_submission? + post? && normalized_path.match?(%r{\A(/org/\d+)?/exercises/\d+/submissions\z}) + end + + def api_v8_submission? + post? && normalized_path.match?(%r{\A/api/v8/core/exercises/\d+/submissions\z}) + end + + def password_reset_request? + PASSWORD_RESET_PATHS.include?(normalized_path) && post? + end + + # Normalized as the password reset controllers look the address up. + def password_reset_email + controller_params['email'].to_s.strip.downcase.presence + end + + def token_issue? + normalized_path == Rack::Attack.issue_token_path && post? + end + + def shared_address_password_check? + auth_password_check? || basic_auth_username.present? + end + + def auth_password_check? + normalized_path == '/auth' && controller_params['password'].present? + end + + # Only SessionsHelper reads Basic credentials as a user's password; Doorkeeper reads them as + # client credentials and the API controllers ignore them. + def basic_auth_username + return @basic_auth_username if defined?(@basic_auth_username) + + @basic_auth_username = unless normalized_path.start_with?('/oauth/', '/api/') + request = action_dispatch_request + ActionController::HttpAuthentication::Basic.user_name_and_password(request).first if ActionController::HttpAuthentication::Basic.has_basic_credentials?(request) + end + end + + # The issuer application and target user of a token issue request. Nil unless the request + # carries a usable issuer token, so that anonymous requests cannot use up a user's handoffs. + def token_issue_bucket + return @token_issue_bucket if defined?(@token_issue_bucket) + + user_id = controller_params['user_id'].to_s if token_issue? + token = access_token if user_id&.match?(/\A\d+\z/) + @token_issue_bucket = ("#{token.application_id}:#{user_id.to_i}" if token && AccessTokenAuthentication.issuer_token?(token)) + end + + # Read from the header or parameters as Doorkeeper reads it. + def access_token_string + return @access_token_string if defined?(@access_token_string) + + @access_token_string = begin + Doorkeeper::OAuth::Token.from_request(action_dispatch_request, *Doorkeeper.config.access_token_methods) + rescue StandardError + nil + end + end + + private + def access_token + Doorkeeper::AccessToken.by_token(access_token_string) if access_token_string.present? + end + + def action_dispatch_request + @action_dispatch_request ||= ActionDispatch::Request.new(env) + end + + def query_params + action_dispatch_request.query_parameters + rescue StandardError + {} + end + end + + def self.issue_token_path + @issue_token_path ||= Rails.application.routes.url_helpers.api_v8_application_user_access_tokens_path + end def self.user_identifier_discriminator(access_token) - db_token = Doorkeeper::AccessToken.find_by(token: access_token) + db_token = Doorkeeper::AccessToken.by_token(access_token) return access_token unless db_token db_token.resource_owner_id end @@ -29,13 +145,9 @@ def self.user_identifier_discriminator(access_token) # RACK Request adds the "HTTP_" infront of the header key. # Exempt from the safelist so the issue endpoint stays throttled for the mooc.fi backend too. safelist('mark courses.mooc.fi backend requests as safe') do |request| - if request.path.start_with?(ISSUE_TOKEN_PATH) - false - elsif ENV['RACK_ATTACK_SAFE_API_KEY'] && request.has_header?('HTTP_RATELIMIT_PROTECTION_SAFE_API_KEY') - ENV['RACK_ATTACK_SAFE_API_KEY'] == request.get_header('HTTP_RATELIMIT_PROTECTION_SAFE_API_KEY') - else - false - end + ENV['RACK_ATTACK_SAFE_API_KEY'].present? && + ActiveSupport::SecurityUtils.secure_compare(request.get_header('HTTP_RATELIMIT_PROTECTION_SAFE_API_KEY').to_s, ENV['RACK_ATTACK_SAFE_API_KEY']) && + request.normalized_path != issue_token_path end # Return 503 Service Unavailable for throttles @@ -47,59 +159,65 @@ def self.user_identifier_discriminator(access_token) end throttle('login attempts per minute', limit: 100, period: 1.minute) do |req| - req.ip if %w[/sessions /oauth/token].include?(req.path) && req.post? + req.ip if req.login_attempt? end throttle('login attempts per hour', limit: 600, period: 1.hour) do |req| - req.ip if %w[/sessions /oauth/token].include?(req.path) && req.post? + req.ip if req.login_attempt? end throttle('login attempts per day', limit: 3000, period: 1.day) do |req| - req.ip if %w[/sessions /oauth/token].include?(req.path) && req.post? + req.ip if req.login_attempt? + end + + # Kept apart from the login buckets so that clients sending Basic credentials on every request + # cannot lock their classroom's shared address out of logging in. + throttle('basic auth and /auth password checks per ip per minute', limit: 300, period: 1.minute) do |req| + req.ip if req.shared_address_password_check? + end + + throttle('basic auth and /auth password checks per ip per hour', limit: 3000, period: 1.hour) do |req| + req.ip if req.shared_address_password_check? end throttle('account creations per minute', limit: 60, period: 1.minute) do |req| - req.ip if %w[/user /api/v8/users].include?(req.path) && req.post? + req.ip if req.account_creation? end throttle('account creations per day', limit: 3000, period: 1.day) do |req| - req.ip if %w[/user /api/v8/users].include?(req.path) && req.post? + req.ip if req.account_creation? end throttle('submissions per ip per 10 minutes', limit: 600, period: 10.minutes) do |req| - req.ip if (req.path =~ %r{^/exercises/\d+/submissions$} || - req.path =~ %r{^/org/\d+/exercises/\d+/submissions$} || - req.path =~ %r{^/api/v8/core/exercises/\d+/submissions$}) && - req.post? + req.ip if req.api_v7_submission? || req.api_v8_submission? end throttle('submissions per ip per day to api v7', limit: 5000, period: 1.day) do |req| - req.ip if (req.path =~ %r{^/exercises/\d+/submissions$} || - req.path =~ %r{^/org/\d+/exercises/\d+/submissions$}) && - req.post? + req.ip if req.api_v7_submission? end - # Limit the number of submissions per user to 250 per day to the api v8 endpoint throttle('submissions per user per day to api v8', limit: 250, period: 1.day) do |req| - if req.path =~ %r{^/api/v8/core/exercises/\d+/submissions$} && req.post? - token = req.params['access_token'] - discriminator = user_identifier_discriminator(token) - discriminator - end + user_identifier_discriminator(req.access_token_string) if req.api_v8_submission? && req.access_token_string.present? end + # Every call comes from the mooc.fi backend, so this per-IP limit is effectively global. throttle('user access tokens per ip per minute', limit: 120, period: 1.minute) do |req| - req.ip if req.path.start_with?(ISSUE_TOKEN_PATH) && req.post? + req.ip if req.token_issue? + end + + throttle('user access tokens per user per 10 minutes', limit: 10, period: 10.minutes) do |req| + req.token_issue_bucket + end + + throttle('user access tokens per user per day', limit: 50, period: 1.day) do |req| + req.token_issue_bucket end - # Limit the number of password resets per ip to 10 per hour throttle('password resets per ip per hour', limit: 10, period: 1.hour) do |req| - req.ip if req.path == '/password_reset_keys' && - req.post? + req.ip if req.password_reset_request? end - # Limit the number of password resets per email to 5 per day throttle('password resets per email per day', limit: 5, period: 1.day) do |req| - req.params['email'] if req.path == '/password_reset_keys' && req.post? + req.password_reset_email if req.password_reset_request? end end diff --git a/config/initializers/secret_path_filter.rb b/config/initializers/secret_path_filter.rb new file mode 100644 index 000000000..f29a78c3a --- /dev/null +++ b/config/initializers/secret_path_filter.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +# Emailed links carry their token in the URL path, which Rails and Rack::Attack would log verbatim. +module SecretPathFilter + TOKEN_SEGMENT = %r{(/reset_password/+|/users/+[^/]+/+(?:verify|destroy)/+)[^/?#]+} + + def self.filter(path) + path.gsub(TOKEN_SEGMENT, '\1[FILTERED]') + end +end + +ActionDispatch::Request.prepend(Module.new do + def filtered_path + SecretPathFilter.filter(super) + end +end) + +Rails.application.config.filter_redirect << SecretPathFilter::TOKEN_SEGMENT diff --git a/config/routes.rb b/config/routes.rb index f12da49bd..202b74cef 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -284,14 +284,14 @@ end end - get '/users/:user_id/verify/:id', to: 'users#confirm_email', as: 'confirm_email' + get '/users/:user_id/verify/:token', to: 'users#confirm_email', as: 'confirm_email' post '/users/:user_id/send_verification_email', to: 'users#send_verification_email', as: 'send_verification_email' resources :users, only: [:index] post '/users/:user_id/send_destroy_email', to: 'users#send_destroy_email', as: 'send_destroy_email' - get '/users/:user_id/destroy/:id', to: 'users#verify_destroying_user', as: 'verify_destroying_user' - delete '/users/:user_id/destroy/:id', to: 'users#destroy_user', as: 'destroy_user' + get '/users/:user_id/destroy/:token', to: 'users#verify_destroying_user', as: 'verify_destroying_user' + delete '/users/:user_id/destroy/:token', to: 'users#destroy_user', as: 'destroy_user' resources :certificates, only: %i[show create] diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 0a7971bcf..4ea7df95f 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -252,7 +252,7 @@ end def do_destroy_user(password) - delete :destroy_user, params: { user_id: user.id, id: verification_token.token, im_sure: '1', + delete :destroy_user, params: { user_id: user.id, token: verification_token.token, im_sure: '1', user: { password: password } } end diff --git a/spec/requests/handoff_token_consumers_spec.rb b/spec/requests/handoff_token_consumers_spec.rb new file mode 100644 index 000000000..ab787f1b1 --- /dev/null +++ b/spec/requests/handoff_token_consumers_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Handoff tokens of users who became privileged', type: :request do + let(:user) { FactoryBot.create(:user) } + let(:application) { Doorkeeper::Application.create!(name: 'client', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob') } + let(:token) { Doorkeeper::AccessToken.create!(application: application, resource_owner_id: user.id, scopes: 'public handoff') } + + before { user.update!(administrator: true) } + + it 'are accepted by the beta API before the user became privileged' do + user.update!(administrator: false) + get '/api/beta/participant', headers: { 'Authorization' => "Bearer #{token.token}" } + + expect(response).to have_http_status(200) + end + + it 'are refused by the beta API' do + get '/api/beta/participant', headers: { 'Authorization' => "Bearer #{token.token}" } + + expect(response).to have_http_status(401) + end + + it 'are refused by /auth' do + get '/auth.text', params: { username: user.login, session_id: token.token } + + expect(response.body).to eq('FAIL') + end +end diff --git a/spec/requests/issuer_token_hardening_spec.rb b/spec/requests/issuer_token_hardening_spec.rb index 3c9119bbd..27d27440e 100644 --- a/spec/requests/issuer_token_hardening_spec.rb +++ b/spec/requests/issuer_token_hardening_spec.rb @@ -38,7 +38,7 @@ def request_token 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' } + 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 diff --git a/spec/requests/rack_attack_throttles_spec.rb b/spec/requests/rack_attack_throttles_spec.rb new file mode 100644 index 000000000..3f0a65450 --- /dev/null +++ b/spec/requests/rack_attack_throttles_spec.rb @@ -0,0 +1,148 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Rack::Attack throttles', type: :request do + let(:app) { Rack::Attack.new(->(_env) { [200, {}, ['ok']] }) } + let(:safe_headers) { { 'HTTP_RATELIMIT_PROTECTION_SAFE_API_KEY' => 'safe-key' } } + + around :each do |example| + Rack::Attack.enabled = true + Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new + ENV['RACK_ATTACK_SAFE_API_KEY'] = 'safe-key' + example.run + ensure + Rack::Attack.enabled = false + ENV.delete('RACK_ATTACK_SAFE_API_KEY') + end + + def call(path, method: 'POST', ip: '203.0.113.7', **options) + env = Rack::MockRequest.env_for(path, { method: method, 'REMOTE_ADDR' => ip }.merge(options)) + # env_for would read the start of //x as a host name. + env['PATH_INFO'] = path.split('?').first + status, = app.call(env) + status + end + + def status_after(times, path, **options) + status = nil + times.times { status = call(path, **options) } + status + end + + def basic(username, password = 'guess') + { 'HTTP_AUTHORIZATION' => ActionController::HttpAuthentication::Basic.encode_credentials(username, password) } + end + + describe 'path variants' do + { + '/oauth/token' => 100, + '/sessions' => 100, + '/api/v8/users' => 60, + '/password_reset_keys' => 10, + '/api/v8/users/password_reset' => 10, + '/api/v8/application/user_access_tokens' => 120 + }.each do |path, limit| + ["#{path}/", "/#{path}", "#{path}.json"].each do |variant| + it "throttles #{variant} together with #{path}" do + expect(status_after(limit, path)).to eq(200) + expect(call(variant)).to eq(503) + end + end + end + + %w[/api/v8/application/user_access_tokens/ //api/v8/application/user_access_tokens /api/v8/application/user_access_tokens.json].each do |variant| + it "keeps #{variant} out of the safelist" do + expect(status_after(120, variant, **safe_headers)).to eq(200) + expect(call(variant, **safe_headers)).to eq(503) + end + end + end + + describe 'Basic auth and /auth password checks per IP' do + it 'throttles Basic auth on web URLs' do + 300.times { |i| call('/courses', method: 'GET', **basic("user#{i}")) } + + expect(call('/courses', method: 'GET', **basic('fresh-user'))).to eq(503) + end + + it 'throttles /auth password checks' do + 300.times { |i| call('/auth', method: 'GET', params: { username: "user#{i}", password: 'x' }) } + + expect(call('/auth.text', params: { username: 'fresh-user', password: 'x' })).to eq(503) + end + + it 'counts query parameters when a JSON body is empty' do + 300.times { |i| call("/auth?username=user#{i}&password=x", method: 'GET', 'CONTENT_TYPE' => 'application/json') } + + expect(call('/auth?username=fresh-user&password=x', method: 'GET', 'CONTENT_TYPE' => 'application/json')).to eq(503) + end + + it 'ignores client credentials sent as Basic auth to Doorkeeper' do + expect(status_after(301, '/oauth/introspect', **basic('client-id'))).to eq(200) + end + + it 'ignores /auth session checks' do + expect(status_after(301, '/auth.text', method: 'GET', params: { username: 'victim', session_id: 'abc' })).to eq(200) + end + + it 'leaves the login bucket of the address alone' do + 300.times { |i| call('/courses', method: 'GET', **basic("user#{i}")) } + + expect(call('/sessions', params: { session: { login: 'fresh-user', password: 'x' } })).to eq(200) + end + end + + describe 'user access tokens per target user' do + let(:path) { '/api/v8/application/user_access_tokens' } + let(:issuer_app) { Doorkeeper::Application.create!(name: 'mooc.fi backend', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob', scopes: 'issue_user_access_tokens') } + let(:issuer_token) { Doorkeeper::AccessToken.create!(application: issuer_app, scopes: 'issue_user_access_tokens') } + let(:bearer) { { 'HTTP_AUTHORIZATION' => "Bearer #{issuer_token.token}" } } + + it 'throttles one user without affecting others' do + 10.times { |i| call(path, ip: "198.51.100.#{i}", **safe_headers, **bearer, params: { user_id: '42' }) } + + expect(call(path, ip: '198.51.100.99', **bearer, params: { user_id: '042' })).to eq(503) + expect(call(path, ip: '198.51.100.99', **bearer, params: { user_id: '43' })).to eq(200) + end + + it 'reads user_id from a JSON body' do + body = { user_id: 42 }.to_json + 10.times { |i| call(path, ip: "198.51.100.#{i}", input: body, 'CONTENT_TYPE' => 'application/json', **bearer) } + + expect(call(path, ip: '198.51.100.99', input: body, 'CONTENT_TYPE' => 'application/json', **bearer)).to eq(503) + end + + it 'does not let requests without a usable issuer token use up the budget' do + user_token = Doorkeeper::AccessToken.create!(application: issuer_app, resource_owner_id: 1, scopes: 'issue_user_access_tokens') + 10.times { |i| call(path, ip: "198.51.100.#{i}", params: { user_id: '42' }) } + 10.times { |i| call(path, ip: "198.51.100.#{i + 20}", 'HTTP_AUTHORIZATION' => 'Bearer forged', params: { user_id: '42' }) } + 10.times { |i| call(path, ip: "198.51.100.#{i + 40}", 'HTTP_AUTHORIZATION' => "Bearer #{user_token.token}", params: { user_id: '42' }) } + + expect(call(path, ip: '198.51.100.99', **bearer, params: { user_id: '42' })).to eq(200) + end + end + + describe 'password resets per email' do + it 'normalizes the address as the controller does' do + %w[Victim@x.fi victim@X.fi].each { |email| call('/password_reset_keys', ip: '198.51.100.1', params: { email: email }) } + call('/password_reset_keys', ip: '198.51.100.2', params: { email: ' victim@x.fi' }) + call('/password_reset_keys', ip: '198.51.100.3', input: { email: 'VICTIM@x.fi' }.to_json, 'CONTENT_TYPE' => 'application/json') + call('/password_reset_keys', ip: '198.51.100.4', params: { email: 'victim@x.fi' }) + + expect(call('/password_reset_keys', ip: '198.51.100.5', params: { email: 'victim@x.fi' })).to eq(503) + expect(call('/api/v8/users/password_reset', ip: '198.51.100.6', params: { email: 'victim@x.fi' })).to eq(503) + end + end + + describe 'submissions per user to api v8' do + let(:user) { FactoryBot.create(:user) } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public') } + + it 'reads the token from the Authorization header' do + 250.times { |i| call('/api/v8/core/exercises/1/submissions', ip: "198.51.100.#{i % 200}", 'HTTP_AUTHORIZATION' => "Bearer #{token.token}") } + + expect(call('/api/v8/core/exercises/1/submissions', ip: '203.0.113.99', params: { access_token: token.token })).to eq(503) + end + end +end diff --git a/spec/requests/secret_path_logging_spec.rb b/spec/requests/secret_path_logging_spec.rb new file mode 100644 index 000000000..e9fad6c29 --- /dev/null +++ b/spec/requests/secret_path_logging_spec.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Tokens in emailed link paths', type: :request do + let(:user) { FactoryBot.create(:user, password: 'secret123') } + + def instrumented(event) + payloads = [] + subscriber = ActiveSupport::Notifications.subscribe(event) { |*, payload| payloads << payload } + yield + payloads + ensure + ActiveSupport::Notifications.unsubscribe(subscriber) + end + + def logged_path_and_params(&block) + payload = instrumented('process_action.action_controller', &block).last + [payload[:path], payload[:request].filtered_parameters.to_s] + end + + it 'keeps the email verification token out of the request log and still verifies' do + token = VerificationToken.create!(user: user, type: :email) + path, params = logged_path_and_params { get "/users/#{user.id}/verify/#{token.token}" } + + expect(path).to eq("/users/#{user.id}/verify/[FILTERED]") + expect(params).not_to include(token.token) + expect(user.reload.email_verified).to be(true) + end + + it 'keeps the password reset token out of the request log' do + key = ActionToken.generate_password_reset_key_for(user) + path, params = logged_path_and_params { get "/reset_password/#{key.token}" } + + expect(path).to eq('/reset_password/[FILTERED]') + expect(params).not_to include(key.token) + expect(response).to have_http_status(200) + expect(response.body).not_to include("key=#{key.token}") + end + + it 'keeps the account deletion token out of redirect logs' do + token = VerificationToken.create!(user: user, type: :delete_user) + post '/sessions', params: { session: { login: user.login, password: 'secret123' } } + + payloads = instrumented('redirect_to.action_controller') do + delete "/users/#{user.id}/destroy/#{token.token}", params: { im_sure: '1', user: { password: 'wrong' } } + end + + expect(response).to redirect_to("http://www.example.com/users/#{user.id}/destroy/#{token.token}") + expect(payloads.map { |payload| payload[:location] }).to eq(['[FILTERED]']) + end + + it 'filters the token from Rack::Attack log lines' do + expect(SecretPathFilter.filter('//users/1/destroy/abc.json')).to eq('//users/1/destroy/[FILTERED]') + expect(SecretPathFilter.filter('/reset_password')).to eq('/reset_password') + end +end From fd3778ebfb91babff643f419f2ed4c89c83aff76 Mon Sep 17 00:00:00 2001 From: Henrik Nygren Date: Wed, 4 Nov 2026 15:58:27 +0200 Subject: [PATCH 08/18] Keep issued user access tokens out of caches --- .../api/v8/application/user_access_tokens_controller.rb | 3 +++ .../api/v8/application/user_access_tokens_controller_spec.rb | 1 + 2 files changed, 4 insertions(+) diff --git a/app/controllers/api/v8/application/user_access_tokens_controller.rb b/app/controllers/api/v8/application/user_access_tokens_controller.rb index b6c39aab2..ecdb53aa6 100644 --- a/app/controllers/api/v8/application/user_access_tokens_controller.rb +++ b/app/controllers/api/v8/application/user_access_tokens_controller.rb @@ -75,6 +75,9 @@ def create ) Rails.logger.info("Issued user access token: issuer application uid=#{doorkeeper_token.application.uid} user_id=#{user.id}") + response.headers['Cache-Control'] = 'no-store' + response.headers['Pragma'] = 'no-cache' + render json: { access_token: token.plaintext_token, token_type: 'bearer', 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 index 74e88340b..6bab3b316 100644 --- a/spec/controllers/api/v8/application/user_access_tokens_controller_spec.rb +++ b/spec/controllers/api/v8/application/user_access_tokens_controller_spec.rb @@ -41,6 +41,7 @@ def post_with(token, user_id) 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 From 9e7e0d1b9a1ddd833b4f13119502e7fecae4a109 Mon Sep 17 00:00:00 2001 From: Henrik Nygren Date: Wed, 11 Nov 2026 10:42:26 +0200 Subject: [PATCH 09/18] Stop throttle param lookups failing on requests without a body type --- config/initializers/rack_attack.rb | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index b246c5640..bec4df790 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -185,10 +185,20 @@ def self.token_issue_bucket(req) sig { params(req: Rack::Attack::Request, name: String).returns(T.nilable(String)) } def self.rails_param(req, name) value = req.GET[name] - value ||= Mime::Type.lookup(req.media_type.to_s).symbol == :json ? json_body_param(req, name) : req.POST[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, name: String).returns(T.nilable(String)) } private_class_method def self.json_body_param(req, name) body = req.body From 4e07a59c87a8fb3b3bd5992368d592a0e5b68d17 Mon Sep 17 00:00:00 2001 From: Henrik Nygren Date: Wed, 11 Nov 2026 10:42:30 +0200 Subject: [PATCH 10/18] Type user access token issuing strictly --- .../user_access_tokens_controller.rb | 34 ++++++++++++------- app/controllers/auths_controller.rb | 2 +- app/models/user.rb | 4 +-- app/services/access_token_authentication.rb | 23 +++++++++++-- config/initializers/secret_path_filter.rb | 5 ++- .../rbi/dsl/generated_path_helpers_module.rbi | 3 ++ .../rbi/dsl/generated_url_helpers_module.rbi | 3 ++ 7 files changed, 56 insertions(+), 18 deletions(-) diff --git a/app/controllers/api/v8/application/user_access_tokens_controller.rb b/app/controllers/api/v8/application/user_access_tokens_controller.rb index ecdb53aa6..7cd5ca983 100644 --- a/app/controllers/api/v8/application/user_access_tokens_controller.rb +++ b/app/controllers/api/v8/application/user_access_tokens_controller.rb @@ -1,3 +1,4 @@ +# typed: strict # frozen_string_literal: true module Api @@ -6,9 +7,10 @@ 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 = 1.day + TOKEN_LIFETIME = T.let(1.day, ActiveSupport::Duration) swagger_path '/api/v8/application/user_access_tokens' do operation :post do @@ -54,9 +56,11 @@ class UserAccessTokensController < Api::V8::BaseController skip_before_action :authenticate_user! skip_authorization_check + sig { void } def create - return respond_unauthorized unless doorkeeper_token&.accessible? - return respond_forbidden unless authorized_issuer? + issuer_token = doorkeeper_token + return respond_unauthorized unless issuer_token&.accessible? + return respond_forbidden unless authorized_issuer?(issuer_token) return respond_with_error('user_id must be an integer', 400) unless params[:user_id].to_s.match?(/\A\d+\z/) @@ -64,7 +68,7 @@ def create return respond_not_found('User not found') unless user return respond_forbidden('Tokens are not issued for administrators, teachers or assistants') if user.privileged? - target_application = Doorkeeper::Application.find_by(uid: Rails.configuration.x.user_access_token_target_application_uid) + 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( @@ -73,7 +77,7 @@ def create scopes: "public #{AccessTokenAuthentication::HANDOFF_SCOPE}", expires_in: TOKEN_LIFETIME.to_i ) - Rails.logger.info("Issued user access token: issuer application uid=#{doorkeeper_token.application.uid} user_id=#{user.id}") + Rails.logger.info("Issued user access token: issuer application uid=#{issuer_token.application&.uid} user_id=#{user.id}") response.headers['Cache-Control'] = 'no-store' response.headers['Pragma'] = 'no-cache' @@ -88,20 +92,26 @@ def create private # Their credentials are served publicly by the credentials endpoint. + sig { returns(T::Array[String]) } def public_client_names - Array(SiteSetting.value('valid_clients')).filter_map { |client| client['name'] } + 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 - def authorized_issuer? - issuer_uids = Rails.configuration.x.user_access_token_issuer_application_uids + 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?(doorkeeper_token) + return false unless AccessTokenAuthentication.issuer_token?(issuer_token) + + application = issuer_token.application + return false unless application - application = doorkeeper_token.application # The target is a frontend client, so its credentials must not be usable to issue tokens. - issuer_uids.include?(application&.uid) && + AccessTokenAuthentication.issuer_application_uids.include?(application.uid) && application.confidential? && - application.uid != Rails.configuration.x.user_access_token_target_application_uid && + application.uid != AccessTokenAuthentication.target_application_uid && public_client_names.exclude?(application.name) end end diff --git a/app/controllers/auths_controller.rb b/app/controllers/auths_controller.rb index d8e95efe6..df9bfd9e0 100644 --- a/app/controllers/auths_controller.rb +++ b/app/controllers/auths_controller.rb @@ -13,7 +13,7 @@ class AuthsController < ApplicationController sig { void } def show if params[:username].present? && params[:session_id].present? - user = User.find_by_login_or_email(params[:username].to_s) + user = User.by_login_or_email(params[:username].to_s) # Allows using oauth2 tokens of the new api for authenticating res = if user && AccessTokenAuthentication.user_for(Doorkeeper::AccessToken.by_token(params[:session_id].to_s)) == user OK_MESSAGE diff --git a/app/models/user.rb b/app/models/user.rb index 09b659b8d..ee7adc938 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -190,7 +190,7 @@ def has_password?(submitted_password) # 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.find_by_login_or_email(username) + def self.by_login_or_email(username) username = username&.strip return nil if username.blank? @@ -211,7 +211,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) - user = find_by_login_or_email(login) + 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? diff --git a/app/services/access_token_authentication.rb b/app/services/access_token_authentication.rb index 6b1ee8776..6f8f4ed29 100644 --- a/app/services/access_token_authentication.rb +++ b/app/services/access_token_authentication.rb @@ -1,14 +1,18 @@ +# 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 revoked or expired, has no owner, or is a handoff - # token of a user who became privileged after it was issued. + # 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? @@ -18,11 +22,26 @@ def self.user_for(token) # 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/config/initializers/secret_path_filter.rb b/config/initializers/secret_path_filter.rb index 175566c3c..9503948bb 100644 --- a/config/initializers/secret_path_filter.rb +++ b/config/initializers/secret_path_filter.rb @@ -26,6 +26,9 @@ def filtered_path end end -ActionDispatch::Request.prepend(SecretPathFilter::RequestFilteredPath) +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/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 From d86e737cc4d68b32ffd8c25faa5b592a6c3e9804 Mon Sep 17 00:00:00 2001 From: Henrik Nygren Date: Wed, 11 Nov 2026 10:42:35 +0200 Subject: [PATCH 11/18] Authenticate API specs with real access tokens --- .../user_access_tokens_controller_spec.rb | 11 ++-- .../api/v8/base_controller_spec.rb | 6 +- .../core/courses/reviews_controller_spec.rb | 8 +-- .../core/courses/unlocks_controller_spec.rb | 2 +- .../api/v8/core/courses_controller_spec.rb | 2 +- .../core/exercises/details_controller_spec.rb | 4 +- .../exercises/solutions_controller_spec.rb | 4 +- .../exercises/submissions_controller_spec.rb | 2 +- .../api/v8/core/exercises_controller_spec.rb | 2 +- .../organizations/courses_controller_spec.rb | 4 +- .../submissions/feedback_controller_spec.rb | 2 +- .../submissions/reviews_controller_spec.rb | 6 +- .../v8/core/submissions_controller_spec.rb | 12 ++-- ...rses_with_this_template_controller_spec.rb | 2 +- .../all_courses_with_this_template_spec.rb | 8 +-- .../exercises/points_controller_spec.rb | 8 +-- .../exercises/users/points_controller_spec.rb | 2 +- .../v8/courses/exercises_controller_spec.rb | 8 +-- .../api/v8/courses/points_controller_spec.rb | 16 ++--- .../submissions/last_hour_controller_spec.rb | 4 +- .../v8/courses/submissions_controller_spec.rb | 8 +-- .../courses/users/points_controller_spec.rb | 10 ++-- .../users/submissions_controller_spec.rb | 10 ++-- .../api/v8/courses_controller_spec.rb | 4 +- .../api/v8/courses_mooc_fi_token_auth_spec.rb | 2 +- .../model_solutions_controller_spec.rb | 2 +- .../users/submissions_controller_spec.rb | 2 +- .../exercises/points_controller_spec.rb | 4 +- .../exercises/users/points_controller_spec.rb | 2 +- .../courses/exercises_controller_spec.rb | 8 +-- .../courses/points_controller_spec.rb | 4 +- .../studyright_eligibility_controller_spec.rb | 2 +- .../courses/submissions_controller_spec.rb | 8 +-- .../courses/users/points_controller_spec.rb | 12 ++-- .../users/submissions_controller_spec.rb | 10 ++-- .../organizations/courses_controller_spec.rb | 4 +- .../v8/users/teacherships_controller_spec.rb | 2 +- .../api/v8/users_controller_spec.rb | 8 +-- .../submission_download_authorization_spec.rb | 2 +- spec/requests/handoff_token_consumers_spec.rb | 1 + spec/requests/issuer_token_hardening_spec.rb | 58 +++++-------------- spec/requests/oauth_token_info_spec.rb | 3 +- spec/requests/rack_attack_throttles_spec.rb | 55 ++++-------------- spec/requests/secret_path_logging_spec.rb | 3 +- 44 files changed, 139 insertions(+), 198 deletions(-) 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 index 6bab3b316..8db72b47c 100644 --- a/spec/controllers/api/v8/application/user_access_tokens_controller_spec.rb +++ b/spec/controllers/api/v8/application/user_access_tokens_controller_spec.rb @@ -1,3 +1,4 @@ +# typed: false # frozen_string_literal: true require 'spec_helper' @@ -32,7 +33,7 @@ def post_with(token, user_id) post_with(issuer_token, user.id) expect(response).to have_http_status(200) - body = JSON.parse(response.body) + 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) @@ -106,7 +107,7 @@ def post_with(token, user_id) post_with(issuer_token, "0#{other_user.id}") expect(response).to have_http_status(200) - issued = Doorkeeper::AccessToken.find_by(token: JSON.parse(response.body)['access_token']) + issued = Doorkeeper::AccessToken.find_by(token: response.parsed_body['access_token']) expect(issued.resource_owner_id).to eq(other_user.id) end @@ -119,7 +120,7 @@ def post_with(token, user_id) 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 } + end.not_to(change { Doorkeeper::AccessToken.where(resource_owner_id: admin.id).count }) expect(response).to have_http_status(403) end @@ -128,7 +129,7 @@ def post_with(token, user_id) 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 } + end.not_to(change { Doorkeeper::AccessToken.where(resource_owner_id: user.id).count }) expect(response).to have_http_status(403) end @@ -137,7 +138,7 @@ def post_with(token, user_id) 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 } + end.not_to(change { Doorkeeper::AccessToken.where(resource_owner_id: user.id).count }) expect(response).to have_http_status(403) end diff --git a/spec/controllers/api/v8/base_controller_spec.rb b/spec/controllers/api/v8/base_controller_spec.rb index 696ce44a2..fe26d0b6e 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, accessible?: true, includes_scope?: false } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public') } it { expect(subject.id).to be user.id } end end @@ -39,7 +39,7 @@ 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, accessible?: true, includes_scope?: false } + let(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: -1, scopes: 'public') } it 'request is not successful' do get :index expect(response).to have_http_status(401) @@ -53,7 +53,7 @@ def index request.accept = 'application/json' get :index expect(response).to have_http_status(401) - expect(JSON.parse(response.body)).to eq('error' => 'Invalid token') + expect(response.parsed_body).to eq('error' => 'Invalid token') end end context 'with an expired token' do 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 159bb1668..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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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 5d0b92065..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, accessible?: true, includes_scope?: false } + 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 4285719b8..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, accessible?: true, includes_scope?: false } + 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 987e1ca88..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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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 4a0e3aa58..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, accessible?: true, includes_scope?: false } + 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 947cf0f39..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, accessible?: true, includes_scope?: false } + 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 7bf9e05ed..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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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 d7c00794e..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, accessible?: true, includes_scope?: false } + 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 7aee8217e..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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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 deb52f1b5..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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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 af201c574..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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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 2592f211e..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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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 c29e319a0..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, accessible?: true, includes_scope?: false } + 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 764ef69dc..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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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 e291ec73b..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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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 2c0ca70bc..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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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 a0c3865bd..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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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 a16a47592..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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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 84efc0e50..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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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 577c87148..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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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 0e95329e4..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, accessible?: true, includes_scope?: false)) + 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 a5fc09eaf..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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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 8f725ee68..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, accessible?: true, includes_scope?: false } + 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 466412cf1..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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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 79cd2dd41..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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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 a704a38c3..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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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 7777be271..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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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 7316a5d93..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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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 d75d0f4dd..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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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/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 8dd749ed9..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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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, accessible?: true, includes_scope?: false } + 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 } 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/requests/handoff_token_consumers_spec.rb b/spec/requests/handoff_token_consumers_spec.rb index ab787f1b1..c53c5a943 100644 --- a/spec/requests/handoff_token_consumers_spec.rb +++ b/spec/requests/handoff_token_consumers_spec.rb @@ -1,3 +1,4 @@ +# typed: false # frozen_string_literal: true require 'spec_helper' diff --git a/spec/requests/issuer_token_hardening_spec.rb b/spec/requests/issuer_token_hardening_spec.rb index 27d27440e..aa6daf9fa 100644 --- a/spec/requests/issuer_token_hardening_spec.rb +++ b/spec/requests/issuer_token_hardening_spec.rb @@ -1,3 +1,4 @@ +# typed: false # frozen_string_literal: true require 'spec_helper' @@ -14,7 +15,7 @@ def request_token request_token expect(response).to have_http_status(200) - expect(JSON.parse(response.body)['expires_in']).to eq(3600) + expect(response.parsed_body['expires_in']).to eq(3600) end it 'leaves client_credentials tokens without the issuer scope non-expiring' do @@ -22,7 +23,7 @@ def request_token post '/oauth/token', params: { grant_type: 'client_credentials', client_id: issuer_app.uid, client_secret: issuer_app.plaintext_secret, scope: 'public' } expect(response).to have_http_status(200) - expect(JSON.parse(response.body)).not_to have_key('expires_in') + expect(response.parsed_body).not_to have_key('expires_in') end it 'leaves password-grant tokens non-expiring' do @@ -30,7 +31,7 @@ def request_token post '/oauth/token', params: { grant_type: 'password', client_id: issuer_app.uid, client_secret: issuer_app.plaintext_secret, username: user.login, password: 'hunter2hunter2' } expect(response).to have_http_status(200) - expect(JSON.parse(response.body)).not_to have_key('expires_in') + expect(response.parsed_body).not_to have_key('expires_in') end end @@ -44,76 +45,47 @@ def request_token end it 'keeps similarly named diagnostics visible' do - params = { exit_code: 1, return_code: 2, token_count: 3 } + params = { exit_code: 1, return_code: 2 } expect(filter.filter(params)).to eq(params) end end describe 'rate limiting' do - let(:app) { Rack::Attack.new(->(_env) { [200, {}, ['ok']] }) } + include_context 'with Rack::Attack' + let(:safe_headers) { { 'HTTP_RATELIMIT_PROTECTION_SAFE_API_KEY' => 'safe-key' } } around :each do |example| - Rack::Attack.enabled = true - Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new ENV['RACK_ATTACK_SAFE_API_KEY'] = 'safe-key' example.run ensure - Rack::Attack.enabled = false ENV.delete('RACK_ATTACK_SAFE_API_KEY') end - def status_after(times, path, headers = {}) - status = nil - times.times { status, = app.call(Rack::MockRequest.env_for(path, { method: 'POST', 'REMOTE_ADDR' => '203.0.113.7' }.merge(headers))) } - status + 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)).to eq(200) + 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)).to eq(200) + expect(status_after(25, '/oauth/token', safe_headers)).not_to eq(503) end it 'throttles unsafelisted oauth token requests' do - expect(status_after(100, '/oauth/token')).to eq(200) + expect(status_after(20, '/oauth/token')).not_to eq(503) expect(status_after(1, '/oauth/token')).to eq(503) end - end - describe 'Rack::Attack in the real stack' do - around :each do |example| - Rack::Attack.enabled = true - Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new - example.run - ensure - Rack::Attack.enabled = false - end - - def hit_token_endpoint(times, env) - times.times { post '/oauth/token', env: env } - end - - it 'is in the middleware stack exactly once' do - expect(Rails.application.middleware.count { |m| m.klass == Rack::Attack }).to eq(1) - end - - it 'throttles by client address' do - hit_token_endpoint(101, 'REMOTE_ADDR' => '203.0.113.7') - - expect(response).to have_http_status(503) - end it 'buckets by the forwarded client address behind a private-range proxy' do proxied = { 'REMOTE_ADDR' => '10.0.0.1' } - hit_token_endpoint(101, proxied.merge('HTTP_X_FORWARDED_FOR' => '203.0.113.9')) - expect(response).to have_http_status(503) - - hit_token_endpoint(1, proxied.merge('HTTP_X_FORWARDED_FOR' => '203.0.113.10')) - expect(response).not_to have_http_status(503) + 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 index b60663eb3..a75d9af04 100644 --- a/spec/requests/oauth_token_info_spec.rb +++ b/spec/requests/oauth_token_info_spec.rb @@ -1,3 +1,4 @@ +# typed: false # frozen_string_literal: true require 'spec_helper' @@ -16,7 +17,7 @@ def token_info(bearer) token_info(token.token) expect(response).to have_http_status(200) - body = JSON.parse(response.body) + 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]) diff --git a/spec/requests/rack_attack_throttles_spec.rb b/spec/requests/rack_attack_throttles_spec.rb index 3f0a65450..edad03762 100644 --- a/spec/requests/rack_attack_throttles_spec.rb +++ b/spec/requests/rack_attack_throttles_spec.rb @@ -1,21 +1,24 @@ +# typed: false # frozen_string_literal: true require 'spec_helper' describe 'Rack::Attack throttles', type: :request do - let(:app) { Rack::Attack.new(->(_env) { [200, {}, ['ok']] }) } + include_context 'with Rack::Attack' + let(:safe_headers) { { 'HTTP_RATELIMIT_PROTECTION_SAFE_API_KEY' => 'safe-key' } } around :each do |example| - Rack::Attack.enabled = true - Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new ENV['RACK_ATTACK_SAFE_API_KEY'] = 'safe-key' example.run ensure - Rack::Attack.enabled = false ENV.delete('RACK_ATTACK_SAFE_API_KEY') end + def 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. @@ -30,15 +33,11 @@ def status_after(times, path, **options) status end - def basic(username, password = 'guess') - { 'HTTP_AUTHORIZATION' => ActionController::HttpAuthentication::Basic.encode_credentials(username, password) } - end - describe 'path variants' do { - '/oauth/token' => 100, - '/sessions' => 100, - '/api/v8/users' => 60, + '/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 @@ -59,40 +58,6 @@ def basic(username, password = 'guess') end end - describe 'Basic auth and /auth password checks per IP' do - it 'throttles Basic auth on web URLs' do - 300.times { |i| call('/courses', method: 'GET', **basic("user#{i}")) } - - expect(call('/courses', method: 'GET', **basic('fresh-user'))).to eq(503) - end - - it 'throttles /auth password checks' do - 300.times { |i| call('/auth', method: 'GET', params: { username: "user#{i}", password: 'x' }) } - - expect(call('/auth.text', params: { username: 'fresh-user', password: 'x' })).to eq(503) - end - - it 'counts query parameters when a JSON body is empty' do - 300.times { |i| call("/auth?username=user#{i}&password=x", method: 'GET', 'CONTENT_TYPE' => 'application/json') } - - expect(call('/auth?username=fresh-user&password=x', method: 'GET', 'CONTENT_TYPE' => 'application/json')).to eq(503) - end - - it 'ignores client credentials sent as Basic auth to Doorkeeper' do - expect(status_after(301, '/oauth/introspect', **basic('client-id'))).to eq(200) - end - - it 'ignores /auth session checks' do - expect(status_after(301, '/auth.text', method: 'GET', params: { username: 'victim', session_id: 'abc' })).to eq(200) - end - - it 'leaves the login bucket of the address alone' do - 300.times { |i| call('/courses', method: 'GET', **basic("user#{i}")) } - - expect(call('/sessions', params: { session: { login: 'fresh-user', password: 'x' } })).to eq(200) - end - end - describe 'user access tokens per target user' do let(:path) { '/api/v8/application/user_access_tokens' } let(:issuer_app) { Doorkeeper::Application.create!(name: 'mooc.fi backend', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob', scopes: 'issue_user_access_tokens') } diff --git a/spec/requests/secret_path_logging_spec.rb b/spec/requests/secret_path_logging_spec.rb index e9fad6c29..b3279cd16 100644 --- a/spec/requests/secret_path_logging_spec.rb +++ b/spec/requests/secret_path_logging_spec.rb @@ -1,3 +1,4 @@ +# typed: false # frozen_string_literal: true require 'spec_helper' @@ -47,7 +48,7 @@ def logged_path_and_params(&block) end expect(response).to redirect_to("http://www.example.com/users/#{user.id}/destroy/#{token.token}") - expect(payloads.map { |payload| payload[:location] }).to eq(['[FILTERED]']) + expect(payloads.pluck(:location)).to eq(['[FILTERED]']) end it 'filters the token from Rack::Attack log lines' do From 62f85eedfe195cf8b7f051c3d88787af4c7a2198 Mon Sep 17 00:00:00 2001 From: Henrik Nygren Date: Wed, 11 Nov 2026 13:23:35 +0200 Subject: [PATCH 12/18] Throttle token issues whose user_id cannot be read --- config/initializers/rack_attack.rb | 11 +++++--- spec/requests/issuer_token_hardening_spec.rb | 27 ++++++++++++++++++++ spec/requests/rack_attack_throttles_spec.rb | 7 +++++ 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index bec4df790..465be8dad 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -60,16 +60,19 @@ def self.user_identifier_discriminator(access_token) # 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) - user_id = rails_param(req, 'user_id') token_string = access_token_string(req) - return unless user_id&.match?(/\A\d+\z/) && token_string + token = token_string && Doorkeeper::AccessToken.by_token(token_string) + return unless token && AccessTokenAuthentication.issuer_token?(token) - token = Doorkeeper::AccessToken.by_token(token_string) - "#{token.application_id}:#{user_id.to_i}" if 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 diff --git a/spec/requests/issuer_token_hardening_spec.rb b/spec/requests/issuer_token_hardening_spec.rb index aa6daf9fa..c4018b8de 100644 --- a/spec/requests/issuer_token_hardening_spec.rb +++ b/spec/requests/issuer_token_hardening_spec.rb @@ -82,6 +82,33 @@ def status_after(times, path, env = {}) 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) diff --git a/spec/requests/rack_attack_throttles_spec.rb b/spec/requests/rack_attack_throttles_spec.rb index edad03762..91ba83384 100644 --- a/spec/requests/rack_attack_throttles_spec.rb +++ b/spec/requests/rack_attack_throttles_spec.rb @@ -78,6 +78,13 @@ def status_after(times, path, **options) 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' }) } From 96033fb43a9cab5963dc1409c21a39fd11c2dbdc Mon Sep 17 00:00:00 2001 From: Henrik Nygren Date: Wed, 11 Nov 2026 13:23:38 +0200 Subject: [PATCH 13/18] Answer revoked and expired tokens with 401 on API v8 --- app/controllers/api/v8/base_controller.rb | 2 + app/controllers/application_controller.rb | 9 ++- .../api/v8/base_controller_spec.rb | 20 ------ .../api_v8_token_authentication_spec.rb | 67 +++++++++++++++++++ 4 files changed, 77 insertions(+), 21 deletions(-) create mode 100644 spec/requests/api_v8_token_authentication_spec.rb diff --git a/app/controllers/api/v8/base_controller.rb b/app/controllers/api/v8/base_controller.rb index 1dd20bbe3..679b450e4 100644 --- a/app/controllers/api/v8/base_controller.rb +++ b/app/controllers/api/v8/base_controller.rb @@ -53,6 +53,8 @@ def authenticate_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/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/spec/controllers/api/v8/base_controller_spec.rb b/spec/controllers/api/v8/base_controller_spec.rb index fe26d0b6e..701677a7f 100644 --- a/spec/controllers/api/v8/base_controller_spec.rb +++ b/spec/controllers/api/v8/base_controller_spec.rb @@ -45,26 +45,6 @@ def index expect(response).to have_http_status(401) end end - context 'with a revoked token' do - let(:user) { FactoryBot.create(:user) } - let(:application) { Doorkeeper::Application.create!(name: 'client', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob') } - let(:token) { Doorkeeper::AccessToken.create!(application: application, resource_owner_id: user.id, scopes: 'public').tap(&:revoke) } - it 'request is not successful' do - request.accept = 'application/json' - get :index - expect(response).to have_http_status(401) - expect(response.parsed_body).to eq('error' => 'Invalid token') - end - end - context 'with an expired token' do - let(:user) { FactoryBot.create(:user) } - let(:application) { Doorkeeper::Application.create!(name: 'client', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob') } - let(:token) { Doorkeeper::AccessToken.create!(application: application, resource_owner_id: user.id, scopes: 'public', expires_in: 60, created_at: 2.minutes.ago) } - it 'request is not successful' do - get :index - expect(response).to have_http_status(401) - end - end context 'with a handoff token' do let(:user) { FactoryBot.create(:user) } let(:application) { Doorkeeper::Application.create!(name: 'client', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob') } 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 From 4ffddccfe3ab862afc7c89e27d4936cd459cc71a Mon Sep 17 00:00:00 2001 From: Henrik Nygren Date: Wed, 11 Nov 2026 13:23:42 +0200 Subject: [PATCH 14/18] Revoke handoff tokens when their user becomes privileged --- app/models/assistantship.rb | 10 +++++++ app/models/teachership.rb | 10 +++++++ app/models/user.rb | 6 +++++ app/services/access_token_authentication.rb | 10 +++++++ spec/requests/oauth_token_info_spec.rb | 30 +++++++++++++++++++++ 5 files changed, 66 insertions(+) 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 ee7adc938..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) @@ -662,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 index 6f8f4ed29..5e3bc717b 100644 --- a/app/services/access_token_authentication.rb +++ b/app/services/access_token_authentication.rb @@ -20,6 +20,16 @@ def self.user_for(token) 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) } diff --git a/spec/requests/oauth_token_info_spec.rb b/spec/requests/oauth_token_info_spec.rb index a75d9af04..b91fabcd9 100644 --- a/spec/requests/oauth_token_info_spec.rb +++ b/spec/requests/oauth_token_info_spec.rb @@ -31,6 +31,36 @@ def token_info(bearer) 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 From 92d68b48aa53567b85399ea9a5af5bdd764ce608 Mon Sep 17 00:00:00 2001 From: Henrik Nygren Date: Wed, 11 Nov 2026 13:23:45 +0200 Subject: [PATCH 15/18] Delete access tokens a month after they expire or are revoked --- .../access_token_cleanup_task.rb | 29 +++++++++++++++++++ .../access_token_cleanup_task_spec.rb | 26 +++++++++++++++++ 2 files changed, 55 insertions(+) create mode 100644 app/background_tasks/access_token_cleanup_task.rb create mode 100644 spec/background_tasks/access_token_cleanup_task_spec.rb 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/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 From e27b268acaf5f9672a58850c49140ffda669fe1e Mon Sep 17 00:00:00 2001 From: Henrik Nygren Date: Wed, 11 Nov 2026 13:23:48 +0200 Subject: [PATCH 16/18] Log refused user access token issues --- .../user_access_tokens_controller.rb | 27 +++++++++++++++---- .../user_access_tokens_controller_spec.rb | 7 ++++- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/app/controllers/api/v8/application/user_access_tokens_controller.rb b/app/controllers/api/v8/application/user_access_tokens_controller.rb index 7cd5ca983..50d3a5301 100644 --- a/app/controllers/api/v8/application/user_access_tokens_controller.rb +++ b/app/controllers/api/v8/application/user_access_tokens_controller.rb @@ -60,13 +60,25 @@ class UserAccessTokensController < Api::V8::BaseController def create issuer_token = doorkeeper_token return respond_unauthorized unless issuer_token&.accessible? - return respond_forbidden unless authorized_issuer?(issuer_token) + + 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 = User.find_by(id: params[:user_id].to_i) - return respond_not_found('User not found') unless user - return respond_forbidden('Tokens are not issued for administrators, teachers or assistants') if user.privileged? + 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 @@ -77,7 +89,7 @@ def create scopes: "public #{AccessTokenAuthentication::HANDOFF_SCOPE}", expires_in: TOKEN_LIFETIME.to_i ) - Rails.logger.info("Issued user access token: issuer application uid=#{issuer_token.application&.uid} user_id=#{user.id}") + 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' @@ -91,6 +103,11 @@ def create 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 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 index 8db72b47c..2ad9f7a82 100644 --- a/spec/controllers/api/v8/application/user_access_tokens_controller_spec.rb +++ b/spec/controllers/api/v8/application/user_access_tokens_controller_spec.rb @@ -20,6 +20,7 @@ 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 @@ -72,6 +73,7 @@ def post_with(token, user_id) 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 @@ -112,9 +114,11 @@ def post_with(token, user_id) end it 'returns 404 for an unknown user' do - post_with(issuer_token, User.maximum(:id) + 1) + 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 @@ -123,6 +127,7 @@ def post_with(token, user_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 From b1b787c5074a44f152e71c874f55278148f3c6b1 Mon Sep 17 00:00:00 2001 From: Henrik Nygren Date: Wed, 11 Nov 2026 13:23:51 +0200 Subject: [PATCH 17/18] Test that handoff tokens cannot request account deletion --- .../api/v8/users/request_deletion_controller_spec.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) 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 From 488d4b050141fb9043946e0dde79cca7f811cd57 Mon Sep 17 00:00:00 2001 From: Henrik Nygren Date: Wed, 11 Nov 2026 13:38:31 +0200 Subject: [PATCH 18/18] Run request, service and background task specs in CI --- .github/workflows/ci.yml | 18 ++++++++++++++++++ bin/spec_requests | 4 ++++ 2 files changed, 22 insertions(+) create mode 100755 bin/spec_requests 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/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"