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

OAuth: Add authorization scopes & remove OpenID compatibility #2734

Merged
merged 53 commits into from
Aug 27, 2024

Conversation

matthieusieben
Copy link
Contributor

@matthieusieben matthieusieben commented Aug 22, 2024

This PR adds support for scopes during the authorization flow. Three scopes are introduced

  • atproto: acts as a negotiation scope between the client and the server so that both know that this is an ATProto OAuth flow
  • transition:generic: This temporary scope allows to give access tokens issued through OAuth the same permissions as current "app passwords".
  • transition:chat.bsky: Grants access to chat functionalities, as was the case for "privileged app passwords"

This change also removes the compatibility with OpenID.

The reason is that although the implementation was technically OpenID compatible, ATProto identifiers are distributed identifiers. When a client relies on OpenID to authenticate users, it will use the auth provider (issuer) in combination with the identifier to uniquely identify the user. Since ATProto identifiers are meant to be able to move from one provider to the other, OpenID compatibility could break authentication after a user was migrated to a different provider. Future authentication attempts would be considered as distinct identities from the one previsouly used by the client.

The way OpenID compliant clients would adapt to this particularity would typically be to remove the issuer + identifier combination and use the identifier alone. While this is indeed the right way to handle ATProto identifiers, it requires more work to avoid impersonation. In particular, when obtaining a user identifier, the client must verify that the issuer of the identity token is indeed the server responsible for that user. This mechanism being not enforced by the OpenID standard, OpenID compatibility could lead to security issues. For this reason, we decided to remove OpenID compatibility from the OAuth provider.

Note that a trusted central authority could still offer OpenID compatibility by relying on ATProto's regular OAuth flow under the hood. This capability is out of the scope of this library.

@matthieusieben matthieusieben changed the base branch from main to msieben/oauth-atproto-scope August 22, 2024 11:34
@matthieusieben matthieusieben marked this pull request as draft August 23, 2024 07:59
@matthieusieben matthieusieben force-pushed the msieben/oauth-atproto-scope branch from c3abe00 to 30e319a Compare August 23, 2024 10:42
@matthieusieben matthieusieben force-pushed the msieben/oauth-scopes branch 2 times, most recently from 71405fc to 9aae108 Compare August 23, 2024 11:25
@matthieusieben matthieusieben force-pushed the msieben/oauth-scopes branch 2 times, most recently from 5050389 to f8a820a Compare August 23, 2024 13:46
@matthieusieben matthieusieben force-pushed the msieben/oauth-scopes branch 2 times, most recently from 4c85f4a to 1f03d97 Compare August 23, 2024 14:29
Copy link
Collaborator

@devinivy devinivy left a comment

Choose a reason for hiding this comment

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

A few questions worth working through, but this looks great— pretty much read to roll.

@matthieusieben matthieusieben changed the base branch from msieben/oauth-atproto-scope to main August 27, 2024 09:53
@itaru2622
Copy link
Contributor

itaru2622 commented Aug 27, 2024

@matthieusieben @devinivy
please change the title of this PR, for easy tracking the lost of openID compatibility.

I understand that this scoping feature needs to break openID compatibility through discussions,
but current title is hard to track when openID compatibility is lost, even the lost is big issue.

@matthieusieben matthieusieben changed the title Add verification of scopes on PDS OAuth: Add authorization scopes & remove OpenID compatibility Aug 27, 2024
Comment on lines 1 to 5
---
"@atproto/pds": patch
---

Disable ability to list app passwords when using an app password
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe this change was reverted.

@devinivy devinivy merged commit dee817b into main Aug 27, 2024
10 checks passed
@devinivy devinivy deleted the msieben/oauth-scopes branch August 27, 2024 17:43
@github-actions github-actions bot mentioned this pull request Aug 26, 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.

3 participants