Use same Secure Context validation for OAuth applications

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/validators/secure_context_uri_validator.rb

@@ -35,24 +35,22 @@ class SecureContextUriValidator < ActiveModel::EachValidator
     begin
       uri = URI.parse(value)
     rescue StandardError
-      contract.errors.add(attribute, :could_not_parse_host_uri)
+      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, :could_not_parse_host_uri)
+      contract.errors.add(attribute, :invalid_url)
       return
     end
 
-    unless secure_context_uri?(uri)
-      contract.errors.add(attribute, :uri_not_secure_context)
+    unless self.class.secure_context_uri?(uri)
+      contract.errors.add(attribute, :url_not_secure_context)
     end
   end
 
-  private
-
-  def secure_context_uri?(uri)
+  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.'

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/config/locales/en.yml

@@ -30,8 +30,6 @@ en:
                 is not fully set up. That Nextcloud instance still redirects to a URL containing 'index.php'. That is a 
                 strong indicator that the server does not have mod_rewrite, mod_headers and/or mod_env fully configured,
                 which is necessary for a Bearer token based authorization of API requests.
-              could_not_parse_host_uri: "could not parse the host URI"
-              uri_not_secure_context: "is not a secure context (HTTPS or localhost)"
         storages/file_link:
           attributes:
             origin_id:

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

@@ -119,13 +119,13 @@ shared_examples_for 'storage contract', :storage_server_helpers, webmock: true d
       context 'when host is an unsafe IP' do
         let(:storage_host) { 'http://172.16.193.146' }
 
-        include_examples 'contract is invalid', host: :uri_not_secure_context
+        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: :uri_not_secure_context
+        include_examples 'contract is invalid', host: :url_not_secure_context
       end
 
       context 'when provider_type is nextcloud' do

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

@@ -53,9 +53,9 @@ describe SecureContextUriValidator do
       describe "when URI is '#{uri}'" do
         let(:host) { uri }
 
-        it "adds an :could_not_parse_host_uri error" do
+        it "adds an :invalid_url error" do
           expect(model_instance.errors).to include(:host)
-          expect(model_instance.errors.first.type).to be :could_not_parse_host_uri
+          expect(model_instance.errors.first.type).to be :invalid_url
         end
       end
     end
@@ -66,9 +66,9 @@ describe SecureContextUriValidator do
       describe "when URI is '#{uri}'" do
         let(:host) { uri }
 
-        it "adds an :could_not_parse_host_uri error" do
+        it "adds an :invalid_url error" do
           expect(model_instance.errors).to include(:host)
-          expect(model_instance.errors.first.type).to be :could_not_parse_host_uri
+          expect(model_instance.errors.first.type).to be :invalid_url
         end
       end
     end
@@ -78,9 +78,9 @@ describe SecureContextUriValidator do
     context 'when host is missing' do
       let(:host) { 'https://' }
 
-      it "adds an :could_not_parse_host_uri error" do
+      it "adds an :invalid_url error" do
         expect(model_instance.errors).to include(:host)
-        expect(model_instance.errors.first.type).to be :could_not_parse_host_uri
+        expect(model_instance.errors.first.type).to be :invalid_url
       end
     end
 
@@ -89,9 +89,9 @@ describe SecureContextUriValidator do
         describe "when URI is '#{uri}'" do
           let(:host) { uri }
 
-          it "adds a :uri_not_secure_context error" do
+          it "adds a :url_not_secure_context error" do
             expect(model_instance.errors).to include(:host)
-            expect(model_instance.errors.first.type).to be :uri_not_secure_context
+            expect(model_instance.errors.first.type).to be :url_not_secure_context
           end
         end
       end