Previously, the current default country was used to turn phone numbers
without an explicit country code into full E164 numbers, which then
had to match the search term when doing a caller ID lookup.
This was inconsistent with EDS, where a weaker
EQUALS_NATIONAL_PHONE_NUMBER was done. The difference is that a
comparison between a number with country code matches one without if
the national number of the same, regardless of the current default
country. This is better because it reduces the influence of the hard
to guess default country on matching.
SyncEvolution also differed from EDS by doing a prefix comparison,
which in theory might have also ignored differences caused by
extensions. It is uncertain whether that was useful, so for the sake
of consistency, a full number comparison of the national number is now
done.
Another advantage of this change is the lower memory consumption and
faster comparison, because strings are now stored in 4 + 8 byte
numbers instead of strings of varying length.
The array operator happens to work on some platforms, but not others
(see previous commit). Make it private without an implementation to
catch the undesired usage of it on platforms whether the code would
happen to work otherwise.
This fixes the following problem, seen with Boost 1.53.0 on altlinux
when compiling for EDS >= 3.6:
/usr/include/boost/smart_ptr/shared_ptr.hpp: In instantiation of 'typename boost::detail::sp_array_access<T>::type boost::shared_ptr<T>::operator[](std::ptrdiff_t) const [with T = char*; typename boost::detail::sp_array_access<T>::type = void; std::ptrdiff_t = long int]':
src/backends/evolution/EvolutionSyncSource.cpp:163:38: required from here
/usr/include/boost/smart_ptr/shared_ptr.hpp:663:22: error: return-statement with a value, in function returning 'void' [-fpermissive]
make[2]: *** [src/backends/evolution/src_backends_evolution_syncecal_la-EvolutionSyncSource.lo]
The "void" type above is wrong, so it looks like a missing type trait
for the pointer type used in the smart_ptr. PlainGStrArray already had
an at() method to work around such issues, so use it. Not sure why this
one usage of [] slipped through.
While running a sync with a binary compiled with -fPIE -pie, a crash
in strlen() occured because a 64 bit string pointer coming from D-Bus
was incorrectly passed through a 32 bit unsigned variable.
These special compile flags merely caused the problem to occur
reliably, it may also have crashed under other circumstances.
Kudos to Tino Keitel for reporting the problem and identifying the
relation to the compile flags.
Running a sync while the UI had no service selected caused a crash in
find_updated_source_progress() because the code dereferences the NULL
prog_data->data->current_service pointer. Affected both the GTK2 and
GTK3 UI.
Fix it by checking for NULL and not doing anything if NULL.
When compiling source files of client-test, use -g as default CXXFLAGS
instead of the "-g -O2" that autotools normally picks. That speeds up
compilation significantly (on some platforms, gcc can't deal with the
many templates in ClientTest.cpp well) and leads to more useful
executables (suitable for interactive debugging) even when the rest of
the code gets optimized.
Explicitly specifying CXXFLAGS still overrides this per-target
default.
This feature depends on GNU make. A configure check is in place
to disable it when not using GNU make.
It seems that sometimes setting up a session with GNOME keyring fails such
that all further communication leads to decoding problem.
There is an internal method to reset the session, but it cannot be called
directly. As a workaround, fake the death of the GNOME keyring daemon
and thus trigger a reconnect when retrying the GNOME keyring access.
This drops the support for libgnome-keyring < 2.20, because older
versions did not have the error->text conversion method which is now
used in revised error and log messages.
This code also adds a retry loop around reading/writing passwords.
This was meant to work around these intermittent Gkr errors:
Gkr: received an invalid, unencryptable, or non-utf8 secret
Gkr: call to daemon returned an invalid response: (null).(null)()
These lead to an "Error communicating with gnome-keyring-daemon" status
code from libgnome-keyring.
However, once the error occurred in a process, it persists for at least
two seconds, possibly forever. Therefore the retry loop is not enabled.
Google CardDAV has one peculiarity: it renames new contacts during PUT without
returning the new path to the client. See also
http://lists.calconnect.org/pipermail/caldeveloper-l/2013-July/000524.html
SyncEvolution already had a workaround for that (PROPGET on old path, extract
new path from response) which happened to work. This workaround was originally
added for Yahoo, which sometimes merges contacts into existing ones. In
contrast to Yahoo, Google really seems to create new items.
Without some server specific hacks, the client cannot tell what happened.
Because Google is currently supported and Yahoo is not, let's change the
hard-coded behavior to "renamed items are new".
The nightly testing configures some platforms such that
XDG_CONFIG/DATA/CACHE_HOME are inside the build dir. It also populates these
dirs with files (for example, GNOME Online Accounts) which must survive all
cleaning of these directories.
Long term it would be better to separate test files from build files,
but that's a task for some other time...
When running a local sync, the syncURL/username/password are not meant
for the sync and cannot be used if they refer to an AuthProvider which
cannot return plain username/password.
In all other cases, this may or may not work, so at least try it instead
of hard-coding the IdentityProviderCredentials.
"username = goa:..." selects an account in GOA and retrieves the
OAuth2 token from that.
The implementation uses the GOA D-Bus API directly, because our C++
D-Bus bindings are easier to use and this avoids an additional library
dependency.
When clients like the GTK sync-ui stored a password, it was always
stored as plain text in the config.ini file by the
syncevo-dbus-server. The necessary code for redirecting the password
storage in a keyring (GNOME or KWallet) simply wasn't called in that
case.
The command line tool, even when using the D-Bus server to run the
operation, had the necessary code active and thus was not affected.
The client template is also used in cases where passwords are not
needed (local sync) and where passwords cannot be stored in a keyring
due to the missing syncURL/remoteDeviceID. Therefore don't set dummy
username/password values in the template.
When configuring a new peer and looking for databases, we need the
database password of an already existing source config, otherwise the
lookup will fail if that password is hidden in a keyring.
The command line tool in --daemon=no mode did not use the GNOME
keyring or KWallet even if the syncevo-dbus-server did, leading
to failing test cases when actually starting to use it by default
there.
Now all components use the same default: use safe password storage if
any was enabled during compilation, don't use if not.
This also makes SyncEvolution work without user intervention on
systems without a password storage.
Figuring out where credentials come from became harder. These debug
messages help. Perhaps they should even be logged as INFO messages
such that normal users can see them?
The code works with gSSO (https://01.org/gsso). With some tweaks to
the configure check and some ifdefs it probably could be made to work
with Ubuntu Online Accounts.
The code depends on an account accessible via libaccounts-glib which
has a provider and and (optionally) services enabled for that
provider. It is not necessary that the account already has a signon
identity ID, the backend will create that for the provider (and thus
shared between all services) if necessary.
Therefore it is possible to use the ag-tool to create and enable the
account and services. Provider and service templates are in the next
commit.
If given an AuthProvider which can handle OAuth2, then OAuth2 is
used instead of plain username/password authentication.
Obtaining the OAuth2 token must be done at a point where we can still
abort the request. If obtaining the token fails, then this should be
considered a fatal error which aborts scanning for resources. Other
errors cause the current URL to be skipped while scanning continues.
This commit moves the "execute request" functionality back into the
Neon::Session class, because that is where most of the logic (retry
request?) and state is (access tokens which persist across requests).
The real username is only relevant when running a sync. When looking
at a config with a D-Bus client like the GTK UI, the username should
always be "id:<config>", to avoid accidentally removing the
indirection, while the password should be the real one, to allow the
user to edit like he normally would with passwords stored in a
keyring.
To achive this, overriding the username must be suppressed when
resolving as part of the D-Bus config API. While at it, move the
entire "iterate over properties" into a common utility function in
PasswordConfigProperty.
Storing a password with just "user=foo" as lookup attributes is problematic
because it is too unspecific. Different services or configs with the same
user, but different passwords end up overwriting each other's passwords. In
practice, the config with "user=foo" even had the effect of removing the entry
for "user=foo server=bar".
The situation can be avoided by using the remotePeerId as fallback when the
syncURL is empty. There is a (minor?) risk that some configs were stored
in the past without that additional key and now won't be found anymore in the
keyring. Users need to re-set such passwords.
If an attempt is made to store a password with insufficient lookup attributes,
GNOME keyring will now reject the attempt.
When instantiating multiple SyncConfig instances, it is important that
they share filter nodes and the underlying tree, because the nodes
store copies of already retrieved credentials (lookup shall only be
done once!) and the trees represent the current content of the config
(which must be consistent when making changes).
Currently the new code is not thread-safe, but nor are nodes and trees,
so a lot more work would be needed to make this safe. Instead we avoid
concurrency.
SyncConfigTest::normalize() only passed because FileConfigTree accidentally
created the "peers" directory inside the peer. That will change, so don't rely
on that. Instead ensure that the config.ini file of the peers gets written
because it contains something.
Instantiating LogDirTest used to create a SyncContext and use that as logger
for the entire duration of testing inside client-test, even when not running
LogDirTest tests at all. This is undesirable and together with caching of the
config tree while in use, broke some other tests (EvolutionCalendarTest)
because obsolete DB names were shared.
It is better to create the context during setUp() and remove it in tearDown().
The previous approach made FileConfigTree more complex than necessary.
Having an abstract ConfigTree::getRootPath() with undefined behavior
is bad design.
The code was had undesiredable side effects: inside a peer config,
another "peers" directory was created because FileConfigTree didn't
know whether creating that dir was required or not.
Now all of the complexity is in SyncConfig, which arguably knows
better what the tree stands for and how to use
it.
In practice, the methods are always called for a specific SyncConfig.
Passing that allows removing several other parameters and, more
importantly, also grants access to the config and through that other
configs. This will be needed for the indirect credential lookup.
"username", "proxyUsername" and "databaseUser" used to be simply a
string containing the name of the respective user or (in the case of
the ActiveSync backend) the account ID in gconf.
Now it is also possible to provide credentials (username + password)
indirectly: when any of these properties is set to "id:<config name>",
then the "username/password" properties in that config are used
instead. This is useful in particular with WebDAV, where credentials
had to be repeated several times (target config, in each database when
used as part of SyncML) or when using a service which requires several
configs (Google via SyncML and CalDAV).
For user names which contain colons, the new "user:<user name>" format
must be used. Strings without colons are assumed to be normal user
names.
This commit changes the SyncConfig APIs for this extension. More work
is needed to make the indirect lookup via "id" functional.
Passwords are cached after the initial check as temporary property
values. The explicit string members are obsolete and can be removed
together with the code using them.
Using asynchronous method calls did not eliminate the default timeout,
as expected. In particular, SyncPeer() still timed out when syncing many
contacts.
Instead set up all necessary parameters (= callbacks and now also a
long timeout) in a hash and pass that to all D-Bus method calls. It's
less code duplication, too.
Using the underscore in the UID has been wrong all along, it only
happened to work because UID sanity checking was missing. After adding
it, the example broke.
Now simply remove the colon. It makes the UID less readable, but it
doesn't have to be, and ensures that file names and database names
contain the UID as-is.
Only saw this once in nightly testing and couldn't reproduce it:
$ make -j 16
perl /data/runtests/work/sources/syncevolution/src/syncevo/readme2c.pl
/data/runtests/work/sources/syncevolution/README.rst
>src/syncevo/CmdlineHelp.c
/usr/bin/xsltproc -o src/dbus/interfaces/syncevo-server-doc.xml
/data/runtests/work/sources/syncevolution/src/dbus/interfaces/spec-to-docbook.xsl
/data/runtests/work/sources/syncevolution/src/dbus/interfaces/syncevo-server-full.xml
/usr/bin/xsltproc -o src/dbus/interfaces/syncevo-connection-doc.xml
/data/runtests/work/sources/syncevolution/src/dbus/interfaces/spec-to-docbook.xsl
/data/runtests/work/sources/syncevolution/src/dbus/interfaces/syncevo-connection-full.xml
/usr/bin/xsltproc -o src/dbus/interfaces/syncevo-session-doc.xml
/data/runtests/work/sources/syncevolution/src/dbus/interfaces/spec-to-docbook.xsl
/data/runtests/work/sources/syncevolution/src/dbus/interfaces/syncevo-session-full.xml
/usr/bin/glib-genmarshal
/data/runtests/work/sources/syncevolution/src/dbus/glib/syncevo-marshal.list
--header --prefix=syncevo_marshal > src/dbus/glib/syncevo-marshal.h
runtime error
xsltApplyStylesheet: saving to src/dbus/interfaces/syncevo-session-doc.xml may
not be possible
/usr/bin/xsltproc -o src/dbus/glib/syncevo-server.xml
/data/runtests/work/sources/syncevolution/src/dbus/interfaces/spec-strip-docs.xsl
/data/runtests/work/sources/syncevolution/src/dbus/interfaces/syncevo-server-full.xml
runtime error
xsltApplyStylesheet: saving to src/dbus/interfaces/syncevo-server-doc.xml may
not be possible
make: *** [src/dbus/interfaces/syncevo-server-doc.xml] Error 9
make: *** Deleting file `src/dbus/interfaces/syncevo-server-doc.xml'
make: *** Waiting for unfinished jobs....
make: *** [src/dbus/interfaces/syncevo-session-doc.xml] Error 9
make: *** Deleting file `src/dbus/interfaces/syncevo-session-doc.xml'
Looks like multiple xsltproc commands ran in parallel and then stepped on each
others toes while creating the src/dbus/interfaces directory, which does not
exist after an out-of-tree configure.
To address the issue, serialize creating that directory by having make create
it as a prerequisite.
While there are sessions pending or active, the server should not shut down.
It did that while executing a long-running PIM Manager SyncPeer() operations,
by default after 10 minutes.
This was not a problem elsewhere because other operations are associated with
a client, whose presence also prevents shutdowns. Perhaps PIM Manager should
also track the caller and treat it like a client.
