1060 Commits

Author SHA1 Message Date
Dave Borowitz
35f6f13fd5 Remove PermissionDeniedException
This exception was only used by the project access handlers. It was not
given any special treatment by RestApiServlet, so it was propagated as a
500, which was probably not the intent. Replace all usages with
AuthException.

Change-Id: I134cbd4155de306fe443865be5e0ea9c4e711b7a
2018-03-29 13:11:33 -04:00
Edwin Kempin
efbb37161e Merge "Merge branch 'stable-2.15' into master" 2018-03-29 15:25:14 +00:00
Dave Borowitz
d2f5af379c Merge branch 'stable-2.15' into master
Fix two test failures due to the merge:

* StandaloneNoteDbMigrationIT assumed that GCing All-Users would prune
  all loose objects, whereas in recent JGit versions, unreachable
  objects are left loose unless gc.pruneExpire is set to "now". Do this
  in the tests.

* AbstractQueryChangesTest#watched modifies the user's project watches.
  In the master branch, watches are now stored in the AccountState
  instance, so we need to recreate the IdentifiedUser in order to reload
  the AccountState.

* origin/stable-2.15: (33 commits)
  Release 2.15
  Mention groups index in documentation of index start command
  Enable UI action to toggle WIP flag for admins
  Docs: Clarify that for external groups the name in GroupInfo can be missing
  AccountGroupAuditLogScreen: Display group UUID if group name is missing
  GetAuditLog: Fix NPE if group UUID cannot be resolved
  Limit assignee suggestions to users that can see the change
  Add account predicate that checks if user can see a certain change
  Allow admins to toggle the WIP flag on all changes
  Included-In dialog polish
  PolyGerrit: Add support for "Included In"
  Make template testing faster
  Limit assignee suggestions to users that can see the change
  Add account predicate that checks if user can see a certain change
  AbstractQueryChangesTest: Extend byDraftBy to include test for "has:draft"
  AbstractQueryChangesTest: Add explicit tests for is:watched and watchedby:
  AbstractQueryChangesTest: Add explicit tests for is:abandoned and status:abandoned
  user-search: Clarify behavior of default search resulting in single change
  user-search: Fix query used in "My > Watched Changes"
  Documentation: Clarify ref-updated event content when ref is deleted
  Update git submodules
  Fix markup in Documentation section sendemail
  Allow graceful rolling restarts
  Release 2.15-rc4
  user-search: Remove incorrect statement about default searches
  user-search: Fix incorrect label: search example
  StandaloneSiteTest: Ignore user and system git config
  Fix documentation about create and delete branches
  GcAllUsers: Respect gc.auto=0 in All-Users
  GcAllUsers: Add extra log line in online case
  MigrateToNoteDb: Auto-flush and close GC writer
  Turn off autoReindexIfStale by default
  GC All-Users after migrating to NoteDb

Change-Id: If2dff6bd1e0b299be75878aca3bc45103a5a875f
2018-03-29 10:26:18 -04:00
Hugo Arès
a2c3e4f828 Merge "ChangeField: Remove duplicate calls to stored() for int range fields" 2018-03-29 12:06:25 +00:00
Hugo Arès
ee8617c3dc Merge "InternalAccountQuery: Remove unused methods" 2018-03-29 12:05:17 +00:00
Patrick Hiesel
7887eb96ef Merge "Remove the non-permission check #isHidden from ProjectControl" 2018-03-29 11:18:36 +00:00
Edwin Kempin
181742ad95 ChangeField: Remove duplicate calls to stored() for int range fields
All int range field must be stored. This is why FieldDef#intRange
already calls the stored() method. Calling it again for int range
fields in ChangeField is unneeded.

Change-Id: I04c11c70568a0ac510a9b95bc1a4489a4e34f9e8
Signed-off-by: Edwin Kempin <ekempin@google.com>
2018-03-29 10:48:42 +02:00
Changcheng Xiao
365424149f Add private constructor to RefUtil
RefUtil is a utility class which only contains statics. Add a private
constructor to prevent creating an instance of it.

Change-Id: If689d8e91886fb98b078d68ef6f6d5d9edf1ccfa
2018-03-29 10:42:41 +02:00
Changcheng Xiao
da44fe4ba2 Remove the non-permission check #isHidden from ProjectControl
PermissionBackend is supposed to only contain permission related checks.
Apparently, this #isHidden is a project state check rather than a
permission check. Thus it should be removed from the ProjectControl,
which is part of the DefaultPermissionBackend.

Before removing this, "ACCESS" permission checks for hidden projects will
only succeed for the project owners. After removing, they may also succeed
for other users, e.g. internal user.

