Fix #1222: allow to issue non-expiring token

NEWS.md

@@ -7,6 +7,8 @@ User-visible changes worth mentioning.
 
 ## master
 
+- [#1225] Allow to explicitly set non-expiring tokens in `custom_access_token_expires_in` configuration
+  option using `Float::INIFINITY` return value.
 - [#1223] Update Hound/Rubocop rules, correct Doorkeeper codebase to follow style-guides.
 
 ## 5.1.0.rc2

lib/doorkeeper/config.rb

@@ -239,7 +239,6 @@ def option(name, options = {})
         Builder.instance_eval do
           remove_method name if method_defined?(name)
           define_method name do |*args, &block|
-            # TODO: is builder_class option being used?
             value = if attribute_builder
                       attribute_builder.new(&block).build
                     else
@@ -464,6 +463,10 @@ def token_grant_types
       @token_grant_types ||= calculate_token_grant_types.freeze
     end
 
+    def option_defined?(name)
+      !instance_variable_get("@#{name}").nil?
+    end
+
     private
 
     # Helper to read boolearized configuration option

lib/doorkeeper/oauth/authorization/token.rb

@@ -23,11 +23,14 @@ def build_context(pre_auth_or_oauth_client, grant_type, scopes)
             )
           end
 
-          def access_token_expires_in(server, context)
-            if (expiration = server.custom_access_token_expires_in.call(context))
-              expiration
+          def access_token_expires_in(configuration, context)
+            if configuration.option_defined?(:custom_access_token_expires_in)
+              expiration = configuration.custom_access_token_expires_in.call(context)
+              return nil if expiration == Float::INFINITY
+
+              expiration || configuration.access_token_expires_in
             else
-              server.access_token_expires_in
+              configuration.access_token_expires_in
             end
           end
 

lib/generators/doorkeeper/templates/initializer.rb

@@ -47,7 +47,12 @@
   # access_token_expires_in 2.hours
 
   # Assign custom TTL for access tokens. Will be used instead of access_token_expires_in
-  # option if defined. `context` has the following properties available
+  # option if defined. In case the block returns `nil` value Doorkeeper fallbacks to
+  # `access_token_expires_in` configuration option value. If you really need to issue a
+  # non-expiring access token (which is not recommended) then you need to return
+  # Float::INFINITY from this block.
+  #
+  # `context` has the following properties available:
   #
   # `client` - the OAuth client application (see Doorkeeper::OAuth::Client)
   # `grant_type` - the grant type of the request (see Doorkeeper::OAuth)

spec/controllers/authorizations_controller_spec.rb

@@ -34,11 +34,14 @@ def translated_error_message(key)
   let(:access_token)  { FactoryBot.build :access_token, resource_owner_id: user.id, application_id: client.id }
 
   before do
+    Doorkeeper.configure do
+      custom_access_token_expires_in(lambda do |context|
+        context.grant_type == Doorkeeper::OAuth::IMPLICIT ? 1234 : nil
+      end)
+    end
+
     allow(Doorkeeper.configuration).to receive(:grant_flows).and_return(["implicit"])
     allow(controller).to receive(:current_resource_owner).and_return(user)
-    allow(Doorkeeper.configuration).to receive(:custom_access_token_expires_in).and_return(proc { |context|
-      context.grant_type == Doorkeeper::OAuth::IMPLICIT ? 1234 : nil
-    })
   end
 
   describe "POST #create" do

spec/lib/oauth/authorization_code_request_spec.rb

@@ -18,6 +18,10 @@ module Doorkeeper::OAuth
     let(:redirect_uri) { client.redirect_uri }
     let(:params) { { redirect_uri: redirect_uri } }
 
+    before do
+      allow(server).to receive(:option_defined?).with(:custom_access_token_expires_in).and_return(true)
+    end
+
     subject do
       AuthorizationCodeRequest.new server, grant, client, params
     end

spec/lib/oauth/base_request_spec.rb

@@ -26,6 +26,10 @@ module Doorkeeper::OAuth
              refresh_token_enabled?: false
     end
 
+    before do
+      allow(server).to receive(:option_defined?).with(:custom_access_token_expires_in).and_return(true)
+    end
+
     subject do
       BaseRequest.new
     end
@@ -113,6 +117,9 @@ module Doorkeeper::OAuth
                         access_token_expires_in: 100,
                         custom_access_token_expires_in: ->(context) { context.scopes == "public" ? 500 : nil },
                         refresh_token_enabled?: false)
+
+        allow(server).to receive(:option_defined?).with(:custom_access_token_expires_in).and_return(true)
+
         result = subject.find_or_create_access_token(
           client,
           "1",
@@ -129,6 +136,9 @@ module Doorkeeper::OAuth
                         refresh_token_enabled?: lambda { |context|
                           context.scopes == "public"
                         })
+
+        allow(server).to receive(:option_defined?).with(:custom_access_token_expires_in).and_return(true)
+
         result = subject.find_or_create_access_token(
           client,
           "1",

spec/lib/oauth/client_credentials/issuer_spec.rb

@@ -4,16 +4,19 @@
 
 class Doorkeeper::OAuth::ClientCredentialsRequest
   describe Issuer do
-    let(:creator) { double :acces_token_creator }
+    let(:creator) { double :access_token_creator }
     let(:server) do
       double(
         :server,
-        access_token_expires_in: 100,
-        custom_access_token_expires_in: ->(_context) { nil }
+        access_token_expires_in: 100
       )
     end
     let(:validation) { double :validation, valid?: true }
 
+    before do
+      allow(server).to receive(:option_defined?).with(:custom_access_token_expires_in).and_return(false)
+    end
+
     subject { Issuer.new(server, validation) }
 
     describe :create do
@@ -80,6 +83,10 @@ class Doorkeeper::OAuth::ClientCredentialsRequest
           )
         end
 
