This enables follow-up changes to not expose implementation details of
reading/writing external IDs (since classes can be kept package
private now).
Change-Id: I4b3394592304d8391d7103821d46da7ce475be52
In NoteDb external IDs are stored in the All-Users repository in a Git
Notes branch called refs/meta/external-ids where the sha1 of the
external ID is used as note name. Each note content is a Git config
file that contains an external ID. It has exactly one externalId
subsection with an accountId and optionally email and password:
[externalId "username:jdoe"]
accountId = 1003407
email = jdoe@example.com
password = bcrypt:4:LCbmSBDivK/hhGVQMfkDpA==:XcWn0pKYSVU/UJgOvhidkEtmqCp6oKB7
Storing the external IDs in a Git Notes branch with using the sha1 of
the external ID as note name ensures that external IDs are unique and
are only assigned to a single account. If it is tried to assign the
same external ID concurrently to different accounts, only one Git
update succeeds while the other Git updates fail with LOCK_FAILURE.
This means assigning external IDs is also safe in a multimaster setup
if a consensus algorithm for updating Git refs is implemented (which
is needed for multimaster in any case). Alternatively it was
considered to store the external IDs per account as Git config file in
the refs/users/<sharded-id> user branches in the All-Users repository
(see abandoned change 9f9f07ef). This approach was given up because in
race conditions it allowed to assign the same external ID to different
accounts by updating different branches in Git.
To support a live migration on a multi-master Gerrit installation, the
migration of external IDs from ReviewDb to NoteDb is done in 2 steps:
- part 1 (this change):
* always write to both backends (ReviewDb and NoteDb)
* always read external IDs from ReviewDb
* upgraded instances write to both backends, old instances only
write to ReviewDb
* after upgrading all instances (all still read from ReviewDb)
run a batch to copy all external IDs from the ReviewDb to NoteDb
- part 2 (next change):
* bump the database schema version
* migrate the external IDs from ReviewDb to NoteDb (for single instance
Gerrit servers)
* read external IDs from NoteDb
* delete the database table
With this change reading external IDs from NoteDb is not implemented
yet. This is because the storage format of external IDs in NoteDb
doesn't support efficient lookup of external IDs by account and this
problem is only addressed in the follow-up change (it adds a cache for
external IDs, but this cache uses the revision of the notes branch as
key, and hence can be only implemented once the external IDs are fully
migrated to NoteDb and storing external IDs in ReviewDb is dropped).
The ExternalIdsUpdate class implements updating of external IDs in
both NoteDb and ReviewDb. It provides various methods to update
external IDs (e.g. insert, upsert, delete, replace). For NoteDb each
method invocation leads to one commit in the Git notes branch.
ExternalIdsUpdate has two factories, User and Server. This allows to
record either the calling user or the Gerrit server identity as
committer for an update of the external Ids.
External IDs are now represented by a new AutoValue class called
ExternalId. This class replaces the usage of the old gwtorm entity
AccountExternalId class. For ExternalId scheme names are the same as for
AccountExternalId but no longer include the trailing ':'.
The class ExternalIdsOnInit makes it possible to update external IDs
during the init phase. This is required for inserting external IDs for
the initial admin user which is created by InitAdminUser. We need a
special class for this since not all dependencies of ExternalIdsUpdate
are available during init.
The class ExternalIdsBatchUpdate allows to do batch updates to
external IDs. For NoteDb all updates will result in a single commit to
the refs/meta/external-ids Git notes branch.
LocalUsernamesToLowerCase is now always converting the usernames in a
single thread only. This allows us to get a single commit for the
username convertion in NoteDb (this would not be possible if workers
do updates in parallel). Since LocalUsernamesToLowerCase is rather
light-weight being able to parallelize work is not really needed and
removing the workers simplifies the code significantly.
To protect the refs/meta/external-ids Git notes branch in the All-Users
repository read access for this ref is only allowed to users that have
the 'Access Database' global capability assigned. In addition
there is a commit validator that disallows updating the
refs/meta/external-ids branch by push. This is to prevent that the
external IDs in NoteDb diverge from the external IDs in ReviewDb while
the migration to NoteDb is not fully done yet.
Change-Id: Ic9bd5791e84ee8d332ccb1f709970b59ee66b308
Signed-off-by: Edwin Kempin <ekempin@google.com>
Having a standard tool for formatting saves reviewers' valuable time.
google-java-format is Google's standard formatter and is somewhat
inspired by gofmt[1]. This commit formats everything using
google-java-format version 1.2.
The downside of this one-off formatting is breaking blame. This can be
somewhat hacked around with a tool like git-hyper-blame[2], but it's
definitely not optimal until/unless this kind of feature makes its way
to git core.
Not in this change:
* Tool support, e.g. Eclipse. The command must be run manually [3].
* Documentation of best practice, e.g. new 100-column default.
[1] https://talks.golang.org/2015/gofmt-en.slide#3
[2] https://commondatastorage.googleapis.com/chrome-infra-docs/flat/depot_tools/docs/html/git-hyper-blame.html
[3] git ls-files | grep java$ | xargs google-java-format -i
Change-Id: Id5f3c6de95ce0b68b41f0a478b5c99a93675aaa3
Signed-off-by: David Pursehouse <dpursehouse@collab.net>
This makes it more clear that a lookup may find no account.
Change-Id: Ia0ffe7547c365be1526eb959ff4b5e25cfa1d37e
Signed-off-by: Edwin Kempin <ekempin@google.com>
Reformat the Bazel build files with the buildifier tool [1].
The style is different for Bazel files. Most notably, indentation level
is 4 spaces instead of 2, and " is used instead of '.
[1] https://github.com/bazelbuild/buildifier
Change-Id: I95c0c6f11b6d76572797853b4ebb5cee5ebd3c98
To run the tests:
bazel test //...
To build the Gerrit plugin API, run:
bazel build gerrit-plugin-api:plugin-api_deploy.jar
To build the Gerrit extension API, run:
bazel build gerrit-extension-api:extension-api_deploy.jar
TODOs:
Licenses
Reduce visibility (all public for now)
Generate HTML Documentation
Core plugins
gerrit_plugin() rule to build plugins in tree and standalone modes
GWT UI (only gwt_module() skylark rule is provided, no gwt_binary())
PolyGerrit UI
WAR
Publish artifacts to Maven Central
Ask Bazel team to add Gerrit to their CI on ci.bazel.io
Contributed-By: Han-Wen Nienhuys <hanwen@google.com>
Change-Id: I9a86e670882a44a5c966579cdeb8ed79b1590de3
It's not necessary to use Guava's helper methods when instantiating
empty collections. Just use the native constructors.
Change-Id: I7f454909b15924ee49e149edf9f053da9f718502
enums, interfaces, and field members in interfaces are by default
static, so don't need to be explicitly declared so.
Change-Id: I29270c28be30965767519ad0105a7d93a24e0ab4
I39f2d5d7 isolated jgit in its own cell, that is based on this JGit
Buck build implementation: [1]. Migration was done seamlessly, meant
that single BUCK file in lib/jgit represents JGit cell root location.
However, the real structure of JGit project is divided to number of
different sub-projects. To map between simplified JGit cell in gerrit
and real JGit project structure in JGit project, java_library() rules
were added to root BUCK file in JGit project that work like proxy to
real rules located in JGit sub-projects. For example //:jgit in JGit
tree was implemented as:
java_library(
name = 'jgit',
exported_deps = ['//org.eclipse.jgit:jgit'],
visibility = ['PUBLIC'],
)
Such proxies are needed for every artifact that is referenced from
gerrit build and make Buck build implementation unnecessary verbose.
Moreover this introduced some subtle issues, like using JGit
dependencies in context of java_doc rules, where $(location :foo) macro
is unable to resolve the underlying files because java_library with
exported dependencies only do not have association with output file.
An attempt to replace java_library with only exported dependenies with
prebuilt_jar: [2] that depends on the real artifact introduced another
problem with assembly of gerrit.war, because now jgit.jar is twice in
the classpath (because prebuilt_jar has output file association). To
fix this we would need to filter potential duplicates in the assembly
process of gerrit.war.
Instead of using proxy approach and to try to provide yet another
workaround to subtle problems, emulate the JGit project structure and
reference directly the same artifacts paths within gerrit JGit cell
in gerrit build:
deps = [
'@jgit//org.eclipse.jgit:jgit',
],
This simplifies JGit Buck build implementation, as we wouldn't need to
proxy all artifacts referenced from gerrit build from the root build
file. And this would fix all remaining issues.
This approach make gerrit build slightly more verbose. JGit upgrade
process would need to touch 4 files instead of only one. But given that
the Gerrit/JGit development integration is important feature, we would
like to support (as this integration attempt shows: [3]) in our build
toolchain, this overhead is justified.
With this change, the root build file in JGit project can be stripped:
[4].
[1] https://git.eclipse.org/r/61938
[2] https://git.eclipse.org/r/66547
[3] https://gerrit-review.googlesource.com/61892
[4] https://git.eclipse.org/r/66562
Change-Id: I2d278f80d0fedc4c5e9943804873f57145877dfe
Consume JGit as first third party library from its own cell. Normally
the cell is defined as lib/jgit directory. It can be easily replaced
with CLI:
buck build --config repositories.jgit=path/to/dev/jgit gerrit
or tweaking the .buckconfig:
[repositories]
jgit = path/to/dev/jgit
The former approach is sufficient to build and run the test from the
CLI, the latter is needed to generate eclipse project.
To isolate the JGit rules in its own cell some refactoring was needed.
JGit patch for GWT module was moved to gerrit-patch-jgit project, and
some symlinks were needed for maven machinery to work. include_defs()
doesn't work for now across cell boundaries, and native `buck fetch`
feature still has some limitations: [1]. Moreover, excluding paths,
unsigning JARs and license linking should be re-implemented on top
of it.
[1] https://github.com/facebook/buck/issues/602
Test Plan:
Normal gerrit build and the build with hijacked JGit cell should work
in both standalone (gerrit.war) and Eclipse environment. Note, that
to test --config repositories.jgit=path/to/dev/jgit use case, the most
recent JGit tree must be used, that contains Buck driven build
implementation.
Change-Id: I39f2d5d75bbac88804406d6242b5e714f4916926
We already have the warning "Incomplete 'switch' cases on enum" enabled,
but it does not warn when a case is missing if the switch has a default
block.
Enable the "Signal even if 'default' case exists" option. This would
have enabled us to catch the problem that was fixed in Ied8ff0f8f.
In most instances of the warning we can simply add the missing case(s)
above the 'default', meaning there is no change in behaviour.
However there are some instances where the missing case is a bug and
should be handled correctly. These are fixed separately in follow-up
commits.
Change-Id: I3675d29981423043266a26b1a78932c5708a6272
* stable-2.12:
Document that ldap.groupBase and ldap.accountBase are repeatable
Put Change-Id after Test: footers in commit messages.
Remove bucklets/local_jar.bucklet soft-link to removed lib/local.defs
Normalize case of {Author,Committer}Predicate
OAuth-Linking: Don't create new account when claimed identity unknown
Update 2.11.5 release notes to mention forked buck
Revert "Update buck to ba9f239f69287a553ca93af76a27484d83693563"
Change-Id: I46c53b5c43ecbdc4d63cb03da25c35737b2c5afd
* stable-2.11:
Document that ldap.groupBase and ldap.accountBase are repeatable
OAuth-Linking: Don't create new account when claimed identity unknown
Update 2.11.5 release notes to mention forked buck
Revert "Update buck to ba9f239f69287a553ca93af76a27484d83693563"
Change-Id: I76b92f8fb11cd2f16e6870e3bd219c454a5bfad8
Claimed Identity feature was enabled to support old Google OpenID
accounts, that cannot be activated anymore. In some corner cases,
when for example the URL is not from the production Gerrit site,
like it's always the case on staging instance, the OpenID identity
may deviate from the original one. In case of mismatch, the lookup
for real user for the claimed identity would fail, and the linking
mode is lost, and as the consequence a new account is created.
Creating new account, when user asked for linking is always the
worst option we have, as this cannot be easily undone. Detect this
case, preserve the linking mode and keep trying to link instead of
create new account. Note that in case this account already exist,
the linking would fail with the sane message.
Test Plan I:
1. Create gerrit site gerrit.example.org in year 2010
2. Configure the site to use OpenId (non SSO mode)
3. Observe that 85% user base using Google OpeID
4. After Google's OpenID shutdown in May 2015, nobody is able
to login anymore using their account and link their identity
primary to Launchpad
5. Swap production site to staging site with entire database
and Git repository
6. Install gerrit-oauth-provider plugin and activate automagically
Google OpenID old token discovery and linking option
7. Configure new OAuth application on Google developer console, but
route it to gerrit-test.example.org. Note that this deviation breaks
the old OpenID tokens!
8. Test with old user, that has OpenID Google account, that was
additionally linked to Launchpad OpenID provider
9. Login with Launchpad OpenID iendtity for this user
10. Profile=>Setting=>Link another identity
11. Select Google OAuth provider offered by the gerrit-oauth-provider
plugin
12. Intead of linking to the existing account (or linking error) new
account is created
13. This diff fixed this. Error is issued, that the account already
exist and linking is not possible
Note that when all this would be done on real production site, this
error wouldn't happen, because the URL wouldn't deviate and there
wouldn't be token mismatch between OpenID token in the database and the
token discovered by Google's OAuth OpenID scope. But still, we could
easily prevent in this very specific corner case the creation of new
account.
Note that it's still possible with this setting to create duplicate
account for this user by signing in directly with Google OAuth provider
without linking mode. However, this would also work as expected on the
real production site, because old OpenID token would match with the
existing Gerrit account and linking would happen automagically. That is
exactly why the option link-to-existing-openid-accounts = true was
invented. Unfortunately there is no way to test that this work as
expected already on staging Gerrit instance.
Test Plan II:
1. User register first time after Google's OpenID 2.0 shut down with
OpenID provider, say Launchpad
2. Login with Launchpad
3. Profile=>Setting=>Link another identity
4. Link with Google OAuth2 provider
Expected: The OAuth2 identity is linked to the existing account.
Actual: New account is created.
This diff fixed it. The problem is that I apparently misunderstood
the migration spec: [1] and assumed that the OpenID token is provided
only when a user was already associated with this site. This is
not true. OpenID token also returned for new users, that were never
registered with this site before. To rectify it, and still to work
for both, known and unknown users, we apply the check. If the OpenID
token is known use it for linking. If it is not known, ignore it, but
preserve linking mode.
* [1] https://developers.google.com/identity/protocols/OpenID2Migration#map-identifiers
Change-Id: Icf70cde5fd96cd72aa383218e1d143107a551b45
So that libs such as gerrit-acceptance-framework can depend on this very
one if need be, without growing too much out of unnecessary transitive
deps.
Change-Id: I808af7b09e5a7cf20a578c4740e3a882f7a74e99
Use the java.nio.charset.StandardCharsets.{ISO_8859_1,UTF_8} constants'
name() methods instead of hard-coding the strings.
Where possible, use method variants that take a Charset rather than
a String. This removes the need to catch UnsupportedEncodingException
in some cases.
Change-Id: I4ac1ba0a753de715e1f38ce631842f527b9e127c
Currently there is no easy way to troubleshoot the problems that are
reported in the wild concerning failures to link different identities
using Hybrid OpenID+OAuth authentication scheme. Add some log output
to simplify this job.
For example with this change there should be hopefully clear what is
going on with this issue: [1].
Signing in phase with OpenID provider:
======================================
DEBUG com.google.gerrit.httpd.auth.openid.LoginForm : mode "SIGN_IN"
DEBUG com.google.gerrit.httpd.auth.openid.LoginForm : OpenId provider
"https://login.launchpad.net/+openid"
DEBUG com.google.gerrit.httpd.auth.openid.OpenIdServiceImpl : OpenID:
openid-realm=http://localhost:8080/
Following by Identity linking phase with OAuth provider:
========================================================
DEBUG com.google.gerrit.httpd.auth.openid.LoginForm : mode
"LINK_IDENTIY"
DEBUG com.google.gerrit.httpd.auth.openid.LoginForm : OAuth provider
"http://gerrit-oauth-provider_-google-oauth"
[...]
DEBUG com.google.gerrit.httpd.auth.openid.OAuthSessionOverOpenID :
Linking "34534523945376523984" to "1000000"
[1] https://github.com/davido/gerrit-oauth-provider/issues/46
Change-Id: Ida117aea49b86c3915c738ae1b9d8afd917b5eb9
* stable-2.10:
OAuth: Simplify protocol implementation
Allow to link user identity to another OAuth provider
Update JGit to the 4.0.0.201505050340-m2 version
Hybrid OpenID/OAuth: Support switching identities
Hybrid OpenID/OAuth: Allow to link identity accross protocols
OAuth: Check for session validity during logout
Change-Id: I9da0073a72d8c4327313405b11c66cd253ff640b
This change support all linking directions:
* From OpenID to OAuth
* From OAuth to OpenID
* From OAuth to OAuth
TEST PLAN:
1. Set up vanilla Gerrit site
2. Assign auth scheme to OpenID
3. Install gerrit-oauth-provider plugin
4. Configure GitHub or Google provider (or both)
5. Sign in with source identity
6. Click User => Settings => Identities => Link Another Identity
7. Select target identity from the login form
8. Confirm that the target identity is linked to the source identity
GitHub-Bug: https://github.com/davido/gerrit-oauth-provider/issues/12
Change-Id: I06e5cfc2ad1dde81050b951c0b7f602461af7992
* stable-2.10:
Bump JGit version to 3.7.1.201504261725-r
Hybrid OpenID/OAuth: Check for session validity during logout
RestApiServlet: Leave OutputStream open when flushing base64 padding
RestApiServlet: Flush pending padding as well
Change-Id: Ic26b95ca86f6429969d2608be77ce31af10761d2
* stable-2.10:
Update 2.10.3 release notes
Improve the version computation for the release notes
Check reachability from R_HEADS/R_TAGS/REFS_CONFIG when creating branches
Update 2.10.3 release notes
Include submitter in ChangeMessage on submission
Support hybrid OpenID and OAuth2 authentication
Release notes for Gerrit 2.10.3
Change-Id: I03e0cbb444e9ae2090f510d6f4e49c6db5ece033
e9707d8f85 exposed OAuth authentication extension point. Using this
extension point plugins can offer OAuth2 authentications.
That is fine for new Gerrit sites, which can restrict the auth scheme
to OAuth2 only.
For the existing sites, that rely on non SSO OpenID auth scheme it
doesn't work to migrate to OAuth2 because of diverse contributors
base that use different OpenID providers. Not all OpenID providers
offer OAuth2 protocol. Particularly, widespread OpenID providers
among open source Gerrit communities are Launchpad/UbuntuOne and
FedoraProject don't offer OAuth2 protocol. To not lock out those
contributors from being able to contribute to open source Gerrit
based projects OpenID must still be supported.
With Google's shut down of their OpenID service in April 2015, big
user base is locked out from contribution to Gerrit based projects
that only support OpenID auth scheme.
The only way to still support OpenID 2.0 providers and new OAuth2
based protocol is native support for hybrid authentication scheme
in Gerrit.
This change extends OpenID auth scheme by making it aware of optional
OAuth plugin-based authentication.
When no oauth-provider plugins are deployed, OpenID auth scheme works
as usual. When OAuth provider plugins are deployed, OAuth2 providers
are offered on the OpenID login form, in addition to hard coded Yahoo!
and Launchpad OpenID providers: [1].
[1] http://imgur.com/IcCrChN
Change-Id: I6d70212f4fea5443a6322c7da683e1e943d058eb
* stable-2.10:
Update version to 2.10.2
Release notes for Gerrit 2.10.2
Do not return 403 when clicking on Gitweb breadcrumb
Add log messages to troubleshoot OAuth/OpenID linking
Remove unused OAuthToken in authorisation URL
OnlineReindexer: log the success/failure numbers on exit
Update replication plugin
OAuth: Allow to link claimed identity to existing accounts
OAuth: Allow to change username
Change-Id: Ia9fc371b9f957c8e0fc3e215084baa3d31dadd41
From May 19, 2014[1] Google is no longer allowing new servers to use
OpenID API to authenticate user accounts. From April 20, 2015 Google
will shut down the OpenID service.
Delete Google Account suggestion from the web interface.
[1] https://developers.google.com/+/api/auth-migration#timetable
Change-Id: Idcf4e5a528e574c0042c897db87ea821fbf89315
From May 19, 2014[1] Google is no longer allowing new servers to use
OpenID API to authenticate user accounts. From April 20, 2015 Google
will shut down the OpenID service.
Delete Google Account suggestion from the web interface.
[1] https://developers.google.com/+/api/auth-migration#timetable
Change-Id: Idcf4e5a528e574c0042c897db87ea821fbf89315
- Warn on empty statements, e.g. "for (;;);". These may be
typos and are easily replaced by "for (;;) {}" which is more
explicit.
- Warn on field hiding. This allows cleanup of many acceptance test
members, at the cost of a couple of renames and the occasional
suppression (when the field is in a public nested enum that shadows
a public constant).
- Warn on unnecessary casts.
- Warn on unused declared thrown exceptions. In addition to reducing
method signature length and number of imports, this also eliminated
some impossible catch blocks.
- Warn on missing @Override annotations.
- Warn on unused parameters. This is likely the most controversial,
as a few relatively common patterns require unused parameters in a
way that Eclipse can't ignore. However, it also resulted in cleanup
of a lot of unnecessary injections and method parameters, so I
think the cost was worth it.
Change-Id: I7224be8b1c798613a127c88507e8cce400679e5d
This reverts commit 5db3bba3e2d85662bacedc5d2f215fff7d8d2805.
I was too fast to submit 5db3bba. Besides forgetting to submit the
referenced changes in some submodules some people reported issues
with the interception of the javax.inject.Inject in some JEE containers.
Change-Id: I931ad329d2e7be6f6ce804b8395489a021c8240b
The Guice team is discouraging use of its specific annotation where
possible, to increase JSR330 compliance.
Leave optional injection alone for now, which needs to be manually
replaced with OptionalBinder.
Change-Id: I4f53a518ba6f36fd67af12f3540dc44cbad07ff8
OAuth GitHub authentication method needs proxy properties as well.
Factor it out to ProxyProperties class and prepare it for reuse in
both places.
Change-Id: Id5c05472b1ab04abcf85d25c230c01c56ad542c8
Login screen accept a token to redirect to on successful login but
if the token contained parameters, they were dropped.
Encode/decode the token to fix this issue.
When a URL has a pound sign, it is included in the token as well.
This removes the hardcoded behaviour to include a pound sign in every
redirect URL. Will now be able to use login redirection for URLs with
no pound sign (e.g. GitWeb)
Change-Id: If8a74a74ce11f79389895e4d036b06a1e81abbe5
The code to extract the redirect token is duplicated in several places.
Move the code into LoginUrlToken.
Change-Id: I8b8917f76d6fd0126f78fbb040594d0bb74aa628
java_library() now accepts provided_deps argument which replaces
our custom use of java_library2().
This change replaces compile_deps with provided_deps parameter in
gerrit_{plugin,extension} rules. Plugins that are using it must be
updated.
Change-Id: I16d53793da567c958267f91ca203e9cae6c4d02f
Plugins can replace the existing implementation with the statement:
DynamicItem.bind(binder(), WebSession.class).to(...);
in a module designated as a "<Gerrit-HttpModule>" in the manifest.
Just the Cache implementation used for web sessions can be changed
by binding to a subclass of the now abstract CacheBasedWebSession
which supplies the Cache in the superclass constructor.
This is a step towards solving web session issues with multi-master.
Change-Id: I255661a62cfcbfe07646cb90005764061d618f3d
GWT only needs the rebind code for CSS and ServerLinker to be
precompiled as bytecode. Save build time by passing no source
files to the java_library() used by gwt_module().
For a full draft build of ui_safari this cuts the refresh time
down from 32.015s to 26.158s on my MacBook. Saving 6s on each
UI reload adds up during development.
The common annotations need to be provided as bytecode, avoiding
spurious warnings from GWT when there is a Java syntax error.
Change-Id: I37826498650c65c05303e7d4d1177d05781c56f6
