From b1c803a70146be9e7cfcb1f910599ffd69589d24 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miquel=20Sabat=C3=A9=20Sol=C3=A0?= <msabate@suse.com>
Date: Mon, 23 Jul 2018 12:52:20 +0200
Subject: [PATCH] user: do not allow the update of the portus user
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The only exception is on the password, that may change depending on
whether the given secret changed or not. For this case, Portus will
always update the password of the portus user on start.

Fixes #1878

Signed-off-by: Miquel Sabaté Solà <msabate@suse.com>
---
 app/models/user.rb                  | 16 +++++++++++++---
 config/initializers/portus_user.rb  | 20 ++++++++++++++++++++
 spec/api/grape_api/v1/users_spec.rb |  9 +++++++++
 3 files changed, 42 insertions(+), 3 deletions(-)
 create mode 100644 config/initializers/portus_user.rb

diff --git a/app/models/user.rb b/app/models/user.rb
index 3d2e5fc5c..ba496609e 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -60,6 +60,7 @@ class User < ActiveRecord::Base
   # Actions performed before/after create.
   validates :username, presence: true, uniqueness: true
   validate :private_namespace_and_team_available, on: :create
+  validate :portus_user_validation, on: :update
   after_create :create_personal_namespace!
 
   # Actions performed before destroy
@@ -83,6 +84,13 @@ def email_required?
     !(Portus::LDAP.enabled? && email.blank?)
   end
 
+  # Adds an error if the user to be updated is the portus one. This is a
+  # validation on update, so it can be skipped when strictly required.
+  def portus_user_validation
+    return unless portus? || portus?(username_was)
+    errors.add(:username, "cannot be updated")
+  end
+
   # It adds an error if the username clashes with either a namespace or a team.
   def private_namespace_and_team_available
     ns = Namespace.make_valid(username)
@@ -90,9 +98,11 @@ def private_namespace_and_team_available
     errors.add(:username, "'#{username}' cannot be transformed into a valid namespace name")
   end
 
-  # Returns true if the current user is the Portus user.
-  def portus?
-    username == "portus"
+  # Returns true if the current user is the Portus user. You can provide a value
+  # as an alternative to the value of `username`.
+  def portus?(field = nil)
+    f = field.nil? ? username : field
+    f == "portus"
   end
 
   # Returns the username to be displayed.
diff --git a/config/initializers/portus_user.rb b/config/initializers/portus_user.rb
new file mode 100644
index 000000000..7422809be
--- /dev/null
+++ b/config/initializers/portus_user.rb
@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+# This file updates the password of the portus hidden user if this
+# exists and the secret is given.
+
+portus_exists = false
+begin
+  portus_exists = User.exists?(username: "portus")
+rescue StandardError
+  # We will ignore any error and skip this initializer. This is done this way
+  # because it can get really tricky to catch all the myriad of exceptions that
+  # might be raised on database errors.
+  portus_exists = false
+end
+
+password = Rails.application.secrets.portus_password
+if portus_exists && password.present?
+  portus = User.find_by(username: "portus")
+  portus&.update_attribute("password", Rails.application.secrets.portus_password)
+end
diff --git a/spec/api/grape_api/v1/users_spec.rb b/spec/api/grape_api/v1/users_spec.rb
index 0e3e525c9..5a104e78d 100644
--- a/spec/api/grape_api/v1/users_spec.rb
+++ b/spec/api/grape_api/v1/users_spec.rb
@@ -141,6 +141,15 @@
         expect(response).to have_http_status(:not_found)
       end
     end
+
+    context "portus user" do
+      it "does not allow portus user to be updated" do
+        create :user, username: "portus", email: "portus@portus.com"
+        portus = User.find_by(username: "portus")
+        put "/api/v1/users/#{portus.id}", { user: user_data }, @header
+        expect(response).to have_http_status(:unprocessable_entity)
+      end
+    end
   end
 
   context "DELETE /api/v1/users/:id" do