Skip to content

caldav + carddav: add principal to resourcetype flag for UserPrinicipal request.#193

Closed
prasad83 wants to merge 8 commits intoemersion:masterfrom
prasad83:master
Closed

caldav + carddav: add principal to resourcetype flag for UserPrinicipal request.#193
prasad83 wants to merge 8 commits intoemersion:masterfrom
prasad83:master

Conversation

@prasad83
Copy link
Contributor

For PROPFIND principalPath request - the response resourcetype should add principal attribute.

<resourcetype xmlns="DAV:">
  <collection xmlns="DAV:"></collection>
  <principal xmlns="DAV:"></principal> <!-- required -->
</resourcetype>

Without this Thunderbird Calendar setup was not making call get List of Calendars

Added PrincipalName variable reference to internal/elements.go
Add principal resourcetype to user principal responses to properly
identify the resource hierarchy for CalDAV clients (Thunderbird)
Copy link
Owner

@emersion emersion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! LGTM overall, just two minor comments!

@emersion
Copy link
Owner

Also, can you rework the commit title to just use the package name as prefix instead of conventional commits? We don't use conventional commits.

@prasad83 prasad83 changed the title fix(caldav): add principal to resourcetype required for Thunderbird CalDAV client. caldav + carddav: add principal to resourcetype flag for UserPrinicipal request. Oct 17, 2025
@prasad83
Copy link
Contributor Author

Updated PR title as recommended. Thanks

current-user-privilege-set should be valid when propfind is issued.
Failing to respond will lead to unexpected behaviour at client-end.

Refer: [rfc3744#section-5.4](https://tools.ietf.org/html/rfc3744#section-5.4)

For now implemented read-and-write as allowed.
@prasad83
Copy link
Contributor Author

prasad83 commented Oct 17, 2025

@emersion - I have also handled current-user-privilege-set prop request (Thunderbird was issuing this when AddressBook was being setup). Took struct approach to set as you have cited on #170

Copy link
Owner

@emersion emersion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've made some changes to your PR. Unfortunately the PR doesn't have the "allow edits from maintainers" checkbox ticked so I had to push the edits to master directly as 0e74e6d and a16253a.

In the future, please open a separate PR instead of pushing multiple orthogonal changes to a single PR.

GetETagName = xml.Name{Namespace, "getetag"}

CurrentUserPrincipalName = xml.Name{Namespace, "current-user-principal"}
CurrentUserPrivilegeSet = xml.Name{Namespace, "current-user-privilege-set"}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed to CurrentUserPrivilegeSetName

}

// https://tools.ietf.org/html/rfc3744#section-5.4
type CurrentUserPrivilege struct {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed to CurrentUserPrivilegeSet

// https://tools.ietf.org/html/rfc3744#section-5.4
type CurrentUserPrivilege struct {
XMLName xml.Name `xml:"DAV: current-user-privilege-set"`
Privilege Privilege
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There can be multiple of these in the element definition

{ContentType: vcard.MIMEType, Version: "4.0"},
},
}),
internal.CurrentUserPrivilegeSet: func(raw *internal.RawXMLValue) (interface{}, error) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

internal.PropFindValue can be used here.

The same should be applied to caldav.

@emersion emersion closed this Oct 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants