Merge branch 'dz-allow-nested-group-routing' into 'master'
Add nested groups support to the routing ## What does this MR do? It allows routing with `/` in namespace name ## Why was this MR needed? For nested groups feature(https://gitlab.com/gitlab-org/gitlab-ce/issues/2772). We need URI like `/group/subgroup/project` be routed correctly ## Does this MR meet the acceptance criteria? - [x] [Changelog entry](https://docs.gitlab.com/ce/development/changelog.html) added - ~~[Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)~~ - ~~API support added~~ - Tests - [x] Added for this feature/bug - [x] All builds are passing - [x] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html) - [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides) - [x] Branch has no merge conflicts with `master` (if it does - rebase it please) - [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits) ## What are the relevant issue numbers? https://gitlab.com/gitlab-org/gitlab-ce/issues/2772 See merge request !7459
This commit is contained in:
commit
a7a9db803b
|
@ -49,6 +49,14 @@ class ApplicationController < ActionController::Base
|
|||
render_404
|
||||
end
|
||||
|
||||
def route_not_found
|
||||
if current_user
|
||||
not_found
|
||||
else
|
||||
redirect_to new_user_session_path
|
||||
end
|
||||
end
|
||||
|
||||
protected
|
||||
|
||||
# This filter handles both private tokens and personal access tokens
|
||||
|
|
|
@ -177,6 +177,7 @@ class Project < ActiveRecord::Base
|
|||
message: Gitlab::Regex.project_name_regex_message }
|
||||
validates :path,
|
||||
presence: true,
|
||||
project_path: true,
|
||||
length: { within: 0..255 },
|
||||
format: { with: Gitlab::Regex.project_path_regex,
|
||||
message: Gitlab::Regex.project_path_regex_message }
|
||||
|
|
|
@ -291,8 +291,12 @@ class User < ActiveRecord::Base
|
|||
end
|
||||
end
|
||||
|
||||
def find_by_username(username)
|
||||
iwhere(username: username).take
|
||||
end
|
||||
|
||||
def find_by_username!(username)
|
||||
find_by!('lower(username) = ?', username.downcase)
|
||||
iwhere(username: username).take!
|
||||
end
|
||||
|
||||
def find_by_personal_access_token(token_string)
|
||||
|
|
|
@ -35,8 +35,22 @@ class NamespaceValidator < ActiveModel::EachValidator
|
|||
users
|
||||
].freeze
|
||||
|
||||
def self.valid?(value)
|
||||
!reserved?(value) && follow_format?(value)
|
||||
end
|
||||
|
||||
def self.reserved?(value)
|
||||
RESERVED.include?(value)
|
||||
end
|
||||
|
||||
def self.follow_format?(value)
|
||||
value =~ Gitlab::Regex.namespace_regex
|
||||
end
|
||||
|
||||
delegate :reserved?, :follow_format?, to: :class
|
||||
|
||||
def validate_each(record, attribute, value)
|
||||
unless value =~ Gitlab::Regex.namespace_regex
|
||||
unless follow_format?(value)
|
||||
record.errors.add(attribute, Gitlab::Regex.namespace_regex_message)
|
||||
end
|
||||
|
||||
|
@ -44,10 +58,4 @@ class NamespaceValidator < ActiveModel::EachValidator
|
|||
record.errors.add(attribute, "#{value} is a reserved name")
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def reserved?(value)
|
||||
RESERVED.include?(value)
|
||||
end
|
||||
end
|
||||
|
|
|
@ -0,0 +1,36 @@
|
|||
# ProjectPathValidator
|
||||
#
|
||||
# Custom validator for GitLab project path values.
|
||||
#
|
||||
# Values are checked for formatting and exclusion from a list of reserved path
|
||||
# names.
|
||||
class ProjectPathValidator < ActiveModel::EachValidator
|
||||
# All project routes with wildcard argument must be listed here.
|
||||
# Otherwise it can lead to routing issues when route considered as project name.
|
||||
#
|
||||
# Example:
|
||||
# /group/project/tree/deploy_keys
|
||||
#
|
||||
# without tree as reserved name routing can match 'group/project' as group name,
|
||||
# 'tree' as project name and 'deploy_keys' as route.
|
||||
#
|
||||
RESERVED = (NamespaceValidator::RESERVED +
|
||||
%w[tree commits wikis new edit create update logs_tree
|
||||
preview blob blame raw files create_dir find_file]).freeze
|
||||
|
||||
def self.valid?(value)
|
||||
!reserved?(value)
|
||||
end
|
||||
|
||||
def self.reserved?(value)
|
||||
RESERVED.include?(value)
|
||||
end
|
||||
|
||||
delegate :reserved?, to: :class
|
||||
|
||||
def validate_each(record, attribute, value)
|
||||
if reserved?(value)
|
||||
record.errors.add(attribute, "#{value} is a reserved name")
|
||||
end
|
||||
end
|
||||
end
|
|
@ -7,7 +7,7 @@
|
|||
= link_to 'Home', namespace_project_wiki_path(@project.namespace, @project, :home)
|
||||
|
||||
= nav_link(path: 'wikis#pages') do
|
||||
= link_to 'Pages', namespace_project_wiki_pages_path(@project.namespace, @project)
|
||||
= link_to 'Pages', namespace_project_wikis_pages_path(@project.namespace, @project)
|
||||
|
||||
= nav_link(path: 'wikis#git_access') do
|
||||
= link_to namespace_project_wikis_git_access_path(@project.namespace, @project) do
|
||||
|
|
|
@ -0,0 +1,4 @@
|
|||
---
|
||||
title: Add nested groups support to the routing
|
||||
merge_request: 7459
|
||||
author:
|
|
@ -1,6 +1,7 @@
|
|||
require 'sidekiq/web'
|
||||
require 'sidekiq/cron/web'
|
||||
require 'api/api'
|
||||
require 'constraints/group_url_constrainer'
|
||||
|
||||
Rails.application.routes.draw do
|
||||
concern :access_requestable do
|
||||
|
@ -78,10 +79,21 @@ Rails.application.routes.draw do
|
|||
draw :user
|
||||
draw :project
|
||||
|
||||
# Get all keys of user
|
||||
get ':username.keys' => 'profiles/keys#get_keys', constraints: { username: /.*/ }
|
||||
|
||||
root to: "root#index"
|
||||
|
||||
get '*unmatched_route', to: 'application#not_found'
|
||||
# Since group show page is wildcard routing
|
||||
# we want all other routing to be checked before matching this one
|
||||
constraints(GroupUrlConstrainer.new) do
|
||||
scope(path: '*id',
|
||||
as: :group,
|
||||
constraints: { id: Gitlab::Regex.namespace_route_regex, format: /(html|json|atom)/ },
|
||||
controller: :groups) do
|
||||
get '/', action: :show
|
||||
patch '/', action: :update
|
||||
put '/', action: :update
|
||||
delete '/', action: :destroy
|
||||
end
|
||||
end
|
||||
|
||||
get '*unmatched_route', to: 'application#route_not_found'
|
||||
end
|
||||
|
|
|
@ -1,37 +1,47 @@
|
|||
scope constraints: { id: /.+\.git/, format: nil } do
|
||||
# Git HTTP clients ('git clone' etc.)
|
||||
get '/info/refs', to: 'git_http#info_refs'
|
||||
post '/git-upload-pack', to: 'git_http#git_upload_pack'
|
||||
post '/git-receive-pack', to: 'git_http#git_receive_pack'
|
||||
scope(path: '*namespace_id/:project_id', constraints: { format: nil }) do
|
||||
scope(constraints: { project_id: Gitlab::Regex.project_git_route_regex }, module: :projects) do
|
||||
# Git HTTP clients ('git clone' etc.)
|
||||
scope(controller: :git_http) do
|
||||
get '/info/refs', action: :info_refs
|
||||
post '/git-upload-pack', action: :git_upload_pack
|
||||
post '/git-receive-pack', action: :git_receive_pack
|
||||
end
|
||||
|
||||
# Git LFS API (metadata)
|
||||
post '/info/lfs/objects/batch', to: 'lfs_api#batch'
|
||||
post '/info/lfs/objects', to: 'lfs_api#deprecated'
|
||||
get '/info/lfs/objects/*oid', to: 'lfs_api#deprecated'
|
||||
# Git LFS API (metadata)
|
||||
scope(path: 'info/lfs/objects', controller: :lfs_api) do
|
||||
post :batch
|
||||
post '/', action: :deprecated
|
||||
get '/*oid', action: :deprecated
|
||||
end
|
||||
|
||||
# GitLab LFS object storage
|
||||
scope constraints: { oid: /[a-f0-9]{64}/ } do
|
||||
get '/gitlab-lfs/objects/*oid', to: 'lfs_storage#download'
|
||||
# GitLab LFS object storage
|
||||
scope(path: 'gitlab-lfs/objects/*oid', controller: :lfs_storage, constraints: { oid: /[a-f0-9]{64}/ }) do
|
||||
get '/', action: :download
|
||||
|
||||
scope constraints: { size: /[0-9]+/ } do
|
||||
put '/gitlab-lfs/objects/*oid/*size/authorize', to: 'lfs_storage#upload_authorize'
|
||||
put '/gitlab-lfs/objects/*oid/*size', to: 'lfs_storage#upload_finalize'
|
||||
scope constraints: { size: /[0-9]+/ } do
|
||||
put '/*size/authorize', action: :upload_authorize
|
||||
put '/*size', action: :upload_finalize
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
# Allow /info/refs, /info/refs?service=git-upload-pack, and
|
||||
# /info/refs?service=git-receive-pack, but nothing else.
|
||||
#
|
||||
git_http_handshake = lambda do |request|
|
||||
request.query_string.blank? ||
|
||||
request.query_string.match(/\Aservice=git-(upload|receive)-pack\z/)
|
||||
end
|
||||
# Redirect /group/project/info/refs to /group/project.git/info/refs
|
||||
scope(constraints: { project_id: Gitlab::Regex.project_route_regex }) do
|
||||
# Allow /info/refs, /info/refs?service=git-upload-pack, and
|
||||
# /info/refs?service=git-receive-pack, but nothing else.
|
||||
#
|
||||
git_http_handshake = lambda do |request|
|
||||
ProjectUrlConstrainer.new.matches?(request) &&
|
||||
(request.query_string.blank? ||
|
||||
request.query_string.match(/\Aservice=git-(upload|receive)-pack\z/))
|
||||
end
|
||||
|
||||
ref_redirect = redirect do |params, request|
|
||||
path = "#{params[:namespace_id]}/#{params[:project_id]}.git/info/refs"
|
||||
path << "?#{request.query_string}" unless request.query_string.blank?
|
||||
path
|
||||
end
|
||||
ref_redirect = redirect do |params, request|
|
||||
path = "#{params[:namespace_id]}/#{params[:project_id]}.git/info/refs"
|
||||
path << "?#{request.query_string}" unless request.query_string.blank?
|
||||
path
|
||||
end
|
||||
|
||||
get '/info/refs', constraints: git_http_handshake, to: ref_redirect
|
||||
get '/info/refs', constraints: git_http_handshake, to: ref_redirect
|
||||
end
|
||||
end
|
||||
|
|
|
@ -1,20 +1,6 @@
|
|||
require 'constraints/group_url_constrainer'
|
||||
|
||||
constraints(GroupUrlConstrainer.new) do
|
||||
scope(path: ':id',
|
||||
as: :group,
|
||||
constraints: { id: Gitlab::Regex.namespace_route_regex },
|
||||
controller: :groups) do
|
||||
get '/', action: :show
|
||||
patch '/', action: :update
|
||||
put '/', action: :update
|
||||
delete '/', action: :destroy
|
||||
end
|
||||
end
|
||||
|
||||
resources :groups, only: [:index, :new, :create]
|
||||
|
||||
scope(path: 'groups/:id',
|
||||
scope(path: 'groups/*id',
|
||||
controller: :groups,
|
||||
constraints: { id: Gitlab::Regex.namespace_route_regex }) do
|
||||
get :edit, as: :edit_group
|
||||
|
@ -24,7 +10,7 @@ scope(path: 'groups/:id',
|
|||
get :activity, as: :activity_group
|
||||
end
|
||||
|
||||
scope(path: 'groups/:group_id',
|
||||
scope(path: 'groups/*group_id',
|
||||
module: :groups,
|
||||
as: :group,
|
||||
constraints: { group_id: Gitlab::Regex.namespace_route_regex }) do
|
||||
|
@ -42,4 +28,4 @@ scope(path: 'groups/:group_id',
|
|||
end
|
||||
|
||||
# Must be last route in this file
|
||||
get 'groups/:id' => 'groups#show', as: :group_canonical, constraints: { id: Gitlab::Regex.namespace_route_regex }
|
||||
get 'groups/*id' => 'groups#show', as: :group_canonical, constraints: { id: Gitlab::Regex.namespace_route_regex }
|
||||
|
|
|
@ -1,28 +1,15 @@
|
|||
resources :projects, constraints: { id: /[^\/]+/ }, only: [:index, :new, :create]
|
||||
require 'constraints/project_url_constrainer'
|
||||
|
||||
resources :namespaces, path: '/', constraints: { id: /[a-zA-Z.0-9_\-]+/ }, only: [] do
|
||||
resources(:projects, constraints: { id: /[a-zA-Z.0-9_\-]+(?<!\.atom)/ }, except:
|
||||
[:new, :create, :index], path: "/") do
|
||||
member do
|
||||
put :transfer
|
||||
delete :remove_fork
|
||||
post :archive
|
||||
post :unarchive
|
||||
post :housekeeping
|
||||
post :toggle_star
|
||||
post :preview_markdown
|
||||
post :export
|
||||
post :remove_export
|
||||
post :generate_new_export
|
||||
get :download_export
|
||||
get :autocomplete_sources
|
||||
get :activity
|
||||
get :refs
|
||||
put :new_issue_address
|
||||
end
|
||||
resources :projects, only: [:index, :new, :create]
|
||||
|
||||
scope module: :projects do
|
||||
draw :git_http
|
||||
draw :git_http
|
||||
|
||||
constraints(ProjectUrlConstrainer.new) do
|
||||
scope(path: '*namespace_id', as: :namespace) do
|
||||
scope(path: ':project_id',
|
||||
constraints: { project_id: Gitlab::Regex.project_route_regex },
|
||||
module: :projects,
|
||||
as: :project) do
|
||||
|
||||
#
|
||||
# Templates
|
||||
|
@ -311,5 +298,28 @@ resources :namespaces, path: '/', constraints: { id: /[a-zA-Z.0-9_\-]+/ }, only:
|
|||
draw :wiki
|
||||
draw :repository
|
||||
end
|
||||
|
||||
resources(:projects,
|
||||
path: '/',
|
||||
constraints: { id: Gitlab::Regex.project_route_regex },
|
||||
only: [:edit, :show, :update, :destroy]) do
|
||||
member do
|
||||
put :transfer
|
||||
delete :remove_fork
|
||||
post :archive
|
||||
post :unarchive
|
||||
post :housekeeping
|
||||
post :toggle_star
|
||||
post :preview_markdown
|
||||
post :export
|
||||
post :remove_export
|
||||
post :generate_new_export
|
||||
get :download_export
|
||||
get :autocomplete_sources
|
||||
get :activity
|
||||
get :refs
|
||||
put :new_issue_address
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -29,82 +29,60 @@ get '/edit/*id', to: 'blob#edit', constraints: { id: /.+/ }, as: 'edit_blob'
|
|||
put '/update/*id', to: 'blob#update', constraints: { id: /.+/ }, as: 'update_blob'
|
||||
post '/preview/*id', to: 'blob#preview', constraints: { id: /.+/ }, as: 'preview_blob'
|
||||
|
||||
scope do
|
||||
get(
|
||||
'/blob/*id/diff',
|
||||
to: 'blob#diff',
|
||||
constraints: { id: /.+/, format: false },
|
||||
as: :blob_diff
|
||||
)
|
||||
get(
|
||||
'/blob/*id',
|
||||
to: 'blob#show',
|
||||
constraints: { id: /.+/, format: false },
|
||||
as: :blob
|
||||
)
|
||||
delete(
|
||||
'/blob/*id',
|
||||
to: 'blob#destroy',
|
||||
constraints: { id: /.+/, format: false }
|
||||
)
|
||||
put(
|
||||
'/blob/*id',
|
||||
to: 'blob#update',
|
||||
constraints: { id: /.+/, format: false }
|
||||
)
|
||||
post(
|
||||
'/blob/*id',
|
||||
to: 'blob#create',
|
||||
constraints: { id: /.+/, format: false }
|
||||
)
|
||||
|
||||
get(
|
||||
'/raw/*id',
|
||||
to: 'raw#show',
|
||||
constraints: { id: /.+/, format: /(html|js)/ },
|
||||
as: :raw
|
||||
)
|
||||
|
||||
get(
|
||||
'/tree/*id',
|
||||
to: 'tree#show',
|
||||
constraints: { id: /.+/, format: /(html|js)/ },
|
||||
as: :tree
|
||||
)
|
||||
|
||||
get(
|
||||
'/find_file/*id',
|
||||
to: 'find_file#show',
|
||||
constraints: { id: /.+/, format: /html/ },
|
||||
as: :find_file
|
||||
)
|
||||
|
||||
get(
|
||||
'/files/*id',
|
||||
to: 'find_file#list',
|
||||
constraints: { id: /(?:[^.]|\.(?!json$))+/, format: /json/ },
|
||||
as: :files
|
||||
)
|
||||
|
||||
post(
|
||||
'/create_dir/*id',
|
||||
to: 'tree#create_dir',
|
||||
constraints: { id: /.+/ },
|
||||
as: 'create_dir'
|
||||
)
|
||||
|
||||
get(
|
||||
'/blame/*id',
|
||||
to: 'blame#show',
|
||||
constraints: { id: /.+/, format: /(html|js)/ },
|
||||
as: :blame
|
||||
)
|
||||
|
||||
# File/dir history
|
||||
get(
|
||||
'/commits/*id',
|
||||
to: 'commits#show',
|
||||
constraints: { id: /.+/, format: false },
|
||||
as: :commits
|
||||
)
|
||||
scope('/blob/*id', as: :blob, controller: :blob, constraints: { id: /.+/, format: false }) do
|
||||
get :diff
|
||||
get '/', action: :show
|
||||
delete '/', action: :destroy
|
||||
post '/', action: :create
|
||||
put '/', action: :update
|
||||
end
|
||||
|
||||
get(
|
||||
'/raw/*id',
|
||||
to: 'raw#show',
|
||||
constraints: { id: /.+/, format: /(html|js)/ },
|
||||
as: :raw
|
||||
)
|
||||
|
||||
get(
|
||||
'/tree/*id',
|
||||
to: 'tree#show',
|
||||
constraints: { id: /.+/, format: /(html|js)/ },
|
||||
as: :tree
|
||||
)
|
||||
|
||||
get(
|
||||
'/find_file/*id',
|
||||
to: 'find_file#show',
|
||||
constraints: { id: /.+/, format: /html/ },
|
||||
as: :find_file
|
||||
)
|
||||
|
||||
get(
|
||||
'/files/*id',
|
||||
to: 'find_file#list',
|
||||
constraints: { id: /(?:[^.]|\.(?!json$))+/, format: /json/ },
|
||||
as: :files
|
||||
)
|
||||
|
||||
post(
|
||||
'/create_dir/*id',
|
||||
to: 'tree#create_dir',
|
||||
constraints: { id: /.+/ },
|
||||
as: 'create_dir'
|
||||
)
|
||||
|
||||
get(
|
||||
'/blame/*id',
|
||||
to: 'blame#show',
|
||||
constraints: { id: /.+/, format: /(html|js)/ },
|
||||
as: :blame
|
||||
)
|
||||
|
||||
# File/dir history
|
||||
get(
|
||||
'/commits/*id',
|
||||
to: 'commits#show',
|
||||
constraints: { id: /.+/, format: false },
|
||||
as: :commits
|
||||
)
|
||||
|
|
|
@ -12,6 +12,9 @@ devise_scope :user do
|
|||
end
|
||||
|
||||
constraints(UserUrlConstrainer.new) do
|
||||
# Get all keys of user
|
||||
get ':username.keys' => 'profiles/keys#get_keys', constraints: { username: Gitlab::Regex.namespace_route_regex }
|
||||
|
||||
scope(path: ':username',
|
||||
as: :user,
|
||||
constraints: { username: Gitlab::Regex.namespace_route_regex },
|
||||
|
|
|
@ -1,16 +1,19 @@
|
|||
WIKI_SLUG_ID = { id: /\S+/ } unless defined? WIKI_SLUG_ID
|
||||
|
||||
scope do
|
||||
# Order matters to give priority to these matches
|
||||
get '/wikis/git_access', to: 'wikis#git_access'
|
||||
get '/wikis/pages', to: 'wikis#pages', as: 'wiki_pages'
|
||||
post '/wikis', to: 'wikis#create'
|
||||
scope(controller: :wikis) do
|
||||
scope(path: 'wikis', as: :wikis) do
|
||||
get :git_access
|
||||
get :pages
|
||||
get '/', to: redirect('/%{namespace_id}/%{project_id}/wikis/home')
|
||||
post '/', to: 'wikis#create'
|
||||
end
|
||||
|
||||
get '/wikis/*id/history', to: 'wikis#history', as: 'wiki_history', constraints: WIKI_SLUG_ID
|
||||
get '/wikis/*id/edit', to: 'wikis#edit', as: 'wiki_edit', constraints: WIKI_SLUG_ID
|
||||
|
||||
get '/wikis/*id', to: 'wikis#show', as: 'wiki', constraints: WIKI_SLUG_ID
|
||||
delete '/wikis/*id', to: 'wikis#destroy', constraints: WIKI_SLUG_ID
|
||||
put '/wikis/*id', to: 'wikis#update', constraints: WIKI_SLUG_ID
|
||||
post '/wikis/*id/preview_markdown', to: 'wikis#preview_markdown', constraints: WIKI_SLUG_ID, as: 'wiki_preview_markdown'
|
||||
scope(path: 'wikis/*id', as: :wiki, constraints: WIKI_SLUG_ID, format: false) do
|
||||
get :edit
|
||||
get :history
|
||||
post :preview_markdown
|
||||
get '/', action: :show
|
||||
put '/', action: :update
|
||||
delete '/', action: :destroy
|
||||
end
|
||||
end
|
||||
|
|
|
@ -2,9 +2,7 @@ class Spinach::Features::Labels < Spinach::FeatureSteps
|
|||
include SharedAuthentication
|
||||
include SharedIssuable
|
||||
include SharedProject
|
||||
include SharedNote
|
||||
include SharedPaths
|
||||
include SharedMarkdown
|
||||
|
||||
step 'And I visit project "Shop" labels page' do
|
||||
visit namespace_project_labels_path(project.namespace, project)
|
||||
|
|
|
@ -1,6 +1,11 @@
|
|||
module SharedDiffNote
|
||||
include Spinach::DSL
|
||||
include RepoHelpers
|
||||
include WaitForAjax
|
||||
|
||||
after do
|
||||
wait_for_ajax if javascript_test?
|
||||
end
|
||||
|
||||
step 'I cancel the diff comment' do
|
||||
page.within(diff_file_selector) do
|
||||
|
|
|
@ -2,6 +2,10 @@ module SharedNote
|
|||
include Spinach::DSL
|
||||
include WaitForAjax
|
||||
|
||||
after do
|
||||
wait_for_ajax if javascript_test?
|
||||
end
|
||||
|
||||
step 'I delete a comment' do
|
||||
page.within('.main-notes-list') do
|
||||
find('.note').hover
|
||||
|
|
|
@ -1,15 +0,0 @@
|
|||
module ConstrainerHelper
|
||||
def extract_resource_path(path)
|
||||
id = path.dup
|
||||
id.sub!(/\A#{relative_url_root}/, '') if relative_url_root
|
||||
id.sub(/\A\/+/, '').sub(/\/+\z/, '').sub(/.atom\z/, '')
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def relative_url_root
|
||||
if defined?(Gitlab::Application.config.relative_url_root)
|
||||
Gitlab::Application.config.relative_url_root
|
||||
end
|
||||
end
|
||||
end
|
|
@ -1,15 +1,17 @@
|
|||
require_relative 'constrainer_helper'
|
||||
|
||||
class GroupUrlConstrainer
|
||||
include ConstrainerHelper
|
||||
|
||||
def matches?(request)
|
||||
id = extract_resource_path(request.path)
|
||||
id = request.params[:id]
|
||||
|
||||
if id =~ Gitlab::Regex.namespace_regex
|
||||
Group.find_by(path: id).present?
|
||||
else
|
||||
false
|
||||
return false unless valid?(id)
|
||||
|
||||
Group.find_by(path: id).present?
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def valid?(id)
|
||||
id.split('/').all? do |namespace|
|
||||
NamespaceValidator.valid?(namespace)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -0,0 +1,13 @@
|
|||
class ProjectUrlConstrainer
|
||||
def matches?(request)
|
||||
namespace_path = request.params[:namespace_id]
|
||||
project_path = request.params[:project_id] || request.params[:id]
|
||||
full_path = namespace_path + '/' + project_path
|
||||
|
||||
unless ProjectPathValidator.valid?(project_path)
|
||||
return false
|
||||
end
|
||||
|
||||
Project.find_with_namespace(full_path).present?
|
||||
end
|
||||
end
|
|
@ -1,15 +1,5 @@
|
|||
require_relative 'constrainer_helper'
|
||||
|
||||
class UserUrlConstrainer
|
||||
include ConstrainerHelper
|
||||
|
||||
def matches?(request)
|
||||
id = extract_resource_path(request.path)
|
||||
|
||||
if id =~ Gitlab::Regex.namespace_regex
|
||||
User.find_by('lower(username) = ?', id.downcase).present?
|
||||
else
|
||||
false
|
||||
end
|
||||
User.find_by_username(request.params[:username]).present?
|
||||
end
|
||||
end
|
||||
|
|
|
@ -8,8 +8,10 @@ module Gitlab
|
|||
# allow non-regex validatiions, etc), `NAMESPACE_REGEX_STR_SIMPLE` serves as a Javascript-compatible version of
|
||||
# `NAMESPACE_REGEX_STR`, with the negative lookbehind assertion removed. This means that the client-side validation
|
||||
# will pass for usernames ending in `.atom` and `.git`, but will be caught by the server-side validation.
|
||||
NAMESPACE_REGEX_STR_SIMPLE = '[a-zA-Z0-9_\.][a-zA-Z0-9_\-\.]*[a-zA-Z0-9_\-]|[a-zA-Z0-9_]'.freeze
|
||||
PATH_REGEX_STR = '[a-zA-Z0-9_\.][a-zA-Z0-9_\-\.]*'.freeze
|
||||
NAMESPACE_REGEX_STR_SIMPLE = PATH_REGEX_STR + '[a-zA-Z0-9_\-]|[a-zA-Z0-9_]'.freeze
|
||||
NAMESPACE_REGEX_STR = '(?:' + NAMESPACE_REGEX_STR_SIMPLE + ')(?<!\.git|\.atom)'.freeze
|
||||
PROJECT_REGEX_STR = PATH_REGEX_STR + '(?<!\.git|\.atom)'.freeze
|
||||
|
||||
def namespace_regex
|
||||
@namespace_regex ||= /\A#{NAMESPACE_REGEX_STR}\z/.freeze
|
||||
|
@ -42,7 +44,15 @@ module Gitlab
|
|||
end
|
||||
|
||||
def project_path_regex
|
||||
@project_path_regex ||= /\A[a-zA-Z0-9_.][a-zA-Z0-9_\-\.]*(?<!\.git|\.atom)\z/.freeze
|
||||
@project_path_regex ||= /\A#{PROJECT_REGEX_STR}\z/.freeze
|
||||
end
|
||||
|
||||
def project_route_regex
|
||||
@project_route_regex ||= /#{PROJECT_REGEX_STR}/.freeze
|
||||
end
|
||||
|
||||
def project_git_route_regex
|
||||
@project_route_git_regex ||= /#{PATH_REGEX_STR}\.git/.freeze
|
||||
end
|
||||
|
||||
def project_path_regex_message
|
||||
|
|
|
@ -1,8 +1,9 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe ApplicationController do
|
||||
let(:user) { create(:user) }
|
||||
|
||||
describe '#check_password_expiration' do
|
||||
let(:user) { create(:user) }
|
||||
let(:controller) { ApplicationController.new }
|
||||
|
||||
it 'redirects if the user is over their password expiry' do
|
||||
|
@ -39,8 +40,6 @@ describe ApplicationController do
|
|||
end
|
||||
end
|
||||
|
||||
let(:user) { create(:user) }
|
||||
|
||||
context "when the 'private_token' param is populated with the private token" do
|
||||
it "logs the user in" do
|
||||
get :index, private_token: user.private_token
|
||||
|
@ -73,7 +72,6 @@ describe ApplicationController do
|
|||
end
|
||||
end
|
||||
|
||||
let(:user) { create(:user) }
|
||||
let(:personal_access_token) { create(:personal_access_token, user: user) }
|
||||
|
||||
context "when the 'personal_access_token' param is populated with the personal access token" do
|
||||
|
@ -100,4 +98,21 @@ describe ApplicationController do
|
|||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#route_not_found' do
|
||||
let(:controller) { ApplicationController.new }
|
||||
|
||||
it 'renders 404 if authenticated' do
|
||||
allow(controller).to receive(:current_user).and_return(user)
|
||||
expect(controller).to receive(:not_found)
|
||||
controller.send(:route_not_found)
|
||||
end
|
||||
|
||||
it 'does redirect to login page if not authenticated' do
|
||||
allow(controller).to receive(:current_user).and_return(nil)
|
||||
expect(controller).to receive(:redirect_to)
|
||||
expect(controller).to receive(:new_user_session_path)
|
||||
controller.send(:route_not_found)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -1,20 +0,0 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe ConstrainerHelper, lib: true do
|
||||
include ConstrainerHelper
|
||||
|
||||
describe '#extract_resource_path' do
|
||||
it { expect(extract_resource_path('/gitlab/')).to eq('gitlab') }
|
||||
it { expect(extract_resource_path('///gitlab//')).to eq('gitlab') }
|
||||
it { expect(extract_resource_path('/gitlab.atom')).to eq('gitlab') }
|
||||
|
||||
context 'relative url' do
|
||||
before do
|
||||
allow(Gitlab::Application.config).to receive(:relative_url_root) { '/gitlab' }
|
||||
end
|
||||
|
||||
it { expect(extract_resource_path('/gitlab/foo')).to eq('foo') }
|
||||
it { expect(extract_resource_path('/foo/bar')).to eq('foo/bar') }
|
||||
end
|
||||
end
|
||||
end
|
|
@ -4,16 +4,20 @@ describe GroupUrlConstrainer, lib: true do
|
|||
let!(:group) { create(:group, path: 'gitlab') }
|
||||
|
||||
describe '#matches?' do
|
||||
context 'root group' do
|
||||
it { expect(subject.matches?(request '/gitlab')).to be_truthy }
|
||||
it { expect(subject.matches?(request '/gitlab.atom')).to be_truthy }
|
||||
it { expect(subject.matches?(request '/gitlab/edit')).to be_falsey }
|
||||
it { expect(subject.matches?(request '/gitlab-ce')).to be_falsey }
|
||||
it { expect(subject.matches?(request '/.gitlab')).to be_falsey }
|
||||
context 'valid request' do
|
||||
let(:request) { build_request(group.path) }
|
||||
|
||||
it { expect(subject.matches?(request)).to be_truthy }
|
||||
end
|
||||
|
||||
context 'invalid request' do
|
||||
let(:request) { build_request('foo') }
|
||||
|
||||
it { expect(subject.matches?(request)).to be_falsey }
|
||||
end
|
||||
end
|
||||
|
||||
def request(path)
|
||||
double(:request, path: path)
|
||||
def build_request(path)
|
||||
double(:request, params: { id: path })
|
||||
end
|
||||
end
|
||||
|
|
|
@ -0,0 +1,32 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe ProjectUrlConstrainer, lib: true do
|
||||
let!(:project) { create(:project) }
|
||||
let!(:namespace) { project.namespace }
|
||||
|
||||
describe '#matches?' do
|
||||
context 'valid request' do
|
||||
let(:request) { build_request(namespace.path, project.path) }
|
||||
|
||||
it { expect(subject.matches?(request)).to be_truthy }
|
||||
end
|
||||
|
||||
context 'invalid request' do
|
||||
context "non-existing project" do
|
||||
let(:request) { build_request('foo', 'bar') }
|
||||
|
||||
it { expect(subject.matches?(request)).to be_falsey }
|
||||
end
|
||||
|
||||
context "project id ending with .git" do
|
||||
let(:request) { build_request(namespace.path, project.path + '.git') }
|
||||
|
||||
it { expect(subject.matches?(request)).to be_falsey }
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def build_request(namespace, project)
|
||||
double(:request, params: { namespace_id: namespace, id: project })
|
||||
end
|
||||
end
|
|
@ -1,16 +1,23 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe UserUrlConstrainer, lib: true do
|
||||
let!(:username) { create(:user, username: 'dz') }
|
||||
let!(:user) { create(:user, username: 'dz') }
|
||||
|
||||
describe '#matches?' do
|
||||
it { expect(subject.matches?(request '/dz')).to be_truthy }
|
||||
it { expect(subject.matches?(request '/dz.atom')).to be_truthy }
|
||||
it { expect(subject.matches?(request '/dz/projects')).to be_falsey }
|
||||
it { expect(subject.matches?(request '/gitlab')).to be_falsey }
|
||||
context 'valid request' do
|
||||
let(:request) { build_request(user.username) }
|
||||
|
||||
it { expect(subject.matches?(request)).to be_truthy }
|
||||
end
|
||||
|
||||
context 'invalid request' do
|
||||
let(:request) { build_request('foo') }
|
||||
|
||||
it { expect(subject.matches?(request)).to be_falsey }
|
||||
end
|
||||
end
|
||||
|
||||
def request(path)
|
||||
double(:request, path: path)
|
||||
def build_request(username)
|
||||
double(:request, params: { username: username })
|
||||
end
|
||||
end
|
||||
|
|
|
@ -752,6 +752,17 @@ describe User, models: true do
|
|||
end
|
||||
end
|
||||
|
||||
describe '.find_by_username' do
|
||||
it 'returns nil if not found' do
|
||||
expect(described_class.find_by_username('JohnDoe')).to be_nil
|
||||
end
|
||||
|
||||
it 'is case-insensitive' do
|
||||
user = create(:user, username: 'JohnDoe')
|
||||
expect(described_class.find_by_username('JOHNDOE')).to eq user
|
||||
end
|
||||
end
|
||||
|
||||
describe '.find_by_username!' do
|
||||
it 'raises RecordNotFound' do
|
||||
expect { described_class.find_by_username!('JohnDoe') }.
|
||||
|
|
File diff suppressed because it is too large
Load Diff
|
@ -9,7 +9,7 @@ require 'spec_helper'
|
|||
# user_calendar_activities GET /u/:username/calendar_activities(.:format)
|
||||
describe UsersController, "routing" do
|
||||
it "to #show" do
|
||||
allow(User).to receive(:find_by).and_return(true)
|
||||
allow_any_instance_of(UserUrlConstrainer).to receive(:matches?).and_return(true)
|
||||
|
||||
expect(get("/User")).to route_to('users#show', username: 'User')
|
||||
end
|
||||
|
@ -195,6 +195,8 @@ describe Profiles::KeysController, "routing" do
|
|||
|
||||
# get all the ssh-keys of a user
|
||||
it "to #get_keys" do
|
||||
allow_any_instance_of(UserUrlConstrainer).to receive(:matches?).and_return(true)
|
||||
|
||||
expect(get("/foo.keys")).to route_to('profiles/keys#get_keys', username: 'foo')
|
||||
end
|
||||
end
|
||||
|
@ -263,13 +265,17 @@ end
|
|||
describe "Groups", "routing" do
|
||||
let(:name) { 'complex.group-namegit' }
|
||||
|
||||
before { allow_any_instance_of(GroupUrlConstrainer).to receive(:matches?).and_return(true) }
|
||||
|
||||
it "to #show" do
|
||||
expect(get("/groups/#{name}")).to route_to('groups#show', id: name)
|
||||
end
|
||||
|
||||
it "also display group#show on the short path" do
|
||||
allow(Group).to receive(:find_by).and_return(true)
|
||||
it "also supports nested groups" do
|
||||
expect(get("/#{name}/#{name}")).to route_to('groups#show', id: "#{name}/#{name}")
|
||||
end
|
||||
|
||||
it "also display group#show on the short path" do
|
||||
expect(get("/#{name}")).to route_to('groups#show', id: name)
|
||||
end
|
||||
|
||||
|
@ -284,6 +290,10 @@ describe "Groups", "routing" do
|
|||
it "to #members" do
|
||||
expect(get("/groups/#{name}/group_members")).to route_to('groups/group_members#index', group_id: name)
|
||||
end
|
||||
|
||||
it "also display group#show with slash in the path" do
|
||||
expect(get('/group/subgroup')).to route_to('groups#show', id: 'group/subgroup')
|
||||
end
|
||||
end
|
||||
|
||||
describe HealthCheckController, 'routing' do
|
||||
|
|
Loading…
Reference in New Issue