diff --git a/NEWS.md b/NEWS.md index 958259442..72fde2d99 100644 --- a/NEWS.md +++ b/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 diff --git a/lib/doorkeeper/config.rb b/lib/doorkeeper/config.rb index 7b32c7b29..b2bf9e421 100644 --- a/lib/doorkeeper/config.rb +++ b/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 diff --git a/lib/doorkeeper/oauth/authorization/token.rb b/lib/doorkeeper/oauth/authorization/token.rb index e945f939d..b252d5543 100644 --- a/lib/doorkeeper/oauth/authorization/token.rb +++ b/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 diff --git a/lib/generators/doorkeeper/templates/initializer.rb b/lib/generators/doorkeeper/templates/initializer.rb index a50480a32..f71243b81 100644 --- a/lib/generators/doorkeeper/templates/initializer.rb +++ b/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) diff --git a/spec/controllers/authorizations_controller_spec.rb b/spec/controllers/authorizations_controller_spec.rb index c1684bb65..1350f21e5 100644 --- a/spec/controllers/authorizations_controller_spec.rb +++ b/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 diff --git a/spec/lib/oauth/authorization_code_request_spec.rb b/spec/lib/oauth/authorization_code_request_spec.rb index 42bd5ce75..7c0083a18 100644 --- a/spec/lib/oauth/authorization_code_request_spec.rb +++ b/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 diff --git a/spec/lib/oauth/base_request_spec.rb b/spec/lib/oauth/base_request_spec.rb index 1d71218cf..a3f7ba0e3 100644 --- a/spec/lib/oauth/base_request_spec.rb +++ b/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", diff --git a/spec/lib/oauth/client_credentials/issuer_spec.rb b/spec/lib/oauth/client_credentials/issuer_spec.rb index 497cadf8a..b77f852a1 100644 --- a/spec/lib/oauth/client_credentials/issuer_spec.rb +++ b/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, diff --git a/spec/lib/oauth/client_credentials_request_spec.rb b/spec/lib/oauth/client_credentials_request_spec.rb index 02a845aaf..3a6573227 100644 --- a/spec/lib/oauth/client_credentials_request_spec.rb +++ b/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 diff --git a/spec/lib/oauth/password_access_token_request_spec.rb b/spec/lib/oauth/password_access_token_request_spec.rb index f31fa9904..6c3b62dfe 100644 --- a/spec/lib/oauth/password_access_token_request_spec.rb +++ b/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")) diff --git a/spec/lib/oauth/refresh_token_request_spec.rb b/spec/lib/oauth/refresh_token_request_spec.rb index 8e448d0ff..50f84d680 100644 --- a/spec/lib/oauth/refresh_token_request_spec.rb +++ b/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 diff --git a/spec/lib/oauth/token_request_spec.rb b/spec/lib/oauth/token_request_spec.rb index 54da168a0..e13239918 100644 --- a/spec/lib/oauth/token_request_spec.rb +++ b/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