Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

Unable to login to ownCloud #19

Open
gellenburg opened this issue Jul 22, 2014 · 24 comments
Open

Unable to login to ownCloud #19

gellenburg opened this issue Jul 22, 2014 · 24 comments

Comments

@gellenburg
Copy link

Update: 2014-07-25 - This affects ownCloud 7.0.0-stable as well.


Flock 0.51 is unable to login/ connect to ownCloud 6.0.4 stable.

HTTP Access Logs reveal the following connection attempts:

  • XXX.YYY.ZZZ.AAA - - [21/Jul/2014:18:54:31 -0400] "PROPFIND /.well-known/carddav HTTP/1.1" 302 231 "-" "Jakarta Commons-HttpClient/3.1"
  • XXX.YYY.ZZZ.AAA - - [21/Jul/2014:18:55:47 -0400] "PROPFIND /remote.php/carddav/.well-known/carddav HTTP/1.1" 404 278 "-" "Jakarta Commons-HttpClient/3.1"
  • XXX.YYY.ZZZ.AAA - - [21/Jul/2014:18:56:10 -0400] "PROPFIND //.well-known/carddav HTTP/1.1" 302 231 "-" "Jakarta Commons-HttpClient/3.1"

We see the Flock client query ownCloud for the well-known URI of ownCloud's CardDAV server, and receive an HTTP/302 redirect to /remote.php/carddav/.

We know that ownCloud is working properly in this instance as we can test the response with curl:

XXXXX:~ XXXXX$ curl --request PROPFIND https://XXXXX.XXXXX.XXXXX/.well-known/carddav

<title>302 Found</title>

Found

The document has moved here\.

XXXXX:~ XXXXX$

Yet Flock is attempting to append yet another well-known-URI request operation to the end of the returned URI from the first operation.

@heikoengel
Copy link

Haven't fully understood what's happening here, but CardDavStore::getCurrentUserPrincipal() is checking for SC_MOVED_PERMANENTLY, where OwnCloud is responding with SC_MOVED_TEMPORARILY. When adding SC_MOVED_TEMPORARILY to flock/src/main/java/org/anhonesteffort/flock/webdav/carddav/CardDavStore.java#101, the request is handed over to AbstractDavComponentStore::getCurrentUserPrincipal(uri) with the new location.
However propFindMethod.getResponseBodyAsMultiStatus() in flock/src/main/java/org/anhonesteffort/flock/webdav/AbstractDavComponentStore.java#L122 still throws an exception with code 302, whereas the command line query with curl on the new location seems to return the expected multistatus:

$ curl --request PROPFIND --user username:passwd https://OWNCLOUD_SERVER/remote.php/carddav/
<?xml version="1.0" encoding="utf-8"?>
<d:multistatus xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns" xmlns:card="urn:ietf:params:xml:ns:carddav"><d:response><d:href>/remote.php/carddav/</d:href><d:propstat><d:prop><d:getlastmodified>Thu, 24 Jul 2014 10:37:49 GMT</d:getlastmodified><d:resourcetype><d:collection/></d:resourcetype></d:prop><d:status>HTTP/1.1 200 OK</d:status></d:propstat></d:response><d:response><d:href>/remote.php/carddav/principals/</d:href><d:propstat><d:prop><d:getlastmodified>Thu, 24 Jul 2014 10:37:49 GMT</d:getlastmodified><d:resourcetype><d:collection/></d:resourcetype></d:prop><d:status>HTTP/1.1 200 OK</d:status></d:propstat></d:response><d:response><d:href>/remote.php/carddav/addressbooks/</d:href><d:propstat><d:prop><d:getlastmodified>Thu, 24 Jul 2014 10:37:49 GMT</d:getlastmodified><d:resourcetype><d:collection/></d:resourcetype></d:prop><d:status>HTTP/1.1 200 OK</d:status></d:propstat></d:response></d:multistatus>

@rhodey
Copy link
Contributor

rhodey commented Aug 26, 2014

@gellenburg @rndhro

I just installed owncloud 7 in a lighttpd configuration and added the following rules to my /etc/lighttpd/lighttpd.conf file

url.redirect = (
  "^/.well-known/carddav" => "/owncloud/remote.php/carddav",
  "^/.well-known/caldav" => "/owncloud/remote.php/caldav",
)

This get's me all the way to half-way through the "handleCalDavTestCreateEditCollectionProperties()" test. Here I get an error from SabreDAV complaining about the "org.anhonesteffort.flock" namespace. Switching the namespace to "http://org.anhonesteffort.flock/ns" resolves this issue.

Unfortunately then I run into the error I was experiencing months ago where OwnCloud only retains a subset of calendar properties and if you try to use a custom property (ie X-CUSTOM-PROP-NAME) it silently fails to set the property and when you check for it next tells you it is not there. This is not in spec with WebDAV and should be resolved in the OwnCloud codebase.

Until OwnCloud starts maintaining collection properties correctly there isn't anything I can do to make the Flock client play nicely with OwnCloud. I will however patch Flock to use the "http://org.anhonesteffort.flock/ns" namespace so that we can get closer to OwnCloud friendly.

