Remove redundant error flash. I don't a reason, why it is needed.

Currently redirect_uri field of OAuth applications (Doorkeeper
Applications) did only check for 'localhost', which is not
complete. Other loopback URLs are also valid, such as
'http://127.0.0.1'.

Now, it is coherent with the allowed hosts of ::Storages::Storage

app/controllers/oauth/applications_controller.rb

@@ -58,7 +58,6 @@ module OAuth
         redirect_to action: :show, id: call.result.id
       else
         @errors = call.errors
-        flash[:error] = call.errors.full_messages.join('\n')
         render action: :new
       end
     end

app/validators/secure_context_uri_validator.rb

@@ -0,0 +1,69 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2022 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+# Please see https://w3c.github.io/webappsec-secure-contexts/
+# for a definition of "secure contexts".
+# Basically, a host has to have either a HTTPS scheme or be
+# localhost to provide a secure context.
+class SecureContextUriValidator < ActiveModel::EachValidator
+  def validate_each(contract, attribute, value)
+    begin
+      uri = URI.parse(value)
+    rescue StandardError
+      contract.errors.add(attribute, :invalid_url)
+      return
+    end
+
+    # The URI could be parsable but not contain a host name
+    if uri.host.nil?
+      contract.errors.add(attribute, :invalid_url)
+      return
+    end
+
+    unless secure_context_uri?(uri)
+      contract.errors.add(attribute, :url_not_secure_context)
+    end
+  end
+
+  def self.secure_context_uri?(uri)
+    return true if uri.scheme == 'https' # https is always safe
+    return true if uri.host == 'localhost' # Simple localhost
+    return true if uri.host =~ /\.localhost\.?$/ # i.e. 'foo.localhost' or 'foo.localhost.'
+
+    # Check for loopback interface. The constructor can throw an exception for non IP addresses.
+    # Those are invalid. And if the host is an IP address then we can check if it is loopback.
+    begin
+      return true if IPAddr.new(uri.host).loopback?
+    rescue StandardError
+      return false
+    end
+
+    # uri.host is an IP but not a loopback
+    false
+  end
+end

config/initializers/doorkeeper.rb

@@ -124,7 +124,7 @@ Doorkeeper.configure do
   #
   # force_ssl_in_redirect_uri !Rails.env.development?
   #
-  force_ssl_in_redirect_uri { |uri| uri.host != 'localhost' }
+  force_ssl_in_redirect_uri { |uri| !SecureContextUriValidator.secure_context_uri?(uri) }
 
   # Specify what redirect URI's you want to block during Application creation.
   # Any redirect URI is whitelisted by default.

config/locales/en.yml

@@ -672,6 +672,8 @@ en:
         unknown_property: "is not a known property."
         unknown_property_nested: "has the unknown path '%{path}'."
         unremovable: "cannot be removed."
+        url_not_secure_context: >
+          is not providing a "Secure Context". Either use HTTPS or a loopback address, such as localhost.
         wrong_length: "is the wrong length (should be %{count} characters)."
       models:
         attachment:
@@ -703,7 +705,7 @@ en:
               fragment_present: 'cannot contain a fragment.'
               invalid_uri: 'must be a valid URI.'
               relative_uri: 'must be an absolute URI.'
-              secured_uri: 'must be an HTTPS/SSL URI.'
+              secured_uri: 'is not providing a "Secure Context". Either use HTTPS or a loopback address, such as localhost.'
               forbidden_uri: 'is forbidden by the server.'
             scopes:
               not_match_configured: "doesn't match available scopes."

modules/storages/app/contracts/storages/storages/base_contract.rb

@@ -35,7 +35,7 @@ require 'uri'
 # (normally it's a model).
 module Storages::Storages
   class BaseContract < ::ModelContract
-    MINIMAL_NEXTCLOUD_VERSION = 23
+    MINIMAL_NEXTCLOUD_VERSION = 22
 
     include ::Storages::Storages::Concerns::ManageStoragesGuarded
     include ActiveModel::Validations
@@ -50,6 +50,6 @@ module Storages::Storages
     validates :host, url: true, length: { maximum: 255 }
     # Check that a host actually is a storage server.
     # But only do so if the validations above for URL were successful.
-    validates :host, nextcloud_compatible_host: true, unless: -> { errors.include?(:host) }
+    validates :host, secure_context_uri: true, nextcloud_compatible_host: true, unless: -> { errors.include?(:host) }
   end
 end

modules/storages/spec/contracts/storages/storages/shared_contract_examples.rb

@@ -116,6 +116,18 @@ shared_examples_for 'storage contract', :storage_server_helpers, webmock: true d
         include_examples 'contract is invalid', host: :url
       end
 
