From 20c014607c940bc5f87b53ff110398399e09d876 Mon Sep 17 00:00:00 2001 From: Garret Alfert Date: Wed, 22 Mar 2023 00:51:33 +0100 Subject: [PATCH 01/20] Basic RemoteStorage settings --- .env.example | 1 + app/models/setting.rb | 10 ++++++++++ .../settings/services/_remotestorage.html.erb | 17 +++++++++++++++++ .../_admin_sidenav_settings_services.html.erb | 7 +++++++ 4 files changed, 35 insertions(+) create mode 100644 app/views/admin/settings/services/_remotestorage.html.erb diff --git a/.env.example b/.env.example index b667033..e8dc0f3 100644 --- a/.env.example +++ b/.env.example @@ -18,6 +18,7 @@ DISCOURSE_PUBLIC_URL='https://community.kosmos.org' GITEA_PUBLIC_URL='https://gitea.kosmos.org' MASTODON_PUBLIC_URL='https://kosmos.social' MEDIAWIKI_PUBLIC_URL='https://wiki.kosmos.org' +RS_STORAGE_URL='https://storage.kosmos.org' EJABBERD_ADMIN_URL='https://xmpp.kosmos.org/admin' EJABBERD_API_URL='https://xmpp.kosmos.org/api' diff --git a/app/models/setting.rb b/app/models/setting.rb index 56aa785..3a7e8c5 100644 --- a/app/models/setting.rb +++ b/app/models/setting.rb @@ -90,4 +90,14 @@ class Setting < RailsSettings::Base # field :nostr_enabled, type: :boolean, default: true + + # + # RemoteStorage + # + + field :remotestorage_enabled, type: :boolean, + default: (ENV["RS_STORAGE_URL"].present?.to_s || false) + + field :rs_storage_url, type: :string, + default: ENV["RS_STORAGE_URL"].presence end diff --git a/app/views/admin/settings/services/_remotestorage.html.erb b/app/views/admin/settings/services/_remotestorage.html.erb new file mode 100644 index 0000000..fdfa3f4 --- /dev/null +++ b/app/views/admin/settings/services/_remotestorage.html.erb @@ -0,0 +1,17 @@ +

RemoteStorage

