Issue short-lived user access tokens to trusted backends

.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:

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

app/controllers/api/beta/base_controller.rb

@@ -36,7 +36,7 @@ def present(hash)
     sig { void }
     def authenticate_user!
       if (token = doorkeeper_token)
-        @current_user = T.let(User.find_by(id: token.resource_owner_id), T.nilable(User))
+        @current_user = T.let(AccessTokenAuthentication.user_for(token), T.nilable(User))
       end
 
       return if @current_user

app/controllers/api/v8/apidocs_controller.rb

@@ -190,6 +190,7 @@ class ApidocsController < ActionController::Base
         Api::V8::Core::SubmissionsController,
         Api::V8::Core::Submissions::FeedbackController,
         Api::V8::Core::Submissions::ReviewsController,
+        Api::V8::Application::UserAccessTokensController,
         AvailablePoint,
         AwardedPoint,
         Course,

app/controllers/api/v8/application/user_access_tokens_controller.rb

@@ -0,0 +1,137 @@
+# typed: strict
+# frozen_string_literal: true
+
+module Api
+  module V8
+    module Application
+      # Issues a login token for a user to a trusted backend after a sign-in handoff verified
+      # elsewhere. Callable only by allow-listed client_credentials apps with the issuer scope.
+      class UserAccessTokensController < Api::V8::BaseController
+        extend T::Sig
+        include Swagger::Blocks
+
+        TOKEN_LIFETIME = T.let(1.day, ActiveSupport::Duration)
+
+        swagger_path '/api/v8/application/user_access_tokens' do
+          operation :post do
+            key :description, "Issues an access token for the given user, owned by the configured target application. Requires a client_credentials token of an allow-listed application with the #{AccessTokenAuthentication::ISSUER_SCOPE} scope. Administrators, teachers and assistants are refused. The token cannot change the user's email or password or delete the account. It expires after #{TOKEN_LIFETIME.to_i} seconds."
+            key :operationId, 'issueUserAccessToken'
+            key :produces, ['application/json']
+            key :tags, ['user']
+            parameter do
+              key :name, :user_id
+              key :in, :body
+              key :description, "The user's id"
+              key :required, true
+              schema do
+                key :'$ref', :UserAccessTokenInput
+              end
+            end
+            response 200 do
+              key :description, 'The issued access token'
+              schema do
+                key :'$ref', :UserAccessToken
+              end
+            end
+            response 400, '$ref': '#/responses/error'
+            response 401, '$ref': '#/responses/error'
+            response 403, '$ref': '#/responses/error'
+            response 404, '$ref': '#/responses/error'
+          end
+        end
+
+        swagger_schema :UserAccessTokenInput do
+          key :required, [:user_id]
+          property :user_id, type: :integer, example: 1
+        end
+
+        swagger_schema :UserAccessToken do
+          key :required, %i[access_token token_type created_at expires_in]
+          property :access_token, type: :string
+          property :token_type, type: :string, example: 'bearer'
+          property :created_at, type: :integer, description: 'Unix timestamp'
+          property :expires_in, type: :integer, description: 'Seconds from created_at', example: 86_400
+        end
+
+        skip_before_action :authenticate_user!
+        skip_authorization_check
+
+        sig { void }
+        def create
+          issuer_token = doorkeeper_token
+          return respond_unauthorized unless issuer_token&.accessible?
+
+          issuer_uid = issuer_token.application&.uid
+          unless authorized_issuer?(issuer_token)
+            log_refusal('not an allowed issuer', issuer_uid)
+            return respond_forbidden
+          end
+
+          return respond_with_error('user_id must be an integer', 400) unless params[:user_id].to_s.match?(/\A\d+\z/)
+
+          user_id = params[:user_id].to_i
+          user = User.find_by(id: user_id)
+          unless user
+            log_refusal('unknown user', issuer_uid, user_id)
+            return respond_not_found('User not found')
+          end
+          if user.privileged?
+            log_refusal('privileged user', issuer_uid, user_id)
+            return respond_forbidden('Tokens are not issued for administrators, teachers or assistants')
+          end
+
+          target_application = Doorkeeper::Application.find_by(uid: AccessTokenAuthentication.target_application_uid)
+          return respond_with_error('Target application is not configured', 500) unless target_application
+
+          token = Doorkeeper::AccessToken.create_for(
+            application: target_application,
+            resource_owner: user,
+            scopes: "public #{AccessTokenAuthentication::HANDOFF_SCOPE}",
+            expires_in: TOKEN_LIFETIME.to_i
+          )
+          Rails.logger.info("Issued user access token: issuer application uid=#{issuer_uid} user_id=#{user.id}")
+
+          response.headers['Cache-Control'] = 'no-store'
+          response.headers['Pragma'] = 'no-cache'
+
+          render json: {
+            access_token: token.plaintext_token,
+            token_type: 'bearer',
+            created_at: token.created_at.to_i,
+            expires_in: token.expires_in
+          }
+        end
+
+        private
+          sig { params(reason: String, issuer_uid: T.nilable(String), user_id: T.nilable(Integer)).void }
+          def log_refusal(reason, issuer_uid, user_id = nil)
+            Rails.logger.warn("Refused to issue user access token (#{reason}): issuer application uid=#{issuer_uid} user_id=#{user_id}")
+          end
+
+          # Their credentials are served publicly by the credentials endpoint.
+          sig { returns(T::Array[String]) }
+          def public_client_names
+            clients = SiteSetting.value('valid_clients')
+            return [] unless clients.is_a?(Array)
+
+            clients.filter_map { |client| client['name'].to_s if client.is_a?(Hash) }
+          end
+
+          sig { params(issuer_token: Doorkeeper::AccessToken).returns(T::Boolean) }
+          def authorized_issuer?(issuer_token)
+            # resource_owner_id is set on user tokens, which never qualify even from an allowed application.
+            return false unless AccessTokenAuthentication.issuer_token?(issuer_token)
+
+            application = issuer_token.application
+            return false unless application
+
+            # The target is a frontend client, so its credentials must not be usable to issue tokens.
+            AccessTokenAuthentication.issuer_application_uids.include?(application.uid) &&
+              application.confidential? &&
+              application.uid != AccessTokenAuthentication.target_application_uid &&
+              public_client_names.exclude?(application.name)
+          end
+      end
+    end
+  end
+end

