diff --git a/app/assets/javascripts/users/connected_devices.js b/app/assets/javascripts/users/connected_devices.js new file mode 100644 index 000000000..c99c18e23 --- /dev/null +++ b/app/assets/javascripts/users/connected_devices.js @@ -0,0 +1,25 @@ +/* global I18n */ + +(function() { + let connectedDeviceDescription = $('.connected-devices-description'); + let revocationModal = $('.device-revocation-modal'); + let deleteButtonModal = $('#confirm-device-remove'); + + $('.x-button').on('click', function() { + deleteButtonModal.attr('href', $(this).data('url')); + deleteButtonModal.data('id', $(this).closest('.table-row').data('id')); + }); + + deleteButtonModal + .on('ajax:success', function() { + $(`.table-row[data-id=${deleteButtonModal.data('id')}]`).remove(); + + // Show correct representation if user does not have any connected device anymore + if (connectedDeviceDescription.find('.table-row').length === 0) { + $('.connected-devices-container').remove(); + connectedDeviceDescription.append(`

${I18n.t('users.registrations.edit.connected_devices.empty_state')}

`); + } + + revocationModal.modal('hide'); + }); +}()); diff --git a/app/assets/stylesheets/settings/device_table.scss b/app/assets/stylesheets/settings/device_table.scss new file mode 100644 index 000000000..210bf4b55 --- /dev/null +++ b/app/assets/stylesheets/settings/device_table.scss @@ -0,0 +1,62 @@ +// scss-lint:disable SelectorDepth NestingDepth IdSelector +#devicesTable { + .devices-table { + display: grid; + grid-auto-rows: 3em 1px; + grid-template-columns: 200px 150px 50px; + min-width: 100%; + + .table-header-cell { + align-items: center; + background-color: $color-concrete; + border: 1px solid $color-white; + display: flex; + height: 3em; + padding: 0 .5em; + z-index: 2; + } + + .table-header { + display: contents; + + &::after { + content: ""; + grid-column: 1/-1; + } + } + + .table-body { + display: contents; + } + + .table-body-cell { + align-items: center; + display: flex; + padding: 0 .5em; + } + + .x-button { + background: 0; + border: 0; + margin-left: 5px; + } + + .table-row { + display: contents; + + &:hover { + .table-body-cell { + background-color: $color-concrete; + } + } + + &::after { + background: $color-concrete; + content: ""; + display: inline-block; + grid-column: 1/-1; + height: 1px; + } + } + } +} diff --git a/app/assets/stylesheets/settings/users.scss b/app/assets/stylesheets/settings/users.scss index f1984281b..ef844a210 100644 --- a/app/assets/stylesheets/settings/users.scss +++ b/app/assets/stylesheets/settings/users.scss @@ -119,6 +119,18 @@ } } +.device-revocation-modal { + .modal-dialog { + height: 216px; + width: 370px; + } +} + +.manage-devices { + position: relative; + top: 30px; +} + @media (max-width: 700px) { .user-settings { .two-factor-container { diff --git a/app/controllers/concerns/token_authentication.rb b/app/controllers/concerns/token_authentication.rb index a6ed6d2cd..46f967bfa 100644 --- a/app/controllers/concerns/token_authentication.rb +++ b/app/controllers/concerns/token_authentication.rb @@ -17,6 +17,8 @@ module TokenAuthentication @token = request.headers['Authorization']&.sub('Bearer ', '') raise JWT::VerificationError, I18n.t('api.core.missing_token') unless @token + check_token_revocation! + @token_iss = Api::CoreJwt.read_iss(@token) raise JWT::InvalidPayload, I18n.t('api.core.no_iss') unless @token_iss @@ -34,4 +36,10 @@ module TokenAuthentication @current_user = User.find_by(id: payload['sub']) raise JWT::InvalidPayload, I18n.t('api.core.no_user_mapping') unless current_user end + + def check_token_revocation! + if Doorkeeper::AccessToken.where.not(revoked_at: nil).exists?(token: @token) + raise JWT::VerificationError, I18n.t('api.core.expired_token') + end + end end diff --git a/app/controllers/doorkeeper/access_tokens_controller.rb b/app/controllers/doorkeeper/access_tokens_controller.rb new file mode 100644 index 000000000..809f16907 --- /dev/null +++ b/app/controllers/doorkeeper/access_tokens_controller.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module Doorkeeper + class AccessTokensController < ApplicationController + before_action :find_token + + def revoke + @token.revoke + end + + private + + def find_token + @token = current_user.access_tokens.find(params[:id]) + end + end +end diff --git a/app/controllers/users/connected_devices_controller.rb b/app/controllers/users/connected_devices_controller.rb new file mode 100644 index 000000000..59d1daf05 --- /dev/null +++ b/app/controllers/users/connected_devices_controller.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module Users + class ConnectedDevicesController < ApplicationController + before_action :check_delete_permissions, only: :destroy + + def destroy + @connected_device.destroy + end + + private + + def check_delete_permissions + @connected_device = ConnectedDevice.for_user(current_user).find_by(id: params[:id]) + render_403 if @connected_device.blank? + end + end +end diff --git a/app/controllers/users/registrations_controller.rb b/app/controllers/users/registrations_controller.rb index c1f7e9d11..056d0ee4b 100644 --- a/app/controllers/users/registrations_controller.rb +++ b/app/controllers/users/registrations_controller.rb @@ -184,6 +184,11 @@ class Users::RegistrationsController < Devise::RegistrationsController end end + def edit + @connected_devices = ConnectedDevice.for_user(current_user) + super + end + def two_factor_enable user = current_user || User.find_by(id: session[:otp_user_id]) if user.valid_otp?(params[:submit_code]) diff --git a/app/models/connected_device.rb b/app/models/connected_device.rb new file mode 100644 index 000000000..40771bc4a --- /dev/null +++ b/app/models/connected_device.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +class ConnectedDevice < ApplicationRecord + belongs_to :oauth_access_token, class_name: 'Doorkeeper::AccessToken' + validates :uid, presence: true + + after_destroy :revoke_token + + def self.for_user(user) + where(oauth_access_token_id: Doorkeeper::AccessToken.select(:id).where(resource_owner_id: user.id)) + end + + def self.from_request_headers(headers, token = nil) + return unless headers['Device-Id'] + + current_token = Doorkeeper::AccessToken.find_by( + token: headers['Authorization']&.gsub(/Bearer\s/, '') + ) + + return unless token || current_token + + connected_device = ConnectedDevice.find_or_initialize_by(uid: headers['Device-Id']) + connected_device.update!( + name: headers['Device-Name'], + metadata: { + os: headers['Device-Os'], + app_version: headers['Device-App-Version'] + }.compact, + last_seen_at: Time.current, + oauth_access_token_id: token&.id || current_token&.id + ) + connected_device + end + + private + + def revoke_token + oauth_access_token.revoke + end +end diff --git a/app/views/users/registrations/_device_revocation_modal.html.erb b/app/views/users/registrations/_device_revocation_modal.html.erb new file mode 100644 index 000000000..6a4da209d --- /dev/null +++ b/app/views/users/registrations/_device_revocation_modal.html.erb @@ -0,0 +1,19 @@ + + diff --git a/app/views/users/registrations/_manage_devices.html.erb b/app/views/users/registrations/_manage_devices.html.erb new file mode 100644 index 000000000..01e30bd20 --- /dev/null +++ b/app/views/users/registrations/_manage_devices.html.erb @@ -0,0 +1,44 @@ +
+

<%= t("users.registrations.edit.connected_devices.title") %>

+ <% if @connected_devices.present? %> +
+

<%= t("users.registrations.edit.connected_devices.description") %>

+
+
+
+
+
+ <%= t("users.registrations.edit.devices_table.device_name") %> +
+
+ <%= t("users.registrations.edit.devices_table.added_on") %> +
+
+
+
+
+ <% @connected_devices.each do |device| %> +
> +
+ <%= device.name %> +
+
+ <%= l(device.last_seen_at, format: :full_date) %> +
+
+ +
+
+ <% end %> +
+
+
+
+
+ <% else %> +

<%= t("users.registrations.edit.connected_devices.empty_state") %>

+ <% end %> +
+ +<%= render partial: 'device_revocation_modal' %> +<%= javascript_include_tag 'users/connected_devices' %> diff --git a/app/views/users/registrations/edit.html.erb b/app/views/users/registrations/edit.html.erb index f7eacf745..02c2e5114 100644 --- a/app/views/users/registrations/edit.html.erb +++ b/app/views/users/registrations/edit.html.erb @@ -38,6 +38,13 @@ +<% if Rails.application.config.x.connected_devices_enabled %> + +
+ <%= render partial: 'users/registrations/manage_devices' %> +
+<% end %> +
@@ -70,6 +77,7 @@
<%= render partial: '2fa_modal' %> + <%= render partial: 'users/shared/user_avatars_modal' %> <%= javascript_pack_tag 'custom/croppie' %> diff --git a/config/application.rb b/config/application.rb index bf486722c..b9b8401c9 100644 --- a/config/application.rb +++ b/config/application.rb @@ -57,6 +57,8 @@ module Scinote config.x.webhooks_enabled = ENV['ENABLE_WEBHOOKS'] == 'true' + config.x.connected_devices_enabled = ENV['CONNECTED_DEVICES_ENABLED'] == 'true' + # Logging config.log_formatter = proc do |severity, datetime, progname, msg| "[#{datetime}] #{severity}: #{msg}\n" @@ -69,6 +71,19 @@ module Scinote config.to_prepare do # Only Authorization endpoint Doorkeeper::AuthorizationsController.layout 'sign_in_halt' + + # Add Connected Device logging + Doorkeeper::TokensController.class_eval do + after_action :log_connected_device, only: :create + + private + + def log_connected_device + return if @authorize_response.is_a?(Doorkeeper::OAuth::ErrorResponse) + + ConnectedDevice.from_request_headers(request.headers, @authorize_response&.token) + end + end end config.action_view.field_error_proc = Proc.new { |html_tag, instance| diff --git a/config/initializers/assets.rb b/config/initializers/assets.rb index 20debf34e..c436623d4 100644 --- a/config/initializers/assets.rb +++ b/config/initializers/assets.rb @@ -97,6 +97,7 @@ Rails.application.config.assets.precompile += %w(reports/save_pdf_to_inventory.j Rails.application.config.assets.precompile += %w(reports/content.js) Rails.application.config.assets.precompile += %w(session_end.js) Rails.application.config.assets.precompile += %w(label_templates/label_templates_datatable.js) +Rails.application.config.assets.precompile += %w(users/connected_devices.js) Rails.application.config.assets.precompile += %w(BrowserPrint-3.0.216.min.js) Rails.application.config.assets.precompile += %w(BrowserPrint-Zebra-1.0.216.min.js) diff --git a/config/locales/en.yml b/config/locales/en.yml index 21c3eb83f..43ecc75bd 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2338,6 +2338,19 @@ en: 2fa_errors: wrong_submit_code: "Not correct code" wrong_password: "Not correct password" + connected_devices: + title: "Manage connected devices" + description: "When you remove a device, you will be logged out from the selected device." + empty_state: "No connected devices." + devices_table: + device_name: "Device Name" + added_on: "Added on" + device_revocation_modal: + title: "Remove connected device" + description: "This device will lose access to your SciNote account." + validation: "Are you sure you want to remove it?" + cancel: "Cancel" + remove: "Remove" new: head_title: "Sign up" team_name_label: "Team name" diff --git a/config/routes.rb b/config/routes.rb index 981cbda1e..d7e9dd634 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -3,6 +3,8 @@ Rails.application.routes.draw do skip_controllers :applications, :authorized_applications, :token_info end + post 'access_tokens/revoke', to: 'doorkeeper/access_tokens#revoke' + # Addons def draw(routes_name) @@ -736,6 +738,8 @@ Rails.application.routes.draw do end end + resources :connected_devices, controller: 'users/connected_devices', only: %i(destroy) + get 'search' => 'search#index' get 'search/new' => 'search#new', as: :new_search diff --git a/db/migrate/20230207140811_create_connected_devices.rb b/db/migrate/20230207140811_create_connected_devices.rb new file mode 100644 index 000000000..fea978686 --- /dev/null +++ b/db/migrate/20230207140811_create_connected_devices.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class CreateConnectedDevices < ActiveRecord::Migration[6.1] + def change + create_table :connected_devices do |t| + t.string :uid + t.string :name + t.references :oauth_access_token, null: false, foreign_key: true + t.json :metadata + t.timestamp :last_seen_at + + t.timestamps + end + end +end diff --git a/spec/controllers/concerns/token_authentication_spec.rb b/spec/controllers/concerns/token_authentication_spec.rb new file mode 100644 index 000000000..427c1bd87 --- /dev/null +++ b/spec/controllers/concerns/token_authentication_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'rails_helper' + +class TokenAuthenticatedController + attr_accessor :token, :current_user + + include TokenAuthentication + + def request + OpenStruct.new( + headers: { 'Authorization' => "Bearer #{@token}" } + ) + end +end + +describe TokenAuthentication do + let(:test_controller_instance) { TokenAuthenticatedController.new } + + let(:user) { create :user } + let(:access_token) { + user.access_tokens.create( + expires_in: 7500 + ) + } + + describe '#authenticate_request' do + it "rejects revoked token" do + test_controller_instance.token = access_token.token + test_controller_instance.current_user = user + + access_token.revoke + expect { test_controller_instance.send(:authenticate_request!) }.to raise_error(JWT::VerificationError) + end + end +end diff --git a/spec/controllers/doorkeeper/access_tokens_controller_spec.rb b/spec/controllers/doorkeeper/access_tokens_controller_spec.rb new file mode 100644 index 000000000..8de196e84 --- /dev/null +++ b/spec/controllers/doorkeeper/access_tokens_controller_spec.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Doorkeeper::AccessTokensController, type: :controller do + login_user + + let!(:access_token) do + subject.current_user.access_tokens.create(expires_in: 7500) + end + + describe 'POST revoke' do + let(:params) do + { + id: access_token.id + } + end + + let(:action) do + put :revoke, params: params + end + + it 'revokes the access token' do + action + expect(access_token.reload.revoked_at).to_not be_nil + end + end +end