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

feat: add bluesky as a provider #281

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

feat: add bluesky as a provider #281

wants to merge 12 commits into from

Conversation

noook
Copy link

@noook noook commented Nov 13, 2024

Resolves: #267

This PR adds Bluesky as a provider.

This provider requires the user to install extra dependencies to properly handle authorization, because of the way Bluesky works.

In order to begin the authorization process, we first need to know the user handle. This is required because we need to know against which instance of Bluesky we need to verify the user. Doing all the verifications manually require a lot of steps and adds complexity, so we use the atproto API instead.

@atinux
Copy link
Owner

atinux commented Nov 14, 2024

Damn you are on 🔥

Regarding the scopes, I think we can anyway remove the duplicate because such a change on defu can have strong side effects, might be easier to do it ourself after the merge.

Something like:

config.scopes = [...new Set(config.scopes)]

@noook
Copy link
Author

noook commented Nov 14, 2024

I'm not sure I'm satisfied regarding the current implementation. Basically Bluesky is just a provider using atproto underneath, and we could have sooner or later another atproto provider.

I think I'm facing the same issues that come with the complexity of providing a generic OIDC provider, but with the additional build time constraints on top of that (exposing the discovery document)

So the configuration would be split into two parts:

  • atproto -> next to oauth and webauthn configuration. Probably a boolean, so we can check that the peer dependencies are installed.
  • oauth.${atprotoProvider} -> Should implement an interface such that matches the "Client ID Metadata Document" section on this document that seems to be a common base for future atproto providers

I guess the new challenge here is to provide the dynamic metadata handler

@atinux
Copy link
Owner

atinux commented Nov 14, 2024

So the configuration would be split into two parts:

  • atproto -> next to oauth and webauthn configuration. Probably a boolean, so we can check that the peer dependencies are installed.
  • oauth.${atprotoProvider} -> Should implement an interface such that matches the "Client ID Metadata Document" section on this document that seems to be a common base for future atproto providers

I love this approach!

@noook
Copy link
Author

noook commented Nov 14, 2024

With the latest commit, Bluesky provider works out of the box with 0 config 😁

Now, I need to extract the whole logic somewhere else + expose the metadata.

@atinux
Copy link
Owner

atinux commented Nov 15, 2024

Please ping me when good to merge, amazing work you are doing @noook ❤️

@noook
Copy link
Author

noook commented Nov 18, 2024

@atinux It should be good. I'm having issue when I need to type useRuntimeConfig(event).oauth[provider]

Element implicitly has an 'any' type because expression of type 'OAuthProvider' can't be used to index type '{ github: { clientId: string; clientSecret: string; redirectURL: string; }; gitlab: { clientId: string; clientSecret: string; redirectURL: string; }; spotify: { clientId: string; clientSecret: string; redirectURL: string; }; ... 23 more ...; authentik: { ...; }; }'.

I understand the error, it makes sense, but I'm not sure how I can do this properly

@atinux
Copy link
Owner

atinux commented Nov 28, 2024

I just noticed that you use useStorage which can be tricky as it needs to be configured in serverless environment in order to properly work. One solution could be to use cookies to store the state, see https://github.com/pilcrowonpaper/atproto-oauth-example/blob/2ebfd7836b5ce9921df31e66fb332ee8ae8d5823/src/pages/login/callback.ts#L15-L17

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.

Add ATProto/Bluesky as provider
2 participants