Fix infinite redirect loop for user without organization and that is …

…not superuser, nor partner (#3808) * Haradd's contributions + suggested changes * that was weird * forgot 1 spec * last commit didn't go through * orginal specs were failing -> changed tests to follow recreation * linter * smalllll change * try * capybara wait wait wait * linter * find body * sleep * linter * move to do before * and capy? * sign in * add puts * ignore_query: true * puts * try different puts * remove content check * try url: true * back to og * fix tests * linter * fix title --------- Co-authored-by: Brock Wilcox <[email protected]> Co-authored-by: Daniel Orner <[email protected]>
rubyforgood · Nov 24, 2023 · 68271ba · 68271ba
1 parent 8cfc864
commit 68271ba
Show file tree

Hide file tree

Showing 4 changed files with 91 additions and 4 deletions.
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
@@ -75,7 +75,7 @@ def dashboard_path_from_current_role
     elsif current_user.organization
       dashboard_path(current_user.organization)
     else
-      root_path
+      "/403"
     end
   end
 

diff --git a/public/403.html b/public/403.html
@@ -0,0 +1,66 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <title>The page you were looking for is forbidden (403)</title>
+  <meta name="viewport" content="width=device-width,initial-scale=1">
+  <style>
+  body {
+    background-color: #EFEFEF;
+    color: #2E2F30;
+    text-align: center;
+    font-family: arial, sans-serif;
+    margin: 0;
+  }
+
+  div.dialog {
+    width: 95%;
+    max-width: 33em;
+    margin: 4em auto 0;
+  }
+
+  div.dialog > div {
+    border: 1px solid #CCC;
+    border-right-color: #999;
+    border-left-color: #999;
+    border-bottom-color: #BBB;
+    border-top: #B00100 solid 4px;
+    border-top-left-radius: 9px;
+    border-top-right-radius: 9px;
+    background-color: white;
+    padding: 7px 12% 0;
+    box-shadow: 0 3px 8px rgba(50, 50, 50, 0.17);
+  }
+
+  h1 {
+    font-size: 100%;
+    color: #730E15;
+    line-height: 1.5em;
+  }
+
+  div.dialog > p {
+    margin: 0 0 1em;
+    padding: 1em;
+    background-color: #F7F7F7;
+    border: 1px solid #CCC;
+    border-right-color: #999;
+    border-left-color: #999;
+    border-bottom-color: #999;
+    border-bottom-left-radius: 4px;
+    border-bottom-right-radius: 4px;
+    border-top-color: #DADADA;
+    color: #666;
+    box-shadow: 0 3px 8px rgba(50, 50, 50, 0.17);
+  }
+  </style>
+</head>
+
+<body>
+  <!-- This file lives in public/403.html -->
+  <div class="dialog">
+    <div>
+      <h1>The page you were looking for is forbidden.</h1>
+    </div>
+    <p>If you are the application owner check the logs for more information.</p>
+  </div>
+</body>
+</html>
diff --git a/spec/requests/static_requests_spec.rb b/spec/requests/static_requests_spec.rb
@@ -34,6 +34,21 @@
     end
   end
 
+  describe "Non super user without org signed in" do
+    let(:user_no_org) { User.create(email: "[email protected]", password: "password!") }
+    before do
+      user_no_org.add_role(:org_user)
+      sign_in(user_no_org)
+    end
+
+    describe "GET #index" do
+      it "redirects to a public/403.html page" do
+        get root_path
+        expect(response).to redirect_to("/403")
+      end
+    end
+  end
+
   describe "Super user without org signed in" do
     before do
       sign_in(@super_admin_no_org)

diff --git a/spec/system/sign_in_system_spec.rb b/spec/system/sign_in_system_spec.rb
@@ -40,13 +40,19 @@
   end
 
   context "when users are valid and don't belong to an organization" do
-    it "redirects to home " do
-      user_no_org = create(:user, organization: nil)
+    let(:user_no_org) { User.create(email: '[email protected]', password: 'password!') }
+
+    before do
+      user_no_org.add_role(:org_user)
+      visit new_user_session_path
+
       fill_in "Email", with: user_no_org.email
       fill_in "Password", with: user_no_org.password
       click_button "Log in"
+    end
 
-      expect(page).to have_current_path(root_path)
+    it "redirects to 403" do
+      expect(page).to have_current_path("/403")
     end
   end
 end
-Original file line number
+Diff line change
@@ Expand Up / @@ -75,7 +75,7 @@ def dashboard_path_from_current_role @@
         elsif current_user.organization
           dashboard_path(current_user.organization)
         else
-          root_path
+          "/403"
         end
       end
@@ Expand Down @@