The existing checks for "ACCESS" could be divided into two categories
base on whether it's helpful for users to change the configuration of
the project state or not.

For the helpful case, this commit preserves the current behavior of
the "ACCESS" check on hidden projects by checking the "READ_CONFIG"
permission which will only succeed for the project owners. For the
other case, this commit rejects directly even for project owners if
the project is hidden.

Change-Id: I20743e6380129eea7cb942d8d9ccad314e29d187
2018-03-29 10:40:19 +02:00
Edwin Kempin
69f297f568 InternalAccountQuery: Remove unused methods
Change-Id: I45ccec5ed306e88a8ba1197402585e64986ab943
Signed-off-by: Edwin Kempin <ekempin@google.com>
2018-03-29 10:40:00 +02:00
Edwin Kempin
b8a580d0e7 Merge "Remove unused variable in CreateChangeSender" 2018-03-29 08:28:57 +00:00
xchangcheng
cf1eba8395 Merge "Remove non-permission related check #canDelete from ChangeControl" 2018-03-29 08:16:39 +00:00
Patrick Hiesel
b1d10c5a4c Remove unused variable in CreateChangeSender
Change-Id: I39d6e18be87173ce1c66f1a67bbe9bbbffd347b1
2018-03-29 09:38:58 +02:00
Edwin Kempin
ff31a7fdb3 Fix suggestion of default reviewers by reviewers plugin
The reviewers plugin passes null as changeNotes to the
ReviewersUtil#suggestReviewers method which led to a
NullPointerException.

Mark changesNotes as Nullable and expect that it can be null.

Change-Id: I6d1656ed7e44719130dd2da393906518b2833245
Signed-off-by: Edwin Kempin <ekempin@google.com>
2018-03-29 08:58:13 +02:00
Changcheng Xiao
e5dfde9c1c Remove non-permission related check #canDelete from ChangeControl
PermissionBackend should only do permission related checks.
Non-permission related checks must be done outside of the
PermissionBackend. Like the case in this commit, the caller
should verify the change is deletable before checking the
DELETE permission.

ChangePermission.DELETE is only used in the DeleteChange
endpoint, which makes it very easy to remove.

Change-Id: I5aa477facf2ca64e6f9aa64fad173a2db081430e
2018-03-29 08:48:01 +02:00
Patrick Hiesel
2ca92a63df Merge changes I6099c83d,Iea329f9b
* changes:
  Migrate CanSeeChangePredicate to use PermissionBackend#absentUser()
  Add absentUser() to PermissionBackend
2018-03-29 06:47:45 +00:00
Dave Borowitz
8a5c427c90 Merge "Allow empty POSTs as OAuthRequests" 2018-03-28 18:58:27 +00:00
Edwin Kempin
343a70f55b ExternalIdNotes: Set affected accounts/emails as footers in commit message
Looking at the history of the refs/meta/external-ids branch in All-Users
when can see from the commit messages from why and from where an
external ID was updated (e.g. see [1]).

However to investigate issues with external IDs we often want to look at
the external IDs of a specific user and how they have changed over time.
Finding all commits that modified an external of an account/email is
difficult because this information is only contained in the blobs.

To make this easier we now add the affected accounts/emails as footers
in the commit message when updating external IDs (e.g. see [2]).

[1]
commit 4ccc8d21f3702ef145b1c5824e87cf93010a39d3 (refs/meta/external-ids)
Author: Gerrit Code Review <ekempin@ekempin0.muc.corp.google.com>
Date:   Wed Mar 28 14:03:15 2018 +0200

    Link External ID

commit 4b733408ab4697539aeb4265028589f5811ee3ad
Author: Administrator <ekempin@ekempin0.muc.corp.google.com>
Date:   Mon Mar 5 16:20:06 2018 +0100

    Set HTTP Password via API

commit cee914f803ec07549cf0dbf604634a2484f077b6
Author: Gerrit Code Review <ekempin@ekempin0.muc.corp.google.com>
Date:   Fri Feb 2 08:26:08 2018 +0100

    Create Account on First Login

[2]
commit 4ccc8d21f3702ef145b1c5824e87cf93010a39d3 (refs/meta/external-ids)
Author: Gerrit Code Review <ekempin@ekempin0.muc.corp.google.com>
Date:   Wed Mar 28 14:03:15 2018 +0200

    Link External ID

    Account: 1000004
    Email: foo@example.com

commit 4b733408ab4697539aeb4265028589f5811ee3ad
Author: Administrator <ekempin@ekempin0.muc.corp.google.com>
Date:   Mon Mar 5 16:20:06 2018 +0100

    Set HTTP Password via API

    Account: 1000015

