Merge pull request doorkeeper-gem#298 from digineo/master

Support for multiple redirect_uris

CHANGELOG.md

@@ -17,6 +17,7 @@
   - [#318] Include "WWW-Authenticate" header with Unauthorized responses
 - enhancements
   - [#293] Adds ActionController::Instrumentation in TokensController
+  - [#298] Support for multiple redirect_uris added.
   - [#313] `AccessToken.revoke_all_for` actually revokes all non-revoked
     tokens for an application/owner instead of deleting them.
   - [#333] Rails 4.1 support

app/controllers/doorkeeper/applications_controller.rb

@@ -51,9 +51,9 @@ def set_application
 
     def application_params
       if params.respond_to?(:permit)
-        params.require(:application).permit(:name, :redirect_uri)
+        params.require(:application).permit(:name, :redirect_uris)
       else
-        params[:application].slice(:name, :redirect_uri) rescue nil
+        params[:application].slice(:name, :redirect_uris) rescue nil
       end
     end
   end

app/validators/redirect_uri_validator.rb

@@ -6,11 +6,17 @@ def self.test_redirect_uri
   end
 
   def validate_each(record, attribute, value)
-    uri = ::URI.parse(value)
-    return if test_redirect_uri?(uri)
-    record.errors.add(attribute, :fragment_present) unless uri.fragment.nil?
-    record.errors.add(attribute, :relative_uri) if uri.scheme.nil? || uri.host.nil?
-    record.errors.add(attribute, :has_query_parameter) unless uri.query.nil?
+    if value.blank?
+      record.errors.add(attribute, :blank)
+    else
+      value.split.each do |val|
+        uri = ::URI.parse(val)
+        return if test_redirect_uri?(uri)
+        record.errors.add(attribute, :fragment_present) unless uri.fragment.nil?
+        record.errors.add(attribute, :relative_uri) if uri.scheme.nil? || uri.host.nil?
+        record.errors.add(attribute, :has_query_parameter) unless uri.query.nil?
+      end
+    end
   rescue URI::InvalidURIError
     record.errors.add(attribute, :invalid_uri)
   end

app/views/doorkeeper/applications/_form.html.erb

@@ -13,10 +13,11 @@
     </div>
 
     <div class="clearfix">
-      <%= f.label :redirect_uri %>
+      <%= f.label :redirect_uris %>
       <div class="input">
-        <%= f.text_field :redirect_uri %>
-        <%= errors_for application, :redirect_uri %>
+        <%= f.text_area :redirect_uris %>
+        <%= errors_for application, :redirect_uris %>
+        <span class="help-inline">Please use one line per URI.</span>
         <% if Doorkeeper.configuration.test_redirect_uri %>
           <span class="help-inline">Use <%= Doorkeeper.configuration.test_redirect_uri %> for local tests</span>
         <% end %>

app/views/doorkeeper/applications/index.html.erb

@@ -23,7 +23,7 @@ input[type=submit] {
       <% @applications.each do |application| %>
         <tr id="application_<%= application.id %>">
           <td><%= link_to application.name, [:oauth, application] %></td>
-          <td><%= application.redirect_uri %></td>
+          <td><%= application.redirect_uris %></td>
           <td><%= link_to 'Edit', edit_oauth_application_path(application) %></td>
           <td><%= render 'delete_form', application: application %></td>
         </tr>

app/views/doorkeeper/applications/show.html.erb

@@ -5,8 +5,10 @@
 </div>
 
 <div class="span10">
-  <h4>Callback url:</h4>
-  <p><code id="callback_url"><%= @application.redirect_uri %></code></p>
+  <h4>Callback urls:</h4>
+  <p id="callback_url">
+    <% @application.redirect_uris.split.each do |uri| %><code><%= uri %></code> <% end %>
+  </p>
 
   <h4>Application Id:</h4>
   <p><code id="application_id"><%= @application.uid %></code></p>
@@ -15,7 +17,7 @@
   <p><code id="secret"><%= @application.secret %></code></p>
 
   <h4>Link to authorization code:</h4>
-  <p><%= link_to 'Authorize', oauth_authorization_path(:client_id => @application.uid, :redirect_uri => @application.redirect_uri, :response_type => 'code' ) %></p>
+  <p><%= link_to 'Authorize', oauth_authorization_path(:client_id => @application.uid, :redirect_uri => @application.redirect_uris.split.first, :response_type => 'code' ) %></p>
 </div>
 
 <div class="span6">

config/locales/en.yml

@@ -4,7 +4,7 @@ en:
       models:
         application:
           attributes:
-            redirect_uri:
+            redirect_uris:
               fragment_present: 'cannot contain a fragment.'
               has_query_parameter: 'cannot contain a query parameter.'
               invalid_uri: 'must be a valid URI.'
@@ -14,7 +14,7 @@ en:
       models:
         application:
           attributes:
-            redirect_uri:
+            redirect_uris:
               fragment_present: 'cannot contain a fragment.'
               has_query_parameter: 'cannot contain a query parameter.'
               invalid_uri: 'must be a valid URI.'
@@ -24,7 +24,7 @@ en:
       models:
         application:
           attributes:
-            redirect_uri:
+            redirect_uris:
               fragment_present: 'cannot contain a fragment.'
               has_query_parameter: 'cannot contain a query parameter.'
               invalid_uri: 'must be a valid URI.'

lib/doorkeeper/models/application.rb

@@ -5,14 +5,14 @@ class Application
     has_many :access_grants, :dependent => :destroy, :class_name => "Doorkeeper::AccessGrant"
     has_many :access_tokens, :dependent => :destroy, :class_name => "Doorkeeper::AccessToken"
 
-    validates :name, :secret, :uid, :redirect_uri, :presence => true
+    validates :name, :secret, :uid, :redirect_uris, :presence => true
     validates :uid, :uniqueness => true
-    validates :redirect_uri, :redirect_uri => true
+    validates :redirect_uris, :redirect_uri => true
 
     before_validation :generate_uid, :generate_secret, :on => :create
 
     if ::Rails.version.to_i < 4 || defined?(ProtectedAttributes)
-      attr_accessible :name, :redirect_uri
+      attr_accessible :name, :redirect_uris
     end
 
     def self.model_name

lib/doorkeeper/models/mongo_mapper/application.rb

@@ -8,11 +8,11 @@ class Application
 
     many :authorized_tokens, :class_name => "Doorkeeper::AccessToken"
 
-    key :name,         String
-    key :uid,          String
-    key :secret,       String
-    key :redirect_uri, String
-    key :scopes,       String
+    key :name,          String
+    key :uid,           String
+    key :secret,        String
+    key :redirect_uris, String
+    key :scopes,        String
 
     def scopes=(value)
       write_attribute :scopes, value if value.present?

lib/doorkeeper/models/mongoid2/application.rb

@@ -8,7 +8,7 @@ class Application
     field :name, :type => String
     field :uid, :type => String
     field :secret, :type => String
-    field :redirect_uri, :type => String
+    field :redirect_uris, :type => String
 
     index :uid, :unique => true
 

lib/doorkeeper/models/mongoid3/application.rb

@@ -8,7 +8,7 @@ class Application
     field :name, :type => String
     field :uid, :type => String
     field :secret, :type => String
-    field :redirect_uri, :type => String
+    field :redirect_uris, :type => String
 
     index({ uid: 1 }, { unique: true })
 

lib/doorkeeper/oauth/client.rb

@@ -17,7 +17,7 @@ def self.authenticate(credentials, method = Doorkeeper::Application.method(:auth
         end
       end
 
-      delegate :id, :name, :uid, :redirect_uri, :to => :@application
+      delegate :id, :name, :uid, :redirect_uris, :to => :@application
 
       def initialize(application)
         @application = application

lib/doorkeeper/oauth/helpers/uri_checker.rb

@@ -16,7 +16,7 @@ def self.matches?(url, client_url)
         end
 
         def self.valid_for_authorization?(url, client_url)
-          valid?(url) && matches?(url, client_url)
+          valid?(url) && client_url.split.any?{|other_url| matches?(url, other_url) }
         end
 
         def self.as_uri(url)

lib/doorkeeper/oauth/pre_authorization.rb

@@ -55,7 +55,7 @@ def validate_scopes
       def validate_redirect_uri
         return false unless redirect_uri.present?
         Helpers::URIChecker.test_uri?(redirect_uri) ||
-        Helpers::URIChecker.valid_for_authorization?(redirect_uri, client.redirect_uri)
+        Helpers::URIChecker.valid_for_authorization?(redirect_uri, client.redirect_uris)
       end
     end
   end

lib/generators/doorkeeper/templates/migration.rb

@@ -1,10 +1,10 @@
 class CreateDoorkeeperTables < ActiveRecord::Migration
   def change
     create_table :oauth_applications do |t|
-      t.string  :name,         :null => false
-      t.string  :uid,          :null => false
-      t.string  :secret,       :null => false
-      t.string  :redirect_uri, :null => false, :limit => 2048
+      t.string  :name,          :null => false
+      t.string  :uid,           :null => false
+      t.string  :secret,        :null => false
+      t.text    :redirect_uris, :null => false
       t.timestamps
     end
 

spec/controllers/applications_controller_spec.rb

@@ -18,7 +18,7 @@ module Doorkeeper
         expect do
           post :create, application: {
             name: 'Example',
-            redirect_uri: 'http://example.com' }
+            redirect_uris: 'http://example.com' }
         end.to_not change { Doorkeeper::Application.count }
       end
     end
@@ -32,7 +32,7 @@ module Doorkeeper
         expect do
           post :create, application: {
             name: 'Example',
-            redirect_uri: 'http://example.com' }
+            redirect_uris: 'http://example.com' }
         end.to change { Doorkeeper::Application.count }.by(1)
         expect(response).to be_redirect
       end
@@ -50,7 +50,7 @@ module Doorkeeper
         application = FactoryGirl.create(:application)
         put :update, id: application.id, application: {
           name: 'Example',
-          redirect_uri: 'http://example.com' }
+          redirect_uris: 'http://example.com' }
         application.reload.name.should eq 'Example'
       end
     end

spec/controllers/authorizations_controller_spec.rb

@@ -21,15 +21,15 @@ def translated_error_message(key)
 
   describe "POST #create" do
     before do
-      post :create, :client_id => client.uid, :response_type => "token", :redirect_uri => client.redirect_uri
+      post :create, :client_id => client.uid, :response_type => "token", :redirect_uri => client.redirect_uris
     end
 
     it "redirects after authorization" do
       expect(response).to be_redirect
     end
 
     it "redirects to client redirect uri" do
-      expect(response.location).to match(%r[^#{client.redirect_uri}])
+      expect(response.location).to match(%r[^#{client.redirect_uris}])
     end
 
     it "includes access token in fragment" do
@@ -56,15 +56,15 @@ def translated_error_message(key)
   describe "POST #create with errors" do
     before do
       default_scopes_exist :public
-      post :create, :client_id => client.uid, :response_type => "token", :scope => "invalid", :redirect_uri => client.redirect_uri
+      post :create, :client_id => client.uid, :response_type => "token", :scope => "invalid", :redirect_uri => client.redirect_uris
     end
 
     it "redirects after authorization" do
       expect(response).to be_redirect
     end
 
     it "redirects to client redirect uri" do
-      expect(response.location).to match(%r[^#{client.redirect_uri}])
+      expect(response.location).to match(%r[^#{client.redirect_uris}])
     end
 
     it "does not include access token in fragment" do
@@ -90,7 +90,7 @@ def translated_error_message(key)
 
   describe "GET #new" do
     before do
-      get :new, :client_id => client.uid, :response_type => "token", :redirect_uri => client.redirect_uri
+      get :new, :client_id => client.uid, :response_type => "token", :redirect_uri => client.redirect_uris
     end
 
     it 'renders new template' do
@@ -103,12 +103,12 @@ def translated_error_message(key)
       Doorkeeper.configuration.stub(:skip_authorization => proc do
           true
         end)
-      get :new, :client_id => client.uid, :response_type => "token", :redirect_uri => client.redirect_uri
+      get :new, :client_id => client.uid, :response_type => "token", :redirect_uri => client.redirect_uris
     end
 
     it "should redirect immediately" do
       response.should be_redirect
-      response.location.should =~ %r[^#{client.redirect_uri}]
+      response.location.should =~ %r[^#{client.redirect_uris}]
     end
 
     it "should issue a token" do

spec/dummy/db/migrate/20131022151523_pluralize_redirect_uri_in_application.rb

@@ -0,0 +1,6 @@
+class PluralizeRedirectUriInApplication < ActiveRecord::Migration
+  def change
+    rename_column :oauth_applications, :redirect_uri, :redirect_uris
+    change_column :oauth_applications, :redirect_uris, :text, :null => false, :limit => nil
+  end
+end

spec/dummy/db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended to check this file into your version control system.
 
-ActiveRecord::Schema.define(:version => 20130902175349) do
+ActiveRecord::Schema.define(:version => 20131022151523) do
 
   create_table "oauth_access_grants", :force => true do |t|
     t.integer  "resource_owner_id",                 :null => false
@@ -42,12 +42,12 @@
   add_index "oauth_access_tokens", ["token"], :name => "index_oauth_access_tokens_on_token", :unique => true
 
   create_table "oauth_applications", :force => true do |t|
-    t.string   "name",                         :null => false
-    t.string   "uid",                          :null => false
-    t.string   "secret",                       :null => false
-    t.string   "redirect_uri", :limit => 2048, :null => false
-    t.datetime "created_at",                   :null => false
-    t.datetime "updated_at",                   :null => false
+    t.string   "name",          :null => false
+    t.string   "uid",           :null => false
+    t.string   "secret",        :null => false
+    t.text     "redirect_uris", :null => false
+    t.datetime "created_at",    :null => false
+    t.datetime "updated_at",    :null => false
     t.integer  "owner_id"
     t.string   "owner_type"
   end

spec/factories/application.rb

@@ -1,6 +1,6 @@
 FactoryGirl.define do
   factory :application, :class => Doorkeeper::Application do
     sequence(:name){ |n| "Application #{n}" }
-    redirect_uri "https://app.com/callback"
+    redirect_uris "https://app.com/callback"
   end
 end

spec/lib/oauth/authorization_code_request_spec.rb

@@ -7,7 +7,7 @@ module Doorkeeper::OAuth
     let(:client) { grant.application }
 
     subject do
-      AuthorizationCodeRequest.new server, grant, client, :redirect_uri => client.redirect_uri
+      AuthorizationCodeRequest.new server, grant, client, :redirect_uri => client.redirect_uris
     end
 
     it 'issues a new token for the client' do

spec/lib/oauth/helpers/uri_checker_spec.rb

@@ -59,6 +59,24 @@ module Doorkeeper::OAuth::Helpers
         uri = client_uri = 'http://app.co/aaa'
         URIChecker.valid_for_authorization?(uri, client_uri).should be_true
       end
+
+      it "is false if valid and mismatches" do
+        uri = 'http://app.co/aaa'
+        client_uri = 'http://app.co/bbb'
+        URIChecker.valid_for_authorization?(uri, client_uri).should be_false
+      end
+
+      it "is true if valid and included in array" do
+        uri = 'http://app.co/aaa'
+        client_uri = "http://example.com/bbb\nhttp://app.co/aaa"
+        URIChecker.valid_for_authorization?(uri, client_uri).should be_true
+      end
+
+      it "is false if valid and not included in array" do
+        uri = 'http://app.co/aaa'
+        client_uri = "http://example.com/bbb\nhttp://app.co/cc"
+        URIChecker.valid_for_authorization?(uri, client_uri).should be_false
+      end
     end
   end
 end

spec/lib/oauth/pre_authorization_spec.rb

@@ -3,7 +3,7 @@
 module Doorkeeper::OAuth
   describe PreAuthorization do
     let(:server) { double :server, :default_scopes => Scopes.new, :scopes => Scopes.from_string('public') }
-    let(:client) { double :client, :redirect_uri => 'http://tst.com/auth' }
+    let(:client) { double :client, :redirect_uris => 'http://tst.com/auth' }
 
     let :attributes do
       {