diff --git a/app/models/personal_access_token.rb b/app/models/personal_access_token.rb index f3e38aba7c9..df8a0612b18 100644 --- a/app/models/personal_access_token.rb +++ b/app/models/personal_access_token.rb @@ -9,7 +9,8 @@ class PersonalAccessToken < ActiveRecord::Base scope :active, -> { where(revoked: false).where("expires_at >= NOW() OR expires_at IS NULL") } scope :inactive, -> { where("revoked = true OR expires_at < NOW()") } - validate :validate_scopes + validates :scopes, presence: true + validate :validate_api_scopes def self.generate(params) personal_access_token = self.new(params) @@ -24,8 +25,8 @@ class PersonalAccessToken < ActiveRecord::Base protected - def validate_scopes - unless Set.new(scopes.map(&:to_sym)).subset?(Set.new(Gitlab::Auth::API_SCOPES)) + def validate_api_scopes + unless scopes.all? { |scope| Gitlab::Auth::API_SCOPES.include?(scope.to_sym) } errors.add :scopes, "can only contain API scopes" end end diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index 92fe770728b..53edc0a9e2c 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -5,10 +5,13 @@ module Gitlab # Scopes used for GitLab API access API_SCOPES = [:api, :read_user].freeze - # Scopes used by doorkeeper-openid_connect + # Scopes used for OpenID Connect OPENID_SCOPES = [:openid].freeze + # Default scopes for OAuth applications that don't define their own DEFAULT_SCOPES = [:api].freeze + + # Other available scopes OPTIONAL_SCOPES = (API_SCOPES + OPENID_SCOPES - DEFAULT_SCOPES).freeze class << self diff --git a/spec/controllers/profiles/personal_access_tokens_spec.rb b/spec/controllers/profiles/personal_access_tokens_spec.rb index 9d5f4c99f6d..19572ce53b7 100644 --- a/spec/controllers/profiles/personal_access_tokens_spec.rb +++ b/spec/controllers/profiles/personal_access_tokens_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe Profiles::PersonalAccessTokensController do let(:user) { create(:user) } + let(:token_attributes) { attributes_for(:personal_access_token) } describe '#create' do def created_token @@ -10,40 +11,24 @@ describe Profiles::PersonalAccessTokensController do before { sign_in(user) } - it "allows creation of a token" do - name = FFaker::Product.brand + it "allows creation of a token with scopes" do + scopes = %w[api read_user] - post :create, personal_access_token: { name: name } + post :create, personal_access_token: token_attributes.merge(scopes: scopes) expect(created_token).not_to be_nil - expect(created_token.name).to eq(name) - expect(created_token.expires_at).to be_nil + expect(created_token.name).to eq(token_attributes[:name]) + expect(created_token.scopes).to eq(scopes) 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 } + post :create, personal_access_token: token_attributes.merge(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: %w(api read_user) } - - expect(created_token).not_to be_nil - expect(created_token.scopes).to eq(%w(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 diff --git a/spec/models/personal_access_token_spec.rb b/spec/models/personal_access_token_spec.rb index 4cc9cf02e6d..50f61ec18fd 100644 --- a/spec/models/personal_access_token_spec.rb +++ b/spec/models/personal_access_token_spec.rb @@ -13,19 +13,27 @@ describe PersonalAccessToken, models: true do end end - describe 'validate_scopes' do + context "validations" do + let(:personal_access_token) { build(:personal_access_token) } + + it "requires at least one scope" do + personal_access_token.scopes = [] + + expect(personal_access_token).not_to be_valid + expect(personal_access_token.errors[:scopes].first).to eq "can't be blank" + end + it "allows creating a token with API scopes" do - personal_access_token = build(:personal_access_token) personal_access_token.scopes = [:api, :read_user] expect(personal_access_token).to be_valid end it "rejects creating a token with non-API scopes" do - personal_access_token = build(:personal_access_token) personal_access_token.scopes = [:openid, :api] expect(personal_access_token).not_to be_valid + expect(personal_access_token.errors[:scopes].first).to eq "can only contain API scopes" end end end