app/controllers/api/v8/base_controller.rb

@@ -50,8 +50,11 @@ def present(hash)
         def authenticate_user!
           return if @current_user
           if (token = doorkeeper_token)
-            @current_user = T.let(User.find_by(id: token.resource_owner_id), T.nilable(User))
-            raise 'Invalid token' unless @current_user
+            @current_user = T.let(AccessTokenAuthentication.user_for(token), T.nilable(User))
+            return respond_unauthorized('Invalid token') unless @current_user
+            @auth_source = T.let(:handoff_token, T.nilable(Symbol)) if AccessTokenAuthentication.handoff?(token)
+          elsif presented_doorkeeper_token
+            return respond_unauthorized('Invalid token')
           elsif Rails.configuration.x.accept_courses_mooc_fi_tokens
             @current_user = CoursesMoocFiAuthentication.user_for(request)
             @auth_source = T.let(:courses_mooc_fi_token, T.nilable(Symbol)) if @current_user

app/controllers/api/v8/users_controller.rb

@@ -325,6 +325,7 @@ def update_email(user)
           if new_email.blank?
             user.errors.add(:email, 'needed')
           elsif user.email.casecmp(new_email) != 0
+            authorize! :update_credentials, user
             user.email = new_email
             user.email_verified = false
             UserMailer.email_confirmation(user, params[:origin], params[:language]).deliver_now
@@ -348,6 +349,7 @@ def set_password(user)
         sig { params(user: User).void }
         def maybe_update_password(user)
           if params[:old_password].present? && params[:password].present?
+            authorize! :update_credentials, user
             if user.managed_externally?
               unless user.update_password_via_courses_mooc_fi(params[:old_password], params[:password])
                 user.errors.add(:password, 'could not be changed. Please check your current password and try again.')

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

app/controllers/auths_controller.rb

