- Notifications
You must be signed in to change notification settings - Fork1k
feat: support the OAuth2 device flow with GitHub for signing in#16585
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Changes from1 commit
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
- Loading branch information
Uh oh!
There was an error while loading.Please reload this page.
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -677,12 +677,13 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd. | ||
| } | ||
| } | ||
| if vals.OAuth2.Github.ClientSecret != ""|| vals.OAuth2.Github.DeviceFlow.Value(){ | ||
| options.GithubOAuth2Config, err = configureGithubOAuth2( | ||
| oauthInstrument, | ||
| vals.AccessURL.Value(), | ||
| vals.OAuth2.Github.ClientID.String(), | ||
| vals.OAuth2.Github.ClientSecret.String(), | ||
| vals.OAuth2.Github.DeviceFlow.Value(), | ||
| vals.OAuth2.Github.AllowSignups.Value(), | ||
| vals.OAuth2.Github.AllowEveryone.Value(), | ||
| vals.OAuth2.Github.AllowedOrgs, | ||
| @@ -1831,8 +1832,10 @@ func configureCAPool(tlsClientCAFile string, tlsConfig *tls.Config) error { | ||
| return nil | ||
| } | ||
| // TODO: convert the argument list to a struct, it's easy to mix up the order of the arguments | ||
| // | ||
| //nolint:revive // Ignore flag-parameter: parameter 'allowEveryone' seems to be a control flag, avoid control coupling (revive) | ||
| func configureGithubOAuth2(instrument *promoauth.Factory, accessURL *url.URL, clientID, clientSecret string,deviceFlow,allowSignups, allowEveryone bool, allowOrgs []string, rawTeams []string, enterpriseBaseURL string) (*coderd.GithubOAuth2Config, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. We should really make these args a struct soon. Not necessary in this PR, just a ton of options that can be misorderd. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Added a TODO | ||
| redirectURL, err := accessURL.Parse("/api/v2/users/oauth2/github/callback") | ||
| if err != nil { | ||
| return nil, xerrors.Errorf("parse github oauth callback url: %w", err) | ||
| @@ -1898,6 +1901,17 @@ func configureGithubOAuth2(instrument *promoauth.Factory, accessURL *url.URL, cl | ||
| return github.NewClient(client), nil | ||
| } | ||
| var deviceAuth *externalauth.DeviceAuth | ||
| if deviceFlow { | ||
| deviceAuth = &externalauth.DeviceAuth{ | ||
| Config: instrumentedOauth, | ||
| ClientID: clientID, | ||
| TokenURL: endpoint.TokenURL, | ||
| Scopes: []string{"read:user", "read:org", "user:email"}, | ||
| CodeURL: endpoint.DeviceAuthURL, | ||
| } | ||
| } | ||
| return &coderd.GithubOAuth2Config{ | ||
| OAuth2Config: instrumentedOauth, | ||
| AllowSignups: allowSignups, | ||
| @@ -1941,6 +1955,19 @@ func configureGithubOAuth2(instrument *promoauth.Factory, accessURL *url.URL, cl | ||
| team, _, err := api.Teams.GetTeamMembershipBySlug(ctx, org, teamSlug, username) | ||
| return team, err | ||
| }, | ||
| DeviceFlowEnabled: deviceFlow, | ||
| ExchangeDeviceCode: func(ctx context.Context, deviceCode string) (*oauth2.Token, error) { | ||
| if !deviceFlow { | ||
| return nil, xerrors.New("device flow is not enabled") | ||
| } | ||
| return deviceAuth.ExchangeDeviceCode(ctx, deviceCode) | ||
| }, | ||
| AuthorizeDevice: func(ctx context.Context) (*codersdk.ExternalAuthDevice, error) { | ||
| if !deviceFlow { | ||
| return nil, xerrors.New("device flow is not enabled") | ||
| } | ||
| return deviceAuth.AuthorizeDevice(ctx) | ||
| }, | ||
| }, nil | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -167,9 +167,16 @@ func ExtractOAuth2(config promoauth.OAuth2Config, client *http.Client, authURLOp | ||
| oauthToken, err := config.Exchange(ctx, code) | ||
| if err != nil { | ||
| errorCode := http.StatusInternalServerError | ||
| detail := err.Error() | ||
| if detail == "authorization_pending" { | ||
| // In the device flow, the token may not be immediately | ||
| // available. This is expected, and the client will retry. | ||
| errorCode = http.StatusBadRequest | ||
| } | ||
Comment on lines +172 to +176 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Is this error comparison correct? Comparing to the usage in the oauth lib, it is checking the Usage:https://github.com/golang/oauth2/blob/master/deviceauth.go#L189-L190 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. We actually don't use the For context, we use a custom implementation instead of the lib's | ||
| httpapi.Write(ctx, rw, errorCode, codersdk.Response{ | ||
| Message: "Failed exchanging Oauth code.", | ||
| Detail: detail, | ||
| }) | ||
| return | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -748,12 +748,32 @@ type GithubOAuth2Config struct { | ||
| ListOrganizationMemberships func(ctx context.Context, client *http.Client) ([]*github.Membership, error) | ||
| TeamMembership func(ctx context.Context, client *http.Client, org, team, username string) (*github.Membership, error) | ||
| DeviceFlowEnabled bool | ||
| ExchangeDeviceCode func(ctx context.Context, deviceCode string) (*oauth2.Token, error) | ||
| AuthorizeDevice func(ctx context.Context) (*codersdk.ExternalAuthDevice, error) | ||
| AllowSignups bool | ||
| AllowEveryone bool | ||
| AllowOrganizations []string | ||
| AllowTeams []GithubOAuth2Team | ||
| } | ||
| func (c *GithubOAuth2Config) Exchange(ctx context.Context, code string, opts ...oauth2.AuthCodeOption) (*oauth2.Token, error) { | ||
| if !c.DeviceFlowEnabled { | ||
| return c.OAuth2Config.Exchange(ctx, code, opts...) | ||
| } | ||
| return c.ExchangeDeviceCode(ctx, code) | ||
| } | ||
| func (c *GithubOAuth2Config) AuthCodeURL(state string, opts ...oauth2.AuthCodeOption) string { | ||
| if !c.DeviceFlowEnabled { | ||
| return c.OAuth2Config.AuthCodeURL(state, opts...) | ||
| } | ||
| // This is an absolute path in the Coder app. The device flow is orchestrated | ||
| // by the Coder frontend, so we need to redirect the user to the device flow page. | ||
| return "/login/device?state=" + state | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Is this a relative url to our Coder app? Not an external auth url? Can you just leave a comment that mentions this. I say that becaused | ||
| } | ||
| // @Summary Get authentication methods | ||
| // @ID get-authentication-methods | ||
| // @Security CoderSessionToken | ||
| @@ -786,6 +806,53 @@ func (api *API) userAuthMethods(rw http.ResponseWriter, r *http.Request) { | ||
| }) | ||
| } | ||
| // @Summary Get Github device auth. | ||
| // @ID get-github-device-auth | ||
| // @Security CoderSessionToken | ||
| // @Produce json | ||
| // @Tags Users | ||
| // @Success 200 {object} codersdk.ExternalAuthDevice | ||
| // @Router /users/oauth2/github/device [get] | ||
| func (api *API) userOAuth2GithubDevice(rw http.ResponseWriter, r *http.Request) { | ||
| var ( | ||
| ctx = r.Context() | ||
| auditor = api.Auditor.Load() | ||
| aReq, commitAudit = audit.InitRequest[database.APIKey](rw, &audit.RequestParams{ | ||
| Audit: *auditor, | ||
| Log: api.Logger, | ||
| Request: r, | ||
| Action: database.AuditActionLogin, | ||
| }) | ||
| ) | ||
| aReq.Old = database.APIKey{} | ||
| defer commitAudit() | ||
| if api.GithubOAuth2Config == nil { | ||
| httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ | ||
| Message: "Github OAuth2 is not enabled.", | ||
| }) | ||
| return | ||
| } | ||
| if !api.GithubOAuth2Config.DeviceFlowEnabled { | ||
| httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ | ||
| Message: "Device flow is not enabled for Github OAuth2.", | ||
| }) | ||
| return | ||
| } | ||
| deviceAuth, err := api.GithubOAuth2Config.AuthorizeDevice(ctx) | ||
| if err != nil { | ||
| httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ | ||
| Message: "Failed to authorize device.", | ||
| Detail: err.Error(), | ||
| }) | ||
| return | ||
| } | ||
| httpapi.Write(ctx, rw, http.StatusOK, deviceAuth) | ||
| } | ||
| // @Summary OAuth 2.0 GitHub Callback | ||
| // @ID oauth-20-github-callback | ||
| // @Security CoderSessionToken | ||
| @@ -1016,7 +1083,14 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) { | ||
| } | ||
| redirect = uriFromURL(redirect) | ||
| if api.GithubOAuth2Config.DeviceFlowEnabled { | ||
| // In the device flow, the redirect is handled client-side. | ||
| httpapi.Write(ctx, rw, http.StatusOK, codersdk.OAuth2DeviceFlowCallbackResponse{ | ||
| RedirectURL: redirect, | ||
| }) | ||
Comment on lines +1087 to +1090 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. 👍 | ||
| } else { | ||
| http.Redirect(rw, r, redirect, http.StatusTemporaryRedirect) | ||
| } | ||
| } | ||
| type OIDCConfig struct { | ||