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

Draft OAuth Spec #326

Merged
merged 18 commits into from
Sep 17, 2024
Merged

Draft OAuth Spec #326

merged 18 commits into from
Sep 17, 2024

Conversation

bnewbold
Copy link
Contributor

@bnewbold bnewbold commented Aug 23, 2024

Making our OAuth spec draft public, for visibility and feedback.

A couple things we are still cooking on:

  • resolve inline TODOs
  • updating examples and SDKs to match this version of the spec
  • copy editing
  • getting production bluesky PDS/entryway compliant with this version of the spec
  • later: iterating on session and token lifetimes, and client metadata trust

As with all atproto specs, this was a big team effort (with matthieu leading the charge). And we are particularly thankful for IETF folks and atproto dev community for early feedback on this work.

content/specs/auth.md Outdated Show resolved Hide resolved
content/specs/auth.md Outdated Show resolved Hide resolved
content/specs/auth.md Outdated Show resolved Hide resolved
content/specs/auth.md Outdated Show resolved Hide resolved
content/specs/auth.md Outdated Show resolved Hide resolved
content/specs/auth.md Outdated Show resolved Hide resolved
content/specs/auth.md Outdated Show resolved Hide resolved
content/specs/auth.md Outdated Show resolved Hide resolved
content/specs/auth.md Outdated Show resolved Hide resolved
content/specs/auth.md Outdated Show resolved Hide resolved
Thanks so much hailey!

Co-authored-by: Hailey <hailey@blueskyweb.xyz>
@blowdart
Copy link

blowdart commented Aug 24, 2024

On return URLs and desktop applications

Limiting localhost to "development scenarios" rules out console applications or an application plugin architecture where you cannot control the host and cannot register your own protocol url to receive callbacks. (And protocol URLs are ridiculously hard to make cross platform)

The localhost approach is commonly used for plugins, for example, authentication within Visual Studio Code where a plugin runs under the context of another app. If you, for example, attempt to auth with GitHub or Azure AD to sync your plugins VS will pop up a browser, with a return URL of localhost and a random port. The plugin itself has opened a simple listener on localhost and the port and waits to get the token from the auth server.

This is also how I (and google) would approach auth in console applications, and to get extremely specific it's how I'd want doing it inside a Windows Widget.

This, of course, presents an interesting drawback, if someone writes an abusive command line utilty, or plug-in, you can't ban the client from a server because it's the hardcoded localhost client without banning every other native app using this approach. Expecting each command line utility to have its own domain name, and client json is a heavy burden (for example, as I continue to mess around writing a library I have multiple sample apps, all command line apps, so one domain per sample is both silly and expensive).

TLDR - conisder extending native client support to use http://localhost/_randomPort_ as a callback URL, and don't limit that to just development environments.

(I realise us desktop app people are old now, but we still exist 😈)

Copy link
Contributor

@surfdude29 surfdude29 left a comment

Choose a reason for hiding this comment

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

I learnt a lot from reading this, I hope my suggestions are helpful :)

content/specs/auth.md Outdated Show resolved Hide resolved
content/specs/auth.md Outdated Show resolved Hide resolved
content/specs/auth.md Outdated Show resolved Hide resolved
content/specs/auth.md Outdated Show resolved Hide resolved
content/specs/auth.md Outdated Show resolved Hide resolved
content/specs/auth.md Outdated Show resolved Hide resolved
content/specs/auth.md Outdated Show resolved Hide resolved
content/specs/auth.md Outdated Show resolved Hide resolved
content/specs/auth.md Outdated Show resolved Hide resolved
content/specs/auth.md Outdated Show resolved Hide resolved
@yamarten
Copy link

@blowdart

Even if it is outside the development scenario, it seems that it is not prohibited to use localhost for redirect_uris. Which description is specifically the problem?

According to my understanding, only client_id mentions localhost as an exception of restriction that it must be a uniquely and fetchable URL. On the other hand, redirect_uris does not have such restrictions.

Either way, I or you are misunderstanding, so it would be worthwhile to clarify the specifications.

@blowdart
Copy link

@yamarten It's probably my reading, but it's implied by these (my emphasis added)

There is a special exception for the localhost development workflow to use http://

To make development workflows easier, a special exception is made for clients with client_id having origin http://localhost (with no port number specified). Authorization Servers are encouraged to support this exception - including in production environments - but it is optional.

Also, that supporting it is optional for servers.

@yamarten
Copy link

@blowdart Well, to me, neither of them only describes client_uri and cannot be read as if the callback URI is restricted.

@blowdart
Copy link

blowdart commented Aug 24, 2024

I'll admit I'm incredibly fussy when it comes to authentication and authorization docs. Anywhere they don't have clarity is an area things could be mistaken or misused :)

It gets even more complicated if you want to use a native client with a client ID and localhost.

As I read it, a native client must have the client metadata file on the internet. Which is fine, except the return URL is going to be static, and for localhost based return URIs the port cannot ever be fixed.

@yamarten
Copy link

Indeed, I understand that if the range of the port cannot be determined in advance, there will be problems.

Since the port number is finite, it seems that it can be solved by listing it all, but it is not a practical method.

@blowdart
Copy link

A potential solution might be special casing native apps.

if application_type == "native" AND
   return_url STARTS WITH "http://localhost" or "http://127.0.0.1" or "http://[::1]" AND
   the port is * 
only match on protocol, host and path, and allow **any** specified port, 

For example, a return_url of "http://localhost:*/oauth/" would allow http://localhost:12345/oauth, http://localhost:12346/oauth etc. But a specific port, "http://localhost:8080/oauth/" would enforce an exact match on a return_url of http://localhost:8080/oauth/

This would allow specific native clients, like my Windows widget, to support the metadata file, have a client ID, name and get the loopback support.

Of course any malicous actor is just going to use the same client ID and name in their own native app, but that'd be the same for any native app, regardless of return protocol.

An aside @bnewbold it may be worth specifying whether values in the meta data file are case sensitive or not.

Thanks so much @surferdude29!

Co-authored-by: surfdude29 <149612116+surfdude29@users.noreply.github.com>
Co-authored-by: Hailey <hailey@blueskyweb.xyz>
@bnewbold
Copy link
Contributor Author

@surfdude29 thanks so much for the copy-editing!

@bnewbold
Copy link
Contributor Author

@blowdart I think we'd like to end up with robust support for all sorts of native apps, but we might have a great solution for your use-case in this phase of OAuth development. It might be good to move this discussion to a separate github discussion thread so this conversation can continue after this draft gets merged... sorry for the run-around, I think this conversation has already bounced between a few venues which is probably frustrating.

My current thoughts and recommendations on native apps:

  • pragmatically, just stick with the legacy auth system (app passwords) for the next few months. It just works, and the transition/deprecation period isn't going to be super-fast. I do see how this could be a bad option for desktop software in particular, as it could be harder to push updates. From a pragmatic standpoint, I think it will be hard to guarantee that any version of this OAuth proposal is rock-solid on many-month/many-year timelines until we've had more chance to let it bake and for the relevant draft IETF RFCs to get standardized.
  • a solid option would be to implement a "token-mediating" backend service. The localhost client path will probably always be a "public client", though in theory unique local clients could generate and expose unique client keys and a unique client_id. a token-mediating backend can be a "confidential client". it would be a fairly minimal web service that client software connects to for token refreshes, then the local native client makes direct connections to the PDS using access tokens. This flow is described in the earlier atproto OAuth proposal.
  • for "headless" tools specifically, like command-line tools and bots, we are leaning towards a separate "API Token" auth system, which would probably be just static bearer tokens with configurable lifetimes and scope (eg, up to years). I'm not sure if that works for your use-case? The UX to generate these has more friction than OAuth, and will probably have some "make sure you know what you are doing!" disclaimers making it not great for end users.
  • I'm honestly not super familiar with the current state of desktop OS app-specific URI schemas and would need to do some research in to that to advise better

If none of those can be made to work, we could consider making the localhost exception more formalized for native apps. But i'd want to take this back and discuss with the whole team and understand whether that really solves the issue in a robust long-term way for a large number of native app use-cases (aka, across multiple platforms).

content/specs/auth.md Outdated Show resolved Hide resolved
@bnewbold bnewbold marked this pull request as ready for review August 27, 2024 19:57
content/specs/auth.md Outdated Show resolved Hide resolved
content/specs/auth.md Outdated Show resolved Hide resolved
- **web clients** include web services and browser apps. Redirect URLs are regular web URLs which open in a browser.
- **native clients** include some mobile and desktop native clients. Redirect URLs may use platform-specific app callback schemes to open in the app itself.

Authorization Servers may maintain a set of "trusted" clients, identified by `client_id`. Because any client could use unverified client metadata to impersonate a better-known app or brand, Authorization Servers should not display such metadata to end users in the Authorization Interface by default. Trusted clients can have additional metadata shown, such as a readable name (`client_name`), project URI (`client_uri`, which may have a different domain/origin than `client_id`) and logo (`logo_uri`). See the "Security Considerations" section for more details.
Copy link
Contributor

Choose a reason for hiding this comment

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

client_uri, which may have a different domain/origin than client_id

I think we should not allow client_uri, tos_uri, or policy_uri to be on distinct origins. Similarly, we should enforce that these endpoints serve text/html content with a 200 response and open CORS, in order to potentially loading their content in the auth interface.

Choose a reason for hiding this comment

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

I'm going to disagree on CORS here. You don't want the auth endpoint loading policies or TOSes, as you shouldn't be trusting them to render it. You want those links to open in new windows, which show the source URI, so an observant user will see where they come from.

(Not that most people will of course, but some of us might)

Copy link
Contributor

Choose a reason for hiding this comment

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

you are totally right, I meant CSP, not CORS, in order to load in an iframe. loading arbitrary html in an auth interface is a very bad idea indeed.

Copy link
Contributor

Choose a reason for hiding this comment

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

We probably don't need this (^) for now though.

I would still want to restrict all the uris of the document to the same origin as the client_id (so that we can relax this later).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tracking here for follow-up: #339

content/specs/auth.md Outdated Show resolved Hide resolved
content/specs/auth.md Outdated Show resolved Hide resolved
content/specs/auth.md Outdated Show resolved Hide resolved
content/specs/auth.md Outdated Show resolved Hide resolved
content/specs/auth.md Outdated Show resolved Hide resolved
content/specs/auth.md Outdated Show resolved Hide resolved
content/specs/auth.md Outdated Show resolved Hide resolved
@blowdart
Copy link

blowdart commented Aug 28, 2024

@bnewbold All of which is fair, if one can assume username / password / app password isn't going away, and it will keep up with scopes. But the reason I want to head down the oauth route is I've spent way too long in security and auth to like usernames anymore, plus someone is going to want a C# sample at some point, so why don't I just write it now :) Add to that you're already describing it as legacy, who wants to code against legacy that is going to vanish at some point?

The mediating service is fine, but honestly the idea of having a man in the middle worries me just a little. It's just too tempting a target to get popped.

content/specs/auth.md Outdated Show resolved Hide resolved

#### Request Fields

A summary of fields relevant to authorization requests with the atproto OAuth profile:

Choose a reason for hiding this comment

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

Specific case sensitivity for values here. Presumably they're all case sensitive, but best to call it out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we refer to / defer to an upstream OAuth RFC on this? there are probably many such details we aren't re-specifying in this document. would like to clarify if we are adding an atproto-specific requirement or not.

content/specs/auth.md Outdated Show resolved Hide resolved
@matthieusieben
Copy link
Contributor

@blowdart loopback redirect uris are allowed for all native clients as described in RFC8252 - OAuth 2.0 for Native Apps. Note that there are some caveats (e.g. http://localhost is not allowed, only http://127.0.0.1 and http://[::1] are).

content/specs/auth.md Outdated Show resolved Hide resolved

Authorization Servers may maintain a set of "trusted" clients, identified by `client_id`. Because any client could use unverified client metadata to impersonate a better-known app or brand, Authorization Servers should not display such metadata to end users in the Authorization Interface by default. Trusted clients can have additional metadata shown, such as a readable name (`client_name`), project URI (`client_uri`, which may have a different domain/origin than `client_id`) and logo (`logo_uri`). See the "Security Considerations" section for more details.

Clients which are only using atproto OAuth for account authentication (without authorization to access PDS resources) should request minimal scopes (see “Scopes” section), but still need to implement most of the authorization flow. In particular, it is critical that they check the `sub` field in a token response to verify the account identity (this is an atproto-specific detail).
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure how to phrase this, and i know that this is also specified later in this document, but this is an important security measure so I think that for devs skimming through the doc while implementing, it is important to repeat this.

Suggested change
Clients which are only using atproto OAuth for account authentication (without authorization to access PDS resources) should request minimal scopes (see “Scopes” section), but still need to implement most of the authorization flow. In particular, it is critical that they check the `sub` field in a token response to verify the account identity (this is an atproto-specific detail).
Clients which are only using atproto OAuth for account authentication (without authorization to access PDS resources) should request minimal scopes (see “Scopes” section), but still need to implement most of the authorization flow. In particular, it is critical that they check the `sub` field in a token response to verify the account identity (this is an atproto-specific detail). Clients must also ensure that the `iss` callback query parameter matches the Authorization Server's (derived from the `sub`) `issuer` metadata.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tracking: #339


The Authorization Server uses the `request_uri` to look up the earlier Authorization Request parameters, authenticates the user (which might include sign-in or account selection), and prompts the user with the Authorization Interface. The user might refine any granular requested scopes, then approves or rejects the request. The Authorization Server redirects the user back to the `redirect_uri`, which might be a web callback URL (for web clients), or a native app URI.

The client uses URL query parameters (`state` and `iss`) to look up and verify session information. Using the `code` query parameter, the client then makes an initial token request to the Authorization Server’s token endpoint. The client completes the PKCE flow by including the earlier value in the `code_verifier` field. Confidential clients need to include a client assertion JWT in the token request; see the "Confidential Client" section. The Authorization Server validates the request and returns a set of tokens, as well as a `sub` field indicating the account identifier (DID) for this session, and the `scope` that is covered by the issued access token.
Copy link
Contributor

Choose a reason for hiding this comment

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

We should explain that replayed the code must be rejected by the AS, and must cause the token previously issued to that code to be revoked.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The later part does make HTTP glitches/retries brittle during the initial token request, which is too bad. And is it really strictly necessary (mandatory, the sort of thing we will eventually write a compliance test for), or just recommended / best practice?

Copy link
Contributor

Choose a reason for hiding this comment

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

Only MAYs and SHOULDs in various RFCs (see bellow).

The point of this protection is to ensure that the user realizes it is under attack when it's session gets invalidated. Sadly, there is no way to distinguish an attack from a network error during the token exchange...

When a client fails to exchange a code for a token, it typically will implement recovery measures. In case of network error, the recovery method is to initiate a new auth flow, which may be silently accepted by the AS (resulting in no UX degradation). Worst case scenario, the user will have to "accept" again.

https://datatracker.ietf.org/doc/html/draft-ietf-oauth-security-topics#section-4.3.1

When a browser navigates to client.example/redirection_endpoint?code=abcd as a result of a redirect from a provider's authorization endpoint, the URL including the authorization code may end up in the browser's history. An attacker with access to the device could obtain the code and try to replay it.

https://www.rfc-editor.org/rfc/rfc6819.html#section-4.4.1.1

If an authorization server observes multiple attempts to redeem an authorization "code", the authorization server may want to revoke all tokens granted based on the authorization "code" (see Section 5.2.1.1).

The authorization server should enforce a one-time usage restriction (see Section 5.1.5.4).

https://www.rfc-editor.org/rfc/rfc6819.html#section-5.1.5.4

For example, if an authorization server observes more than one attempt to redeem an authorization "code", the authorization server may want to revoke all access tokens granted based on the authorization "code" as well as reject the current request.

https://www.rfc-editor.org/rfc/rfc6819.html#section-5.2.3.6

An authorization server may revoke a client's secret in order to prevent abuse of a revealed secret.

Note: This measure will immediately invalidate any authorization "code" or refresh token issued to the respective client.

Copy link
Contributor

@matthieusieben matthieusieben Sep 9, 2024

Choose a reason for hiding this comment

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

The real issue here is with refresh_token rotation, as replaying those would result in the whole session to get invalidated. This could happen "randomly" from the user's POV.

One mitigation here, for clients, is to proactively refresh (without waiting the access token to be expired) at a moment when having to login back in is less inconvenient for the user (for example when reactivating the app).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated text to add this as a "should" in the PKCE session (this is about codes).

tracking discussion of whether this is a "must": #339


At this point it is critical (mandatory) for all clients to verify that the account identified by the `sub` field is consistent with the Authorization Server "issuer" (present in the `iss` query string), either by validating against the originally-supplied account DID, or by resolving the accounts DID to confirm the PDS is consistent with the Authorization Server. See “Identity Authentication” section. The Authorization always returns the scopes approved for the session in the `scopes` field (even if they are the same as the request, as an atproto OAuth profile requirement), which may reflect partial authorization by the user. Clients must reject the session if the response does not include `atproto` in the returned scopes.

Authentication-only clients can end the flow here.
Copy link
Contributor

@matthieusieben matthieusieben Sep 6, 2024

Choose a reason for hiding this comment

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

We should encourage clients to revoke credentials they no longer use. The reason being to prevent an attacker from using those credentials without the user noticing.

This requires that we spec that AS must implement a revocation_endpoint (which is an OIDC feature)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

bigger change, tracking here: #339

Co-authored-by: Matthieu Sieben <matthieusieben@users.noreply.github.com>
Copy link

cloudflare-workers-and-pages bot commented Sep 8, 2024

Deploying atproto-website with  Cloudflare Pages  Cloudflare Pages

Latest commit: 74e4f8e
Status:🚫  Build failed.

View logs

@bnewbold
Copy link
Contributor Author

bnewbold commented Sep 8, 2024

I'm generally inclined to keep the summary section at the top as short as possible, as a true summary, and not digress too much on defining terms/specs or enumerating too many security aspects. The main text should be the actual authority. I don't want to distract devs when they are getting the big picture. But maybe a couple small tweaks are ok, and turning terms in to hyperlinks or mouse-over tool tips shouldn't impact readability.

I only skimmed the other stuff so far, it seems mostly fine, maybe with some wording tweaks. We might want to give examples of good/bad client_id values (as we do in, eg, the handle spec document), though overall I think we should bias towards getting this doc merged ASAP and do a follow-up PR with any tweaks, instead of getting the initial version perfect.

Co-authored-by: Matthieu Sieben <matthieusieben@users.noreply.github.com>
@pfrazee
Copy link
Collaborator

pfrazee commented Sep 10, 2024

BTW when this is ready to merge ping me and I'll update it for the new website

@bnewbold
Copy link
Contributor Author

As an update for external folks: please don't comment here on this PR any more, open a new issue on this repo instead. We are moving to merge this PR soon and don't want to lose additional conversation threads or comments.

@bnewbold bnewbold mentioned this pull request Sep 16, 2024
12 tasks
@arcalinea arcalinea temporarily deployed to bnewbold/spec-oauth - atproto-website PR #326 September 17, 2024 00:42 — with Render Destroyed
@bnewbold bnewbold merged commit 40e95d7 into main Sep 17, 2024
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.

8 participants