-
Notifications
You must be signed in to change notification settings - Fork 784
Single sign-on #4060
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
base: main
Are you sure you want to change the base?
Single sign-on #4060
Conversation
|
As a heads up, many of the relevant Bluesky staff are traveling for a couple weeks, and it may take some time to comment or give focused feedback. |
|
All we need now is to think of now is what rate limits to apply if I'm correct. |
Minion3665
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A comment about a build failure here - as well as you seem not to have run pnpm codegen which causes further failures
I think your code probably(?) all works, but it definitely doesn't build in any mode where full type checking is happening. I'll comment more once I've got this up and running and tested this a bit...
| if (typeof req.headers.cookie === 'string') { | ||
| try { | ||
| const cookies: Record<string, string | undefined> | ||
| = cookie.parse(req.headers.cookie); | ||
|
|
||
| callbackId = cookies["atproto-callback"]; | ||
| } catch (err) { | ||
| throw new InvalidRequestError(`Invalid cookie header: ${err}`); | ||
| } | ||
| } | ||
|
|
||
| if (!callbackId) { | ||
| throw new InvalidRequestError("Missing callback cookie"); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't really need the cookie here, since you have that from the state parameter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I rewrote the cookie logic to do encryption, so that the state inside the cookie can provide CSRF protection.
379fa70 to
7ab3425
Compare
f29e577 to
985743e
Compare
Implements the first step towards #2796. This PR introduces foundational support for single sign-on in the PDS, allowing users to authenticate using identity providers. Tested successfully with Github, Discord (OAuth) and Gitlab (OIDC) so far.
Summary of changes
SSOManagerclass responsible for verifying external identity tokens and mapping them to PDS accounts.Key behavior
Notes
com.atproto.sso.getRedirectandcom.atproto.sso.getCallback, this might not be desired behavior.SSOManagerabstraction.Test it yourself
make deps && pnpm codegen && make buildand run the PDS behind a reverse proxy (OIDC requires usage of TLS for the redirect URI usually).curl -X POST -H "Content-Type: application/json" -d @github.json https://<hostname>/xrpc/com.atproto.admin.createIdentityProvider(see below).curl --verbose "https://<hostname>/xrpc/com.atproto.sso.getRedirect?idpId=gitlab&redirectUri=https%3A%2F%2F<hostname>%2Fxrpc%2Fcom.atproto.sso.getCallback".Locationheader and open the link.Example of
github.json{ "id": "github", "name": "Github", "issuer": "https://github.com", "clientId": "xxx", "clientSecret": "xxx", "scope": "read:user user:email", "usePkce": false, "discoverable": false, "metadata": { "endpoints": { "authorization": "https://github.com/login/oauth/authorize", "token": "https://github.com/login/oauth/access_token", "userinfo": "https://api.github.com/user" }, "mappings": { "sub": "id", "username": "login" }, "authMethods": ["client_secret_post"] } }On assignment from @muni-town collective.