commit cee914f803ec07549cf0dbf604634a2484f077b6
Author: Gerrit Code Review <ekempin@ekempin0.muc.corp.google.com>
Date:   Fri Feb 2 08:26:08 2018 +0100

    Create Account on First Login

    Account: 1000015

Change-Id: I26e72866e8effe777945c1f197bc0aeed6d3ddb1
Signed-off-by: Edwin Kempin <ekempin@google.com>
2018-03-28 15:00:06 +00:00
Edwin Kempin
216473fc2c Fix removal of email/password on external ID update
Change-Id: Ia8f06bd02dc4aba95d8ff103f33572f8054b39cf
Signed-off-by: Edwin Kempin <ekempin@google.com>
2018-03-28 15:00:00 +00:00
Patrick Hiesel
d98cb3df65 Migrate CanSeeChangePredicate to use PermissionBackend#absentUser()
Change-Id: I6099c83d820794d32000ad80d7fed8e51ef3f645
2018-03-28 16:50:39 +02:00
Edwin Kempin
f4c0f17c1e Merge "Drop ReviewDb group tables" 2018-03-28 13:41:17 +00:00
Patrick Hiesel
86a05337a9 Add absentUser() to PermissionBackend
Checking permissions of users that aren't the caller of the current request
can have implications on the security of the system. The most prominent
one is creating a group-oracle.

A group-oracle is created when a user who can modify the access settings
of a host or project can use a feature of the system to get feedback if
a specified user has access to a resource. If access was granted by
group-membership through an external group, this can be used to probe
for membership in the external system.

To limit the cases where we could potentially expose Gerrit to these
threats, PermissionBackend adds a new method to provide checks for absent
users. We consider a user to be absent if they are not the issuer of the
current request and not the target of impersonation.

This way, we can easily check where we perform permission checks on
absent users. We can use this knowledge to better assess and mitigate
this risk.

PermissionBackends that do not support impersonation can have a
checkState() call in PermissionBackend#user() to ensure the provided
user is the current user.

This commit migrates the first caller to use #absentUser(). Follow-up
commits will migrate the rest.

Change-Id: Iea329f9bbbff74d1bd3521ccd2ec217c2befefc0
2018-03-28 15:23:41 +02:00
Dave Borowitz
4cfa526b1b Merge changes from topic "notedb-push-option"
* changes:
  Disallow pushing to NoteDb without a special push option
  ReceiveCommits: Factor out parsePushOptions method
2018-03-28 11:32:48 +00:00
Edwin Kempin
6e4d4ca13e Drop ReviewDb group tables
Groups are now in NoteDb and the group tables in ReviewDb are no longer
needed.

Change-Id: I5fcff38aa88f2c62921f5bc9c891ba7299a67b33
Signed-off-by: Edwin Kempin <ekempin@google.com>
2018-03-28 09:03:35 +02:00
Edwin Kempin
e0193a7c17 Merge "Adapt JavaDoc of PermissionBackend to 'Returns' instead of @return" 2018-03-28 06:25:21 +00:00
Edwin Kempin
50484b7b94 Merge changes Ie87c1c8e,I530116c8
* changes:
  Remove support for groups in ReviewDb
  Migrate groups to NoteDb
2018-03-28 06:17:45 +00:00
Logan Hanks
f329ea2b2a Allow empty POSTs as OAuthRequests
This has always been allowed for input types that have a @DefaultInput.
Previously, a body-less POST to /a/changes/.../submit would return a
400 error because SubmitInput has no default input fields.

Change-Id: Ie0741e7f1082c2bf95bc89acf3a31d1ce3091d0c
2018-03-27 22:36:25 +00:00
Patrick Hiesel
9104fa9ad9 Refactor Reachable to take Project.NameKey instead of ProjectState
This resolved a TODO I had put in code and makes the method more
light-weight.

Change-Id: I229b197d1e3798b409edaa409cb5ec4dedae2ccb
2018-03-27 18:44:35 +00:00
David Pursehouse
7769a2a437 Merge "Remove unused variables" 2018-03-27 16:32:55 +00:00
David Pursehouse
1aacb966ef Merge changes I8eace895,I3eafef10
* changes:
  Respect the configured server ID during in memory tests
  Make sure that footers in group config commit messages are sorted
2018-03-27 16:19:20 +00:00
Patrick Hiesel
4dcc9f36b3 Remove unused variables
These came out of a larger refactoring in I601ea1200a.

Change-Id: Ib3921d414bd26de5360b72ff709dd751382e8f18
2018-03-27 18:01:14 +02:00
Patrick Hiesel
21ef5af63b Adapt JavaDoc of PermissionBackend to 'Returns' instead of @return
This is preferred in case there is no summary fragment:
https://google.github.io/styleguide/javaguide.html#s7.2-summary-fragment

Change-Id: Ieeda088f22d3707a1efd627501f6d3c0f4b1a743
2018-03-27 17:39:00 +02:00
Patrick Hiesel
ef17720a73 Merge "Remove PermissionBackend#user(Provider<CurrentUser>)" 2018-03-27 14:50:44 +00:00
Patrick Hiesel
4bdef6c030 Remove PermissionBackend#user(Provider<CurrentUser>)
Checking permissions of users that aren't the caller on the current request
can have implications on the security of the system. The most prominent
one is creating a group-oracle.

To limit the cases where we could potentially expose Gerrit to these
threats, PermissionBackend removes the method that was operating solely
on the provider of the current user.

Change-Id: I601ea1200a15a5f262ca0770b23cc1c7bee126b1
2018-03-27 15:57:45 +02:00
Edwin Kempin
9cae606cda Enable UI action to toggle WIP flag for admins
Change I4878f066b6 allowed administrators to toggle the WIP flag on any
change but the UI action was still disabled for admins.

Change-Id: I55dd6400dc07d57fe2aaaf3528ff429d5baf48ed
Signed-off-by: Edwin Kempin <ekempin@google.com>
2018-03-27 15:57:39 +02:00
Patrick Hiesel
659ea71969 Add currentUser() to PermissionBackend
Passing in a Provider<CurrentUser> into PermissionBackend is
boiler-platy. This change adds a convenience method to PermissionBackend
and DefaultPermissionBackend to limit this boiler-plate. Future commits
will remove #user(Provider<CurrentUser>) from PermissionBackend, once
all callers were moved.

Change-Id: Ifcd07fe2c7d2673a66b2b4f9eff06ee8a3b6b943
2018-03-27 10:58:37 +00:00
Edwin Kempin
95df75fae6 Remove support for groups in ReviewDb
Groups have been migrated to NoteDb. Hence we no longer need to be able
to read groups from ReviewDb.

Change-Id: Ie87c1c8e604cf1344af5291f0b369cd24af8387d
Signed-off-by: Edwin Kempin <ekempin@google.com>
2018-03-27 10:46:26 +02:00
Alice Kober-Sotzek
a9aabc879c Migrate groups to NoteDb
This change adds a schema migration that migrates Gerrit groups from
ReviewDb to NoteDb.

In NoteDb groups are stored in group refs in the All-Users repository.
When a group is migrated its group ref in NoteDb is overwritten if it
already exists.

If groups in ReviewDb have already been disabled (e.g. a new Gerrit
instance that directly used groups in NoteDb, or if groups have been
migrated differently) this schema migration does nothing, as we don't
want to overwrite group information in NoteDb with potentially outdated
ReviewDb content.

The commits in the group refs form the audit log of the group. This is
why the migration creates one commit per audit event in the group ref.

When members or subgroups are added or removed they are listed as
footers in the commit message. For subgroups this footer line contains
the group name and the group UUID. The schema migration can set the
proper group name only for Gerrit internal groups and system groups,
but not for external groups since the external group backends, which are
needed to resolve the UUID to the group name, are not available during
init. For groups which cannot be resolved during init the UUID is used
as group name. This is only a cosmetic issue with the commits of the
group refs that might affect human readers of the history. When Gerrit
is reading the audit log it doesn't rely on the group names in the
footers, but resolves the group UUIDs via the group backends.

After the migration has been done all groups are now fully in NoteDb.
The default values for the group migration configuration are changed so
that NoteDb is used as primary storage for groups and groups in ReviewDb
are disabled. Writing groups to NoteDb can no longer be disabled because
after this point there will be no further migration to copy group data
from ReviewDb to NoteDb.

GroupRebuilderIT is merged into the new
Schema_166_to_167_WithGroupsInReviewDbTest.

Change-Id: I530116c8c5a6a5c595d24ca2445ffa921c2d3eb0
Signed-off-by: Edwin Kempin <ekempin@google.com>
2018-03-27 10:46:20 +02:00
Patrick Hiesel
548d22f29b Merge "Add Java implementation of the label functions" 2018-03-27 08:43:44 +00:00
Maxime Guerreiro
c275089fc9 Add Java implementation of the label functions
Add unit tests for the labels functions.
Check if prolog rules are defined for this project or its parents, and
if not default to the added Java implementations of LabelFunctons.

