Commit f706a973 authored by Timothy Andrew's avatar Timothy Andrew

View-related (and other minor) changes to !5951 based on @rymai's review.

- The `scopes_form` partial can be used in the `admin/applications` view
  as well

- Don't allow partials to access instance variables directly. Instead, pass
  in the instance variables as local variables, and use `local_assigns.fetch`
  to assert that the variables are passed in as expected.

- Change a few instances of `render :partial` to `render`

- Remove an instance of `required: false` in a view, since this is the default

- Inline many instances of a local variable (`ip = 'ip'`) in `auth_spec`
parent f14d423d
...@@ -22,11 +22,7 @@ ...@@ -22,11 +22,7 @@
.form-group .form-group
= f.label :scopes, class: 'col-sm-2 control-label' = f.label :scopes, class: 'col-sm-2 control-label'
.col-sm-10 .col-sm-10
- @scopes.each do |scope| = render 'shared/tokens/scopes_form', prefix: 'doorkeeper_application', token: application, scopes: @scopes
%fieldset
= check_box_tag 'doorkeeper_application[scopes][]', scope, application.scopes.include?(scope), id: "doorkeeper_application_scopes_#{scope}"
= label_tag "doorkeeper_application_scopes_#{scope}", scope
%span= "(#{t(scope, scope: [:doorkeeper, :scopes])})"
.form-actions .form-actions
= f.submit 'Submit', class: "btn btn-save wide" = f.submit 'Submit', class: "btn btn-save wide"
......
...@@ -23,7 +23,7 @@ ...@@ -23,7 +23,7 @@
%div %div
%span.monospace= uri %span.monospace= uri
= render partial: "shared/tokens/scopes_list" = 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'
......
...@@ -19,7 +19,7 @@ ...@@ -19,7 +19,7 @@
.form-group .form-group
= f.label :scopes, class: 'label-light' = f.label :scopes, class: 'label-light'
= render partial: 'shared/tokens/scopes_form', locals: { prefix: 'doorkeeper_application', token: application } = 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"
...@@ -23,7 +23,7 @@ ...@@ -23,7 +23,7 @@
%div %div
%span.monospace= uri %span.monospace= uri
= render partial: "shared/tokens/scopes_list" = 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'
......
= form_for [:profile, @personal_access_token], method: :post, html: { class: 'js-requires-input' } do |f| - personal_access_token = local_assigns.fetch(:personal_access_token)
- scopes = local_assigns.fetch(:scopes)
= form_errors(@personal_access_token) = form_for [:profile, personal_access_token], method: :post, html: { class: 'js-requires-input' } do |f|
= form_errors(personal_access_token)
.form-group .form-group
= f.label :name, class: 'label-light' = f.label :name, class: 'label-light'
...@@ -8,11 +11,11 @@ ...@@ -8,11 +11,11 @@
.form-group .form-group
= f.label :expires_at, class: 'label-light' = f.label :expires_at, class: 'label-light'
= f.text_field :expires_at, class: "datepicker form-control", required: false = f.text_field :expires_at, class: "datepicker form-control"
.form-group .form-group
= f.label :scopes, class: 'label-light' = f.label :scopes, class: 'label-light'
= render partial: 'shared/tokens/scopes_form', locals: { prefix: 'personal_access_token', token: @personal_access_token } = render 'shared/tokens/scopes_form', prefix: 'personal_access_token', token: personal_access_token, scopes: scopes
.prepend-top-default .prepend-top-default
= f.submit 'Create Personal Access Token', class: "btn btn-create" = f.submit 'Create Personal Access Token', class: "btn btn-create"
...@@ -29,7 +29,7 @@ ...@@ -29,7 +29,7 @@
%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.
= render "form" = render "form", personal_access_token: @personal_access_token, scopes: @scopes
%hr %hr
......
- @scopes.each do |scope| - scopes = local_assigns.fetch(:scopes)
- prefix = local_assigns.fetch(:prefix)
- token = local_assigns.fetch(:token)
- scopes.each do |scope|
%fieldset %fieldset
= check_box_tag "#{prefix}[scopes][]", scope, token.scopes.include?(scope), id: "#{prefix}_scopes_#{scope}" = check_box_tag "#{prefix}[scopes][]", scope, token.scopes.include?(scope), id: "#{prefix}_scopes_#{scope}"
= label_tag "#{prefix}_scopes_#{scope}", scope = label_tag "#{prefix}_scopes_#{scope}", scope
......
- if @application.scopes.present? - token = local_assigns.fetch(:token)
%tr
%td - return unless token.scopes.present?
Scopes
%td %tr
%ul.scopes-list.append-bottom-0 %td
- @application.scopes.each do |scope| Scopes
%li %td
%span.scope-name= scope %ul.scopes-list.append-bottom-0
= "(#{t(scope, scope: [:doorkeeper, :scopes])})" - token.scopes.each do |scope|
%li
%span.scope-name= scope
= "(#{t(scope, scope: [:doorkeeper, :scopes])})"
...@@ -47,36 +47,31 @@ describe Gitlab::Auth, lib: true do ...@@ -47,36 +47,31 @@ 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
context "while using OAuth tokens as passwords" do context "while using OAuth tokens as passwords" do
...@@ -84,20 +79,18 @@ describe Gitlab::Auth, lib: true do ...@@ -84,20 +79,18 @@ describe Gitlab::Auth, lib: true do
user = create(:user) user = create(:user)
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, scopes: "api") token = Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: "api")
ip = 'ip'
expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: 'oauth2') 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)) 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 end
it 'fails for OAuth tokens with other scopes' do it 'fails for OAuth tokens with other scopes' do
user = create(:user) user = create(:user)
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, scopes: "read_user") token = Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: "read_user")
ip = 'ip'
expect(gl_auth).to receive(:rate_limit!).with(ip, success: false, login: 'oauth2') 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)) expect(gl_auth.find_for_git_client("oauth2", token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(nil, nil))
end end
end end
...@@ -105,28 +98,25 @@ describe Gitlab::Auth, lib: true do ...@@ -105,28 +98,25 @@ describe Gitlab::Auth, lib: true do
it 'succeeds for personal access tokens with the `api` scope' do it 'succeeds for personal access tokens with the `api` scope' do
user = create(:user) user = create(:user)
personal_access_token = create(:personal_access_token, user: user, scopes: ['api']) personal_access_token = create(:personal_access_token, user: user, scopes: ['api'])
ip = 'ip'
expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: user.email) 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)) 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 end
it 'fails for personal access tokens with other scopes' do it 'fails for personal access tokens with other scopes' do
user = create(:user) user = create(:user)
personal_access_token = create(:personal_access_token, user: user, scopes: ['read_user']) personal_access_token = create(:personal_access_token, user: user, scopes: ['read_user'])
ip = 'ip'
expect(gl_auth).to receive(:rate_limit!).with(ip, success: false, login: user.email) 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)) 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 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
......
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