Commit ca6bf62e authored by Rémy Coutable's avatar Rémy Coutable

Merge branch '20492-access-token-scopes' into 'master'

Resolve "Add a doorkeeper scope suitable for authentication"

## What does this MR do?

- Add a single new scope (in addition to the `api` scope we've had) - `read_user`
- Allow creating OAuth applications and Personal access tokens with a scope selected
- Enforce scopes in the API

## What are the relevant issue numbers?

- Closes #20492 
- EE counterpart for this MR: gitlab-org/gitlab-ee!946

See merge request !5951
parents 34875519 eb434b15
...@@ -262,3 +262,13 @@ table.u2f-registrations { ...@@ -262,3 +262,13 @@ table.u2f-registrations {
border-right: solid 1px transparent; border-right: solid 1px transparent;
} }
} }
.oauth-application-show {
.scope-name {
font-weight: 600;
}
.scopes-list {
padding-left: 18px;
}
}
\ No newline at end of file
class Admin::ApplicationsController < Admin::ApplicationController class Admin::ApplicationsController < Admin::ApplicationController
include OauthApplications
before_action :set_application, only: [:show, :edit, :update, :destroy] before_action :set_application, only: [:show, :edit, :update, :destroy]
before_action :load_scopes, only: [:new, :edit]
def index def index
@applications = Doorkeeper::Application.where("owner_id IS NULL") @applications = Doorkeeper::Application.where("owner_id IS NULL")
...@@ -47,6 +50,6 @@ class Admin::ApplicationsController < Admin::ApplicationController ...@@ -47,6 +50,6 @@ class Admin::ApplicationsController < Admin::ApplicationController
# Only allow a trusted parameter "white list" through. # Only allow a trusted parameter "white list" through.
def application_params def application_params
params[:doorkeeper_application].permit(:name, :redirect_uri) params[:doorkeeper_application].permit(:name, :redirect_uri, :scopes)
end end
end end
module OauthApplications
extend ActiveSupport::Concern
included do
before_action :prepare_scopes, only: [:create, :update]
end
def prepare_scopes
scopes = params.dig(:doorkeeper_application, :scopes)
if scopes
params[:doorkeeper_application][:scopes] = scopes.join(' ')
end
end
def load_scopes
@scopes = Doorkeeper.configuration.scopes
end
end
...@@ -2,10 +2,12 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController ...@@ -2,10 +2,12 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController
include Gitlab::CurrentSettings include Gitlab::CurrentSettings
include Gitlab::GonHelper include Gitlab::GonHelper
include PageLayoutHelper include PageLayoutHelper
include OauthApplications
before_action :verify_user_oauth_applications_enabled before_action :verify_user_oauth_applications_enabled
before_action :authenticate_user! before_action :authenticate_user!
before_action :add_gon_variables before_action :add_gon_variables
before_action :load_scopes, only: [:index, :create, :edit]
layout 'profile' layout 'profile'
......
class Profiles::PersonalAccessTokensController < Profiles::ApplicationController class Profiles::PersonalAccessTokensController < Profiles::ApplicationController
before_action :load_personal_access_tokens, only: :index
def index def index
@personal_access_token = current_user.personal_access_tokens.build set_index_vars
end end
def create def create
...@@ -12,7 +10,7 @@ class Profiles::PersonalAccessTokensController < Profiles::ApplicationController ...@@ -12,7 +10,7 @@ class Profiles::PersonalAccessTokensController < Profiles::ApplicationController
flash[:personal_access_token] = @personal_access_token.token flash[:personal_access_token] = @personal_access_token.token
redirect_to profile_personal_access_tokens_path, notice: "Your new personal access token has been created." redirect_to profile_personal_access_tokens_path, notice: "Your new personal access token has been created."
else else
load_personal_access_tokens set_index_vars
render :index render :index
end end
end end
...@@ -32,10 +30,12 @@ class Profiles::PersonalAccessTokensController < Profiles::ApplicationController ...@@ -32,10 +30,12 @@ class Profiles::PersonalAccessTokensController < Profiles::ApplicationController
private private
def personal_access_token_params def personal_access_token_params
params.require(:personal_access_token).permit(:name, :expires_at) params.require(:personal_access_token).permit(:name, :expires_at, scopes: [])
end end
def load_personal_access_tokens def set_index_vars
@personal_access_token ||= current_user.personal_access_tokens.build
@scopes = Gitlab::Auth::SCOPES
@active_personal_access_tokens = current_user.personal_access_tokens.active.order(:expires_at) @active_personal_access_tokens = current_user.personal_access_tokens.active.order(:expires_at)
@inactive_personal_access_tokens = current_user.personal_access_tokens.inactive @inactive_personal_access_tokens = current_user.personal_access_tokens.inactive
end end
......
...@@ -2,6 +2,8 @@ class PersonalAccessToken < ActiveRecord::Base ...@@ -2,6 +2,8 @@ class PersonalAccessToken < ActiveRecord::Base
include TokenAuthenticatable include TokenAuthenticatable
add_authentication_token_field :token add_authentication_token_field :token
serialize :scopes, Array
belongs_to :user belongs_to :user
scope :active, -> { where(revoked: false).where("expires_at >= NOW() OR expires_at IS NULL") } scope :active, -> { where(revoked: false).where("expires_at >= NOW() OR expires_at IS NULL") }
......
AccessTokenValidationService = Struct.new(:token) do
# Results:
VALID = :valid
EXPIRED = :expired
REVOKED = :revoked
INSUFFICIENT_SCOPE = :insufficient_scope
def validate(scopes: [])
if token.expired?
return EXPIRED
elsif token.revoked?
return REVOKED
elsif !self.include_any_scope?(scopes)
return INSUFFICIENT_SCOPE
else
return VALID
end
end
# True if the token's scope contains any of the passed scopes.
def include_any_scope?(scopes)
if scopes.blank?
true
else
# Check whether the token is allowed access to any of the required scopes.
Set.new(scopes).intersection(Set.new(token.scopes)).present?
end
end
end
module Oauth2::AccessTokenValidationService
# Results:
VALID = :valid
EXPIRED = :expired
REVOKED = :revoked
INSUFFICIENT_SCOPE = :insufficient_scope
class << self
def validate(token, scopes: [])
if token.expired?
return EXPIRED
elsif token.revoked?
return REVOKED
elsif !self.sufficient_scope?(token, scopes)
return INSUFFICIENT_SCOPE
else
return VALID
end
end
protected
# True if the token's scope is a superset of required scopes,
# or the required scopes is empty.
def sufficient_scope?(token, scopes)
if scopes.blank?
# if no any scopes required, the scopes of token is sufficient.
return true
else
# If there are scopes required, then check whether
# the set of authorized scopes is a superset of the set of required scopes
required_scopes = Set.new(scopes)
authorized_scopes = Set.new(token.scopes)
return authorized_scopes >= required_scopes
end
end
end
end
...@@ -18,6 +18,12 @@ ...@@ -18,6 +18,12 @@
Use Use
%code= Doorkeeper.configuration.native_redirect_uri %code= Doorkeeper.configuration.native_redirect_uri
for local tests for local tests
.form-group
= f.label :scopes, class: 'col-sm-2 control-label'
.col-sm-10
= render 'shared/tokens/scopes_form', prefix: 'doorkeeper_application', token: application, scopes: @scopes
.form-actions .form-actions
= f.submit 'Submit', class: "btn btn-save wide" = f.submit 'Submit', class: "btn btn-save wide"
= link_to "Cancel", admin_applications_path, class: "btn btn-default" = link_to "Cancel", admin_applications_path, class: "btn btn-default"
...@@ -2,8 +2,7 @@ ...@@ -2,8 +2,7 @@
%h3.page-title %h3.page-title
Application: #{@application.name} Application: #{@application.name}
.table-holder.oauth-application-show
.table-holder
%table.table %table.table
%tr %tr
%td %td
...@@ -23,6 +22,9 @@ ...@@ -23,6 +22,9 @@
- @application.redirect_uri.split.each do |uri| - @application.redirect_uri.split.each do |uri|
%div %div
%span.monospace= uri %span.monospace= uri
= render "shared/tokens/scopes_list", token: @application
.form-actions .form-actions
= link_to 'Edit', edit_admin_application_path(@application), class: 'btn btn-primary wide pull-left' = link_to 'Edit', edit_admin_application_path(@application), class: 'btn btn-primary wide pull-left'
= render 'delete_form', application: @application, submit_btn_css: 'btn btn-danger prepend-left-10' = render 'delete_form', application: @application, submit_btn_css: 'btn btn-danger prepend-left-10'
...@@ -17,5 +17,9 @@ ...@@ -17,5 +17,9 @@
%code= Doorkeeper.configuration.native_redirect_uri %code= Doorkeeper.configuration.native_redirect_uri
for local tests for local tests
.form-group
= f.label :scopes, class: 'label-light'
= render 'shared/tokens/scopes_form', prefix: 'doorkeeper_application', token: application, scopes: @scopes
.prepend-top-default .prepend-top-default
= f.submit 'Save application', class: "btn btn-create" = f.submit 'Save application', class: "btn btn-create"
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
%h3.page-title %h3.page-title
Application: #{@application.name} Application: #{@application.name}
.table-holder .table-holder.oauth-application-show
%table.table %table.table
%tr %tr
%td %td
...@@ -22,6 +22,9 @@ ...@@ -22,6 +22,9 @@
- @application.redirect_uri.split.each do |uri| - @application.redirect_uri.split.each do |uri|
%div %div
%span.monospace= uri %span.monospace= uri
= render "shared/tokens/scopes_list", token: @application
.form-actions .form-actions
= link_to 'Edit', edit_oauth_application_path(@application), class: 'btn btn-primary wide pull-left' = link_to 'Edit', edit_oauth_application_path(@application), class: 'btn btn-primary wide pull-left'
= render 'delete_form', application: @application, submit_btn_css: 'btn btn-danger prepend-left-10' = render 'delete_form', application: @application, submit_btn_css: 'btn btn-danger prepend-left-10'
- personal_access_token = local_assigns.fetch(:personal_access_token)
- scopes = local_assigns.fetch(:scopes)
= form_for [:profile, personal_access_token], method: :post, html: { class: 'js-requires-input' } do |f|
= form_errors(personal_access_token)
.form-group
= f.label :name, class: 'label-light'
= f.text_field :name, class: "form-control", required: true
.form-group
= f.label :expires_at, class: 'label-light'
= f.text_field :expires_at, class: "datepicker form-control"
.form-group
= f.label :scopes, class: 'label-light'
= render 'shared/tokens/scopes_form', prefix: 'personal_access_token', token: personal_access_token, scopes: scopes
.prepend-top-default
= f.submit 'Create Personal Access Token', class: "btn btn-create"
...@@ -28,21 +28,8 @@ ...@@ -28,21 +28,8 @@
Add a Personal Access Token Add a Personal Access Token
%p.profile-settings-content %p.profile-settings-content
Pick a name for the application, and we'll give you a unique token. Pick a name for the application, and we'll give you a unique token.
= form_for [:profile, @personal_access_token],
method: :post, html: { class: 'js-requires-input' } do |f|
= form_errors(@personal_access_token) = render "form", personal_access_token: @personal_access_token, scopes: @scopes
.form-group
= f.label :name, class: 'label-light'
= f.text_field :name, class: "form-control", required: true
.form-group
= f.label :expires_at, class: 'label-light'
= f.text_field :expires_at, class: "datepicker form-control", required: false
.prepend-top-default
= f.submit 'Create Personal Access Token', class: "btn btn-create"
%hr %hr
...@@ -56,6 +43,7 @@ ...@@ -56,6 +43,7 @@
%th Name %th Name
%th Created %th Created
%th Expires %th Expires
%th Scopes
%th %th
%tbody %tbody
- @active_personal_access_tokens.each do |token| - @active_personal_access_tokens.each do |token|
...@@ -67,6 +55,7 @@ ...@@ -67,6 +55,7 @@
= token.expires_at.to_date.to_s(:medium) = token.expires_at.to_date.to_s(:medium)
- else - else
%span.personal-access-tokens-never-expires-label Never %span.personal-access-tokens-never-expires-label Never
%td= token.scopes.present? ? token.scopes.join(", ") : "<no scopes selected>"
%td= link_to "Revoke", revoke_profile_personal_access_token_path(token), method: :put, class: "btn btn-danger pull-right", data: { confirm: "Are you sure you want to revoke this token? This action cannot be undone." } %td= link_to "Revoke", revoke_profile_personal_access_token_path(token), method: :put, class: "btn btn-danger pull-right", data: { confirm: "Are you sure you want to revoke this token? This action cannot be undone." }
- else - else
......
- scopes = local_assigns.fetch(:scopes)
- prefix = local_assigns.fetch(:prefix)
- token = local_assigns.fetch(:token)
- scopes.each do |scope|
%fieldset
= check_box_tag "#{prefix}[scopes][]", scope, token.scopes.include?(scope), id: "#{prefix}_scopes_#{scope}"
= label_tag "#{prefix}_scopes_#{scope}", scope
%span= "(#{t(scope, scope: [:doorkeeper, :scopes])})"
- token = local_assigns.fetch(:token)
- return unless token.scopes.present?
%tr
%td
Scopes
%td
%ul.scopes-list.append-bottom-0
- token.scopes.each do |scope|
%li
%span.scope-name= scope
= "(#{t(scope, scope: [:doorkeeper, :scopes])})"
---
title: Add scopes for personal access tokens and OAuth tokens
merge_request: 5951
author:
...@@ -52,8 +52,8 @@ Doorkeeper.configure do ...@@ -52,8 +52,8 @@ Doorkeeper.configure do
# Define access token scopes for your provider # Define access token scopes for your provider
# For more information go to # For more information go to
# https://github.com/doorkeeper-gem/doorkeeper/wiki/Using-Scopes # https://github.com/doorkeeper-gem/doorkeeper/wiki/Using-Scopes
default_scopes :api default_scopes(*Gitlab::Auth::DEFAULT_SCOPES)
# optional_scopes :write, :update optional_scopes(*Gitlab::Auth::OPTIONAL_SCOPES)
# Change the way client credentials are retrieved from the request object. # Change the way client credentials are retrieved from the request object.
# By default it retrieves first from the `HTTP_AUTHORIZATION` header, then # By default it retrieves first from the `HTTP_AUTHORIZATION` header, then
......
...@@ -59,6 +59,7 @@ en: ...@@ -59,6 +59,7 @@ en:
unknown: "The access token is invalid" unknown: "The access token is invalid"
scopes: scopes:
api: Access your API api: Access your API
read_user: Read user information
flash: flash:
applications: applications:
......
# The default needs to be `[]`, but all existing access tokens need to have `scopes` set to `['api']`.
# It's easier to achieve this by adding the column with the `['api']` default, and then changing the default to
# `[]`.
class AddColumnScopesToPersonalAccessTokens < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
add_column_with_default :personal_access_tokens, :scopes, :string, default: ['api'].to_yaml
end
def down
remove_column :personal_access_tokens, :scopes
end
end
# The default needs to be `[]`, but all existing access tokens need to have `scopes` set to `['api']`.
# It's easier to achieve this by adding the column with the `['api']` default (regular migration), and
# then changing the default to `[]` (in this post-migration).
#
# Details: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5951#note_19721973
class ChangePersonalAccessTokensDefaultBackToEmptyArray < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def up
change_column_default :personal_access_tokens, :scopes, [].to_yaml
end
def down
change_column_default :personal_access_tokens, :scopes, ['api'].to_yaml
end
end
...@@ -854,6 +854,7 @@ ActiveRecord::Schema.define(version: 20161213172958) do ...@@ -854,6 +854,7 @@ ActiveRecord::Schema.define(version: 20161213172958) do
t.datetime "expires_at" t.datetime "expires_at"
t.datetime "created_at", null: false t.datetime "created_at", null: false
t.datetime "updated_at", null: false t.datetime "updated_at", null: false
t.string "scopes", default: "--- []\n", null: false
end end
add_index "personal_access_tokens", ["token"], name: "index_personal_access_tokens_on_token", unique: true, using: :btree add_index "personal_access_tokens", ["token"], name: "index_personal_access_tokens_on_token", unique: true, using: :btree
......
...@@ -3,6 +3,8 @@ module API ...@@ -3,6 +3,8 @@ module API
include APIGuard include APIGuard
version 'v3', using: :path version 'v3', using: :path
before { allow_access_with_scope :api }
rescue_from Gitlab::Access::AccessDeniedError do rescue_from Gitlab::Access::AccessDeniedError do
rack_response({ 'message' => '403 Forbidden' }.to_json, 403) rack_response({ 'message' => '403 Forbidden' }.to_json, 403)
end end
......
...@@ -6,6 +6,9 @@ module API ...@@ -6,6 +6,9 @@ module API
module APIGuard module APIGuard
extend ActiveSupport::Concern extend ActiveSupport::Concern
PRIVATE_TOKEN_HEADER = "HTTP_PRIVATE_TOKEN"
PRIVATE_TOKEN_PARAM = :private_token
included do |base| included do |base|
# OAuth2 Resource Server Authentication # OAuth2 Resource Server Authentication
use Rack::OAuth2::Server::Resource::Bearer, 'The API' do |request| use Rack::OAuth2::Server::Resource::Bearer, 'The API' do |request|
...@@ -44,27 +47,60 @@ module API ...@@ -44,27 +47,60 @@ module API
access_token = find_access_token access_token = find_access_token
return nil unless access_token return nil unless access_token
case validate_access_token(access_token, scopes) case AccessTokenValidationService.new(access_token).validate(scopes: scopes)
when Oauth2::AccessTokenValidationService::INSUFFICIENT_SCOPE when AccessTokenValidationService::INSUFFICIENT_SCOPE
raise InsufficientScopeError.new(scopes) raise InsufficientScopeError.new(scopes)
when Oauth2::AccessTokenValidationService::EXPIRED when AccessTokenValidationService::EXPIRED
raise ExpiredError raise ExpiredError
when Oauth2::AccessTokenValidationService::REVOKED when AccessTokenValidationService::REVOKED
raise RevokedError raise RevokedError
when Oauth2::AccessTokenValidationService::VALID when AccessTokenValidationService::VALID
@current_user = User.find(access_token.resource_owner_id) @current_user = User.find(access_token.resource_owner_id)
end end
end end
def find_user_by_private_token(scopes: [])
token_string = (params[PRIVATE_TOKEN_PARAM] || env[PRIVATE_TOKEN_HEADER]).to_s
return nil unless token_string.present?
find_user_by_authentication_token(token_string) || find_user_by_personal_access_token(token_string, scopes)
end
def current_user def current_user
@current_user @current_user
end end
# Set the authorization scope(s) allowed for the current request.
#
# Note: A call to this method adds to any previous scopes in place. This is done because
# `Grape` callbacks run from the outside-in: the top-level callback (API::API) runs first, then
# the next-level callback (API::API::Users, for example) runs. All these scopes are valid for the
# given endpoint (GET `/api/users` is accessible by the `api` and `read_user` scopes), and so they
# need to be stored.
def allow_access_with_scope(*scopes)
@scopes ||= []
@scopes.concat(scopes.map(&:to_s))
end
private private
def find_user_by_authentication_token(token_string)
User.find_by_authentication_token(token_string)
end
def find_user_by_personal_access_token(token_string, scopes)
access_token = PersonalAccessToken.active.find_by_token(token_string)
return unless access_token
if AccessTokenValidationService.new(access_token).include_any_scope?(scopes)
User.find(access_token.user_id)
end
end
def find_access_token def find_access_token
@access_token ||= Doorkeeper.authenticate(doorkeeper_request, Doorkeeper.configuration.access_token_methods) @access_token ||= Doorkeeper.authenticate(doorkeeper_request, Doorkeeper.configuration.access_token_methods)
end end
...@@ -72,10 +108,6 @@ module API ...@@ -72,10 +108,6 @@ module API
def doorkeeper_request def doorkeeper_request
@doorkeeper_request ||= ActionDispatch::Request.new(env) @doorkeeper_request ||= ActionDispatch::Request.new(env)
end end
def validate_access_token(access_token, scopes)
Oauth2::AccessTokenValidationService.validate(access_token, scopes: scopes)
end
end end
module ClassMethods module ClassMethods
......
...@@ -2,8 +2,6 @@ module API ...@@ -2,8 +2,6 @@ module API
module Helpers module Helpers
include Gitlab::Utils include Gitlab::Utils
PRIVATE_TOKEN_HEADER = "HTTP_PRIVATE_TOKEN"
PRIVATE_TOKEN_PARAM = :private_token
SUDO_HEADER = "HTTP_SUDO" SUDO_HEADER = "HTTP_SUDO"
SUDO_PARAM = :sudo SUDO_PARAM = :sudo
...@@ -308,7 +306,7 @@ module API ...@@ -308,7 +306,7 @@ module API
private private
def private_token def private_token
params[PRIVATE_TOKEN_PARAM] || env[PRIVATE_TOKEN_HEADER] params[APIGuard::PRIVATE_TOKEN_PARAM] || env[APIGuard::PRIVATE_TOKEN_HEADER]
end end
def warden def warden
...@@ -323,18 +321,11 @@ module API ...@@ -323,18 +321,11 @@ module API
warden.try(:authenticate) if %w[GET HEAD].include?(env['REQUEST_METHOD']) warden.try(:authenticate) if %w[GET HEAD].include?(env['REQUEST_METHOD'])
end end
def find_user_by_private_token
token = private_token
return nil unless token.present?
User.find_by_authentication_token(token) || User.find_by_personal_access_token(token)
end
def initial_current_user def initial_current_user
return @initial_current_user if defined?(@initial_current_user) return @initial_current_user if defined?(@initial_current_user)
@initial_current_user ||= find_user_by_private_token @initial_current_user ||= find_user_by_private_token(scopes: @scopes)
@initial_current_user ||= doorkeeper_guard @initial_current_user ||= doorkeeper_guard(scopes: @scopes)
@initial_current_user ||= find_user_from_warden @initial_current_user ||= find_user_from_warden
unless @initial_current_user && Gitlab::UserAccess.new(@initial_current_user).allowed? unless @initial_current_user && Gitlab::UserAccess.new(@initial_current_user).allowed?
......
...@@ -2,7 +2,10 @@ module API ...@@ -2,7 +2,10 @@ module API
class Users < Grape::API class Users < Grape::API
include PaginationParams include PaginationParams
before { authenticate! } before do
allow_access_with_scope :read_user if request.get?
authenticate!
end
resource :users, requirements: { uid: /[0-9]*/, id: /[0-9]*/ } do resource :users, requirements: { uid: /[0-9]*/, id: /[0-9]*/ } do
helpers do helpers do
......
...@@ -2,6 +2,10 @@ module Gitlab ...@@ -2,6 +2,10 @@ module Gitlab
module Auth module Auth
class MissingPersonalTokenError < StandardError; end class MissingPersonalTokenError < StandardError; end
SCOPES = [:api, :read_user]
DEFAULT_SCOPES = [:api]
OPTIONAL_SCOPES = SCOPES - DEFAULT_SCOPES
class << self class << self
def find_for_git_client(login, password, project:, ip:) def find_for_git_client(login, password, project:, ip:)
raise "Must provide an IP for rate limiting" if ip.nil? raise "Must provide an IP for rate limiting" if ip.nil?
...@@ -88,7 +92,7 @@ module Gitlab ...@@ -88,7 +92,7 @@ module Gitlab
def oauth_access_token_check(login, password) def oauth_access_token_check(login, password)
if login == "oauth2" && password.present? if login == "oauth2" && password.present?
token = Doorkeeper::AccessToken.by_token(password) token = Doorkeeper::AccessToken.by_token(password)
if token && token.accessible? if valid_oauth_token?(token)
user = User.find_by(id: token.resource_owner_id) user = User.find_by(id: token.resource_owner_id)
Gitlab::Auth::Result.new(user, nil, :oauth, read_authentication_abilities) Gitlab::Auth::Result.new(user, nil, :oauth, read_authentication_abilities)
end end
...@@ -97,12 +101,27 @@ module Gitlab ...@@ -97,12 +101,27 @@ module Gitlab
def personal_access_token_check(login, password) def personal_access_token_check(login, password)
if login && password if login && password
user = User.find_by_personal_access_token(password) token = PersonalAccessToken.active.find_by_token(password)
validation = User.by_login(login) validation = User.by_login(login)
Gitlab::Auth::Result.new(user, nil, :personal_token, full_authentication_abilities) if user.present? && user == validation
if valid_personal_access_token?(token, validation)
Gitlab::Auth::Result.new(validation, nil, :personal_token, full_authentication_abilities)
end
end end
end end
def valid_oauth_token?(token)
token && token.accessible? && valid_api_token?(token)
end
def valid_personal_access_token?(token, user)
token && token.user == user && valid_api_token?(token)
end
def valid_api_token?(token)
AccessTokenValidationService.new(token).include_any_scope?(['api'])
end
def lfs_token_check(login, password) def lfs_token_check(login, password)
deploy_key_matches = login.match(/\Alfs\+deploy-key-(\d+)\z/) deploy_key_matches = login.match(/\Alfs\+deploy-key-(\d+)\z/)
......
require 'spec_helper'
describe Profiles::PersonalAccessTokensController do
let(:user) { create(:user) }
describe '#create' do
def created_token
PersonalAccessToken.order(:created_at).last
end
before { sign_in(user) }
it "allows creation of a token" do
name = FFaker::Product.brand
post :create, personal_access_token: { name: name }
expect(created_token).not_to be_nil
expect(created_token.name).to eq(name)
expect(created_token.expires_at).to be_nil
expect(PersonalAccessToken.active).to include(created_token)
end
it "allows creation of a token with an expiry date" do
expires_at = 5.days.from_now
post :create, personal_access_token: { name: FFaker::Product.brand, expires_at: expires_at }
expect(created_token).not_to be_nil
expect(created_token.expires_at.to_i).to eq(expires_at.to_i)
end
context "scopes" do
it "allows creation of a token with scopes" do
post :create, personal_access_token: { name: FFaker::Product.brand, scopes: ['api', 'read_user'] }
expect(created_token).not_to be_nil
expect(created_token.scopes).to eq(['api', 'read_user'])
end
it "allows creation of a token with no scopes" do
post :create, personal_access_token: { name: FFaker::Product.brand, scopes: [] }
expect(created_token).not_to be_nil
expect(created_token.scopes).to eq([])
end
end
end
end
...@@ -5,5 +5,6 @@ FactoryGirl.define do ...@@ -5,5 +5,6 @@ FactoryGirl.define do
name { FFaker::Product.brand } name { FFaker::Product.brand }
revoked false revoked false
expires_at { 5.days.from_now } expires_at { 5.days.from_now }
scopes ['api']
end end
end end
...@@ -27,28 +27,25 @@ describe 'Profile > Personal Access Tokens', feature: true, js: true do ...@@ -27,28 +27,25 @@ describe 'Profile > Personal Access Tokens', feature: true, js: true do
describe "token creation" do describe "token creation" do
it "allows creation of a token" do it "allows creation of a token" do
visit profile_personal_access_tokens_path name = FFaker::Product.brand
fill_in "Name", with: FFaker::Product.brand
expect {click_on "Create Personal Access Token"}.to change { PersonalAccessToken.count }.by(1)
expect(created_personal_access_token).to eq(PersonalAccessToken.last.token)
expect(active_personal_access_tokens).to have_text(PersonalAccessToken.last.name)
expect(active_personal_access_tokens).to have_text("Never")
end
it "allows creation of a token with an expiry date" do
visit profile_personal_access_tokens_path visit profile_personal_access_tokens_path
fill_in "Name", with: FFaker::Product.brand fill_in "Name", with: name
# Set date to 1st of next month # Set date to 1st of next month
find_field("Expires at").trigger('focus') find_field("Expires at").trigger('focus')
find("a[title='Next']").click find("a[title='Next']").click
click_on "1" click_on "1"
expect {click_on "Create Personal Access Token"}.to change { PersonalAccessToken.count }.by(1) # Scopes
expect(created_personal_access_token).to eq(PersonalAccessToken.last.token) check "api"
expect(active_personal_access_tokens).to have_text(PersonalAccessToken.last.name) check "read_user"
click_on "Create Personal Access Token"
expect(active_personal_access_tokens).to have_text(name)
expect(active_personal_access_tokens).to have_text(Date.today.next_month.at_beginning_of_month.to_s(:medium)) expect(active_personal_access_tokens).to have_text(Date.today.next_month.at_beginning_of_month.to_s(:medium))
expect(active_personal_access_tokens).to have_text('api')
expect(active_personal_access_tokens).to have_text('read_user')
end end
context "when creation fails" do context "when creation fails" do
...@@ -85,7 +82,7 @@ describe 'Profile > Personal Access Tokens', feature: true, js: true do ...@@ -85,7 +82,7 @@ describe 'Profile > Personal Access Tokens', feature: true, js: true do
disallow_personal_access_token_saves! disallow_personal_access_token_saves!
visit profile_personal_access_tokens_path visit profile_personal_access_tokens_path
expect { click_on "Revoke" }.not_to change { PersonalAccessToken.inactive.count } click_on "Revoke"
expect(active_personal_access_tokens).to have_text(personal_access_token.name) expect(active_personal_access_tokens).to have_text(personal_access_token.name)
expect(page).to have_content("Could not revoke") expect(page).to have_content("Could not revoke")
end end
......
...@@ -47,54 +47,76 @@ describe Gitlab::Auth, lib: true do ...@@ -47,54 +47,76 @@ describe Gitlab::Auth, lib: true do
project.create_drone_ci_service(active: true) project.create_drone_ci_service(active: true)
project.drone_ci_service.update(token: 'token') project.drone_ci_service.update(token: 'token')
ip = 'ip' expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: 'drone-ci-token')
expect(gl_auth.find_for_git_client('drone-ci-token', 'token', project: project, ip: 'ip')).to eq(Gitlab::Auth::Result.new(nil, project, :ci, build_authentication_abilities))
expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: 'drone-ci-token')
expect(gl_auth.find_for_git_client('drone-ci-token', 'token', project: project, ip: ip)).to eq(Gitlab::Auth::Result.new(nil, project, :ci, build_authentication_abilities))
end end
it 'recognizes master passwords' do it 'recognizes master passwords' do
user = create(:user, password: 'password') user = create(:user, password: 'password')
ip = 'ip'
expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: user.username) expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: user.username)
expect(gl_auth.find_for_git_client(user.username, 'password', project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, nil, :gitlab_or_ldap, full_authentication_abilities)) expect(gl_auth.find_for_git_client(user.username, 'password', project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(user, nil, :gitlab_or_ldap, full_authentication_abilities))
end end
it 'recognizes user lfs tokens' do it 'recognizes user lfs tokens' do
user = create(:user) user = create(:user)
ip = 'ip'
token = Gitlab::LfsToken.new(user).token token = Gitlab::LfsToken.new(user).token
expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: user.username) expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: user.username)
expect(gl_auth.find_for_git_client(user.username, token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, nil, :lfs_token, full_authentication_abilities)) expect(gl_auth.find_for_git_client(user.username, token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(user, nil, :lfs_token, full_authentication_abilities))
end end
it 'recognizes deploy key lfs tokens' do it 'recognizes deploy key lfs tokens' do
key = create(:deploy_key) key = create(:deploy_key)
ip = 'ip'
token = Gitlab::LfsToken.new(key).token token = Gitlab::LfsToken.new(key).token
expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: "lfs+deploy-key-#{key.id}") expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: "lfs+deploy-key-#{key.id}")
expect(gl_auth.find_for_git_client("lfs+deploy-key-#{key.id}", token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(key, nil, :lfs_deploy_token, read_authentication_abilities)) expect(gl_auth.find_for_git_client("lfs+deploy-key-#{key.id}", token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(key, nil, :lfs_deploy_token, read_authentication_abilities))
end end
it 'recognizes OAuth tokens' do context "while using OAuth tokens as passwords" do
user = create(:user) it 'succeeds for OAuth tokens with the `api` scope' do
application = Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user) user = create(:user)
token = Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id) application = Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user)
ip = 'ip' token = Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: "api")
expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: 'oauth2')
expect(gl_auth.find_for_git_client("oauth2", token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(user, nil, :oauth, read_authentication_abilities))
end
expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: 'oauth2') it 'fails for OAuth tokens with other scopes' do
expect(gl_auth.find_for_git_client("oauth2", token.token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, nil, :oauth, read_authentication_abilities)) user = create(:user)
application = Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user)
token = Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: "read_user")
expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: 'oauth2')
expect(gl_auth.find_for_git_client("oauth2", token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(nil, nil))
end
end
context "while using personal access tokens as passwords" do
it 'succeeds for personal access tokens with the `api` scope' do
user = create(:user)
personal_access_token = create(:personal_access_token, user: user, scopes: ['api'])
expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: user.email)
expect(gl_auth.find_for_git_client(user.email, personal_access_token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(user, nil, :personal_token, full_authentication_abilities))
end
it 'fails for personal access tokens with other scopes' do
user = create(:user)
personal_access_token = create(:personal_access_token, user: user, scopes: ['read_user'])
expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: user.email)
expect(gl_auth.find_for_git_client(user.email, personal_access_token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(nil, nil))
end
end end
it 'returns double nil for invalid credentials' do it 'returns double nil for invalid credentials' do
login = 'foo' login = 'foo'
ip = 'ip'
expect(gl_auth).to receive(:rate_limit!).with(ip, success: false, login: login) expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: login)
expect(gl_auth.find_for_git_client(login, 'bar', project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new) expect(gl_auth.find_for_git_client(login, 'bar', project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new)
end end
end end
......
...@@ -5,7 +5,7 @@ describe API::API, api: true do ...@@ -5,7 +5,7 @@ describe API::API, api: true do
let!(:user) { create(:user) } let!(:user) { create(:user) }
let!(:application) { Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user) } let!(:application) { Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user) }
let!(:token) { Doorkeeper::AccessToken.create! application_id: application.id, resource_owner_id: user.id } let!(:token) { Doorkeeper::AccessToken.create! application_id: application.id, resource_owner_id: user.id, scopes: "api" }
describe "when unauthenticated" do describe "when unauthenticated" do
it "returns authentication success" do it "returns authentication success" do
......
require 'spec_helper' require 'spec_helper'
describe API::Helpers, api: true do describe API::Helpers, api: true do
include API::APIGuard::HelperMethods
include API::Helpers include API::Helpers
include SentryHelper include SentryHelper
...@@ -15,24 +16,24 @@ describe API::Helpers, api: true do ...@@ -15,24 +16,24 @@ describe API::Helpers, api: true do
def set_env(user_or_token, identifier) def set_env(user_or_token, identifier)
clear_env clear_env
clear_param clear_param
env[API::Helpers::PRIVATE_TOKEN_HEADER] = user_or_token.respond_to?(:private_token) ? user_or_token.private_token : user_or_token env[API::APIGuard::PRIVATE_TOKEN_HEADER] = user_or_token.respond_to?(:private_token) ? user_or_token.private_token : user_or_token
env[API::Helpers::SUDO_HEADER] = identifier.to_s env[API::Helpers::SUDO_HEADER] = identifier.to_s
end end
def set_param(user_or_token, identifier) def set_param(user_or_token, identifier)
clear_env clear_env
clear_param clear_param
params[API::Helpers::PRIVATE_TOKEN_PARAM] = user_or_token.respond_to?(:private_token) ? user_or_token.private_token : user_or_token params[API::APIGuard::PRIVATE_TOKEN_PARAM] = user_or_token.respond_to?(:private_token) ? user_or_token.private_token : user_or_token
params[API::Helpers::SUDO_PARAM] = identifier.to_s params[API::Helpers::SUDO_PARAM] = identifier.to_s
end end
def clear_env def clear_env
env.delete(API::Helpers::PRIVATE_TOKEN_HEADER) env.delete(API::APIGuard::PRIVATE_TOKEN_HEADER)
env.delete(API::Helpers::SUDO_HEADER) env.delete(API::Helpers::SUDO_HEADER)
end end
def clear_param def clear_param
params.delete(API::Helpers::PRIVATE_TOKEN_PARAM) params.delete(API::APIGuard::PRIVATE_TOKEN_PARAM)
params.delete(API::Helpers::SUDO_PARAM) params.delete(API::Helpers::SUDO_PARAM)
end end
...@@ -94,22 +95,28 @@ describe API::Helpers, api: true do ...@@ -94,22 +95,28 @@ describe API::Helpers, api: true do
describe "when authenticating using a user's private token" do describe "when authenticating using a user's private token" do
it "returns nil for an invalid token" do it "returns nil for an invalid token" do
env[API::Helpers::PRIVATE_TOKEN_HEADER] = 'invalid token' env[API::APIGuard::PRIVATE_TOKEN_HEADER] = 'invalid token'
allow_any_instance_of(self.class).to receive(:doorkeeper_guard){ false } allow_any_instance_of(self.class).to receive(:doorkeeper_guard){ false }
expect(current_user).to be_nil expect(current_user).to be_nil
end end
it "returns nil for a user without access" do it "returns nil for a user without access" do
env[API::Helpers::PRIVATE_TOKEN_HEADER] = user.private_token env[API::APIGuard::PRIVATE_TOKEN_HEADER] = user.private_token
allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(false) allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(false)
expect(current_user).to be_nil expect(current_user).to be_nil
end end
it "leaves user as is when sudo not specified" do it "leaves user as is when sudo not specified" do
env[API::Helpers::PRIVATE_TOKEN_HEADER] = user.private_token env[API::APIGuard::PRIVATE_TOKEN_HEADER] = user.private_token
expect(current_user).to eq(user) expect(current_user).to eq(user)
clear_env clear_env
params[API::Helpers::PRIVATE_TOKEN_PARAM] = user.private_token
params[API::APIGuard::PRIVATE_TOKEN_PARAM] = user.private_token
expect(current_user).to eq(user) expect(current_user).to eq(user)
end end
end end
...@@ -117,37 +124,51 @@ describe API::Helpers, api: true do ...@@ -117,37 +124,51 @@ describe API::Helpers, api: true do
describe "when authenticating using a user's personal access tokens" do describe "when authenticating using a user's personal access tokens" do
let(:personal_access_token) { create(:personal_access_token, user: user) } let(:personal_access_token) { create(:personal_access_token, user: user) }
before do
allow_any_instance_of(self.class).to receive(:doorkeeper_guard) { false }
end
it "returns nil for an invalid token" do it "returns nil for an invalid token" do
env[API::Helpers::PRIVATE_TOKEN_HEADER] = 'invalid token' env[API::APIGuard::PRIVATE_TOKEN_HEADER] = 'invalid token'
allow_any_instance_of(self.class).to receive(:doorkeeper_guard){ false }
expect(current_user).to be_nil expect(current_user).to be_nil
end end
it "returns nil for a user without access" do it "returns nil for a user without access" do
env[API::Helpers::PRIVATE_TOKEN_HEADER] = personal_access_token.token env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token
allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(false) allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(false)
expect(current_user).to be_nil
end
it "returns nil for a token without the appropriate scope" do
personal_access_token = create(:personal_access_token, user: user, scopes: ['read_user'])
env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token
allow_access_with_scope('write_user')
expect(current_user).to be_nil expect(current_user).to be_nil
end end
it "leaves user as is when sudo not specified" do it "leaves user as is when sudo not specified" do
env[API::Helpers::PRIVATE_TOKEN_HEADER] = personal_access_token.token env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token
expect(current_user).to eq(user) expect(current_user).to eq(user)
clear_env clear_env
params[API::Helpers::PRIVATE_TOKEN_PARAM] = personal_access_token.token params[API::APIGuard::PRIVATE_TOKEN_PARAM] = personal_access_token.token
expect(current_user).to eq(user) expect(current_user).to eq(user)
end end
it 'does not allow revoked tokens' do it 'does not allow revoked tokens' do
personal_access_token.revoke! personal_access_token.revoke!
env[API::Helpers::PRIVATE_TOKEN_HEADER] = personal_access_token.token env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token
allow_any_instance_of(self.class).to receive(:doorkeeper_guard){ false }
expect(current_user).to be_nil expect(current_user).to be_nil
end end
it 'does not allow expired tokens' do it 'does not allow expired tokens' do
personal_access_token.update_attributes!(expires_at: 1.day.ago) personal_access_token.update_attributes!(expires_at: 1.day.ago)
env[API::Helpers::PRIVATE_TOKEN_HEADER] = personal_access_token.token env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token
allow_any_instance_of(self.class).to receive(:doorkeeper_guard){ false }
expect(current_user).to be_nil expect(current_user).to be_nil
end end
end end
......
...@@ -230,7 +230,7 @@ describe 'Git HTTP requests', lib: true do ...@@ -230,7 +230,7 @@ describe 'Git HTTP requests', lib: true do
context "when an oauth token is provided" do context "when an oauth token is provided" do
before do before do
application = Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user) application = Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user)
@token = Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id) @token = Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: "api")
end end
it "downloads get status 200" do it "downloads get status 200" do
......
require 'spec_helper'
describe AccessTokenValidationService, services: true do
describe ".include_any_scope?" do
it "returns true if the required scope is present in the token's scopes" do
token = double("token", scopes: [:api, :read_user])
expect(described_class.new(token).include_any_scope?([:api])).to be(true)
end
it "returns true if more than one of the required scopes is present in the token's scopes" do
token = double("token", scopes: [:api, :read_user, :other_scope])
expect(described_class.new(token).include_any_scope?([:api, :other_scope])).to be(true)
end
it "returns true if the list of required scopes is an exact match for the token's scopes" do
token = double("token", scopes: [:api, :read_user, :other_scope])
expect(described_class.new(token).include_any_scope?([:api, :read_user, :other_scope])).to be(true)
end
it "returns true if the list of required scopes contains all of the token's scopes, in addition to others" do
token = double("token", scopes: [:api, :read_user])
expect(described_class.new(token).include_any_scope?([:api, :read_user, :other_scope])).to be(true)
end
it 'returns true if the list of required scopes is blank' do
token = double("token", scopes: [])
expect(described_class.new(token).include_any_scope?([])).to be(true)
end
it "returns false if there are no scopes in common between the required scopes and the token scopes" do
token = double("token", scopes: [:api, :read_user])
expect(described_class.new(token).include_any_scope?([:other_scope])).to be(false)
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment