From b4fb766b23f4b50b51a366f55b451770ece3153a Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 11 May 2018 11:49:12 +0200 Subject: [PATCH] Add REST API for Web Push Notifications subscriptions (#7445) - POST /api/v1/push/subscription - PUT /api/v1/push/subscription - DELETE /api/v1/push/subscription - New OAuth scope: "push" (required for the above methods) --- .../api/v1/push/subscriptions_controller.rb | 50 +++++++++++ .../api/web/push_subscriptions_controller.rb | 11 +-- app/controllers/shares_controller.rb | 1 + app/models/user.rb | 2 +- app/models/web/push_subscription.rb | 47 +++++++---- app/serializers/initial_state_serializer.rb | 4 +- .../rest/web_push_subscription_serializer.rb | 13 +++ .../web/notification_serializer.rb | 2 +- app/services/notify_service.rb | 21 +++-- app/workers/web/push_notification_worker.rb | 18 ++++ app/workers/web_push_notification_worker.rb | 25 ------ config/initializers/doorkeeper.rb | 2 +- config/locales/doorkeeper.en.yml | 1 + config/routes.rb | 4 + ...cess_token_id_to_web_push_subscriptions.rb | 6 ++ ...10230049_migrate_web_push_subscriptions.rb | 13 +++ db/schema.rb | 8 +- .../v1/push/subscriptions_controller_spec.rb | 83 +++++++++++++++++++ .../web/push_subscriptions_controller_spec.rb | 16 ++-- spec/models/web/push_subscription_spec.rb | 12 --- 20 files changed, 258 insertions(+), 81 deletions(-) create mode 100644 app/controllers/api/v1/push/subscriptions_controller.rb create mode 100644 app/serializers/rest/web_push_subscription_serializer.rb create mode 100644 app/workers/web/push_notification_worker.rb delete mode 100644 app/workers/web_push_notification_worker.rb create mode 100644 db/migrate/20180510214435_add_access_token_id_to_web_push_subscriptions.rb create mode 100644 db/migrate/20180510230049_migrate_web_push_subscriptions.rb create mode 100644 spec/controllers/api/v1/push/subscriptions_controller_spec.rb diff --git a/app/controllers/api/v1/push/subscriptions_controller.rb b/app/controllers/api/v1/push/subscriptions_controller.rb new file mode 100644 index 00000000000..5038cc03c72 --- /dev/null +++ b/app/controllers/api/v1/push/subscriptions_controller.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +class Api::V1::Push::SubscriptionsController < Api::BaseController + before_action -> { doorkeeper_authorize! :push } + before_action :require_user! + before_action :set_web_push_subscription + + def create + @web_subscription&.destroy! + + @web_subscription = ::Web::PushSubscription.create!( + endpoint: subscription_params[:endpoint], + key_p256dh: subscription_params[:keys][:p256dh], + key_auth: subscription_params[:keys][:auth], + data: data_params, + user_id: current_user.id, + access_token_id: doorkeeper_token.id + ) + + render json: @web_subscription, serializer: REST::WebPushSubscriptionSerializer + end + + def update + raise ActiveRecord::RecordNotFound if @web_subscription.nil? + + @web_subscription.update!(data: data_params) + + render json: @web_subscription, serializer: REST::WebPushSubscriptionSerializer + end + + def destroy + @web_subscription&.destroy! + render_empty + end + + private + + def set_web_push_subscription + @web_subscription = ::Web::PushSubscription.find_by(access_token_id: doorkeeper_token.id) + end + + def subscription_params + params.require(:subscription).permit(:endpoint, keys: [:auth, :p256dh]) + end + + def data_params + return {} if params[:data].blank? + params.require(:data).permit(alerts: [:follow, :favourite, :reblog, :mention]) + end +end diff --git a/app/controllers/api/web/push_subscriptions_controller.rb b/app/controllers/api/web/push_subscriptions_controller.rb index 249e7c1860d..fe8e4258088 100644 --- a/app/controllers/api/web/push_subscriptions_controller.rb +++ b/app/controllers/api/web/push_subscriptions_controller.rb @@ -31,22 +31,23 @@ class Api::Web::PushSubscriptionsController < Api::Web::BaseController endpoint: subscription_params[:endpoint], key_p256dh: subscription_params[:keys][:p256dh], key_auth: subscription_params[:keys][:auth], - data: data + data: data, + user_id: active_session.user_id, + access_token_id: active_session.access_token_id ) active_session.update!(web_push_subscription: web_subscription) - render json: web_subscription.as_payload + render json: web_subscription, serializer: REST::WebPushSubscriptionSerializer end def update params.require([:id]) web_subscription = ::Web::PushSubscription.find(params[:id]) - web_subscription.update!(data: data_params) - render json: web_subscription.as_payload + render json: web_subscription, serializer: REST::WebPushSubscriptionSerializer end private @@ -56,6 +57,6 @@ class Api::Web::PushSubscriptionsController < Api::Web::BaseController end def data_params - @data_params ||= params.require(:data).permit(:alerts) + @data_params ||= params.require(:data).permit(alerts: [:follow, :favourite, :reblog, :mention]) end end diff --git a/app/controllers/shares_controller.rb b/app/controllers/shares_controller.rb index 3ec831a72da..9ef1e07491d 100644 --- a/app/controllers/shares_controller.rb +++ b/app/controllers/shares_controller.rb @@ -15,6 +15,7 @@ class SharesController < ApplicationController def initial_state_params text = [params[:title], params[:text], params[:url]].compact.join(' ') + { settings: Web::Setting.find_by(user: current_user)&.data || {}, push_subscription: current_account.user.web_push_subscription(current_session), diff --git a/app/models/user.rb b/app/models/user.rb index f5f542f07d6..21c217e77d3 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -245,7 +245,7 @@ class User < ApplicationRecord end def web_push_subscription(session) - session.web_push_subscription.nil? ? nil : session.web_push_subscription.as_payload + session.web_push_subscription.nil? ? nil : session.web_push_subscription end def invite_code=(code) diff --git a/app/models/web/push_subscription.rb b/app/models/web/push_subscription.rb index 1736106f79a..df549c6d3d1 100644 --- a/app/models/web/push_subscription.rb +++ b/app/models/web/push_subscription.rb @@ -3,38 +3,51 @@ # # Table name: web_push_subscriptions # -# id :bigint(8) not null, primary key -# endpoint :string not null -# key_p256dh :string not null -# key_auth :string not null -# data :json -# created_at :datetime not null -# updated_at :datetime not null +# id :bigint(8) not null, primary key +# endpoint :string not null +# key_p256dh :string not null +# key_auth :string not null +# data :json +# created_at :datetime not null +# updated_at :datetime not null +# access_token_id :bigint(8) +# user_id :bigint(8) # -require 'webpush' - class Web::PushSubscription < ApplicationRecord + belongs_to :user, optional: true + belongs_to :access_token, class_name: 'Doorkeeper::AccessToken', optional: true + has_one :session_activation def push(notification) - I18n.with_locale(session_activation.user.locale || I18n.default_locale) do + I18n.with_locale(associated_user.locale || I18n.default_locale) do push_payload(message_from(notification), 48.hours.seconds) end end def pushable?(notification) - data&.key?('alerts') && data['alerts'][notification.type.to_s] + data&.key?('alerts') && ActiveModel::Type::Boolean.new.cast(data['alerts'][notification.type.to_s]) end - def as_payload - payload = { id: id, endpoint: endpoint } - payload[:alerts] = data['alerts'] if data&.key?('alerts') - payload + def associated_user + return @associated_user if defined?(@associated_user) + + @associated_user = if user_id.nil? + session_activation.user + else + user + end end - def access_token - find_or_create_access_token.token + def associated_access_token + return @associated_access_token if defined?(@associated_access_token) + + @associated_access_token = if access_token_id.nil? + find_or_create_access_token.token + else + access_token + end end private diff --git a/app/serializers/initial_state_serializer.rb b/app/serializers/initial_state_serializer.rb index 3b908e224e7..6c9fba2f5b5 100644 --- a/app/serializers/initial_state_serializer.rb +++ b/app/serializers/initial_state_serializer.rb @@ -2,7 +2,9 @@ class InitialStateSerializer < ActiveModel::Serializer attributes :meta, :compose, :accounts, - :media_attachments, :settings, :push_subscription + :media_attachments, :settings + + has_one :push_subscription, serializer: REST::WebPushSubscriptionSerializer def meta store = { diff --git a/app/serializers/rest/web_push_subscription_serializer.rb b/app/serializers/rest/web_push_subscription_serializer.rb new file mode 100644 index 00000000000..7fd952a567b --- /dev/null +++ b/app/serializers/rest/web_push_subscription_serializer.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class REST::WebPushSubscriptionSerializer < ActiveModel::Serializer + attributes :id, :endpoint, :alerts, :server_key + + def alerts + object.data&.dig('alerts') || {} + end + + def server_key + Rails.configuration.x.vapid_public_key + end +end diff --git a/app/serializers/web/notification_serializer.rb b/app/serializers/web/notification_serializer.rb index e5524fe7a7b..31c70383248 100644 --- a/app/serializers/web/notification_serializer.rb +++ b/app/serializers/web/notification_serializer.rb @@ -54,7 +54,7 @@ class Web::NotificationSerializer < ActiveModel::Serializer def access_token return if actions.empty? - current_push_subscription.access_token + current_push_subscription.associated_access_token end def message diff --git a/app/services/notify_service.rb b/app/services/notify_service.rb index ba086449cc1..6490d2735bc 100644 --- a/app/services/notify_service.rb +++ b/app/services/notify_service.rb @@ -9,6 +9,7 @@ class NotifyService < BaseService return if recipient.user.nil? || blocked? create_notification + push_notification if @notification.browserable? send_email if email_enabled? rescue ActiveRecord::RecordInvalid return @@ -101,25 +102,27 @@ class NotifyService < BaseService def create_notification @notification.save! - return unless @notification.browserable? + end + + def push_notification + return if @notification.activity.nil? + Redis.current.publish("timeline:#{@recipient.id}", Oj.dump(event: :notification, payload: InlineRenderer.render(@notification, @recipient, :notification))) send_push_notifications end def send_push_notifications - # HACK: Can be caused by quickly unfavouriting a status, since creating - # a favourite and creating a notification are not wrapped in a transaction. - return if @notification.activity.nil? + subscriptions_ids = ::Web::PushSubscription.where(user_id: @recipient.user.id) + .select { |subscription| subscription.pushable?(@notification) } + .map(&:id) - sessions_with_subscriptions = @recipient.user.session_activations.where.not(web_push_subscription: nil) - sessions_with_subscriptions_ids = sessions_with_subscriptions.select { |session| session.web_push_subscription.pushable? @notification }.map(&:id) - - WebPushNotificationWorker.push_bulk(sessions_with_subscriptions_ids) do |session_activation_id| - [session_activation_id, @notification.id] + ::Web::PushNotificationWorker.push_bulk(subscriptions_ids) do |subscription_id| + [subscription_id, @notification.id] end end def send_email + return if @notification.activity.nil? NotificationMailer.public_send(@notification.type, @recipient, @notification).deliver_later end diff --git a/app/workers/web/push_notification_worker.rb b/app/workers/web/push_notification_worker.rb new file mode 100644 index 00000000000..4a40e5c8bd9 --- /dev/null +++ b/app/workers/web/push_notification_worker.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class Web::PushNotificationWorker + include Sidekiq::Worker + + sidekiq_options backtrace: true + + def perform(subscription_id, notification_id) + subscription = ::Web::PushSubscription.find(subscription_id) + notification = Notification.find(notification_id) + + subscription.push(notification) unless notification.activity.nil? + rescue Webpush::InvalidSubscription, Webpush::ExpiredSubscription + subscription.destroy! + rescue ActiveRecord::RecordNotFound + true + end +end diff --git a/app/workers/web_push_notification_worker.rb b/app/workers/web_push_notification_worker.rb deleted file mode 100644 index eacea04c34a..00000000000 --- a/app/workers/web_push_notification_worker.rb +++ /dev/null @@ -1,25 +0,0 @@ -# frozen_string_literal: true - -class WebPushNotificationWorker - include Sidekiq::Worker - - sidekiq_options backtrace: true - - def perform(session_activation_id, notification_id) - session_activation = SessionActivation.find(session_activation_id) - notification = Notification.find(notification_id) - - return if session_activation.web_push_subscription.nil? || notification.activity.nil? - - session_activation.web_push_subscription.push(notification) - rescue Webpush::InvalidSubscription, Webpush::ExpiredSubscription - # Subscription expiration is not currently implemented in any browser - - session_activation.web_push_subscription.destroy! - session_activation.update!(web_push_subscription: nil) - - true - rescue ActiveRecord::RecordNotFound - true - end -end diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index 074f8c410be..4695538030f 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -55,7 +55,7 @@ Doorkeeper.configure do # For more information go to # https://github.com/doorkeeper-gem/doorkeeper/wiki/Using-Scopes default_scopes :read - optional_scopes :write, :follow + optional_scopes :write, :follow, :push # Change the way client credentials are retrieved from the request object. # By default it retrieves first from the `HTTP_AUTHORIZATION` header, then diff --git a/config/locales/doorkeeper.en.yml b/config/locales/doorkeeper.en.yml index 33d544bed5a..eca1fc675f1 100644 --- a/config/locales/doorkeeper.en.yml +++ b/config/locales/doorkeeper.en.yml @@ -115,5 +115,6 @@ en: title: OAuth authorization required scopes: follow: follow, block, unblock and unfollow accounts + push: receive push notifications for your account read: read your account's data write: post on your behalf diff --git a/config/routes.rb b/config/routes.rb index 4c920cf74f9..b7bd1a7ed56 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -306,6 +306,10 @@ Rails.application.routes.draw do resources :lists, only: [:index, :create, :show, :update, :destroy] do resource :accounts, only: [:show, :create, :destroy], controller: 'lists/accounts' end + + namespace :push do + resource :subscription, only: [:create, :update, :destroy] + end end namespace :web do diff --git a/db/migrate/20180510214435_add_access_token_id_to_web_push_subscriptions.rb b/db/migrate/20180510214435_add_access_token_id_to_web_push_subscriptions.rb new file mode 100644 index 00000000000..94ef8e0f591 --- /dev/null +++ b/db/migrate/20180510214435_add_access_token_id_to_web_push_subscriptions.rb @@ -0,0 +1,6 @@ +class AddAccessTokenIdToWebPushSubscriptions < ActiveRecord::Migration[5.2] + def change + add_reference :web_push_subscriptions, :access_token, null: true, default: nil, foreign_key: { on_delete: :cascade, to_table: :oauth_access_tokens }, index: false + add_reference :web_push_subscriptions, :user, null: true, default: nil, foreign_key: { on_delete: :cascade }, index: false + end +end diff --git a/db/migrate/20180510230049_migrate_web_push_subscriptions.rb b/db/migrate/20180510230049_migrate_web_push_subscriptions.rb new file mode 100644 index 00000000000..6de1bed795f --- /dev/null +++ b/db/migrate/20180510230049_migrate_web_push_subscriptions.rb @@ -0,0 +1,13 @@ +class MigrateWebPushSubscriptions < ActiveRecord::Migration[5.2] + disable_ddl_transaction! + + def up + add_index :web_push_subscriptions, :user_id, algorithm: :concurrently + add_index :web_push_subscriptions, :access_token_id, algorithm: :concurrently + end + + def down + remove_index :web_push_subscriptions, :user_id + remove_index :web_push_subscriptions, :access_token_id + end +end diff --git a/db/schema.rb b/db/schema.rb index f7fa24b835a..221c08d98e4 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: 2018_05_06_221944) do +ActiveRecord::Schema.define(version: 2018_05_10_230049) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -539,6 +539,10 @@ ActiveRecord::Schema.define(version: 2018_05_06_221944) do t.json "data" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.bigint "access_token_id" + t.bigint "user_id" + t.index ["access_token_id"], name: "index_web_push_subscriptions_on_access_token_id" + t.index ["user_id"], name: "index_web_push_subscriptions_on_user_id" end create_table "web_settings", force: :cascade do |t| @@ -605,5 +609,7 @@ ActiveRecord::Schema.define(version: 2018_05_06_221944) do add_foreign_key "subscriptions", "accounts", name: "fk_9847d1cbb5", on_delete: :cascade add_foreign_key "users", "accounts", name: "fk_50500f500d", on_delete: :cascade add_foreign_key "users", "invites", on_delete: :nullify + add_foreign_key "web_push_subscriptions", "oauth_access_tokens", column: "access_token_id", on_delete: :cascade + add_foreign_key "web_push_subscriptions", "users", on_delete: :cascade add_foreign_key "web_settings", "users", name: "fk_11910667b2", on_delete: :cascade end diff --git a/spec/controllers/api/v1/push/subscriptions_controller_spec.rb b/spec/controllers/api/v1/push/subscriptions_controller_spec.rb new file mode 100644 index 00000000000..01146294f80 --- /dev/null +++ b/spec/controllers/api/v1/push/subscriptions_controller_spec.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Api::V1::Push::SubscriptionsController do + render_views + + let(:user) { Fabricate(:user) } + let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'push') } + + before do + allow(controller).to receive(:doorkeeper_token) { token } + end + + let(:create_payload) do + { + subscription: { + endpoint: 'https://fcm.googleapis.com/fcm/send/fiuH06a27qE:APA91bHnSiGcLwdaxdyqVXNDR9w1NlztsHb6lyt5WDKOC_Z_Q8BlFxQoR8tWFSXUIDdkyw0EdvxTu63iqamSaqVSevW5LfoFwojws8XYDXv_NRRLH6vo2CdgiN4jgHv5VLt2A8ah6lUX', + keys: { + p256dh: 'BEm_a0bdPDhf0SOsrnB2-ategf1hHoCnpXgQsFj5JCkcoMrMt2WHoPfEYOYPzOIs9mZE8ZUaD7VA5vouy0kEkr8=', + auth: 'eH_C8rq2raXqlcBVDa1gLg==', + }, + } + }.with_indifferent_access + end + + let(:alerts_payload) do + { + data: { + alerts: { + follow: true, + favourite: false, + reblog: true, + mention: false, + } + } + }.with_indifferent_access + end + + describe 'POST #create' do + it 'saves push subscriptions' do + post :create, params: create_payload + + push_subscription = Web::PushSubscription.find_by(endpoint: create_payload[:subscription][:endpoint]) + + expect(push_subscription.endpoint).to eq(create_payload[:subscription][:endpoint]) + expect(push_subscription.key_p256dh).to eq(create_payload[:subscription][:keys][:p256dh]) + expect(push_subscription.key_auth).to eq(create_payload[:subscription][:keys][:auth]) + expect(push_subscription.user_id).to eq user.id + expect(push_subscription.access_token_id).to eq token.id + end + + it 'replaces old subscription on repeat calls' do + post :create, params: create_payload + post :create, params: create_payload + + expect(Web::PushSubscription.where(endpoint: create_payload[:subscription][:endpoint]).count).to eq 1 + end + end + + describe 'PUT #update' do + it 'changes alert settings' do + post :create, params: create_payload + put :update, params: alerts_payload + + push_subscription = Web::PushSubscription.find_by(endpoint: create_payload[:subscription][:endpoint]) + + expect(push_subscription.data.dig('alerts', 'follow')).to eq(alerts_payload[:data][:alerts][:follow].to_s) + expect(push_subscription.data.dig('alerts', 'favourite')).to eq(alerts_payload[:data][:alerts][:favourite].to_s) + expect(push_subscription.data.dig('alerts', 'reblog')).to eq(alerts_payload[:data][:alerts][:reblog].to_s) + expect(push_subscription.data.dig('alerts', 'mention')).to eq(alerts_payload[:data][:alerts][:mention].to_s) + end + end + + describe 'DELETE #destroy' do + it 'removes the subscription' do + post :create, params: create_payload + delete :destroy + + expect(Web::PushSubscription.find_by(endpoint: create_payload[:subscription][:endpoint])).to be_nil + end + end +end diff --git a/spec/controllers/api/web/push_subscriptions_controller_spec.rb b/spec/controllers/api/web/push_subscriptions_controller_spec.rb index bbf94c5c667..381cdeab949 100644 --- a/spec/controllers/api/web/push_subscriptions_controller_spec.rb +++ b/spec/controllers/api/web/push_subscriptions_controller_spec.rb @@ -59,10 +59,10 @@ describe Api::Web::PushSubscriptionsController do push_subscription = Web::PushSubscription.find_by(endpoint: create_payload[:subscription][:endpoint]) - expect(push_subscription.data['follow']).to eq(alerts_payload[:data][:follow]) - expect(push_subscription.data['favourite']).to eq(alerts_payload[:data][:favourite]) - expect(push_subscription.data['reblog']).to eq(alerts_payload[:data][:reblog]) - expect(push_subscription.data['mention']).to eq(alerts_payload[:data][:mention]) + expect(push_subscription.data['alerts']['follow']).to eq(alerts_payload[:data][:alerts][:follow].to_s) + expect(push_subscription.data['alerts']['favourite']).to eq(alerts_payload[:data][:alerts][:favourite].to_s) + expect(push_subscription.data['alerts']['reblog']).to eq(alerts_payload[:data][:alerts][:reblog].to_s) + expect(push_subscription.data['alerts']['mention']).to eq(alerts_payload[:data][:alerts][:mention].to_s) end end end @@ -81,10 +81,10 @@ describe Api::Web::PushSubscriptionsController do push_subscription = Web::PushSubscription.find_by(endpoint: create_payload[:subscription][:endpoint]) - expect(push_subscription.data['follow']).to eq(alerts_payload[:data][:follow]) - expect(push_subscription.data['favourite']).to eq(alerts_payload[:data][:favourite]) - expect(push_subscription.data['reblog']).to eq(alerts_payload[:data][:reblog]) - expect(push_subscription.data['mention']).to eq(alerts_payload[:data][:mention]) + expect(push_subscription.data['alerts']['follow']).to eq(alerts_payload[:data][:alerts][:follow].to_s) + expect(push_subscription.data['alerts']['favourite']).to eq(alerts_payload[:data][:alerts][:favourite].to_s) + expect(push_subscription.data['alerts']['reblog']).to eq(alerts_payload[:data][:alerts][:reblog].to_s) + expect(push_subscription.data['alerts']['mention']).to eq(alerts_payload[:data][:alerts][:mention].to_s) end end end diff --git a/spec/models/web/push_subscription_spec.rb b/spec/models/web/push_subscription_spec.rb index 574da55ac26..c6665611c6e 100644 --- a/spec/models/web/push_subscription_spec.rb +++ b/spec/models/web/push_subscription_spec.rb @@ -2,20 +2,8 @@ require 'rails_helper' RSpec.describe Web::PushSubscription, type: :model do let(:alerts) { { mention: true, reblog: false, follow: true, follow_request: false, favourite: true } } - let(:payload_no_alerts) { Web::PushSubscription.new(id: 1, endpoint: 'a', key_p256dh: 'c', key_auth: 'd').as_payload } - let(:payload_alerts) { Web::PushSubscription.new(id: 1, endpoint: 'a', key_p256dh: 'c', key_auth: 'd', data: { alerts: alerts }).as_payload } let(:push_subscription) { Web::PushSubscription.new(data: { alerts: alerts }) } - describe '#as_payload' do - it 'only returns id and endpoint' do - expect(payload_no_alerts.keys).to eq [:id, :endpoint] - end - - it 'returns alerts if set' do - expect(payload_alerts.keys).to eq [:id, :endpoint, :alerts] - end - end - describe '#pushable?' do it 'obeys alert settings' do expect(push_subscription.send(:pushable?, Notification.new(activity_type: 'Mention'))).to eq true