MultiUsers: require username when logging in (#1019)

Author: mockdeep

.rubocop.yml

@@ -15,7 +15,9 @@ AllCops:
 
 Layout/LineLength: { Max: 80 }
 Layout/RedundantLineBreak: { InspectBlocks: true }
-Metrics/AbcSize: { Exclude: [db/migrate/20230305010750_create_good_jobs.rb] }
+Metrics/AbcSize:
+  Exclude: [db/migrate/20230305010750_create_good_jobs.rb]
+  CountRepeatedAttributes: false
 Metrics/BlockLength: { Exclude: [config/**/*.rb, spec/**/*.rb, db/seeds/**/*.rb] }
 Metrics/MethodLength: { Exclude: [db/migrate/*.rb] }
 Rails/SkipsModelValidations: { AllowedMethods: [update_all] }

app/commands/user/sign_in_user.rb

@@ -1,8 +1,10 @@
 # frozen_string_literal: true
 
 module SignInUser
-  def self.call(submitted_password)
-    user = User.first
+  def self.call(username, submitted_password)
+    user = User.find_by(username:)
+    return unless user
+
     user_password = BCrypt::Password.new(user.password_digest)
 
     user if user_password == submitted_password

app/controllers/sessions_controller.rb

@@ -9,7 +9,7 @@ def new
 
   def create
     authorization.skip
-    user = SignInUser.call(params[:password])
+    user = SignInUser.call(params[:username], params[:password])
     if user
       session[:user_id] = user.id
 

app/views/sessions/new.erb

@@ -4,10 +4,15 @@
 
   <hr />
 
-  <%= form_with(url: "/login") do %>
-    <input class="hidden" value="username" />
+  <%= form_with(url: "/login") do |form| %>
+    <p><%= t('sessions.new.fields.unknown_username_html') %></p>
     <div class="control-group">
-      <input name="password" id="password" type="password" autofocus />
+      <%= form.text_field :username, class: "form-control", required: true %>
+      <%= form.label :username, t('sessions.new.fields.username'), class: "field-label" %>
+    </div>
+
+    <div class="control-group">
+      <input name="password" class="form-control" id="password" type="password" required />
       <i class="icon-lock field-icon"></i>
       <label id="password-label" class="field-label" for="password"><%= t('sessions.new.fields.password') %></label>
     </div>

config/locales/en.yml

@@ -137,6 +137,11 @@ en:
         logged_out_successfully: You have been signed out!
     new:
       fields:
+        unknown_username_html:
+          <strong>Note:</strong> if you didn't set a username when you set up
+          your stringer instance, then your username will be "stringer". You
+          can change this on your profile page after logging in.
+        username: Username
         password: Password
         submit: Login
       flash:

spec/commands/user/sign_in_user_spec.rb

@@ -2,15 +2,21 @@
 
 RSpec.describe SignInUser do
   it "returns the user if the password is valid" do
-    result = described_class.call(default_user.password)
+    result = described_class.call(default_user.username, default_user.password)
 
     expect(result).to eq(default_user)
   end
 
   it "returns nil if password is invalid" do
     create(:user)
 
-    result = described_class.call("not-the-pw")
+    result = described_class.call(default_user.username, "not-the-pw")
+
+    expect(result).to be_nil
+  end
+
+  it "returns nil if the user does not exist" do
+    result = described_class.call("not-the-username", "not-the-pw")
 
     expect(result).to be_nil
   end

spec/requests/sessions_controller_spec.rb

@@ -13,33 +13,38 @@
 
   describe "#create" do
     it "denies access when password is incorrect" do
-      create(:user)
-      post "/login", params: { password: "not-the-password" }
+      user = create(:user)
+      params = { username: user.username, password: "not-the-password" }
+
+      post("/login", params:)
 
       expect(rendered).to have_selector(".error")
     end
 
     it "allows access when password is correct" do
       user = default_user
+      params = { username: user.username, password: user.password }
 
-      post "/login", params: { password: user.password }
+      post("/login", params:)
 
       expect(session[:user_id]).to eq(user.id)
     end
 
     it "redirects to the root page" do
       user = default_user
+      params = { username: user.username, password: user.password }
 
-      post "/login", params: { password: user.password }
+      post("/login", params:)
 
       expect(URI.parse(response.location).path).to eq("/")
     end
 
     it "redirects to the previous path when present" do
       user = default_user
-
+      params = { username: user.username, password: user.password }
       get("/archive")
-      post("/login", params: { password: user.password })
+
+      post("/login", params:)
 
       expect(URI.parse(response.location).path).to eq("/archive")
     end

spec/support/request_helpers.rb

@@ -2,7 +2,7 @@
 
 module RequestHelpers
   def login_as(user)
-    post("/login", params: { password: user.password })
+    post("/login", params: { username: user.username, password: user.password })
   end
 
   def rendered

spec/support/system_helpers.rb

@@ -3,6 +3,7 @@
 module SystemHelpers
   def login_as(user)
     visit(login_path)
+    fill_in("Username", with: user.username)
     fill_in("Password", with: user.password)
     click_button("Login")
   end