Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix token introspection tokens validation #1209

Merged
merged 1 commit into from
Mar 9, 2019
Merged

Fix token introspection tokens validation #1209

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ User-visible changes worth mentioning.

## master

- [#1202] Use correct HTTP status codes for error responses
- [#1209] Fix tokens validation for Token Introspection request.
- [#1202] Use correct HTTP status codes for error responses.

**[IMPORTANT]**: this change might break your application if you were relying on the previous
401 status codes, this is now a 400 by default, or a 401 for `invalid_client` and `invalid_token` errors.
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/doorkeeper/tokens_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def introspect
if introspection.authorized?
render json: introspection.to_json, status: 200
else
error = OAuth::ErrorResponse.new(name: introspection.error)
error = introspection.error_response
response.headers.merge!(error.headers)
render json: error.body, status: error.status
end
Expand Down
58 changes: 54 additions & 4 deletions lib/doorkeeper/oauth/token_introspection.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,16 @@ def authorized?
@error.blank?
end

def error_response
return if @error.blank?

if @error == :invalid_token
OAuth::InvalidTokenResponse.from_access_token(authorized_token)
else
OAuth::ErrorResponse.new(name: @error)
end
end

def to_json
active? ? success_response : failure_response
end
Expand All @@ -37,13 +47,29 @@ def to_json
#
# @see https://www.oauth.com/oauth2-servers/token-introspection-endpoint/
#
# To prevent token scanning attacks, the endpoint MUST also require
# some form of authorization to access this endpoint, such as client
# authentication as described in OAuth 2.0 [RFC6749] or a separate
# OAuth 2.0 access token such as the bearer token described in OAuth
# 2.0 Bearer Token Usage [RFC6750].
#
def authorize!
# Requested client authorization
if server.credentials
@error = :invalid_client unless authorized_client
else
elsif authorized_token
# Requested bearer token authorization
@error = :invalid_request unless authorized_token
#
# If the protected resource uses an OAuth 2.0 bearer token to authorize
nbulaj marked this conversation as resolved.
Show resolved Hide resolved
# its call to the introspection endpoint and the token used for
# authorization does not contain sufficient privileges or is otherwise
nbulaj marked this conversation as resolved.
Show resolved Hide resolved
# invalid for this request, the authorization server responds with an
# HTTP 401 code as described in Section 3 of OAuth 2.0 Bearer Token
# Usage [RFC6750].
#
@error = :invalid_token if authorized_token_matches_introspected? || !authorized_token.accessible?
nbulaj marked this conversation as resolved.
Show resolved Hide resolved
else
@error = :invalid_request
end
end

Expand Down Expand Up @@ -103,6 +129,25 @@ def failure_response
# * The token expired
# * The token was issued to a different client than is making this request
#
# Since resource servers using token introspection rely on the
# authorization server to determine the state of a token, the
# authorization server MUST perform all applicable checks against a
# token's state. For instance, these tests include the following:
#
# o If the token can expire, the authorization server MUST determine
# whether or not the token has expired.
# o If the token can be issued before it is able to be used, the
# authorization server MUST determine whether or not a token's valid
# period has started yet.
# o If the token can be revoked after it was issued, the authorization
# server MUST determine whether or not such a revocation has taken
# place.
# o If the token has been signed, the authorization server MUST
# validate the signature.
# o If the token can be used only at certain resource servers, the
# authorization server MUST determine whether or not the token can
# be used at the resource server making the introspection call.
#
def active?
if authorized_client
valid_token? && authorized_for_client?
Expand All @@ -113,13 +158,18 @@ def active?

# Token can be valid only if it is not expired or revoked.
def valid_token?
@token.present? && @token.accessible?
@token && @token.accessible?
nbulaj marked this conversation as resolved.
Show resolved Hide resolved
end

# RFC7662 Section 2.1
def authorized_token_matches_introspected?
authorized_token.token == @token.token
end

# If token doesn't belong to some client, then it is public.
# Otherwise in it required for token to be connected to the same client.
def authorized_for_client?
if @token.application.present?
if @token.application
@token.application == authorized_client.application
else
true
Expand Down
83 changes: 52 additions & 31 deletions spec/controllers/tokens_controller_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,28 +130,26 @@
end

describe 'when requested token introspection' do
context 'authorized using Bearer token' do
let(:client) { FactoryBot.create(:application) }
let(:access_token) { FactoryBot.create(:access_token, application: client) }
let(:client) { FactoryBot.create(:application) }
let(:access_token) { FactoryBot.create(:access_token, application: client) }
let(:token_for_introspection) { FactoryBot.create(:access_token, application: client) }

context 'authorized using valid Bearer token' do
it 'responds with full token introspection' do
request.headers['Authorization'] = "Bearer #{access_token.token}"

post :introspect, params: { token: access_token.token }
post :introspect, params: { token: token_for_introspection.token }

should_have_json 'active', true
expect(json_response).to include('client_id', 'token_type', 'exp', 'iat')
end
end

context 'authorized using Client Authentication' do
let(:client) { FactoryBot.create(:application) }
let(:access_token) { FactoryBot.create(:access_token, application: client) }

context 'authorized using valid Client Authentication' do
it 'responds with full token introspection' do
request.headers['Authorization'] = basic_auth_header_for_client(client)

post :introspect, params: { token: access_token.token }
post :introspect, params: { token: token_for_introspection.token }

should_have_json 'active', true
expect(json_response).to include('client_id', 'token_type', 'exp', 'iat')
Expand All @@ -160,9 +158,6 @@
end

context 'using custom introspection response' do
let(:client) { FactoryBot.create(:application) }
let(:access_token) { FactoryBot.create(:access_token, application: client) }

before do
Doorkeeper.configure do
orm DOORKEEPER_ORM
Expand All @@ -178,7 +173,7 @@
it 'responds with full token introspection' do
request.headers['Authorization'] = "Bearer #{access_token.token}"

post :introspect, params: { token: access_token.token }
post :introspect, params: { token: token_for_introspection.token }

expect(json_response).to include('client_id', 'token_type', 'exp', 'iat', 'sub', 'aud')
should_have_json 'sub', 'Z5O3upPC88QrAjx00dis'
Expand All @@ -187,13 +182,12 @@
end

context 'public access token' do
let(:client) { FactoryBot.create(:application) }
let(:access_token) { FactoryBot.create(:access_token, application: nil) }
let(:token_for_introspection) { FactoryBot.create(:access_token, application: nil) }

it 'responds with full token introspection' do
request.headers['Authorization'] = basic_auth_header_for_client(client)

post :introspect, params: { token: access_token.token }
post :introspect, params: { token: token_for_introspection.token }

should_have_json 'active', true
expect(json_response).to include('client_id', 'token_type', 'exp', 'iat')
Expand All @@ -202,14 +196,12 @@
end

context 'token was issued to a different client than is making this request' do
let(:client) { FactoryBot.create(:application) }
let(:different_client) { FactoryBot.create(:application) }
let(:access_token) { FactoryBot.create(:access_token, application: client) }

it 'responds with only active state' do
request.headers['Authorization'] = basic_auth_header_for_client(different_client)

post :introspect, params: { token: access_token.token }
post :introspect, params: { token: token_for_introspection.token }

expect(response).to be_successful

Expand All @@ -218,6 +210,36 @@
end
end

context 'authorized using invalid Bearer token' do
let(:token_for_introspection) do
FactoryBot.create(:access_token, application: client, revoked_at: 1.day.ago)
end

it 'responds with invalid token error' do
request.headers['Authorization'] = "Bearer #{token_for_introspection.token}"

post :introspect, params: { token: access_token.token }

response_status_should_be 401

should_not_have_json 'active'
should_have_json 'error', 'invalid_token'
end
end

context 'authorized using the Bearer token that need to be introspected' do
it 'responds with invalid token error' do
request.headers['Authorization'] = "Bearer #{access_token.token}"

post :introspect, params: { token: access_token.token }

response_status_should_be 401

should_not_have_json 'active'
should_have_json 'error', 'invalid_token'
end
end

context 'using invalid credentials to authorize' do
let(:client) { double(uid: '123123', secret: '666999') }
let(:access_token) { FactoryBot.create(:access_token) }
Expand All @@ -236,9 +258,6 @@
end

context 'using wrong token value' do
let(:client) { FactoryBot.create(:application) }
let(:access_token) { FactoryBot.create(:access_token, application: client) }

it 'responds with only active state' do
request.headers['Authorization'] = basic_auth_header_for_client(client)

Expand All @@ -249,39 +268,41 @@
end
end

context 'when requested Access Token expired' do
let(:client) { FactoryBot.create(:application) }
let(:access_token) { FactoryBot.create(:access_token, application: client, created_at: 1.year.ago) }
context 'when requested access token expired' do
let(:token_for_introspection) do
FactoryBot.create(:access_token, application: client, created_at: 1.year.ago)
end

it 'responds with only active state' do
request.headers['Authorization'] = basic_auth_header_for_client(client)

post :introspect, params: { token: access_token.token }
post :introspect, params: { token: token_for_introspection.token }

should_have_json 'active', false
expect(json_response).not_to include('client_id', 'token_type', 'exp', 'iat')
end
end

context 'when requested Access Token revoked' do
let(:client) { FactoryBot.create(:application) }
let(:access_token) { FactoryBot.create(:access_token, application: client, revoked_at: 1.year.ago) }
let(:token_for_introspection) do
FactoryBot.create(:access_token, application: client, revoked_at: 1.year.ago)
end

it 'responds with only active state' do
request.headers['Authorization'] = basic_auth_header_for_client(client)

post :introspect, params: { token: access_token.token }
post :introspect, params: { token: token_for_introspection.token }

should_have_json 'active', false
expect(json_response).not_to include('client_id', 'token_type', 'exp', 'iat')
end
end

context 'unauthorized (no bearer token or client credentials)' do
let(:access_token) { FactoryBot.create(:access_token) }
let(:token_for_introspection) { FactoryBot.create(:access_token) }

it 'responds with invalid_request error' do
post :introspect, params: { token: access_token.token }
post :introspect, params: { token: token_for_introspection.token }

expect(response).not_to be_successful
response_status_should_be 400
Expand Down