Skip to content

Commit

Permalink
Merge pull request #1102 from thatsmydoing/scope-based-expiry
Browse files Browse the repository at this point in the history
Add custom expiration time based on scopes
  • Loading branch information
nbulaj authored Jun 6, 2018
2 parents bbcd94e + 0b336a1 commit 37abb5e
Show file tree
Hide file tree
Showing 17 changed files with 132 additions and 28 deletions.
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ upgrade guides.
User-visible changes worth mentioning.

## master
- [#1102] Expiration time based on scopes
- [#1099] All the configuration variables in `Doorkeeper.configuration` now
always return a non-nil value (`true` or `false`)
- [#1099] ORM / Query optimization: Do not revoke the refresh token if it is not enabled
Expand Down
1 change: 1 addition & 0 deletions lib/doorkeeper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
require 'doorkeeper/validations'

require 'doorkeeper/oauth/authorization/code'
require 'doorkeeper/oauth/authorization/context'
require 'doorkeeper/oauth/authorization/token'
require 'doorkeeper/oauth/authorization/uri_builder'
require 'doorkeeper/oauth/helpers/scope_checker'
Expand Down
2 changes: 1 addition & 1 deletion lib/doorkeeper/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ def option(name, options = {})
default: ->(_request, _response) {}
option :skip_authorization, default: ->(_routes) {}
option :access_token_expires_in, default: 7200
option :custom_access_token_expires_in, default: ->(_app, _grant) { nil }
option :custom_access_token_expires_in, default: ->(_context) { nil }
option :authorization_code_expires_in, default: 600
option :orm, default: :active_record
option :native_redirect_uri, default: 'urn:ietf:wg:oauth:2.0:oob'
Expand Down
15 changes: 15 additions & 0 deletions lib/doorkeeper/oauth/authorization/context.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
module Doorkeeper
module OAuth
module Authorization
class Context
attr_reader :client, :grant_type, :scopes

def initialize(client, grant_type, scopes)
@client = client
@grant_type = grant_type
@scopes = scopes
end
end
end
end
end
16 changes: 11 additions & 5 deletions lib/doorkeeper/oauth/authorization/token.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ class Token
attr_accessor :pre_auth, :resource_owner, :token

class << self
def access_token_expires_in(server, pre_auth_or_oauth_client, grant_type)
if (expiration = custom_expiration(server, pre_auth_or_oauth_client, grant_type))
def access_token_expires_in(server, pre_auth_or_oauth_client, grant_type, scopes)
if (expiration = custom_expiration(server, pre_auth_or_oauth_client, grant_type, scopes))
expiration
else
server.access_token_expires_in
Expand All @@ -15,14 +15,19 @@ def access_token_expires_in(server, pre_auth_or_oauth_client, grant_type)

private

def custom_expiration(server, pre_auth_or_oauth_client, grant_type)
def custom_expiration(server, pre_auth_or_oauth_client, grant_type, scopes)
oauth_client = if pre_auth_or_oauth_client.respond_to?(:client)
pre_auth_or_oauth_client.client
else
pre_auth_or_oauth_client
end
context = Doorkeeper::OAuth::Authorization::Context.new(
oauth_client,
grant_type,
scopes
)

server.custom_access_token_expires_in.call(oauth_client, grant_type)
server.custom_access_token_expires_in.call(context)
end
end

Expand All @@ -39,7 +44,8 @@ def issue_token
self.class.access_token_expires_in(
configuration,
pre_auth,
Doorkeeper::OAuth::IMPLICIT
Doorkeeper::OAuth::IMPLICIT,
pre_auth.scopes
),
false
)
Expand Down
2 changes: 1 addition & 1 deletion lib/doorkeeper/oauth/base_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def find_or_create_access_token(client, resource_owner_id, scopes, server)
client,
resource_owner_id,
scopes,
Authorization::Token.access_token_expires_in(server, client, grant_type),
Authorization::Token.access_token_expires_in(server, client, grant_type, scopes),
server.refresh_token_enabled?
)
end
Expand Down
3 changes: 2 additions & 1 deletion lib/doorkeeper/oauth/client_credentials/issuer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ def create_token(client, scopes, creator)
ttl = Authorization::Token.access_token_expires_in(
@server,
client,
Doorkeeper::OAuth::CLIENT_CREDENTIALS
Doorkeeper::OAuth::CLIENT_CREDENTIALS,
scopes
)

creator.call(
Expand Down
3 changes: 2 additions & 1 deletion lib/doorkeeper/oauth/refresh_token_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ def access_token_expires_in
Authorization::Token.access_token_expires_in(
server,
client,
Doorkeeper::OAuth::REFRESH_TOKEN
Doorkeeper::OAuth::REFRESH_TOKEN,
scopes
)
end

Expand Down
10 changes: 7 additions & 3 deletions lib/generators/doorkeeper/templates/initializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,14 @@
# access_token_expires_in 2.hours

# Assign custom TTL for access tokens. Will be used instead of access_token_expires_in
# option if defined.
# option if defined. `context` has the following properties available
#
# custom_access_token_expires_in do |oauth_client, grant_type|
# oauth_client.application.additional_settings.implicit_oauth_expiration
# `client` - the OAuth client application (see Doorkeeper::OAuth::Client)
# `grant_type` - the grant type of the request (see Doorkeeper::OAuth)
# `scopes` - the requested scopes (see Doorkeeper::OAuth::Scopes)
#
# custom_access_token_expires_in do |context|
# context.client.application.additional_settings.implicit_oauth_expiration
# end

# Use a custom class for generating the access token.
Expand Down
4 changes: 2 additions & 2 deletions spec/controllers/authorizations_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ def translated_error_message(key)
before do
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 { |_app, grant|
grant == Doorkeeper::OAuth::IMPLICIT ? 1234 : nil
allow(Doorkeeper.configuration).to receive(:custom_access_token_expires_in).and_return(proc { |context|
context.grant_type == Doorkeeper::OAuth::IMPLICIT ? 1234 : nil
})
end

Expand Down
4 changes: 3 additions & 1 deletion spec/lib/oauth/authorization_code_request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ module Doorkeeper::OAuth
double :server,
access_token_expires_in: 2.days,
refresh_token_enabled?: false,
custom_access_token_expires_in: ->(_app, grant) { grant == Doorkeeper::OAuth::AUTHORIZATION_CODE ? 1234 : nil }
custom_access_token_expires_in: lambda { |context|
context.grant_type == Doorkeeper::OAuth::AUTHORIZATION_CODE ? 1234 : nil
}
end

let(:grant) { FactoryBot.create :access_grant }
Expand Down
16 changes: 15 additions & 1 deletion spec/lib/oauth/base_request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ module Doorkeeper::OAuth
let(:server) do
double :server,
access_token_expires_in: 100,
custom_access_token_expires_in: ->(_app, _grant) { nil },
custom_access_token_expires_in: ->(_context) { nil },
refresh_token_enabled?: false
end

Expand Down Expand Up @@ -105,6 +105,20 @@ module Doorkeeper::OAuth

expect(result).to be_an_instance_of(Doorkeeper::AccessToken)
end

it "respects custom_access_token_expires_in" do
server = double(:server,
access_token_expires_in: 100,
custom_access_token_expires_in: ->(context) { context.scopes == "public" ? 500 : nil },
refresh_token_enabled?: false)
result = subject.find_or_create_access_token(
client,
"1",
"public",
server
)
expect(result.expires_in).to eql(500)
end
end

describe "#scopes" do
Expand Down
31 changes: 26 additions & 5 deletions spec/lib/oauth/client_credentials/issuer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class Doorkeeper::OAuth::ClientCredentialsRequest
double(
:server,
access_token_expires_in: 100,
custom_access_token_expires_in: ->(_app, _grant) { nil }
custom_access_token_expires_in: ->(_context) { nil }
)
end
let(:validation) { double :validation, valid?: true }
Expand Down Expand Up @@ -61,23 +61,44 @@ class Doorkeeper::OAuth::ClientCredentialsRequest
end

context 'with custom expirations' do
let(:custom_ttl) { 1234 }
let(:custom_ttl_grant) { 1234 }
let(:custom_ttl_scope) { 1235 }
let(:custom_scope) { 'special' }
let(:server) do
double(
:server,
custom_access_token_expires_in: ->(_app, grant) { grant == Doorkeeper::OAuth::CLIENT_CREDENTIALS ? custom_ttl : nil }
custom_access_token_expires_in: lambda { |context|
# scopes is normally an object but is a string in this test
if context.scopes == custom_scope
custom_ttl_scope
elsif context.grant_type == Doorkeeper::OAuth::CLIENT_CREDENTIALS
custom_ttl_grant
else
nil
end
}
)
end

it 'creates with correct token parameters' do
it 'respects grant based rules' do
expect(creator).to receive(:call).with(
client,
scopes,
expires_in: custom_ttl,
expires_in: custom_ttl_grant,
use_refresh_token: false
)
subject.create client, scopes, creator
end

it 'respects scope based rules' do
expect(creator).to receive(:call).with(
client,
custom_scope,
expires_in: custom_ttl_scope,
use_refresh_token: false
)
subject.create client, custom_scope, creator
end
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/lib/oauth/client_credentials_request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Doorkeeper::OAuth
double(
default_scopes: nil,
access_token_expires_in: 2.hours,
custom_access_token_expires_in: ->(_app, _grant) { nil }
custom_access_token_expires_in: ->(_context) { nil }
)
end

Expand Down
36 changes: 35 additions & 1 deletion spec/lib/oauth/password_access_token_request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ module Doorkeeper::OAuth
default_scopes: Doorkeeper::OAuth::Scopes.new,
access_token_expires_in: 2.hours,
refresh_token_enabled?: false,
custom_access_token_expires_in: ->(_app, grant) { grant == Doorkeeper::OAuth::PASSWORD ? 1234 : nil }
custom_access_token_expires_in: lambda { |context|
context.grant_type == Doorkeeper::OAuth::PASSWORD ? 1234 : nil
}
)
end
let(:client) { FactoryBot.create(:application) }
Expand Down Expand Up @@ -93,5 +95,37 @@ module Doorkeeper::OAuth
expect(Doorkeeper::AccessToken.last.scopes).to include('public')
end
end

describe 'with custom expiry' do
let(:server) do
double(
:server,
default_scopes: Doorkeeper::OAuth::Scopes.new,
access_token_expires_in: 2.hours,
refresh_token_enabled?: false,
custom_access_token_expires_in: lambda { |context|
context.scopes.exists?('public') ? 222 : nil
}
)
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'))
expect do
subject.authorize
end.to change { Doorkeeper::AccessToken.count }.by(1)
expect(Doorkeeper::AccessToken.last.expires_in).to eq(222)
end

it 'falls back to the default otherwise' do
subject = PasswordAccessTokenRequest.new(server, client, owner, scope: 'private')
allow(server).to receive(:scopes).and_return(Doorkeeper::OAuth::Scopes.from_string('private'))
expect do
subject.authorize
end.to change { Doorkeeper::AccessToken.count }.by(1)
expect(Doorkeeper::AccessToken.last.expires_in).to eq(2.hours)
end
end
end
end
10 changes: 7 additions & 3 deletions spec/lib/oauth/refresh_token_request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ module Doorkeeper::OAuth
let(:server) do
double :server,
access_token_expires_in: 2.minutes,
custom_access_token_expires_in: ->(_oauth_client, _grant) { nil }
custom_access_token_expires_in: ->(_context) { nil }
end

let(:refresh_token) do
Expand All @@ -30,7 +30,9 @@ module Doorkeeper::OAuth
it 'issues a new token for the client with custom expires_in' do
server = double :server,
access_token_expires_in: 2.minutes,
custom_access_token_expires_in: ->(_app, grant) { grant == Doorkeeper::OAuth::REFRESH_TOKEN ? 1234 : nil }
custom_access_token_expires_in: lambda { |context|
context.grant_type == Doorkeeper::OAuth::REFRESH_TOKEN ? 1234 : nil
}

allow(Doorkeeper::AccessToken).to receive(:refresh_token_revoked_on_use?).and_return(false)

Expand Down Expand Up @@ -85,7 +87,9 @@ module Doorkeeper::OAuth
let(:server) do
double :server,
access_token_expires_in: 2.minutes,
custom_access_token_expires_in: ->(_oauth_client, grant) { grant == Doorkeeper::OAuth::REFRESH_TOKEN ? 1234 : nil }
custom_access_token_expires_in: lambda { |context|
context.grant_type == Doorkeeper::OAuth::REFRESH_TOKEN ? 1234 : nil
}
end

before do
Expand Down
4 changes: 2 additions & 2 deletions spec/lib/oauth/token_request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ module Doorkeeper::OAuth
before do
Doorkeeper.configure do
orm DOORKEEPER_ORM
custom_access_token_expires_in do |_oauth_client, grant_type|
grant_type == Doorkeeper::OAuth::IMPLICIT ? 1234 : nil
custom_access_token_expires_in do |context|
context.grant_type == Doorkeeper::OAuth::IMPLICIT ? 1234 : nil
end
end
end
Expand Down

0 comments on commit 37abb5e

Please sign in to comment.