@heikoengel
Copy link

@rhodey thanks for looking into this! Closing ticket #21 made me think the ownloud side was already fixed.
I stubled upon this conversation: http://owncloud.10557.n7.nabble.com/Impressionist-App-for-ownCloud-td5688.html
This thread is already 2 years old, but the users seems to have similar issues and could resolv them. Namespace adjustments are also mentioned - is this maybe related?

@rhodey
Copy link
Contributor

rhodey commented Sep 18, 2014

@rndhro sorry for the confusion. I'm positive this is an issue with the OwnCloud server and not the Flock client, until someone patches OwnCloud Flock just isn't going to pass the server tests.

Here is the DB structure for calendars in OwnCloud:

CREATE TABLE calendars (
    id SERIAL NOT NULL,
    principaluri VARCHAR(100),
    displayname VARCHAR(100),
    uri VARCHAR(200),
    ctag INTEGER NOT NULL DEFAULT 0,
    description TEXT,
    calendarorder INTEGER NOT NULL DEFAULT 0,
    calendarcolor VARCHAR(10),
    timezone TEXT,
    components VARCHAR(20),
    transparent SMALLINT NOT NULL DEFAULT '0'
);

The issue here is that the WebDAV spec requires that arbitrary properties can be set on WebDAV collections (in CalDAV calendars are modelled as WebDAV collections) while the OwnCloud database structure only allows for a static set of properties, ie "displayname", "description", "calendarorder", etc.

To properly conform to WebDAV, OwnCloud's calendar database structure should be more like...

CREATE TABLE calendars (
    id SERIAL NOT NULL,
    principaluri VARCHAR(100),
    uri VARCHAR(200),
    components VARCHAR(20),
    transparent SMALLINT NOT NULL DEFAULT '0'
);

CREATE TABLE calendar_properties (
    id_calendar SERIAL,
    property_name VARCHAR(1024) NOT NULL,
    property_value VARCHAR(1024) NOT NULL,
    FOREIGN KEY (id_calendar) REFERENCES calendars(id)
);

This would still allow for the "displayname", "description", "calendarcolor", etc properties but would also allow for arbitrary properties to be defined on calendars. I'd be psyched to see something like this patched into OwnCloud but I guess if that doesn't happen for another couple months maybe I'll have to write the patch myself :(

@rhodey rhodey added the wontfix label Sep 18, 2014
@rhodey
Copy link
Contributor

rhodey commented Sep 18, 2014

even though this is not an issue with the Flock client I've decided to keep this issue open for now under the label "wontfix" for purposes of documentation.

@devurandom
Copy link

Is there an ownCloud Calendar issue for this?

@jaromil
Copy link

jaromil commented Sep 19, 2014

just did owncloud/calendar#569

@evert
Copy link

evert commented Sep 21, 2014

Supporting storage of arbitrary properties is not something that is explicitly demanded by the specification. The WebDAV framework that CalDAV builds upon also does not specifically require it, and many servers don't allow it. It's a bit presumptuous to assume that everyone else is wrong.

Many clients do store custom properties on caldav servers, but I haven't seen a single client that does not have adequate fallback behavior. iCal specifically assumes that the server may reject one or more properties in PROPPATCH requests, and therefore actually does 1 PROPPATCH per property, so that if one property is accepted by the server, and others aren't, it doesn't block the entire operation.

However:

  1. This is just talking about WebDAV properties, retention of custom iCalendar properties in calendar objects should be supported. And if a server drops X- properties, it should be treated as a violation of the spec.
  2. Since sabredav 2.0 there is now a facility that does allow defining arbitrary webdav properties on any resource. Documentation. Neither owncloud nor baikal support sabredav 2.0 yet though, as far as I can tell they are both still on 1.8.

Note that the property storage system only allows storing simple string-based properties at the moment. Storing complex xml structures in properties is something we'll work on in the future.

@rhodey
Copy link
Contributor

rhodey commented Sep 23, 2014

Thanks for the background @evert

I will admit that I have been very surprised by the lack of support for custom WebDAV collection properties, retrospectively it makes sense that I should have done some more research on existing WebDAV server implementations. Section 9.2 of RFC 4918 states that "DAV-compliant resources SHOULD support the setting of arbitrary dead properties" so you are correct in stating that it is not demanded.

Unfortunately there really isn't any acceptable fallback behavior that Flock can take when arbitrary collection properties are not supported because 4 new "arbitrary dead properties" are defined and required, namely:

  1. X-FLOCK-COLLECTION - collections without this property are ignored by Flock.
  2. X-FLOCK-HIDDEN-CALENDAR-COLOR - calendar color is traditionally defined as an integer, this property allows Flock to store color as base64 encoded ciphertext.
  3. X-KEY-MATERIAL-SALT - defined on the "key collection", the salt for all cryptographic opertations.
  4. X-ENCRYPTED-KEY-MATERIAL - defined on the "key collection", encrypted key material used to bootstrap new clients.

There are some things that could be done to hack these properties into webdav servers that reject arbitrary dead properties, eg packing them into a special-case .ical or .vcard resource and storing that within the collection-- but IMO this is a really ugly hack and I'd rather WebDAV servers be patched to support the recommended behavior.