Before this commit, the Prolog rules engine was always invoked to check
wether a change can be submitted or not, even if no prolog rules were
defined.
Doing so should also make it easier to extract Prolog as a plugin
without losing any of its currently offered features (label functions
and default rules implementation).

The LabelFunction code is inspired by Saša Živkov's change Iffe1567,
adjusted to live directly in the enum.

Change-Id: I5e18b0d494be3f0423bb533ed84a63ea4b8a31df
2018-03-27 09:47:23 +02:00
xchangcheng
5d5a21c8d9 Merge "CheckAccess: don't catch PermissionBackendException" 2018-03-27 07:16:19 +00:00
Edwin Kempin
ca90860ef4 Merge "GroupBundle: Fix reading visibleToAll from ReviewDb" 2018-03-27 07:14:45 +00:00
Edwin Kempin
4a55f249ff Respect the configured server ID during in memory tests
Some tests want to configure the server ID via the Gerrit config.
InMemoryModule should respect this setting if it is there and only
fallback to the hard-coded server ID if this config is missing.

Change-Id: I8eace895978d221b8d4e726a4c5428bc16f9e77b
Signed-off-by: Edwin Kempin <ekempin@google.com>
2018-03-27 09:01:30 +02:00
Edwin Kempin
99cdabf582 Make sure that footers in group config commit messages are sorted
E.g. GroupConfigCommitMessage#getFooters is using Sets for the footers
and hence the order was not guaranteed. As result of this tests checking
group audit logs could be flaky.

The AuditLogFormatter in AbstractGroupTest is now loading the real group
names instead of using 'Group <uuid>'. This is needed to control the
order of the subgroup modification footers from the AuditLogReaderTest.
If the real name is not included into these footers the sort order
depends on the generated UUIDs. Since the UUIDs are generated based on
the group name and the server ID they are stable for each run, but
relying on the order of the UUIDs makes the test at least less readable.

The AuditLogFormatter in AbstractGroupTest is loading the group from the
repo each time a group name is needed. Since it's an in memory
repository and there are only few tests using this AuditLogFormatter we
don't bother about caching here.

Reading footers from group config commit messages works regardless of
the order of the footers. This means this change doesn't require
rewriting already existing group refs.

Change-Id: I3eafef10e916890b90d9f9ac222595eaf2246e27
Signed-off-by: Edwin Kempin <ekempin@google.com>
2018-03-27 08:49:08 +02:00
Changcheng Xiao
a2ece68f1a DefaultRefFilter: log error for PermissionBackendException
This commit changes this class so that every PBE will be logged
out as an error. But another option is throwing out this
exception since it stands for an error on the server side.

Change-Id: I231ab13ebeb9e5b37788875355d675c17f68745b
2018-03-27 08:16:30 +02:00
Changcheng Xiao
6f7b253901 CheckAccess: don't catch PermissionBackendException
By design, PermissionBackendException stands for some error
in the permission backend. It doesn't mean the user doesn't
hold the checked/tested permission. Thus this endpoint should
not catch PBE and treat it the same with AuthException.

Change-Id: Ibbb99fb3648a1bfdbdea922cdb94a77f6824c141
2018-03-26 21:09:03 +02:00
Patrick Hiesel
059e7e7a33 Merge changes from topic "remove-project-ctl-isHidden"
* changes:
  ProjectControl: remove "isHidden" when checking "READ" permission
  UploadArchive: check ProjectState readable before checking "READ" permission
  ListProjects: check ProjectState readable before checking "READ" permission
  ProjectIsVisibleToPredicate: check ProjectState readable before checking "READ" permission
  DefaultRefFilter: check ProjectState readable before checking "READ" permission
  AsyncReceiveCommits: check ProjectState readable before checking "READ" permission
  GitWebServlet: check ProjectState readable before checking "READ" permission
2018-03-26 15:40:25 +00:00
Changcheng Xiao
3d002433fc ProjectControl: remove "isHidden" when checking "READ" permission
Change-Id: I687f2957135bfba3c3a66617828980ddc5b14c85
2018-03-26 13:12:45 +00:00
Changcheng Xiao
581dbf9f32 UploadArchive: check ProjectState readable before checking "READ" permission
Change-Id: I77756f59e60dba33fc6f3b659bebb0c1bbac2ea6
2018-03-26 13:12:39 +00:00
Changcheng Xiao
80e5273562 ListProjects: check ProjectState readable before checking "READ" permission
Change-Id: I7c3724927afe7427434e6ae84b33310491c3e098
2018-03-26 13:12:29 +00:00