From 7c4bad860af70dc5a3bc10f609746774e112c633 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Mon, 2 Nov 2026 13:01:44 +0400 Subject: [PATCH 01/12] Store per-device user agent for remoteStorage authorizations - Add user_agent to remote_storage_authorizations and rs_authorization_codes - Capture the browser's user agent at consent, carry it on the authorization code, and store it on the authorization when the code is exchanged - Store it directly for the implicit flow too With the reuse shortcut gone, each authorization is already a distinct per-device token; this adds the metadata needed to label and display them. Part of #254 --- app/controllers/rs/oauth_controller.rb | 4 +++- app/services/rs_oauth_manager/exchange_code.rb | 3 ++- app/services/rs_oauth_manager/issue_code.rb | 4 +++- .../20261102130000_add_user_agent_to_rs_authorizations.rb | 6 ++++++ db/schema.rb | 4 +++- spec/controllers/rs/oauth_controller_spec.rb | 6 ++++++ spec/services/rs_oauth_manager/exchange_code_spec.rb | 2 ++ spec/services/rs_oauth_manager/issue_code_spec.rb | 4 +++- 8 files changed, 28 insertions(+), 5 deletions(-) create mode 100644 db/migrate/20261102130000_add_user_agent_to_rs_authorizations.rb diff --git a/app/controllers/rs/oauth_controller.rb b/app/controllers/rs/oauth_controller.rb index fe69198..c5e1952 100644 --- a/app/controllers/rs/oauth_controller.rb +++ b/app/controllers/rs/oauth_controller.rb @@ -112,6 +112,7 @@ class Rs::OauthController < ApplicationController redirect_uri: redirect_uri, permissions: permissions, expire_at: expire_at, + user_agent: request.user_agent, code_challenge: challenge, code_challenge_method: method ) @@ -124,7 +125,8 @@ class Rs::OauthController < ApplicationController client_id: client_id, redirect_uri: redirect_uri, app_name: client_id, - expire_at: expire_at + expire_at: expire_at, + user_agent: request.user_agent ) redirect_to url_with_state("#{redirect_uri}#access_token=#{auth.token}", state), diff --git a/app/services/rs_oauth_manager/exchange_code.rb b/app/services/rs_oauth_manager/exchange_code.rb index 131fbe2..8ed5298 100644 --- a/app/services/rs_oauth_manager/exchange_code.rb +++ b/app/services/rs_oauth_manager/exchange_code.rb @@ -36,7 +36,8 @@ module RsOauthManager client_id: auth_code.client_id, redirect_uri: auth_code.redirect_uri, app_name: auth_code.client_id, - expire_at: auth_code.permission_expire_at + expire_at: auth_code.permission_expire_at, + user_agent: auth_code.user_agent ) end diff --git a/app/services/rs_oauth_manager/issue_code.rb b/app/services/rs_oauth_manager/issue_code.rb index b155117..87ccb79 100644 --- a/app/services/rs_oauth_manager/issue_code.rb +++ b/app/services/rs_oauth_manager/issue_code.rb @@ -2,7 +2,7 @@ module RsOauthManager class IssueCode < RsOauthManagerService def initialize(user:, client_id:, redirect_uri:, permissions:, code_challenge:, code_challenge_method: "S256", - expire_at: nil) + expire_at: nil, user_agent: nil) @user = user @client_id = client_id @redirect_uri = redirect_uri @@ -10,6 +10,7 @@ module RsOauthManager @code_challenge = code_challenge @code_challenge_method = code_challenge_method @expire_at = expire_at + @user_agent = user_agent end def call @@ -19,6 +20,7 @@ module RsOauthManager redirect_uri: @redirect_uri, permissions: @permissions, permission_expire_at: @expire_at, + user_agent: @user_agent, code_challenge: @code_challenge, code_challenge_method: @code_challenge_method ) diff --git a/db/migrate/20261102130000_add_user_agent_to_rs_authorizations.rb b/db/migrate/20261102130000_add_user_agent_to_rs_authorizations.rb new file mode 100644 index 0000000..841369e --- /dev/null +++ b/db/migrate/20261102130000_add_user_agent_to_rs_authorizations.rb @@ -0,0 +1,6 @@ +class AddUserAgentToRsAuthorizations < ActiveRecord::Migration[8.1] + def change + add_column :remote_storage_authorizations, :user_agent, :string + add_column :rs_authorization_codes, :user_agent, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 79a53bc..524de14 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[8.1].define(version: 2026_11_02_120000) do +ActiveRecord::Schema[8.1].define(version: 2026_11_02_130000) do create_table "active_storage_attachments", force: :cascade do |t| t.bigint "blob_id", null: false t.datetime "created_at", null: false @@ -110,6 +110,7 @@ ActiveRecord::Schema[8.1].define(version: 2026_11_02_120000) do t.string "redirect_uri" t.string "token" t.datetime "updated_at", null: false + t.string "user_agent" t.integer "user_id", null: false t.integer "web_app_id" t.index ["permissions"], name: "index_remote_storage_authorizations_on_permissions" @@ -128,6 +129,7 @@ ActiveRecord::Schema[8.1].define(version: 2026_11_02_120000) do t.text "permissions", default: "[]" t.string "redirect_uri", null: false t.datetime "updated_at", null: false + t.string "user_agent" t.integer "user_id", null: false t.index ["code_digest"], name: "index_rs_authorization_codes_on_code_digest", unique: true t.index ["user_id"], name: "index_rs_authorization_codes_on_user_id" diff --git a/spec/controllers/rs/oauth_controller_spec.rb b/spec/controllers/rs/oauth_controller_spec.rb index 6e2c233..e06be1a 100644 --- a/spec/controllers/rs/oauth_controller_spec.rb +++ b/spec/controllers/rs/oauth_controller_spec.rb @@ -346,6 +346,7 @@ RSpec.describe Rs::OauthController, type: :controller do context "when user is signed in" do before do sign_in user + request.user_agent = "RSpec Device" end after do @@ -370,6 +371,10 @@ RSpec.describe Rs::OauthController, type: :controller do expect(@auth.permissions).to eq(%w(documents photos contacts:rw videos:r tasks/work:r)) end + it "stores the device's user agent" do + expect(@auth.user_agent).to eq("RSpec Device") + end + it "redirects to the redirect_uri" do expect(response).to redirect_to("https://example.com#access_token=#{@auth.token}&state=foobar123") end @@ -499,6 +504,7 @@ RSpec.describe Rs::OauthController, type: :controller do expect(@code.permissions).to eq(%w(documents photos contacts:rw)) expect(@code.code_challenge).to eq("E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM") expect(@code.code_challenge_method).to eq("S256") + expect(@code.user_agent).to eq("RSpec Device") end it "does not create a remote storage authorization" do diff --git a/spec/services/rs_oauth_manager/exchange_code_spec.rb b/spec/services/rs_oauth_manager/exchange_code_spec.rb index 8bf1661..e0169a2 100644 --- a/spec/services/rs_oauth_manager/exchange_code_spec.rb +++ b/spec/services/rs_oauth_manager/exchange_code_spec.rb @@ -20,6 +20,7 @@ RSpec.describe RsOauthManager::ExchangeCode do permissions: %w(documents:rw), code_challenge: code_challenge, code_challenge_method: "S256", + user_agent: "RSpec Device", **overrides ) end @@ -43,6 +44,7 @@ RSpec.describe RsOauthManager::ExchangeCode do expect(auth.permissions).to eq(%w(documents:rw)) expect(auth.client_id).to eq("example.com") expect(auth.token).to be_present + expect(auth.user_agent).to eq("RSpec Device") expect(RsAuthorizationCode.exists?(code.id)).to be(false) end diff --git a/spec/services/rs_oauth_manager/issue_code_spec.rb b/spec/services/rs_oauth_manager/issue_code_spec.rb index c80593f..3e424d1 100644 --- a/spec/services/rs_oauth_manager/issue_code_spec.rb +++ b/spec/services/rs_oauth_manager/issue_code_spec.rb @@ -11,7 +11,8 @@ RSpec.describe RsOauthManager::IssueCode do permissions: %w(documents:rw), code_challenge: "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM", code_challenge_method: "S256", - expire_at: 1.month.from_now + expire_at: 1.month.from_now, + user_agent: "RSpec Device" ) end @@ -21,6 +22,7 @@ RSpec.describe RsOauthManager::IssueCode do expect(subject.client_id).to eq("example.com") expect(subject.permissions).to eq(%w(documents:rw)) expect(subject.code_challenge).to eq("E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM") + expect(subject.user_agent).to eq("RSpec Device") end it "does not create a remote storage authorization" do From eeb399ff8389632a683a4b2291168d135ca8328f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Mon, 2 Nov 2026 17:40:00 +0400 Subject: [PATCH 02/12] Document the storage-first launch flow Add an explicit spec for the storage-first handoff (remoteStorage spec section 11): launching an app passes only `#remotestorage=
`, never an access token. The app then runs the application-first (now PKCE-capable) flow itself. Part of #254 --- app/controllers/services/rs_auths_controller.rb | 2 ++ spec/controllers/services/rs_auths_controller_spec.rb | 8 ++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/app/controllers/services/rs_auths_controller.rb b/app/controllers/services/rs_auths_controller.rb index 2fff161..c081538 100644 --- a/app/controllers/services/rs_auths_controller.rb +++ b/app/controllers/services/rs_auths_controller.rb @@ -29,6 +29,8 @@ class Services::RsAuthsController < Services::BaseController current_user.address end + # Storage-first bearer token issuance (remoteStorage spec §11): the app + # receives the address and runs the application-first flow itself. launch_url = "#{@auth.launch_url}#remotestorage=#{user_address}" redirect_to launch_url, allow_other_host: true diff --git a/spec/controllers/services/rs_auths_controller_spec.rb b/spec/controllers/services/rs_auths_controller_spec.rb index 1c619c2..812a842 100644 --- a/spec/controllers/services/rs_auths_controller_spec.rb +++ b/spec/controllers/services/rs_auths_controller_spec.rb @@ -11,7 +11,7 @@ RSpec.describe Services::RsAuthsController, type: :controller do ]) end - describe "GET /services/storage/rs_auths/:id/launch_app" do + describe "GET /services/storage/rs_auths/:id/launch_app (storage-first)" do context "when user is signed in" do before do sign_in user @@ -32,10 +32,14 @@ RSpec.describe Services::RsAuthsController, type: :controller do @auth.destroy end - it "redirects to the given URL with the correct RS URL fragment params" do + it "hands the app the user address in the URL fragment" do launch_url = "https://app.example.com#remotestorage=#{user.address}" expect(response).to redirect_to(launch_url) end + + it "does not hand over an access token" do + expect(response.location).not_to include("access_token") + end end context "when remoteStorage is not enabled for the user" do From 3dcd263d438f8b41457278218c7a06e7221ee91c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Tue, 3 Nov 2026 11:42:06 +0400 Subject: [PATCH 03/12] Store the remoteStorage user agent as text, not varchar(255) Real-world user agents (in-app browsers, Android WebView variants) commonly exceed 255 characters. On PostgreSQL that would raise and 500 the consent POST and token exchange; SQLite dev/test does not enforce the limit, so it went unnoticed. Use text columns for user_agent on both tables. Part of #254 --- .../20261102130000_add_user_agent_to_rs_authorizations.rb | 4 ++-- db/schema.rb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/db/migrate/20261102130000_add_user_agent_to_rs_authorizations.rb b/db/migrate/20261102130000_add_user_agent_to_rs_authorizations.rb index 841369e..d310450 100644 --- a/db/migrate/20261102130000_add_user_agent_to_rs_authorizations.rb +++ b/db/migrate/20261102130000_add_user_agent_to_rs_authorizations.rb @@ -1,6 +1,6 @@ class AddUserAgentToRsAuthorizations < ActiveRecord::Migration[8.1] def change - add_column :remote_storage_authorizations, :user_agent, :string - add_column :rs_authorization_codes, :user_agent, :string + add_column :remote_storage_authorizations, :user_agent, :text + add_column :rs_authorization_codes, :user_agent, :text end end diff --git a/db/schema.rb b/db/schema.rb index 524de14..4518582 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -110,7 +110,7 @@ ActiveRecord::Schema[8.1].define(version: 2026_11_02_130000) do t.string "redirect_uri" t.string "token" t.datetime "updated_at", null: false - t.string "user_agent" + t.text "user_agent" t.integer "user_id", null: false t.integer "web_app_id" t.index ["permissions"], name: "index_remote_storage_authorizations_on_permissions" @@ -129,7 +129,7 @@ ActiveRecord::Schema[8.1].define(version: 2026_11_02_130000) do t.text "permissions", default: "[]" t.string "redirect_uri", null: false t.datetime "updated_at", null: false - t.string "user_agent" + t.text "user_agent" t.integer "user_id", null: false t.index ["code_digest"], name: "index_rs_authorization_codes_on_code_digest", unique: true t.index ["user_id"], name: "index_rs_authorization_codes_on_user_id" From c1548035ec084d7626a558d784ad5d65de809d22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Mon, 2 Nov 2026 13:04:42 +0400 Subject: [PATCH 04/12] Add storage dashboard card and per-app details modal - Rework RsAuthComponent into a card that shows the app, its device, an access summary and the permission status - Group connected apps by app and device, so repeated authorizations for the same app and device (e.g. after a browser update) are shown together, each with its own revoke action - Add RsAuthDetailsModalComponent with the full scope list, device, connected tokens and permission expiry, plus Launch and Revoke actions (built on the existing ModalComponent) - Add RemoteStorageAuthorization#device_name (parsed from the user agent) and #expired? Part of #254 --- app/components/rs_auth_component.html.erb | 62 ++++++----- app/components/rs_auth_component.rb | 39 ++++++- .../rs_auth_details_modal_component.html.erb | 47 ++++++++ .../rs_auth_details_modal_component.rb | 35 ++++++ .../services/rs_auths_controller.rb | 12 ++- app/models/remote_storage_authorization.rb | 28 +++++ app/views/services/rs_auths/index.html.erb | 8 +- spec/features/services/rs_auths_spec.rb | 101 ++++++++++++++++++ .../remote_storage_authorization_spec.rb | 51 +++++++++ 9 files changed, 350 insertions(+), 33 deletions(-) create mode 100644 app/components/rs_auth_details_modal_component.html.erb create mode 100644 app/components/rs_auth_details_modal_component.rb create mode 100644 spec/features/services/rs_auths_spec.rb diff --git a/app/components/rs_auth_component.html.erb b/app/components/rs_auth_component.html.erb index a26d35d..863af3a 100644 --- a/app/components/rs_auth_component.html.erb +++ b/app/components/rs_auth_component.html.erb @@ -1,27 +1,41 @@ -
-
- <%= render AppCatalog::WebAppIconComponent.new(web_app: @web_app) %> +
+
+
+ <%= render AppCatalog::WebAppIconComponent.new(web_app: @web_app) %> +
+
+

<%= name %>

+

<%= @auth.client_id %>

+
-
-

- <%= @web_app&.name || @auth.app_name %> -

-

- <%= @auth.client_id %> -

+ +
+
+
Access
+
<%= scope_summary %>
+
+
+
Device
+
<%= device_name %>
+
+
+
Status
+
"><%= expiry_label %>
+
+ <% if multiple_authorizations? %> +
+
Tokens
+
<%= @authorizations.size %>
+
+ <% end %> +
+ +
+
- <%= render DropdownComponent.new do %> - <%= render DropdownLinkComponent.new( - href: launch_app_services_storage_rs_auth_url(@auth), - open_in_new_tab: true - ) do %> - Launch app - <% end %> - <%= render DropdownLinkComponent.new( - href: revoke_services_storage_rs_auth_url(@auth), - separator: true, add_class: "text-red-700" - ) do %> - Revoke access - <% end %> - <% end %> + + <%= render RsAuthDetailsModalComponent.new(authorizations: @authorizations) %>
diff --git a/app/components/rs_auth_component.rb b/app/components/rs_auth_component.rb index ed588bf..56e003a 100644 --- a/app/components/rs_auth_component.rb +++ b/app/components/rs_auth_component.rb @@ -1,8 +1,41 @@ # frozen_string_literal: true class RsAuthComponent < ViewComponent::Base - def initialize(auth:) - @auth = auth - @web_app = auth.web_app + include OauthHelper + + def initialize(authorizations:) + @authorizations = authorizations + @auth = authorizations.first + @web_app = @auth.web_app + end + + def name + @web_app&.name.presence || @auth.app_name.presence || @auth.client_id + end + + def root_access? + (@auth.permissions & [":r", ":rw"]).any? + end + + def scope_summary + return "All folders and files" if root_access? + + count = @auth.permissions.size + "#{count} #{'folder'.pluralize(count)}" + end + + def device_name + @auth.device_name.presence || "Unknown device" + end + + def expiry_label + return "Never expires" if @auth.expire_at.blank? + return "Expired" if @auth.expired? + + "Expires #{@auth.expire_at.strftime('%-d %b %Y')}" + end + + def multiple_authorizations? + @authorizations.size > 1 end end diff --git a/app/components/rs_auth_details_modal_component.html.erb b/app/components/rs_auth_details_modal_component.html.erb new file mode 100644 index 0000000..ce0233a --- /dev/null +++ b/app/components/rs_auth_details_modal_component.html.erb @@ -0,0 +1,47 @@ +<%= render ModalComponent.new do %> +
+
+ <%= render AppCatalog::WebAppIconComponent.new(web_app: @web_app) %> +
+
+

<%= name %>

+

<%= @auth.client_id %>

+
+
+ +
Access
+ <% if root_access? %> +

All folders and files

+ <% else %> +
    + <% @auth.permissions.each do |scope| %> +
  • <%= scope_label(scope) %>
  • + <% end %> +
+ <% end %> + +
Device
+

<%= device_name %>

+ +
+ <%= @authorizations.size > 1 ? "Connected tokens" : "Connected" %> +
+
    + <% @authorizations.each do |auth| %> +
  • + + <%= auth.created_at.strftime('%-d %b %Y') %> + · + <%= status_label(auth) %> + + <%= link_to "Revoke access", revoke_services_storage_rs_auth_path(auth), + class: "text-red-700 whitespace-nowrap" %> +
  • + <% end %> +
+ +
+ <%= link_to "Launch app", launch_app_services_storage_rs_auth_path(@auth), + class: "btn-md btn-blue", target: "_blank", rel: "noopener" %> +
+<% end %> diff --git a/app/components/rs_auth_details_modal_component.rb b/app/components/rs_auth_details_modal_component.rb new file mode 100644 index 0000000..257329c --- /dev/null +++ b/app/components/rs_auth_details_modal_component.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +class RsAuthDetailsModalComponent < ViewComponent::Base + include OauthHelper + + def initialize(authorizations:) + @authorizations = authorizations + @auth = authorizations.first + @web_app = @auth.web_app + end + + def name + @web_app&.name.presence || @auth.app_name.presence || @auth.client_id + end + + def root_access? + (@auth.permissions & [":r", ":rw"]).any? + end + + def scope_label(scope) + permission = scope_permissions(scope) == "r" ? "read only" : "read/write" + "#{scope_name(scope)} (#{permission})" + end + + def device_name + @auth.device_name.presence || "Unknown device" + end + + def status_label(auth) + return "Never expires" if auth.expire_at.blank? + return "Expired on #{auth.expire_at.strftime('%-d %b %Y')}" if auth.expired? + + "Expires #{auth.expire_at.strftime('%-d %b %Y')}" + end +end diff --git a/app/controllers/services/rs_auths_controller.rb b/app/controllers/services/rs_auths_controller.rb index c081538..3518b2a 100644 --- a/app/controllers/services/rs_auths_controller.rb +++ b/app/controllers/services/rs_auths_controller.rb @@ -5,8 +5,12 @@ class Services::RsAuthsController < Services::BaseController before_action :find_rs_auth, only: [:destroy, :launch_app] def index - @rs_auths = current_user.remote_storage_authorizations - # TODO sort by app name? + @rs_auth_groups = current_user.remote_storage_authorizations + .includes(:web_app) + .group_by { |auth| [auth.client_id, auth.device_name] } + .values + .map { |auths| auths.sort_by(&:created_at).reverse } + .sort_by { |auths| app_name_for(auths.first).downcase } end def destroy @@ -38,6 +42,10 @@ class Services::RsAuthsController < Services::BaseController private + def app_name_for(auth) + auth.web_app&.name.presence || auth.app_name.presence || auth.client_id + end + def require_service_enabled unless current_user.service_enabled?(:remotestorage) http_status :forbidden diff --git a/app/models/remote_storage_authorization.rb b/app/models/remote_storage_authorization.rb index a8dfd1c..b27e5c2 100644 --- a/app/models/remote_storage_authorization.rb +++ b/app/models/remote_storage_authorization.rb @@ -40,6 +40,34 @@ class RemoteStorageAuthorization < ApplicationRecord end end + def expired? + expire_at.present? && expire_at.past? + end + + def device_name + return nil if user_agent.blank? + + browser = + case user_agent + when /Firefox/ then "Firefox" + when /Edg\// then "Edge" + when /Chrome/ then "Chrome" + when /Safari/ then "Safari" + else "Unknown browser" + end + + os = + case user_agent + when /Android/ then "Android" + when /iPhone|iPad|iPod/ then "iOS" + when /Mac OS X/ then "macOS" + when /Windows/ then "Windows" + when /Linux/ then "Linux" + end + + os ? "#{browser} on #{os}" : browser + end + def delete_token_from_redis key = "authorizations:#{user.cn}:#{token}" redis.srem? key, redis.smembers(key) diff --git a/app/views/services/rs_auths/index.html.erb b/app/views/services/rs_auths/index.html.erb index 939df14..9141066 100644 --- a/app/views/services/rs_auths/index.html.erb +++ b/app/views/services/rs_auths/index.html.erb @@ -10,10 +10,10 @@ <%= render partial: "shared/tabnav_remotestorage" %>
- <% if @rs_auths.any? %> -
- <% @rs_auths.each do |auth| %> - <%= render RsAuthComponent.new(auth: auth) %> + <% if @rs_auth_groups.any? %> +
+ <% @rs_auth_groups.each do |authorizations| %> + <%= render RsAuthComponent.new(authorizations: authorizations) %> <% end %>
<% else %> diff --git a/spec/features/services/rs_auths_spec.rb b/spec/features/services/rs_auths_spec.rb new file mode 100644 index 0000000..9fd69b0 --- /dev/null +++ b/spec/features/services/rs_auths_spec.rb @@ -0,0 +1,101 @@ +require 'rails_helper' + +RSpec.describe "Storage connected apps", type: :feature do + let(:user) { create :user } + + let(:firefox_ua) do + "Mozilla/5.0 (X11; Linux x86_64; rv:120.0) Gecko/20100101 Firefox/120.0" + end + + let(:chrome_ua) do + "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 " \ + "(KHTML, like Gecko) Chrome/119.0.0.0 Safari/537.36" + end + + before do + allow_any_instance_of(AppCatalog::WebApp).to receive(:update_metadata).and_return(true) + allow_any_instance_of(RemoteStorageAuthorization).to receive(:remove_token_expiry_job).and_return(nil) + allow_any_instance_of(User).to receive(:ldap_entry).and_return( + uid: user.cn, ou: user.ou, mail: user.email, admin: nil, + services_enabled: ["remotestorage"] + ) + login_as user, scope: :user + end + + def create_auth(client:, device:, permissions: %w(documents:rw)) + user.remote_storage_authorizations.create!( + permissions: permissions, + client_id: client, + redirect_uri: "https://#{client}", + expire_at: 1.month.from_now, + user_agent: device + ) + end + + context "with a connected app" do + before do + AppCatalog::WebApp.create!(url: "https://notes.example.com", name: "Notes", metadata: {}) + end + + let!(:auth) do + create_auth(client: "notes.example.com", device: firefox_ua, + permissions: %w(documents:rw photos:r)) + end + + after { auth.destroy } + + it "shows the app, its device and permission status" do + visit apps_services_storage_path + + expect(page).to have_content("Notes") + expect(page).to have_content("notes.example.com") + expect(page).to have_content("Firefox on Linux") + expect(page).to have_content("2 folders") + expect(page).to have_content("Expires") + end + + it "renders a details modal with scopes and actions" do + visit apps_services_storage_path + + expect(page).to have_selector("li", text: "documents (read/write)", visible: false) + expect(page).to have_selector("li", text: "photos (read only)", visible: false) + expect(page).to have_link("Launch app", visible: false) + expect(page).to have_selector("a", text: "Revoke access", visible: false, count: 1) + end + end + + context "with duplicates" do + let!(:first) { create_auth(client: "notes.example.com", device: firefox_ua) } + let!(:second) { create_auth(client: "notes.example.com", device: firefox_ua) } + let!(:other) { create_auth(client: "notes.example.com", device: chrome_ua) } + + after do + first.destroy + second.destroy + other.destroy + end + + it "groups authorizations for the same app and device into one card" do + visit apps_services_storage_path + + expect(page).to have_selector(".rs-auth-card", count: 2) + expect(page).to have_content("Firefox on Linux") + expect(page).to have_content("Chrome on macOS") + expect(page).to have_content("Tokens") + end + + it "lists every authorization in the group with its own revoke action" do + visit apps_services_storage_path + + expect(page).to have_selector("a", text: "Revoke access", visible: false, count: 3) + end + end + + context "without connected apps" do + it "shows the empty state" do + visit apps_services_storage_path + + expect(page).to have_content("No apps connected") + end + end +end diff --git a/spec/models/remote_storage_authorization_spec.rb b/spec/models/remote_storage_authorization_spec.rb index 378ebb6..70e0377 100644 --- a/spec/models/remote_storage_authorization_spec.rb +++ b/spec/models/remote_storage_authorization_spec.rb @@ -303,4 +303,55 @@ RSpec.describe RemoteStorageAuthorization, type: :model do end end end + describe "#expired?" do + def auth_with_expiry(expire_at) + RemoteStorageAuthorization.new( + user: user, permissions: %w(documents:rw), client_id: "example.com", + redirect_uri: "https://example.com", expire_at: expire_at + ) + end + + it "is false without an expiry" do + expect(auth_with_expiry(nil)).not_to be_expired + end + + it "is false before the expiry" do + expect(auth_with_expiry(1.day.from_now)).not_to be_expired + end + + it "is true after the expiry" do + expect(auth_with_expiry(1.day.ago)).to be_expired + end + end + + describe "#device_name" do + def auth_with_user_agent(user_agent) + RemoteStorageAuthorization.new(user: user, user_agent: user_agent) + end + + it "returns nil without a user agent" do + expect(auth_with_user_agent(nil).device_name).to be_nil + end + + it "recognizes Firefox on Linux" do + ua = "Mozilla/5.0 (X11; Linux x86_64; rv:120.0) Gecko/20100101 Firefox/120.0" + expect(auth_with_user_agent(ua).device_name).to eq("Firefox on Linux") + end + + it "recognizes Chrome on macOS" do + ua = "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 " \ + "(KHTML, like Gecko) Chrome/119.0.0.0 Safari/537.36" + expect(auth_with_user_agent(ua).device_name).to eq("Chrome on macOS") + end + + it "recognizes Safari on iOS" do + ua = "Mozilla/5.0 (iPhone; CPU iPhone OS 17_1 like Mac OS X) " \ + "AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.1 Mobile/15E148 Safari/604.1" + expect(auth_with_user_agent(ua).device_name).to eq("Safari on iOS") + end + + it "falls back to the browser when the OS is unknown" do + expect(auth_with_user_agent("Firefox/120.0").device_name).to eq("Firefox") + end + end end From c4df1772e0e79565b631fbc837f967f98e7dadc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Mon, 2 Nov 2026 22:58:49 +0400 Subject: [PATCH 05/12] Determine device names with device_detector Replace the hand-rolled user-agent regexes with the device_detector gem. Its UA database recognises many more clients (and, e.g., GNOME Web when its UA carries the Epiphany token). WebKit "Safari" user agents on Linux (WebKitGTK, which GNOME Web sends) are labelled "WebKit on Linux", since Safari never runs on Linux. Part of #254 --- Gemfile | 1 + Gemfile.lock | 2 ++ app/models/remote_storage_authorization.rb | 26 +++++++------------ .../remote_storage_authorization_spec.rb | 22 ++++++++++++++-- 4 files changed, 32 insertions(+), 19 deletions(-) diff --git a/Gemfile b/Gemfile index 5ec961f..e41a186 100644 --- a/Gemfile +++ b/Gemfile @@ -43,6 +43,7 @@ gem 'flipper-ui' gem 'gpgme', '~> 2.0.24' gem 'zbase32', '~> 0.1.1' gem 'kramdown' +gem 'device_detector' # HTTP requests gem 'faraday' diff --git a/Gemfile.lock b/Gemfile.lock index 07a76dc..9838525 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -136,6 +136,7 @@ GEM database_cleaner-core (~> 2.0.0) database_cleaner-core (2.0.1) date (3.5.1) + device_detector (1.1.3) devise (5.0.4) bcrypt (~> 3.0) orm_adapter (~> 0.1) @@ -543,6 +544,7 @@ DEPENDENCIES capybara cssbundling-rails database_cleaner + device_detector devise (~> 5.0) devise_ldap_authenticatable discourse_api diff --git a/app/models/remote_storage_authorization.rb b/app/models/remote_storage_authorization.rb index b27e5c2..d7011e2 100644 --- a/app/models/remote_storage_authorization.rb +++ b/app/models/remote_storage_authorization.rb @@ -47,25 +47,17 @@ class RemoteStorageAuthorization < ApplicationRecord def device_name return nil if user_agent.blank? - browser = - case user_agent - when /Firefox/ then "Firefox" - when /Edg\// then "Edge" - when /Chrome/ then "Chrome" - when /Safari/ then "Safari" - else "Unknown browser" - end + client = DeviceDetector.new(user_agent) + name = client.name.presence + os = client.os_name.presence + os = "Linux" if os == "GNU/Linux" + os = "macOS" if os == "Mac" - os = - case user_agent - when /Android/ then "Android" - when /iPhone|iPad|iPod/ then "iOS" - when /Mac OS X/ then "macOS" - when /Windows/ then "Windows" - when /Linux/ then "Linux" - end + # Safari never runs on Linux. A WebKit "Safari" user agent on Linux is a + # WebKitGTK browser (e.g. GNOME Web), which sends a Safari-compatible UA. + name = "WebKit" if name == "Safari" && os == "Linux" - os ? "#{browser} on #{os}" : browser + [name, os].compact.join(" on ").presence end def delete_token_from_redis diff --git a/spec/models/remote_storage_authorization_spec.rb b/spec/models/remote_storage_authorization_spec.rb index 70e0377..51cd7a0 100644 --- a/spec/models/remote_storage_authorization_spec.rb +++ b/spec/models/remote_storage_authorization_spec.rb @@ -344,10 +344,28 @@ RSpec.describe RemoteStorageAuthorization, type: :model do expect(auth_with_user_agent(ua).device_name).to eq("Chrome on macOS") end - it "recognizes Safari on iOS" do + it "recognizes Mobile Safari on iOS" do ua = "Mozilla/5.0 (iPhone; CPU iPhone OS 17_1 like Mac OS X) " \ "AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.1 Mobile/15E148 Safari/604.1" - expect(auth_with_user_agent(ua).device_name).to eq("Safari on iOS") + expect(auth_with_user_agent(ua).device_name).to eq("Mobile Safari on iOS") + end + + it "recognizes GNOME Web on Linux (Epiphany's Safari-like user agent)" do + ua = "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/605.1.15 (KHTML, like Gecko) " \ + "Version/16.0 Safari/605.1.15 Epiphany/45.0" + expect(auth_with_user_agent(ua).device_name).to eq("GNOME Web on Linux") + end + + it "treats a WebKit Safari user agent on Linux as WebKit (WebKitGTK)" do + ua = "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/605.1.15 (KHTML, like Gecko) " \ + "Version/60.5 Safari/605.1.15" + expect(auth_with_user_agent(ua).device_name).to eq("WebKit on Linux") + end + + it "keeps Safari on macOS as Safari" do + ua = "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 " \ + "(KHTML, like Gecko) Version/17.1 Safari/605.1.15" + expect(auth_with_user_agent(ua).device_name).to eq("Safari on macOS") end it "falls back to the browser when the OS is unknown" do From f2248ea744eee6c46741c0d9df92ba9d19d99a17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Mon, 2 Nov 2026 22:59:06 +0400 Subject: [PATCH 06/12] Rework storage dashboard and fix authorization revocation - One card per app (grouped by client_id) showing distinct device and category counts; the whole card opens the details modal (with hover and focus highlight) instead of a separate Details button - Details modal: access list styled like the OAuth consent dialog via a shared RsScopeListComponent, plus per-token device, connected date and expiry, each with its own Revoke - "folder" -> "category" in the card, modal, consent dialog and the auth-created notification - Make revoke a real DELETE: it was a GET link that Turbo 8's prefetch-on-hover fired on its own, silently deleting authorizations. Missing or foreign authorizations now return 404 instead of 500. - Move "Launch app" to the app level (GET /services/storage/apps/:client_id/launch); no token id in the URL Part of #254 --- app/components/rs_auth_component.html.erb | 53 ++++++--------- app/components/rs_auth_component.rb | 21 ++---- .../rs_auth_details_modal_component.html.erb | 53 +++++++-------- .../rs_auth_details_modal_component.rb | 9 +-- .../rs_scope_list_component.html.erb | 25 +++++++ app/components/rs_scope_list_component.rb | 14 ++++ .../services/rs_auths_controller.rb | 26 ++++--- app/mailers/notification_mailer.rb | 2 +- app/views/rs/oauth/new.html.erb | 28 +------- app/views/services/rs_auths/index.html.erb | 4 +- config/routes.rb | 9 +-- .../services/rs_auths_controller_spec.rb | 56 ++++++++++++++-- spec/features/rs/oauth_spec.rb | 2 +- spec/features/services/rs_auths_spec.rb | 67 ++++++++++++------- 14 files changed, 213 insertions(+), 156 deletions(-) create mode 100644 app/components/rs_scope_list_component.html.erb create mode 100644 app/components/rs_scope_list_component.rb diff --git a/app/components/rs_auth_component.html.erb b/app/components/rs_auth_component.html.erb index 863af3a..205a760 100644 --- a/app/components/rs_auth_component.html.erb +++ b/app/components/rs_auth_component.html.erb @@ -1,41 +1,28 @@ -
-
-
+
+ -
+ + <%= render RsAuthDetailsModalComponent.new(authorizations: @authorizations) %>
diff --git a/app/components/rs_auth_component.rb b/app/components/rs_auth_component.rb index 56e003a..c5f90a1 100644 --- a/app/components/rs_auth_component.rb +++ b/app/components/rs_auth_component.rb @@ -17,25 +17,18 @@ class RsAuthComponent < ViewComponent::Base (@auth.permissions & [":r", ":rw"]).any? end - def scope_summary - return "All folders and files" if root_access? + def category_summary + return "All categories and files" if root_access? count = @auth.permissions.size - "#{count} #{'folder'.pluralize(count)}" + "#{count} #{'category'.pluralize(count)}" end - def device_name - @auth.device_name.presence || "Unknown device" + def device_count + @authorizations.map { |auth| auth.device_name.presence || "Unknown device" }.uniq.size end - def expiry_label - return "Never expires" if @auth.expire_at.blank? - return "Expired" if @auth.expired? - - "Expires #{@auth.expire_at.strftime('%-d %b %Y')}" - end - - def multiple_authorizations? - @authorizations.size > 1 + def device_summary + "#{device_count} #{'device'.pluralize(device_count)}" end end diff --git a/app/components/rs_auth_details_modal_component.html.erb b/app/components/rs_auth_details_modal_component.html.erb index ce0233a..7955bfe 100644 --- a/app/components/rs_auth_details_modal_component.html.erb +++ b/app/components/rs_auth_details_modal_component.html.erb @@ -1,47 +1,42 @@ <%= render ModalComponent.new do %>
-
+
<%= render AppCatalog::WebAppIconComponent.new(web_app: @web_app) %>
-

<%= name %>

+

<%= name %>

<%= @auth.client_id %>

-
Access
- <% if root_access? %> -

All folders and files

- <% else %> -
    - <% @auth.permissions.each do |scope| %> -
  • <%= scope_label(scope) %>
  • - <% end %> -
- <% end %> - -
Device
-

<%= device_name %>

- -
- <%= @authorizations.size > 1 ? "Connected tokens" : "Connected" %> +
+ Access
-
    + <%= render RsScopeListComponent.new(scopes: @auth.permissions, root: root_access?) %> + +
    + Connected devices +
    +
      <% @authorizations.each do |auth| %> -
    • - - <%= auth.created_at.strftime('%-d %b %Y') %> - · - <%= status_label(auth) %> - - <%= link_to "Revoke access", revoke_services_storage_rs_auth_path(auth), - class: "text-red-700 whitespace-nowrap" %> +
    • +
      +

      <%= device_name(auth) %>

      +

      + Connected <%= auth.created_at.strftime('%-d %b %Y') %> + · + "><%= status_label(auth) %> +

      +
      + <%= button_to "Revoke access", services_storage_rs_auth_path(auth), method: :delete, + form: { class: "flex-none" }, + class: "whitespace-nowrap text-sm text-red-700 hover:underline bg-transparent border-0 p-0 cursor-pointer" %>
    • <% end %>
    - <%= link_to "Launch app", launch_app_services_storage_rs_auth_path(@auth), - class: "btn-md btn-blue", target: "_blank", rel: "noopener" %> + <%= link_to "Launch app", launch_app_services_storage_path(@auth.client_id), + class: "btn-md btn-blue", target: "_blank", rel: "noopener", data: { turbo: false } %>
    <% end %> diff --git a/app/components/rs_auth_details_modal_component.rb b/app/components/rs_auth_details_modal_component.rb index 257329c..3bd9481 100644 --- a/app/components/rs_auth_details_modal_component.rb +++ b/app/components/rs_auth_details_modal_component.rb @@ -17,13 +17,8 @@ class RsAuthDetailsModalComponent < ViewComponent::Base (@auth.permissions & [":r", ":rw"]).any? end - def scope_label(scope) - permission = scope_permissions(scope) == "r" ? "read only" : "read/write" - "#{scope_name(scope)} (#{permission})" - end - - def device_name - @auth.device_name.presence || "Unknown device" + def device_name(auth) + auth.device_name.presence || "Unknown device" end def status_label(auth) diff --git a/app/components/rs_scope_list_component.html.erb b/app/components/rs_scope_list_component.html.erb new file mode 100644 index 0000000..42ae256 --- /dev/null +++ b/app/components/rs_scope_list_component.html.erb @@ -0,0 +1,25 @@ +<% if @root %> +

    + + <%= render partial: "icons/alert-triangle", + locals: { custom_class: "inline-block align-bottom mr-1.5" } %> + All categories and files + + <% if read_only? %> + (read only) + <% end %> +

    +<% else %> + <% @scopes.each do |scope| %> +

    + + <%= render partial: "icons/folder", + locals: { custom_class: "inline-block align-bottom mr-1.5" } %> + <%= scope_name(scope) %> + + <% if scope_permissions(scope) == "r" %> + (read only) + <% end %> +

    + <% end %> +<% end %> diff --git a/app/components/rs_scope_list_component.rb b/app/components/rs_scope_list_component.rb new file mode 100644 index 0000000..198ef08 --- /dev/null +++ b/app/components/rs_scope_list_component.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class RsScopeListComponent < ViewComponent::Base + include OauthHelper + + def initialize(scopes:, root: false) + @scopes = scopes + @root = root + end + + def read_only? + @scopes.include?(":r") + end +end diff --git a/app/controllers/services/rs_auths_controller.rb b/app/controllers/services/rs_auths_controller.rb index 3518b2a..b60a684 100644 --- a/app/controllers/services/rs_auths_controller.rb +++ b/app/controllers/services/rs_auths_controller.rb @@ -2,15 +2,15 @@ class Services::RsAuthsController < Services::BaseController before_action :authenticate_user! before_action :require_service_enabled before_action :require_service_available - before_action :find_rs_auth, only: [:destroy, :launch_app] + before_action :find_rs_auth, only: [:destroy] def index - @rs_auth_groups = current_user.remote_storage_authorizations - .includes(:web_app) - .group_by { |auth| [auth.client_id, auth.device_name] } - .values - .map { |auths| auths.sort_by(&:created_at).reverse } - .sort_by { |auths| app_name_for(auths.first).downcase } + @rs_apps = current_user.remote_storage_authorizations + .includes(:web_app) + .group_by(&:client_id) + .values + .map { |auths| auths.sort_by(&:created_at).reverse } + .sort_by { |auths| app_name_for(auths.first).downcase } end def destroy @@ -26,6 +26,12 @@ class Services::RsAuthsController < Services::BaseController end def launch_app + auth = current_user.remote_storage_authorizations + .where(client_id: params[:client_id]) + .order(created_at: :desc) + .first + http_status(:not_found) and return unless auth + user_address = if Rails.env.development? "#{current_user.cn}@#{request.host_with_port}" @@ -35,7 +41,7 @@ class Services::RsAuthsController < Services::BaseController # Storage-first bearer token issuance (remoteStorage spec §11): the app # receives the address and runs the application-first flow itself. - launch_url = "#{@auth.launch_url}#remotestorage=#{user_address}" + launch_url = "#{auth.launch_url}#remotestorage=#{user_address}" redirect_to launch_url, allow_other_host: true end @@ -57,7 +63,7 @@ class Services::RsAuthsController < Services::BaseController end def find_rs_auth - @auth = current_user.remote_storage_authorizations.find(params[:id]) - http_status :not_found unless @auth.present? + @auth = current_user.remote_storage_authorizations.find_by(id: params[:id]) + http_status :not_found unless @auth end end diff --git a/app/mailers/notification_mailer.rb b/app/mailers/notification_mailer.rb index d281380..0f2414b 100644 --- a/app/mailers/notification_mailer.rb +++ b/app/mailers/notification_mailer.rb @@ -11,7 +11,7 @@ class NotificationMailer < ApplicationMailer @auth = params[:auth] @permissions = @auth.permissions.map do |p| access = p.split(":")[1] == 'r' ? 'read' : 'read/write' - directory = p.split(':')[0] == '' ? 'all folders and files' : p.split(':')[0] + directory = p.split(':')[0] == '' ? 'all categories and files' : p.split(':')[0] "#{access} #{directory}" end @subject = "New app connected to your storage" diff --git a/app/views/rs/oauth/new.html.erb b/app/views/rs/oauth/new.html.erb index 4b7bc25..2668c14 100644 --- a/app/views/rs/oauth/new.html.erb +++ b/app/views/rs/oauth/new.html.erb @@ -5,34 +5,10 @@

    The app on <%= link_to @client_id, "https://#{@client_id}" %> - is asking for access to these folders: + is asking for access to these categories:

    - <% if @root_access_requested %> -

    - - <%= render partial: "icons/alert-triangle", - locals: { custom_class: "inline-block align-bottom mr-1.5" } %> - All files and directories - - <% if (@scopes & [":r"]).any? %> - (read only) - <% end %> -

    - <% else %> - <% @scopes.each do |scope| %> -

    - - <%= render partial: "icons/folder", - locals: { custom_class: "inline-block align-bottom mr-1.5" } %> - <%= scope_name(scope) %> - - <% if scope_permissions(scope) == "r" %> - (read only) - <% end %> -

    - <% end %> - <% end %> + <%= render RsScopeListComponent.new(scopes: @scopes, root: @root_access_requested) %> <%= form_with(url: rs_oauth_path, method: :post, data: { turbo: false }) do |f| %> <%= f.hidden_field :redirect_uri, value: @redirect_uri %> diff --git a/app/views/services/rs_auths/index.html.erb b/app/views/services/rs_auths/index.html.erb index 9141066..edd6ade 100644 --- a/app/views/services/rs_auths/index.html.erb +++ b/app/views/services/rs_auths/index.html.erb @@ -10,9 +10,9 @@ <%= render partial: "shared/tabnav_remotestorage" %>
    - <% if @rs_auth_groups.any? %> + <% if @rs_apps.any? %>
    - <% @rs_auth_groups.each do |authorizations| %> + <% @rs_apps.each do |authorizations| %> <%= render RsAuthComponent.new(authorizations: authorizations) %> <% end %>
    diff --git a/config/routes.rb b/config/routes.rb index d8af5b2..5b78bad 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -52,12 +52,9 @@ Rails.application.routes.draw do resource :storage, controller: 'remotestorage', only: [:show] do get :apps, to: "rs_auths#index" - resources :rs_auths, only: [:index, :destroy] do - member do - get :revoke, to: 'rs_auths#destroy' - get :launch_app - end - end + get "apps/:client_id/launch", to: "rs_auths#launch_app", as: :launch_app, + constraints: { client_id: /[^\/]+/ } + resources :rs_auths, only: [:index, :destroy] end end diff --git a/spec/controllers/services/rs_auths_controller_spec.rb b/spec/controllers/services/rs_auths_controller_spec.rb index 812a842..0970c3c 100644 --- a/spec/controllers/services/rs_auths_controller_spec.rb +++ b/spec/controllers/services/rs_auths_controller_spec.rb @@ -11,13 +11,13 @@ RSpec.describe Services::RsAuthsController, type: :controller do ]) end - describe "GET /services/storage/rs_auths/:id/launch_app (storage-first)" do + describe "GET /services/storage/apps/:client_id/launch (storage-first)" do context "when user is signed in" do before do sign_in user end - context "token exists" do + context "authorization exists" do before do @auth = user.remote_storage_authorizations.create!( permissions: %w(documents), client_id: "app.example.com", @@ -25,7 +25,7 @@ RSpec.describe Services::RsAuthsController, type: :controller do expire_at: 2.days.from_now ) - get :launch_app, params: { id: @auth.id } + get :launch_app, params: { client_id: @auth.client_id } end after do @@ -48,7 +48,7 @@ RSpec.describe Services::RsAuthsController, type: :controller do { services_enabled: [] } ]) - get :launch_app, params: { id: 1 } + get :launch_app, params: { client_id: "app.example.com" } end it "responds with forbidden" do @@ -57,4 +57,52 @@ RSpec.describe Services::RsAuthsController, type: :controller do end end end + + describe "GET /services/storage/apps/:client_id/launch for an unknown app" do + before { sign_in user } + + it "returns 404 instead of raising" do + get :launch_app, params: { client_id: "nonexistent.example.com" } + + expect(response).to have_http_status(:not_found) + end + end + + describe "DELETE /services/storage/rs_auths/:id" do + context "when user is signed in" do + before { sign_in user } + + it "revokes the authorization" do + auth = user.remote_storage_authorizations.create!( + permissions: %w(documents), client_id: "app.example.com", + redirect_uri: "https://app.example.com", expire_at: 2.days.from_now + ) + + expect { + delete :destroy, params: { id: auth.id } + }.to change(RemoteStorageAuthorization, :count).by(-1) + + expect(response).to redirect_to(apps_services_storage_url) + end + + it "returns 404 for an unknown authorization" do + delete :destroy, params: { id: 0 } + + expect(response).to have_http_status(:not_found) + end + + it "returns 404 for another user's authorization" do + other = create :user, id: 99, cn: "otheruser", email: "other@example.com" + auth = other.remote_storage_authorizations.create!( + permissions: %w(documents), client_id: "app.example.com", + redirect_uri: "https://app.example.com" + ) + + delete :destroy, params: { id: auth.id } + + expect(response).to have_http_status(:not_found) + expect(RemoteStorageAuthorization.exists?(auth.id)).to be(true) + end + end + end end diff --git a/spec/features/rs/oauth_spec.rb b/spec/features/rs/oauth_spec.rb index de0527d..085198e 100644 --- a/spec/features/rs/oauth_spec.rb +++ b/spec/features/rs/oauth_spec.rb @@ -44,7 +44,7 @@ RSpec.describe 'remoteStorage OAuth Dialog', type: :feature do it "shows a special permission for all files and dirs" do within ".scope" do - expect(page).to have_content("All files and directories") + expect(page).to have_content("All categories and files") end end end diff --git a/spec/features/services/rs_auths_spec.rb b/spec/features/services/rs_auths_spec.rb index 9fd69b0..6cec9a7 100644 --- a/spec/features/services/rs_auths_spec.rb +++ b/spec/features/services/rs_auths_spec.rb @@ -44,50 +44,71 @@ RSpec.describe "Storage connected apps", type: :feature do after { auth.destroy } - it "shows the app, its device and permission status" do + it "shows the app with device and category counts" do visit apps_services_storage_path - expect(page).to have_content("Notes") - expect(page).to have_content("notes.example.com") - expect(page).to have_content("Firefox on Linux") - expect(page).to have_content("2 folders") - expect(page).to have_content("Expires") + within ".rs-auth-card" do + expect(page).to have_content("Notes") + expect(page).to have_content("notes.example.com") + expect(page).to have_content("1 device") + expect(page).to have_content("2 categories") + end end - it "renders a details modal with scopes and actions" do + it "does not show the permission expiry on the card" do visit apps_services_storage_path - expect(page).to have_selector("li", text: "documents (read/write)", visible: false) - expect(page).to have_selector("li", text: "photos (read only)", visible: false) + within ".rs-auth-card" do + expect(page).not_to have_content("Expires") + end + end + + it "renders access and per-token details in the modal" do + visit apps_services_storage_path + + expect(page).to have_selector("p.scope", text: "documents", visible: false) + expect(page).to have_selector("p.scope", text: "photos", visible: false) + expect(page).to have_selector("h5", text: "Connected devices", visible: false) + expect(page).to have_selector("p", text: "Firefox on Linux", visible: false) + expect(page).to have_selector("p", text: /Expires/, visible: false) + expect(page).to have_button("Revoke access", visible: false) expect(page).to have_link("Launch app", visible: false) - expect(page).to have_selector("a", text: "Revoke access", visible: false, count: 1) end end - context "with duplicates" do - let!(:first) { create_auth(client: "notes.example.com", device: firefox_ua) } - let!(:second) { create_auth(client: "notes.example.com", device: firefox_ua) } - let!(:other) { create_auth(client: "notes.example.com", device: chrome_ua) } + context "with multiple apps" do + let!(:notes) { create_auth(client: "notes.example.com", device: firefox_ua) } + let!(:photos) { create_auth(client: "photos.example.com", device: firefox_ua) } after do - first.destroy - second.destroy - other.destroy + notes.destroy + photos.destroy end - it "groups authorizations for the same app and device into one card" do + it "shows one card per app" do visit apps_services_storage_path expect(page).to have_selector(".rs-auth-card", count: 2) - expect(page).to have_content("Firefox on Linux") - expect(page).to have_content("Chrome on macOS") - expect(page).to have_content("Tokens") + end + end + + context "with one app on multiple devices" do + let!(:laptop) { create_auth(client: "notes.example.com", device: firefox_ua) } + let!(:phone) { create_auth(client: "notes.example.com", device: chrome_ua) } + + after do + laptop.destroy + phone.destroy end - it "lists every authorization in the group with its own revoke action" do + it "shows a single card with the device count" do visit apps_services_storage_path - expect(page).to have_selector("a", text: "Revoke access", visible: false, count: 3) + expect(page).to have_selector(".rs-auth-card", count: 1) + + within ".rs-auth-card" do + expect(page).to have_content("2 devices") + end end end From 27168dde0639418f436a1f206943fa56a09c6a11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Tue, 3 Nov 2026 11:44:27 +0400 Subject: [PATCH 07/12] Address review nits in the storage dashboard - Move the app display name and a root-access helper onto RemoteStorageAuthorization, used by both components and the controller, removing the duplicated fallback chain and the unused OauthHelper includes - Summarise the card's categories from the union of the app's authorizations' scopes, so an older token with broader access is not understated Part of #254 --- app/components/rs_auth_component.html.erb | 2 +- app/components/rs_auth_component.rb | 12 ++++++------ .../rs_auth_details_modal_component.html.erb | 4 ++-- app/components/rs_auth_details_modal_component.rb | 10 ---------- app/controllers/services/rs_auths_controller.rb | 2 +- app/models/remote_storage_authorization.rb | 8 ++++++++ 6 files changed, 18 insertions(+), 20 deletions(-) diff --git a/app/components/rs_auth_component.html.erb b/app/components/rs_auth_component.html.erb index 205a760..64ea405 100644 --- a/app/components/rs_auth_component.html.erb +++ b/app/components/rs_auth_component.html.erb @@ -10,7 +10,7 @@
-

<%= name %>

+

<%= @auth.display_name %>

<%= @auth.client_id %>

<%= device_summary %> diff --git a/app/components/rs_auth_component.rb b/app/components/rs_auth_component.rb index c5f90a1..b80d225 100644 --- a/app/components/rs_auth_component.rb +++ b/app/components/rs_auth_component.rb @@ -1,26 +1,26 @@ # frozen_string_literal: true class RsAuthComponent < ViewComponent::Base - include OauthHelper - def initialize(authorizations:) @authorizations = authorizations @auth = authorizations.first @web_app = @auth.web_app end - def name - @web_app&.name.presence || @auth.app_name.presence || @auth.client_id + # The card summarises the app, so reflect the union of its authorizations' + # scopes (an older token may have broader access than the newest one). + def permissions + @authorizations.flat_map(&:permissions).uniq end def root_access? - (@auth.permissions & [":r", ":rw"]).any? + (permissions & [":r", ":rw"]).any? end def category_summary return "All categories and files" if root_access? - count = @auth.permissions.size + count = permissions.size "#{count} #{'category'.pluralize(count)}" end diff --git a/app/components/rs_auth_details_modal_component.html.erb b/app/components/rs_auth_details_modal_component.html.erb index 7955bfe..dcb64b1 100644 --- a/app/components/rs_auth_details_modal_component.html.erb +++ b/app/components/rs_auth_details_modal_component.html.erb @@ -4,7 +4,7 @@ <%= render AppCatalog::WebAppIconComponent.new(web_app: @web_app) %>

-

<%= name %>

+

<%= @auth.display_name %>

<%= @auth.client_id %>

@@ -12,7 +12,7 @@
Access
- <%= render RsScopeListComponent.new(scopes: @auth.permissions, root: root_access?) %> + <%= render RsScopeListComponent.new(scopes: @auth.permissions, root: @auth.root_access?) %>
Connected devices diff --git a/app/components/rs_auth_details_modal_component.rb b/app/components/rs_auth_details_modal_component.rb index 3bd9481..0f281cc 100644 --- a/app/components/rs_auth_details_modal_component.rb +++ b/app/components/rs_auth_details_modal_component.rb @@ -1,22 +1,12 @@ # frozen_string_literal: true class RsAuthDetailsModalComponent < ViewComponent::Base - include OauthHelper - def initialize(authorizations:) @authorizations = authorizations @auth = authorizations.first @web_app = @auth.web_app end - def name - @web_app&.name.presence || @auth.app_name.presence || @auth.client_id - end - - def root_access? - (@auth.permissions & [":r", ":rw"]).any? - end - def device_name(auth) auth.device_name.presence || "Unknown device" end diff --git a/app/controllers/services/rs_auths_controller.rb b/app/controllers/services/rs_auths_controller.rb index b60a684..f4df2c1 100644 --- a/app/controllers/services/rs_auths_controller.rb +++ b/app/controllers/services/rs_auths_controller.rb @@ -49,7 +49,7 @@ class Services::RsAuthsController < Services::BaseController private def app_name_for(auth) - auth.web_app&.name.presence || auth.app_name.presence || auth.client_id + auth.display_name end def require_service_enabled diff --git a/app/models/remote_storage_authorization.rb b/app/models/remote_storage_authorization.rb index d7011e2..cf5167f 100644 --- a/app/models/remote_storage_authorization.rb +++ b/app/models/remote_storage_authorization.rb @@ -60,6 +60,14 @@ class RemoteStorageAuthorization < ApplicationRecord [name, os].compact.join(" on ").presence end + def display_name + web_app&.name.presence || app_name.presence || client_id + end + + def root_access? + (permissions & [":r", ":rw"]).any? + end + def delete_token_from_redis key = "authorizations:#{user.cn}:#{token}" redis.srem? key, redis.smembers(key) From 30e45d3d34b9a6f9bd37bd807af74d95bb5e9769 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Tue, 3 Nov 2026 12:01:56 +0400 Subject: [PATCH 08/12] Address second-round review nits in the storage dashboard Show the union of the app's authorizations' scopes in the modal's Access section too, so it matches the card's summary, and cover the union summary with a feature spec. Part of #254 --- .../rs_auth_details_modal_component.html.erb | 2 +- .../rs_auth_details_modal_component.rb | 9 ++++++ spec/features/services/rs_auths_spec.rb | 28 +++++++++++++++++++ 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/app/components/rs_auth_details_modal_component.html.erb b/app/components/rs_auth_details_modal_component.html.erb index dcb64b1..bf86eef 100644 --- a/app/components/rs_auth_details_modal_component.html.erb +++ b/app/components/rs_auth_details_modal_component.html.erb @@ -12,7 +12,7 @@
Access
- <%= render RsScopeListComponent.new(scopes: @auth.permissions, root: @auth.root_access?) %> + <%= render RsScopeListComponent.new(scopes: permissions, root: root_access?) %>
Connected devices diff --git a/app/components/rs_auth_details_modal_component.rb b/app/components/rs_auth_details_modal_component.rb index 0f281cc..46daa37 100644 --- a/app/components/rs_auth_details_modal_component.rb +++ b/app/components/rs_auth_details_modal_component.rb @@ -7,6 +7,15 @@ class RsAuthDetailsModalComponent < ViewComponent::Base @web_app = @auth.web_app end + # Reflect the union of the app's scopes, matching the card's summary. + def permissions + @authorizations.flat_map(&:permissions).uniq + end + + def root_access? + (permissions & [":r", ":rw"]).any? + end + def device_name(auth) auth.device_name.presence || "Unknown device" end diff --git a/spec/features/services/rs_auths_spec.rb b/spec/features/services/rs_auths_spec.rb index 6cec9a7..7496ba0 100644 --- a/spec/features/services/rs_auths_spec.rb +++ b/spec/features/services/rs_auths_spec.rb @@ -112,6 +112,34 @@ RSpec.describe "Storage connected apps", type: :feature do end end + context "with one app on multiple devices and differing scopes" do + let!(:laptop) do + create_auth(client: "notes.example.com", device: firefox_ua, permissions: %w(documents:rw)) + end + + let!(:phone) do + create_auth(client: "notes.example.com", device: chrome_ua, + permissions: %w(documents:rw photos:r)) + end + + after do + laptop.destroy + phone.destroy + end + + it "summarises the union of the app's scopes in the card and the modal" do + visit apps_services_storage_path + + within ".rs-auth-card" do + expect(page).to have_content("2 devices") + expect(page).to have_content("2 categories") + end + + expect(page).to have_selector("p.scope", text: "documents", visible: false) + expect(page).to have_selector("p.scope", text: "photos", visible: false) + end + end + context "without connected apps" do it "shows the empty state" do visit apps_services_storage_path From d94fda6d26960629d5edc453417051d9a4f348e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Tue, 3 Nov 2026 14:36:39 +0400 Subject: [PATCH 09/12] Polish the storage dashboard and details modal Visual refinement pass on the connected-apps dashboard introduced for #254. Card: - Restructure into a header row (app icon, name, host, chevron) with the device/category summary as a full-width footer, so the icon aligns with the name and the metrics sit on a consistent baseline - Always show the host alongside the app name, except when it is identical - Render the icon fallback as a neutral rounded tile instead of a faint watermark, and stop distorting real icons (object-contain) - Summarise the union of the app's scopes, with monitor/folder icons Details modal: - Replace the duplicated blue buttons with a secondary Close and a primary Launch app, split 50:50 on desktop like the consent dialog - Tighten the header, section spacing and device rows; hide a redundant host - Add exact date/time tooltips to the connected and expiry labels - Align the scope-list folder icons with their labels Misc: - Use fewer near-identical grays across the cards and modal - Drop the redundant top margin on the app grid Part of #254 --- .../web_app_icon_component.html.erb | 6 ++- app/components/rs_auth_component.html.erb | 42 ++++++++++++------- .../rs_auth_details_modal_component.html.erb | 37 ++++++++++------ .../rs_auth_details_modal_component.rb | 8 ++++ .../rs_scope_list_component.html.erb | 39 ++++++++--------- app/components/rs_scope_list_component.rb | 7 +++- app/views/services/rs_auths/index.html.erb | 2 +- 7 files changed, 87 insertions(+), 54 deletions(-) diff --git a/app/components/app_catalog/web_app_icon_component.html.erb b/app/components/app_catalog/web_app_icon_component.html.erb index 3f9c5f3..37af872 100644 --- a/app/components/app_catalog/web_app_icon_component.html.erb +++ b/app/components/app_catalog/web_app_icon_component.html.erb @@ -1,5 +1,7 @@ <% if @image_url %> - <%= image_tag @image_url, class: "h-full w-full" %> + <%= image_tag @image_url, class: "h-full w-full rounded-lg object-contain" %> <% else %> - <%= render partial: "icons/remotestorage", locals: { custom_class: "h-full w-full p-0.5 text-gray-200" } %> + + <%= render partial: "icons/remotestorage", locals: { custom_class: "h-1/2 w-1/2 text-gray-400" } %> + <% end %> diff --git a/app/components/rs_auth_component.html.erb b/app/components/rs_auth_component.html.erb index 64ea405..3bc335e 100644 --- a/app/components/rs_auth_component.html.erb +++ b/app/components/rs_auth_component.html.erb @@ -1,26 +1,36 @@
diff --git a/app/components/rs_auth_details_modal_component.html.erb b/app/components/rs_auth_details_modal_component.html.erb index bf86eef..6fd0dfe 100644 --- a/app/components/rs_auth_details_modal_component.html.erb +++ b/app/components/rs_auth_details_modal_component.html.erb @@ -1,31 +1,37 @@ -<%= render ModalComponent.new do %> +<%= render ModalComponent.new(show_close_button: false) do %>
-
+
<%= render AppCatalog::WebAppIconComponent.new(web_app: @web_app) %>
-

<%= @auth.display_name %>

-

<%= @auth.client_id %>

+

<%= @auth.display_name %>

+ <% if @auth.client_id != @auth.display_name %> +

<%= @auth.client_id %>

+ <% end %>
-
+
Access
- <%= render RsScopeListComponent.new(scopes: permissions, root: root_access?) %> + <%= render RsScopeListComponent.new(scopes: permissions, root: root_access?, + icon_class: "h-5 w-5") %> -
+
Connected devices
-
    +
      <% @authorizations.each do |auth| %>
    • -

      <%= device_name(auth) %>

      +

      <%= device_name(auth) %>

      - Connected <%= auth.created_at.strftime('%-d %b %Y') %> + <%= tag.span "Connected #{auth.created_at.strftime('%-d %b %Y')}", + title: connected_at_title(auth) %> · - "><%= status_label(auth) %> + <%= tag.span status_label(auth), + title: (expires_at_title(auth) if auth.expire_at.present?), + class: ("text-red-700" if auth.expired?) %>

      <%= button_to "Revoke access", services_storage_rs_auth_path(auth), method: :delete, @@ -35,8 +41,13 @@ <% end %>
    -
    +
    <%= link_to "Launch app", launch_app_services_storage_path(@auth.client_id), - class: "btn-md btn-blue", target: "_blank", rel: "noopener", data: { turbo: false } %> + class: "btn-md btn-blue w-full sm:grow sm:order-last text-center", + target: "_blank", rel: "noopener", data: { turbo: false } %> +
    <% end %> diff --git a/app/components/rs_auth_details_modal_component.rb b/app/components/rs_auth_details_modal_component.rb index 46daa37..20c55d5 100644 --- a/app/components/rs_auth_details_modal_component.rb +++ b/app/components/rs_auth_details_modal_component.rb @@ -26,4 +26,12 @@ class RsAuthDetailsModalComponent < ViewComponent::Base "Expires #{auth.expire_at.strftime('%-d %b %Y')}" end + + def connected_at_title(auth) + auth.created_at.strftime("%-d %b %Y, %H:%M") + end + + def expires_at_title(auth) + auth.expire_at.strftime("%-d %b %Y, %H:%M") + end end diff --git a/app/components/rs_scope_list_component.html.erb b/app/components/rs_scope_list_component.html.erb index 42ae256..7391b3d 100644 --- a/app/components/rs_scope_list_component.html.erb +++ b/app/components/rs_scope_list_component.html.erb @@ -1,25 +1,22 @@ -<% if @root %> -

    - +

    + <% if @root %> +

    <%= render partial: "icons/alert-triangle", - locals: { custom_class: "inline-block align-bottom mr-1.5" } %> - All categories and files - - <% if read_only? %> - (read only) - <% end %> -

    -<% else %> - <% @scopes.each do |scope| %> -

    - - <%= render partial: "icons/folder", - locals: { custom_class: "inline-block align-bottom mr-1.5" } %> - <%= scope_name(scope) %> - - <% if scope_permissions(scope) == "r" %> - (read only) + locals: { custom_class: "#{icon_classes} text-red-700" } %> + All categories and files + <% if read_only? %> + (read only) <% end %>

    + <% else %> + <% @scopes.each do |scope| %> +

    + <%= render partial: "icons/folder", locals: { custom_class: icon_classes } %> + <%= scope_name(scope) %> + <% if scope_permissions(scope) == "r" %> + (read only) + <% end %> +

    + <% end %> <% end %> -<% end %> +
    diff --git a/app/components/rs_scope_list_component.rb b/app/components/rs_scope_list_component.rb index 198ef08..43c431a 100644 --- a/app/components/rs_scope_list_component.rb +++ b/app/components/rs_scope_list_component.rb @@ -3,12 +3,17 @@ class RsScopeListComponent < ViewComponent::Base include OauthHelper - def initialize(scopes:, root: false) + def initialize(scopes:, root: false, icon_class: nil) @scopes = scopes @root = root + @icon_class = icon_class end def read_only? @scopes.include?(":r") end + + def icon_classes + ["flex-none", @icon_class].compact.join(" ") + end end diff --git a/app/views/services/rs_auths/index.html.erb b/app/views/services/rs_auths/index.html.erb index edd6ade..cbb6352 100644 --- a/app/views/services/rs_auths/index.html.erb +++ b/app/views/services/rs_auths/index.html.erb @@ -11,7 +11,7 @@
    <% if @rs_apps.any? %> -
    +
    <% @rs_apps.each do |authorizations| %> <%= render RsAuthComponent.new(authorizations: authorizations) %> <% end %> From 614ead90a956573d6fe356402881a09f6c25f8df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Tue, 3 Nov 2026 16:55:05 +0400 Subject: [PATCH 10/12] Distinguish tokens without a recorded device Tokens issued before user agents were captured cannot be attributed to a single device, and the per-app tokens issued earlier could even be reused from several devices at once. - Add RemoteStorageAuthorization#device_label, falling back to "Unknown device(s)" so the card and the details modal share one source - Show a lower-bound device count with a "+" whenever an app has such a token, so it reads as "could be more" instead of claiming a number we cannot support - Drop the now-redundant device-name wrapper from the details modal Part of #254 --- app/components/rs_auth_component.rb | 16 +++++++-- .../rs_auth_details_modal_component.html.erb | 2 +- .../rs_auth_details_modal_component.rb | 4 --- app/models/remote_storage_authorization.rb | 8 +++++ spec/features/services/rs_auths_spec.rb | 34 +++++++++++++++++++ .../remote_storage_authorization_spec.rb | 15 ++++++++ 6 files changed, 71 insertions(+), 8 deletions(-) diff --git a/app/components/rs_auth_component.rb b/app/components/rs_auth_component.rb index b80d225..5984941 100644 --- a/app/components/rs_auth_component.rb +++ b/app/components/rs_auth_component.rb @@ -24,11 +24,21 @@ class RsAuthComponent < ViewComponent::Base "#{count} #{'category'.pluralize(count)}" end - def device_count - @authorizations.map { |auth| auth.device_name.presence || "Unknown device" }.uniq.size + def known_device_count + @authorizations.map(&:device_name).compact.uniq.size + end + + def unrecorded_device? + @authorizations.any? { |auth| auth.device_name.blank? } end def device_summary - "#{device_count} #{'device'.pluralize(device_count)}" + count = known_device_count + return "#{count} #{'device'.pluralize(count)}" unless unrecorded_device? + + # An authorization without a recorded user agent may cover several devices, + # so only a lower bound can be given. + lower_bound = count + 1 + "#{lower_bound}+ #{'device'.pluralize(lower_bound)}" end end diff --git a/app/components/rs_auth_details_modal_component.html.erb b/app/components/rs_auth_details_modal_component.html.erb index 6fd0dfe..36da0f5 100644 --- a/app/components/rs_auth_details_modal_component.html.erb +++ b/app/components/rs_auth_details_modal_component.html.erb @@ -24,7 +24,7 @@ <% @authorizations.each do |auth| %>
  • -

    <%= device_name(auth) %>

    +

    <%= auth.device_label %>

    <%= tag.span "Connected #{auth.created_at.strftime('%-d %b %Y')}", title: connected_at_title(auth) %> diff --git a/app/components/rs_auth_details_modal_component.rb b/app/components/rs_auth_details_modal_component.rb index 20c55d5..0344e31 100644 --- a/app/components/rs_auth_details_modal_component.rb +++ b/app/components/rs_auth_details_modal_component.rb @@ -16,10 +16,6 @@ class RsAuthDetailsModalComponent < ViewComponent::Base (permissions & [":r", ":rw"]).any? end - def device_name(auth) - auth.device_name.presence || "Unknown device" - end - def status_label(auth) return "Never expires" if auth.expire_at.blank? return "Expired on #{auth.expire_at.strftime('%-d %b %Y')}" if auth.expired? diff --git a/app/models/remote_storage_authorization.rb b/app/models/remote_storage_authorization.rb index cf5167f..6c7ae6e 100644 --- a/app/models/remote_storage_authorization.rb +++ b/app/models/remote_storage_authorization.rb @@ -2,6 +2,10 @@ class RemoteStorageAuthorization < ApplicationRecord belongs_to :user belongs_to :web_app, class_name: "AppCatalog::WebApp", optional: true + # Tokens issued before user agents were recorded may have been used from any + # number of devices, so they cannot be attributed to a single one. + UNKNOWN_DEVICE = "Unknown device(s)" + serialize :permissions, coder: YAML unless Rails.env.production? validates_presence_of :permissions @@ -60,6 +64,10 @@ class RemoteStorageAuthorization < ApplicationRecord [name, os].compact.join(" on ").presence end + def device_label + device_name || UNKNOWN_DEVICE + end + def display_name web_app&.name.presence || app_name.presence || client_id end diff --git a/spec/features/services/rs_auths_spec.rb b/spec/features/services/rs_auths_spec.rb index 7496ba0..44435c5 100644 --- a/spec/features/services/rs_auths_spec.rb +++ b/spec/features/services/rs_auths_spec.rb @@ -140,6 +140,40 @@ RSpec.describe "Storage connected apps", type: :feature do end end + context "with an authorization from before user agents were recorded" do + let!(:legacy) { create_auth(client: "notes.example.com", device: nil) } + + after { legacy.destroy } + + it "only claims a lower-bound device count and labels the device as unknown" do + visit apps_services_storage_path + + within ".rs-auth-card" do + expect(page).to have_content("1+ device") + end + + expect(page).to have_selector("p", text: "Unknown device(s)", visible: false) + end + end + + context "with an unrecorded authorization alongside a recorded device" do + let!(:legacy) { create_auth(client: "notes.example.com", device: nil) } + let!(:laptop) { create_auth(client: "notes.example.com", device: firefox_ua) } + + after do + legacy.destroy + laptop.destroy + end + + it "counts the unrecorded authorization as a possible extra device" do + visit apps_services_storage_path + + within ".rs-auth-card" do + expect(page).to have_content("2+ devices") + end + end + end + context "without connected apps" do it "shows the empty state" do visit apps_services_storage_path diff --git a/spec/models/remote_storage_authorization_spec.rb b/spec/models/remote_storage_authorization_spec.rb index 51cd7a0..7616603 100644 --- a/spec/models/remote_storage_authorization_spec.rb +++ b/spec/models/remote_storage_authorization_spec.rb @@ -372,4 +372,19 @@ RSpec.describe RemoteStorageAuthorization, type: :model do expect(auth_with_user_agent("Firefox/120.0").device_name).to eq("Firefox") end end + + describe "#device_label" do + def auth_with_user_agent(user_agent) + RemoteStorageAuthorization.new(user: user, user_agent: user_agent) + end + + it "falls back to the unrecorded placeholder without a user agent" do + expect(auth_with_user_agent(nil).device_label).to eq("Unknown device(s)") + end + + it "returns the device name when one can be determined" do + ua = "Mozilla/5.0 (X11; Linux x86_64; rv:120.0) Gecko/20100101 Firefox/120.0" + expect(auth_with_user_agent(ua).device_label).to eq("Firefox on Linux") + end + end end From faa7bf61468178c6480184e084ec4ff45135b455 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Tue, 3 Nov 2026 17:45:37 +0400 Subject: [PATCH 11/12] Revoke device authorizations in place with Turbo Streams Revoking a device redirected away from the storage dashboard, which closed the details modal and reloaded the page. Refreshing the affected parts instead keeps the modal open and shows the change where it happened. - Extract the card and the modal's access list into their own components, so either can be re-rendered without touching the modal's open state, and share the group summaries between the components - Derive the app's DOM ids from its client_id (the grouping key) and the device row ids from the authorization, through one helper used by both the templates and the controller - Revoke one of several devices as a Turbo Stream: remove the row and refresh the card behind the modal and the access list - Revoke an app's last device with a redirect, so the dashboard re-renders itself and grouping, sorting and the empty state stay in one place - Fade the removed row out, deferring Turbo's own removal until the animation has ended Part of #254 --- .../stylesheets/components/animations.css | 24 ++++++++ .../rs_auth_access_component.html.erb | 7 +++ app/components/rs_auth_access_component.rb | 15 +++++ .../rs_auth_card_component.html.erb | 35 ++++++++++++ app/components/rs_auth_card_component.rb | 16 ++++++ app/components/rs_auth_component.html.erb | 38 +------------ app/components/rs_auth_component.rb | 42 ++------------ .../rs_auth_details_modal_component.html.erb | 10 +--- .../rs_auth_details_modal_component.rb | 13 +---- app/components/rs_auth_summary.rb | 40 +++++++++++++ .../services/rs_auths_controller.rb | 39 +++++++++++-- app/helpers/rs_auths_helper.rb | 14 +++++ app/javascript/application.js | 1 + app/javascript/turbo_stream_animations.js | 30 ++++++++++ config/importmap.rb | 1 + .../services/rs_auths_controller_spec.rb | 56 +++++++++++++++++++ spec/features/services/rs_auths_spec.rb | 10 ++++ 17 files changed, 295 insertions(+), 96 deletions(-) create mode 100644 app/components/rs_auth_access_component.html.erb create mode 100644 app/components/rs_auth_access_component.rb create mode 100644 app/components/rs_auth_card_component.html.erb create mode 100644 app/components/rs_auth_card_component.rb create mode 100644 app/components/rs_auth_summary.rb create mode 100644 app/helpers/rs_auths_helper.rb create mode 100644 app/javascript/turbo_stream_animations.js diff --git a/app/assets/stylesheets/components/animations.css b/app/assets/stylesheets/components/animations.css index 23b7664..315ab7f 100644 --- a/app/assets/stylesheets/components/animations.css +++ b/app/assets/stylesheets/components/animations.css @@ -14,3 +14,27 @@ animation-duration: 0.15s; animation-timing-function: cubic-bezier(0.2, 0, 0.13, 1); } + +@keyframes removeRow { + from { + opacity: 1; + transform: translateX(0); + } + to { + opacity: 0; + transform: translateX(0.75rem); + } +} + +.animate-remove { + animation: removeRow 0.2s ease-out forwards; + pointer-events: none; +} + +@media (prefers-reduced-motion: reduce) { + /* Keep the animation, so the deferred removal still completes, but make it + effectively instant. */ + .animate-remove { + animation-duration: 0.01s; + } +} diff --git a/app/components/rs_auth_access_component.html.erb b/app/components/rs_auth_access_component.html.erb new file mode 100644 index 0000000..cc50b66 --- /dev/null +++ b/app/components/rs_auth_access_component.html.erb @@ -0,0 +1,7 @@ +

    +
    + Access +
    + <%= render RsScopeListComponent.new(scopes: permissions, root: root_access?, + icon_class: "h-5 w-5") %> +
    \ No newline at end of file diff --git a/app/components/rs_auth_access_component.rb b/app/components/rs_auth_access_component.rb new file mode 100644 index 0000000..605d742 --- /dev/null +++ b/app/components/rs_auth_access_component.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class RsAuthAccessComponent < ViewComponent::Base + include RsAuthSummary + include RsAuthsHelper + + def initialize(authorizations:) + @authorizations = authorizations + @auth = authorizations.first + end + + def access_id + rs_app_dom_id(@auth.client_id, :access) + end +end \ No newline at end of file diff --git a/app/components/rs_auth_card_component.html.erb b/app/components/rs_auth_card_component.html.erb new file mode 100644 index 0000000..3ff311e --- /dev/null +++ b/app/components/rs_auth_card_component.html.erb @@ -0,0 +1,35 @@ + \ No newline at end of file diff --git a/app/components/rs_auth_card_component.rb b/app/components/rs_auth_card_component.rb new file mode 100644 index 0000000..ea8e7f1 --- /dev/null +++ b/app/components/rs_auth_card_component.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class RsAuthCardComponent < ViewComponent::Base + include RsAuthSummary + include RsAuthsHelper + + def initialize(authorizations:) + @authorizations = authorizations + @auth = authorizations.first + @web_app = @auth.web_app + end + + def card_id + rs_app_dom_id(@auth.client_id, :card) + end +end \ No newline at end of file diff --git a/app/components/rs_auth_component.html.erb b/app/components/rs_auth_component.html.erb index 3bc335e..2244194 100644 --- a/app/components/rs_auth_component.html.erb +++ b/app/components/rs_auth_component.html.erb @@ -1,38 +1,4 @@
    - - + <%= render RsAuthCardComponent.new(authorizations: @authorizations) %> <%= render RsAuthDetailsModalComponent.new(authorizations: @authorizations) %> -
    +
    \ No newline at end of file diff --git a/app/components/rs_auth_component.rb b/app/components/rs_auth_component.rb index 5984941..cb29f0c 100644 --- a/app/components/rs_auth_component.rb +++ b/app/components/rs_auth_component.rb @@ -1,44 +1,10 @@ # frozen_string_literal: true +# Wrapper around one app's card and its details modal. The modal's open state +# lives on this element (via the `modal` controller), so the card and the modal +# can be re-rendered independently without closing it. class RsAuthComponent < ViewComponent::Base def initialize(authorizations:) @authorizations = authorizations - @auth = authorizations.first - @web_app = @auth.web_app end - - # The card summarises the app, so reflect the union of its authorizations' - # scopes (an older token may have broader access than the newest one). - def permissions - @authorizations.flat_map(&:permissions).uniq - end - - def root_access? - (permissions & [":r", ":rw"]).any? - end - - def category_summary - return "All categories and files" if root_access? - - count = permissions.size - "#{count} #{'category'.pluralize(count)}" - end - - def known_device_count - @authorizations.map(&:device_name).compact.uniq.size - end - - def unrecorded_device? - @authorizations.any? { |auth| auth.device_name.blank? } - end - - def device_summary - count = known_device_count - return "#{count} #{'device'.pluralize(count)}" unless unrecorded_device? - - # An authorization without a recorded user agent may cover several devices, - # so only a lower bound can be given. - lower_bound = count + 1 - "#{lower_bound}+ #{'device'.pluralize(lower_bound)}" - end -end +end \ No newline at end of file diff --git a/app/components/rs_auth_details_modal_component.html.erb b/app/components/rs_auth_details_modal_component.html.erb index 36da0f5..2462dc6 100644 --- a/app/components/rs_auth_details_modal_component.html.erb +++ b/app/components/rs_auth_details_modal_component.html.erb @@ -11,18 +11,14 @@
  • -
    - Access -
    - <%= render RsScopeListComponent.new(scopes: permissions, root: root_access?, - icon_class: "h-5 w-5") %> + <%= render RsAuthAccessComponent.new(authorizations: @authorizations) %>
    Connected devices
      <% @authorizations.each do |auth| %> -
    • +
    • <%= auth.device_label %>

      @@ -50,4 +46,4 @@ Close

      -<% end %> +<% end %> \ No newline at end of file diff --git a/app/components/rs_auth_details_modal_component.rb b/app/components/rs_auth_details_modal_component.rb index 0344e31..be140d1 100644 --- a/app/components/rs_auth_details_modal_component.rb +++ b/app/components/rs_auth_details_modal_component.rb @@ -1,21 +1,14 @@ # frozen_string_literal: true class RsAuthDetailsModalComponent < ViewComponent::Base + include RsAuthsHelper + def initialize(authorizations:) @authorizations = authorizations @auth = authorizations.first @web_app = @auth.web_app end - # Reflect the union of the app's scopes, matching the card's summary. - def permissions - @authorizations.flat_map(&:permissions).uniq - end - - def root_access? - (permissions & [":r", ":rw"]).any? - end - def status_label(auth) return "Never expires" if auth.expire_at.blank? return "Expired on #{auth.expire_at.strftime('%-d %b %Y')}" if auth.expired? @@ -30,4 +23,4 @@ class RsAuthDetailsModalComponent < ViewComponent::Base def expires_at_title(auth) auth.expire_at.strftime("%-d %b %Y, %H:%M") end -end +end \ No newline at end of file diff --git a/app/components/rs_auth_summary.rb b/app/components/rs_auth_summary.rb new file mode 100644 index 0000000..550a779 --- /dev/null +++ b/app/components/rs_auth_summary.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +# Summaries of a group of authorizations for the same app, shared by the card, +# the details modal and the Turbo Stream responses that refresh them. +module RsAuthSummary + # The union of the group's scopes: an older token may grant more than the + # newest one. + def permissions + @authorizations.flat_map(&:permissions).uniq + end + + def root_access? + (permissions & [":r", ":rw"]).any? + end + + def category_summary + return "All categories and files" if root_access? + + count = permissions.size + "#{count} #{'category'.pluralize(count)}" + end + + def known_device_count + @authorizations.map(&:device_name).compact.uniq.size + end + + def unrecorded_device? + @authorizations.any? { |auth| auth.device_name.blank? } + end + + def device_summary + count = known_device_count + return "#{count} #{'device'.pluralize(count)}" unless unrecorded_device? + + # An authorization without a recorded user agent may cover several devices, + # so only a lower bound can be given. + lower_bound = count + 1 + "#{lower_bound}+ #{'device'.pluralize(lower_bound)}" + end +end \ No newline at end of file diff --git a/app/controllers/services/rs_auths_controller.rb b/app/controllers/services/rs_auths_controller.rb index f4df2c1..6245eab 100644 --- a/app/controllers/services/rs_auths_controller.rb +++ b/app/controllers/services/rs_auths_controller.rb @@ -14,14 +14,23 @@ class Services::RsAuthsController < Services::BaseController end def destroy + client_id = @auth.client_id @auth.destroy! + remaining = current_user.remote_storage_authorizations + .where(client_id: client_id) + .order(created_at: :desc) + .includes(:web_app) + .to_a + respond_to do |format| - format.html do redirect_to apps_services_storage_url, flash: { - success: 'App authorization revoked' - } - end format.json { head :no_content } + format.html do + redirect_to apps_services_storage_url, flash: { + success: 'App authorization revoked' + } + end + format.turbo_stream { revoke_stream(client_id, remaining) } end end @@ -48,6 +57,26 @@ class Services::RsAuthsController < Services::BaseController private + # Revoking one of several devices updates the open details modal in place. + # Revoking the app's last device leaves nothing to show, so the dashboard + # re-renders itself instead: that keeps grouping, sorting and the empty + # state in one place. + def revoke_stream(client_id, remaining) + if remaining.empty? + redirect_to apps_services_storage_url, flash: { + success: 'App authorization revoked' + } + else + render turbo_stream: [ + turbo_stream.remove(helpers.rs_device_dom_id(@auth)), + turbo_stream.replace(helpers.rs_app_dom_id(client_id, :card), + RsAuthCardComponent.new(authorizations: remaining)), + turbo_stream.replace(helpers.rs_app_dom_id(client_id, :access), + RsAuthAccessComponent.new(authorizations: remaining)) + ] + end + end + def app_name_for(auth) auth.display_name end @@ -66,4 +95,4 @@ class Services::RsAuthsController < Services::BaseController @auth = current_user.remote_storage_authorizations.find_by(id: params[:id]) http_status :not_found unless @auth end -end +end \ No newline at end of file diff --git a/app/helpers/rs_auths_helper.rb b/app/helpers/rs_auths_helper.rb new file mode 100644 index 0000000..125ae92 --- /dev/null +++ b/app/helpers/rs_auths_helper.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module RsAuthsHelper + # An app is summarised by the authorizations grouped under its client_id, so + # its DOM ids derive from the client_id: revoking any single authorization must + # not invalidate them. + def rs_app_dom_id(client_id, part) + "rs_app_#{part}_#{Digest::SHA256.hexdigest(client_id)[0, 12]}" + end + + def rs_device_dom_id(auth) + ActionView::RecordIdentifier.dom_id(auth, :device) + end +end \ No newline at end of file diff --git a/app/javascript/application.js b/app/javascript/application.js index 0d7b494..1e2d3dc 100644 --- a/app/javascript/application.js +++ b/app/javascript/application.js @@ -1,3 +1,4 @@ // Configure your import map in config/importmap.rb. Read more: https://github.com/rails/importmap-rails import "@hotwired/turbo-rails" import "controllers" +import "turbo_stream_animations" \ No newline at end of file diff --git a/app/javascript/turbo_stream_animations.js b/app/javascript/turbo_stream_animations.js new file mode 100644 index 0000000..1cf8356 --- /dev/null +++ b/app/javascript/turbo_stream_animations.js @@ -0,0 +1,30 @@ +// A Turbo Stream `remove` action deletes its target right away. Pause that until +// the element's CSS animation has finished, so a revoked device row can fade out +// instead of vanishing. + +const REMOVE_ANIMATION_CLASS = "animate-remove" +const REMOVE_ANIMATION_TIMEOUT = 1000 + +document.addEventListener("turbo:before-stream-render", (event) => { + const renderDefault = event.detail.render + + event.detail.render = (streamElement) => { + const target = + streamElement.action === "remove" ? document.getElementById(streamElement.target) : null + + if (!target) return renderDefault(streamElement) + + let rendered = false + const render = () => { + if (rendered) return + rendered = true + renderDefault(streamElement) + } + + target.addEventListener("animationend", render, { once: true }) + target.classList.add(REMOVE_ANIMATION_CLASS) + + // Don't leave the element behind if the animation cannot run at all. + setTimeout(render, REMOVE_ANIMATION_TIMEOUT) + } +}) \ No newline at end of file diff --git a/config/importmap.rb b/config/importmap.rb index cb396cc..692f89b 100644 --- a/config/importmap.rb +++ b/config/importmap.rb @@ -4,5 +4,6 @@ pin "application", preload: true pin "@hotwired/turbo-rails", to: "turbo.min.js", preload: true pin "@hotwired/stimulus", to: "stimulus.min.js", preload: true pin "@hotwired/stimulus-loading", to: "stimulus-loading.js", preload: true +pin "turbo_stream_animations" pin_all_from "app/javascript/controllers", under: "controllers" pin "tailwindcss-stimulus-components" # @4.0.3 diff --git a/spec/controllers/services/rs_auths_controller_spec.rb b/spec/controllers/services/rs_auths_controller_spec.rb index 0970c3c..cd4813a 100644 --- a/spec/controllers/services/rs_auths_controller_spec.rb +++ b/spec/controllers/services/rs_auths_controller_spec.rb @@ -103,6 +103,62 @@ RSpec.describe Services::RsAuthsController, type: :controller do expect(response).to have_http_status(:not_found) expect(RemoteStorageAuthorization.exists?(auth.id)).to be(true) end + + context "when another device remains connected to the app" do + let!(:revoked) do + user.remote_storage_authorizations.create!( + permissions: %w(documents), client_id: "app.example.com", + redirect_uri: "https://app.example.com", expire_at: 2.days.from_now + ) + end + + let!(:remaining) do + user.remote_storage_authorizations.create!( + permissions: %w(documents photos), client_id: "app.example.com", + redirect_uri: "https://app.example.com", expire_at: 2.days.from_now + ) + end + + after do + revoked.destroy + remaining.destroy + end + + it "streams the removed device row and the refreshed card and access list" do + delete :destroy, params: { id: revoked.id }, format: :turbo_stream + + expect(response.media_type).to eq("text/vnd.turbo-stream.html") + expect(response.body).to include( + %(action="remove" target="#{ActionView::RecordIdentifier.dom_id(revoked, :device)}") + ) + expect(response.body).to include( + %(action="replace" target="#{controller.helpers.rs_app_dom_id('app.example.com', :card)}") + ) + expect(response.body).to include( + %(action="replace" target="#{controller.helpers.rs_app_dom_id('app.example.com', :access)}") + ) + end + + it "leaves the other device connected" do + delete :destroy, params: { id: revoked.id }, format: :turbo_stream + + expect(RemoteStorageAuthorization.exists?(remaining.id)).to be(true) + end + end + + context "when the app's last device is revoked" do + it "redirects so that the dashboard re-renders itself" do + auth = user.remote_storage_authorizations.create!( + permissions: %w(documents), client_id: "app.example.com", + redirect_uri: "https://app.example.com", expire_at: 2.days.from_now + ) + + delete :destroy, params: { id: auth.id }, format: :turbo_stream + + expect(response).to redirect_to(apps_services_storage_url) + expect(flash[:success]).to eq("App authorization revoked") + end + end end end end diff --git a/spec/features/services/rs_auths_spec.rb b/spec/features/services/rs_auths_spec.rb index 44435c5..a8b8bc7 100644 --- a/spec/features/services/rs_auths_spec.rb +++ b/spec/features/services/rs_auths_spec.rb @@ -74,6 +74,16 @@ RSpec.describe "Storage connected apps", type: :feature do expect(page).to have_button("Revoke access", visible: false) expect(page).to have_link("Launch app", visible: false) end + + it "renders the ids that the revoke stream targets" do + visit apps_services_storage_path + + expect(page).to have_selector(".rs-auth-card[id^='rs_app_card_']") + expect(page).to have_selector("div[id^='rs_app_access_']", visible: false) + expect(page).to have_selector( + "li##{ActionView::RecordIdentifier.dom_id(auth, :device)}", visible: false + ) + end end context "with multiple apps" do From 17bad40bf4dc522f4ab03f60b82762dc044ce365 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Tue, 3 Nov 2026 18:28:14 +0400 Subject: [PATCH 12/12] Show device timestamps in the reader's local time The exact issuance and expiry times in the device rows' hover titles were rendered in UTC, which reads as wrong to anyone not living there. - Mark the dates with a
    <% @authorizations.each do |auth| %> -
  • +
  • <%= auth.device_label %>

    - <%= tag.span "Connected #{auth.created_at.strftime('%-d %b %Y')}", - title: connected_at_title(auth) %> + Connected <%= timestamp_tag(auth.created_at, title: connected_at_title(auth)) %> · - <%= tag.span status_label(auth), - title: (expires_at_title(auth) if auth.expire_at.present?), - class: ("text-red-700" if auth.expired?) %> + <%= tag.span class: ("text-red-700" if auth.expired?) do %> + <% if auth.expire_at.present? %> + <%= expiry_prefix(auth) %> <%= timestamp_tag(auth.expire_at, title: expires_at_title(auth)) %> + <% else %> + Never expires + <% end %> + <% end %>

    <%= button_to "Revoke access", services_storage_rs_auth_path(auth), method: :delete, diff --git a/app/components/rs_auth_details_modal_component.rb b/app/components/rs_auth_details_modal_component.rb index be140d1..90aef7f 100644 --- a/app/components/rs_auth_details_modal_component.rb +++ b/app/components/rs_auth_details_modal_component.rb @@ -9,13 +9,15 @@ class RsAuthDetailsModalComponent < ViewComponent::Base @web_app = @auth.web_app end - def status_label(auth) - return "Never expires" if auth.expire_at.blank? - return "Expired on #{auth.expire_at.strftime('%-d %b %Y')}" if auth.expired? + # "Expires" / "Expired on", or nothing when there is no expiry to describe. + def expiry_prefix(auth) + return nil if auth.expire_at.blank? - "Expires #{auth.expire_at.strftime('%-d %b %Y')}" + auth.expired? ? "Expired on" : "Expires" end + # Fallbacks for the hover titles, used when JavaScript is unavailable. + # app/javascript/local_time.js replaces them with the reader's own zone. def connected_at_title(auth) auth.created_at.strftime("%-d %b %Y, %H:%M") end @@ -23,4 +25,10 @@ class RsAuthDetailsModalComponent < ViewComponent::Base def expires_at_title(auth) auth.expire_at.strftime("%-d %b %Y, %H:%M") end + + # A