+ diff --git a/app/views/shared/_admin_sidenav_settings_services.html.erb b/app/views/shared/_admin_sidenav_settings_services.html.erb index c897b6d..142f6fc 100644 --- a/app/views/shared/_admin_sidenav_settings_services.html.erb +++ b/app/views/shared/_admin_sidenav_settings_services.html.erb @@ -47,3 +47,10 @@ icon: Setting.nostr_enabled? ? "check" : "x", active: current_page?(admin_settings_services_path(params: { s: "nostr" })), ) %> +<%= render SidenavLinkComponent.new( + level: 2, + name: "RemoteStorage", + path: admin_settings_services_path(params: { s: "remotestorage" }), + icon: Setting.remotestorage_enabled? ? "check" : "x", + active: current_page?(admin_settings_services_path(params: { s: "remotestorage" })), +) %> From 7acc3b21060dd97be743dc9348fc61f056c6dc1c Mon Sep 17 00:00:00 2001 From: Garret Alfert Date: Sun, 12 Mar 2023 21:46:03 +0100 Subject: [PATCH 02/20] RemoteStorage OAuth dialog --- app/controllers/rs/oauth_controller.rb | 131 +++++++++++++++++++++++++ app/helpers/oauth_helper.rb | 11 +++ app/services/remote_storage.rb | 18 ++++ app/views/icons/_asterisk.html.erb | 1 + app/views/icons/_folder.html.erb | 2 +- app/views/rs/oauth/new.html.erb | 60 +++++++++++ config/routes.rb | 6 ++ 7 files changed, 228 insertions(+), 1 deletion(-) create mode 100644 app/controllers/rs/oauth_controller.rb create mode 100644 app/helpers/oauth_helper.rb create mode 100644 app/services/remote_storage.rb create mode 100644 app/views/icons/_asterisk.html.erb create mode 100644 app/views/rs/oauth/new.html.erb diff --git a/app/controllers/rs/oauth_controller.rb b/app/controllers/rs/oauth_controller.rb new file mode 100644 index 0000000..c403bcf --- /dev/null +++ b/app/controllers/rs/oauth_controller.rb @@ -0,0 +1,131 @@ +class Rs::OauthController < ApplicationController + before_action :require_user_signed_in + + def new + username, org = params[:useraddress].split("@") + @user = User.where(cn: username.downcase, ou: org).first + @scopes = parse_scopes params[:scope] + @redirect_uri = params[:redirect_uri] + @client_id = params[:client_id] + @state = params[:state] + @root_access_requested = (@scopes & [":r",":rw"]).any? + + @denial_url = url_with_state("#{@redirect_uri}#error=access_denied", @state) + + @expire_at_dates = [["Never", nil], + ["In 1 month", 1.month.from_now], + ["In 1 day", 1.day.from_now]] + + http_status :bad_request and return unless @redirect_uri.present? + + unless current_user == @user + sign_out :user + + redirect_to new_rs_oauth_url(@user.address, + scope: params[:scope], + redirect_uri: params[:redirect_uri], + client_id: params[:client_id], + state: params[:state]) + return + end + + unless @client_id.present? + redirect_to url_with_state("#{@redirect_uri}#error=invalid_request", @state) and return + end + + if @scopes.empty? + redirect_to url_with_state("#{@redirect_uri}#error=invalid_scope", @state) and return + end + + unless hostname_of(@client_id) == hostname_of(@redirect_uri) + redirect_to url_with_state("#{@redirect_uri}#error=invalid_client", @state) and return + end + + @client_id.gsub!(/http(s)?:\/\//, "") + + # TODO + # if auth = current_user.remote_storage_authorizations.valid.where(permissions: @scopes, client_id: @client_id).first + # redirect_to url_with_state("#{@redirect_uri}#access_token=#{auth.token}", @state), allow_other_host: true + # end + end + + def create + unless current_user.id.to_s == params[:user_id] + Rails.logger.info("NO MATCH: #{params[:user_id]}, #{current_user.id}") + http_status :forbidden and return + end + + permissions = parse_scopes params[:scope] + redirect_uri = params[:redirect_uri].presence + client_id = params[:client_id].presence + state = params[:state].presence + expire_at = params[:expire_at].presence + + http_status :bad_request and return unless redirect_uri.present? + + if permissions.empty? + redirect_to url_with_state("#{redirect_uri}#error=invalid_scope", state), allow_other_host: true and return + end + + unless client_id.present? + redirect_to url_with_state("#{redirect_uri}#error=invalid_request", state), allow_other_host: true and return + end + + unless hostname_of(client_id) == hostname_of(redirect_uri) + redirect_to url_with_state("#{redirect_uri}#error=invalid_client", state), allow_other_host: true and return + end + + client_id.gsub!(/http(s)?:\/\//, "") + + rs = RemoteStorage.new + auth = rs.create_authorization(current_user, { + permissions: permissions, + client_id: client_id, + redirect_uri: redirect_uri, + app_name: client_id, #TODO use user-defined name + expire_at: expire_at + }) + + redirect_to url_with_state("#{redirect_uri}#access_token=#{auth.token}", state), allow_other_host: true + end + + # GET /rs/oauth/token/:id/launch_app + def launch_app + auth = current_user.remote_storage_authorizations.find(params[:id]) + + redirect_to app_auth_url(auth) + end + + private + + def app_auth_url(auth) + url = "#{auth.url}#remotestorage=#{current_user.address}" + url += "&access_token=#{auth.token}" + url + end + + def hostname_of(uri) + uri.gsub(/http(s)?:\/\//, "").split(":")[0].split("/")[0] + end + + def parse_scopes(scope_string) + return [] if scope_string.blank? + + scopes = scope_string. + gsub(/\[|\]/, ""). + gsub(/\,/, " "). + gsub(/\/:/, ":"). + split(/\s/).map(&:strip). + reject(&:empty?) + + scopes = [":r"] if scopes.include?("*:r") + scopes = [":rw"] if scopes.include?("*:rw") + + scopes + end + + def url_with_state(url, state) + state ? "#{url}&state=#{CGI.escape(state)}" : url + end + +end diff --git a/app/helpers/oauth_helper.rb b/app/helpers/oauth_helper.rb new file mode 100644 index 0000000..12f76a5 --- /dev/null +++ b/app/helpers/oauth_helper.rb @@ -0,0 +1,11 @@ +module OauthHelper + + def scope_name(scope) + scope.gsub(/(\:.+)/, '') + end + + def scope_permissions(scope) + scope.match(/\:r$/) ? "r" : "rw" + end + +end diff --git a/app/services/remote_storage.rb b/app/services/remote_storage.rb new file mode 100644 index 0000000..a6a80af --- /dev/null +++ b/app/services/remote_storage.rb @@ -0,0 +1,18 @@ +require 'ostruct' + +class RemoteStorage + + def initialize + end + + def create_authorization(user, auth_data) + + return OpenStruct.new(token: "SOME-FANCY-TOKEN") + # permissions: permissions, + # client_id: client_id, + # redirect_uri: redirect_uri, + # app_name: client_id, #TODO use user-defined name + # expire_at: expire_at + end + +end diff --git a/app/views/icons/_asterisk.html.erb b/app/views/icons/_asterisk.html.erb new file mode 100644 index 0000000..a97802b --- /dev/null +++ b/app/views/icons/_asterisk.html.erb @@ -0,0 +1 @@ + diff --git a/app/views/icons/_folder.html.erb b/app/views/icons/_folder.html.erb index 134458b..69e55bb 100644 --- a/app/views/icons/_folder.html.erb +++ b/app/views/icons/_folder.html.erb @@ -1 +1 @@ - \ No newline at end of file + \ No newline at end of file diff --git a/app/views/rs/oauth/new.html.erb b/app/views/rs/oauth/new.html.erb new file mode 100644 index 0000000..5cee5b4 --- /dev/null +++ b/app/views/rs/oauth/new.html.erb @@ -0,0 +1,60 @@ +<%= render HeaderComponent.new(title: "App Authorization") %> + +<%= render MainSimpleComponent.new do %> +
+

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

+ +

+ <% if @root_access_requested %> + + + <%= render partial: "icons/asterisk", locals: { custom_class: "inline-block align-middle mb-1" } %> + 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-middle mb-2" } %> + <%= scope_name(scope) %> + + <% if scope_permissions(scope) == "r" %> + (read only) + <% end %> + + <% end %> + <% end %> +

+ + <%= form_with(url: rs_oauth_path, method: :post, data: { turbo: false }) do |f| %> + <%= f.hidden_field :redirect_uri, value: @redirect_uri %> + <%= f.hidden_field :scope, value: @scopes.join(" ") %> + <%= f.hidden_field :user_id, value: @user.id %> + <%= f.hidden_field :client_id, value: @client_id %> + <%= f.hidden_field :state, value: @state %> +

+ <%= f.label :expire_at, "Expire:" %> + <%= f.select :expire_at, options_for_select(@expire_at_dates) %> +

+

+ You can revoke access for this app at any time on your storage dashboard. +

+
+ <%= f.submit class: "btn-md btn-blue w-full sm:w-auto", data: { disable_with: "Saving..." } do %> + Allow + <% end %> + <%= link_to @denial_url, class: "btn-md btn-red w-full sm:w-auto" do %> + Deny + <% end %> +
+ <% end %> +
+<% end %> diff --git a/config/routes.rb b/config/routes.rb index 8b4866b..b83d631 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -54,6 +54,12 @@ Rails.application.routes.draw do end end + namespace :rs do + resource :oauth, only: [:new, :create], path_names: { new: ':useraddress' }, + controller: 'oauth', constraints: { useraddress: /[^\/]+/} + get 'oauth/token/:id/launch_app' => 'oauth#launch_app', as: :launch_app + end + authenticate :user, ->(user) { user.is_admin? } do mount Sidekiq::Web => '/sidekiq' end From ee42d6847152aebc92fc2db0041dfd0da3081d2d Mon Sep 17 00:00:00 2001 From: Garret Alfert Date: Tue, 28 Mar 2023 00:02:07 +0200 Subject: [PATCH 03/20] Add RemoteStorageAuthorization model --- app/controllers/rs/oauth_controller.rb | 5 ++- app/models/remote_storage_authorization.rb | 32 +++++++++++++++++++ app/models/user.rb | 2 ++ app/services/remote_storage.rb | 18 ----------- ...30_create_remote_storage_authorizations.rb | 17 ++++++++++ db/schema.rb | 17 +++++++++- 6 files changed, 69 insertions(+), 22 deletions(-) create mode 100644 app/models/remote_storage_authorization.rb delete mode 100644 app/services/remote_storage.rb create mode 100644 db/migrate/20230312212030_create_remote_storage_authorizations.rb diff --git a/app/controllers/rs/oauth_controller.rb b/app/controllers/rs/oauth_controller.rb index c403bcf..2aa25e0 100644 --- a/app/controllers/rs/oauth_controller.rb +++ b/app/controllers/rs/oauth_controller.rb @@ -77,14 +77,13 @@ class Rs::OauthController < ApplicationController client_id.gsub!(/http(s)?:\/\//, "") - rs = RemoteStorage.new - auth = rs.create_authorization(current_user, { + auth = current_user.remote_storage_authorizations.create!( permissions: permissions, client_id: client_id, redirect_uri: redirect_uri, app_name: client_id, #TODO use user-defined name expire_at: expire_at - }) + ) redirect_to url_with_state("#{redirect_uri}#access_token=#{auth.token}", state), allow_other_host: true end diff --git a/app/models/remote_storage_authorization.rb b/app/models/remote_storage_authorization.rb new file mode 100644 index 0000000..e42deca --- /dev/null +++ b/app/models/remote_storage_authorization.rb @@ -0,0 +1,32 @@ +class RemoteStorageAuthorization < ApplicationRecord + belongs_to :user + + serialize :permissions + + validates_presence_of :permissions + validates_presence_of :client_id + + scope :valid, -> { where(expire_at: nil).or(where(expire_at: (DateTime.now)..)) } + scope :expired, -> { where(expire_at: ..(DateTime.now)) } + + after_initialize do |a| + a.permisisons = [] if a.permissions == nil + end + + before_create :generate_token + + def url + if self.redirect_uri + uri = URI.parse self.redirect_uri + "#{uri.scheme}://#{client_id}" + else + "http://#{client_id}" + end + end + + private + + def generate_token(length=16) + self.token = SecureRandom.hex(length) if self.token.blank? + end +end diff --git a/app/models/user.rb b/app/models/user.rb index ebe76cb..ecc50f5 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -14,6 +14,8 @@ class User < ApplicationRecord has_many :accounts, through: :lndhub_user + has_many :remote_storage_authorizations + validates_uniqueness_of :cn validates_length_of :cn, :minimum => 3 validates_format_of :cn, with: /\A([a-z0-9\-])*\z/, diff --git a/app/services/remote_storage.rb b/app/services/remote_storage.rb deleted file mode 100644 index a6a80af..0000000 --- a/app/services/remote_storage.rb +++ /dev/null @@ -1,18 +0,0 @@ -require 'ostruct' - -class RemoteStorage - - def initialize - end - - def create_authorization(user, auth_data) - - return OpenStruct.new(token: "SOME-FANCY-TOKEN") - # permissions: permissions, - # client_id: client_id, - # redirect_uri: redirect_uri, - # app_name: client_id, #TODO use user-defined name - # expire_at: expire_at - end - -end diff --git a/db/migrate/20230312212030_create_remote_storage_authorizations.rb b/db/migrate/20230312212030_create_remote_storage_authorizations.rb new file mode 100644 index 0000000..ebb8733 --- /dev/null +++ b/db/migrate/20230312212030_create_remote_storage_authorizations.rb @@ -0,0 +1,17 @@ +class CreateRemoteStorageAuthorizations < ActiveRecord::Migration[7.0] + def change + create_table :remote_storage_authorizations do |t| + t.references :user, null: false, foreign_key: true + t.string :token + t.text :permissions, array: true, default: [].to_yaml + t.string :client_id + t.string :redirect_uri + t.string :app_name + t.datetime :expire_at + + t.timestamps + end + + add_index :remote_storage_authorizations, :permissions, using: 'gin' + end +end diff --git a/db/schema.rb b/db/schema.rb index f87e1b0..18a0b86 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[7.0].define(version: 2023_02_23_115536) do +ActiveRecord::Schema[7.0].define(version: 2023_03_12_212030) do create_table "donations", force: :cascade do |t| t.integer "user_id" t.integer "amount_sats" @@ -34,6 +34,20 @@ ActiveRecord::Schema[7.0].define(version: 2023_02_23_115536) do t.index ["user_id"], name: "index_invitations_on_user_id" end + create_table "remote_storage_authorizations", force: :cascade do |t| + t.integer "user_id", null: false + t.string "token" + t.text "permissions", default: "--- []\n" + t.string "client_id" + t.string "redirect_uri" + t.string "app_name" + t.datetime "expire_at" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["permissions"], name: "index_remote_storage_authorizations_on_permissions" + t.index ["user_id"], name: "index_remote_storage_authorizations_on_user_id" + end + create_table "settings", force: :cascade do |t| t.string "var", null: false t.text "value" @@ -61,4 +75,5 @@ ActiveRecord::Schema[7.0].define(version: 2023_02_23_115536) do t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true end + add_foreign_key "remote_storage_authorizations", "users" end From dabd892a25b26268f39e0efb33c60d1344fa12d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Thu, 13 Apr 2023 16:21:48 +0200 Subject: [PATCH 04/20] Improve RS OAuth UI --- app/views/icons/_alert-triangle.html.erb | 2 +- app/views/rs/oauth/new.html.erb | 74 ++++++++++++------------ 2 files changed, 37 insertions(+), 39 deletions(-) diff --git a/app/views/icons/_alert-triangle.html.erb b/app/views/icons/_alert-triangle.html.erb index 6dcb096..36b0133 100644 --- a/app/views/icons/_alert-triangle.html.erb +++ b/app/views/icons/_alert-triangle.html.erb @@ -1 +1 @@ - \ No newline at end of file + diff --git a/app/views/rs/oauth/new.html.erb b/app/views/rs/oauth/new.html.erb index 5cee5b4..d34001b 100644 --- a/app/views/rs/oauth/new.html.erb +++ b/app/views/rs/oauth/new.html.erb @@ -1,38 +1,38 @@ -<%= render HeaderComponent.new(title: "App Authorization") %> +<%= render HeaderCompactComponent.new(title: "Storage") %> -<%= render MainSimpleComponent.new do %> -
-

- The app +<%= render MainCompactComponent.new do %> +

+

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

-

- <% if @root_access_requested %> - - - <%= render partial: "icons/asterisk", locals: { custom_class: "inline-block align-middle mb-1" } %> - 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-middle mb-2" } %> - <%= scope_name(scope) %> - - <% if scope_permissions(scope) == "r" %> - (read only) - <% end %> - - <% end %> + <% 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 %> <%= form_with(url: rs_oauth_path, method: :post, data: { turbo: false }) do |f| %> <%= f.hidden_field :redirect_uri, value: @redirect_uri %> @@ -40,20 +40,18 @@ <%= f.hidden_field :user_id, value: @user.id %> <%= f.hidden_field :client_id, value: @client_id %> <%= f.hidden_field :state, value: @state %> -

- <%= f.label :expire_at, "Expire:" %> +

+ <%= f.label :expire_at, "Permission expires:", class: "mr-1.5" %> <%= f.select :expire_at, options_for_select(@expire_at_dates) %>

-

+

You can revoke access for this app at any time on your storage dashboard.

-
- <%= f.submit class: "btn-md btn-blue w-full sm:w-auto", data: { disable_with: "Saving..." } do %> - Allow - <% end %> - <%= link_to @denial_url, class: "btn-md btn-red w-full sm:w-auto" do %> - Deny - <% end %> +

+ <%= f.submit "Allow", + class: "btn-md btn-blue w-full sm:order-last sm:grow", + data: { disable_with: "Saving..." } %> + <%= link_to "Deny", @denial_url, class: "btn-md btn-gray text-red-700 w-full sm:grow" %>

<% end %>
From 42af1481684f5c451eb47084f8ff1cf6c431c6f1 Mon Sep 17 00:00:00 2001 From: Garret Alfert Date: Tue, 20 Jun 2023 14:02:48 +0200 Subject: [PATCH 05/20] Persist RS auth tokens in Redis --- ...expire_remote_storage_authorization_job.rb | 10 ++++++ app/models/remote_storage_authorization.rb | 31 ++++++++++++++++ .../remote_storage_authorizations.rb | 9 +++++ ...e_remote_storage_authorization_job_spec.rb | 36 +++++++++++++++++++ 4 files changed, 86 insertions(+) create mode 100644 app/jobs/expire_remote_storage_authorization_job.rb create mode 100644 spec/factories/remote_storage_authorizations.rb create mode 100644 spec/jobs/expire_remote_storage_authorization_job_spec.rb diff --git a/app/jobs/expire_remote_storage_authorization_job.rb b/app/jobs/expire_remote_storage_authorization_job.rb new file mode 100644 index 0000000..8007e2c --- /dev/null +++ b/app/jobs/expire_remote_storage_authorization_job.rb @@ -0,0 +1,10 @@ +class ExpireRemoteStorageAuthorizationJob < ApplicationJob + queue_as :remote_storage + + def perform(rs_auth_id) + rs_auth = RemoteStorageAuthorization.find rs_auth_id + return unless rs_auth.expire_at.nil? || rs_auth.expire_at <= DateTime.now + + rs_auth.destroy! + end +end diff --git a/app/models/remote_storage_authorization.rb b/app/models/remote_storage_authorization.rb index e42deca..13dfd9c 100644 --- a/app/models/remote_storage_authorization.rb +++ b/app/models/remote_storage_authorization.rb @@ -14,6 +14,10 @@ class RemoteStorageAuthorization < ApplicationRecord end before_create :generate_token + before_create :store_token_in_redis + after_create :schedule_token_expiry + before_destroy :delete_token_from_redis + after_destroy :remove_token_expiry_job def url if self.redirect_uri @@ -24,9 +28,36 @@ class RemoteStorageAuthorization < ApplicationRecord end end + def delete_token_from_redis + key = "rs:authorizations:#{user.address}:#{token}" + # You can't delete multiple members of a set with Redis 2 + redis.smembers(key).each { |auth| redis.srem(key, auth) } + end + private + def redis + @redis ||= Redis.new(url: Setting.redis_url) + end + def generate_token(length=16) self.token = SecureRandom.hex(length) if self.token.blank? end + + def store_token_in_redis + redis.sadd "rs:authorizations:#{user.address}:#{token}", permissions + end + + def schedule_token_expiry + return unless expire_at.present? + ExpireRemoteStorageAuthorizationJob.set(wait_unil: expire_at).perform_later(id) + end + + def remove_token_expiry_job + queue = Sidekiq::Queue.new(ExpireRemoteStorageAuthorizationJob.queue_name) + queue.each do |job| + next unless job.display_class == "ExpireRemoteStorageAuthorizationJob" + job.delete if job.display_args == [id] + end + end end diff --git a/spec/factories/remote_storage_authorizations.rb b/spec/factories/remote_storage_authorizations.rb new file mode 100644 index 0000000..be7c810 --- /dev/null +++ b/spec/factories/remote_storage_authorizations.rb @@ -0,0 +1,9 @@ +FactoryBot.define do + factory :remote_storage_authorization do + permissions { ["documents:rw"] } + client_id { "some-fancy-app" } + redirect_uri { "https://example.com/some-fancy-app" } + app_name { "Fancy App" } + expire_at { nil } + end +end diff --git a/spec/jobs/expire_remote_storage_authorization_job_spec.rb b/spec/jobs/expire_remote_storage_authorization_job_spec.rb new file mode 100644 index 0000000..f2771fa --- /dev/null +++ b/spec/jobs/expire_remote_storage_authorization_job_spec.rb @@ -0,0 +1,36 @@ +require 'rails_helper' + +RSpec.describe ExpireRemoteStorageAuthorizationJob, type: :job do + before do + @user = create :user, cn: "ronald", ou: "kosmos.org" + @rs_authorization = create :remote_storage_authorization, user: @user, expire_at: 1.day.ago + end + + after do + clear_enqueued_jobs + clear_performed_jobs + end + + subject(:job) { + described_class.perform_later(@rs_authorization.id) + } + + let(:redis) { + @redis ||= Redis.new(url: Setting.redis_url) + } + + it "removes the RS authorization from redis" do + redis_key = "rs:authorizations:#{@user.address}:#{@rs_authorization.token}" + expect(redis.keys(redis_key)).to_not be_empty + + perform_enqueued_jobs { job } + + expect(redis.keys(redis_key)).to be_empty + end + + it "deletes the RS authorization object" do + expect { + perform_enqueued_jobs { job } + }.to change(RemoteStorageAuthorization, :count).by(-1) + end +end From 5c2df3df075abe6fa0b02eabdcfc1c6e7a20ddd4 Mon Sep 17 00:00:00 2001 From: Garret Alfert Date: Tue, 27 Jun 2023 15:07:28 +0200 Subject: [PATCH 06/20] Add Redis service to Drone config --- .drone.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.drone.yml b/.drone.yml index eb43e96..8a60399 100644 --- a/.drone.yml +++ b/.drone.yml @@ -42,6 +42,10 @@ steps: branch: - master +services: + - name: redis + image: redis + volumes: - name: cache host: From a68825493fe91de4875c4810c75ddd9253969f72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Tue, 4 Jul 2023 16:44:11 +0200 Subject: [PATCH 07/20] Add Redis config in CI --- .drone.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.drone.yml b/.drone.yml index 8a60399..dbe8c0f 100644 --- a/.drone.yml +++ b/.drone.yml @@ -20,6 +20,7 @@ steps: image: guildeducation/rails:2.7.2-14.20.0 environment: RAILS_ENV: test + REDIS_URL: redis://redis:6379/0 commands: - bundle config unset deployment - bundle config set cache_all 'true' From e5a5633e442ab469779090546c044926d75fafeb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Tue, 4 Jul 2023 17:00:04 +0200 Subject: [PATCH 08/20] Add Redis config for dev with Redis on localhost --- .env.test | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.env.test b/.env.test index 92892e3..0bd5bc9 100644 --- a/.env.test +++ b/.env.test @@ -1,5 +1,7 @@ PRIMARY_DOMAIN=kosmos.org +REDIS_URL='redis://localhost:6379/21' + DISCOURSE_PUBLIC_URL='http://discourse.example.com' DISCOURSE_CONNECT_SECRET='discourse_connect_ftw' From 4d88a4010926d314013fc33d50a06ff9af12aaba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Fri, 14 Jul 2023 15:27:30 +0200 Subject: [PATCH 09/20] Add separate config for RS Redis --- .drone.yml | 1 + .env.example | 1 + .env.test | 3 ++- app/models/setting.rb | 5 ++++- app/views/admin/settings/services/_remotestorage.html.erb | 6 +++++- docker-compose.yml | 2 ++ 6 files changed, 15 insertions(+), 3 deletions(-) diff --git a/.drone.yml b/.drone.yml index dbe8c0f..daf1849 100644 --- a/.drone.yml +++ b/.drone.yml @@ -21,6 +21,7 @@ steps: environment: RAILS_ENV: test REDIS_URL: redis://redis:6379/0 + RS_REDIS_URL: redis://redis:6379/1 commands: - bundle config unset deployment - bundle config set cache_all 'true' diff --git a/.env.example b/.env.example index fef8c93..081d0ae 100644 --- a/.env.example +++ b/.env.example @@ -26,6 +26,7 @@ GITEA_PUBLIC_URL='https://gitea.kosmos.org' MASTODON_PUBLIC_URL='https://kosmos.social' MEDIAWIKI_PUBLIC_URL='https://wiki.kosmos.org' RS_STORAGE_URL='https://storage.kosmos.org' +RS_REDIS_URL='redis://localhost:6379/2' EJABBERD_ADMIN_URL='https://xmpp.kosmos.org/admin' EJABBERD_API_URL='https://xmpp.kosmos.org/api' diff --git a/.env.test b/.env.test index 0bd5bc9..33761c3 100644 --- a/.env.test +++ b/.env.test @@ -1,6 +1,6 @@ PRIMARY_DOMAIN=kosmos.org -REDIS_URL='redis://localhost:6379/21' +REDIS_URL='redis://localhost:6379/0' DISCOURSE_PUBLIC_URL='http://discourse.example.com' DISCOURSE_CONNECT_SECRET='discourse_connect_ftw' @@ -14,5 +14,6 @@ LNDHUB_PUBLIC_URL='https://lndhub.kosmos.org' LNDHUB_PUBLIC_KEY='024cd3be18617f39cf645851e3ba63f51fc13f0bb09e3bb25e6fd4de556486d946' RS_STORAGE_URL='https://storage.kosmos.org' +RS_REDIS_URL='redis://localhost:6379/1' WEBHOOKS_ALLOWED_IPS='10.1.1.23' diff --git a/app/models/setting.rb b/app/models/setting.rb index f32c86f..fc3b068 100644 --- a/app/models/setting.rb +++ b/app/models/setting.rb @@ -12,7 +12,7 @@ class Setting < RailsSettings::Base # Internal services # - field :redis_url, type: :string, readonly: true, + field :redis_url, type: :string, default: ENV["REDIS_URL"] || "redis://localhost:6379/0" # @@ -131,4 +131,7 @@ class Setting < RailsSettings::Base field :rs_storage_url, type: :string, default: ENV["RS_STORAGE_URL"].presence + + field :rs_redis_url, type: :string, + default: ENV["RS_REDIS_URL"] || "redis://localhost:6379/1" end diff --git a/app/views/admin/settings/services/_remotestorage.html.erb b/app/views/admin/settings/services/_remotestorage.html.erb index 42aea32..5b8f47b 100644 --- a/app/views/admin/settings/services/_remotestorage.html.erb +++ b/app/views/admin/settings/services/_remotestorage.html.erb @@ -11,7 +11,11 @@ <% if Setting.remotestorage_enabled? %> <%= render FormElements::FieldsetResettableSettingComponent.new( key: :rs_storage_url, - title: "Storage URL" + title: "Storage Base URL" + ) %> + <%= render FormElements::FieldsetResettableSettingComponent.new( + key: :rs_redis_url, + title: "Redis URL" ) %> <% end %> diff --git a/docker-compose.yml b/docker-compose.yml index 9242fb6..3690616 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -38,6 +38,7 @@ services: RAILS_ENV: development PRIMARY_DOMAIN: kosmos.org REDIS_URL: redis://redis:6379/0 + RS_REDIS_URL: redis://redis:6379/1 LDAP_HOST: ldap LDAP_PORT: 3389 LDAP_ADMIN_PASSWORD: passthebutter @@ -57,6 +58,7 @@ services: RAILS_ENV: development PRIMARY_DOMAIN: kosmos.org REDIS_URL: redis://redis:6379/0 + RS_REDIS_URL: redis://redis:6379/1 LDAP_HOST: ldap LDAP_PORT: 3389 LDAP_ADMIN_PASSWORD: passthebutter From 4c6c81171bef7a243e21d0c2c5a0037fa771a29d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Fri, 14 Jul 2023 15:27:57 +0200 Subject: [PATCH 10/20] Fix typo --- app/models/remote_storage_authorization.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/remote_storage_authorization.rb b/app/models/remote_storage_authorization.rb index 13dfd9c..5e71c57 100644 --- a/app/models/remote_storage_authorization.rb +++ b/app/models/remote_storage_authorization.rb @@ -10,7 +10,7 @@ class RemoteStorageAuthorization < ApplicationRecord scope :expired, -> { where(expire_at: ..(DateTime.now)) } after_initialize do |a| - a.permisisons = [] if a.permissions == nil + a.permissions = [] if a.permissions == nil end before_create :generate_token From b0bfc290c476de08116ccdefe07cd9447d30ebe9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Fri, 14 Jul 2023 15:28:09 +0200 Subject: [PATCH 11/20] Refactor code for newer Redis --- app/models/remote_storage_authorization.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/models/remote_storage_authorization.rb b/app/models/remote_storage_authorization.rb index 5e71c57..e56246a 100644 --- a/app/models/remote_storage_authorization.rb +++ b/app/models/remote_storage_authorization.rb @@ -30,8 +30,7 @@ class RemoteStorageAuthorization < ApplicationRecord def delete_token_from_redis key = "rs:authorizations:#{user.address}:#{token}" - # You can't delete multiple members of a set with Redis 2 - redis.smembers(key).each { |auth| redis.srem(key, auth) } + redis.srem? key, redis.smembers(key) end private From 12b24337e7b04272193faaeb550c3d0004b794c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Fri, 14 Jul 2023 15:28:45 +0200 Subject: [PATCH 12/20] Fix typo --- app/models/remote_storage_authorization.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/app/models/remote_storage_authorization.rb b/app/models/remote_storage_authorization.rb index e56246a..be43a1e 100644 --- a/app/models/remote_storage_authorization.rb +++ b/app/models/remote_storage_authorization.rb @@ -47,10 +47,11 @@ class RemoteStorageAuthorization < ApplicationRecord redis.sadd "rs:authorizations:#{user.address}:#{token}", permissions end - def schedule_token_expiry - return unless expire_at.present? - ExpireRemoteStorageAuthorizationJob.set(wait_unil: expire_at).perform_later(id) - end + def schedule_token_expiry + return unless expire_at.present? + RemoteStorageExpireAuthorizationJob.set(wait_until: expire_at) + .perform_later(id) + end def remove_token_expiry_job queue = Sidekiq::Queue.new(ExpireRemoteStorageAuthorizationJob.queue_name) From e11be727a10b24a423c7c0f28f9096dd8699a911 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Fri, 14 Jul 2023 15:29:04 +0200 Subject: [PATCH 13/20] Indentation --- app/models/remote_storage_authorization.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/app/models/remote_storage_authorization.rb b/app/models/remote_storage_authorization.rb index be43a1e..15f677e 100644 --- a/app/models/remote_storage_authorization.rb +++ b/app/models/remote_storage_authorization.rb @@ -35,17 +35,17 @@ class RemoteStorageAuthorization < ApplicationRecord private - def redis - @redis ||= Redis.new(url: Setting.redis_url) - end + def redis + @redis ||= Redis.new(url: Setting.rs_redis_url) + end - def generate_token(length=16) - self.token = SecureRandom.hex(length) if self.token.blank? - end + def generate_token(length=16) + self.token = SecureRandom.hex(length) if self.token.blank? + end - def store_token_in_redis - redis.sadd "rs:authorizations:#{user.address}:#{token}", permissions - end + def store_token_in_redis + redis.sadd "rs:authorizations:#{user.address}:#{token}", permissions + end def schedule_token_expiry return unless expire_at.present? From 645abac810ea20d9c36c68d39c6ea9d8c52fa156 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Fri, 14 Jul 2023 15:29:29 +0200 Subject: [PATCH 14/20] Rename RS token expiry job --- ...rb => remote_storage_expire_authorization_job.rb} | 4 ++-- app/models/remote_storage_authorization.rb | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) rename app/jobs/{expire_remote_storage_authorization_job.rb => remote_storage_expire_authorization_job.rb} (69%) diff --git a/app/jobs/expire_remote_storage_authorization_job.rb b/app/jobs/remote_storage_expire_authorization_job.rb similarity index 69% rename from app/jobs/expire_remote_storage_authorization_job.rb rename to app/jobs/remote_storage_expire_authorization_job.rb index 8007e2c..62c240c 100644 --- a/app/jobs/expire_remote_storage_authorization_job.rb +++ b/app/jobs/remote_storage_expire_authorization_job.rb @@ -1,5 +1,5 @@ -class ExpireRemoteStorageAuthorizationJob < ApplicationJob - queue_as :remote_storage +class RemoteStorageExpireAuthorizationJob < ApplicationJob + queue_as :remotestorage def perform(rs_auth_id) rs_auth = RemoteStorageAuthorization.find rs_auth_id diff --git a/app/models/remote_storage_authorization.rb b/app/models/remote_storage_authorization.rb index 15f677e..82ac744 100644 --- a/app/models/remote_storage_authorization.rb +++ b/app/models/remote_storage_authorization.rb @@ -53,11 +53,11 @@ class RemoteStorageAuthorization < ApplicationRecord .perform_later(id) end - def remove_token_expiry_job - queue = Sidekiq::Queue.new(ExpireRemoteStorageAuthorizationJob.queue_name) - queue.each do |job| - next unless job.display_class == "ExpireRemoteStorageAuthorizationJob" - job.delete if job.display_args == [id] + def remove_token_expiry_job + queue = Sidekiq::Queue.new(RemoteStorageExpireAuthorizationJob.queue_name) + queue.each do |job| + next unless job.display_class == "RemoteStorageExpireAuthorizationJob" + job.delete if job.display_args == [id] + end end - end end From ec9bcacd46b22456837203c9971ba18adcc68643 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Fri, 14 Jul 2023 15:31:20 +0200 Subject: [PATCH 15/20] Add specs for RemoteStorageAuthorization model --- .../remote_storage_authorization_spec.rb | 212 ++++++++++++++++++ spec/rails_helper.rb | 1 + spec/support/helpers/redis_helper.rb | 8 + 3 files changed, 221 insertions(+) create mode 100644 spec/models/remote_storage_authorization_spec.rb create mode 100644 spec/support/helpers/redis_helper.rb diff --git a/spec/models/remote_storage_authorization_spec.rb b/spec/models/remote_storage_authorization_spec.rb new file mode 100644 index 0000000..3d046cf --- /dev/null +++ b/spec/models/remote_storage_authorization_spec.rb @@ -0,0 +1,212 @@ +require 'rails_helper' + +RSpec.describe RemoteStorageAuthorization, type: :model do + include ActiveJob::TestHelper + + let(:user) { create :user } + + describe "#create" do + after(:each) { clear_enqueued_jobs } + after(:all) { redis_rs_delete_keys("rs:authorizations:*") } + + let(:auth) do + user.remote_storage_authorizations.create!( + permissions: %w(documents photos contacts:rw videos:r tasks/work:r), + client_id: "example.com", + redirect_uri: "https://example.com" + ) + end + + it "generates a token" do + expect(auth.token).to match(/[a-zA-Z0-9]+/) + end + + it "stores a token in redis" do + user_auth_keys = redis_rs.keys("rs:authorizations:#{user.address}:*") + expect(user_auth_keys.length).to eq(1) + + authorizations = redis_rs.smembers(user_auth_keys.first) + expect(authorizations.sort).to eq(%w(documents photos contacts:rw videos:r tasks/work:r).sort) + end + + context "with expiry set" do + it "enqueues an expiration job" do + auth_with_expiry = user.remote_storage_authorizations.create!( + permissions: %w(documents:rw), client_id: "example.com", + redirect_uri: "https://example.com", + expire_at: 1.month.from_now + ) + job = enqueued_jobs.select{|j| j['job_class'] == "RemoteStorageExpireAuthorizationJob"}.first + expect(job['arguments'][0]).to eq(auth_with_expiry.id) + end + end + end + + describe "#destroy" do + after(:each) { clear_enqueued_jobs } + after(:all) { redis_rs_delete_keys("rs:authorizations:*") } + + it "removes the token from redis" do + auth = user.remote_storage_authorizations.create!( + permissions: %w(shares:rw documents pictures:r), + client_id: "sharesome.5apps.com", + redirect_uri: "https://sharesome.5apps.com" + ) + auth.destroy! + + expect(redis_rs.keys("rs:authorizations:#{user.address}:*")).to be_empty + end + + context "with expiry set" do + it "removes the expiration job" do + auth_with_expiry = user.remote_storage_authorizations.create!( + permissions: %w(documents:rw), client_id: "example.com", + redirect_uri: "https://example.com", + expire_at: 1.month.from_now + ) + # Cannot test for removal from the actual Sidekiq::Queue, because it is + # not used in specs, but the method directly removes jobs from there + expect(auth_with_expiry).to receive(:remove_token_expiry_job) + auth_with_expiry.destroy! + end + end + end + + # describe "#find_or_create_web_app" do + # context "with origin that looks hosted" do + # before do + # auth = user.remote_storage_authorizations.create!( + # permissions: %w(documents photos contacts:rw videos:r tasks/work:r), + # client_id: "example.com", + # redirect_uri: "https://example.com", + # expire_at: 1.month.from_now + # ) + # end + # + # it "generates a web_app" do + # expect(auth.web_app).to be_a(AppCatalog::WebApp) + # end + # + # it "uses the Web App's name as app name" do + # expect(auth.app_name).to eq("Example Domain") + # end + # end + # + # context "when creating two authorizations for the same app" do + # before do + # user_2 = create :user + # ResqueSpec.reset! + # auth_1 = user.remote_storage_authorizations.create!( + # permissions: %w(documents photos contacts:rw videos:r tasks/work:r), + # client_id: "example.com", + # redirect_uri: "https://example.com", + # expire_at: 1.month.from_now + # ) + # auth_2 = user_2.remote_storage_authorizations.create!( + # permissions: %w(documents photos contacts:rw videos:r tasks/work:r), + # client_id: "example.com", + # redirect_uri: "https://example.com", + # expire_at: 1.month.from_now + # ) + # end + # + # after do + # auth_1.destroy + # auth_2.destroy + # user_2.destroy + # end + # + # it "uses the same web app instance for both authorizations" do + # expect(auth_1.web_app).to be_a(AppCatalog::WebApp) + # expect(auth_1.web_app).to eq(auth_2.web_app) + # end + # end + # + # describe "non-production app origins" do + # context "when host is not an FQDN" do + # before do + # auth = user.remote_storage_authorizations.create!( + # permissions: %w(recipes), + # client_id: "localhost:4200", + # redirect_uri: "http://localhost:4200" + # ) + # end + # + # it "does not create a web app" do + # expect(auth.web_app).to be_nil + # expect(auth.app_name).to eq("localhost:4200") + # end + # end + # + # context "when host is an IP address" do + # before do + # auth = user.remote_storage_authorizations.create!( + # permissions: %w(recipes), + # client_id: "192.168.0.23:3000", + # redirect_uri: "http://192.168.0.23:3000" + # ) + # end + # + # it "does not create a web app" do + # expect(auth.web_app).to be_nil + # expect(auth.app_name).to eq("192.168.0.23:3000") + # end + # end + # + # context "when host is an extension URL" do # before do + # auth = user.remote_storage_authorizations.create!( + # permissions: %w(bookmarks), + # client_id: "123.addons.allizom.org", + # redirect_uri: "123.addons.allizom.org/foo" + # ) + # end + # + # it "does not create a web app" do + # expect(auth.web_app).to be_nil + # expect(auth.app_name).to eq("123.addons.allizom.org") + # end + # end + # end + # end + + # describe "auth notifications" do + # context "with auth notifications enabled" do + # before do + # ResqueSpec.reset! + # user.push(mailing_lists: "rs-auth-notifications-#{Rails.env}") + # auth = user.remote_storage_authorizations.create!( + # :permissions => %w(documents photos contacts:rw videos:r tasks/work:r), + # :client_id => "example.com", + # :redirect_uri => "https://example.com" + # ) + # end + # + # it "notifies the user via email" do + # expect(enqueued_jobs.size).to eq(1) + # job = enqueued_jobs.first + # expect(job).to eq( + # job: ActionMailer::DeliveryJob, + # args: ['StorageAuthorizationMailer', 'authorized_rs_app', 'deliver_now', + # auth.id.to_s], + # queue: 'mailers' + # ) + # end + # end + # + # context "with auth notifications disabled" do + # before do + # ResqueSpec.reset! + # user.pull(mailing_lists: "rs-auth-notifications-#{Rails.env}") + # auth = user.remote_storage_authorizations.create!( + # :permissions => %w(documents photos contacts:rw videos:r tasks/work:r), + # :client_id => "example.com", + # :redirect_uri => "https://example.com" + # ) + # end + # + # it "does not notify the user via email about new RS app" do + # expect(enqueued_jobs.size).to eq(0) + # end + # end + # end +end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index eab4dee..ae6309d 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -10,6 +10,7 @@ require 'capybara' require 'devise' require 'support/controller_macros' require 'support/database_cleaner' +require 'support/helpers/redis_helper' require "view_component/test_helpers" require "capybara/rspec" diff --git a/spec/support/helpers/redis_helper.rb b/spec/support/helpers/redis_helper.rb new file mode 100644 index 0000000..2571fce --- /dev/null +++ b/spec/support/helpers/redis_helper.rb @@ -0,0 +1,8 @@ +def redis_rs + @redis_rs ||= Redis.new(url: Setting.rs_redis_url) +end + +def redis_rs_delete_keys(pattern) + keys = redis_rs.keys(pattern) + redis_rs.del(*keys) +end From fcf9a065e121bca1f4d3f75f79b92d3e1643f2a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Fri, 14 Jul 2023 15:56:04 +0200 Subject: [PATCH 16/20] Fix specs --- ...pec.rb => remote_storage_expire_authorization_job_spec.rb} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename spec/jobs/{expire_remote_storage_authorization_job_spec.rb => remote_storage_expire_authorization_job_spec.rb} (87%) diff --git a/spec/jobs/expire_remote_storage_authorization_job_spec.rb b/spec/jobs/remote_storage_expire_authorization_job_spec.rb similarity index 87% rename from spec/jobs/expire_remote_storage_authorization_job_spec.rb rename to spec/jobs/remote_storage_expire_authorization_job_spec.rb index f2771fa..e65662e 100644 --- a/spec/jobs/expire_remote_storage_authorization_job_spec.rb +++ b/spec/jobs/remote_storage_expire_authorization_job_spec.rb @@ -1,6 +1,6 @@ require 'rails_helper' -RSpec.describe ExpireRemoteStorageAuthorizationJob, type: :job do +RSpec.describe RemoteStorageExpireAuthorizationJob, type: :job do before do @user = create :user, cn: "ronald", ou: "kosmos.org" @rs_authorization = create :remote_storage_authorization, user: @user, expire_at: 1.day.ago @@ -16,7 +16,7 @@ RSpec.describe ExpireRemoteStorageAuthorizationJob, type: :job do } let(:redis) { - @redis ||= Redis.new(url: Setting.redis_url) + @redis ||= Redis.new(url: Setting.rs_redis_url) } it "removes the RS authorization from redis" do From a2d27bf57537e988dfaf199122d42209b7d2e01e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Tue, 1 Aug 2023 13:00:22 +0200 Subject: [PATCH 17/20] Support pre-filling of username in login form --- app/views/devise/sessions/new.html.erb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/views/devise/sessions/new.html.erb b/app/views/devise/sessions/new.html.erb index d3d02e0..ddb0a44 100644 --- a/app/views/devise/sessions/new.html.erb +++ b/app/views/devise/sessions/new.html.erb @@ -12,7 +12,8 @@
<%= f.label :cn, 'User', class: 'block mb-2 font-bold' %>

- <%= f.text_field :cn, autofocus: true, autocomplete: "username", + <%= f.text_field :cn, value: h(params[:cn]), + autofocus: params[:cn].blank?, autocomplete: "username", required: true, class: "relative grow", tabindex: "1" %> @ <%= Setting.primary_domain %>

@@ -20,7 +21,8 @@

<%= f.label :password, class: 'block mb-2 font-bold' %> <%= f.password_field :password, autocomplete: "current-password", - required: true, class: "w-full", tabindex: "2" %> + autofocus: params[:cn].present?, required: true, + class: "w-full", tabindex: "2" %>

<%= tag.div class: "flex items-center mb-8 gap-x-3", data: { From 5f921f1b53846b2a93b74381f9ae2d42c4205b45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Tue, 1 Aug 2023 13:01:03 +0200 Subject: [PATCH 18/20] RS OAuth pre-fills username for login --- app/controllers/rs/oauth_controller.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/app/controllers/rs/oauth_controller.rb b/app/controllers/rs/oauth_controller.rb index 2aa25e0..e083ef6 100644 --- a/app/controllers/rs/oauth_controller.rb +++ b/app/controllers/rs/oauth_controller.rb @@ -1,5 +1,5 @@ class Rs::OauthController < ApplicationController - before_action :require_user_signed_in + before_action :require_signed_in_with_username def new username, org = params[:useraddress].split("@") @@ -97,6 +97,13 @@ class Rs::OauthController < ApplicationController private + def require_signed_in_with_username + unless user_signed_in? + username, org = params[:useraddress].split("@") + redirect_to new_user_session_path(cn: username, ou: org) + end + end + def app_auth_url(auth) url = "#{auth.url}#remotestorage=#{current_user.address}" url += "&access_token=#{auth.token}" From ba0cbba96bc8096abae8a966e1c2272120d51aea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Tue, 1 Aug 2023 13:01:33 +0200 Subject: [PATCH 19/20] Add feature spec for RS OAuth dialog --- app/views/rs/oauth/new.html.erb | 6 +-- spec/features/rs/oauth_spec.rb | 66 +++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 3 deletions(-) create mode 100644 spec/features/rs/oauth_spec.rb diff --git a/app/views/rs/oauth/new.html.erb b/app/views/rs/oauth/new.html.erb index d34001b..3d63f81 100644 --- a/app/views/rs/oauth/new.html.erb +++ b/app/views/rs/oauth/new.html.erb @@ -1,7 +1,7 @@ <%= render HeaderCompactComponent.new(title: "Storage") %> <%= render MainCompactComponent.new do %> -
+

The app on <%= link_to @client_id, "https://#{@client_id}", class: "ks-text-link" %> @@ -9,7 +9,7 @@

<% if @root_access_requested %> -

+

<%= render partial: "icons/alert-triangle", locals: { custom_class: "inline-block align-bottom mr-1.5" } %> @@ -21,7 +21,7 @@

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

+

<%= render partial: "icons/folder", locals: { custom_class: "inline-block align-bottom mr-1.5" } %> diff --git a/spec/features/rs/oauth_spec.rb b/spec/features/rs/oauth_spec.rb new file mode 100644 index 0000000..48a5a13 --- /dev/null +++ b/spec/features/rs/oauth_spec.rb @@ -0,0 +1,66 @@ +require 'rails_helper' + +RSpec.describe 'remoteStorage OAuth Dialog', type: :feature do + context "when signed in" do + let(:user) { create :user } + + before do + login_as user, :scope => :user + end + + context "with normal permissions" do + before do + visit new_rs_oauth_path(useraddress: user.address, + redirect_uri: "http://example.com", + client_id: "http://example.com", + scope: "documents,[photos], contacts:r") + end + + it "shows the permissions in a list" do + within ".permissions" do + expect(page).to have_content("documents") + expect(page).to have_content("photos") + expect(page).to have_content("contacts") + end + + within ".scope:first-of-type" do + expect(page).not_to have_content("read only") + end + + within ".scope:last-of-type" do + expect(page).to have_content("read only") + end + end + end + + context "root access" do + context "full" do + before do + visit new_rs_oauth_path(useraddress: user.address, + redirect_uri: "http://example.com", + client_id: "http://example.com", + scope: ":rw") + end + + it "shows a special permission for all files and dirs" do + within ".scope" do + expect(page).to have_content("All files and directories") + end + end + end + end + end + + context "when signed out" do + let(:user) { create :user } + + it "prefills the username field in the signin form" do + visit new_rs_oauth_path(useraddress: user.address, + redirect_uri: "http://example.com", + client_id: "http://example.com", + scope: "documents,[photos], contacts:r") + + expect(find("#user_cn").value).to eq(user.cn) + end + end +end From 353b55fe1a5f78e03f4db275febee943076f0825 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Tue, 1 Aug 2023 13:58:52 +0200 Subject: [PATCH 20/20] Add RS OAuth controller specs --- Gemfile | 1 + Gemfile.lock | 5 + app/controllers/rs/oauth_controller.rb | 34 +- app/views/shared/status_bad_request.html.erb | 6 + config/routes.rb | 5 +- spec/controllers/rs/oauth_controller_spec.rb | 465 +++++++++++++++++++ 6 files changed, 501 insertions(+), 15 deletions(-) create mode 100644 app/views/shared/status_bad_request.html.erb create mode 100644 spec/controllers/rs/oauth_controller_spec.rb diff --git a/Gemfile b/Gemfile index 1f0a7b3..ff3396f 100644 --- a/Gemfile +++ b/Gemfile @@ -64,6 +64,7 @@ group :development, :test do # Use sqlite3 as the database for Active Record gem 'sqlite3', '~> 1.4' gem 'rspec-rails' + gem 'rails-controller-testing' gem "byebug", "~> 11.1" end diff --git a/Gemfile.lock b/Gemfile.lock index 439cd1e..04c1e55 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -269,6 +269,10 @@ GEM activesupport (= 7.0.5) bundler (>= 1.15.0) railties (= 7.0.5) + rails-controller-testing (1.0.5) + actionpack (>= 5.0.1.rc1) + actionview (>= 5.0.1.rc1) + activesupport (>= 5.0.1.rc1) rails-dom-testing (2.0.3) activesupport (>= 4.2.0) nokogiri (>= 1.6) @@ -444,6 +448,7 @@ DEPENDENCIES pg (~> 1.2.3) puma (~> 4.1) rails (~> 7.0.2) + rails-controller-testing rails-settings-cached (~> 2.8.3) rqrcode (~> 2.0) rspec-rails diff --git a/app/controllers/rs/oauth_controller.rb b/app/controllers/rs/oauth_controller.rb index e083ef6..061bf84 100644 --- a/app/controllers/rs/oauth_controller.rb +++ b/app/controllers/rs/oauth_controller.rb @@ -1,5 +1,6 @@ class Rs::OauthController < ApplicationController - before_action :require_signed_in_with_username + before_action :require_signed_in_with_username, only: :new + before_action :authenticate_user!, only: :create def new username, org = params[:useraddress].split("@") @@ -30,23 +31,26 @@ class Rs::OauthController < ApplicationController end unless @client_id.present? - redirect_to url_with_state("#{@redirect_uri}#error=invalid_request", @state) and return + redirect_to(url_with_state("#{@redirect_uri}#error=invalid_request", @state), + allow_other_host: true) and return end if @scopes.empty? - redirect_to url_with_state("#{@redirect_uri}#error=invalid_scope", @state) and return + redirect_to(url_with_state("#{@redirect_uri}#error=invalid_scope", @state), + allow_other_host: true) and return end unless hostname_of(@client_id) == hostname_of(@redirect_uri) - redirect_to url_with_state("#{@redirect_uri}#error=invalid_client", @state) and return + redirect_to(url_with_state("#{@redirect_uri}#error=invalid_client", @state), + allow_other_host: true) and return end @client_id.gsub!(/http(s)?:\/\//, "") - # TODO - # if auth = current_user.remote_storage_authorizations.valid.where(permissions: @scopes, client_id: @client_id).first - # redirect_to url_with_state("#{@redirect_uri}#access_token=#{auth.token}", @state), allow_other_host: true - # end + if auth = current_user.remote_storage_authorizations.valid.where(permissions: @scopes, client_id: @client_id).first + redirect_to(url_with_state("#{@redirect_uri}#access_token=#{auth.token}", @state), + allow_other_host: true) and return + end end def create @@ -64,15 +68,18 @@ class Rs::OauthController < ApplicationController http_status :bad_request and return unless redirect_uri.present? if permissions.empty? - redirect_to url_with_state("#{redirect_uri}#error=invalid_scope", state), allow_other_host: true and return + redirect_to(url_with_state("#{redirect_uri}#error=invalid_scope", state), + allow_other_host: true) and return end unless client_id.present? - redirect_to url_with_state("#{redirect_uri}#error=invalid_request", state), allow_other_host: true and return + redirect_to(url_with_state("#{redirect_uri}#error=invalid_request", state), + allow_other_host: true) and return end unless hostname_of(client_id) == hostname_of(redirect_uri) - redirect_to url_with_state("#{redirect_uri}#error=invalid_client", state), allow_other_host: true and return + redirect_to(url_with_state("#{redirect_uri}#error=invalid_client", state), + allow_other_host: true) and return end client_id.gsub!(/http(s)?:\/\//, "") @@ -85,14 +92,15 @@ class Rs::OauthController < ApplicationController expire_at: expire_at ) - redirect_to url_with_state("#{redirect_uri}#access_token=#{auth.token}", state), allow_other_host: true + redirect_to url_with_state("#{redirect_uri}#access_token=#{auth.token}", state), + allow_other_host: true end # GET /rs/oauth/token/:id/launch_app def launch_app auth = current_user.remote_storage_authorizations.find(params[:id]) - redirect_to app_auth_url(auth) + redirect_to app_auth_url(auth), allow_other_host: true end private diff --git a/app/views/shared/status_bad_request.html.erb b/app/views/shared/status_bad_request.html.erb new file mode 100644 index 0000000..d8851df --- /dev/null +++ b/app/views/shared/status_bad_request.html.erb @@ -0,0 +1,6 @@ +<%= render HeaderCompactComponent.new(title: "404") %> + +<%= render MainCompactComponent.new do %> +

Bad request

+

Please go back and try again.

+<% end %> diff --git a/config/routes.rb b/config/routes.rb index 0f05b71..7d51379 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -67,8 +67,9 @@ Rails.application.routes.draw do end namespace :rs do - resource :oauth, only: [:new, :create], path_names: { new: ':useraddress' }, - controller: 'oauth', constraints: { useraddress: /[^\/]+/} + resource :oauth, only: [:new, :create], path_names: { + new: ':useraddress', create: ':useraddress' + }, controller: 'oauth', constraints: { useraddress: /[^\/]+/} get 'oauth/token/:id/launch_app' => 'oauth#launch_app', as: :launch_app end diff --git a/spec/controllers/rs/oauth_controller_spec.rb b/spec/controllers/rs/oauth_controller_spec.rb new file mode 100644 index 0000000..48ca14b --- /dev/null +++ b/spec/controllers/rs/oauth_controller_spec.rb @@ -0,0 +1,465 @@ +require 'rails_helper' + +RSpec.describe Rs::OauthController, type: :controller do + let(:user) { create :user } + + describe "GET /rs/oauth/:useraddress" do + context "when user is signed in" do + before do + sign_in user + end + + context "when username is different than current user" do + let(:other_user) { create :user, id: 23, cn: "jomokenyatta", email: "jomo@hotmail.com" } + + before do + get :new, params: { + useraddress: other_user.address, + redirect_uri: "https://example.com", + client_id: "example.com", + scope: "examples" + } + end + + it "logs out the users and repeats the request" do + url = new_rs_oauth_url other_user.address, + redirect_uri: "https://example.com", + client_id: "example.com", + scope: "examples" + + expect(response).to redirect_to(url) + end + end + + context "when no valid token exists" do + before do + get :new, params: { + useraddress: user.address, + redirect_uri: "https://example.com", + client_id: "example.com", + scope: "documents,[photos], contacts:rw videos:r tasks/work/:r", + state: "foobar123" + } + end + + it "returns a 200" do + expect(response.response_code).to eq(200) + end + + it "sets the instance variables" do + expected_scopes = %w(documents photos contacts:rw videos:r tasks/work:r) + + expect(assigns["user"]).to eq(user) + expect(assigns["redirect_uri"]).to eq("https://example.com") + expect(assigns["scopes"]).to eq(expected_scopes) + expect(assigns["client_id"]).to eq("example.com") + expect(assigns["root_access_requested"]).to eq(false) + expect(assigns["state"]).to eq("foobar123") + expect(assigns["denial_url"]).to eq("https://example.com#error=access_denied&state=foobar123") + end + + context "no redirect_uri" do + before do + get :new, params: { + useraddress: user.address, + scope: "documents,[photos], contacts:rw videos:r tasks/work/:r", + client_id: "https://example.com" + } + end + + it "returns a 400" do + expect(response.response_code).to eq(400) + end + end + + context "no client_id" do + before do + get :new, params: { + useraddress: user.address, + scope: "documents,[photos], contacts:rw videos:r tasks/work/:r", + redirect_uri: "https://example.com" + } + end + + it "redirects with invalid_request error" do + expect(response).to redirect_to("https://example.com#error=invalid_request") + end + end + + context "different host for client_id and redirect_uri" do + before do + get :new, params: { + useraddress: user.address, + scope: "documents,[photos], contacts:rw videos:r tasks/work/:r", + redirect_uri: "https://example.com/foobar", + client_id: "https://google.com" + } + end + + it "redirects with invalid_client error" do + expect(response).to redirect_to("https://example.com/foobar#error=invalid_client") + end + end + end + + context "when valid token already exists" do + before do + @auth = user.remote_storage_authorizations.create!( + permissions: %w(documents photos contacts:rw videos:r tasks/work:r), + client_id: "example.com", redirect_uri: "https://example.com", + expire_at: 1.day.from_now + ) + end + + after { @auth.destroy } + + context "with same host for client_id and redirect_uri" do + before do + get :new, params: { + useraddress: user.address, + scope: "documents,[photos], contacts:rw videos:r tasks/work/:r", + redirect_uri: "https://example.com", + client_id: "https://example.com" + } + end + + it "redirects to the redirect_uri with the existing token" do + expect(response).to redirect_to("https://example.com#access_token=#{@auth.token}") + end + end + + context "with different host for client_id and redirect_uri" do + before do + get :new, params: { + useraddress: user.address, + scope: "documents,[photos], contacts:rw videos:r tasks/work/:r", + redirect_uri: "https://app.example.com", + client_id: "https://example.com" + } + end + + it "redirects with invalid_client error" do + expect(response).to redirect_to("https://app.example.com#error=invalid_client") + end + end + + context "with different redirect_uri" do + before do + get :new, params: { + useraddress: user.address, + scope: "documents,[photos], contacts:rw videos:r tasks/work/:r", + redirect_uri: "https://example.com/a_new_route", + client_id: "https://example.com" + } + end + + it "redirects to the new redirect_uri" do + expect(response).to redirect_to("https://example.com/a_new_route#access_token=#{@auth.token}") + end + end + + context "with state param given" do + before do + get :new, params: { + useraddress: user.address, + scope: "documents,[photos], contacts:rw videos:r tasks/work/:r", + redirect_uri: "https://example.com", + client_id: "https://example.com", + state: "foobar123" + } + end + + it "redirects to the redirect_uri with token and state" do + expect(response).to redirect_to("https://example.com#access_token=#{@auth.token}&state=foobar123") + end + end + end + + context "no scope" do + before do + get :new, params: { + useraddress: user.address, + redirect_uri: "https://example.com", + client_id: "https://example.com", + state: "foobar123" + } + end + + it "redirects to the redirect_uri with an error code" do + expect(response).to redirect_to("https://example.com#error=invalid_scope&state=foobar123") + end + end + + context "empty scope" do + before do + get :new, params: { + useraddress: user.address, + scope: "", + redirect_uri: "https://example.com", + client_id: "https://example.com", + state: "foobar123" + } + end + + it "redirects to the redirect_uri with an error code" do + expect(response).to redirect_to("https://example.com#error=invalid_scope&state=foobar123") + end + end + end + + context "when user is not signed in" do + it "redirects to the signin page with username pre-filled" do + get :new, params: { + useraddress: user.address, + scope: "documents,photos", + redirect_uri: "https://example.com" + } + + expect(response).to redirect_to(new_user_session_path(cn: user.cn, ou: user.ou)) + end + end + + describe "root access" do + before do + sign_in user + end + + describe "full" do + before do + get :new, params: { + useraddress: user.address, + scope: "*:rw", + redirect_uri: "https://example.com", + client_id: "example.com" + } + end + + it "sets the instance variables" do + expect(assigns["scopes"]).to eq([":rw"]) + expect(assigns["root_access_requested"]).to be(true) + end + end + + describe "read-only" do + before do + get :new, params: { + useraddress: user.address, + scope: "*:r", + redirect_uri: "https://example.com", + client_id: "example.com" + } + end + + it "sets the instance variables" do + expect(assigns["scopes"]).to eq([":r"]) + expect(assigns["root_access_requested"]).to be(true) + end + end + end + end + + describe "POST /rs/oauth/:useraddress" do + context "when user is signed in" do + before do + sign_in user + end + + after do + user.remote_storage_authorizations.destroy_all + end + + context "when no valid token exists" do + before do + post :create, params: { + user_id: user.id, + scope: "documents,[photos], contacts:rw videos:r tasks/work/:r", + redirect_uri: "https://example.com", + client_id: "example.com", + state: "foobar123", + expire_at: 1.day.from_now + } + @auth = user.reload.remote_storage_authorizations.first + end + + it "creates a new token" do + expect(@auth.permissions).to eq(%w(documents photos contacts:rw videos:r tasks/work:r)) + end + + it "redirects to the redirect_uri" do + expect(response).to redirect_to("https://example.com#access_token=#{@auth.token}&state=foobar123") + end + end + + context "when token is expired" do + before do + @auth = user.remote_storage_authorizations.create!( + permissions: %w(documents), + client_id: "example.com", + redirect_uri: "https://example.com", + expire_at: 1.day.ago, + token: nil + ) + post :create, params: { + user_id: user.id, + scope: "documents", + redirect_uri: "https://example.com", + client_id: "example.com", + state: "foobar123", + expire_at: 1.month.from_now + } + end + + it "updates the token" do + expect(@auth.reload.token).not_to be_nil + end + end + + context "root access with several scopes" do + before do + post :create, params: { + user_id: user.id, + scope: "*:rw contacts:r", + redirect_uri: "https://example.com", + client_id: "example.com", + expire_at: 1.month.from_now + } + @auth = user.reload.remote_storage_authorizations.first + end + + it "removes all scopes except for the root permission" do + expect(@auth.permissions).to eq(%w(:rw)) + end + end + + context "no redirect_uri" do + before do + post :create, params: { + user_id: user.id, + scope: "documents,[photos], contacts:rw videos:r tasks/work/:r", + client_id: "example.com", + expire_at: 1.month.from_now + } + end + + it "returns a 400" do + expect(response.response_code).to eq(400) + end + end + + context "no client_id" do + before do + post :create, params: { + user_id: user.id, + scope: "documents,[photos], contacts:rw videos:r tasks/work/:r", + redirect_uri: "https://example.com", + expire_at: 1.month.from_now + } + end + + it "redirects with invalid_request error" do + expect(response).to redirect_to("https://example.com#error=invalid_request") + end + end + + context "hostnames of client_id and redirect_uri do not match" do + before do + post :create, params: { + user_id: user.id, + scope: "documents,[photos], contacts:rw videos:r tasks/work/:r", + client_id: "fishing.com", + redirect_uri: "https://example.com", + expire_at: 1.month.from_now + } + end + + it "redirects with invalid_client error" do + expect(response).to redirect_to("https://example.com#error=invalid_client") + end + end + + context "empty scope" do + before do + post :create, params: { + user_id: user.id, + scope: "", + redirect_uri: "https://example.com", + client_id: "example.com", + state: "foobar123", + expire_at: 1.month.from_now + } + end + + it "redirects to the redirect_uri with an error code" do + expect(response).to redirect_to("https://example.com#error=invalid_scope&state=foobar123") + end + end + + context "when the user_id is different from the signed in user" do + before do + post :create, params: { + user_id: user.id, + scope: "documents,photos", + redirect_uri: "https://example.com", + client_id: "example.com", + expire_at: 1.month.from_now + } + end + + it "returns a 403" do + post :create, params: { + user_id: "69", + scope: "documents,photos", + redirect_uri: "https://example.com", + client_id: "example.com", + expire_at: 1.month.from_now + } + + expect(response.response_code).to eq(403) + end + end + end + + context "when user is not signed in" do + it "redirects to the signin page" do + post :create, params: { + user_id: user.id, + scope: "documents,photos", + redirect_uri: "https://example.com", + client_id: "example.com", + expire_at: 1.month.from_now + } + + expect(response).to redirect_to(new_user_session_path) + end + end + end + + describe "GET /rs/oauth/token/:id/launch_app" do + context "when user is signed in" do + before do + sign_in user + end + + context "token exists" do + before 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 + ) + + get :launch_app, params: { id: @auth.id } + end + + after do + @auth.destroy + end + + it "redirects to the given URL with the correct RS URL fragment params" do + launch_url = "https://app.example.com#remotestorage=#{user.address}&access_token=#{@auth.token}" + expect(response).to redirect_to(launch_url) + end + end + end + end +end