Skip to content

Commit

Permalink
Adds custom expiration time based on grant type
Browse files Browse the repository at this point in the history
  • Loading branch information
rishabhsairawat committed Mar 12, 2018
1 parent 2214701 commit e9bf05b
Show file tree
Hide file tree
Showing 19 changed files with 51 additions and 28 deletions.
2 changes: 1 addition & 1 deletion NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
User-visible changes worth mentioning.

## master

- [#996] Expiration Time Base On Grant Type
- [#997] Allow PKCE authorization_code flow as specified in RCF7636
- [#907] Fix lookup for matching tokens in certain edge-cases
- [#992] Add API option to use Doorkeeper without management views for API only
Expand Down
1 change: 1 addition & 0 deletions lib/doorkeeper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
require 'doorkeeper/oauth/helpers/uri_checker'
require 'doorkeeper/oauth/helpers/unique_token'

require 'doorkeeper/oauth'
require 'doorkeeper/oauth/scopes'
require 'doorkeeper/oauth/error'
require 'doorkeeper/oauth/base_response'
Expand Down
2 changes: 1 addition & 1 deletion lib/doorkeeper/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,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) { nil }
option :custom_access_token_expires_in, default: ->(_app, _grant) { 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
11 changes: 11 additions & 0 deletions lib/doorkeeper/oauth.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
module Doorkeeper
module OAuth
GRANT_TYPES = [
AUTHORIZATION_CODE = 'authorization_code'.freeze,
IMPLICIT = 'implicit'.freeze,
PASSWORD = 'password'.freeze,
CLIENT_CREDENTIALS = 'client_credentials'.freeze,
REFRESH_TOKEN = 'refresh_token'.freeze
].freeze
end
end
10 changes: 5 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)
if (expiration = custom_expiration(server, pre_auth_or_oauth_client))
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))
expiration
else
server.access_token_expires_in
Expand All @@ -15,14 +15,14 @@ def access_token_expires_in(server, pre_auth_or_oauth_client)

private

def custom_expiration(server, pre_auth_or_oauth_client)
def custom_expiration(server, pre_auth_or_oauth_client, grant_type)
oauth_client = if pre_auth_or_oauth_client.respond_to?(:client)
pre_auth_or_oauth_client.client
else
pre_auth_or_oauth_client
end

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

Expand All @@ -36,7 +36,7 @@ def issue_token
pre_auth.client,
resource_owner.id,
pre_auth.scopes,
self.class.access_token_expires_in(configuration, pre_auth),
self.class.access_token_expires_in(configuration, pre_auth, Doorkeeper::OAuth::IMPLICIT),
false
)
end
Expand Down
1 change: 1 addition & 0 deletions lib/doorkeeper/oauth/authorization_code_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ def initialize(server, grant, client, parameters = {})
@server = server
@client = client
@grant = grant
@grant_type = Doorkeeper::OAuth::AUTHORIZATION_CODE
@redirect_uri = parameters[:redirect_uri]
@code_verifier = parameters[:code_verifier]
@client = client_by_uid(parameters) if no_secret_allowed_for_pkce?
Expand Down
4 changes: 3 additions & 1 deletion lib/doorkeeper/oauth/base_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ module Doorkeeper
module OAuth
class BaseRequest
include Validations

attr_reader :grant_type

def authorize
validate
Expand Down Expand Up @@ -37,7 +39,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),
Authorization::Token.access_token_expires_in(server, client, grant_type),
server.refresh_token_enabled?
)
end
Expand Down
2 changes: 1 addition & 1 deletion lib/doorkeeper/oauth/client_credentials/issuer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def create(client, scopes, creator = Creator.new)
private

def create_token(client, scopes, creator)
ttl = Authorization::Token.access_token_expires_in(@server, client)
ttl = Authorization::Token.access_token_expires_in(@server, client, Doorkeeper::OAuth::CLIENT_CREDENTIALS)

creator.call(
client,
Expand Down
1 change: 1 addition & 0 deletions lib/doorkeeper/oauth/password_access_token_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ def initialize(server, client, resource_owner, parameters = {})
@client = client
@parameters = parameters
@original_scopes = parameters[:scope]
@grant_type = Doorkeeper::OAuth::PASSWORD
end

private
Expand Down
2 changes: 1 addition & 1 deletion lib/doorkeeper/oauth/refresh_token_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def access_token_attributes
end

def access_token_expires_in
Authorization::Token.access_token_expires_in(server, client)
Authorization::Token.access_token_expires_in(server, client, Doorkeeper::OAuth::REFRESH_TOKEN)
end

def validate_token_presence
Expand Down
2 changes: 1 addition & 1 deletion lib/generators/doorkeeper/templates/initializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
# access_token_expires_in 2.hours

# Assign a custom TTL for implicit grants.
# custom_access_token_expires_in do |oauth_client|
# custom_access_token_expires_in do |oauth_client, grant_type|
# oauth_client.application.additional_settings.implicit_oauth_expiration
# end

Expand Down
13 changes: 8 additions & 5 deletions spec/controllers/authorizations_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,13 @@ def translated_error_message(key)
let(:client) { FactoryBot.create :application }
let(:user) { User.create!(name: 'Joe', password: 'sekret') }
let(:access_token) { FactoryBot.build :access_token, resource_owner_id: user.id, application_id: client.id }

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
})
end

describe 'POST #create' do
Expand All @@ -58,7 +61,7 @@ def translated_error_message(key)
end