+        before do
+          allow(server).to receive(:option_defined?).with(:custom_access_token_expires_in).and_return(true)
+        end
+
         it "respects grant based rules" do
           expect(creator).to receive(:call).with(
             client,

spec/lib/oauth/client_credentials_request_spec.rb

@@ -16,6 +16,10 @@ module Doorkeeper::OAuth
     let(:client)        { double :client, application: application }
     let(:token_creator) { double :issuer, create: true, token: double }
 
+    before do
+      allow(server).to receive(:option_defined?).with(:custom_access_token_expires_in).and_return(true)
+    end
+
     subject { ClientCredentialsRequest.new(server, client) }
 
     before do

spec/lib/oauth/password_access_token_request_spec.rb

@@ -18,6 +18,10 @@ module Doorkeeper::OAuth
     let(:client) { FactoryBot.create(:application) }
     let(:owner)  { double :owner, id: 99 }
 
+    before do
+      allow(server).to receive(:option_defined?).with(:custom_access_token_expires_in).and_return(true)
+    end
+
     subject do
       PasswordAccessTokenRequest.new(server, client, owner)
     end
@@ -149,11 +153,21 @@ module Doorkeeper::OAuth
           access_token_expires_in: 2.hours,
           refresh_token_enabled?: false,
           custom_access_token_expires_in: lambda { |context|
-            context.scopes.exists?("public") ? 222 : nil
+            if context.scopes.exists?("public")
+              222
+            elsif context.scopes.exists?("magic")
+              Float::INFINITY
+            else
+              nil
+            end
           }
         )
       end
 
+      before do
+        allow(server).to receive(:option_defined?).with(:custom_access_token_expires_in).and_return(true)
+      end
+
       it "checks scopes" do
         subject = PasswordAccessTokenRequest.new(server, client, owner, scope: "public")
         allow(server).to receive(:scopes).and_return(Doorkeeper::OAuth::Scopes.from_string("public"))

spec/lib/oauth/refresh_token_request_spec.rb

@@ -4,14 +4,9 @@
 
 module Doorkeeper::OAuth
   describe RefreshTokenRequest do
-    before do
-      allow(Doorkeeper::AccessToken).to receive(:refresh_token_revoked_on_use?).and_return(false)
-    end
-
     let(:server) do
       double :server,
-             access_token_expires_in: 2.minutes,
-             custom_access_token_expires_in: ->(_context) { nil }
+             access_token_expires_in: 2.minutes
     end
 
     let(:refresh_token) do
@@ -21,6 +16,11 @@ module Doorkeeper::OAuth
     let(:client) { refresh_token.application }
     let(:credentials) { Client::Credentials.new(client.uid, client.secret) }
 
+    before do
+      allow(Doorkeeper::AccessToken).to receive(:refresh_token_revoked_on_use?).and_return(false)
+      allow(server).to receive(:option_defined?).with(:custom_access_token_expires_in).and_return(false)
+    end
+
     subject { RefreshTokenRequest.new server, refresh_token, credentials }
 
     it "issues a new token for the client" do
@@ -36,6 +36,7 @@ module Doorkeeper::OAuth
                         context.grant_type == Doorkeeper::OAuth::REFRESH_TOKEN ? 1234 : nil
                       }
 
+      allow(server).to receive(:option_defined?).with(:custom_access_token_expires_in).and_return(true)
       allow(Doorkeeper::AccessToken).to receive(:refresh_token_revoked_on_use?).and_return(false)
 
       RefreshTokenRequest.new(server, refresh_token, credentials).authorize

spec/lib/oauth/token_request_spec.rb

@@ -48,20 +48,58 @@ module Doorkeeper::OAuth
       expect(subject.authorize).to be_a(ErrorResponse)
     end
 
-    context "with custom expirations" do
-      before do
-        Doorkeeper.configure do
-          orm DOORKEEPER_ORM
-          custom_access_token_expires_in do |context|
-            context.grant_type == Doorkeeper::OAuth::IMPLICIT ? 1234 : nil
+    describe "with custom expiration" do
+      context "when proper TTL returned" do
+        before do
+          Doorkeeper.configure do
+            orm DOORKEEPER_ORM
+            custom_access_token_expires_in do |context|
+              context.grant_type == Doorkeeper::OAuth::IMPLICIT ? 1234 : nil
+            end
           end
         end
+
+        it "should use the custom ttl" do
+          subject.authorize
+          token = Doorkeeper::AccessToken.first
+          expect(token.expires_in).to eq(1234)
+        end
       end
 
-      it "should use the custom ttl" do
-        subject.authorize
-        token = Doorkeeper::AccessToken.first
-        expect(token.expires_in).to eq(1234)
+      context "when nil TTL returned" do
+        before do
+          Doorkeeper.configure do
+            orm DOORKEEPER_ORM
+            access_token_expires_in 654
+            custom_access_token_expires_in do |_context|
+              nil
+            end
+          end
+        end
+
+        it "should fallback to access_token_expires_in" do
+          subject.authorize
+          token = Doorkeeper::AccessToken.first
+          expect(token.expires_in).to eq(654)
+        end
+      end
+
+      context "when infinite TTL returned" do
+        before do
+          Doorkeeper.configure do
+            orm DOORKEEPER_ORM
+            access_token_expires_in 654
+            custom_access_token_expires_in do |_context|
+              Float::INFINITY
+            end
+          end
+        end
+
+        it "should fallback to access_token_expires_in" do
+          subject.authorize
+          token = Doorkeeper::AccessToken.first
+          expect(token.expires_in).to be_nil
+        end
       end
     end