+      context 'when host is an unsafe IP' do
+        let(:storage_host) { 'http://172.16.193.146' }
+
+        include_examples 'contract is invalid', host: :url_not_secure_context
+      end
+
+      context 'when host is an unsafe hostname' do
+        let(:storage_host) { 'http://nc.openproject.com' }
+
+        include_examples 'contract is invalid', host: :url_not_secure_context
+      end
+
       context 'when provider_type is nextcloud' do
         let(:capabilities_response_body) { nil } # use default
         let(:capabilities_response_code) { nil } # use default
@@ -195,5 +207,19 @@ shared_examples_for 'storage contract', :storage_server_helpers, webmock: true d
         end
       end
     end
+
+    context 'when host secure' do
+      context 'when host is localhost' do
+        let(:storage_host) { 'http://localhost:1234' }
+
+        include_examples 'contract is valid'
+      end
+
+      context 'when host uses https protocol' do
+        let(:storage_host) { 'https://172.16.193.146' }
+
+        include_examples 'contract is valid'
+      end
+    end
   end
 end

spec/features/admin/oauth/oauth_applications_management_spec.rb

@@ -51,6 +51,14 @@ describe 'OAuth applications management', type: :feature, js: true do
 
     expect(page).to have_selector('.errorExplanation', text: 'Redirect URI must be an absolute URI.')
 
+    SeleniumHubWaiter.wait
+    fill_in('application_redirect_uri', with: "")
+    # Fill rediret_uri which does not provide a Secure Context
+    fill_in 'application_redirect_uri', with: "http://example.org"
+    click_on 'Create'
+
+    expect(page).to have_selector('.errorExplanation', text: 'Redirect URI is not providing a "Secure Context"')
+
     # Can create localhost without https (https://community.openproject.com/wp/34025)
     SeleniumHubWaiter.wait
     fill_in 'application_redirect_uri', with: "urn:ietf:wg:oauth:2.0:oob\nhttp://localhost/my/callback"

spec/validator/secure_context_uri_validator_spec.rb

@@ -0,0 +1,134 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2022 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+# require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper')
+
+require 'spec_helper'
+
+describe SecureContextUriValidator do
+  let(:host) { nil }
+  let(:model_class) do
+    Class.new do
+      include ActiveModel::Validations
+      def self.model_name
+        ActiveModel::Name.new(self, nil, "ValidatedModel")
+      end
+
+      attr_accessor :host
+
+      validates :host, secure_context_uri: true
+    end
+  end
+  let(:model_instance) { model_class.new.tap { |instance| instance.host = host } }
+
+  before { model_instance.validate }
+
+  context 'with empty URI' do
+    ['', ' ', nil].each do |uri|
+      describe "when URI is '#{uri}'" do
+        let(:host) { uri }
+
+        it "adds an :could_not_parse_host_uri error" do
+          expect(model_instance.errors).to include(:host)
+          expect(model_instance.errors.first.type).to be :could_not_parse_host_uri
+        end
+      end
+    end
+  end
+
+  context 'with invalid URI' do
+    %w(some_string http://<>ample.com).each do |uri|
+      describe "when URI is '#{uri}'" do
+        let(:host) { uri }
+
+        it "adds an :could_not_parse_host_uri error" do
+          expect(model_instance.errors).to include(:host)
+          expect(model_instance.errors.first.type).to be :could_not_parse_host_uri
+        end
+      end
+    end
+  end
+
+  context 'with valid URI' do
+    context 'when host is missing' do
+      let(:host) { 'https://' }
+
+      it "adds an :could_not_parse_host_uri error" do
+        expect(model_instance.errors).to include(:host)
+        expect(model_instance.errors.first.type).to be :could_not_parse_host_uri
+      end
+    end
+
+    context 'when not providing a Secure Context' do
+      %w{http://128.0.0.1 http://foo.com http://[::2]}.each do |uri|
+        describe "when URI is '#{uri}'" do
+          let(:host) { uri }
+
+          it "adds a :url_not_secure_context error" do
+            expect(model_instance.errors).to include(:host)
+            expect(model_instance.errors.first.type).to be :url_not_secure_context
+          end
+        end
+      end
+    end
+
+    context 'when providing a Secure Context' do
+      context 'with a loopback IP' do
+        %w{http://127.0.0.1 http://127.1.1.1}.each do |uri|
+          describe "when URI is '#{uri}'" do
+            let(:host) { uri }
+
+            it "does not add an error" do
+              expect(model_instance.errors).not_to include(:host)
+            end
+          end
+        end
+      end
+
+      context 'with a domain name' do
+        %w(https://example.com http://localhost http://.localhost http://foo.localhost. http://foo.localhost).each do |uri|
+          describe "when URI is '#{uri}'" do
+            let(:host) { uri }
+
+            it "does not add an error" do
+              expect(model_instance.errors).not_to include(:host)
+            end
+          end
+        end
+      end
+
+      context 'with IPV6 loopback URI' do
+        let(:host) { 'http://[::1]' }
+
+        it "does not add an error" do
+          expect(model_instance.errors).not_to include(:host)
+        end
+      end
+    end
+  end
+end