There is a really and dirty SabreDAV (and thus ownCloud) patch that would make Flock happy:

ALTER TABLE calendars ADD COLUMN x_flock_collection INTEGER;
ALTER TABLE calendars ADD COLUMN x_hidden_calendar_color VARCHAR(1024);
ALTER TABLE calendars ADD COLUMN x_key_material_salt VARCHAR(1024);
ALTER TABLE calendars ADD COLUMN x_encrypted_key_material VARCHAR(1024);

The default for all of these new columns would be null so this alone wouldn't break or affect the server. All that would be needed after this is to patch whatever code applies the displayname, description, etc. properties to include these 4 new props. I might end up working on this today but would prefer that arbitrary collection properties be supported instead of just these 4 because I would like the option of defining more properties in the future.

Quick note on iCal: the response to a PROPPATCH is multi-status meaning you can update multiple properties in a single PROPPATCH and then check the response to see which succeeded, each property has its own http status result code. Flock checks each response code within the multi-status but still fails because it can't do without these 4.

@evert
Copy link

evert commented Sep 23, 2014

Hi!

The properties you are talking about, sound more like iCalendar properties as opposed to WebDAV properties to me. Is there some confusion going on here?

You uppercase them and prefix them with X- which is typical for embedding custom data in iCalendar, and CalDAV servers definitely should retain these.

@rhodey
Copy link
Contributor

rhodey commented Sep 23, 2014

@evert If I had the chance to rename these properties I would have dropped the X- prefix :) it is in-fact due to a prior misunderstanding of property naming conventions. However, these properties are being set on WebDAV collections (calendars in CalDAV) and not resources (.ical files in CalDAV).

There are additional properties which Flock sets on WebDAV resources (.vcard & .ical files) but thankfully there are no issues with these:

  • X-FLOCK-HIDDEN - VCard property used to store ciphertext of VCard
  • X-FLOCK-HIDDEN-PHOTO - VCard property used to store ciphertext of photo (if present).
  • X- FLOCK-HIDDEN-CALENDAR - iCal property used to store ciphertext of iCal

If I had the chance to drop the X- prefix in the new Flock WebDAV properties I would also take the chance to drop X-FLOCK-HIDDEN-CALENDAR and use X-FLOCK-HIDDEN in both VCard and iCal to store ciphertext.

@evert
Copy link

evert commented Sep 23, 2014

Makes sense then! Unfortunately my earlier comment does stand... We added support for storing any arbitrary properties, but the wait is for Baikal and Owncloud to upgrade to SabreDAV 2 and add the plugin:

http://sabre.io/dav/property-storage

Crossing fingers that they will for their next releases :)

@rhodey
Copy link
Contributor

rhodey commented Sep 23, 2014

@evert I remember testing Flock with ownCloud many months ago, running into this arbitrary property problem, and thinking "damn, well I've still got a couple months till release-- hope they fix by then!" haha so yeah, looks like unfortunately no relief presently.

Thanks for your work on SabreDAV :) property-storage looks great!

@jaromil
Copy link

jaromil commented Oct 14, 2014

Good work on this guys, thanks for following up. One more reason to get property-storage into SabreDAV 2: it will fix so many webcalendars! not just Owncloud, but also Roundcube for instance. This is nailing down a long term problem that prevented many people around the world from using their own synced calendar on their own servers.

@timeggleston
Copy link

I've created a bounty for this over at OwnCloud:

https://www.bountysource.com/issues/4482683-support-arbitrary-properties-conforming-to-caldav-spec

Although I suppose that may be a little optimistic if they need to switch out their SabreDAV version...

@rhodey
Copy link
Contributor

rhodey commented Nov 28, 2014

@timeggleston thanks for organizing this :) I'll throw into the bounty as soon as I get an account setup.

@untitaker
Copy link

@rhodey Fallback behavior that comes to mind: You could create a special resource with a very specific, not randomly generated href. I.e. for CalDAV you upload a resource called flock-properties.ics, which could contain a valid VCALENDAR, but also any data you wanted to store inside a collection property.

EDIT: nvm didn't read that you already thought about this

@monolithonadmin
Copy link

What about owncloud 8?

@untitaker
Copy link

@Muvuk ownCloud uses SabreDAV 1.8, support for arbitrary props was introduced in 2.0.

@devurandom
Copy link

@untitaker So when does OC plan to update to SabreDAV 2?

@untitaker
Copy link

@devurandom I don't know, I am not affiliated with them.

@joeykrim
Copy link

Latest I could find on ownCloud and SabreDev 2.0 is from a blog post on the ownCloud blog on April 14th 2015: "Vincent will be part of the work to update sabre/dav in ownCloud to version 2. It requires some “cleanup and re-factoring,” but stands to make “our code more beautiful.” The upgrade will be preceded by “adding unit tests everywhere to make sure the upgrade is smooth.”"

@scroom
Copy link

scroom commented Jul 12, 2015

Does OC 8.1 solve the issue?

@ichEben
Copy link

ichEben commented Jul 12, 2015

I'm using OC 8.1 and the problem still exists

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests