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
This commit is contained in:
raucao committed 2026-11-03 12:00:39 +04:00
1 parent c4df1772e0
commit f2248ea744
14 files changed
+213 -156

No files matched your search

+20 -33
View File
@@ -1,41 +1,28 @@
<div class="rs-auth-card flex flex-col h-full rounded border border-gray-200 p-4"
data-controller="modal" data-action="keydown.esc->modal#close">
<div class="flex items-center gap-4">
<div class="h-16 w-16 flex-none">
<div class="h-full" data-controller="modal" data-action="keydown.esc->modal#close">
<button type="button"
class="rs-auth-card group flex h-full w-full items-center gap-4 rounded-lg border
border-gray-200 bg-white p-5 text-left transition
hover:border-gray-400 hover:bg-gray-50
focus:outline-none focus-visible:ring-2 focus-visible:ring-blue-500"
data-action="click->modal#open">
<div class="h-14 w-14 flex-none">
<%= render AppCatalog::WebAppIconComponent.new(web_app: @web_app) %>
</div>
<div class="flex-grow min-w-0">
<h4 class="mb-1 text-lg font-bold truncate"><%= name %></h4>
<div class="min-w-0 flex-grow">
<h4 class="text-lg font-bold truncate"><%= name %></h4>
<p class="text-sm text-gray-500 truncate"><%= @auth.client_id %></p>
<p class="mt-2 text-sm text-gray-600">
<%= device_summary %>
<span class="mx-1 text-gray-300">·</span>
<%= category_summary %>
</p>
</div>
</div>
<dl class="mt-4 flex-grow space-y-1 text-sm text-gray-600">
<div class="flex justify-between gap-2">
<dt>Access</dt>
<dd class="text-right"><%= scope_summary %></dd>
</div>
<div class="flex justify-between gap-2">
<dt>Device</dt>
<dd class="text-right"><%= device_name %></dd>
</div>
<div class="flex justify-between gap-2">
<dt>Status</dt>
<dd class="text-right <%= "text-red-700" if @auth.expired? %>"><%= expiry_label %></dd>
</div>
<% if multiple_authorizations? %>
<div class="flex justify-between gap-2">
<dt>Tokens</dt>
<dd class="text-right text-amber-700"><%= @authorizations.size %></dd>
</div>
<% end %>
</dl>
<div class="mt-4">
<button class="btn-md btn-outline w-full" data-action="click->modal#open">
Details
</button>
</div>
<span class="flex-none text-gray-300 transition group-hover:text-gray-500" aria-hidden="true">
<%= render partial: "icons/chevron-right", locals: { custom_class: "h-5 w-5" } %>
</span>
</button>
<%= render RsAuthDetailsModalComponent.new(authorizations: @authorizations) %>
</div>
+7 -14
View File
@@ -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
@@ -1,47 +1,42 @@
<%= render ModalComponent.new do %>
<div class="flex items-center gap-4">
<div class="h-16 w-16 flex-none">
<div class="h-14 w-14 flex-none">
<%= render AppCatalog::WebAppIconComponent.new(web_app: @web_app) %>
</div>
<div class="min-w-0">
<h4 class="mb-1 text-lg font-bold truncate"><%= name %></h4>
<h4 class="text-lg font-bold truncate"><%= name %></h4>
<p class="text-sm text-gray-500 truncate"><%= @auth.client_id %></p>
</div>
</div>
<h5 class="mt-6 mb-1 font-bold">Access</h5>
<% if root_access? %>
<p class="text-sm text-gray-600">All folders and files</p>
<% else %>
<ul class="list-disc list-inside text-sm text-gray-600">
<% @auth.permissions.each do |scope| %>
<li><%= scope_label(scope) %></li>
<% end %>
</ul>
<% end %>
<h5 class="mt-4 mb-1 font-bold">Device</h5>
<p class="text-sm text-gray-600"><%= device_name %></p>
<h5 class="mt-4 mb-2 font-bold">
<%= @authorizations.size > 1 ? "Connected tokens" : "Connected" %>
<h5 class="mt-6 mb-2 text-xs font-semibold uppercase tracking-wide text-gray-500">
Access
</h5>
<ul class="space-y-2">
<%= render RsScopeListComponent.new(scopes: @auth.permissions, root: root_access?) %>
<h5 class="mt-6 mb-2 text-xs font-semibold uppercase tracking-wide text-gray-500">
Connected devices
</h5>
<ul class="divide-y divide-gray-100 rounded border border-gray-200" data-turbo-prefetch="false">
<% @authorizations.each do |auth| %>
<li class="flex items-center justify-between gap-3 text-sm text-gray-600">
<span>
<%= auth.created_at.strftime('%-d %b %Y') %>
&middot;
<%= status_label(auth) %>
</span>
<%= link_to "Revoke access", revoke_services_storage_rs_auth_path(auth),
class: "text-red-700 whitespace-nowrap" %>
<li class="flex items-center justify-between gap-4 px-4 py-3">
<div class="min-w-0">
<p class="truncate text-sm font-medium text-gray-900"><%= device_name(auth) %></p>
<p class="truncate text-xs text-gray-500">
Connected <%= auth.created_at.strftime('%-d %b %Y') %>
<span class="mx-1">&middot;</span>
<span class="<%= "text-red-700" if auth.expired? %>"><%= status_label(auth) %></span>
</p>
</div>
<%= 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" %>
</li>
<% end %>
</ul>
<div class="mt-6 flex flex-col sm:flex-row gap-2 sm:justify-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 } %>
</div>
<% end %>
@@ -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)
@@ -0,0 +1,25 @@
<% if @root %>
<p class="scope text-lg">
<span class="text-red-700">
<%= render partial: "icons/alert-triangle",
locals: { custom_class: "inline-block align-bottom mr-1.5" } %>
All categories and files
</span>
<% if read_only? %>
<span class="text-sm text-gray-500">(read only)</span>
<% end %>
</p>
<% else %>
<% @scopes.each do |scope| %>
<p class="scope text-gray-600">
<span class="text-lg">
<%= render partial: "icons/folder",
locals: { custom_class: "inline-block align-bottom mr-1.5" } %>
<%= scope_name(scope) %>
</span>
<% if scope_permissions(scope) == "r" %>
<span>(read only)</span>
<% end %>
</p>
<% end %>
<% end %>
+14
View File
@@ -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
+16 -10
View File
@@ -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
+1 -1
View File
@@ -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"
+2 -26
View File
@@ -5,34 +5,10 @@
<p class="mb-8">
The app on
<%= link_to @client_id, "https://#{@client_id}" %>
is asking for access to these folders:
is asking for access to these categories:
</p>
<% if @root_access_requested %>
<p class="scope text-lg">
<span class="text-red-700">
<%= render partial: "icons/alert-triangle",
locals: { custom_class: "inline-block align-bottom mr-1.5" } %>
All files and directories
</span>
<% if (@scopes & [":r"]).any? %>
<span class="text-sm text-gray-500">(read only)</span>
<% end %>
</p>
<% else %>
<% @scopes.each do |scope| %>
<p class="scope text-gray-600">
<span class="text-lg">
<%= render partial: "icons/folder",
locals: { custom_class: "inline-block align-bottom mr-1.5" } %>
<%= scope_name(scope) %>
</span>
<% if scope_permissions(scope) == "r" %>
<span>(read only)</span>
<% end %>
</p>
<% 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 %>
+2 -2
View File
@@ -10,9 +10,9 @@
<%= render partial: "shared/tabnav_remotestorage" %>
<section>
<% if @rs_auth_groups.any? %>
<% if @rs_apps.any? %>
<div class="w-full grid grid-cols-1 sm:grid-cols-2 lg:grid-cols-3 gap-6 mt-4">
<% @rs_auth_groups.each do |authorizations| %>
<% @rs_apps.each do |authorizations| %>
<%= render RsAuthComponent.new(authorizations: authorizations) %>
<% end %>
</div>
+3 -6
View File
@@ -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
@@ -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
+1 -1
View File
@@ -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
+44 -23
View File
@@ -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