it 'includes token expiration in fragment' do
expect(response.query_params['expires_in'].to_i).to eq(2.hours.to_i)
expect(response.query_params['expires_in'].to_i).to eq(1234)
end

it 'issues the token for the current client' do
Expand Down Expand Up @@ -96,7 +99,7 @@ def translated_error_message(key)
end

it "includes token expiration in fragment" do
expect(redirect_uri.match(/expires_in=(\d+)&?/)[1].to_i).to eq 2.hours
expect(redirect_uri.match(/expires_in=(\d+)&?/)[1].to_i).to eq 1234
end

it "issues the token for the current client" do
Expand Down Expand Up @@ -260,7 +263,7 @@ def translated_error_message(key)
end

it 'includes token expiration in fragment' do
expect(response.query_params['expires_in'].to_i).to eq(2.hours.to_i)
expect(response.query_params['expires_in'].to_i).to eq(1234)
end

it 'issues the token for the current client' do
Expand Down Expand Up @@ -297,7 +300,7 @@ def translated_error_message(key)
redirect_uri = JSON.parse(response.body)["redirect_uri"]
expect(redirect_uri).to_not be_nil
expect(redirect_uri.match(/token_type=(\w+)&?/)[1]).to eq "bearer"
expect(redirect_uri.match(/expires_in=(\d+)&?/)[1].to_i).to eq 2.hours
expect(redirect_uri.match(/expires_in=(\d+)&?/)[1].to_i).to eq 1234
expect(
redirect_uri.match(/access_token=([a-f0-9]+)&?/)[1]
).to eq Doorkeeper::AccessToken.first.token
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,7 @@ module Doorkeeper::OAuth
double :server,
access_token_expires_in: 2.days,
refresh_token_enabled?: false,
custom_access_token_expires_in: ->(_app) { nil }
custom_access_token_expires_in: ->(_app, _grant) { _grant == Doorkeeper::OAuth::AUTHORIZATION_CODE ? 1234 : nil }
end

let(:grant) { FactoryBot.create :access_grant }
Expand All @@ -22,6 +22,8 @@ module Doorkeeper::OAuth
expect do
subject.authorize
end.to change { client.reload.access_tokens.count }.by(1)

expect(client.reload.access_tokens.sort_by(&:created_at).last.expires_in).to eq(1234)
end

it "issues the token with same grant's scopes" do
Expand Down
2 changes: 1 addition & 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: ->(_) { nil },
custom_access_token_expires_in: ->(_app, _grant) { nil },
refresh_token_enabled?: false
end

Expand Down
6 changes: 3 additions & 3 deletions spec/lib/oauth/client_credentials/issuer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class Doorkeeper::OAuth::ClientCredentialsRequest
double(
:server,
access_token_expires_in: 100,
custom_access_token_expires_in: ->(_app) { nil }
custom_access_token_expires_in: ->(_app, _grant) { nil }
)
end
let(:validation) { double :validation, valid?: true }
Expand Down Expand Up @@ -63,11 +63,11 @@ class Doorkeeper::OAuth::ClientCredentialsRequest
end

context 'with custom expirations' do
let(:custom_ttl) { 1233 }
let(:custom_ttl) { 1234 }
let(:server) do
double(
:server,
custom_access_token_expires_in: ->(_app) { custom_ttl }
custom_access_token_expires_in: ->(_app, _grant) { _grant == Doorkeeper::OAuth::CLIENT_CREDENTIALS ? custom_ttl : nil }
)
end

Expand Down
3 changes: 2 additions & 1 deletion spec/lib/oauth/client_credentials_request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ module Doorkeeper::OAuth
let(:server) do
double(
default_scopes: nil,
custom_access_token_expires_in: ->(_app) { nil }
access_token_expires_in: 2.hours,
custom_access_token_expires_in: ->(_app, _grant) { nil }
)
end

Expand Down
3 changes: 2 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,7 @@ 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) { nil }
custom_access_token_expires_in: ->(_app, _grant) { _grant == Doorkeeper::OAuth::PASSWORD ? 1234 : nil }
)
end
let(:client) { FactoryBot.create(:application) }
Expand All @@ -22,6 +22,7 @@ module Doorkeeper::OAuth
expect do
subject.authorize
end.to change { client.reload.access_tokens.count }.by(1)
expect(client.reload.access_tokens.sort_by(&:created_at).last.expires_in).to eq(1234)
end

it 'issues a new token without a client' do
Expand Down
6 changes: 3 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) { nil }
custom_access_token_expires_in: -> (_oauth_client, _grant) { nil }
end

let(:refresh_token) do
Expand All @@ -30,7 +30,7 @@ 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: ->(_oauth_client) { 1234 }
custom_access_token_expires_in: ->(_app, _grant) { _grant == 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 +85,7 @@ module Doorkeeper::OAuth
let(:server) do
double :server,
access_token_expires_in: 2.minutes,
custom_access_token_expires_in: ->(_oauth_client) { 1234 }
custom_access_token_expires_in: ->(_oauth_client, _grant) { 1234 }
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 @@ -51,8 +51,8 @@ module Doorkeeper::OAuth
before do
Doorkeeper.configure do
orm DOORKEEPER_ORM
custom_access_token_expires_in do |_oauth_client|
1234
custom_access_token_expires_in do |_oauth_client, _grant_type|
_grant_type == Doorkeeper::OAuth::IMPLICIT ? 1234 : nil
end
end
end
Expand Down

0 comments on commit e9bf05b

Please sign in to comment.