@@ -13,10 +13,9 @@ class AuthsController < ApplicationController
   sig { void }
   def show
     if params[:username].present? && params[:session_id].present?
-      user = User.find_by(login: params[:username])
-      user ||= User.find_by('lower(email) = ?', params[:username].downcase)
+      user = User.by_login_or_email(params[:username].to_s)
       # Allows using oauth2 tokens of the new api for authenticating
-      res = if user && Doorkeeper::AccessToken.find_by(resource_owner_id: user.id, token: params[:session_id])&.accessible?
+      res = if user && AccessTokenAuthentication.user_for(Doorkeeper::AccessToken.by_token(params[:session_id].to_s)) == user
         OK_MESSAGE
       elsif user && find_session_by_id(params[:session_id])&.belongs_to?(user)
         OK_MESSAGE

app/controllers/users_controller.rb

@@ -109,7 +109,7 @@ def update
   sig { void }
   def confirm_email
     language = params[:language]
-    VerificationToken.unscoped.email.find_by!(user_id: params[:user_id], token: params[:id])
+    VerificationToken.unscoped.email.find_by!(user_id: params[:user_id], token: params[:token])
     redirect_path = root_url
     redirect_path = "https://course.elementsofai.com/#{language && language != "en" ? "#{language}/" : ''}email-verification" if params[:origin] && params[:origin].start_with?('elements_of_ai')
     redirect_path = "https://buildingai.elementsofai.com/#{language && language != "en" ? "#{language}/" : ''}email-verified" if params[:origin] && params[:origin].start_with?('building_ai')
@@ -130,7 +130,7 @@ def send_verification_email
   sig { void }
   def verify_destroying_user
     @user = authenticate_current_user_destroy
-    VerificationToken.delete_user.find_by!(user: @user, token: params[:id])
+    VerificationToken.delete_user.find_by!(user: @user, token: params[:token])
   end
 
   sig { void }
@@ -146,7 +146,7 @@ def destroy_user
       redirect_to verify_destroying_user_url, alert: failed_authentication_alert(status)
       return
     end
-    VerificationToken.delete_user.find_by!(user: user, token: params[:id])
+    VerificationToken.delete_user.find_by!(user: user, token: params[:token])
     sign_out if current_user == user
     email = user.email
     username = user.login

app/models/ability.rb

@@ -8,10 +8,13 @@ class Ability
   extend T::Sig
   include CanCan::Ability
 
-  # +auth_source+ is :courses_mooc_fi_token when the user authenticated with a courses.mooc.fi
-  # access token; account changes are then denied to everyone, administrators included.
+  # +auth_source+ restricts account changes for tokens that are not the user's own login:
+  # :courses_mooc_fi_token denies all, administrators included; :handoff_token allows profile
+  # edits but no +:update_credentials+ (email, password) and no account deletion.
   sig { params(user: User, auth_source: T.nilable(Symbol)).void }
   def initialize(user, auth_source: nil)
+    alias_action :update_credentials, to: :update
+
     if user.administrator?
       can :manage, :all
       can :create, Course
@@ -294,10 +297,13 @@ def initialize(user, auth_source: nil)
       end
     end
 
-    return unless auth_source == :courses_mooc_fi_token
-
-    # The token is scoped to exercise services; it must not take over or delete the account.
-    cannot :update, User
-    cannot :destroy, User
+    case auth_source
+    when :courses_mooc_fi_token
+      # The token is scoped to exercise services; it must not take over or delete the account.
+      cannot :update, User
+      cannot :destroy, User
+    when :handoff_token
+      cannot %i[update_credentials destroy], User
+    end
   end
 end

app/models/assistantship.rb

@@ -2,11 +2,21 @@
 # frozen_string_literal: true
 
 class Assistantship < ApplicationRecord
+  extend T::Sig
+
   belongs_to :user
   belongs_to :course
 
   validates :user,
             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

app/models/teachership.rb

@@ -2,11 +2,21 @@
 # frozen_string_literal: true
 
 class Teachership < ApplicationRecord
+  extend T::Sig
+
   belongs_to :user
   belongs_to :organization
 
   validates :user,
             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