Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implements CurrentUserPrivilegeSet to add compatibility with thunderb… #170

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mheers
Copy link

@mheers mheers commented Sep 23, 2024

…ird address book

Copy link

@oliverpool oliverpool left a comment

Choose a reason for hiding this comment

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

I just got the exact same issue myself and solved it before seeing your PR 🙈

internal/elements.go Outdated Show resolved Hide resolved
Comment on lines 424 to 429
Privileges []Privilege `xml:"privilege"`
}
type Privilege struct {
Read bool `xml:"read"`
Write bool `xml:"write"`
}

Choose a reason for hiding this comment

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

In my case, I got it to work with Thunderbird locally with the following structure (didn't tried yours TBH):

Suggested change
Privileges []Privilege `xml:"privilege"`
}
type Privilege struct {
Read bool `xml:"read"`
Write bool `xml:"write"`
}
Privileges []Privilege `xml:"privilege>dynamic"`
}
type Privilege struct {
XMLName xml.Name
}

And then return read, write like this:

return &internal.CurrentUserPrivilegeSet{Privileges: []internal.Privilege{{XMLName: xml.Name{"", "read"}}, {XMLName: xml.Name{"", "write"}}}}, nil

(ideally the read/write privileges information should come from the backend)

Copy link
Owner

Choose a reason for hiding this comment

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

Seems like the second part of the suggestion hasn't been applied. (But I prefer the struct approach.)

@oliverpool
Copy link

Reference: https://stackoverflow.com/a/57124617/3207406

mheers and others added 2 commits October 5, 2024 07:55
// https://datatracker.ietf.org/doc/html/rfc3744#section-5.4
type CurrentUserPrivilegeSet struct {
XMLName xml.Name `xml:"DAV: current-user-privilege-set"`
Privileges []Privilege `xml:"privilege>dynamic"`
Copy link
Owner

Choose a reason for hiding this comment

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

Nit: this file use the same name as the XML element even if there may be multiple.

// https://datatracker.ietf.org/doc/html/rfc3744#section-5.4
type CurrentUserPrivilegeSet struct {
XMLName xml.Name `xml:"DAV: current-user-privilege-set"`
Privileges []Privilege `xml:"privilege>dynamic"`
Copy link
Owner

Choose a reason for hiding this comment

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

Why dynamic? This doesn't appear in the RFC AFAIK.

XMLName xml.Name `xml:"DAV: current-user-privilege-set"`
Privileges []Privilege `xml:"privilege>dynamic"`
}
type Privilege struct {
Copy link
Owner

Choose a reason for hiding this comment

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

Instead of a list, could we use a struct with fields for read/write?

@oliverpool
Copy link

FYI my PR is ready for review: #172 (supports user-defined ReadOnly)

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.

3 participants