indieweb / indieauth

IndieAuth.net website code and IndieAuth Specification
52 stars 7 forks source link

Looking to implement IndieAuth and have some questions #135

Closed anderspitman closed 1 week ago

anderspitman commented 1 month ago

I'm looking to implement IndieAuth in obligator (which powers LastLogin), with an eye towards contributing a patch to Rauthy as well. This would give people a couple more options for hosting their own IndieAuth server.

I have a couple questions:

  1. It looks like rel=authorization_endpoint and rel=token_endpoint are deprecated. I'm concerned this forces an additional round trip for fetching the AS metadata, which can cause a significant performance hit, especially for users on native clients with slower internet or low-powered devices. Is there a way to avoid this?

  2. The spec currently recommends that clients provide client information to the AS, including client name and logo. Does trusting this information not open users up to significant security issues? For example, if gooogle.com hosts client data with name "Google" and the actual Google logo, users will be pretty likely to trust the login. If we're supporting logins without requiring client registration (which is a killer feature of IndieAuth IMO), isn't the only information we can trust enough to present to the user the client domain itself? Not fetching the client metadata also saves another round trip. It looks like this is already possible since most of the wording in this sections is SHOULDs, and that the AS can choose to simply verify the redirect_uri is on the same domain as the client_id and forego fetching the client data, correct?

aaronpk commented 1 month ago

It looks like rel=authorization_endpoint and rel=token_endpoint are deprecated.

Yes, we have switched to the AS metadata discovery which makes it easier to delegate to an external provider. While it does require an extra request from the client, I'm not sure I would call it a "significant performance hit" since it's just one request to a static relatively small JSON document. Plus it's really only a concern for mobile clients, since non-mobile clients should have a web server backend and then it's a server-to-server request that doesn't even touch the client.

Does trusting this information not open users up to significant security issues?

Yes, this is a potential risk, and is mentioned in the security considerations, which is why there is a recommendation to display the client_id to the user as well. Also note this is not unique to IndieAuth, since you can go register an OAuth client at GitHub or Google and create an app named something similarly suspicious too.

isn't the only information we can trust enough to present to the user the client domain itself

Yes, which is why that recommendation is to display the client_id

Also worth noting if you haven't found it already that there is a discussion of updating the client metadata discovery to a client metadata JSON document as well: #133. I've already implemented that in webmention.io and am using the new client_id URL with it as well: https://webmention.io/id

anderspitman commented 1 month ago

Thanks @aaronpk!

I'm not sure I would call it a "significant performance hit" since it's just one request to a static relatively small JSON document.

It's more the round trip latency than how the request itself is handled. There's never a guarantee of network performance between any two endpoints so I try to keep requests to a minimum. That said, I understand the decision. I just want to voice my opinion that leaving the rel links in provides a useful optimization and would love for them to be kept, even if only in the Link header form since that doesn't add the dependency of HTML parsing.

Yes, this is a potential risk, and is mentioned in the security considerations, which is why there is a recommendation to display the client_id to the user as well.

Displaying the client_id to the user is good (I would argue necessary), but generally if a logo is presented I think people are more likely to make their trust decision based off that than the domain.

Also note this is not unique to IndieAuth, since you can go register an OAuth client at GitHub or Google and create an app named something similarly suspicious too.

This is true, and I think it's bad security practice. There have been real world exploits. Note that Google's review process has gotten pretty rigorous. It was quite a pain for me to get LastLogin approved, and it only requests the most basic login scope values. Also they likely have advanced systems for flagging logos and domains that are trying to pretend to be other orgs. This is all beneficial, but it would be better to only show the domain for any org that hasn't been reviewed by a human IMO.

Also worth noting if you haven't found it already that there is a discussion of updating the client metadata discovery to a client metadata JSON document as well

I did see that. I think it's a great direction if you insist on supported client metadata. Since I only plan on displaying the client domain (and avoiding another round trip), my implementation hopefully won't be impacted. Though I suspect @sebadob would insist on any PRs to Rauthy supporting it.

anderspitman commented 1 week ago

LastLogin now has a reasonably functional IndieAuth implementation in a dev branch.