search

elementary / portals

https://elementary.io
GNU General Public License v3.0
23 stars 6 forks source link

Evolution credentials portal #83

Closed leolost2605 closed 1 year ago

leolost2605 commented 1 year ago

This is a protoype for an evolution credentials portal that allows flatpak apps to access passwords

It's somewhat working but I would like to get some feedback on some things before finishing/fine tuning the api. So this can be seen as a proof of concept/suggestion and if there are too many problems with this approach it should be rejected.

Currently it works like this:

  1. An application calls the portal and asks for credentials for an evolution source
  2. The portal checks whether this already has been allowed/forbidden and if no permissions were found it shows a dialog asking the user whether the app should get access to the users online accounts (the app actually only asks for access to the credentials but I guess this phrasing is good enough to not confuse the user)
  3. If the request is accepted by the user or an "allow" permission was found the portal returns the credentials stored for the account if they can be found

Now some things about the technical side:

Now some things about the general "flow" and way of implementation:

Please let me know your thoughts about this!

For more discussion see elementary/switchboard-plug-onlineaccounts#209 elementary/tasks#263 elementary/mail#591

Closes #19

Marukesu commented 1 year ago

Currently it works like this:

  1. An application calls the portal and asks for credentials for an evolution source
  2. The portal checks whether this already has been allowed/forbidden and if no permissions were found it shows a dialog asking the user whether the app should get access to the users online accounts (the app actually only asks for access to the credentials but I guess this phrasing is good enough to not confuse the user)
  3. If the request is accepted by the user or an "allow" permission was found the portal returns the credentials stored for the account if they can be found

IMO, i think you are making the some mistakes with the portal direction:

i also though more about this since the proposed API, and i really think the way to go is not returning the credentials to the sandbox, but having all the authentication step moved to the host.

for the application/frontend comunication, we need:

the frontend/backend communication i still need to think about.

that's, of course, more complex that what is proposed here. but is necessary to grant security around the credential and enforce permissions over them.

Now some things about the technical side:

  • Frontend/Backend aren't split as this portal will be elementary exclusive and it therefore is probably not necessary

frontend and backends have proper defined responsabilities, frontends handles sandbox recoginition, permission checking and argument validation. backends provides integration with the desktop environment of choose by the user, it can be a UI service, it can be a daemon. a more agnostic implementation of the forntend interface will let other distros to make backends to they own credential services.

i see two backends needed here, currently:

the original proposed workflow also defines a AccountChooser dialog that can redirect to the swithcboard plug, that would need a third backend.

  • I've currently put it in a subfolder of src and construct it in the main function in XdgDesktopPortalPantheon.vala. This is probably the easiest way but because this is not a backend like the rest we could move it somewhere else if that makes more sense (own executable and then maybe even own repo? although that might be overkill)

frontend code should be in a separate binary, or at least in a separate connection. otherwise we expose the backends we implement here to the sandboxes too and that will be a security role.

organization-wise, i try to follow a folder per portal interface. so:

  • It somewhat deviates from the common portal flow with requests as it doesn't return a ObjectPath in the initial call but rather makes a handle_token mandatory. This is (at least how I understood it) the better way as documented here but can't be used everywhere because of some backwards compatability reasons we don't have to care about or am I wrong here?

we should always return a path or a error, this portal will ask the user authorization so a response can take more time than the default bus timeout.

i recommend reading the README in the xdg frontend repo, that gives you more info about the overall design of the portal.

Now some things about the general "flow" and way of implementation:

about the portal implementation, i had a working prototype at the time i wrote the suggestion, the implementation of both is simple if you skip the frontend code, that is the actual hard part.

  1. The evolution sources are still shared between apps so that they don't have to be configured by every app

even with the proposed interface that works, but would seen as not recommended, since direct access to the host EDS leaks account information.

also, like said in that proposal, removing the dependence in the host EDS is something more good than bad for the application, since it won't need to keep checking that both host and sandboxed EDS are in the same version, or that the host EDS supports some newer configuration from some newer EDS version.

  1. This is probably the main security hole but otherwise every app would have to configure their sources themselves which is currently handled by the onlineaccounts plug. This would also bring pretty big portal requests with it as we wouldn't want the user to have to configure their accounts in every app. Furthermore if for example settings that affect both mail sources and calendar sources are changed for example in Calendar these changes wouldn't apply to Mail (currently not that important as only separate caldav and imap sources are supported but once collection sources e.g. a Google/Microsoft/Apple etc. account are supported this will get problematic). This could of course be solved by sharing these settings via the portal but then we are getting close to reimplementing the Evolution data server (at least the DBus side of sharing soures) which isn't (or is it?) the goal.

i think you are seeing EDS as more than a implementation detail of the current version of the online accounts plug, as a Credential/Authentication Portal backend EDS is overkill and remove the possibility of usage for non-EDS backed applications. also i can't think about a shared information of collection sources that purpose-specific applications like Mail, Calendar and Tasks would need, and non-purpose-specific applications will prefer to handle all of it, like Evolution does.

leolost2605 commented 1 year ago

First of all I want to say English isn't my native language so if I missunderstood something please tell me :)

it shouldn't be EDS specific, it should be a OnlineAccounts portal not a EvolutionCredentails portal. we shoudn't use EDS-specific information here.

IMHO and if we are realistic we are probably the only ones that are going to use this portal and we are currently exclusively using EDS when handling online accounts. I think for the sake of simplicity and maintainability we could at least take a portal specialising on EDS for now into consideration. A general OnlineAccounts portal would of course be better but I think such a portal makes more sense and has more value when implemented upstream so I'm not really sure how much value it would add here.

the fact that the credential is sent to the sandbox is really bad, we can't guarantee the application saved or sent it to a remote server, that is a thing that i don't like in the old proposed API and in this too.

This of course is a security hole and once again it would be much better not having to send it. However I currently can't think of a way we could still use EDS and its libraries in e.g. Mail without sending the password. In general I don't think this is really possible if we don't want to be responsible for more than authentication in the portal. However maybe there is a way.. (I think I found the android implementation and they are using auth tokens that they generate to let applications access servers without having to handle passwords directly but I don't know what exactly these auth tokens are/how they work)

Also I would like to give some feedback/I have some questions about your proposal (Just to be sure we are talking about this proposal right?) This proposal states that the host EDS shouldn't be used but it doesn't have a way to send the application some more account details that are needed for it to cofigure its own EDS. Depending on how much we would like to be configurable centrally in the switchboard plug I think there are currently alone for IMAP/SMTP accounts about ten keys we would have to give the application (e.g. server, username, protocol, name, mail address, port, etc.). Or did I miss something there?

about the portal implementation, i had a working prototype at the time i wrote the suggestion, the implementation of both is simple if you skip the frontend code, that is the actual hard part.

Now given the things stated above about the proposal, I think this might get a lot more not really complicated but tedious really fast. AFAIK we would have to completely rewrite the switchboard plug and then copy what was formerly handled by the switchboard plug to the applications. And this still will send everything to the application so security wise there isn't a lot gained over this implementation. Although I have to say it would bring some advantages like as you said not relying on a host EDS and having a user conformation before the app can access all of the information whereas here there is only a user conformation for passwords.

i think you are seeing EDS as more than a implementation detail of the current version of the online accounts plug, as a Credential/Authentication Portal backend EDS is overkill and remove the possibility of usage for non-EDS backed applications

Correct me if I'm wrong here but I wouldn't say EDS is "just" an implementation detail of the current switchboard plug. I would rather say EDS is the base on which all of elementaries office applications and the switchboard plug is built. Of course this base could be split out into the single applications and their flatpaks but like I said that might be a bit of work.

also i can't think about a shared information of collection sources that purpose-specific applications like Mail, Calendar and Tasks would need, and non-purpose-specific applications will prefer to handle all of it, like Evolution does.

I thought about that again and I think you're right there probably isn't apart from the aforementioned server stuff. The only thing I could think of is if a user would have two e.g. mail apps installed but that probably isn't very likely :)

My personal thoughts about this:

I went into this with the primary goal of making it possible to ship Mail, Calendar and Tasks as Flatpaks. Added security would be a bonus but wasn't necessary for me.

I still think that a solution like this would for now bring quite a big improvement for a relatively small amount of work. IMO this could be a good temporary solution that might be worth considering and implementing as it doesn't bring a big maintenance effort and nothing big is going to be built on top of it so that it could just be dropped at any time if we've got something better. Of course like I said this is by no means a perfect solution because first and foremost it doesn't bring added security. However it also doesn't reduce security and brings the advantage of being able to package EDS dependent apps as flatpaks.

What I want to say is:

What's the opinion on having such a implementation (technical details can still be smoothed out) as a temporary solution to the problem at hand and then maybe looking upstream for a secure and more general OnlineAccounts portal? Or is this really a no go?

Marukesu commented 1 year ago

First of all I want to say English isn't my native language so if I missunderstood something please tell me :)

It's fine, feel free to point out any doubts, english is not my first language either. 🙂

it shouldn't be EDS specific, it should be a OnlineAccounts portal not a EvolutionCredentails portal. we shoudn't use EDS-specific information here.

