Skip to content

Opening this for comparison and review, NOT READY FOR MERGE #4779

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

Open
wants to merge 31 commits into
base: develop
Choose a base branch
from

Conversation

immber
Copy link
Contributor

@immber immber commented Apr 30, 2025

Hi Coral! I'm opening a sample of a PR on my branch, based on previous conversations on this issue...

#4745

What does this PR do?

  • adds a "BskyConfig" section to Admin/Config/Auth
  • adds BskyAuthIntegration type of settings to tenant model
  • adds a BskyProfile to user model
  • adds SignInWithBskyContainer form to stream auth/views/ && adminviews/SignIn
  • adss SignUpWithBskyContainer form to stream auth/views/
  • adds BskyButton framework components with butterfly svg
  • adds authentication routes and handlers for signin /api/auth/bsky, /api/auth/bsky/callback, /api/auth/bsky/metadata.json
  • includes client side handle validation on the forms, with form error handling
  • instantiates a BskyAuthIntegration parent class that wraps Atproto's oauth-client-node creating the atproto/bluesky oauth client. (This client is currently instantiated and cached on the tenant, but might need to move this to a single instance per auth request instead)

What is still missing to make this feature complete?

  • Hitting this bug in the oauth-client-node... Automatic token refresh not working. TokenRefreshError: The session was deleted by another process bluesky-social/atproto#3637. When you authorize with a valid atproto handle, we see the error The session was deleted by another process. (I think this could be another fetch thing, bc of where it fails, still looking into this)
  • Confidential Client Authentication: ie hooking up the jwks.json endpoint and connecting the client secret value to the key generation. Currently some tokens are exposed in the browser.
  • Handling session termination and refresh: sessions are created, but it hits the bug at the retrieval of profiles which means a Bsky session gets created, but a Coral one can not be. Once this bug is resolved or a workaround found, this still needs a revoke listener on the client and a token refresh helper function to handle session events automatically.
  • Uncomment the local host version and switch to tenant url where client-metadata is generated. (This is hardcoded and hacky for the moment as i was just trying to get it working locally first.)

These changes will impact:

  • [x ] commenters
  • [x ] moderators
  • [x ] admins
  • developers

What changes to the GraphQL/Database Schema does this PR introduce?

  • creates BskyAuthIntegration type and adds it to AuthIntegrations
  • creates BskyProfile type and adds it to Profile
  • creates SettingsBskyAuthIntegrationInput input and adds it to SettingsAuthIntegrationsInput

Does this PR introduce any new environment variables or feature flags?

nope

If any indexes were added, were they added to INDEXES.md?

no indexes

How do I test this PR?

  • New auth integration tests mentioned in the issue above are included in this branch.
    To manually test:
  1. Enable the auth integration in Admin/Config/Auth
  2. Then trying to login with a valid bluesky or atproto handle

Were any tests migrated to React Testing Library?

nope

How do we deploy this PR?

normally, but the per tenant client-metadata.json route needs to be tested, and correct URL callback values confirmed in a staging env prior to deploying to production envs.

immber and others added 30 commits February 26, 2025 17:05
- proxy agents and secure pre-compute agents are missing in
  scraper.ts and in fetch.ts -> createFetch() func
Copy link

netlify bot commented Apr 30, 2025

Deploy Preview for gallant-galileo-14878c canceled.

Name Link
🔨 Latest commit efa8237
🔍 Latest deploy log https://app.netlify.com/sites/gallant-galileo-14878c/deploys/68126fac72f8610008626699

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.

2 participants