From ed7dc1704dc3ce82567d9aac366b095f02ce181f Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sun, 25 Jun 2017 23:51:32 +0200 Subject: [PATCH] Bind web UI access tokens to sessions (#3940) * Add overview of active sessions * Better display of browser/platform name * Improve how browser information is stored and displayed for sessions overview * Fix test * Fix #2347 - Bind web UI access token to session When you logout, session also destroys the access token, so it's no longer valid. If access token is destroyed some other way, the session is also destroyed, requiring a re-login. Fix #1681 - Add scheduler to remove revoked access tokens and grants * Fix test --- app/controllers/application_controller.rb | 5 +++ app/controllers/home_controller.rb | 12 +---- app/models/session_activation.rb | 44 ++++++++++++++----- .../scheduler/doorkeeper_cleanup_scheduler.rb | 11 +++++ config/sidekiq.yml | 3 ++ ..._access_token_id_to_session_activations.rb | 6 +++ db/schema.rb | 4 +- 7 files changed, 63 insertions(+), 22 deletions(-) create mode 100644 app/workers/scheduler/doorkeeper_cleanup_scheduler.rb create mode 100644 db/migrate/20170625140443_add_access_token_id_to_session_activations.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 9cb397aa829..865fcd12576 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -11,6 +11,7 @@ class ApplicationController < ActionController::Base include UserTrackingConcern helper_method :current_account + helper_method :current_session helper_method :single_user_mode? rescue_from ActionController::RoutingError, with: :not_found @@ -68,6 +69,10 @@ class ApplicationController < ActionController::Base @current_account ||= current_user.try(:account) end + def current_session + @current_session ||= SessionActivation.find_by(session_id: session['auth_id']) + end + def cache_collection(raw, klass) return raw unless klass.respond_to?(:with_includes) diff --git a/app/controllers/home_controller.rb b/app/controllers/home_controller.rb index 1d41892cddc..6209a3ae932 100644 --- a/app/controllers/home_controller.rb +++ b/app/controllers/home_controller.rb @@ -5,7 +5,7 @@ class HomeController < ApplicationController def index @body_classes = 'app-body' - @token = find_or_create_access_token.token + @token = current_session.token @web_settings = Web::Setting.find_by(user: current_user)&.data || {} @admin = Account.find_local(Setting.site_contact_username) @streaming_api_base_url = Rails.configuration.x.streaming_api_base_url @@ -16,14 +16,4 @@ class HomeController < ApplicationController def authenticate_user! redirect_to(single_user_mode? ? account_path(Account.first) : about_path) unless user_signed_in? end - - def find_or_create_access_token - Doorkeeper::AccessToken.find_or_create_for( - Doorkeeper::Application.where(superapp: true).first, - current_user.id, - Doorkeeper::OAuth::Scopes.from_string('read write follow'), - Doorkeeper.configuration.access_token_expires_in, - Doorkeeper.configuration.refresh_token_enabled? - ) - end end diff --git a/app/models/session_activation.rb b/app/models/session_activation.rb index 75339b5f75c..02a918e8ac5 100644 --- a/app/models/session_activation.rb +++ b/app/models/session_activation.rb @@ -3,16 +3,23 @@ # # Table name: session_activations # -# id :integer not null, primary key -# user_id :integer not null -# session_id :string not null -# created_at :datetime not null -# updated_at :datetime not null -# user_agent :string default(""), not null -# ip :inet +# id :integer not null, primary key +# user_id :integer not null +# session_id :string not null +# created_at :datetime not null +# updated_at :datetime not null +# user_agent :string default(""), not null +# ip :inet +# access_token_id :integer # class SessionActivation < ApplicationRecord + belongs_to :access_token, class_name: 'Doorkeeper::AccessToken', dependent: :destroy + + delegate :token, + to: :access_token, + allow_nil: true + def detection @detection ||= Browser.new(user_agent) end @@ -25,9 +32,8 @@ class SessionActivation < ApplicationRecord detection.platform.id end - before_save do - self.user_agent = '' if user_agent.nil? - end + before_create :assign_access_token + before_save :assign_user_agent class << self def active?(id) @@ -53,4 +59,22 @@ class SessionActivation < ApplicationRecord where('session_id != ?', id).destroy_all end end + + private + + def assign_user_agent + self.user_agent = '' if user_agent.nil? + end + + def assign_access_token + superapp = Doorkeeper::Application.find_by(superapp: true) + + return if superapp.nil? + + self.access_token = Doorkeeper::AccessToken.create!(application_id: superapp.id, + resource_owner_id: user_id, + scopes: 'read write follow', + expires_in: Doorkeeper.configuration.access_token_expires_in, + use_refresh_token: Doorkeeper.configuration.refresh_token_enabled?) + end end diff --git a/app/workers/scheduler/doorkeeper_cleanup_scheduler.rb b/app/workers/scheduler/doorkeeper_cleanup_scheduler.rb new file mode 100644 index 00000000000..6488798cd8d --- /dev/null +++ b/app/workers/scheduler/doorkeeper_cleanup_scheduler.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true +require 'sidekiq-scheduler' + +class Scheduler::DoorkeeperCleanupScheduler + include Sidekiq::Worker + + def perform + Doorkeeper::AccessToken.where('revoked_at IS NOT NULL').where('revoked_at < NOW()').delete_all + Doorkeeper::AccessGrant.where('revoked_at IS NOT NULL').where('revoked_at < NOW()').delete_all + end +end diff --git a/config/sidekiq.yml b/config/sidekiq.yml index 6ed0aa4b54c..78aaa311cf3 100644 --- a/config/sidekiq.yml +++ b/config/sidekiq.yml @@ -15,3 +15,6 @@ feed_cleanup_scheduler: cron: '0 0 * * *' class: Scheduler::FeedCleanupScheduler + doorkeeper_cleanup_scheduler: + cron: '1 1 * * 0' + class: Scheduler::DoorkeeperCleanupScheduler diff --git a/db/migrate/20170625140443_add_access_token_id_to_session_activations.rb b/db/migrate/20170625140443_add_access_token_id_to_session_activations.rb new file mode 100644 index 00000000000..213a77a83da --- /dev/null +++ b/db/migrate/20170625140443_add_access_token_id_to_session_activations.rb @@ -0,0 +1,6 @@ +class AddAccessTokenIdToSessionActivations < ActiveRecord::Migration[5.1] + def change + add_column :session_activations, :access_token_id, :integer + add_foreign_key :session_activations, :oauth_access_tokens, column: :access_token_id, on_delete: :cascade + end +end diff --git a/db/schema.rb b/db/schema.rb index 1e7d6c0b337..159704c6a18 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170624134742) do +ActiveRecord::Schema.define(version: 20170625140443) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -257,6 +257,7 @@ ActiveRecord::Schema.define(version: 20170624134742) do t.datetime "updated_at", null: false t.string "user_agent", default: "", null: false t.inet "ip" + t.integer "access_token_id" t.index ["session_id"], name: "index_session_activations_on_session_id", unique: true t.index ["user_id"], name: "index_session_activations_on_user_id" end @@ -406,6 +407,7 @@ ActiveRecord::Schema.define(version: 20170624134742) do add_foreign_key "reports", "accounts", column: "action_taken_by_account_id", on_delete: :nullify add_foreign_key "reports", "accounts", column: "target_account_id", on_delete: :cascade add_foreign_key "reports", "accounts", on_delete: :cascade + add_foreign_key "session_activations", "oauth_access_tokens", column: "access_token_id", on_delete: :cascade add_foreign_key "session_activations", "users", on_delete: :cascade add_foreign_key "statuses", "accounts", column: "in_reply_to_account_id", on_delete: :nullify add_foreign_key "statuses", "accounts", on_delete: :cascade