Do Not Merge: v5.0.3 + fixed validation ordering

NEWS.md

@@ -5,9 +5,10 @@ upgrade guides.
 
 User-visible changes worth mentioning.
 
-## master
+## 5.0.3
 
-- [#] Add your description here.
+[#1371] Backport: Add #as_json method and attributes serialization restriction for Application model.
+        Fixes information disclosure vulnerability (CVE-2020-10187).
 
 ## 5.0.2
 

app/controllers/doorkeeper/applications_controller.rb

@@ -19,7 +19,7 @@ def index
     def show
       respond_to do |format|
         format.html
-        format.json { render json: @application }
+        format.json { render json: @application, as_owner: true }
       end
     end
 
@@ -35,7 +35,7 @@ def create
 
         respond_to do |format|
           format.html { redirect_to oauth_application_url(@application) }
-          format.json { render json: @application }
+          format.json { render json: @application, as_owner: true }
         end
       else
         respond_to do |format|
@@ -53,7 +53,7 @@ def update
 
         respond_to do |format|
           format.html { redirect_to oauth_application_url(@application) }
-          format.json { render json: @application }
+          format.json { render json: @application, as_owner: true }
         end
       else
         respond_to do |format|

app/controllers/doorkeeper/authorized_applications_controller.rb

@@ -9,7 +9,7 @@ def index
 
       respond_to do |format|
         format.html
-        format.json { render json: @applications }
+        format.json { render json: @applications, current_resource_owner: current_resource_owner }
       end
     end
 

lib/doorkeeper/oauth/error_response.rb

@@ -36,6 +36,8 @@ def status
       end
 
       def redirectable?
+        return false if @redirect_uri.blank?
+
         name != :invalid_redirect_uri && name != :invalid_client &&
           !URIChecker.native_uri?(@redirect_uri)
       end

lib/doorkeeper/orm/active_record/application.rb

@@ -45,6 +45,29 @@ def self.revoke_tokens_and_grants_for(id, resource_owner)
       AccessGrant.revoke_all_for(id, resource_owner)
     end
 
+    # Represents client as set of it's attributes in JSON format.
+    # This is the right way how we want to override ActiveRecord #to_json.
+    #
+    # Respects privacy settings and serializes minimum set of attributes
+    # for public/private clients and full set for authorized owners.
+    #
+    # @return [Hash] entity attributes for JSON
+    #
+    def as_json(options = {})
+      # if application belongs to some owner we need to check if it's the same as
+      # the one passed in the options or check if we render the client as an owner
+      if (respond_to?(:owner) && owner && owner == options[:current_resource_owner]) ||
+         options[:as_owner]
+        # Owners can see all the client attributes, fallback to ActiveModel serialization
+        super
+      else
+        # if application has no owner or it's owner doesn't match one from the options
+        # we render only minimum set of attributes that could be exposed to a public
+        only = extract_serializable_attributes(options)
+        super(options.merge(only: only))
+      end
+    end
+
     private
 
     def generate_uid
@@ -65,5 +88,37 @@ def scopes_match_configured
     def enforce_scopes?
       Doorkeeper.configuration.enforce_configured_scopes?
     end
+
+    # Helper method to extract collection of serializable attribute names
+     # considering serialization options (like `only`, `except` and so on).
+     #
+     # @param options [Hash] serialization options
+     #
+     # @return [Array<String>]
+     #   collection of attributes to be serialized using #as_json
+     #
+     def extract_serializable_attributes(options = {})
+       opts = options.try(:dup) || {}
+       only = Array.wrap(opts[:only]).map(&:to_s)
+
+       only = if only.blank?
+                serializable_attributes
+              else
+                only & serializable_attributes
+              end
+
+       only -= Array.wrap(opts[:except]).map(&:to_s) if opts.key?(:except)
+       only.uniq
+     end
+
+     # Collection of attributes that could be serialized for public.
+     # Override this method if you need additional attributes to be serialized.
+     #
+     # @return [Array<String>] collection of serializable attributes
+     def serializable_attributes
+       attributes = %w[id name created_at]
+       attributes << "uid" unless confidential?
+       attributes
+     end
   end
 end

lib/doorkeeper/version.rb

@@ -9,7 +9,7 @@ module VERSION
     # Semantic versioning
     MAJOR = 5
     MINOR = 0
-    TINY = 2
+    TINY = 3
     PRE = nil
 
     # Full version number

spec/lib/oauth/error_response_spec.rb

@@ -8,6 +8,13 @@ module Doorkeeper::OAuth
       end
     end
 
+    describe 'the redirect uri' do
+      it 'is not redirectable when redirect uri is nil' do
+        response = ErrorResponse.new(name: :some_error, state: nil)
+        expect(response.redirectable?).to eq(false)
+      end
+    end
+
     describe :from_request do
       it 'has the error from request' do
         error = ErrorResponse.from_request double(error: :some_error)