HttpServletRequest#getPathInfo() raises StringIndexOutOfBoundsException
when the path was not set.
Add a pattern to detect this and don't attempt to call getPathInfo().
Bug: Issue 6205
Change-Id: If21d8cbebbedfc1d1aa1d12aee295d5a55d9da41
* changes:
AccountsUpdate: Rename atomicUpdate to update
AccountsUpdate: Rename update method to replace
Always update accounts atomically
Migrate accounts to NoteDb (part 2)
Disallow updates to account.config by direct push or submit
Migrate accounts to NoteDb (part 1)
Let AccountsUpdate#insert create the Account instance
AccountsUpdate: Remove upsert method
This is the second part of migrating accounts from ReviewDb to NoteDb.
This change:
* migrates the accounts from ReviewDb to NoteDb (for single instance
Gerrit servers)
* adds a configuration parameter (user.readAccountsFromGit) that
controls whether external IDs are read from ReviewDb or NoteDb
AccountIT is now loading external IDs of an account directly from NoteDb
instead of retrieving them via the account cache. This is because for
the test deleteUserBranchWithAccessDatabaseCapability() the admin
account gets deleted by deleting its user branch and then the @After
restoreExternalIds() method couldn't delete the external IDs of that
account anymore (because the account was deleted it couldn't be
retrieved via the account cache anymore).
Change-Id: I41fa3a6bdb76f497c79a05bdc76e97a7e73624a6
Signed-off-by: Edwin Kempin <ekempin@google.com>
This is attempt 3 at rolling-forward c/106190
New Dependency
==============
This adds polymer-resin as a bower archive.
See `bower info polymer-resin\#1.2.6-beta` for details.
Polymer-resin is part of the larger polymer project so is
license compatible.
Integration
===========
The main application element, app/elements/gr-app.html, now HTML
imports polymer-resin per
github.com/Polymer/polymer-resin/blob/master/getting-started.md#loading
It uses the following configuration:
1. All dynamic IDs are allowed.
2. Policy violation reports are sent to the dev console.
test/common-test-setup.html does the same so that tests are run in the
same environment.
Testing
=======
1. Running local tests
gerrit $ ./polygerrit-ui/app/run_tests.sh
With 1.2.6-beta tests run green on (Chrome, Firefox, Safari).
2. Testing for false positives
I ran two servers.
a. polygerrit-ui/run_server.sh
b. gerrit.war per https://git.eclipse.org/r/Documentation/dev-readme.html
I noticed that in both the dev console showed 'initResin' early and
paging around showed no violation reports.
3. Testing for true negatives
I patched in the diff at the end of this description, and reran
both server environments.
I noted that browsing to localhost:8081/#javascript:alert(1)
and localhost:8080/#javascript:alert(1) both showed a
violation report about javascript:alert(1) being rejected.
Clicking Changes / XSS did not result in a popup.
Differences
===========
This loads the non-debug version but configured with a console reporter
so should minimize code size and speed overhead.
This loads via gr-app so the input is automatically vulcanized.
--- a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.js
+++ b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.js
@@ -56,6 +56,11 @@
url: '/q/status:abandoned',
name: 'Abandoned',
},
+ { // HACK DO NOT SUBMIT
+ url: (location.hash && location.hash.replace(/^#/, ''))
+ || '/echoes_hash',
+ name: 'XSS',
+ },
],
}];
--- a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.js
+++ b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.js
@@ -93,6 +93,7 @@
},
_computeRelativeURL(path) {
+if (path && /^\w+\:/.test(path)) { return path; } // HACK DO NOT SUBMIT
const host = window.location.host;
return this._computeURLHelper(host, path);
},
Change-Id: I38bfa124abd4fb35972833f29fc1664ec2404e34
md5 and sha1 are deprecated in Guava 22. Where possible, replace
them with the recommended alternatives.
In cases where we need to keep using sha1 for compatibility, add
a warning suppression with comment.
Change-Id: I3f80d52ea4e299c2a7c86fbda25457814bff750c
Swaps out Source Code Pro for Roboto Mono. In addition, modifies the
syntax highlighting theme for numbers and class selectors from the
darker #7F0055 to the ligher #9E0069 and removes the bold font weight
from the selector to better differentiate the selector, especially with
a red background color.
Bug: Issue 6021
Change-Id: I7588e9aa1ef0b2e4ccde8b7a5ed0c24e56760a11
PolyGerrit and GWT UI running from Gerrit itself are running on the
same origin, so they are allowed to read XHR responses even if no CORS
headers are allowed in the response. Browsers however may still send
Origin header, which likely was not whitelisted by the administrator.
Allow these requests from PolyGerrit and GWT by letting them pass
through into processing. GET requests will be fulfilled, and the
browser will enforce whether or not the JS can see the XHR reply.
POST/PUT/DELETE requests are still secured by the XSRF token being
required as proof-of-access. Invalid origins won't have the XSRF
token, and won't be able to present it in the X-Gerrit-Auth header.
Add tests for these conditions to reduce the risk of it being broken
again in the future.
Change-Id: Ia425bd7614a14b011f44910cce49f0f4f9e686a0
CORS preflight for POST, PUT, DELETE makes every mutation operation
require 2 round trips with the server. This can increase latency for
any application running on a different origin.
There is a workaround available in modern browsers: use POST with
Content-Type: text/plain. This does not require CORS preflight, as
servers should already be using XSRF protection strategies.
Unfortunately this is incompatible with the current REST API, as many
operations require PUT or DELETE methods, and a Content-Type of
application/json. Support the requester to select a different method
using query parameter '$m' and Content-Type with '$ct' in the URL,
mocking the request with those.
Using this style of request still requires the user session to be
valid for access. Accept identity through the query parameters as
'access_token'.
The XSRF token isn't necessary in this type of request as only
permitted websites would be allowed to read cookie content to obtain
the GerritAccount cookie value and include it in the URL.
Change-Id: Ic7bc5ad2e57eef27b0d2e13523be78e8a2d0a65c
This supports integrations with other web applications by making it
possible for another website to send mutation XHRs to Gerrit.
Change-Id: Ifb40f7a5ab2fa53d484af2ccbc2162275b2b02e1
Callers have to pass a number of arguments to build and configure the
VisibleRefFilter. Instead of forcing callers to pass around many
arguments, use an assisted injection factory to create the instance.
Rely on the context Provider<ReviewDb> and Provider<CurrentUser> to
gain database access and user identity within the filter. Given all
current call sites, these should already be populated.
Change-Id: I8197ee773c94f16472d53162fb70791c45899c1b
* changes:
Replace canUpload with CREATE_CHANGE
Convert GetAccess and canAddRefs to PermissionBackend
Convert forge author, committer, server to PermissionBackend
Push option bypass-review disables validation
Check RefPermission.CREATE_CHANGE before creating a change, instead of
relying on the legacy RefControl.canUpload(). This delegates creation
decisions through the PermissionBackend.
Change-Id: Ic76e96ca6079a179ae65fa2de436fb6df929b6d9
The Accounts class should become the single class that reads accounts.
At the moment it always reads the accounts from ReviewDb, but in future
it will read accounts from NoteDb.
Change-Id: I7507b734e302ca3953e86fe612aac63ea10c75da
Signed-off-by: Edwin Kempin <ekempin@google.com>
Remove AccountAccess#firstNById(int) and find the first n accounts by
looking at the user refs in All-Users instead. Since change I81491a253
it is ensured that we have a user branch for each account.
This is a preparation step for migrating the accounts from ReviewDb to
NoteDb.
Change-Id: I0d8a74d6d2de253b4311b8e91517568c1a09f2a8
Signed-off-by: Edwin Kempin <ekempin@google.com>
* stable-2.14:
Document how to set default UI
Note on branch-specific labels with custom rules.pl
GWT UI: Allow to set blocking label range rules
Extend LFS plugin servlet so that Git LFS 2.0 Lock API is handled
PolyGerrit: Fix undefined url in gr-dropdown
Change-Id: I01320e404daf1abd052c39321346e2eacff3dd09
The change extends LFS serve regex so that the following operations are
handled (according to [1]):
POST /locks - create lock
GET /locks - list locks
POST /locks/verify - list locks for verification
POST /locks/lock_id/unlock - delete lock
Common LFS definitions, like paths, regexps, content type, etc., were
moved to gerrit-extension-api so that they can be shared between Gerrit
core and LFS plugin.
[1] https://github.com/git-lfs/git-lfs/blob/master/docs/api/locking.md
Change-Id: I8299000c827b5a34d6de1ed5fc650f74be4164a2
Signed-off-by: Jacek Centkowski <jcentkowski@collab.net>
polymer-resin intercepts polymer property assignments
before they reach XSS-vulnerable sinks like `href="..."`
and text nodes in `<script>` elements.
This follows the instructions in WORKSPACE for adding a new bower
dependency with kaspern's tweak to use the dependency in a rule so
that it's found. //lib/js/bower_components.bzl has already been
rolled-back per those instructions.
The license is the polymer license as can be seen at
https://github.com/Polymer/polymer-resin/blob/master/LICENSE though
I'm not sure that //tools/js/bower2bazel.py recognizes it as such.
Docs for the added component are available at
https://github.com/Polymer/polymer-resin/blob/master/README.mdhttps://github.com/Polymer/polymer-resin/blob/master/getting-started.md
With this change, when I introduce an XSS vulnerability as below,
polymer-resin intercepts and stops it.
Patch that introduces a strawman vulnerability.
--- a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.js
+++ b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.js
@@ -55,6 +55,10 @@
url: '/q/status:abandoned',
name: 'Abandoned',
},
+ {
+ url: location.hash.replace(/^#/, '') || 'http://example.com/#fragment_echoed_here',
+ name: 'XSS Me',
+ },
],
}];
---
Address kaspern's and paladox's comments.
---
Undo version bumps for bower dependencies.
---
Change Soy index template to parallel app/index.html.
---
update polymer-resin to version 1.1.1-beta
----
Load polymer-resin into polygerrit-ui/**/*_test.html
After this, I ran the tests with
-l chrome
-l firefox
I ran a handful of tests with -p and observed that the
console shows "initResin" is called before test cases start
executing.
These changes were done programmaticly by running the script below
(approximately) thus:
```
gerrit/ $ cd polygerrit-ui/app
app/ $ find . -name \*test.html | xargs perl hack-tests.pl
```
```
use strict;
sub removeResin($) {
my $s = $_[0];
$s =~ s@<link rel="import" href="[^"]*/polymer-resin/[^"]*"[^>]*>\n?@@;
$s =~ s@<script src="[^"]*/polymer-resin/[^"]*"></script>\n?@@;
$s =~ s@<script>\s*security\.polymer_resin.*?</script>\n?@@s;
return $s;
}
for my $f (@ARGV) {
next if $f =~ m@/bower_components/|/node_modules/@;
system('git', 'checkout', $f);
print "$f\n";
my @lines = ();
open(IN, "<$f") or die "$f: $!";
my $maxLineOfMatch = 0;
while (<IN>) {
push(@lines, $_);
# Put a marker after core loading directives.
$maxLineOfMatch = scalar(@lines)
if m@/webcomponentsjs/|/polymer[.]html\b|/browser[.]js@;
}
close(IN) or die "$f: $!";
die "$f missing loading directives" unless $maxLineOfMatch;
# Given ./a/b/c/my_test.html, $pathToRoot is "../../.."
# assuming no non-leading . or .. components in the path from find.
my $pathToRoot = $f;
$pathToRoot =~ s@^\.\/@@;
$pathToRoot =~ s@^(.*?/)?app/@@;
$pathToRoot =~ s@\/[^\/]*$@@;
$pathToRoot =~ s@[^/]+@..@g;
my $nLines = scalar(@lines);
open(OUT, ">$f") or die "$f: $!";
# Output the lines up to the last polymer-resin dependency
# loaded explicitly by this test.
my $before = join '', @lines[0..($maxLineOfMatch - 1)];
$before = removeResin($before);
print OUT "$before";
# Dump out the lines that load polymer-resin and configure it for
# polygerrit.
if (1) {
print OUT qq'<link rel="import" href="$pathToRoot/bower_components/polymer-resin/standalone/polymer-resin-debug.html"/>
<script>
security.polymer_resin.install({allowedIdentifierPrefixes: [\'\']});
</script>
';
}
# Emit any remaining lines.
my $after = join '', @lines[$maxLineOfMatch..$#lines];
$after = removeResin($after);
$after =~ s/^\n*//;
print OUT "$after";
close(OUT) or die "$f: $!";
}
```
---
update polymer-resin to version 1.2.1-beta
---
update Soy index template to new style polymer-resin initialization
----
fix lint warnings
----
Load test/common-test-setup.html into *_test.html
Instead of inserting instructions to load and initialize polymer-resin into
every test file, add a common-test-setup.html that does that and also fold
iron-test-helpers loading into it.
----
imported files do not need to load webcomponentsjs
Change-Id: I71221c36ed8a0fe7f8720c1064a2fcc9555bb8df
FileInputStream and FileOutputStream rely on finalize() method to ensure
resources are closed. This implies they are added to the finalizer queue
which causes additional work for the JVM GC process.
This is an open bug on the OpenJDK [1] and the recommended workaround is
to use the Files.newInputStream and Files.newOutputStream static methods
instead.
[1] https://bugs.openjdk.java.net/browse/JDK-8080225
Change-Id: I3cef6fcf198dde2be7cd15bded8d2fa247177654
* changes:
Convert more RestModifyViews to retrying helper
Convert reviewer modify views to retrying helper
Convert most revision modify views to retrying helper
Convert most ChangeApi handlers to retrying wrappers
Add a convenience class for making REST API handlers retryable
Add static util for throwing exceptions from API implementations
Excludes a few that will require more work to plumb the
BatchUpdate.Factory into the class that actually does the work.
Change-Id: Id8c679c91ed8ea142f1856fcc2d976c4fb609dc5
We want to get all callers of BatchUpdate to use the new RetryHelper,
but explicitly passing a lambda to the RetryHelper instance in every
case is going to be a lot of boilerplate. Add some sugar for the most
common case, a REST API handler. Using the RetryingRestModifyView,
converting a handler to be retry-friendly is as simple as adding the
base class and changing a method name (and some minor cleanup like
removing a BatchUpdate.Factory field and twiddling exceptions).
Adding an intervening abstract class requires a tweak to the way
RestApiServlet extracts the input type using reflection. Fortunately,
there is a TypeLiteral method that does exactly what we want.
Change-Id: If2b78f0d556a49972ea9976057dd5f9dc21ec750
In firefox if the keydown event overrides the default behaviour,
when the enter key is pressed, the autocomplete will not work.
This commit changes the event listener to use keyup instead of
keydown to allow for firefox autocomplete to work correctly.
Bug: Issue 6083
Change-Id: I6a91631166c0d21b92079af36f976b010c0244fd
Reading the access sections for a project requires ACCESS
permission, as the handlers filter references and groups
depending on the caller.
Change-Id: Ie5a3d58b5456593db6e6ac9725bb981bcef8b3a8
GitOverHttpServlet requires the caller has ACCESS permission
for the target project, as ref filtering may happen later.
GitwebServlet requires READ permission as there is no ref
filtering available within the gitweb CGI.
This slightly changes the behavior of GitOverHttpServlet by allowing
a project owner to access a hidden project when using Git over HTTP.
Change-Id: I454f03597bdf5ed8447e73985769c2fe4d478056
Leave a poorly named backdoor in CapabilityControl for the existing
ProjectControl, RefControl, ChangeControl and GroupControl to test
administrator permission.
Update test expecting a failure when administateServer is not granted.
Change-Id: I0f523dbf26506ea53c38ffb02aeef74f3cf18ba6
When dynamicbeans are defined by an external plugin on commands from a
different plugin, instantiate the dynamic bean with a delegating
classloader which uses the command's classloader as the parent and
resolves resources via the dynamicbean's classloader. Also instantiate
the dynamicbeans using the injector from the command.
Change-Id: If2cff8235a9680eb64c58b77f2d482c5896baf0f
Replace CapabilityUtils with support in PermissionBackend to check if
the caller has at least one of the specified permissions parsed from
class annotation.
This enables hiding canPerform(String) from CapabilityControl, which
makes it much harder to bypass the PermissionBackend.
Assume anyone with ADMINISTRATE_SERVER also has any PluginPermission.
This is carried over from CapabilityUtils, which skip any further
checks when the user has canAdministrateServer.
Update the error message in GarbageCollectionIT to now be the generic
"maintain server not permitted".
Change-Id: I9458bd55fa1c9709557ae1ad95a57a1d968c52a3
Some cleanup on the use of Soy index:
* The canonical-path property of GR-APP is removed because it did not
initialize at the right time and a top-level global is used instead.
* Only set a top-level global for canonical path if it is not the empty
string. In this way, the index document does not include an inline
script when it is not needed.
* Give the index Soy template a more descriptive name. index.html.soy
was too generic, and the intermediate period would cause problems with
some internal build tooling.
Bug: Issue 5919
Change-Id: I0bf5964b17cb4f4f9df150b0c6abe8ae1e49f22b
