Commit dc3b4321 authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Merge branch 'group-information-leak' into 'master'

Don't leak private group existence by redirecting from namespace controller to group controller.

See merge request !440
parents 0622b875 a4608a8d
...@@ -50,6 +50,7 @@ v 7.10.0 (unreleased) ...@@ -50,6 +50,7 @@ v 7.10.0 (unreleased)
- API: Add pagination to project events - API: Add pagination to project events
- Don't show commit comment button when user is not signed in. - Don't show commit comment button when user is not signed in.
- Fix admin user projects lists. - Fix admin user projects lists.
- Don't leak private group existence by redirecting from namespace controller to group controller.
v 7.9.0 v 7.9.0
- Send EmailsOnPush email when branch or tag is created or deleted. - Send EmailsOnPush email when branch or tag is created or deleted.
......
...@@ -4,14 +4,22 @@ class NamespacesController < ApplicationController ...@@ -4,14 +4,22 @@ class NamespacesController < ApplicationController
def show def show
namespace = Namespace.find_by(path: params[:id]) namespace = Namespace.find_by(path: params[:id])
unless namespace if namespace
return render_404 if namespace.is_a?(Group)
group = namespace
else
user = namespace.owner
end
end end
if namespace.type == "Group" if user
redirect_to group_path(namespace) redirect_to user_path(user)
elsif group && can?(current_user, :read_group, group)
redirect_to group_path(group)
elsif current_user.nil?
authenticate_user!
else else
redirect_to user_path(namespace.owner) render_404
end end
end end
end end
...@@ -52,7 +52,7 @@ module Mentionable ...@@ -52,7 +52,7 @@ module Mentionable
if identifier == "all" if identifier == "all"
users.push(*project.team.members.flatten) users.push(*project.team.members.flatten)
elsif namespace = Namespace.find_by(path: identifier) elsif namespace = Namespace.find_by(path: identifier)
if namespace.type == "Group" if namespace.is_a?(Group)
users.push(*namespace.users) users.push(*namespace.users)
else else
users << namespace.owner users << namespace.owner
......
...@@ -237,7 +237,7 @@ module Gitlab ...@@ -237,7 +237,7 @@ module Gitlab
link_to("@all", namespace_project_url(project.namespace, project), options) link_to("@all", namespace_project_url(project.namespace, project), options)
elsif namespace = Namespace.find_by(path: identifier) elsif namespace = Namespace.find_by(path: identifier)
url = url =
if namespace.type == "Group" if namespace.is_a?(Group)
group_url(identifier) group_url(identifier)
else else
user_url(identifier) user_url(identifier)
......
require 'spec_helper'
describe NamespacesController do
let!(:user) { create(:user, avatar: fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "image/png")) }
describe "GET show" do
context "when the namespace belongs to a user" do
let!(:other_user) { create(:user) }
it "redirects to the user's page" do
get :show, id: other_user.username
expect(response).to redirect_to(user_path(other_user))
end
end
context "when the namespace belongs to a group" do
let!(:group) { create(:group) }
let!(:project) { create(:project, namespace: group) }
context "when the group has public projects" do
before do
project.update_attribute(:visibility_level, Project::PUBLIC)
end
context "when not signed in" do
it "redirects to the group's page" do
get :show, id: group.path
expect(response).to redirect_to(group_path(group))
end
end
context "when signed in" do
before do
sign_in(user)
end
it "redirects to the group's page" do
get :show, id: group.path
expect(response).to redirect_to(group_path(group))
end
end
end
context "when the project doesn't have public projects" do
context "when not signed in" do
it "redirects to the sign in page" do
get :show, id: group.path
expect(response).to redirect_to(new_user_session_path)
end
end
context "when signed in" do
before do
sign_in(user)
end
context "when the user has access to the project" do
before do
project.team << [user, :master]
end
context "when the user is blocked" do
before do
user.block
project.team << [user, :master]
end
it "redirects to the sign in page" do
get :show, id: group.path
expect(response).to redirect_to(new_user_session_path)
end
end
context "when the user isn't blocked" do
it "redirects to the group's page" do
get :show, id: group.path
expect(response).to redirect_to(group_path(group))
end
end
end
context "when the user doesn't have access to the project" do
it "responds with status 404" do
get :show, id: group.path
expect(response.status).to eq(404)
end
end
end
end
end
context "when the namespace doesn't exist" do
context "when signed in" do
before do
sign_in(user)
end
it "responds with status 404" do
get :show, id: "doesntexist"
expect(response.status).to eq(404)
end
end
context "when not signed in" do
it "redirects to the sign in page" do
get :show, id: "doesntexist"
expect(response).to redirect_to(new_user_session_path)
end
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