IMHO and if we are realistic we are probably the only ones that are going to use this portal and we are currently exclusively using EDS when handling online accounts. I think for the sake of simplicity and maintainability we could at least take a portal specialising on EDS for now into consideration. A general OnlineAccounts portal would of course be better but I think such a portal makes more sense and has more value when implemented upstream so I'm not really sure how much value it would add here.

If we are only dealing with authentication, a generic one will be as simple and maintainable as a EDS focused one, while supporting being used by more than only EDS. If other functionality than authentication is needed to be provided by a portal, other portal interface would need to be tough.

Also I would like to give some feedback/I have some questions about your proposal (Just to be sure we are talking about this proposal right?) This proposal states that the host EDS shouldn't be used but it doesn't have a way to send the application some more account details that are needed for it to cofigure its own EDS. Depending on how much we would like to be configurable centrally in the switchboard plug I think there are currently alone for IMAP/SMTP accounts about ten keys we would have to give the application (e.g. server, username, protocol, name, mail address, port, etc.). Or did I miss something there?

Non sensible information would be returned in response to the LookupAccount method, if the call was successful (the user has given permission to share that information). The sensible information would be returned as part of the GetCredential call.

about the portal implementation, i had a working prototype at the time i wrote the suggestion, the implementation of both is simple if you skip the frontend code, that is the actual hard part.

Now given the things stated above about the proposal, I think this might get a lot more not really complicated but tedious really fast. AFAIK we would have to completely rewrite the switchboard plug and then copy what was formerly handled by the switchboard plug to the applications. And this still will send everything to the application so security wise there isn't a lot gained over this implementation. Although I have to say it would bring some advantages like as you said not relying on a host EDS and having a user conformation before the app can access all of the information whereas here there is only a user conformation for passwords.

I was speaking more about the portal frontend, (arguments validation, permission checking and application identification, what xdg-desktop-portal does for the backends we implement here).

To make a proper portal frontend, it needs to:

without any of this, we cannot really call it a portal.

About the rest:

I think you are seeing EDS as more than a implementation detail of the current version of the online accounts plug, as a Credential/Authentication Portal backend EDS is overkill and remove the possibility of usage for non-EDS backed applications

Correct me if I'm wrong here but I wouldn't say EDS is "just" an implementation detail of the current switchboard plug. I would rather say EDS is the base on which all of elementaries office applications and the switchboard plug is built. Of course this base could be split out into the single applications and their flatpaks but like I said that might be a bit of work.

Currently the plug/application workflow goes this way:

  plug -> EDS <-> app

However, a good chunk of the code there could (and, IMO, should) be part of a EDS module:

  plug -> backend <-> backend EDS module <-> EDS <-> app

Since doing that won't require application changes, i see it as a implementation detail.

My personal thoughts about this:

I went into this with the primary goal of making it possible to ship Mail, Calendar and Tasks as Flatpaks. Added security would be a bonus but wasn't necessary for me.

I still think that a solution like this would for now bring quite a big improvement for a relatively small amount of work. IMO this could be a good temporary solution that might be worth considering and implementing as it doesn't bring a big maintenance effort and nothing big is going to be built on top of it so that it could just be dropped at any time if we've got something better. Of course like I said this is by no means a perfect solution because first and foremost it doesn't bring added security. However it also doesn't reduce security and brings the advantage of being able to package EDS dependent apps as flatpaks.

What I want to say is:

What's the opinion on having such a implementation (technical details can still be smoothed out) as a temporary solution to the problem at hand and then maybe looking upstream for a secure and more general OnlineAccounts portal? Or is this really a no go?

I, actually, wrote a daemon before, with the same goal, see the dbus-daemon branch in the plug repository.

However, i should remark the commentary about why the need for this being a portal:

because we need a way to tell if the credentials requesting app is authorized. Otherwise a malicious app could request any credentials and/or spoof another app's id to do so. So a simple DBus handing over whatever credentials are requested is a serious security hole. That's why it's a portal, you always know the origin of the request that way.

So, just to be clear, in complete stage, this and the old proposal are really similar (this will only lack the LookupAccount method), and the level of effort to write properly this will be the same of any other complete implementation. So, i not sure that it makes sense in doing a "temporary solution" portal. It's better to do this right from the start and don't need to, later, add a proper portal and write code that can work with both portals, because removing a portal would be a API break and we shouldn't break portal API that easily.

In the actual form, this is a non-go as a portal, because it isn't one, it's the same as the daemon.

leolost2605 commented 1 year ago

Ok I'll close it for now