From 353b55fe1a5f78e03f4db275febee943076f0825 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A2u=20Cao?= <raucao@kip.pe>
Date: Tue, 1 Aug 2023 13:58:52 +0200
Subject: [PATCH] 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 %>
+  <h2>Bad request</h2>
+  <p>Please go back and try again.</p>
+<% 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