-
-
Notifications
You must be signed in to change notification settings - Fork 3
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
add DCLOGIN
uri scheme
#48
Conversation
for cbor: one should also take into account, that the code needs to be generated somehow, by ppl not that deep into development maybe, cbor is not the best choice in that regard. it is not that know and has less support than url-parameter or base64 or json. in general, i am wondering if json is a good format for this purpose. using the url-parameter-format as for the other schemes has some advantages:
of course, there are also drawbacks :) at least i want to bring up this point more clearly, i've seen it is already partly discussed above :) interesting, while not totally matching: https://datatracker.ietf.org/doc/html/rfc5092 |
given the available size and the scanning by other apps i guess i feel more for using a plain URI format that's well known. Could flatten everything to query parameters etc, which is all well supported in many languages also for generation like r10s suggests. |
So you are all more in favour of implementing the uri encoding/decoding?
uri encoding/decoding is easily forgotten and then people wonder why it does not work. (like it worked with this simple password, but does not work with my super secure passwords that contain special characters (which I don't know that they need special encoding so I blame it on DC)) So going for a binary format forces people to think about the documentation. Also I would create a web tool to generate it.
msgpack(https://msgpack.org) and cbor(https://cbor.io/impls.html) have implementations for most popular programming languages. The thing about json (or binary json) is that it is easier for us to work with, than uri encoding which can have a someencoding edge cases like macOS encoding the The idea of the version part is that we can change the format, if we decide on the uri-scheme format from the start we could also put the version into a query parameter, I believe we should still have a version, especially if we later might want to allow stuff like oauth authentication in those links/qr-codes, and then we need a way for the old versions of deltachat to tell to the user "this requires a newer version of DC, please update". an uri based version was already proposed in the forum:
which follows the url conventions a bit, kinda like (https://datatracker.ietf.org/doc/html/rfc5092) There is still the question of how to shorten the parameters, but I don't think it is even needed (we have ~1000 chars, with all parameters set we might get 400-500), also shortening them to single letters makes the format more complicated again. I'd be in favour of the url based variant though I'm worried the escaping stuff will get too complicated. |
thanks for all the considerations! maybe, the need of a webtool seems to underline that things are designed too complex :) also, having a webtool does not really help if you want to generate the code automatically somehow. also, this is questionable security wise as one has to enter the password on some platform (even if that is not uploaded somewhere, one has to explain that etc.) (or install the tool locally, which is also some effort) i would not die on that hill, tbh, i am also quite a bit unsure about the whole scope of DCLOGIN and why it is needed now, however, i would go the easy path. another thing: consider also to adapt the existing DCACCOUNT scheme as that is handled more and more by the different UIs. from the view of the UI, both things work the same i think? EDIT: ahh: and there is currently also a BACKUP-TRANSFER QR-code evolving: deltachat/deltachat-core-rust#3489 - at least that targets a similar field. |
It's needed for a project.
data at some web endpoint (DCACCOUNT) vs data inside in the qrcode (DCLOGIN). Yes UI is similar, maybe different wording but apart from that it can share the same code base.
yeah similar field, but also very different So I will update this pr to propose a uri based approach soon, binary brings it's own complexities so we can try the uri encoding stuff. |
Changed it to a fully URI based approach, what do you think now? |
uri-schemes.md
Outdated
dclogin://user:password@host?options | ||
dclogin:user:password@host/?options | ||
# example: (email: me@example.com, password: securePassword) | ||
dclogin://me:securePassword@example.com?v=1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure if it is a thing but IIRC some servers use only the local part of the email address as the username to log in, how would that be specified here? or this is only for servers that the login is with the email address and that the server address ends with @host
and host is the right imap/smtp host? I guess it will have then to do the DC autoconfig to detect right host and ports
personally would have preferred something like dclogin://addr:password?mail_server=...&send_server=...&(other dc config options here)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and then calling it dcsetup:
would makes more sense since you could setup several config options for the user, would be more useful for admins setting up servers for easy onboarding, example if creating the DC account is triggered from inside another service /account and then you could preconfigure the same username that the user has in that other service
| ---------- | ------------------------- | --------------------------------------------------- | --------------------- | | ||
| `v` | `version` | defines the format version, more explanation below | `v=1` | | ||
| ---------- | ------------------------- | --------------------------------------------------- | ---------------- | | ||
| `ih` | `imap_host` | IMAP host | `ih=imap.example.com` | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mmh ok so actually the host can be specified in options, then splitting the email address in local part and host kinda feels unnecessary 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's because url spec has username:password@host
as format, so to be consistent with other schemes/standards
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
following this "auth spec" seems not to be needed and does not really match anyway as we have to deal with potentially two different usernames and passwords. and if we follow: the part before the colon is the username, so that would be the full email address in many, but of course not all cases.
so, trying to reconstruct the email address and username from different parts is doable, but would results in quite some "if" and unneeded complexity.
it seems much easier to have a dedicated field for the email address: DCLOGIN:?a=foo%40bar.de&ipw= password
just map everything 1:1 to the existing config fields. having that as a parameter also eliminates doubts how and if to urlencode, it is just all the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure we need to "simplify" everything that much,
maybe DCLOGIN:email@example.com?pw=1234&v=1
, but I don't see the big benefit in specifying everything in the query part, though we could do it, minimum version (no advanced options) would then look like:
DCLOGIN:?a=email@example.com&p=1234&v=1
and when reusing the existing imap password field (though that might be more annoying to explain than an extra property):
DCLOGIN:?a=email@example.com&ipw=1234&v=1
BTW: the v=1
is there for future proofing, that we can increase this number to force specific advanced options like oauth2 or similar in the future, if it's a number not supported by dc it will tell the user to update the app.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also can have separate passwords for IMAP & SMTP, right? So what about recommending DCLOGIN://user:password@example.com/?v=1
as default, not having a p=
option at all, but instead smtp_password=
and imap_password=
options, which override the password in the "authority" field?
That would feel most intuitive for me, and kind of maps the login field with the advanced login settings.
edit: I just saw that there are smtp_password
and imap_password
parameters already 🙃 I like to have the password in the front, as it makes it clearer what overrides which imho.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not really willing to change everything at this stage again, r10s is right that it might be more complicated to do it in the userinfo part (because URI encoding and stuff like that, passwords tend to contain special characters)
uri-schemes.md
Outdated
dclogin:user@host?p=password&v=1[&options] | ||
dclogin:user@host/?p=password&v=1[&options] | ||
dclogin://user@host/?p=password&v=1[&options] | ||
# example: (email: me@example.com, password: securePassword) | ||
dclogin://me@example.com?p=securePassword&v=1 | ||
# example: (email: myself@example.com, password: url/Encoded\@passw@rd) | ||
dclogin://myself@example.com?p=url%2FEncoded%5C%40passw%40rd&v=1 | ||
# example: (email: myself@example.com, password: 123456, insecure smtp at different server) | ||
dclogin://myself@example.com?p=123456&v=1&sh=mail.example.com&sc=3&ss=plain |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you're diverging a bit far from the URI syntax.
I'd refer to the URI spec and say:
- must have
userinfo
- must have
host
- must be single
/
for path - query parameters ad defined
This means example 1 and 2 should be considered invalid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so path
is allowed to be emtpy, may as well omit /
in that case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So force using ://
at beginning and no / for the path?
It is parsed by an URL parser lib we already use in core so path is just ignored anyway but we could still remove that example from the spec.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That the existing parser lib parses it is already great. As usual being strict in the spec but liberal in parsing is a good thing probably.
Yes to requiring ://
as separator (though if parsing without works then that's great), so I'd remove the examples without it. All the other examples are already following the URI spec I think.
And maybe it's also worth specifically describing the syntax wrt to URI schema. Something like:
The generic URI schema is:
URI = scheme ":" ["//" authority] path ["?" query] ["#" fragment]
authority = [userinfo "@"] host [":" port]
For DCLOGIN this means:
- *scheme* is always dclogin
- There must be an *authority* section
- The *userinfo* part is required.
- The *path* may be omitted or be a single `/`.
- The fragment must be omitted.
- The query parameters ... (as currently described)
The benefit of this is that it explicitly acknowledges the URI format and signals that we're aiming to be compatible. Though I won't mind too much if you don't agree with this.
| short name | stands for | description | example | | ||
| ---------- | ------------------------- | --------------------------------------------------- | --------------------- | | ||
| `v` | `version` | defines the format version, more explanation below | `v=1` | | ||
| `p` | `password` | required in version 1, password of the account | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would not require p=
in the options if it is supplied in the userinfo part.
What do you think about this specification?
If there are no issues I'll implement it in the next days.
Implementation state
dcaccount:
anddclogin:
scheme deltachat-desktop#2823Related Forum topics:
https://support.delta.chat/t/configure-email-account-via-qr-code/1805
https://support.delta.chat/t/configure-dc-account-through-qr-code/1164
https://support.delta.chat/t/allow-users-to-generate-qr-codes-to-simplify-the-login-process/1406