From 4e9c234379c8ee67314bca6a45a0cf97e1cffea4 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Tue, 27 Jun 2023 15:54:29 +0000 Subject: [PATCH 01/17] feat: add github device flow for authentication This will allow us to add a GitHub OAuth provider out-of-the-box to reduce setup requirements. --- cli/server.go | 8 + coderd/apidoc/docs.go | 6 + coderd/apidoc/swagger.json | 6 + coderd/coderd.go | 16 +- coderd/gitauth.go | 186 +++++++++++ coderd/gitauth/config.go | 24 +- coderd/gitauth/config_test.go | 9 + coderd/gitauth/oauth.go | 122 +++++++- coderd/gitauth_test.go | 290 ++++++++++++++++++ coderd/httpmw/oauth2.go | 8 +- coderd/workspaceagents.go | 64 ---- coderd/workspaceagents_test.go | 268 ---------------- codersdk/authorization.go | 18 ++ codersdk/deployment.go | 22 +- docs/api/general.md | 2 + docs/api/schemas.md | 32 +- site/src/AppRouter.tsx | 10 +- site/src/api/api.ts | 8 + site/src/api/typesGenerated.ts | 7 + .../components/SignInLayout/SignInLayout.tsx | 3 + .../GitAuthDevicePage/GitAuthDevicePage.tsx | 158 ++++++++++ site/vite.config.ts | 6 + 22 files changed, 901 insertions(+), 372 deletions(-) create mode 100644 coderd/gitauth.go create mode 100644 coderd/gitauth_test.go create mode 100644 site/src/pages/GitAuthDevicePage/GitAuthDevicePage.tsx diff --git a/cli/server.go b/cli/server.go index 3e48b2d318b7e..e69451a666501 100644 --- a/cli/server.go +++ b/cli/server.go @@ -148,6 +148,14 @@ func ReadGitAuthProvidersFromEnv(environ []string) ([]codersdk.GitAuthConfig, er provider.ValidateURL = v.Value case "REGEX": provider.Regex = v.Value + case "DEVICE_FLOW": + b, err := strconv.ParseBool(v.Value) + if err != nil { + return nil, xerrors.Errorf("parse bool: %s", v.Value) + } + provider.DeviceFlow = b + case "DEVICE_AUTH_URL": + provider.DeviceAuthURL = v.Value case "NO_REFRESH": b, err := strconv.ParseBool(v.Value) if err != nil { diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 872fd022878cf..da3d236337b65 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -7449,6 +7449,12 @@ const docTemplate = `{ "client_id": { "type": "string" }, + "device_auth_url": { + "type": "string" + }, + "device_flow": { + "type": "boolean" + }, "id": { "type": "string" }, diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 56db90e9f26e8..c3779de6be6e1 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -6670,6 +6670,12 @@ "client_id": { "type": "string" }, + "device_auth_url": { + "type": "string" + }, + "device_flow": { + "type": "boolean" + }, "id": { "type": "string" }, diff --git a/coderd/coderd.go b/coderd/coderd.go index 41ddcf4bbda58..87301524013f5 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -458,11 +458,17 @@ func New(options *Options) *API { r.Route("/gitauth", func(r chi.Router) { for _, gitAuthConfig := range options.GitAuthConfigs { r.Route(fmt.Sprintf("/%s", gitAuthConfig.ID), func(r chi.Router) { - r.Use( - httpmw.ExtractOAuth2(gitAuthConfig, options.HTTPClient, nil), - apiKeyMiddlewareRedirect, - ) - r.Get("/callback", api.gitAuthCallback(gitAuthConfig)) + r.Use(apiKeyMiddlewareRedirect) + + useDeviceAuth := gitAuthConfig.DeviceAuth != nil + if useDeviceAuth { + r.Use(api.gitAuthDeviceRedirect(gitAuthConfig)) + r.Post("/exchange", api.postGitAuthExchange(gitAuthConfig)) + } else { + // If device auth isn't in use, then the git provider is using OAuth2! + r.Use(httpmw.ExtractOAuth2(gitAuthConfig, options.HTTPClient, nil)) + r.Get("/callback", api.gitAuthCallback(gitAuthConfig)) + } }) } }) diff --git a/coderd/gitauth.go b/coderd/gitauth.go new file mode 100644 index 0000000000000..d65c838733de0 --- /dev/null +++ b/coderd/gitauth.go @@ -0,0 +1,186 @@ +package coderd + +import ( + "database/sql" + "errors" + "fmt" + "net/http" + "net/url" + + "github.com/coder/coder/coderd/database" + "github.com/coder/coder/coderd/gitauth" + "github.com/coder/coder/coderd/httpapi" + "github.com/coder/coder/coderd/httpmw" + "github.com/coder/coder/codersdk" +) + +func (*API) gitAuthDeviceRedirect(gitAuthConfig *gitauth.Config) func(http.Handler) http.Handler { + route := fmt.Sprintf("/gitauth/%s/device", gitAuthConfig.ID) + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + deviceCode := r.URL.Query().Get("device_code") + if r.Method != http.MethodGet || deviceCode != "" { + next.ServeHTTP(w, r) + return + } + // If no device code is provided, redirect to the dashboard with query params! + deviceAuth, err := gitAuthConfig.DeviceAuth.AuthorizeDevice(r.Context()) + if err != nil { + httpapi.Write(r.Context(), w, http.StatusInternalServerError, codersdk.Response{ + Message: "Failed to authorize device.", + Detail: err.Error(), + }) + return + } + v := url.Values{ + "device_code": {deviceAuth.DeviceCode}, + "user_code": {deviceAuth.UserCode}, + "expires_in": {fmt.Sprintf("%d", deviceAuth.ExpiresIn)}, + "interval": {fmt.Sprintf("%d", deviceAuth.Interval)}, + "verification_uri": {deviceAuth.VerificationURI}, + } + http.Redirect(w, r, fmt.Sprintf("%s?%s", route, v.Encode()), http.StatusTemporaryRedirect) + }) + } +} + +func (api *API) postGitAuthExchange(gitAuthConfig *gitauth.Config) http.HandlerFunc { + return func(rw http.ResponseWriter, r *http.Request) { + ctx := r.Context() + apiKey := httpmw.APIKey(r) + + var req codersdk.ExchangeGitAuthRequest + if !httpapi.Read(ctx, rw, r, &req) { + return + } + + if gitAuthConfig.DeviceAuth == nil { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Git auth provider does not support device flow.", + }) + return + } + + token, err := gitAuthConfig.DeviceAuth.ExchangeDeviceCode(ctx, req.DeviceCode) + if err != nil { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Failed to exchange device code.", + Detail: err.Error(), + }) + return + } + + _, err = api.Database.GetGitAuthLink(ctx, database.GetGitAuthLinkParams{ + ProviderID: gitAuthConfig.ID, + UserID: apiKey.UserID, + }) + if err != nil { + if !errors.Is(err, sql.ErrNoRows) { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Failed to get git auth link.", + Detail: err.Error(), + }) + return + } + + _, err = api.Database.InsertGitAuthLink(ctx, database.InsertGitAuthLinkParams{ + ProviderID: gitAuthConfig.ID, + UserID: apiKey.UserID, + CreatedAt: database.Now(), + UpdatedAt: database.Now(), + OAuthAccessToken: token.AccessToken, + OAuthRefreshToken: token.RefreshToken, + OAuthExpiry: token.Expiry, + }) + if err != nil { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Failed to insert git auth link.", + Detail: err.Error(), + }) + return + } + } else { + _, err = api.Database.UpdateGitAuthLink(ctx, database.UpdateGitAuthLinkParams{ + ProviderID: gitAuthConfig.ID, + UserID: apiKey.UserID, + UpdatedAt: database.Now(), + OAuthAccessToken: token.AccessToken, + OAuthRefreshToken: token.RefreshToken, + OAuthExpiry: token.Expiry, + }) + if err != nil { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Failed to update git auth link.", + Detail: err.Error(), + }) + return + } + } + httpapi.Write(ctx, rw, http.StatusNoContent, nil) + } +} + +// device get +func (api *API) gitAuthCallback(gitAuthConfig *gitauth.Config) http.HandlerFunc { + return func(rw http.ResponseWriter, r *http.Request) { + var ( + ctx = r.Context() + state = httpmw.OAuth2(r) + apiKey = httpmw.APIKey(r) + ) + + _, err := api.Database.GetGitAuthLink(ctx, database.GetGitAuthLinkParams{ + ProviderID: gitAuthConfig.ID, + UserID: apiKey.UserID, + }) + if err != nil { + if !errors.Is(err, sql.ErrNoRows) { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Failed to get git auth link.", + Detail: err.Error(), + }) + return + } + + _, err = api.Database.InsertGitAuthLink(ctx, database.InsertGitAuthLinkParams{ + ProviderID: gitAuthConfig.ID, + UserID: apiKey.UserID, + CreatedAt: database.Now(), + UpdatedAt: database.Now(), + OAuthAccessToken: state.Token.AccessToken, + OAuthRefreshToken: state.Token.RefreshToken, + OAuthExpiry: state.Token.Expiry, + }) + if err != nil { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Failed to insert git auth link.", + Detail: err.Error(), + }) + return + } + } else { + _, err = api.Database.UpdateGitAuthLink(ctx, database.UpdateGitAuthLinkParams{ + ProviderID: gitAuthConfig.ID, + UserID: apiKey.UserID, + UpdatedAt: database.Now(), + OAuthAccessToken: state.Token.AccessToken, + OAuthRefreshToken: state.Token.RefreshToken, + OAuthExpiry: state.Token.Expiry, + }) + if err != nil { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Failed to update git auth link.", + Detail: err.Error(), + }) + return + } + } + + redirect := state.Redirect + if redirect == "" { + // This is a nicely rendered screen on the frontend + redirect = "/gitauth" + } + http.Redirect(rw, r, redirect, http.StatusTemporaryRedirect) + } +} diff --git a/coderd/gitauth/config.go b/coderd/gitauth/config.go index 133af4023fb8f..bdb9c961eb69f 100644 --- a/coderd/gitauth/config.go +++ b/coderd/gitauth/config.go @@ -36,6 +36,8 @@ type Config struct { // returning it to the user. If omitted, tokens will // not be validated before being returned. ValidateURL string + // DeviceAuth is set if the provider uses the device flow. + DeviceAuth *DeviceAuth } // RefreshToken automatically refreshes the token if expired and permitted. @@ -187,17 +189,33 @@ func ConvertConfig(entries []codersdk.GitAuthConfig, accessURL *url.URL) ([]*Con var oauthConfig httpmw.OAuth2Config = oauth2Config // Azure DevOps uses JWT token authentication! if typ == codersdk.GitProviderAzureDevops { - oauthConfig = newJWTOAuthConfig(oauth2Config) + oauthConfig = &jwtConfig{oauth2Config} } - configs = append(configs, &Config{ + cfg := &Config{ OAuth2Config: oauthConfig, ID: entry.ID, Regex: regex, Type: typ, NoRefresh: entry.NoRefresh, ValidateURL: entry.ValidateURL, - }) + } + + if entry.DeviceFlow { + if entry.DeviceAuthURL == "" { + entry.DeviceAuthURL = deviceAuthURL[typ] + } + if entry.DeviceAuthURL == "" { + return nil, xerrors.Errorf("git auth provider %q: device auth url must be provided", entry.ID) + } + cfg.DeviceAuth = &DeviceAuth{ + config: oauth2Config, + URL: entry.DeviceAuthURL, + ID: entry.ID, + } + } + + configs = append(configs, cfg) } return configs, nil } diff --git a/coderd/gitauth/config_test.go b/coderd/gitauth/config_test.go index 69bb564738056..82595bbbe34e5 100644 --- a/coderd/gitauth/config_test.go +++ b/coderd/gitauth/config_test.go @@ -169,6 +169,15 @@ func TestConvertYAML(t *testing.T) { Regex: `\K`, }}, Error: "compile regex for git auth provider", + }, { + Name: "NoDeviceURL", + Input: []codersdk.GitAuthConfig{{ + Type: string(codersdk.GitProviderGitLab), + ClientID: "example", + ClientSecret: "example", + DeviceFlow: true, + }}, + Error: "device auth url must be provided", }} { tc := tc t.Run(tc.Name, func(t *testing.T) { diff --git a/coderd/gitauth/oauth.go b/coderd/gitauth/oauth.go index c9008dff7697b..5ad611589017d 100644 --- a/coderd/gitauth/oauth.go +++ b/coderd/gitauth/oauth.go @@ -1,14 +1,18 @@ package gitauth import ( + "bytes" "context" + "encoding/json" + "net/http" "net/url" "regexp" + "strings" "golang.org/x/oauth2" "golang.org/x/oauth2/github" + "golang.org/x/xerrors" - "github.com/coder/coder/coderd/httpmw" "github.com/coder/coder/codersdk" ) @@ -36,6 +40,10 @@ var validateURL = map[codersdk.GitProvider]string{ codersdk.GitProviderBitBucket: "https://api.bitbucket.org/2.0/user", } +var deviceAuthURL = map[codersdk.GitProvider]string{ + codersdk.GitProviderGitHub: "https://github.com/login/device/code", +} + // scope contains defaults for each Git provider. var scope = map[codersdk.GitProvider][]string{ codersdk.GitProviderAzureDevops: {"vso.code_write"}, @@ -54,13 +62,9 @@ var regex = map[codersdk.GitProvider]*regexp.Regexp{ codersdk.GitProviderGitHub: regexp.MustCompile(`^(https?://)?github\.com(/.*)?$`), } -// newJWTOAuthConfig creates a new OAuth2 config that uses a custom +// jwtConfig is a new OAuth2 config that uses a custom // assertion method that works with Azure Devops. See: // https://learn.microsoft.com/en-us/azure/devops/integrate/get-started/authentication/oauth?view=azure-devops -func newJWTOAuthConfig(config *oauth2.Config) httpmw.OAuth2Config { - return &jwtConfig{config} -} - type jwtConfig struct { *oauth2.Config } @@ -89,3 +93,109 @@ func (c *jwtConfig) Exchange(ctx context.Context, code string, opts ...oauth2.Au )..., ) } + +type DeviceAuth struct { + config *oauth2.Config + + ID string + URL string +} + +// DeviceAuthorization is the response from the device authorization endpoint. +// See: https://tools.ietf.org/html/rfc8628#section-3.2 +type DeviceAuthorization struct { + DeviceCode string `json:"device_code"` + UserCode string `json:"user_code"` + VerificationURI string `json:"verification_uri"` + ExpiresIn int `json:"expires_in"` + Interval int `json:"interval"` +} + +// AuthorizeDevice begins the device authorization flow. +// See: https://tools.ietf.org/html/rfc8628#section-3.1 +func (c *DeviceAuth) AuthorizeDevice(ctx context.Context) (*DeviceAuthorization, error) { + if c.URL == "" { + return nil, xerrors.New("oauth2: device code URL not set") + } + req, err := http.NewRequestWithContext(ctx, http.MethodPost, c.formatDeviceCodeURL(), nil) + if err != nil { + return nil, err + } + req.Header.Set("Accept", "application/json") + resp, err := http.DefaultClient.Do(req) + if err != nil { + return nil, err + } + defer resp.Body.Close() + var da DeviceAuthorization + return &da, json.NewDecoder(resp.Body).Decode(&da) +} + +// ExchangeDeviceCode exchanges a device code for an access token. +// The boolean returned indicates whether the device code is still pending +// and the caller should try again. +func (c *DeviceAuth) ExchangeDeviceCode(ctx context.Context, deviceCode string) (*oauth2.Token, error) { + if c.URL == "" { + return nil, xerrors.New("oauth2: device code URL not set") + } + req, err := http.NewRequestWithContext(ctx, http.MethodPost, c.formatDeviceTokenURL(deviceCode), nil) + if err != nil { + return nil, err + } + req.Header.Set("Accept", "application/json") + resp, err := http.DefaultClient.Do(req) + if err != nil { + return nil, err + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + return nil, codersdk.ReadBodyAsError(resp) + } + var body struct { + *oauth2.Token + Error string `json:"error"` + ErrorDescription string `json:"error_description"` + } + err = json.NewDecoder(resp.Body).Decode(&body) + if err != nil { + return nil, err + } + if body.Error != "" { + return nil, xerrors.Errorf("%s", body.Error) + } + return body.Token, nil +} + +func (c *DeviceAuth) formatDeviceTokenURL(deviceCode string) string { + var buf bytes.Buffer + _, _ = buf.WriteString(c.config.Endpoint.TokenURL) + v := url.Values{ + "client_id": {c.config.ClientID}, + "device_code": {deviceCode}, + "grant_type": {"urn:ietf:params:oauth:grant-type:device_code"}, + } + if strings.Contains(c.config.Endpoint.TokenURL, "?") { + _ = buf.WriteByte('&') + } else { + _ = buf.WriteByte('?') + } + _, _ = buf.WriteString(v.Encode()) + return buf.String() +} + +func (c *DeviceAuth) formatDeviceCodeURL() string { + var buf bytes.Buffer + _, _ = buf.WriteString(c.URL) + + v := url.Values{ + "client_id": {c.config.ClientID}, + "scope": c.config.Scopes, + } + if strings.Contains(c.URL, "?") { + _ = buf.WriteByte('&') + } else { + _ = buf.WriteByte('?') + } + _, _ = buf.WriteString(v.Encode()) + return buf.String() +} diff --git a/coderd/gitauth_test.go b/coderd/gitauth_test.go new file mode 100644 index 0000000000000..ddea1357e9422 --- /dev/null +++ b/coderd/gitauth_test.go @@ -0,0 +1,290 @@ +package coderd_test + +import ( + "context" + "fmt" + "net/http" + "net/http/httptest" + "regexp" + "strings" + "testing" + "time" + + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/oauth2" + + "github.com/coder/coder/coderd/coderdtest" + "github.com/coder/coder/coderd/database" + "github.com/coder/coder/coderd/gitauth" + "github.com/coder/coder/codersdk" + "github.com/coder/coder/codersdk/agentsdk" + "github.com/coder/coder/provisioner/echo" + "github.com/coder/coder/provisionersdk/proto" + "github.com/coder/coder/testutil" +) + +// nolint:bodyclose +func TestWorkspaceAgentsGitAuth(t *testing.T) { + t.Parallel() + t.Run("NoMatchingConfig", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{ + IncludeProvisionerDaemon: true, + GitAuthConfigs: []*gitauth.Config{}, + }) + user := coderdtest.CreateFirstUser(t, client) + authToken := uuid.NewString() + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ + Parse: echo.ParseComplete, + ProvisionPlan: echo.ProvisionComplete, + ProvisionApply: echo.ProvisionApplyWithAgent(authToken), + }) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) + coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) + + agentClient := agentsdk.New(client.URL) + agentClient.SetSessionToken(authToken) + _, err := agentClient.GitAuth(context.Background(), "github.com", false) + var apiError *codersdk.Error + require.ErrorAs(t, err, &apiError) + require.Equal(t, http.StatusNotFound, apiError.StatusCode()) + }) + t.Run("ReturnsURL", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{ + IncludeProvisionerDaemon: true, + GitAuthConfigs: []*gitauth.Config{{ + OAuth2Config: &testutil.OAuth2Config{}, + ID: "github", + Regex: regexp.MustCompile(`github\.com`), + Type: codersdk.GitProviderGitHub, + }}, + }) + user := coderdtest.CreateFirstUser(t, client) + authToken := uuid.NewString() + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ + Parse: echo.ParseComplete, + ProvisionPlan: echo.ProvisionComplete, + ProvisionApply: []*proto.Provision_Response{{ + Type: &proto.Provision_Response_Complete{ + Complete: &proto.Provision_Complete{ + Resources: []*proto.Resource{{ + Name: "example", + Type: "aws_instance", + Agents: []*proto.Agent{{ + Id: uuid.NewString(), + Auth: &proto.Agent_Token{ + Token: authToken, + }, + }}, + }}, + }, + }, + }}, + }) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) + coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) + + agentClient := agentsdk.New(client.URL) + agentClient.SetSessionToken(authToken) + token, err := agentClient.GitAuth(context.Background(), "github.com/asd/asd", false) + require.NoError(t, err) + require.True(t, strings.HasSuffix(token.URL, fmt.Sprintf("/gitauth/%s", "github"))) + }) + t.Run("UnauthorizedCallback", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{ + IncludeProvisionerDaemon: true, + GitAuthConfigs: []*gitauth.Config{{ + OAuth2Config: &testutil.OAuth2Config{}, + ID: "github", + Regex: regexp.MustCompile(`github\.com`), + Type: codersdk.GitProviderGitHub, + }}, + }) + resp := coderdtest.RequestGitAuthCallback(t, "github", client) + require.Equal(t, http.StatusSeeOther, resp.StatusCode) + }) + t.Run("AuthorizedCallback", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{ + IncludeProvisionerDaemon: true, + GitAuthConfigs: []*gitauth.Config{{ + OAuth2Config: &testutil.OAuth2Config{}, + ID: "github", + Regex: regexp.MustCompile(`github\.com`), + Type: codersdk.GitProviderGitHub, + }}, + }) + _ = coderdtest.CreateFirstUser(t, client) + resp := coderdtest.RequestGitAuthCallback(t, "github", client) + require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) + location, err := resp.Location() + require.NoError(t, err) + require.Equal(t, "/gitauth", location.Path) + + // Callback again to simulate updating the token. + resp = coderdtest.RequestGitAuthCallback(t, "github", client) + require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) + }) + t.Run("ValidateURL", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitLong) + + srv := httptest.NewServer(nil) + defer srv.Close() + client := coderdtest.New(t, &coderdtest.Options{ + IncludeProvisionerDaemon: true, + GitAuthConfigs: []*gitauth.Config{{ + ValidateURL: srv.URL, + OAuth2Config: &testutil.OAuth2Config{}, + ID: "github", + Regex: regexp.MustCompile(`github\.com`), + Type: codersdk.GitProviderGitHub, + }}, + }) + user := coderdtest.CreateFirstUser(t, client) + authToken := uuid.NewString() + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ + Parse: echo.ParseComplete, + ProvisionPlan: echo.ProvisionComplete, + ProvisionApply: echo.ProvisionApplyWithAgent(authToken), + }) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) + coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) + + agentClient := agentsdk.New(client.URL) + agentClient.SetSessionToken(authToken) + + resp := coderdtest.RequestGitAuthCallback(t, "github", client) + require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) + + // If the validation URL says unauthorized, the callback + // URL to re-authenticate should be returned. + srv.Config.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + }) + res, err := agentClient.GitAuth(ctx, "github.com/asd/asd", false) + require.NoError(t, err) + require.NotEmpty(t, res.URL) + + // If the validation URL gives a non-OK status code, this + // should be treated as an internal server error. + srv.Config.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusForbidden) + w.Write([]byte("Something went wrong!")) + }) + _, err = agentClient.GitAuth(ctx, "github.com/asd/asd", false) + var apiError *codersdk.Error + require.ErrorAs(t, err, &apiError) + require.Equal(t, http.StatusInternalServerError, apiError.StatusCode()) + require.Equal(t, "validate git auth token: status 403: body: Something went wrong!", apiError.Detail) + }) + + t.Run("ExpiredNoRefresh", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{ + IncludeProvisionerDaemon: true, + GitAuthConfigs: []*gitauth.Config{{ + OAuth2Config: &testutil.OAuth2Config{ + Token: &oauth2.Token{ + AccessToken: "token", + RefreshToken: "something", + Expiry: database.Now().Add(-time.Hour), + }, + }, + ID: "github", + Regex: regexp.MustCompile(`github\.com`), + Type: codersdk.GitProviderGitHub, + NoRefresh: true, + }}, + }) + user := coderdtest.CreateFirstUser(t, client) + authToken := uuid.NewString() + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ + Parse: echo.ParseComplete, + ProvisionPlan: echo.ProvisionComplete, + ProvisionApply: echo.ProvisionApplyWithAgent(authToken), + }) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) + coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) + + agentClient := agentsdk.New(client.URL) + agentClient.SetSessionToken(authToken) + + token, err := agentClient.GitAuth(context.Background(), "github.com/asd/asd", false) + require.NoError(t, err) + require.NotEmpty(t, token.URL) + + // In the configuration, we set our OAuth provider + // to return an expired token. Coder consumes this + // and stores it. + resp := coderdtest.RequestGitAuthCallback(t, "github", client) + require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) + + // Because the token is expired and `NoRefresh` is specified, + // a redirect URL should be returned again. + token, err = agentClient.GitAuth(context.Background(), "github.com/asd/asd", false) + require.NoError(t, err) + require.NotEmpty(t, token.URL) + }) + + t.Run("FullFlow", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{ + IncludeProvisionerDaemon: true, + GitAuthConfigs: []*gitauth.Config{{ + OAuth2Config: &testutil.OAuth2Config{}, + ID: "github", + Regex: regexp.MustCompile(`github\.com`), + Type: codersdk.GitProviderGitHub, + }}, + }) + user := coderdtest.CreateFirstUser(t, client) + authToken := uuid.NewString() + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ + Parse: echo.ParseComplete, + ProvisionPlan: echo.ProvisionComplete, + ProvisionApply: echo.ProvisionApplyWithAgent(authToken), + }) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) + coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) + + agentClient := agentsdk.New(client.URL) + agentClient.SetSessionToken(authToken) + + token, err := agentClient.GitAuth(context.Background(), "github.com/asd/asd", false) + require.NoError(t, err) + require.NotEmpty(t, token.URL) + + // Start waiting for the token callback... + tokenChan := make(chan agentsdk.GitAuthResponse, 1) + go func() { + token, err := agentClient.GitAuth(context.Background(), "github.com/asd/asd", true) + assert.NoError(t, err) + tokenChan <- token + }() + + time.Sleep(250 * time.Millisecond) + + resp := coderdtest.RequestGitAuthCallback(t, "github", client) + require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) + token = <-tokenChan + require.Equal(t, "access_token", token.Username) + + token, err = agentClient.GitAuth(context.Background(), "github.com/asd/asd", false) + require.NoError(t, err) + }) +} diff --git a/coderd/httpmw/oauth2.go b/coderd/httpmw/oauth2.go index be2648f474512..0e8251a8faaf8 100644 --- a/coderd/httpmw/oauth2.go +++ b/coderd/httpmw/oauth2.go @@ -67,19 +67,19 @@ func ExtractOAuth2(config OAuth2Config, client *http.Client, authURLOpts map[str // OIDC errors can be returned as query parameters. This can happen // if for example we are providing and invalid scope. // We should terminate the OIDC process if we encounter an error. - oidcError := r.URL.Query().Get("error") + errorMsg := r.URL.Query().Get("error") errorDescription := r.URL.Query().Get("error_description") errorURI := r.URL.Query().Get("error_uri") - if oidcError != "" { + if errorMsg != "" { // Combine the errors into a single string if either is provided. if errorDescription == "" && errorURI != "" { errorDescription = fmt.Sprintf("error_uri: %s", errorURI) } else if errorDescription != "" && errorURI != "" { errorDescription = fmt.Sprintf("%s, error_uri: %s", errorDescription, errorURI) } - oidcError = fmt.Sprintf("Encountered error in oidc process: %s", oidcError) + errorMsg = fmt.Sprintf("Encountered error in oidc process: %s", errorMsg) httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: oidcError, + Message: errorMsg, // This message might be blank. This is ok. Detail: errorDescription, }) diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go index 25c7861ce4ae4..4a1aa8b633bcd 100644 --- a/coderd/workspaceagents.go +++ b/coderd/workspaceagents.go @@ -1990,70 +1990,6 @@ func formatGitAuthAccessToken(typ codersdk.GitProvider, token string) agentsdk.G return resp } -func (api *API) gitAuthCallback(gitAuthConfig *gitauth.Config) http.HandlerFunc { - return func(rw http.ResponseWriter, r *http.Request) { - var ( - ctx = r.Context() - state = httpmw.OAuth2(r) - apiKey = httpmw.APIKey(r) - ) - - _, err := api.Database.GetGitAuthLink(ctx, database.GetGitAuthLinkParams{ - ProviderID: gitAuthConfig.ID, - UserID: apiKey.UserID, - }) - if err != nil { - if !errors.Is(err, sql.ErrNoRows) { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Failed to get git auth link.", - Detail: err.Error(), - }) - return - } - - _, err = api.Database.InsertGitAuthLink(ctx, database.InsertGitAuthLinkParams{ - ProviderID: gitAuthConfig.ID, - UserID: apiKey.UserID, - CreatedAt: database.Now(), - UpdatedAt: database.Now(), - OAuthAccessToken: state.Token.AccessToken, - OAuthRefreshToken: state.Token.RefreshToken, - OAuthExpiry: state.Token.Expiry, - }) - if err != nil { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Failed to insert git auth link.", - Detail: err.Error(), - }) - return - } - } else { - _, err = api.Database.UpdateGitAuthLink(ctx, database.UpdateGitAuthLinkParams{ - ProviderID: gitAuthConfig.ID, - UserID: apiKey.UserID, - UpdatedAt: database.Now(), - OAuthAccessToken: state.Token.AccessToken, - OAuthRefreshToken: state.Token.RefreshToken, - OAuthExpiry: state.Token.Expiry, - }) - if err != nil { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Failed to update git auth link.", - Detail: err.Error(), - }) - return - } - } - - redirect := state.Redirect - if redirect == "" { - // This is a nicely rendered screen on the frontend - redirect = "/gitauth" - } - http.Redirect(rw, r, redirect, http.StatusTemporaryRedirect) - } -} - // wsNetConn wraps net.Conn created by websocket.NetConn(). Cancel func // is called if a read or write error is encountered. type wsNetConn struct { diff --git a/coderd/workspaceagents_test.go b/coderd/workspaceagents_test.go index 7fcde117fd74a..1fb69015fda67 100644 --- a/coderd/workspaceagents_test.go +++ b/coderd/workspaceagents_test.go @@ -6,8 +6,6 @@ import ( "fmt" "net" "net/http" - "net/http/httptest" - "regexp" "runtime" "strconv" "strings" @@ -17,14 +15,12 @@ import ( "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "golang.org/x/oauth2" "cdr.dev/slog" "cdr.dev/slog/sloggers/slogtest" "github.com/coder/coder/agent" "github.com/coder/coder/coderd/coderdtest" "github.com/coder/coder/coderd/database" - "github.com/coder/coder/coderd/gitauth" "github.com/coder/coder/codersdk" "github.com/coder/coder/codersdk/agentsdk" "github.com/coder/coder/provisioner/echo" @@ -1000,270 +996,6 @@ func TestWorkspaceAgentAppHealth(t *testing.T) { require.EqualValues(t, codersdk.WorkspaceAppHealthUnhealthy, manifest.Apps[1].Health) } -// nolint:bodyclose -func TestWorkspaceAgentsGitAuth(t *testing.T) { - t.Parallel() - t.Run("NoMatchingConfig", func(t *testing.T) { - t.Parallel() - client := coderdtest.New(t, &coderdtest.Options{ - IncludeProvisionerDaemon: true, - GitAuthConfigs: []*gitauth.Config{}, - }) - user := coderdtest.CreateFirstUser(t, client) - authToken := uuid.NewString() - version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ - Parse: echo.ParseComplete, - ProvisionPlan: echo.ProvisionComplete, - ProvisionApply: echo.ProvisionApplyWithAgent(authToken), - }) - template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) - coderdtest.AwaitTemplateVersionJob(t, client, version.ID) - workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) - coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) - - agentClient := agentsdk.New(client.URL) - agentClient.SetSessionToken(authToken) - _, err := agentClient.GitAuth(context.Background(), "github.com", false) - var apiError *codersdk.Error - require.ErrorAs(t, err, &apiError) - require.Equal(t, http.StatusNotFound, apiError.StatusCode()) - }) - t.Run("ReturnsURL", func(t *testing.T) { - t.Parallel() - client := coderdtest.New(t, &coderdtest.Options{ - IncludeProvisionerDaemon: true, - GitAuthConfigs: []*gitauth.Config{{ - OAuth2Config: &testutil.OAuth2Config{}, - ID: "github", - Regex: regexp.MustCompile(`github\.com`), - Type: codersdk.GitProviderGitHub, - }}, - }) - user := coderdtest.CreateFirstUser(t, client) - authToken := uuid.NewString() - version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ - Parse: echo.ParseComplete, - ProvisionPlan: echo.ProvisionComplete, - ProvisionApply: []*proto.Provision_Response{{ - Type: &proto.Provision_Response_Complete{ - Complete: &proto.Provision_Complete{ - Resources: []*proto.Resource{{ - Name: "example", - Type: "aws_instance", - Agents: []*proto.Agent{{ - Id: uuid.NewString(), - Auth: &proto.Agent_Token{ - Token: authToken, - }, - }}, - }}, - }, - }, - }}, - }) - template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) - coderdtest.AwaitTemplateVersionJob(t, client, version.ID) - workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) - coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) - - agentClient := agentsdk.New(client.URL) - agentClient.SetSessionToken(authToken) - token, err := agentClient.GitAuth(context.Background(), "github.com/asd/asd", false) - require.NoError(t, err) - require.True(t, strings.HasSuffix(token.URL, fmt.Sprintf("/gitauth/%s", "github"))) - }) - t.Run("UnauthorizedCallback", func(t *testing.T) { - t.Parallel() - client := coderdtest.New(t, &coderdtest.Options{ - IncludeProvisionerDaemon: true, - GitAuthConfigs: []*gitauth.Config{{ - OAuth2Config: &testutil.OAuth2Config{}, - ID: "github", - Regex: regexp.MustCompile(`github\.com`), - Type: codersdk.GitProviderGitHub, - }}, - }) - resp := coderdtest.RequestGitAuthCallback(t, "github", client) - require.Equal(t, http.StatusSeeOther, resp.StatusCode) - }) - t.Run("AuthorizedCallback", func(t *testing.T) { - t.Parallel() - client := coderdtest.New(t, &coderdtest.Options{ - IncludeProvisionerDaemon: true, - GitAuthConfigs: []*gitauth.Config{{ - OAuth2Config: &testutil.OAuth2Config{}, - ID: "github", - Regex: regexp.MustCompile(`github\.com`), - Type: codersdk.GitProviderGitHub, - }}, - }) - _ = coderdtest.CreateFirstUser(t, client) - resp := coderdtest.RequestGitAuthCallback(t, "github", client) - require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) - location, err := resp.Location() - require.NoError(t, err) - require.Equal(t, "/gitauth", location.Path) - - // Callback again to simulate updating the token. - resp = coderdtest.RequestGitAuthCallback(t, "github", client) - require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) - }) - t.Run("ValidateURL", func(t *testing.T) { - t.Parallel() - ctx := testutil.Context(t, testutil.WaitLong) - - srv := httptest.NewServer(nil) - defer srv.Close() - client := coderdtest.New(t, &coderdtest.Options{ - IncludeProvisionerDaemon: true, - GitAuthConfigs: []*gitauth.Config{{ - ValidateURL: srv.URL, - OAuth2Config: &testutil.OAuth2Config{}, - ID: "github", - Regex: regexp.MustCompile(`github\.com`), - Type: codersdk.GitProviderGitHub, - }}, - }) - user := coderdtest.CreateFirstUser(t, client) - authToken := uuid.NewString() - version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ - Parse: echo.ParseComplete, - ProvisionPlan: echo.ProvisionComplete, - ProvisionApply: echo.ProvisionApplyWithAgent(authToken), - }) - template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) - coderdtest.AwaitTemplateVersionJob(t, client, version.ID) - workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) - coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) - - agentClient := agentsdk.New(client.URL) - agentClient.SetSessionToken(authToken) - - resp := coderdtest.RequestGitAuthCallback(t, "github", client) - require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) - - // If the validation URL says unauthorized, the callback - // URL to re-authenticate should be returned. - srv.Config.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusUnauthorized) - }) - res, err := agentClient.GitAuth(ctx, "github.com/asd/asd", false) - require.NoError(t, err) - require.NotEmpty(t, res.URL) - - // If the validation URL gives a non-OK status code, this - // should be treated as an internal server error. - srv.Config.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusForbidden) - w.Write([]byte("Something went wrong!")) - }) - _, err = agentClient.GitAuth(ctx, "github.com/asd/asd", false) - var apiError *codersdk.Error - require.ErrorAs(t, err, &apiError) - require.Equal(t, http.StatusInternalServerError, apiError.StatusCode()) - require.Equal(t, "validate git auth token: status 403: body: Something went wrong!", apiError.Detail) - }) - - t.Run("ExpiredNoRefresh", func(t *testing.T) { - t.Parallel() - client := coderdtest.New(t, &coderdtest.Options{ - IncludeProvisionerDaemon: true, - GitAuthConfigs: []*gitauth.Config{{ - OAuth2Config: &testutil.OAuth2Config{ - Token: &oauth2.Token{ - AccessToken: "token", - RefreshToken: "something", - Expiry: database.Now().Add(-time.Hour), - }, - }, - ID: "github", - Regex: regexp.MustCompile(`github\.com`), - Type: codersdk.GitProviderGitHub, - NoRefresh: true, - }}, - }) - user := coderdtest.CreateFirstUser(t, client) - authToken := uuid.NewString() - version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ - Parse: echo.ParseComplete, - ProvisionPlan: echo.ProvisionComplete, - ProvisionApply: echo.ProvisionApplyWithAgent(authToken), - }) - template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) - coderdtest.AwaitTemplateVersionJob(t, client, version.ID) - workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) - coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) - - agentClient := agentsdk.New(client.URL) - agentClient.SetSessionToken(authToken) - - token, err := agentClient.GitAuth(context.Background(), "github.com/asd/asd", false) - require.NoError(t, err) - require.NotEmpty(t, token.URL) - - // In the configuration, we set our OAuth provider - // to return an expired token. Coder consumes this - // and stores it. - resp := coderdtest.RequestGitAuthCallback(t, "github", client) - require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) - - // Because the token is expired and `NoRefresh` is specified, - // a redirect URL should be returned again. - token, err = agentClient.GitAuth(context.Background(), "github.com/asd/asd", false) - require.NoError(t, err) - require.NotEmpty(t, token.URL) - }) - - t.Run("FullFlow", func(t *testing.T) { - t.Parallel() - client := coderdtest.New(t, &coderdtest.Options{ - IncludeProvisionerDaemon: true, - GitAuthConfigs: []*gitauth.Config{{ - OAuth2Config: &testutil.OAuth2Config{}, - ID: "github", - Regex: regexp.MustCompile(`github\.com`), - Type: codersdk.GitProviderGitHub, - }}, - }) - user := coderdtest.CreateFirstUser(t, client) - authToken := uuid.NewString() - version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ - Parse: echo.ParseComplete, - ProvisionPlan: echo.ProvisionComplete, - ProvisionApply: echo.ProvisionApplyWithAgent(authToken), - }) - template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) - coderdtest.AwaitTemplateVersionJob(t, client, version.ID) - workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) - coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) - - agentClient := agentsdk.New(client.URL) - agentClient.SetSessionToken(authToken) - - token, err := agentClient.GitAuth(context.Background(), "github.com/asd/asd", false) - require.NoError(t, err) - require.NotEmpty(t, token.URL) - - // Start waiting for the token callback... - tokenChan := make(chan agentsdk.GitAuthResponse, 1) - go func() { - token, err := agentClient.GitAuth(context.Background(), "github.com/asd/asd", true) - assert.NoError(t, err) - tokenChan <- token - }() - - time.Sleep(250 * time.Millisecond) - - resp := coderdtest.RequestGitAuthCallback(t, "github", client) - require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) - token = <-tokenChan - require.Equal(t, "access_token", token.Username) - - token, err = agentClient.GitAuth(context.Background(), "github.com/asd/asd", false) - require.NoError(t, err) - }) -} - func TestWorkspaceAgentReportStats(t *testing.T) { t.Parallel() diff --git a/codersdk/authorization.go b/codersdk/authorization.go index 4e8a6eed7019f..fbb62aa5f8698 100644 --- a/codersdk/authorization.go +++ b/codersdk/authorization.go @@ -3,6 +3,7 @@ package codersdk import ( "context" "encoding/json" + "fmt" "net/http" ) @@ -20,6 +21,10 @@ type AuthorizationRequest struct { Checks map[string]AuthorizationCheck `json:"checks"` } +type ExchangeGitAuthRequest struct { + DeviceCode string `json:"device_code"` +} + // AuthorizationCheck is used to check if the currently authenticated user (or the specified user) can do a given action to a given set of objects. // // @Description AuthorizationCheck is used to check if the currently authenticated user (or the specified user) can do a given action to a given set of objects. @@ -70,3 +75,16 @@ func (c *Client) AuthCheck(ctx context.Context, req AuthorizationRequest) (Autho var resp AuthorizationResponse return resp, json.NewDecoder(res.Body).Decode(&resp) } + +// ExchangeGitAuth exchanges a device code for a git auth token. +func (c *Client) ExchangeGitAuth(ctx context.Context, provider string, req ExchangeGitAuthRequest) error { + res, err := c.Request(ctx, http.MethodPost, fmt.Sprintf("/api/v2/gitauth/%s/exchange", provider), req) + if err != nil { + return err + } + defer res.Body.Close() + if res.StatusCode != http.StatusNoContent { + return ReadBodyAsError(res) + } + return nil +} diff --git a/codersdk/deployment.go b/codersdk/deployment.go index dc758e5a76242..5c73f267a7a88 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -297,16 +297,18 @@ type TraceConfig struct { } type GitAuthConfig struct { - ID string `json:"id"` - Type string `json:"type"` - ClientID string `json:"client_id"` - ClientSecret string `json:"-" yaml:"client_secret"` - AuthURL string `json:"auth_url"` - TokenURL string `json:"token_url"` - ValidateURL string `json:"validate_url"` - Regex string `json:"regex"` - NoRefresh bool `json:"no_refresh"` - Scopes []string `json:"scopes"` + ID string `json:"id"` + Type string `json:"type"` + ClientID string `json:"client_id"` + ClientSecret string `json:"-" yaml:"client_secret"` + AuthURL string `json:"auth_url"` + TokenURL string `json:"token_url"` + ValidateURL string `json:"validate_url"` + Regex string `json:"regex"` + NoRefresh bool `json:"no_refresh"` + Scopes []string `json:"scopes"` + DeviceFlow bool `json:"device_flow"` + DeviceAuthURL string `json:"device_auth_url"` } type ProvisionerConfig struct { diff --git a/docs/api/general.md b/docs/api/general.md index 1655fb9d2fb00..9c57b5f42f33b 100644 --- a/docs/api/general.md +++ b/docs/api/general.md @@ -202,6 +202,8 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \ { "auth_url": "string", "client_id": "string", + "device_auth_url": "string", + "device_flow": true, "id": "string", "no_refresh": true, "regex": "string", diff --git a/docs/api/schemas.md b/docs/api/schemas.md index f332d03968fb1..a084cadd16e90 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -595,6 +595,8 @@ { "auth_url": "string", "client_id": "string", + "device_auth_url": "string", + "device_flow": true, "id": "string", "no_refresh": true, "regex": "string", @@ -1879,6 +1881,8 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in { "auth_url": "string", "client_id": "string", + "device_auth_url": "string", + "device_flow": true, "id": "string", "no_refresh": true, "regex": "string", @@ -2210,6 +2214,8 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in { "auth_url": "string", "client_id": "string", + "device_auth_url": "string", + "device_flow": true, "id": "string", "no_refresh": true, "regex": "string", @@ -2577,6 +2583,8 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in { "auth_url": "string", "client_id": "string", + "device_auth_url": "string", + "device_flow": true, "id": "string", "no_refresh": true, "regex": "string", @@ -2589,17 +2597,19 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in ### Properties -| Name | Type | Required | Restrictions | Description | -| -------------- | --------------- | -------- | ------------ | ----------- | -| `auth_url` | string | false | | | -| `client_id` | string | false | | | -| `id` | string | false | | | -| `no_refresh` | boolean | false | | | -| `regex` | string | false | | | -| `scopes` | array of string | false | | | -| `token_url` | string | false | | | -| `type` | string | false | | | -| `validate_url` | string | false | | | +| Name | Type | Required | Restrictions | Description | +| ----------------- | --------------- | -------- | ------------ | ----------- | +| `auth_url` | string | false | | | +| `client_id` | string | false | | | +| `device_auth_url` | string | false | | | +| `device_flow` | boolean | false | | | +| `id` | string | false | | | +| `no_refresh` | boolean | false | | | +| `regex` | string | false | | | +| `scopes` | array of string | false | | | +| `token_url` | string | false | | | +| `type` | string | false | | | +| `validate_url` | string | false | | | ## codersdk.GitProvider diff --git a/site/src/AppRouter.tsx b/site/src/AppRouter.tsx index 1a8fa700ae77c..621d2a663457b 100644 --- a/site/src/AppRouter.tsx +++ b/site/src/AppRouter.tsx @@ -116,6 +116,9 @@ const NetworkSettingsPage = lazy( ), ) const GitAuthPage = lazy(() => import("./pages/GitAuthPage/GitAuthPage")) +const GitAuthDevicePage = lazy( + () => import("./pages/GitAuthDevicePage/GitAuthDevicePage"), +) const TemplateVersionPage = lazy( () => import("./pages/TemplateVersionPage/TemplateVersionPage"), ) @@ -193,7 +196,12 @@ export const AppRouter: FC = () => { }> } /> - } /> + + } /> + + } /> + + } /> diff --git a/site/src/api/api.ts b/site/src/api/api.ts index df8477b65e926..4e6be6db050fc 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -799,6 +799,14 @@ export const getExperiments = async (): Promise => { } } +export const exchangeGitAuth = async ( + provider: string, + req: TypesGen.ExchangeGitAuthRequest, +): Promise => { + const resp = await axios.post(`/gitauth/${provider}/exchange`, req) + return resp.data +} + export const getAuditLogs = async ( options: TypesGen.AuditLogsRequest, ): Promise => { diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 49bdcdbb326ff..96b3b026e8d5b 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -392,6 +392,11 @@ export interface Entitlements { readonly require_telemetry: boolean } +// From codersdk/authorization.go +export interface ExchangeGitAuthRequest { + readonly device_code: string +} + // From codersdk/deployment.go export type Experiments = Experiment[] @@ -425,6 +430,8 @@ export interface GitAuthConfig { readonly regex: string readonly no_refresh: boolean readonly scopes: string[] + readonly device_flow: boolean + readonly device_auth_url: string } // From codersdk/gitsshkey.go diff --git a/site/src/components/SignInLayout/SignInLayout.tsx b/site/src/components/SignInLayout/SignInLayout.tsx index 5c9a4eb13f772..402010451cfb7 100644 --- a/site/src/components/SignInLayout/SignInLayout.tsx +++ b/site/src/components/SignInLayout/SignInLayout.tsx @@ -16,6 +16,9 @@ export const useStyles = makeStyles((theme) => ({ container: { marginTop: theme.spacing(-8), maxWidth: 385, + display: "flex", + flexDirection: "column", + alignItems: "center", }, footer: { fontSize: 12, diff --git a/site/src/pages/GitAuthDevicePage/GitAuthDevicePage.tsx b/site/src/pages/GitAuthDevicePage/GitAuthDevicePage.tsx new file mode 100644 index 0000000000000..c861dc8bd7fa8 --- /dev/null +++ b/site/src/pages/GitAuthDevicePage/GitAuthDevicePage.tsx @@ -0,0 +1,158 @@ +import OpenInNewIcon from "@mui/icons-material/OpenInNew" +import CircularProgress from "@mui/material/CircularProgress" +import Link from "@mui/material/Link" +import { makeStyles } from "@mui/styles" +import { useQuery } from "@tanstack/react-query" +import { exchangeGitAuth } from "api/api" +import { ApiErrorResponse } from "api/errors" +import { isAxiosError } from "axios" +import { Alert } from "components/Alert/Alert" +import { CopyButton } from "components/CopyButton/CopyButton" +import { SignInLayout } from "components/SignInLayout/SignInLayout" +import { Welcome } from "components/Welcome/Welcome" +import { FC, useEffect, useMemo, useState } from "react" +import { useNavigate, useParams, useSearchParams } from "react-router-dom" + +const GitAuthDevicePage: FC = () => { + const styles = useStyles() + const { provider } = useParams() + const [searchParams, setSearchParams] = useSearchParams() + const navigate = useNavigate() + const [deviceCode] = useState(() => searchParams.get("device_code") || "") + const [userCode] = useState(() => searchParams.get("user_code") || "") + const [verificationUri] = useState( + () => searchParams.get("verification_uri") || "", + ) + const [interval] = useState( + () => Number.parseInt(searchParams.get("interval") || "") || 5, + ) + + useEffect(() => { + // This will clear the query parameters. It's a nice UX, because + // then a user can reload the page to obtain a new device code in + // case the current one expires! + if (deviceCode) { + setSearchParams({}) + } + }, [deviceCode, setSearchParams, navigate]) + + const exchange = useQuery({ + queryFn: () => + exchangeGitAuth(provider as string, { device_code: deviceCode }), + queryKey: ["gitauth", provider as string, deviceCode], + retry: true, + retryDelay: interval * 1000, + refetchOnWindowFocus: (query) => + query.state.status === "success" ? false : "always", + }) + + useEffect(() => { + if (!exchange.isSuccess) { + return + } + // Redirect to `/gitauth` to notify any listeners that the + // authentication was successful. + navigate("/gitauth") + }, [exchange.isSuccess, navigate]) + + let status = ( +

+ + Checking for authentication... +

+ ) + if (isAxiosError(exchange.failureReason)) { + // See https://datatracker.ietf.org/doc/html/rfc8628#section-3.5 + switch (exchange.failureReason.response?.data?.detail) { + case "authorization_pending": + break + case "expired_token": + status = ( + + The one-time code has expired. Refresh to get a new one! + + ) + break + case "access_denied": + status = ( + Access to the Git provider was denied. + ) + break + } + } + + return ( + + + +

+ Copy your one-time code:  +

+ {userCode} {" "} + +
+
+ Then open the link below and paste it: +

+ + + Open and Paste + + {status} + + ) +} + +export default GitAuthDevicePage + +const useStyles = makeStyles((theme) => ({ + title: { + fontSize: theme.spacing(4), + fontWeight: 400, + lineHeight: "140%", + margin: 0, + }, + + text: { + fontSize: 16, + color: theme.palette.text.secondary, + marginBottom: 0, + textAlign: "center", + lineHeight: "160%", + }, + + copyCode: { + display: "inline-flex", + alignItems: "center", + }, + + code: { + fontWeight: "bold", + color: theme.palette.text.primary, + }, + + lineBreak: { + whiteSpace: "nowrap", + }, + + link: { + display: "flex", + alignItems: "center", + justifyContent: "center", + fontSize: 16, + gap: theme.spacing(1), + margin: theme.spacing(2, 0), + }, + + status: { + display: "flex", + alignItems: "center", + gap: theme.spacing(1), + color: theme.palette.text.disabled, + }, +})) diff --git a/site/vite.config.ts b/site/vite.config.ts index 6e15f7e0f9531..d8ace5c9c5387 100644 --- a/site/vite.config.ts +++ b/site/vite.config.ts @@ -62,6 +62,12 @@ export default defineConfig({ }) }, }, + // The device route is visual! + "^/gitauth/(?!.*(device))": { + changeOrigin: true, + target: process.env.CODER_HOST || "http://localhost:3000", + secure: process.env.NODE_ENV === "production", + }, "/swagger": { target: process.env.CODER_HOST || "http://localhost:3000", secure: process.env.NODE_ENV === "production", From 76541912e8a44206023182221092ebc28472676d Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Tue, 27 Jun 2023 16:04:21 +0000 Subject: [PATCH 02/17] Improve askpass view --- cli/gitaskpass.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/gitaskpass.go b/cli/gitaskpass.go index 14822b9616293..5bb67adf82416 100644 --- a/cli/gitaskpass.go +++ b/cli/gitaskpass.go @@ -51,9 +51,9 @@ func (r *RootCmd) gitAskpass() *clibase.Cmd { } if token.URL != "" { if err := openURL(inv, token.URL); err == nil { - cliui.Infof(inv.Stderr, "Your browser has been opened to authenticate with Git:\n\n\t%s\n\n", token.URL) + cliui.Infof(inv.Stderr, "Your browser has been opened to authenticate with Git:\n\n%s\n", token.URL) } else { - cliui.Infof(inv.Stderr, "Open the following URL to authenticate with Git:\n\n\t%s\n\n", token.URL) + cliui.Infof(inv.Stderr, "Open the following URL to authenticate with Git:\n\n%s\n", token.URL) } for r := retry.New(250*time.Millisecond, 10*time.Second); r.Wait(ctx); { From 16ce6e1c7a71467114c17c5b662203964ea18995 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Tue, 27 Jun 2023 18:10:16 +0000 Subject: [PATCH 03/17] Add routes to improve clarity of git auth --- coderd/coderd.go | 31 +++-- coderd/gitauth.go | 201 +++++++++++++++++------------ coderd/gitauth/config.go | 156 +++++++++++++++++----- coderd/gitauth/oauth.go | 58 ++++----- coderd/gitauth_test.go | 188 ++++++++++++++++++++++++++- coderd/httpmw/gitauthparam.go | 40 ++++++ coderd/httpmw/gitauthparam_test.go | 49 +++++++ coderd/workspaceagents.go | 22 ++-- codersdk/authorization.go | 18 --- codersdk/deployment.go | 26 ++-- codersdk/gitauth.go | 85 ++++++++++++ site/vite.config.ts | 6 - 12 files changed, 671 insertions(+), 209 deletions(-) create mode 100644 coderd/httpmw/gitauthparam.go create mode 100644 coderd/httpmw/gitauthparam_test.go create mode 100644 codersdk/gitauth.go diff --git a/coderd/coderd.go b/coderd/coderd.go index 87301524013f5..09911d1f88c44 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -455,23 +455,23 @@ func New(options *Options) *API { }) }) + // Register callback handlers for each OAuth2 provider. r.Route("/gitauth", func(r chi.Router) { for _, gitAuthConfig := range options.GitAuthConfigs { + // We don't need to register a callback handler for device auth. + if gitAuthConfig.DeviceAuth != nil { + continue + } r.Route(fmt.Sprintf("/%s", gitAuthConfig.ID), func(r chi.Router) { - r.Use(apiKeyMiddlewareRedirect) - - useDeviceAuth := gitAuthConfig.DeviceAuth != nil - if useDeviceAuth { - r.Use(api.gitAuthDeviceRedirect(gitAuthConfig)) - r.Post("/exchange", api.postGitAuthExchange(gitAuthConfig)) - } else { - // If device auth isn't in use, then the git provider is using OAuth2! - r.Use(httpmw.ExtractOAuth2(gitAuthConfig, options.HTTPClient, nil)) - r.Get("/callback", api.gitAuthCallback(gitAuthConfig)) - } + r.Use( + apiKeyMiddlewareRedirect, + httpmw.ExtractOAuth2(gitAuthConfig, options.HTTPClient, nil), + ) + r.Get("/callback", api.gitAuthCallback(gitAuthConfig)) }) } }) + r.Route("/api/v2", func(r chi.Router) { api.APIHandler = r @@ -519,6 +519,15 @@ func New(options *Options) *API { r.Get("/{fileID}", api.fileByID) r.Post("/", api.postFile) }) + r.Route("/gitauth/{gitauth}", func(r chi.Router) { + r.Use( + apiKeyMiddleware, + httpmw.ExtractGitAuthParam(options.GitAuthConfigs), + ) + r.Get("/", api.gitAuthByID) + r.Post("/device", api.postGitAuthDeviceByID) + r.Get("/device", api.gitAuthDeviceByID) + }) r.Route("/organizations", func(r chi.Router) { r.Use( apiKeyMiddleware, diff --git a/coderd/gitauth.go b/coderd/gitauth.go index d65c838733de0..4a5bd87ae6eaa 100644 --- a/coderd/gitauth.go +++ b/coderd/gitauth.go @@ -5,7 +5,8 @@ import ( "errors" "fmt" "net/http" - "net/url" + + "golang.org/x/sync/errgroup" "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/gitauth" @@ -14,113 +15,143 @@ import ( "github.com/coder/coder/codersdk" ) -func (*API) gitAuthDeviceRedirect(gitAuthConfig *gitauth.Config) func(http.Handler) http.Handler { - route := fmt.Sprintf("/gitauth/%s/device", gitAuthConfig.ID) - return func(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - deviceCode := r.URL.Query().Get("device_code") - if r.Method != http.MethodGet || deviceCode != "" { - next.ServeHTTP(w, r) - return - } - // If no device code is provided, redirect to the dashboard with query params! - deviceAuth, err := gitAuthConfig.DeviceAuth.AuthorizeDevice(r.Context()) - if err != nil { - httpapi.Write(r.Context(), w, http.StatusInternalServerError, codersdk.Response{ - Message: "Failed to authorize device.", - Detail: err.Error(), - }) - return - } - v := url.Values{ - "device_code": {deviceAuth.DeviceCode}, - "user_code": {deviceAuth.UserCode}, - "expires_in": {fmt.Sprintf("%d", deviceAuth.ExpiresIn)}, - "interval": {fmt.Sprintf("%d", deviceAuth.Interval)}, - "verification_uri": {deviceAuth.VerificationURI}, - } - http.Redirect(w, r, fmt.Sprintf("%s?%s", route, v.Encode()), http.StatusTemporaryRedirect) +// gitAuthByID returns the git auth status for the given git auth config ID. +func (api *API) gitAuthByID(w http.ResponseWriter, r *http.Request) { + config := httpmw.GitAuthParam(r) + apiKey := httpmw.APIKey(r) + ctx := r.Context() + + res := codersdk.GitAuth{ + Authenticated: false, + Device: config.DeviceAuth != nil, + } + + link, err := api.Database.GetGitAuthLink(ctx, database.GetGitAuthLinkParams{ + ProviderID: config.ID, + UserID: apiKey.UserID, + }) + if err == nil { + var eg errgroup.Group + eg.Go(func() (err error) { + res.Authenticated, res.User, err = config.ValidateToken(ctx, link.OAuthAccessToken) + return err + }) + eg.Go(func() (err error) { + res.AppInstallations, err = config.AppInstallations(ctx, link.OAuthAccessToken) + return err }) + err = eg.Wait() + if err != nil { + httpapi.Write(ctx, w, http.StatusInternalServerError, codersdk.Response{ + Message: "Failed to validate token.", + Detail: err.Error(), + }) + return + } } + + httpapi.Write(ctx, w, http.StatusOK, res) } -func (api *API) postGitAuthExchange(gitAuthConfig *gitauth.Config) http.HandlerFunc { - return func(rw http.ResponseWriter, r *http.Request) { - ctx := r.Context() - apiKey := httpmw.APIKey(r) +func (api *API) postGitAuthDeviceByID(rw http.ResponseWriter, r *http.Request) { + ctx := r.Context() + apiKey := httpmw.APIKey(r) + config := httpmw.GitAuthParam(r) - var req codersdk.ExchangeGitAuthRequest - if !httpapi.Read(ctx, rw, r, &req) { - return - } + var req codersdk.GitAuthDeviceExchange + if !httpapi.Read(ctx, rw, r, &req) { + return + } + + if config.DeviceAuth == nil { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Git auth provider does not support device flow.", + }) + return + } - if gitAuthConfig.DeviceAuth == nil { + token, err := config.DeviceAuth.ExchangeDeviceCode(ctx, req.DeviceCode) + if err != nil { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Failed to exchange device code.", + Detail: err.Error(), + }) + return + } + + _, err = api.Database.GetGitAuthLink(ctx, database.GetGitAuthLinkParams{ + ProviderID: config.ID, + UserID: apiKey.UserID, + }) + if err != nil { + if !errors.Is(err, sql.ErrNoRows) { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Git auth provider does not support device flow.", + Message: "Failed to get git auth link.", + Detail: err.Error(), }) return } - token, err := gitAuthConfig.DeviceAuth.ExchangeDeviceCode(ctx, req.DeviceCode) + _, err = api.Database.InsertGitAuthLink(ctx, database.InsertGitAuthLinkParams{ + ProviderID: config.ID, + UserID: apiKey.UserID, + CreatedAt: database.Now(), + UpdatedAt: database.Now(), + OAuthAccessToken: token.AccessToken, + OAuthRefreshToken: token.RefreshToken, + OAuthExpiry: token.Expiry, + }) if err != nil { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Failed to exchange device code.", + Message: "Failed to insert git auth link.", Detail: err.Error(), }) return } - - _, err = api.Database.GetGitAuthLink(ctx, database.GetGitAuthLinkParams{ - ProviderID: gitAuthConfig.ID, - UserID: apiKey.UserID, + } else { + _, err = api.Database.UpdateGitAuthLink(ctx, database.UpdateGitAuthLinkParams{ + ProviderID: config.ID, + UserID: apiKey.UserID, + UpdatedAt: database.Now(), + OAuthAccessToken: token.AccessToken, + OAuthRefreshToken: token.RefreshToken, + OAuthExpiry: token.Expiry, }) if err != nil { - if !errors.Is(err, sql.ErrNoRows) { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Failed to get git auth link.", - Detail: err.Error(), - }) - return - } - - _, err = api.Database.InsertGitAuthLink(ctx, database.InsertGitAuthLinkParams{ - ProviderID: gitAuthConfig.ID, - UserID: apiKey.UserID, - CreatedAt: database.Now(), - UpdatedAt: database.Now(), - OAuthAccessToken: token.AccessToken, - OAuthRefreshToken: token.RefreshToken, - OAuthExpiry: token.Expiry, - }) - if err != nil { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Failed to insert git auth link.", - Detail: err.Error(), - }) - return - } - } else { - _, err = api.Database.UpdateGitAuthLink(ctx, database.UpdateGitAuthLinkParams{ - ProviderID: gitAuthConfig.ID, - UserID: apiKey.UserID, - UpdatedAt: database.Now(), - OAuthAccessToken: token.AccessToken, - OAuthRefreshToken: token.RefreshToken, - OAuthExpiry: token.Expiry, + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Failed to update git auth link.", + Detail: err.Error(), }) - if err != nil { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Failed to update git auth link.", - Detail: err.Error(), - }) - return - } + return } - httpapi.Write(ctx, rw, http.StatusNoContent, nil) } + httpapi.Write(ctx, rw, http.StatusNoContent, nil) +} + +// gitAuthDeviceByID issues a new device auth code for the given git auth config ID. +func (*API) gitAuthDeviceByID(rw http.ResponseWriter, r *http.Request) { + config := httpmw.GitAuthParam(r) + ctx := r.Context() + + if config.DeviceAuth == nil { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Git auth device flow not supported.", + }) + return + } + + deviceAuth, err := config.DeviceAuth.AuthorizeDevice(r.Context()) + if err != nil { + httpapi.Write(r.Context(), rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Failed to authorize device.", + Detail: err.Error(), + }) + return + } + + httpapi.Write(ctx, rw, http.StatusOK, deviceAuth) } -// device get func (api *API) gitAuthCallback(gitAuthConfig *gitauth.Config) http.HandlerFunc { return func(rw http.ResponseWriter, r *http.Request) { var ( @@ -179,7 +210,7 @@ func (api *API) gitAuthCallback(gitAuthConfig *gitauth.Config) http.HandlerFunc redirect := state.Redirect if redirect == "" { // This is a nicely rendered screen on the frontend - redirect = "/gitauth" + redirect = fmt.Sprintf("/gitauth/%s", gitAuthConfig.ID) } http.Redirect(rw, r, redirect, http.StatusTemporaryRedirect) } diff --git a/coderd/gitauth/config.go b/coderd/gitauth/config.go index bdb9c961eb69f..21ed28f3ee3bd 100644 --- a/coderd/gitauth/config.go +++ b/coderd/gitauth/config.go @@ -2,6 +2,7 @@ package gitauth import ( "context" + "encoding/json" "fmt" "io" "net/http" @@ -11,15 +12,22 @@ import ( "golang.org/x/oauth2" "golang.org/x/xerrors" + "github.com/google/go-github/v43/github" + "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/httpapi" - "github.com/coder/coder/coderd/httpmw" "github.com/coder/coder/codersdk" ) +type OAuth2Config interface { + AuthCodeURL(state string, opts ...oauth2.AuthCodeOption) string + Exchange(ctx context.Context, code string, opts ...oauth2.AuthCodeOption) (*oauth2.Token, error) + TokenSource(context.Context, *oauth2.Token) oauth2.TokenSource +} + // Config is used for authentication for Git operations. type Config struct { - httpmw.OAuth2Config + OAuth2Config // ID is a unique identifier for the authenticator. ID string // Regex is a regexp that URLs will match against. @@ -36,6 +44,14 @@ type Config struct { // returning it to the user. If omitted, tokens will // not be validated before being returned. ValidateURL string + // InstallURL is for GitHub App's (and hopefully others eventually) + // to provide a link to install the app. There's installation + // of the application, and user authentication. It's possible + // for the user to authenticate but the application to not. + AppInstallURL string + // InstallationsURL is an API endpoint that returns a list of + // installations for the user. This is used for GitHub Apps. + AppInstallationsURL string // DeviceAuth is set if the provider uses the device flow. DeviceAuth *DeviceAuth } @@ -60,15 +76,13 @@ func (c *Config) RefreshToken(ctx context.Context, db database.Store, gitAuthLin return gitAuthLink, false, nil } - if c.ValidateURL != "" { - valid, err := c.ValidateToken(ctx, token.AccessToken) - if err != nil { - return gitAuthLink, false, xerrors.Errorf("validate git auth token: %w", err) - } - if !valid { - // The token is no longer valid! - return gitAuthLink, false, nil - } + valid, _, err := c.ValidateToken(ctx, token.AccessToken) + if err != nil { + return gitAuthLink, false, xerrors.Errorf("validate git auth token: %w", err) + } + if !valid { + // The token is no longer valid! + return gitAuthLink, false, nil } if token.AccessToken != gitAuthLink.OAuthAccessToken { @@ -89,26 +103,98 @@ func (c *Config) RefreshToken(ctx context.Context, db database.Store, gitAuthLin } // ValidateToken ensures the Git token provided is valid! -func (c *Config) ValidateToken(ctx context.Context, token string) (bool, error) { +// The user is optionally returned if the provider supports it. +func (c *Config) ValidateToken(ctx context.Context, token string) (bool, *codersdk.GitAuthUser, error) { + if c.ValidateURL == "" { + // Default that the token is valid if no validation URL is provided. + return true, nil, nil + } req, err := http.NewRequestWithContext(ctx, http.MethodGet, c.ValidateURL, nil) if err != nil { - return false, err + return false, nil, err } req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token)) res, err := http.DefaultClient.Do(req) if err != nil { - return false, err + return false, nil, err } defer res.Body.Close() if res.StatusCode == http.StatusUnauthorized { // The token is no longer valid! - return false, nil + return false, nil, nil } if res.StatusCode != http.StatusOK { data, _ := io.ReadAll(res.Body) - return false, xerrors.Errorf("status %d: body: %s", res.StatusCode, data) + return false, nil, xerrors.Errorf("status %d: body: %s", res.StatusCode, data) + } + + var user *codersdk.GitAuthUser + if c.Type == codersdk.GitProviderGitHub { + var ghUser github.User + err = json.NewDecoder(res.Body).Decode(&ghUser) + if err == nil { + user = &codersdk.GitAuthUser{ + Login: ghUser.GetLogin(), + AvatarURL: ghUser.GetAvatarURL(), + ProfileURL: ghUser.GetHTMLURL(), + Name: ghUser.GetName(), + } + } + } + + return true, user, nil +} + +type AppInstallation struct { + ID int + // Login is the username of the installation. + Login string + // URL is a link to configure the app install. + URL string +} + +// AppInstallations returns a list of app installations for the given token. +// If the provider does not support app installations, it returns nil. +func (c *Config) AppInstallations(ctx context.Context, token string) ([]codersdk.GitAuthAppInstallation, error) { + if c.AppInstallationsURL == "" { + return nil, nil + } + req, err := http.NewRequestWithContext(ctx, http.MethodGet, c.AppInstallationsURL, nil) + if err != nil { + return nil, err + } + req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token)) + res, err := http.DefaultClient.Do(req) + if err != nil { + return nil, err } - return true, nil + defer res.Body.Close() + + installs := []codersdk.GitAuthAppInstallation{} + if c.Type == codersdk.GitProviderGitHub { + var ghInstalls []github.Installation + err = json.NewDecoder(res.Body).Decode(&ghInstalls) + if err != nil { + return nil, err + } + for _, installation := range ghInstalls { + install := codersdk.GitAuthAppInstallation{ + ID: int(installation.GetID()), + ConfigureURL: installation.GetHTMLURL(), + } + account := installation.GetAccount() + if account != nil { + install.Account = &codersdk.GitAuthUser{ + Login: account.GetLogin(), + AvatarURL: account.GetAvatarURL(), + ProfileURL: account.GetHTMLURL(), + Name: account.GetName(), + } + } + installs = append(installs, install) + } + } + return installs, nil } // ConvertConfig converts the SDK configuration entry format @@ -165,7 +251,7 @@ func ConvertConfig(entries []codersdk.GitAuthConfig, accessURL *url.URL) ([]*Con } } - oauth2Config := &oauth2.Config{ + oc := &oauth2.Config{ ClientID: entry.ClientID, ClientSecret: entry.ClientSecret, Endpoint: endpoint[typ], @@ -174,31 +260,36 @@ func ConvertConfig(entries []codersdk.GitAuthConfig, accessURL *url.URL) ([]*Con } if entry.AuthURL != "" { - oauth2Config.Endpoint.AuthURL = entry.AuthURL + oc.Endpoint.AuthURL = entry.AuthURL } if entry.TokenURL != "" { - oauth2Config.Endpoint.TokenURL = entry.TokenURL + oc.Endpoint.TokenURL = entry.TokenURL } if entry.Scopes != nil && len(entry.Scopes) > 0 { - oauth2Config.Scopes = entry.Scopes + oc.Scopes = entry.Scopes } if entry.ValidateURL == "" { entry.ValidateURL = validateURL[typ] } + if entry.AppInstallationsURL == "" { + entry.AppInstallationsURL = appInstallationsURL[typ] + } - var oauthConfig httpmw.OAuth2Config = oauth2Config + var oauthConfig OAuth2Config = oc // Azure DevOps uses JWT token authentication! if typ == codersdk.GitProviderAzureDevops { - oauthConfig = &jwtConfig{oauth2Config} + oauthConfig = &jwtConfig{oc} } cfg := &Config{ - OAuth2Config: oauthConfig, - ID: entry.ID, - Regex: regex, - Type: typ, - NoRefresh: entry.NoRefresh, - ValidateURL: entry.ValidateURL, + OAuth2Config: oauthConfig, + ID: entry.ID, + Regex: regex, + Type: typ, + NoRefresh: entry.NoRefresh, + ValidateURL: entry.ValidateURL, + AppInstallationsURL: entry.AppInstallationsURL, + AppInstallURL: entry.AppInstallURL, } if entry.DeviceFlow { @@ -209,9 +300,10 @@ func ConvertConfig(entries []codersdk.GitAuthConfig, accessURL *url.URL) ([]*Con return nil, xerrors.Errorf("git auth provider %q: device auth url must be provided", entry.ID) } cfg.DeviceAuth = &DeviceAuth{ - config: oauth2Config, - URL: entry.DeviceAuthURL, - ID: entry.ID, + ClientID: entry.ClientID, + TokenURL: entry.TokenURL, + Scopes: entry.Scopes, + CodeURL: entry.DeviceAuthURL, } } diff --git a/coderd/gitauth/oauth.go b/coderd/gitauth/oauth.go index 5ad611589017d..6374556fe97e1 100644 --- a/coderd/gitauth/oauth.go +++ b/coderd/gitauth/oauth.go @@ -44,6 +44,10 @@ var deviceAuthURL = map[codersdk.GitProvider]string{ codersdk.GitProviderGitHub: "https://github.com/login/device/code", } +var appInstallationsURL = map[codersdk.GitProvider]string{ + codersdk.GitProviderGitHub: "https://api.github.com/user/installations", +} + // scope contains defaults for each Git provider. var scope = map[codersdk.GitProvider][]string{ codersdk.GitProviderAzureDevops: {"vso.code_write"}, @@ -95,26 +99,16 @@ func (c *jwtConfig) Exchange(ctx context.Context, code string, opts ...oauth2.Au } type DeviceAuth struct { - config *oauth2.Config - - ID string - URL string -} - -// DeviceAuthorization is the response from the device authorization endpoint. -// See: https://tools.ietf.org/html/rfc8628#section-3.2 -type DeviceAuthorization struct { - DeviceCode string `json:"device_code"` - UserCode string `json:"user_code"` - VerificationURI string `json:"verification_uri"` - ExpiresIn int `json:"expires_in"` - Interval int `json:"interval"` + ClientID string + TokenURL string + Scopes []string + CodeURL string } // AuthorizeDevice begins the device authorization flow. // See: https://tools.ietf.org/html/rfc8628#section-3.1 -func (c *DeviceAuth) AuthorizeDevice(ctx context.Context) (*DeviceAuthorization, error) { - if c.URL == "" { +func (c *DeviceAuth) AuthorizeDevice(ctx context.Context) (*codersdk.GitAuthDevice, error) { + if c.CodeURL == "" { return nil, xerrors.New("oauth2: device code URL not set") } req, err := http.NewRequestWithContext(ctx, http.MethodPost, c.formatDeviceCodeURL(), nil) @@ -127,16 +121,22 @@ func (c *DeviceAuth) AuthorizeDevice(ctx context.Context) (*DeviceAuthorization, return nil, err } defer resp.Body.Close() - var da DeviceAuthorization + var da codersdk.GitAuthDevice return &da, json.NewDecoder(resp.Body).Decode(&da) } +type ExchangeDeviceCodeResponse struct { + *oauth2.Token + Error string `json:"error"` + ErrorDescription string `json:"error_description"` +} + // ExchangeDeviceCode exchanges a device code for an access token. // The boolean returned indicates whether the device code is still pending // and the caller should try again. func (c *DeviceAuth) ExchangeDeviceCode(ctx context.Context, deviceCode string) (*oauth2.Token, error) { - if c.URL == "" { - return nil, xerrors.New("oauth2: device code URL not set") + if c.TokenURL == "" { + return nil, xerrors.New("oauth2: token URL not set") } req, err := http.NewRequestWithContext(ctx, http.MethodPost, c.formatDeviceTokenURL(deviceCode), nil) if err != nil { @@ -151,11 +151,7 @@ func (c *DeviceAuth) ExchangeDeviceCode(ctx context.Context, deviceCode string) if resp.StatusCode != http.StatusOK { return nil, codersdk.ReadBodyAsError(resp) } - var body struct { - *oauth2.Token - Error string `json:"error"` - ErrorDescription string `json:"error_description"` - } + var body ExchangeDeviceCodeResponse err = json.NewDecoder(resp.Body).Decode(&body) if err != nil { return nil, err @@ -168,13 +164,13 @@ func (c *DeviceAuth) ExchangeDeviceCode(ctx context.Context, deviceCode string) func (c *DeviceAuth) formatDeviceTokenURL(deviceCode string) string { var buf bytes.Buffer - _, _ = buf.WriteString(c.config.Endpoint.TokenURL) + _, _ = buf.WriteString(c.TokenURL) v := url.Values{ - "client_id": {c.config.ClientID}, + "client_id": {c.ClientID}, "device_code": {deviceCode}, "grant_type": {"urn:ietf:params:oauth:grant-type:device_code"}, } - if strings.Contains(c.config.Endpoint.TokenURL, "?") { + if strings.Contains(c.TokenURL, "?") { _ = buf.WriteByte('&') } else { _ = buf.WriteByte('?') @@ -185,13 +181,13 @@ func (c *DeviceAuth) formatDeviceTokenURL(deviceCode string) string { func (c *DeviceAuth) formatDeviceCodeURL() string { var buf bytes.Buffer - _, _ = buf.WriteString(c.URL) + _, _ = buf.WriteString(c.CodeURL) v := url.Values{ - "client_id": {c.config.ClientID}, - "scope": c.config.Scopes, + "client_id": {c.ClientID}, + "scope": c.Scopes, } - if strings.Contains(c.URL, "?") { + if strings.Contains(c.CodeURL, "?") { _ = buf.WriteByte('&') } else { _ = buf.WriteByte('?') diff --git a/coderd/gitauth_test.go b/coderd/gitauth_test.go index ddea1357e9422..406866ba5709c 100644 --- a/coderd/gitauth_test.go +++ b/coderd/gitauth_test.go @@ -10,6 +10,7 @@ import ( "testing" "time" + "github.com/google/go-github/v43/github" "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -18,6 +19,7 @@ import ( "github.com/coder/coder/coderd/coderdtest" "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/gitauth" + "github.com/coder/coder/coderd/httpapi" "github.com/coder/coder/codersdk" "github.com/coder/coder/codersdk/agentsdk" "github.com/coder/coder/provisioner/echo" @@ -25,8 +27,190 @@ import ( "github.com/coder/coder/testutil" ) +func TestGitAuthByID(t *testing.T) { + t.Parallel() + t.Run("Unauthenticated", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{ + GitAuthConfigs: []*gitauth.Config{{ + ID: "test", + OAuth2Config: &testutil.OAuth2Config{}, + Type: codersdk.GitProviderGitHub, + }}, + }) + coderdtest.CreateFirstUser(t, client) + auth, err := client.GitAuthByID(context.Background(), "test") + require.NoError(t, err) + require.False(t, auth.Authenticated) + }) + t.Run("AuthenticatedNoUser", func(t *testing.T) { + // Ensures that a provider that can't obtain a user can + // still return that the provider is authenticated. + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{ + GitAuthConfigs: []*gitauth.Config{{ + ID: "test", + OAuth2Config: &testutil.OAuth2Config{}, + // AzureDevops doesn't have a user endpoint! + Type: codersdk.GitProviderAzureDevops, + }}, + }) + coderdtest.CreateFirstUser(t, client) + coderdtest.RequestGitAuthCallback(t, "test", client) + auth, err := client.GitAuthByID(context.Background(), "test") + require.NoError(t, err) + require.True(t, auth.Authenticated) + }) + t.Run("AuthenticatedWithUser", func(t *testing.T) { + t.Parallel() + validateSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + httpapi.Write(r.Context(), w, http.StatusOK, github.User{ + Login: github.String("kyle"), + AvatarURL: github.String("https://avatars.githubusercontent.com/u/12345678?v=4"), + }) + })) + defer validateSrv.Close() + client := coderdtest.New(t, &coderdtest.Options{ + GitAuthConfigs: []*gitauth.Config{{ + ID: "test", + ValidateURL: validateSrv.URL, + OAuth2Config: &testutil.OAuth2Config{}, + Type: codersdk.GitProviderGitHub, + }}, + }) + coderdtest.CreateFirstUser(t, client) + coderdtest.RequestGitAuthCallback(t, "test", client) + auth, err := client.GitAuthByID(context.Background(), "test") + require.NoError(t, err) + require.True(t, auth.Authenticated) + require.NotNil(t, auth.User) + require.Equal(t, "kyle", auth.User.Login) + }) + t.Run("AuthenticatedWithInstalls", func(t *testing.T) { + t.Parallel() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/user": + httpapi.Write(r.Context(), w, http.StatusOK, github.User{ + Login: github.String("kyle"), + AvatarURL: github.String("https://avatars.githubusercontent.com/u/12345678?v=4"), + }) + case "/installs": + httpapi.Write(r.Context(), w, http.StatusOK, []github.Installation{{ + ID: github.Int64(12345678), + Account: &github.User{ + Login: github.String("coder"), + }, + }}) + } + })) + defer srv.Close() + client := coderdtest.New(t, &coderdtest.Options{ + GitAuthConfigs: []*gitauth.Config{{ + ID: "test", + ValidateURL: srv.URL + "/user", + AppInstallationsURL: srv.URL + "/installs", + OAuth2Config: &testutil.OAuth2Config{}, + Type: codersdk.GitProviderGitHub, + }}, + }) + coderdtest.CreateFirstUser(t, client) + coderdtest.RequestGitAuthCallback(t, "test", client) + auth, err := client.GitAuthByID(context.Background(), "test") + require.NoError(t, err) + require.True(t, auth.Authenticated) + require.NotNil(t, auth.User) + require.Equal(t, "kyle", auth.User.Login) + require.NotNil(t, auth.AppInstallations) + require.Len(t, auth.AppInstallations, 1) + }) +} + +func TestGitAuthDevice(t *testing.T) { + t.Parallel() + t.Run("NotSupported", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{ + GitAuthConfigs: []*gitauth.Config{{ + ID: "test", + }}, + }) + coderdtest.CreateFirstUser(t, client) + _, err := client.GitAuthDeviceByID(context.Background(), "test") + var sdkErr *codersdk.Error + require.ErrorAs(t, err, &sdkErr) + require.Equal(t, http.StatusBadRequest, sdkErr.StatusCode()) + }) + t.Run("FetchCode", func(t *testing.T) { + t.Parallel() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + httpapi.Write(r.Context(), w, http.StatusOK, codersdk.GitAuthDevice{ + UserCode: "hey", + }) + })) + defer srv.Close() + client := coderdtest.New(t, &coderdtest.Options{ + GitAuthConfigs: []*gitauth.Config{{ + ID: "test", + DeviceAuth: &gitauth.DeviceAuth{ + ClientID: "test", + CodeURL: srv.URL, + Scopes: []string{"repo"}, + }, + }}, + }) + coderdtest.CreateFirstUser(t, client) + device, err := client.GitAuthDeviceByID(context.Background(), "test") + require.NoError(t, err) + require.Equal(t, "hey", device.UserCode) + }) + t.Run("ExchangeCode", func(t *testing.T) { + t.Parallel() + resp := gitauth.ExchangeDeviceCodeResponse{ + Error: "authorization_pending", + } + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + httpapi.Write(r.Context(), w, http.StatusOK, resp) + })) + defer srv.Close() + client := coderdtest.New(t, &coderdtest.Options{ + GitAuthConfigs: []*gitauth.Config{{ + ID: "test", + DeviceAuth: &gitauth.DeviceAuth{ + ClientID: "test", + TokenURL: srv.URL, + Scopes: []string{"repo"}, + }, + }}, + }) + coderdtest.CreateFirstUser(t, client) + err := client.GitAuthDeviceExchange(context.Background(), "test", codersdk.GitAuthDeviceExchange{ + DeviceCode: "hey", + }) + var sdkErr *codersdk.Error + require.ErrorAs(t, err, &sdkErr) + require.Equal(t, http.StatusBadRequest, sdkErr.StatusCode()) + require.Equal(t, "authorization_pending", sdkErr.Detail) + + resp = gitauth.ExchangeDeviceCodeResponse{ + Token: &oauth2.Token{ + AccessToken: "hey", + }, + } + + err = client.GitAuthDeviceExchange(context.Background(), "test", codersdk.GitAuthDeviceExchange{ + DeviceCode: "hey", + }) + require.NoError(t, err) + + auth, err := client.GitAuthByID(context.Background(), "test") + require.NoError(t, err) + require.True(t, auth.Authenticated) + }) +} + // nolint:bodyclose -func TestWorkspaceAgentsGitAuth(t *testing.T) { +func TestGitAuthCallback(t *testing.T) { t.Parallel() t.Run("NoMatchingConfig", func(t *testing.T) { t.Parallel() @@ -127,7 +311,7 @@ func TestWorkspaceAgentsGitAuth(t *testing.T) { require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) location, err := resp.Location() require.NoError(t, err) - require.Equal(t, "/gitauth", location.Path) + require.Equal(t, "/gitauth/github", location.Path) // Callback again to simulate updating the token. resp = coderdtest.RequestGitAuthCallback(t, "github", client) diff --git a/coderd/httpmw/gitauthparam.go b/coderd/httpmw/gitauthparam.go new file mode 100644 index 0000000000000..2ce592d54f98a --- /dev/null +++ b/coderd/httpmw/gitauthparam.go @@ -0,0 +1,40 @@ +package httpmw + +import ( + "context" + "net/http" + + "github.com/go-chi/chi/v5" + + "github.com/coder/coder/coderd/gitauth" + "github.com/coder/coder/coderd/httpapi" +) + +type gitAuthParamContextKey struct{} + +func GitAuthParam(r *http.Request) *gitauth.Config { + config, ok := r.Context().Value(gitAuthParamContextKey{}).(*gitauth.Config) + if !ok { + panic("developer error: gitauth param middleware not provided") + } + return config +} + +func ExtractGitAuthParam(configs []*gitauth.Config) func(next http.Handler) http.Handler { + configByID := make(map[string]*gitauth.Config) + for _, c := range configs { + configByID[c.ID] = c + } + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + config, ok := configByID[chi.URLParam(r, "gitauth")] + if !ok { + httpapi.ResourceNotFound(w) + return + } + + r = r.WithContext(context.WithValue(r.Context(), gitAuthParamContextKey{}, config)) + next.ServeHTTP(w, r) + }) + } +} diff --git a/coderd/httpmw/gitauthparam_test.go b/coderd/httpmw/gitauthparam_test.go new file mode 100644 index 0000000000000..01ea35470f025 --- /dev/null +++ b/coderd/httpmw/gitauthparam_test.go @@ -0,0 +1,49 @@ +package httpmw_test + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/go-chi/chi/v5" + "github.com/stretchr/testify/require" + + "github.com/coder/coder/coderd/gitauth" + "github.com/coder/coder/coderd/httpmw" +) + +//nolint:bodyclose +func TestGitAuthParam(t *testing.T) { + t.Parallel() + t.Run("Found", func(t *testing.T) { + t.Parallel() + routeCtx := chi.NewRouteContext() + routeCtx.URLParams.Add("gitauth", "my-id") + r := httptest.NewRequest(http.MethodGet, "/", nil) + r = r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, routeCtx)) + res := httptest.NewRecorder() + + httpmw.ExtractGitAuthParam([]*gitauth.Config{{ + ID: "my-id", + }})(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, "my-id", httpmw.GitAuthParam(r).ID) + w.WriteHeader(http.StatusOK) + })).ServeHTTP(res, r) + + require.Equal(t, http.StatusOK, res.Result().StatusCode) + }) + + t.Run("NotFound", func(t *testing.T) { + t.Parallel() + routeCtx := chi.NewRouteContext() + routeCtx.URLParams.Add("gitauth", "my-id") + r := httptest.NewRequest(http.MethodGet, "/", nil) + r = r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, routeCtx)) + res := httptest.NewRecorder() + + httpmw.ExtractGitAuthParam([]*gitauth.Config{})(nil).ServeHTTP(res, r) + + require.Equal(t, http.StatusNotFound, res.Result().StatusCode) + }) +} diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go index 4a1aa8b633bcd..4ae04cba22b72 100644 --- a/coderd/workspaceagents.go +++ b/coderd/workspaceagents.go @@ -1902,18 +1902,16 @@ func (api *API) workspaceAgentsGitAuth(rw http.ResponseWriter, r *http.Request) if gitAuthLink.OAuthExpiry.Before(database.Now()) && !gitAuthLink.OAuthExpiry.IsZero() { continue } - if gitAuthConfig.ValidateURL != "" { - valid, err := gitAuthConfig.ValidateToken(ctx, gitAuthLink.OAuthAccessToken) - if err != nil { - api.Logger.Warn(ctx, "failed to validate git auth token", - slog.F("workspace_owner_id", workspace.OwnerID.String()), - slog.F("validate_url", gitAuthConfig.ValidateURL), - slog.Error(err), - ) - } - if !valid { - continue - } + valid, _, err := gitAuthConfig.ValidateToken(ctx, gitAuthLink.OAuthAccessToken) + if err != nil { + api.Logger.Warn(ctx, "failed to validate git auth token", + slog.F("workspace_owner_id", workspace.OwnerID.String()), + slog.F("validate_url", gitAuthConfig.ValidateURL), + slog.Error(err), + ) + } + if !valid { + continue } httpapi.Write(ctx, rw, http.StatusOK, formatGitAuthAccessToken(gitAuthConfig.Type, gitAuthLink.OAuthAccessToken)) return diff --git a/codersdk/authorization.go b/codersdk/authorization.go index fbb62aa5f8698..4e8a6eed7019f 100644 --- a/codersdk/authorization.go +++ b/codersdk/authorization.go @@ -3,7 +3,6 @@ package codersdk import ( "context" "encoding/json" - "fmt" "net/http" ) @@ -21,10 +20,6 @@ type AuthorizationRequest struct { Checks map[string]AuthorizationCheck `json:"checks"` } -type ExchangeGitAuthRequest struct { - DeviceCode string `json:"device_code"` -} - // AuthorizationCheck is used to check if the currently authenticated user (or the specified user) can do a given action to a given set of objects. // // @Description AuthorizationCheck is used to check if the currently authenticated user (or the specified user) can do a given action to a given set of objects. @@ -75,16 +70,3 @@ func (c *Client) AuthCheck(ctx context.Context, req AuthorizationRequest) (Autho var resp AuthorizationResponse return resp, json.NewDecoder(res.Body).Decode(&resp) } - -// ExchangeGitAuth exchanges a device code for a git auth token. -func (c *Client) ExchangeGitAuth(ctx context.Context, provider string, req ExchangeGitAuthRequest) error { - res, err := c.Request(ctx, http.MethodPost, fmt.Sprintf("/api/v2/gitauth/%s/exchange", provider), req) - if err != nil { - return err - } - defer res.Body.Close() - if res.StatusCode != http.StatusNoContent { - return ReadBodyAsError(res) - } - return nil -} diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 5c73f267a7a88..ee33f8aafcaba 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -297,18 +297,20 @@ type TraceConfig struct { } type GitAuthConfig struct { - ID string `json:"id"` - Type string `json:"type"` - ClientID string `json:"client_id"` - ClientSecret string `json:"-" yaml:"client_secret"` - AuthURL string `json:"auth_url"` - TokenURL string `json:"token_url"` - ValidateURL string `json:"validate_url"` - Regex string `json:"regex"` - NoRefresh bool `json:"no_refresh"` - Scopes []string `json:"scopes"` - DeviceFlow bool `json:"device_flow"` - DeviceAuthURL string `json:"device_auth_url"` + ID string `json:"id"` + Type string `json:"type"` + ClientID string `json:"client_id"` + ClientSecret string `json:"-" yaml:"client_secret"` + AuthURL string `json:"auth_url"` + TokenURL string `json:"token_url"` + ValidateURL string `json:"validate_url"` + AppInstallURL string `json:"app_install_url"` + AppInstallationsURL string `json:"app_installations_url"` + Regex string `json:"regex"` + NoRefresh bool `json:"no_refresh"` + Scopes []string `json:"scopes"` + DeviceFlow bool `json:"device_flow"` + DeviceAuthURL string `json:"device_auth_url"` } type ProvisionerConfig struct { diff --git a/codersdk/gitauth.go b/codersdk/gitauth.go new file mode 100644 index 0000000000000..790e0291c23d5 --- /dev/null +++ b/codersdk/gitauth.go @@ -0,0 +1,85 @@ +package codersdk + +import ( + "context" + "encoding/json" + "fmt" + "net/http" +) + +type GitAuth struct { + Authenticated bool `json:"authenticated"` + Device bool `json:"device"` + + // User is the user that authenticated with the provider. + User *GitAuthUser `json:"user"` + // AppInstallations are the installations that the user has access to. + AppInstallations []GitAuthAppInstallation `json:"installations"` +} + +type GitAuthAppInstallation struct { + ID int `json:"id"` + Account *GitAuthUser `json:"account"` + ConfigureURL string `json:"configure_url"` +} + +type GitAuthUser struct { + Login string + AvatarURL string + ProfileURL string + Name string +} + +// GitAuthDevice is the response from the device authorization endpoint. +// See: https://tools.ietf.org/html/rfc8628#section-3.2 +type GitAuthDevice struct { + DeviceCode string `json:"device_code"` + UserCode string `json:"user_code"` + VerificationURI string `json:"verification_uri"` + ExpiresIn int `json:"expires_in"` + Interval int `json:"interval"` +} + +type GitAuthDeviceExchange struct { + DeviceCode string `json:"device_code"` +} + +func (c *Client) GitAuthDeviceByID(ctx context.Context, provider string) (GitAuthDevice, error) { + res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/gitauth/%s/device", provider), nil) + if err != nil { + return GitAuthDevice{}, err + } + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + return GitAuthDevice{}, ReadBodyAsError(res) + } + var gitauth GitAuthDevice + return gitauth, json.NewDecoder(res.Body).Decode(&gitauth) +} + +// ExchangeGitAuth exchanges a device code for a git auth token. +func (c *Client) GitAuthDeviceExchange(ctx context.Context, provider string, req GitAuthDeviceExchange) error { + res, err := c.Request(ctx, http.MethodPost, fmt.Sprintf("/api/v2/gitauth/%s/device", provider), req) + if err != nil { + return err + } + defer res.Body.Close() + if res.StatusCode != http.StatusNoContent { + return ReadBodyAsError(res) + } + return nil +} + +// GitAuthByID returns the git auth for the given provider by ID. +func (c *Client) GitAuthByID(ctx context.Context, provider string) (GitAuth, error) { + res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/gitauth/%s", provider), nil) + if err != nil { + return GitAuth{}, err + } + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + return GitAuth{}, ReadBodyAsError(res) + } + var gitauth GitAuth + return gitauth, json.NewDecoder(res.Body).Decode(&gitauth) +} diff --git a/site/vite.config.ts b/site/vite.config.ts index d8ace5c9c5387..6e15f7e0f9531 100644 --- a/site/vite.config.ts +++ b/site/vite.config.ts @@ -62,12 +62,6 @@ export default defineConfig({ }) }, }, - // The device route is visual! - "^/gitauth/(?!.*(device))": { - changeOrigin: true, - target: process.env.CODER_HOST || "http://localhost:3000", - secure: process.env.NODE_ENV === "production", - }, "/swagger": { target: process.env.CODER_HOST || "http://localhost:3000", secure: process.env.NODE_ENV === "production", From 0af6a37dca1591fde788e0ccd830f75c07ca7d63 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Tue, 27 Jun 2023 22:32:21 +0000 Subject: [PATCH 04/17] Redesign the git auth page --- cli/server.go | 4 + coderd/apidoc/docs.go | 6 + coderd/apidoc/swagger.json | 6 + coderd/gitauth.go | 4 +- coderd/gitauth/config.go | 22 +- codersdk/gitauth.go | 17 +- docs/api/general.md | 2 + docs/api/schemas.md | 36 ++- site/src/AppRouter.tsx | 7 +- site/src/api/api.ts | 20 +- site/src/api/typesGenerated.ts | 47 ++- .../components/Dashboard/DashboardLayout.tsx | 1 + .../components/SignInLayout/SignInLayout.tsx | 2 +- site/src/components/Welcome/Welcome.tsx | 2 +- .../GitAuthDevicePage/GitAuthDevicePage.tsx | 7 +- site/src/pages/GitAuthPage/GitAuthPage.tsx | 280 ++++++++++++++++-- site/src/testHelpers/entities.ts | 1 + site/src/xServices/auth/authXService.ts | 7 + 18 files changed, 400 insertions(+), 71 deletions(-) diff --git a/cli/server.go b/cli/server.go index e69451a666501..07adc0f620eda 100644 --- a/cli/server.go +++ b/cli/server.go @@ -164,6 +164,10 @@ func ReadGitAuthProvidersFromEnv(environ []string) ([]codersdk.GitAuthConfig, er provider.NoRefresh = b case "SCOPES": provider.Scopes = strings.Split(v.Value, " ") + case "APP_INSTALL_URL": + provider.AppInstallURL = v.Value + case "APP_INSTALLATIONS_URL": + provider.AppInstallationsURL = v.Value } providers[providerNum] = provider } diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index da3d236337b65..36c2095581d9c 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -7443,6 +7443,12 @@ const docTemplate = `{ "codersdk.GitAuthConfig": { "type": "object", "properties": { + "app_install_url": { + "type": "string" + }, + "app_installations_url": { + "type": "string" + }, "auth_url": { "type": "string" }, diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index c3779de6be6e1..0b6323fce65d4 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -6664,6 +6664,12 @@ "codersdk.GitAuthConfig": { "type": "object", "properties": { + "app_install_url": { + "type": "string" + }, + "app_installations_url": { + "type": "string" + }, "auth_url": { "type": "string" }, diff --git a/coderd/gitauth.go b/coderd/gitauth.go index 4a5bd87ae6eaa..4ed4b2f6f9604 100644 --- a/coderd/gitauth.go +++ b/coderd/gitauth.go @@ -24,6 +24,8 @@ func (api *API) gitAuthByID(w http.ResponseWriter, r *http.Request) { res := codersdk.GitAuth{ Authenticated: false, Device: config.DeviceAuth != nil, + AppInstallURL: config.AppInstallURL, + Type: config.Type.Pretty(), } link, err := api.Database.GetGitAuthLink(ctx, database.GetGitAuthLinkParams{ @@ -37,7 +39,7 @@ func (api *API) gitAuthByID(w http.ResponseWriter, r *http.Request) { return err }) eg.Go(func() (err error) { - res.AppInstallations, err = config.AppInstallations(ctx, link.OAuthAccessToken) + res.AppInstallations, res.AppInstallable, err = config.AppInstallations(ctx, link.OAuthAccessToken) return err }) err = eg.Wait() diff --git a/coderd/gitauth/config.go b/coderd/gitauth/config.go index 21ed28f3ee3bd..c0b79842ea48c 100644 --- a/coderd/gitauth/config.go +++ b/coderd/gitauth/config.go @@ -44,7 +44,7 @@ type Config struct { // returning it to the user. If omitted, tokens will // not be validated before being returned. ValidateURL string - // InstallURL is for GitHub App's (and hopefully others eventually) + // AppInstallURL is for GitHub App's (and hopefully others eventually) // to provide a link to install the app. There's installation // of the application, and user authentication. It's possible // for the user to authenticate but the application to not. @@ -155,29 +155,31 @@ type AppInstallation struct { // AppInstallations returns a list of app installations for the given token. // If the provider does not support app installations, it returns nil. -func (c *Config) AppInstallations(ctx context.Context, token string) ([]codersdk.GitAuthAppInstallation, error) { +func (c *Config) AppInstallations(ctx context.Context, token string) ([]codersdk.GitAuthAppInstallation, bool, error) { if c.AppInstallationsURL == "" { - return nil, nil + return nil, false, nil } req, err := http.NewRequestWithContext(ctx, http.MethodGet, c.AppInstallationsURL, nil) if err != nil { - return nil, err + return nil, false, err } req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token)) res, err := http.DefaultClient.Do(req) if err != nil { - return nil, err + return nil, false, err } defer res.Body.Close() installs := []codersdk.GitAuthAppInstallation{} if c.Type == codersdk.GitProviderGitHub { - var ghInstalls []github.Installation + var ghInstalls struct { + Installations []*github.Installation `json:"installations"` + } err = json.NewDecoder(res.Body).Decode(&ghInstalls) if err != nil { - return nil, err + return nil, false, err } - for _, installation := range ghInstalls { + for _, installation := range ghInstalls.Installations { install := codersdk.GitAuthAppInstallation{ ID: int(installation.GetID()), ConfigureURL: installation.GetHTMLURL(), @@ -194,7 +196,7 @@ func (c *Config) AppInstallations(ctx context.Context, token string) ([]codersdk installs = append(installs, install) } } - return installs, nil + return installs, true, nil } // ConvertConfig converts the SDK configuration entry format @@ -301,7 +303,7 @@ func ConvertConfig(entries []codersdk.GitAuthConfig, accessURL *url.URL) ([]*Con } cfg.DeviceAuth = &DeviceAuth{ ClientID: entry.ClientID, - TokenURL: entry.TokenURL, + TokenURL: oc.Endpoint.TokenURL, Scopes: entry.Scopes, CodeURL: entry.DeviceAuthURL, } diff --git a/codersdk/gitauth.go b/codersdk/gitauth.go index 790e0291c23d5..1b52a245e7321 100644 --- a/codersdk/gitauth.go +++ b/codersdk/gitauth.go @@ -8,13 +8,18 @@ import ( ) type GitAuth struct { - Authenticated bool `json:"authenticated"` - Device bool `json:"device"` + Authenticated bool `json:"authenticated"` + Device bool `json:"device"` + Type string `json:"type"` // User is the user that authenticated with the provider. User *GitAuthUser `json:"user"` + // AppInstallable is true if the request for app installs was successful. + AppInstallable bool `json:"app_installable"` // AppInstallations are the installations that the user has access to. AppInstallations []GitAuthAppInstallation `json:"installations"` + // AppInstallURL is the URL to install the app. + AppInstallURL string `json:"app_install_url"` } type GitAuthAppInstallation struct { @@ -24,10 +29,10 @@ type GitAuthAppInstallation struct { } type GitAuthUser struct { - Login string - AvatarURL string - ProfileURL string - Name string + Login string `json:"login"` + AvatarURL string `json:"avatar_url"` + ProfileURL string `json:"profile_url"` + Name string `json:"name"` } // GitAuthDevice is the response from the device authorization endpoint. diff --git a/docs/api/general.md b/docs/api/general.md index 9c57b5f42f33b..f0d6d7f2e1c9f 100644 --- a/docs/api/general.md +++ b/docs/api/general.md @@ -200,6 +200,8 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \ "git_auth": { "value": [ { + "app_install_url": "string", + "app_installations_url": "string", "auth_url": "string", "client_id": "string", "device_auth_url": "string", diff --git a/docs/api/schemas.md b/docs/api/schemas.md index a084cadd16e90..890a0cd91b65c 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -593,6 +593,8 @@ { "value": [ { + "app_install_url": "string", + "app_installations_url": "string", "auth_url": "string", "client_id": "string", "device_auth_url": "string", @@ -1879,6 +1881,8 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "git_auth": { "value": [ { + "app_install_url": "string", + "app_installations_url": "string", "auth_url": "string", "client_id": "string", "device_auth_url": "string", @@ -2212,6 +2216,8 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "git_auth": { "value": [ { + "app_install_url": "string", + "app_installations_url": "string", "auth_url": "string", "client_id": "string", "device_auth_url": "string", @@ -2581,6 +2587,8 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in ```json { + "app_install_url": "string", + "app_installations_url": "string", "auth_url": "string", "client_id": "string", "device_auth_url": "string", @@ -2597,19 +2605,21 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in ### Properties -| Name | Type | Required | Restrictions | Description | -| ----------------- | --------------- | -------- | ------------ | ----------- | -| `auth_url` | string | false | | | -| `client_id` | string | false | | | -| `device_auth_url` | string | false | | | -| `device_flow` | boolean | false | | | -| `id` | string | false | | | -| `no_refresh` | boolean | false | | | -| `regex` | string | false | | | -| `scopes` | array of string | false | | | -| `token_url` | string | false | | | -| `type` | string | false | | | -| `validate_url` | string | false | | | +| Name | Type | Required | Restrictions | Description | +| ----------------------- | --------------- | -------- | ------------ | ----------- | +| `app_install_url` | string | false | | | +| `app_installations_url` | string | false | | | +| `auth_url` | string | false | | | +| `client_id` | string | false | | | +| `device_auth_url` | string | false | | | +| `device_flow` | boolean | false | | | +| `id` | string | false | | | +| `no_refresh` | boolean | false | | | +| `regex` | string | false | | | +| `scopes` | array of string | false | | | +| `token_url` | string | false | | | +| `type` | string | false | | | +| `validate_url` | string | false | | | ## codersdk.GitProvider diff --git a/site/src/AppRouter.tsx b/site/src/AppRouter.tsx index 621d2a663457b..168d13e2f4b10 100644 --- a/site/src/AppRouter.tsx +++ b/site/src/AppRouter.tsx @@ -196,12 +196,7 @@ export const AppRouter: FC = () => { }> } /> - - } /> - - } /> - - + } /> } /> diff --git a/site/src/api/api.ts b/site/src/api/api.ts index 4e6be6db050fc..6047d71cce0aa 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -799,11 +799,25 @@ export const getExperiments = async (): Promise => { } } -export const exchangeGitAuth = async ( +export const getGitAuthProvider = async ( provider: string, - req: TypesGen.ExchangeGitAuthRequest, +): Promise => { + const resp = await axios.get(`/api/v2/gitauth/${provider}`) + return resp.data +} + +export const getGitAuthDevice = async ( + provider: string, +): Promise => { + const resp = await axios.get(`/api/v2/gitauth/${provider}/device`) + return resp.data +} + +export const exchangeGitAuthDevice = async ( + provider: string, + req: TypesGen.GitAuthDeviceExchange, ): Promise => { - const resp = await axios.post(`/gitauth/${provider}/exchange`, req) + const resp = await axios.post(`/api/v2/gitauth/${provider}/device`, req) return resp.data } diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 96b3b026e8d5b..4c8b5bf1da181 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -392,11 +392,6 @@ export interface Entitlements { readonly require_telemetry: boolean } -// From codersdk/authorization.go -export interface ExchangeGitAuthRequest { - readonly device_code: string -} - // From codersdk/deployment.go export type Experiments = Experiment[] @@ -419,6 +414,24 @@ export interface GetUsersResponse { readonly count: number } +// From codersdk/gitauth.go +export interface GitAuth { + readonly authenticated: boolean + readonly device: boolean + readonly type: string + readonly user?: GitAuthUser + readonly app_installable: boolean + readonly installations: GitAuthAppInstallation[] + readonly app_install_url: string +} + +// From codersdk/gitauth.go +export interface GitAuthAppInstallation { + readonly id: number + readonly account?: GitAuthUser + readonly configure_url: string +} + // From codersdk/deployment.go export interface GitAuthConfig { readonly id: string @@ -427,6 +440,8 @@ export interface GitAuthConfig { readonly auth_url: string readonly token_url: string readonly validate_url: string + readonly app_install_url: string + readonly app_installations_url: string readonly regex: string readonly no_refresh: boolean readonly scopes: string[] @@ -434,6 +449,28 @@ export interface GitAuthConfig { readonly device_auth_url: string } +// From codersdk/gitauth.go +export interface GitAuthDevice { + readonly device_code: string + readonly user_code: string + readonly verification_uri: string + readonly expires_in: number + readonly interval: number +} + +// From codersdk/gitauth.go +export interface GitAuthDeviceExchange { + readonly device_code: string +} + +// From codersdk/gitauth.go +export interface GitAuthUser { + readonly login: string + readonly avatar_url: string + readonly profile_url: string + readonly name: string +} + // From codersdk/gitsshkey.go export interface GitSSHKey { readonly user_id: string diff --git a/site/src/components/Dashboard/DashboardLayout.tsx b/site/src/components/Dashboard/DashboardLayout.tsx index 6aa8db92785ed..cf85fa65631a1 100644 --- a/site/src/components/Dashboard/DashboardLayout.tsx +++ b/site/src/components/Dashboard/DashboardLayout.tsx @@ -110,5 +110,6 @@ const useStyles = makeStyles({ siteContent: { flex: 1, paddingBottom: dashboardContentBottomPadding, // Add bottom space since we don't use a footer + display: "flex", }, }) diff --git a/site/src/components/SignInLayout/SignInLayout.tsx b/site/src/components/SignInLayout/SignInLayout.tsx index 402010451cfb7..022bda5fe8be5 100644 --- a/site/src/components/SignInLayout/SignInLayout.tsx +++ b/site/src/components/SignInLayout/SignInLayout.tsx @@ -3,7 +3,7 @@ import { FC, ReactNode } from "react" export const useStyles = makeStyles((theme) => ({ root: { - height: "100vh", + flex: 1, display: "flex", justifyContent: "center", alignItems: "center", diff --git a/site/src/components/Welcome/Welcome.tsx b/site/src/components/Welcome/Welcome.tsx index 6297f7db422c5..660e7cc381f6d 100644 --- a/site/src/components/Welcome/Welcome.tsx +++ b/site/src/components/Welcome/Welcome.tsx @@ -44,7 +44,7 @@ const useStyles = makeStyles((theme) => ({ margin: 0, marginBottom: theme.spacing(2), marginTop: theme.spacing(2), - lineHeight: 1, + lineHeight: 1.25, "& strong": { fontWeight: 600, diff --git a/site/src/pages/GitAuthDevicePage/GitAuthDevicePage.tsx b/site/src/pages/GitAuthDevicePage/GitAuthDevicePage.tsx index c861dc8bd7fa8..41744c1595c41 100644 --- a/site/src/pages/GitAuthDevicePage/GitAuthDevicePage.tsx +++ b/site/src/pages/GitAuthDevicePage/GitAuthDevicePage.tsx @@ -3,14 +3,13 @@ import CircularProgress from "@mui/material/CircularProgress" import Link from "@mui/material/Link" import { makeStyles } from "@mui/styles" import { useQuery } from "@tanstack/react-query" -import { exchangeGitAuth } from "api/api" -import { ApiErrorResponse } from "api/errors" +import { exchangeGitAuthDevice } from "api/api" import { isAxiosError } from "axios" import { Alert } from "components/Alert/Alert" import { CopyButton } from "components/CopyButton/CopyButton" import { SignInLayout } from "components/SignInLayout/SignInLayout" import { Welcome } from "components/Welcome/Welcome" -import { FC, useEffect, useMemo, useState } from "react" +import { FC, useEffect, useState } from "react" import { useNavigate, useParams, useSearchParams } from "react-router-dom" const GitAuthDevicePage: FC = () => { @@ -38,7 +37,7 @@ const GitAuthDevicePage: FC = () => { const exchange = useQuery({ queryFn: () => - exchangeGitAuth(provider as string, { device_code: deviceCode }), + exchangeGitAuthDevice(provider as string, { device_code: deviceCode }), queryKey: ["gitauth", provider as string, deviceCode], retry: true, retryDelay: interval * 1000, diff --git a/site/src/pages/GitAuthPage/GitAuthPage.tsx b/site/src/pages/GitAuthPage/GitAuthPage.tsx index 18c251a1e4b14..9e5f7fc349b20 100644 --- a/site/src/pages/GitAuthPage/GitAuthPage.tsx +++ b/site/src/pages/GitAuthPage/GitAuthPage.tsx @@ -1,64 +1,302 @@ -import Button from "@mui/material/Button" +import RefreshIcon from "@mui/icons-material/Refresh" +import OpenInNewIcon from "@mui/icons-material/OpenInNew" +import CircularProgress from "@mui/material/CircularProgress" +import Link from "@mui/material/Link" +import Tooltip from "@mui/material/Tooltip" import { makeStyles } from "@mui/styles" +import { useQuery, useQueryClient } from "@tanstack/react-query" +import { + exchangeGitAuthDevice, + getGitAuthDevice, + getGitAuthProvider, +} from "api/api" +import { isAxiosError } from "axios" +import { Alert } from "components/Alert/Alert" +import { Avatar } from "components/Avatar/Avatar" +import { CopyButton } from "components/CopyButton/CopyButton" import { SignInLayout } from "components/SignInLayout/SignInLayout" import { Welcome } from "components/Welcome/Welcome" import { FC, useEffect } from "react" -import { Link as RouterLink } from "react-router-dom" +import { useParams } from "react-router-dom" import { REFRESH_GITAUTH_BROADCAST_CHANNEL } from "xServices/createWorkspace/createWorkspaceXService" +import { usePermissions } from "hooks" const GitAuthPage: FC = () => { const styles = useStyles() + const { provider } = useParams() + if (!provider) { + throw new Error("provider must exist") + } + const permissions = usePermissions() + const queryClient = useQueryClient() + const query = useQuery({ + queryKey: ["gitauth", provider], + queryFn: () => getGitAuthProvider(provider), + refetchOnWindowFocus: true, + }) + useEffect(() => { + if (!query.data?.authenticated) { + return + } // This is used to notify the parent window that the Git auth token has been refreshed. // It's critical in the create workspace flow! // eslint-disable-next-line compat/compat -- It actually is supported... not sure why it's complaining. const bc = new BroadcastChannel(REFRESH_GITAUTH_BROADCAST_CHANNEL) // The message doesn't matter, any message refreshes the page! bc.postMessage("noop") - window.close() - }, []) + }, [query.data?.authenticated]) + + if (query.isLoading || !query.data) { + return null + } + + if (!query.data.authenticated) { + return ( + + + + {query.data.device && } + + ) + } + + const hasInstallations = query.data.installations?.length > 0 return ( - +

- Your Git authentication token will be refreshed to keep you signed in. + Hey @{query.data.user?.login} 👋! You are now authenticated with Git. + Feel free to close this window!

+ {query.data.installations?.length !== 0 && ( +
+ {query.data.installations.map((install) => { + if (!install.account) { + return + } + return ( + + + + {install.account.login} + + + + ) + })} +   + {query.data.installations.length} organization + {query.data.installations.length !== 1 && "s are"} authorized +
+ )} +
- + {!hasInstallations && query.data.app_installable && ( + + You must{" "} + + install the {query.data.type} App + {" "} + to clone private repositories. Accounts will appear here once + authorized. + + )} + + {permissions.viewGitAuthConfig && + query.data.app_install_url && + query.data.app_installable && ( + + + {query.data.installations?.length + ? "Configure" + : "Install"} the {query.data.type} App + + )} + { + queryClient.setQueryData(["gitauth", provider], { + ...query.data, + authenticated: false, + }) + }} + > + Reauthenticate +
) } +const GitDeviceAuth: FC<{ + provider: string +}> = ({ provider }) => { + const styles = useStyles() + const device = useQuery({ + queryFn: () => getGitAuthDevice(provider), + queryKey: ["gitauth", provider, "device"], + refetchOnMount: false, + }) + + const client = useQueryClient() + const exchange = useQuery({ + queryFn: () => + exchangeGitAuthDevice(provider, { + device_code: device.data?.device_code || "", + }), + queryKey: ["gitauth", provider, device.data?.device_code], + enabled: Boolean(device.data), + onSuccess: () => { + // Force a refresh of the Git auth status. + client.invalidateQueries(["gitauth", provider]).catch((ex) => { + console.error("invalidate queries", ex) + }) + }, + retry: true, + retryDelay: (device.data?.interval || 5) * 1000, + refetchOnWindowFocus: (query) => + query.state.status === "success" ? false : "always", + }) + + let status = ( +

+ + Checking for authentication... +

+ ) + if (isAxiosError(exchange.failureReason)) { + // See https://datatracker.ietf.org/doc/html/rfc8628#section-3.5 + switch (exchange.failureReason.response?.data?.detail) { + case "authorization_pending": + break + case "expired_token": + status = ( + + The one-time code has expired. Refresh to get a new one! + + ) + break + case "access_denied": + status = ( + Access to the Git provider was denied. + ) + break + default: + status = ( + + An unknown error occurred. Please try again:{" "} + {exchange.failureReason.message} + + ) + break + } + } + + if (!device.data) { + return + } + + return ( +
+

+ Copy your one-time code:  +

+ {device.data.user_code} {" "} + +
+
+ Then open the link below and paste it: +

+
+ + + Open and Paste + +
+ + {status} +
+ ) +} + export default GitAuthPage const useStyles = makeStyles((theme) => ({ - title: { - fontSize: theme.spacing(4), - fontWeight: 400, - lineHeight: "140%", - margin: 0, - }, - text: { fontSize: 16, color: theme.palette.text.secondary, - marginBottom: theme.spacing(4), textAlign: "center", lineHeight: "160%", + margin: 0, + }, + + copyCode: { + display: "inline-flex", + alignItems: "center", + }, + + code: { + fontWeight: "bold", + color: theme.palette.text.primary, }, - lineBreak: { - whiteSpace: "nowrap", + installAlert: { + margin: theme.spacing(2), }, links: { display: "flex", - justifyContent: "flex-end", - paddingTop: theme.spacing(1), + gap: theme.spacing(0.5), + margin: theme.spacing(2), + flexDirection: "column", + }, + + link: { + display: "flex", + alignItems: "center", + justifyContent: "center", + fontSize: 16, + gap: theme.spacing(1), + }, + + status: { + display: "flex", + alignItems: "center", + justifyContent: "center", + gap: theme.spacing(1), + color: theme.palette.text.disabled, + }, + + authorizedInstalls: { + display: "flex", + gap: 4, + color: theme.palette.text.disabled, + margin: theme.spacing(4), }, })) diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index c76f91ccefb2a..f8a5a8092ce29 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -1579,6 +1579,7 @@ export const MockPermissions: Permissions = { viewDeploymentValues: true, viewUpdateCheck: true, viewDeploymentStats: true, + viewGitAuthConfig: true, } export const MockAppearance: TypesGen.AppearanceConfig = { diff --git a/site/src/xServices/auth/authXService.ts b/site/src/xServices/auth/authXService.ts index 8c5ce5dd8edfc..267935b82edb9 100644 --- a/site/src/xServices/auth/authXService.ts +++ b/site/src/xServices/auth/authXService.ts @@ -17,6 +17,7 @@ export const checks = { viewDeploymentValues: "viewDeploymentValues", createGroup: "createGroup", viewUpdateCheck: "viewUpdateCheck", + viewGitAuthConfig: "viewGitAuthConfig", viewDeploymentStats: "viewDeploymentStats", } as const @@ -75,6 +76,12 @@ export const permissionsToCheck = { }, action: "read", }, + [checks.viewGitAuthConfig]: { + object: { + resource_type: "deployment_config", + }, + action: "read", + }, [checks.viewDeploymentStats]: { object: { resource_type: "deployment_stats", From ae60ebf3960d4f6f7e2085fb6aee7e875728b8fa Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Wed, 28 Jun 2023 00:40:25 +0000 Subject: [PATCH 05/17] Refactor to add a page view --- coderd/gitauth.go | 4 + coderd/gitauth/config.go | 17 +- codersdk/gitauth.go | 6 +- site/.storybook/preview.jsx | 11 +- site/package.json | 1 + site/src/AppRouter.tsx | 2 +- site/src/api/typesGenerated.ts | 2 +- .../components/SignInLayout/SignInLayout.tsx | 1 - .../GitAuthDevicePage/GitAuthDevicePage.tsx | 157 --------- .../pages/GitAuthPage/GitAuthPage.stories.tsx | 12 - site/src/pages/GitAuthPage/GitAuthPage.tsx | 310 +++--------------- .../GitAuthPage/GitAuthPageView.stories.tsx | 119 +++++++ .../src/pages/GitAuthPage/GitAuthPageView.tsx | 275 ++++++++++++++++ site/yarn.lock | 12 + 14 files changed, 481 insertions(+), 448 deletions(-) delete mode 100644 site/src/pages/GitAuthDevicePage/GitAuthDevicePage.tsx delete mode 100644 site/src/pages/GitAuthPage/GitAuthPage.stories.tsx create mode 100644 site/src/pages/GitAuthPage/GitAuthPageView.stories.tsx create mode 100644 site/src/pages/GitAuthPage/GitAuthPageView.tsx diff --git a/coderd/gitauth.go b/coderd/gitauth.go index 4ed4b2f6f9604..2a2ffd55e0852 100644 --- a/coderd/gitauth.go +++ b/coderd/gitauth.go @@ -52,6 +52,10 @@ func (api *API) gitAuthByID(w http.ResponseWriter, r *http.Request) { } } + if res.AppInstallations == nil { + res.AppInstallations = []codersdk.GitAuthAppInstallation{} + } + httpapi.Write(ctx, w, http.StatusOK, res) } diff --git a/coderd/gitauth/config.go b/coderd/gitauth/config.go index c0b79842ea48c..4e12befea7a17 100644 --- a/coderd/gitauth/config.go +++ b/coderd/gitauth/config.go @@ -180,20 +180,21 @@ func (c *Config) AppInstallations(ctx context.Context, token string) ([]codersdk return nil, false, err } for _, installation := range ghInstalls.Installations { - install := codersdk.GitAuthAppInstallation{ + + account := installation.GetAccount() + if account == nil { + continue + } + installs = append(installs, codersdk.GitAuthAppInstallation{ ID: int(installation.GetID()), ConfigureURL: installation.GetHTMLURL(), - } - account := installation.GetAccount() - if account != nil { - install.Account = &codersdk.GitAuthUser{ + Account: codersdk.GitAuthUser{ Login: account.GetLogin(), AvatarURL: account.GetAvatarURL(), ProfileURL: account.GetHTMLURL(), Name: account.GetName(), - } - } - installs = append(installs, install) + }, + }) } } return installs, true, nil diff --git a/codersdk/gitauth.go b/codersdk/gitauth.go index 1b52a245e7321..c8259771207b8 100644 --- a/codersdk/gitauth.go +++ b/codersdk/gitauth.go @@ -23,9 +23,9 @@ type GitAuth struct { } type GitAuthAppInstallation struct { - ID int `json:"id"` - Account *GitAuthUser `json:"account"` - ConfigureURL string `json:"configure_url"` + ID int `json:"id"` + Account GitAuthUser `json:"account"` + ConfigureURL string `json:"configure_url"` } type GitAuthUser struct { diff --git a/site/.storybook/preview.jsx b/site/.storybook/preview.jsx index 840e232076a3c..b84e9d25caa48 100644 --- a/site/.storybook/preview.jsx +++ b/site/.storybook/preview.jsx @@ -1,7 +1,6 @@ import CssBaseline from "@mui/material/CssBaseline" import { StyledEngineProvider, ThemeProvider } from "@mui/material/styles" - -import { MemoryRouter } from "react-router-dom" +import { withRouter } from "storybook-addon-react-router-v6" import { HelmetProvider } from "react-helmet-async" import { dark } from "../src/theme" import "../src/theme/globalFonts" @@ -16,13 +15,7 @@ export const decorators = [ ), - (Story) => { - return ( - - - - ) - }, + withRouter, (Story) => { return ( diff --git a/site/package.json b/site/package.json index 13bb643ffcd87..32e3681340bdc 100644 --- a/site/package.json +++ b/site/package.json @@ -149,6 +149,7 @@ "resize-observer": "1.0.4", "semver": "7.3.7", "storybook": "7.0.4", + "storybook-addon-react-router-v6": "1.0.2", "storybook-react-context": "0.6.0", "ts-proto": "1.150.0", "typescript": "4.8.2", diff --git a/site/src/AppRouter.tsx b/site/src/AppRouter.tsx index 168d13e2f4b10..df8a0c36b7be0 100644 --- a/site/src/AppRouter.tsx +++ b/site/src/AppRouter.tsx @@ -115,7 +115,7 @@ const NetworkSettingsPage = lazy( "./pages/DeploySettingsPage/NetworkSettingsPage/NetworkSettingsPage" ), ) -const GitAuthPage = lazy(() => import("./pages/GitAuthPage/GitAuthPage")) +const GitAuthPage = lazy(() => import("./pages/GitAuthPage/GitAuthPageView")) const GitAuthDevicePage = lazy( () => import("./pages/GitAuthDevicePage/GitAuthDevicePage"), ) diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 4c8b5bf1da181..22e9393dfdbfe 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -428,7 +428,7 @@ export interface GitAuth { // From codersdk/gitauth.go export interface GitAuthAppInstallation { readonly id: number - readonly account?: GitAuthUser + readonly account: GitAuthUser readonly configure_url: string } diff --git a/site/src/components/SignInLayout/SignInLayout.tsx b/site/src/components/SignInLayout/SignInLayout.tsx index 022bda5fe8be5..080274ca00c5e 100644 --- a/site/src/components/SignInLayout/SignInLayout.tsx +++ b/site/src/components/SignInLayout/SignInLayout.tsx @@ -14,7 +14,6 @@ export const useStyles = makeStyles((theme) => ({ alignItems: "center", }, container: { - marginTop: theme.spacing(-8), maxWidth: 385, display: "flex", flexDirection: "column", diff --git a/site/src/pages/GitAuthDevicePage/GitAuthDevicePage.tsx b/site/src/pages/GitAuthDevicePage/GitAuthDevicePage.tsx deleted file mode 100644 index 41744c1595c41..0000000000000 --- a/site/src/pages/GitAuthDevicePage/GitAuthDevicePage.tsx +++ /dev/null @@ -1,157 +0,0 @@ -import OpenInNewIcon from "@mui/icons-material/OpenInNew" -import CircularProgress from "@mui/material/CircularProgress" -import Link from "@mui/material/Link" -import { makeStyles } from "@mui/styles" -import { useQuery } from "@tanstack/react-query" -import { exchangeGitAuthDevice } from "api/api" -import { isAxiosError } from "axios" -import { Alert } from "components/Alert/Alert" -import { CopyButton } from "components/CopyButton/CopyButton" -import { SignInLayout } from "components/SignInLayout/SignInLayout" -import { Welcome } from "components/Welcome/Welcome" -import { FC, useEffect, useState } from "react" -import { useNavigate, useParams, useSearchParams } from "react-router-dom" - -const GitAuthDevicePage: FC = () => { - const styles = useStyles() - const { provider } = useParams() - const [searchParams, setSearchParams] = useSearchParams() - const navigate = useNavigate() - const [deviceCode] = useState(() => searchParams.get("device_code") || "") - const [userCode] = useState(() => searchParams.get("user_code") || "") - const [verificationUri] = useState( - () => searchParams.get("verification_uri") || "", - ) - const [interval] = useState( - () => Number.parseInt(searchParams.get("interval") || "") || 5, - ) - - useEffect(() => { - // This will clear the query parameters. It's a nice UX, because - // then a user can reload the page to obtain a new device code in - // case the current one expires! - if (deviceCode) { - setSearchParams({}) - } - }, [deviceCode, setSearchParams, navigate]) - - const exchange = useQuery({ - queryFn: () => - exchangeGitAuthDevice(provider as string, { device_code: deviceCode }), - queryKey: ["gitauth", provider as string, deviceCode], - retry: true, - retryDelay: interval * 1000, - refetchOnWindowFocus: (query) => - query.state.status === "success" ? false : "always", - }) - - useEffect(() => { - if (!exchange.isSuccess) { - return - } - // Redirect to `/gitauth` to notify any listeners that the - // authentication was successful. - navigate("/gitauth") - }, [exchange.isSuccess, navigate]) - - let status = ( -

- - Checking for authentication... -

- ) - if (isAxiosError(exchange.failureReason)) { - // See https://datatracker.ietf.org/doc/html/rfc8628#section-3.5 - switch (exchange.failureReason.response?.data?.detail) { - case "authorization_pending": - break - case "expired_token": - status = ( - - The one-time code has expired. Refresh to get a new one! - - ) - break - case "access_denied": - status = ( - Access to the Git provider was denied. - ) - break - } - } - - return ( - - - -

- Copy your one-time code:  -

- {userCode} {" "} - -
-
- Then open the link below and paste it: -

- - - Open and Paste - - {status} - - ) -} - -export default GitAuthDevicePage - -const useStyles = makeStyles((theme) => ({ - title: { - fontSize: theme.spacing(4), - fontWeight: 400, - lineHeight: "140%", - margin: 0, - }, - - text: { - fontSize: 16, - color: theme.palette.text.secondary, - marginBottom: 0, - textAlign: "center", - lineHeight: "160%", - }, - - copyCode: { - display: "inline-flex", - alignItems: "center", - }, - - code: { - fontWeight: "bold", - color: theme.palette.text.primary, - }, - - lineBreak: { - whiteSpace: "nowrap", - }, - - link: { - display: "flex", - alignItems: "center", - justifyContent: "center", - fontSize: 16, - gap: theme.spacing(1), - margin: theme.spacing(2, 0), - }, - - status: { - display: "flex", - alignItems: "center", - gap: theme.spacing(1), - color: theme.palette.text.disabled, - }, -})) diff --git a/site/src/pages/GitAuthPage/GitAuthPage.stories.tsx b/site/src/pages/GitAuthPage/GitAuthPage.stories.tsx deleted file mode 100644 index 422bf964de9bc..0000000000000 --- a/site/src/pages/GitAuthPage/GitAuthPage.stories.tsx +++ /dev/null @@ -1,12 +0,0 @@ -import { ComponentMeta, Story } from "@storybook/react" -import GitAuthPage from "./GitAuthPage" - -export default { - title: "pages/GitAuthPage", - component: GitAuthPage, -} as ComponentMeta - -const Template: Story = (args) => - -export const Default = Template.bind({}) -Default.args = {} diff --git a/site/src/pages/GitAuthPage/GitAuthPage.tsx b/site/src/pages/GitAuthPage/GitAuthPage.tsx index 9e5f7fc349b20..b0469f62af609 100644 --- a/site/src/pages/GitAuthPage/GitAuthPage.tsx +++ b/site/src/pages/GitAuthPage/GitAuthPage.tsx @@ -1,302 +1,100 @@ -import RefreshIcon from "@mui/icons-material/Refresh" -import OpenInNewIcon from "@mui/icons-material/OpenInNew" -import CircularProgress from "@mui/material/CircularProgress" -import Link from "@mui/material/Link" -import Tooltip from "@mui/material/Tooltip" -import { makeStyles } from "@mui/styles" import { useQuery, useQueryClient } from "@tanstack/react-query" import { exchangeGitAuthDevice, getGitAuthDevice, getGitAuthProvider, } from "api/api" -import { isAxiosError } from "axios" -import { Alert } from "components/Alert/Alert" -import { Avatar } from "components/Avatar/Avatar" -import { CopyButton } from "components/CopyButton/CopyButton" -import { SignInLayout } from "components/SignInLayout/SignInLayout" -import { Welcome } from "components/Welcome/Welcome" -import { FC, useEffect } from "react" -import { useParams } from "react-router-dom" -import { REFRESH_GITAUTH_BROADCAST_CHANNEL } from "xServices/createWorkspace/createWorkspaceXService" import { usePermissions } from "hooks" +import { useEffect } from "react" +import { redirect, useParams } from "react-router-dom" +import { REFRESH_GITAUTH_BROADCAST_CHANNEL } from "xServices/createWorkspace/createWorkspaceXService" +import GitAuthPageView from "./GitAuthPageView" +import { ApiErrorResponse } from "api/errors" +import { isAxiosError } from "axios" -const GitAuthPage: FC = () => { - const styles = useStyles() +const GitAuthPage = () => { const { provider } = useParams() if (!provider) { throw new Error("provider must exist") } const permissions = usePermissions() const queryClient = useQueryClient() - const query = useQuery({ + const getGitAuthProviderQuery = useQuery({ queryKey: ["gitauth", provider], queryFn: () => getGitAuthProvider(provider), refetchOnWindowFocus: true, }) - useEffect(() => { - if (!query.data?.authenticated) { - return - } - // This is used to notify the parent window that the Git auth token has been refreshed. - // It's critical in the create workspace flow! - // eslint-disable-next-line compat/compat -- It actually is supported... not sure why it's complaining. - const bc = new BroadcastChannel(REFRESH_GITAUTH_BROADCAST_CHANNEL) - // The message doesn't matter, any message refreshes the page! - bc.postMessage("noop") - }, [query.data?.authenticated]) - - if (query.isLoading || !query.data) { - return null - } - - if (!query.data.authenticated) { - return ( - - - - {query.data.device && } - - ) - } - - const hasInstallations = query.data.installations?.length > 0 - - return ( - - -

- Hey @{query.data.user?.login} 👋! You are now authenticated with Git. - Feel free to close this window! -

- - {query.data.installations?.length !== 0 && ( -
- {query.data.installations.map((install) => { - if (!install.account) { - return - } - return ( - - - - {install.account.login} - - - - ) - })} -   - {query.data.installations.length} organization - {query.data.installations.length !== 1 && "s are"} authorized -
- )} - -
- {!hasInstallations && query.data.app_installable && ( - - You must{" "} - - install the {query.data.type} App - {" "} - to clone private repositories. Accounts will appear here once - authorized. - - )} - - {permissions.viewGitAuthConfig && - query.data.app_install_url && - query.data.app_installable && ( - - - {query.data.installations?.length - ? "Configure" - : "Install"} the {query.data.type} App - - )} - { - queryClient.setQueryData(["gitauth", provider], { - ...query.data, - authenticated: false, - }) - }} - > - Reauthenticate - -
- - ) -} - -const GitDeviceAuth: FC<{ - provider: string -}> = ({ provider }) => { - const styles = useStyles() - const device = useQuery({ + const getGitAuthDeviceQuery = useQuery({ + enabled: + Boolean(!getGitAuthProviderQuery.data?.authenticated) && + Boolean(getGitAuthProviderQuery.data?.device), queryFn: () => getGitAuthDevice(provider), queryKey: ["gitauth", provider, "device"], refetchOnMount: false, }) - - const client = useQueryClient() - const exchange = useQuery({ + const exchangeGitAuthDeviceQuery = useQuery({ queryFn: () => exchangeGitAuthDevice(provider, { - device_code: device.data?.device_code || "", + device_code: getGitAuthDeviceQuery.data?.device_code || "", }), - queryKey: ["gitauth", provider, device.data?.device_code], - enabled: Boolean(device.data), + queryKey: ["gitauth", provider, getGitAuthDeviceQuery.data?.device_code], + enabled: Boolean(getGitAuthDeviceQuery.data), onSuccess: () => { // Force a refresh of the Git auth status. - client.invalidateQueries(["gitauth", provider]).catch((ex) => { + queryClient.invalidateQueries(["gitauth", provider]).catch((ex) => { console.error("invalidate queries", ex) }) }, retry: true, - retryDelay: (device.data?.interval || 5) * 1000, + retryDelay: (getGitAuthDeviceQuery.data?.interval || 5) * 1000, refetchOnWindowFocus: (query) => query.state.status === "success" ? false : "always", }) - let status = ( -

- - Checking for authentication... -

- ) - if (isAxiosError(exchange.failureReason)) { - // See https://datatracker.ietf.org/doc/html/rfc8628#section-3.5 - switch (exchange.failureReason.response?.data?.detail) { - case "authorization_pending": - break - case "expired_token": - status = ( - - The one-time code has expired. Refresh to get a new one! - - ) - break - case "access_denied": - status = ( - Access to the Git provider was denied. - ) - break - default: - status = ( - - An unknown error occurred. Please try again:{" "} - {exchange.failureReason.message} - - ) - break + useEffect(() => { + if (!getGitAuthProviderQuery.data?.authenticated) { + return } + // This is used to notify the parent window that the Git auth token has been refreshed. + // It's critical in the create workspace flow! + // eslint-disable-next-line compat/compat -- It actually is supported... not sure why it's complaining. + const bc = new BroadcastChannel(REFRESH_GITAUTH_BROADCAST_CHANNEL) + // The message doesn't matter, any message refreshes the page! + bc.postMessage("noop") + }, [getGitAuthProviderQuery.data?.authenticated]) + + if (getGitAuthProviderQuery.isLoading || !getGitAuthProviderQuery.data) { + return null } - if (!device.data) { - return + let deviceExchangeError: ApiErrorResponse | undefined + if (isAxiosError(exchangeGitAuthDeviceQuery.failureReason)) { + deviceExchangeError = + exchangeGitAuthDeviceQuery.failureReason.response?.data } - return ( -
-

- Copy your one-time code:  -

- {device.data.user_code} {" "} - -
-
- Then open the link below and paste it: -

-
- - - Open and Paste - -
+ if ( + !getGitAuthProviderQuery.data.authenticated && + !getGitAuthProviderQuery.data.device + ) { + return redirect(`/gitauth/${provider}/callback`) + } - {status} -
+ return ( + { + queryClient.setQueryData(["gitauth", provider], { + ...getGitAuthProviderQuery.data, + authenticated: false, + }) + }} + viewGitAuthConfig={permissions.viewGitAuthConfig} + deviceExchangeError={deviceExchangeError} + gitAuthDevice={getGitAuthDeviceQuery.data} + /> ) } export default GitAuthPage - -const useStyles = makeStyles((theme) => ({ - text: { - fontSize: 16, - color: theme.palette.text.secondary, - textAlign: "center", - lineHeight: "160%", - margin: 0, - }, - - copyCode: { - display: "inline-flex", - alignItems: "center", - }, - - code: { - fontWeight: "bold", - color: theme.palette.text.primary, - }, - - installAlert: { - margin: theme.spacing(2), - }, - - links: { - display: "flex", - gap: theme.spacing(0.5), - margin: theme.spacing(2), - flexDirection: "column", - }, - - link: { - display: "flex", - alignItems: "center", - justifyContent: "center", - fontSize: 16, - gap: theme.spacing(1), - }, - - status: { - display: "flex", - alignItems: "center", - justifyContent: "center", - gap: theme.spacing(1), - color: theme.palette.text.disabled, - }, - - authorizedInstalls: { - display: "flex", - gap: 4, - color: theme.palette.text.disabled, - margin: theme.spacing(4), - }, -})) diff --git a/site/src/pages/GitAuthPage/GitAuthPageView.stories.tsx b/site/src/pages/GitAuthPage/GitAuthPageView.stories.tsx new file mode 100644 index 0000000000000..4f0a9dc45be6c --- /dev/null +++ b/site/src/pages/GitAuthPage/GitAuthPageView.stories.tsx @@ -0,0 +1,119 @@ +import { Meta, StoryFn } from "@storybook/react" +import GitAuthPageView, { GitAuthPageViewProps } from "./GitAuthPageView" + +export default { + title: "pages/GitAuthPageView", + component: GitAuthPageView, +} as Meta + +const Template: StoryFn = (args) => ( + +) + +export const WebAuthenticated = Template.bind({}) +WebAuthenticated.args = { + gitAuth: { + type: "BitBucket", + authenticated: true, + device: false, + installations: [], + app_install_url: "", + app_installable: false, + user: { + avatar_url: "", + login: "kylecarbs", + name: "Kyle Carberry", + profile_url: "", + }, + }, +} + +export const DeviceUnauthenticated = Template.bind({}) +DeviceUnauthenticated.args = { + gitAuth: { + type: "GitHub", + authenticated: false, + device: true, + installations: [], + app_install_url: "", + app_installable: false, + }, + gitAuthDevice: { + device_code: "1234-5678", + expires_in: 900, + interval: 5, + user_code: "ABCD-EFGH", + verification_uri: "", + }, +} + +export const DeviceUnauthenticatedError = Template.bind({}) +DeviceUnauthenticatedError.args = { + gitAuth: { + type: "GitHub", + authenticated: false, + device: true, + installations: [], + app_install_url: "", + app_installable: false, + }, + gitAuthDevice: { + device_code: "1234-5678", + expires_in: 900, + interval: 5, + user_code: "ABCD-EFGH", + verification_uri: "", + }, + deviceExchangeError: { + message: "Error exchanging device code.", + detail: "expired_token", + }, +} + +export const DeviceAuthenticatedNotInstalled = Template.bind({}) +DeviceAuthenticatedNotInstalled.args = { + viewGitAuthConfig: true, + gitAuth: { + type: "GitHub", + authenticated: true, + device: true, + installations: [], + app_install_url: "https://example.com", + app_installable: true, + user: { + avatar_url: "", + login: "kylecarbs", + name: "Kyle Carberry", + profile_url: "", + }, + }, +} + +export const DeviceAuthenticatedInstalled = Template.bind({}) +DeviceAuthenticatedInstalled.args = { + gitAuth: { + type: "GitHub", + authenticated: true, + device: true, + installations: [ + { + configure_url: "https://example.com", + id: 1, + account: { + avatar_url: "https://github.com/coder.png", + login: "coder", + name: "Coder", + profile_url: "https://github.com/coder", + }, + }, + ], + app_install_url: "https://example.com", + app_installable: true, + user: { + avatar_url: "", + login: "kylecarbs", + name: "Kyle Carberry", + profile_url: "", + }, + }, +} diff --git a/site/src/pages/GitAuthPage/GitAuthPageView.tsx b/site/src/pages/GitAuthPage/GitAuthPageView.tsx new file mode 100644 index 0000000000000..9815a6cd01efe --- /dev/null +++ b/site/src/pages/GitAuthPage/GitAuthPageView.tsx @@ -0,0 +1,275 @@ +import OpenInNewIcon from "@mui/icons-material/OpenInNew" +import RefreshIcon from "@mui/icons-material/Refresh" +import Button from "@mui/material/Button" +import CircularProgress from "@mui/material/CircularProgress" +import Link from "@mui/material/Link" +import Tooltip from "@mui/material/Tooltip" +import { makeStyles } from "@mui/styles" +import { ApiErrorResponse } from "api/errors" +import { GitAuth, GitAuthDevice } from "api/typesGenerated" +import { Alert } from "components/Alert/Alert" +import { Avatar } from "components/Avatar/Avatar" +import { CopyButton } from "components/CopyButton/CopyButton" +import { SignInLayout } from "components/SignInLayout/SignInLayout" +import { Welcome } from "components/Welcome/Welcome" +import { FC, useEffect } from "react" +import { REFRESH_GITAUTH_BROADCAST_CHANNEL } from "xServices/createWorkspace/createWorkspaceXService" + +export interface GitAuthPageViewProps { + gitAuth: GitAuth + viewGitAuthConfig: boolean + + gitAuthDevice?: GitAuthDevice + deviceExchangeError?: ApiErrorResponse + + onReauthenticate: () => void +} + +const GitAuthPageView: FC = ({ + deviceExchangeError, + gitAuth, + gitAuthDevice, + onReauthenticate, + viewGitAuthConfig, +}) => { + const styles = useStyles() + + useEffect(() => { + if (!gitAuth.authenticated) { + return + } + // This is used to notify the parent window that the Git auth token has been refreshed. + // It's critical in the create workspace flow! + // eslint-disable-next-line compat/compat -- It actually is supported... not sure why it's complaining. + const bc = new BroadcastChannel(REFRESH_GITAUTH_BROADCAST_CHANNEL) + // The message doesn't matter, any message refreshes the page! + bc.postMessage("noop") + }, [gitAuth.authenticated]) + + if (!gitAuth.authenticated) { + return ( + + + + {gitAuth.device && ( + + )} + + ) + } + + const hasInstallations = gitAuth.installations.length > 0 + + return ( + + +

+ Hey @{gitAuth.user?.login} 👋! You are now authenticated with Git. Feel + free to close this window! +

+ + {gitAuth.installations.length > 0 && ( +
+ {gitAuth.installations.map((install) => { + if (!install.account) { + return + } + return ( + + + + {install.account.login} + + + + ) + })} +   + {gitAuth.installations.length} organization + {gitAuth.installations.length !== 1 && "s are"} authorized +
+ )} + +
+ {!hasInstallations && gitAuth.app_installable && ( + + You must{" "} + + install the {gitAuth.type} App + {" "} + to clone private repositories. Accounts will appear here once + authorized. + + )} + + {viewGitAuthConfig && + gitAuth.app_install_url && + gitAuth.app_installable && ( + + + {gitAuth.installations.length > 0 + ? "Configure" + : "Install"} the {gitAuth.type} App + + )} + { + onReauthenticate() + }} + > + Reauthenticate + +
+ + ) +} + +const GitDeviceAuth: FC<{ + gitAuthDevice?: GitAuthDevice + deviceExchangeError?: ApiErrorResponse +}> = ({ gitAuthDevice, deviceExchangeError }) => { + const styles = useStyles() + + let status = ( +

+ + Checking for authentication... +

+ ) + if (deviceExchangeError) { + // See https://datatracker.ietf.org/doc/html/rfc8628#section-3.5 + switch (deviceExchangeError.detail) { + case "authorization_pending": + break + case "expired_token": + status = ( + + The one-time code has expired. Refresh to get a new one! + + ) + break + case "access_denied": + status = ( + Access to the Git provider was denied. + ) + break + default: + status = ( + + An unknown error occurred. Please try again:{" "} + {deviceExchangeError.message} + + ) + break + } + } + + if (!gitAuthDevice) { + return + } + + return ( +
+

+ Copy your one-time code:  +

+ {gitAuthDevice.user_code} {" "} + +
+
+ Then open the link below and paste it: +

+
+ + + Open and Paste + +
+ + {status} +
+ ) +} + +export default GitAuthPageView + +const useStyles = makeStyles((theme) => ({ + text: { + fontSize: 16, + color: theme.palette.text.secondary, + textAlign: "center", + lineHeight: "160%", + margin: 0, + }, + + copyCode: { + display: "inline-flex", + alignItems: "center", + }, + + code: { + fontWeight: "bold", + color: theme.palette.text.primary, + }, + + installAlert: { + margin: theme.spacing(2), + }, + + links: { + display: "flex", + gap: theme.spacing(0.5), + margin: theme.spacing(2), + flexDirection: "column", + }, + + link: { + display: "flex", + alignItems: "center", + justifyContent: "center", + fontSize: 16, + gap: theme.spacing(1), + }, + + status: { + display: "flex", + alignItems: "center", + justifyContent: "center", + gap: theme.spacing(1), + color: theme.palette.text.disabled, + }, + + authorizedInstalls: { + display: "flex", + gap: 4, + color: theme.palette.text.disabled, + margin: theme.spacing(4), + }, +})) diff --git a/site/yarn.lock b/site/yarn.lock index 9db5108cb7f19..89ad84ef3c25c 100644 --- a/site/yarn.lock +++ b/site/yarn.lock @@ -10061,6 +10061,11 @@ react-inspector@^6.0.0: resolved "https://registry.yarnpkg.com/react-inspector/-/react-inspector-6.0.1.tgz#1a37f0165d9df81ee804d63259eaaeabe841287d" integrity sha512-cxKSeFTf7jpSSVddm66sKdolG90qURAX3g1roTeaN6x0YEbtWc8JpmFN9+yIqLNH2uEkYerWLtJZIXRIFuBKrg== +react-inspector@^6.0.1: + version "6.0.2" + resolved "https://registry.yarnpkg.com/react-inspector/-/react-inspector-6.0.2.tgz#aa3028803550cb6dbd7344816d5c80bf39d07e9d" + integrity sha512-x+b7LxhmHXjHoU/VrFAzw5iutsILRoYyDq97EDYdFpPLcvqtEzk4ZSZSQjnFPbr5T57tLXnHcqFYoN1pI6u8uQ== + react-is@18.1.0: version "18.1.0" resolved "https://registry.yarnpkg.com/react-is/-/react-is-18.1.0.tgz#61aaed3096d30eacf2a2127118b5b41387d32a67" @@ -10986,6 +10991,13 @@ store2@^2.12.0, store2@^2.14.2: resolved "https://registry.yarnpkg.com/store2/-/store2-2.14.2.tgz#56138d200f9fe5f582ad63bc2704dbc0e4a45068" integrity sha512-siT1RiqlfQnGqgT/YzXVUNsom9S0H1OX+dpdGN1xkyYATo4I6sep5NmsRD/40s3IIOvlCq6akxkqG82urIZW1w== +storybook-addon-react-router-v6@1.0.2: + version "1.0.2" + resolved "https://registry.yarnpkg.com/storybook-addon-react-router-v6/-/storybook-addon-react-router-v6-1.0.2.tgz#0f1239de31291821c93e8266079b8f6235a9c717" + integrity sha512-38W+9D2sIrYAi+oRSbsLhR/umNoLVw2DWF84Jp4f/ZoB8Cg0Qtbvwk043oHqzNOpZrfgj0FaV006oaJBVpE8Kw== + dependencies: + react-inspector "^6.0.1" + storybook-react-context@0.6.0: version "0.6.0" resolved "https://registry.yarnpkg.com/storybook-react-context/-/storybook-react-context-0.6.0.tgz#06c7b48dc95f4619cf12e59429305fbd6f2b1373" From 8933b493582ecb463fa23cd72d725ef9c1756c44 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Wed, 28 Jun 2023 00:49:26 +0000 Subject: [PATCH 06/17] Fix sideways layout --- site/src/AppRouter.tsx | 5 +---- site/src/components/Dashboard/DashboardLayout.tsx | 1 + site/src/pages/GitAuthPage/GitAuthPage.tsx | 6 +++--- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/site/src/AppRouter.tsx b/site/src/AppRouter.tsx index df8a0c36b7be0..472be48b4de97 100644 --- a/site/src/AppRouter.tsx +++ b/site/src/AppRouter.tsx @@ -115,10 +115,7 @@ const NetworkSettingsPage = lazy( "./pages/DeploySettingsPage/NetworkSettingsPage/NetworkSettingsPage" ), ) -const GitAuthPage = lazy(() => import("./pages/GitAuthPage/GitAuthPageView")) -const GitAuthDevicePage = lazy( - () => import("./pages/GitAuthDevicePage/GitAuthDevicePage"), -) +const GitAuthPage = lazy(() => import("./pages/GitAuthPage/GitAuthPage")) const TemplateVersionPage = lazy( () => import("./pages/TemplateVersionPage/TemplateVersionPage"), ) diff --git a/site/src/components/Dashboard/DashboardLayout.tsx b/site/src/components/Dashboard/DashboardLayout.tsx index cf85fa65631a1..72b9f910f66c0 100644 --- a/site/src/components/Dashboard/DashboardLayout.tsx +++ b/site/src/components/Dashboard/DashboardLayout.tsx @@ -111,5 +111,6 @@ const useStyles = makeStyles({ flex: 1, paddingBottom: dashboardContentBottomPadding, // Add bottom space since we don't use a footer display: "flex", + flexDirection: "column", }, }) diff --git a/site/src/pages/GitAuthPage/GitAuthPage.tsx b/site/src/pages/GitAuthPage/GitAuthPage.tsx index b0469f62af609..bb31be8c8c42a 100644 --- a/site/src/pages/GitAuthPage/GitAuthPage.tsx +++ b/site/src/pages/GitAuthPage/GitAuthPage.tsx @@ -5,14 +5,14 @@ import { getGitAuthProvider, } from "api/api" import { usePermissions } from "hooks" -import { useEffect } from "react" +import { FC, useEffect } from "react" import { redirect, useParams } from "react-router-dom" import { REFRESH_GITAUTH_BROADCAST_CHANNEL } from "xServices/createWorkspace/createWorkspaceXService" import GitAuthPageView from "./GitAuthPageView" import { ApiErrorResponse } from "api/errors" import { isAxiosError } from "axios" -const GitAuthPage = () => { +const GitAuthPage: FC = () => { const { provider } = useParams() if (!provider) { throw new Error("provider must exist") @@ -78,7 +78,7 @@ const GitAuthPage = () => { !getGitAuthProviderQuery.data.authenticated && !getGitAuthProviderQuery.data.device ) { - return redirect(`/gitauth/${provider}/callback`) + redirect(`/gitauth/${provider}/callback`) } return ( From 18022ba60b51f76c8bea27d3fc5fe9fb3bac5791 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Wed, 28 Jun 2023 00:55:26 +0000 Subject: [PATCH 07/17] Remove legacy notify --- cli/cliui/gitauth_test.go | 3 +-- coderd/templateversions.go | 8 -------- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/cli/cliui/gitauth_test.go b/cli/cliui/gitauth_test.go index 13310ab85ffda..dfe142f99be28 100644 --- a/cli/cliui/gitauth_test.go +++ b/cli/cliui/gitauth_test.go @@ -2,7 +2,6 @@ package cliui_test import ( "context" - "net/url" "sync/atomic" "testing" "time" @@ -33,7 +32,7 @@ func TestGitAuth(t *testing.T) { ID: "github", Type: codersdk.GitProviderGitHub, Authenticated: fetched.Load(), - AuthenticateURL: "https://example.com/gitauth/github?redirect=" + url.QueryEscape("/gitauth?notify"), + AuthenticateURL: "https://example.com/gitauth/github", }}, nil }, FetchInterval: time.Millisecond, diff --git a/coderd/templateversions.go b/coderd/templateversions.go index 7af3029d83e12..df5ae1793ed8b 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -320,14 +320,6 @@ func (api *API) templateVersionGitAuth(rw http.ResponseWriter, r *http.Request) }) return } - query := redirectURL.Query() - // The frontend uses a BroadcastChannel to notify listening pages for - // Git auth updates if the "notify" query parameter is set. - // - // It's important we do this in the backend, because the same endpoint - // is used for CLI authentication. - query.Add("redirect", "/gitauth?notify") - redirectURL.RawQuery = query.Encode() provider := codersdk.TemplateVersionGitAuth{ ID: config.ID, From fb5264c5e84edfeb9c4ec738c0dce0ad53d6cada Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Wed, 28 Jun 2023 14:49:29 +0000 Subject: [PATCH 08/17] Fix git auth redirects --- coderd/coderd.go | 4 ++-- coderd/gitauth/oauth.go | 14 +++++++++-- site/src/pages/GitAuthPage/GitAuthPage.tsx | 4 +++- .../src/pages/GitAuthPage/GitAuthPageView.tsx | 23 +++++++++++++------ 4 files changed, 33 insertions(+), 12 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index 09911d1f88c44..d76c691128af7 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -462,12 +462,12 @@ func New(options *Options) *API { if gitAuthConfig.DeviceAuth != nil { continue } - r.Route(fmt.Sprintf("/%s", gitAuthConfig.ID), func(r chi.Router) { + r.Route(fmt.Sprintf("/%s/callback", gitAuthConfig.ID), func(r chi.Router) { r.Use( apiKeyMiddlewareRedirect, httpmw.ExtractOAuth2(gitAuthConfig, options.HTTPClient, nil), ) - r.Get("/callback", api.gitAuthCallback(gitAuthConfig)) + r.Get("/", api.gitAuthCallback(gitAuthConfig)) }) } }) diff --git a/coderd/gitauth/oauth.go b/coderd/gitauth/oauth.go index 6374556fe97e1..6756517347539 100644 --- a/coderd/gitauth/oauth.go +++ b/coderd/gitauth/oauth.go @@ -121,8 +121,18 @@ func (c *DeviceAuth) AuthorizeDevice(ctx context.Context) (*codersdk.GitAuthDevi return nil, err } defer resp.Body.Close() - var da codersdk.GitAuthDevice - return &da, json.NewDecoder(resp.Body).Decode(&da) + var r struct { + codersdk.GitAuthDevice + ErrorDescription string `json:"error_description"` + } + err = json.NewDecoder(resp.Body).Decode(&r) + if err != nil { + return nil, err + } + if r.ErrorDescription != "" { + return nil, xerrors.Errorf("%s", r.ErrorDescription) + } + return &r.GitAuthDevice, nil } type ExchangeDeviceCodeResponse struct { diff --git a/site/src/pages/GitAuthPage/GitAuthPage.tsx b/site/src/pages/GitAuthPage/GitAuthPage.tsx index bb31be8c8c42a..5a7f16f0aa066 100644 --- a/site/src/pages/GitAuthPage/GitAuthPage.tsx +++ b/site/src/pages/GitAuthPage/GitAuthPage.tsx @@ -78,7 +78,9 @@ const GitAuthPage: FC = () => { !getGitAuthProviderQuery.data.authenticated && !getGitAuthProviderQuery.data.device ) { - redirect(`/gitauth/${provider}/callback`) + window.location.href = `/gitauth/${provider}/callback` + + return null } return ( diff --git a/site/src/pages/GitAuthPage/GitAuthPageView.tsx b/site/src/pages/GitAuthPage/GitAuthPageView.tsx index 9815a6cd01efe..3b8155a0dfcfc 100644 --- a/site/src/pages/GitAuthPage/GitAuthPageView.tsx +++ b/site/src/pages/GitAuthPage/GitAuthPageView.tsx @@ -63,6 +63,21 @@ const GitAuthPageView: FC = ({ const hasInstallations = gitAuth.installations.length > 0 + // We only want to wrap this with a link if an install URL is available! + let installTheApp: JSX.Element = <>{`install the ${gitAuth.type} App`} + if (gitAuth.app_install_url) { + installTheApp = ( + + {installTheApp} + + ) + } + return ( @@ -105,13 +120,7 @@ const GitAuthPageView: FC = ({ {!hasInstallations && gitAuth.app_installable && ( You must{" "} - - install the {gitAuth.type} App - {" "} + {installTheApp}{" "} to clone private repositories. Accounts will appear here once authorized. From 2146bd202a171654ac8e7afc584a0513c4ff286f Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Wed, 28 Jun 2023 18:03:25 +0000 Subject: [PATCH 09/17] Add E2E tests --- cli/server.go | 4 +- coderd/gitauth/config.go | 11 +- codersdk/deployment.go | 2 +- site/e2e/constants.ts | 15 ++ site/e2e/helpers.ts | 32 +++++ site/e2e/playwright.config.ts | 51 ++++++- site/e2e/tests/gitAuth.spec.ts | 135 ++++++++++++++++++ site/package.json | 1 + .../src/pages/GitAuthPage/GitAuthPageView.tsx | 6 +- site/yarn.lock | 12 ++ 10 files changed, 251 insertions(+), 18 deletions(-) create mode 100644 site/e2e/tests/gitAuth.spec.ts diff --git a/cli/server.go b/cli/server.go index 07adc0f620eda..30d57020c1ecd 100644 --- a/cli/server.go +++ b/cli/server.go @@ -154,8 +154,8 @@ func ReadGitAuthProvidersFromEnv(environ []string) ([]codersdk.GitAuthConfig, er return nil, xerrors.Errorf("parse bool: %s", v.Value) } provider.DeviceFlow = b - case "DEVICE_AUTH_URL": - provider.DeviceAuthURL = v.Value + case "DEVICE_CODE_URL": + provider.DeviceCodeURL = v.Value case "NO_REFRESH": b, err := strconv.ParseBool(v.Value) if err != nil { diff --git a/coderd/gitauth/config.go b/coderd/gitauth/config.go index 4e12befea7a17..24635cd790fcf 100644 --- a/coderd/gitauth/config.go +++ b/coderd/gitauth/config.go @@ -239,9 +239,6 @@ func ConvertConfig(entries []codersdk.GitAuthConfig, accessURL *url.URL) ([]*Con if entry.ClientID == "" { return nil, xerrors.Errorf("%q git auth provider: client_id must be provided", entry.ID) } - if entry.ClientSecret == "" { - return nil, xerrors.Errorf("%q git auth provider: client_secret must be provided", entry.ID) - } authRedirect, err := accessURL.Parse(fmt.Sprintf("/gitauth/%s/callback", entry.ID)) if err != nil { return nil, xerrors.Errorf("parse gitauth callback url: %w", err) @@ -296,17 +293,17 @@ func ConvertConfig(entries []codersdk.GitAuthConfig, accessURL *url.URL) ([]*Con } if entry.DeviceFlow { - if entry.DeviceAuthURL == "" { - entry.DeviceAuthURL = deviceAuthURL[typ] + if entry.DeviceCodeURL == "" { + entry.DeviceCodeURL = deviceAuthURL[typ] } - if entry.DeviceAuthURL == "" { + if entry.DeviceCodeURL == "" { return nil, xerrors.Errorf("git auth provider %q: device auth url must be provided", entry.ID) } cfg.DeviceAuth = &DeviceAuth{ ClientID: entry.ClientID, TokenURL: oc.Endpoint.TokenURL, Scopes: entry.Scopes, - CodeURL: entry.DeviceAuthURL, + CodeURL: entry.DeviceCodeURL, } } diff --git a/codersdk/deployment.go b/codersdk/deployment.go index ee33f8aafcaba..c29770f628152 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -310,7 +310,7 @@ type GitAuthConfig struct { NoRefresh bool `json:"no_refresh"` Scopes []string `json:"scopes"` DeviceFlow bool `json:"device_flow"` - DeviceAuthURL string `json:"device_auth_url"` + DeviceCodeURL string `json:"device_code_url"` } type ProvisionerConfig struct { diff --git a/site/e2e/constants.ts b/site/e2e/constants.ts index 1e56c3d8b3b28..b141f637db62f 100644 --- a/site/e2e/constants.ts +++ b/site/e2e/constants.ts @@ -5,3 +5,18 @@ export const defaultPort = 3000 export const username = "admin" export const password = "SomeSecurePassword!" export const email = "admin@coder.com" + +export const gitAuth = { + deviceProvider: "device", + webProvider: "web", + // These ports need to be hardcoded so that they can be + // used in `playwright.config.ts` to set the environment + // variables for the server. + devicePort: 50515, + webPort: 50516, + + tokenPath: "/token", + codePath: "/code", + validatePath: "/validate", + installationsPath: "/installations", +} diff --git a/site/e2e/helpers.ts b/site/e2e/helpers.ts index 2c16aa606fba7..f9d37e3190eee 100644 --- a/site/e2e/helpers.ts +++ b/site/e2e/helpers.ts @@ -2,6 +2,7 @@ import { expect, Page } from "@playwright/test" import { spawn } from "child_process" import { randomUUID } from "crypto" import path from "path" +import express from "express" import { TarWriter } from "utils/tar" import { Agent, @@ -227,3 +228,34 @@ const createTemplateVersionTar = async ( const randomName = () => { return randomUUID().slice(0, 8) } + +// Awaiter is a helper that allows you to wait for a callback to be called. +// It is useful for waiting for events to occur. +export class Awaiter { + private promise: Promise + private callback?: () => void + + constructor() { + this.promise = new Promise((r) => (this.callback = r)) + } + + public done(): void { + if (this.callback) { + this.callback() + } else { + this.promise = Promise.resolve() + } + } + + public wait(): Promise { + return this.promise + } +} + +export const createServer = async ( + port: number, +): Promise> => { + const e = express() + await new Promise((r) => e.listen(port, r)) + return e +} diff --git a/site/e2e/playwright.config.ts b/site/e2e/playwright.config.ts index a8d2472b502cc..8dbd45e388f87 100644 --- a/site/e2e/playwright.config.ts +++ b/site/e2e/playwright.config.ts @@ -1,6 +1,6 @@ import { defineConfig } from "@playwright/test" import path from "path" -import { defaultPort } from "./constants" +import { defaultPort, gitAuth } from "./constants" export const port = process.env.CODER_E2E_PORT ? Number(process.env.CODER_E2E_PORT) @@ -10,7 +10,11 @@ const coderMain = path.join(__dirname, "../../enterprise/cmd/coder/main.go") export const STORAGE_STATE = path.join(__dirname, ".auth.json") -const config = defineConfig({ +const localURL = (port: number, path: string): string => { + return `http://localhost:${port}${path}` +} + +export default defineConfig({ projects: [ { name: "setup", @@ -39,9 +43,48 @@ const config = defineConfig({ `--provisioner-daemons 10 ` + `--provisioner-daemons-echo ` + `--provisioner-daemon-poll-interval 50ms`, + env: { + ...process.env, + + // This is the test provider for git auth with devices! + CODER_GITAUTH_0_ID: gitAuth.deviceProvider, + CODER_GITAUTH_0_TYPE: "github", + CODER_GITAUTH_0_CLIENT_ID: "client", + CODER_GITAUTH_0_DEVICE_FLOW: "true", + CODER_GITAUTH_0_APP_INSTALL_URL: + "https://github.com/apps/coder/installations/new", + CODER_GITAUTH_0_APP_INSTALLATIONS_URL: localURL( + gitAuth.devicePort, + gitAuth.installationsPath, + ), + CODER_GITAUTH_0_TOKEN_URL: localURL( + gitAuth.devicePort, + gitAuth.tokenPath, + ), + CODER_GITAUTH_0_DEVICE_CODE_URL: localURL( + gitAuth.devicePort, + gitAuth.codePath, + ), + CODER_GITAUTH_0_VALIDATE_URL: localURL( + gitAuth.devicePort, + gitAuth.validatePath, + ), + + CODER_GITAUTH_1_ID: gitAuth.webProvider, + CODER_GITAUTH_1_TYPE: "github", + CODER_GITAUTH_1_CLIENT_ID: "client", + CODER_GITAUTH_1_CLIENT_SECRET: "secret", + CODER_GITAUTH_1_TOKEN_URL: localURL(gitAuth.webPort, gitAuth.tokenPath), + CODER_GITAUTH_1_DEVICE_CODE_URL: localURL( + gitAuth.webPort, + gitAuth.codePath, + ), + CODER_GITAUTH_1_VALIDATE_URL: localURL( + gitAuth.webPort, + gitAuth.validatePath, + ), + }, port, reuseExistingServer: false, }, }) - -export default config diff --git a/site/e2e/tests/gitAuth.spec.ts b/site/e2e/tests/gitAuth.spec.ts new file mode 100644 index 0000000000000..1a99aa3ed0f0b --- /dev/null +++ b/site/e2e/tests/gitAuth.spec.ts @@ -0,0 +1,135 @@ +import { test } from "@playwright/test" +import { gitAuth } from "../constants" +import { Endpoints } from "@octokit/types" +import { GitAuthDevice } from "api/typesGenerated" +import { Awaiter, createServer } from "../helpers" + +test("git auth device", async ({ page }) => { + const device: GitAuthDevice = { + device_code: "1234", + user_code: "1234-5678", + expires_in: 900, + interval: 1, + verification_uri: "", + } + + const srv = await createServer(gitAuth.devicePort) + // The GitHub validate endpoint returns the currently authenticated user! + srv.use(gitAuth.validatePath, (req, res) => { + res.write(JSON.stringify(ghUser)) + res.end() + }) + srv.use(gitAuth.codePath, (req, res) => { + res.write(JSON.stringify(device)) + res.end() + }) + srv.use(gitAuth.installationsPath, (req, res) => { + res.write(JSON.stringify(ghInstall)) + res.end() + }) + + const token = { + access_token: "", + error: "authorization_pending", + error_description: "", + } + + const sentPending = new Awaiter() + srv.use(gitAuth.tokenPath, (req, res) => { + res.write(JSON.stringify(token)) + res.end() + sentPending.done() + }) + + await page.goto(`/gitauth/${gitAuth.deviceProvider}`, { + waitUntil: "networkidle", + }) + await page.getByText(device.user_code).isVisible() + await sentPending.wait() + token.error = "" + token.access_token = "hello-world" + await page.getByText("1 organization authorized").isVisible() +}) + +test("git auth web", async ({ page }) => { + const srv = await createServer(gitAuth.webPort) + // The GitHub validate endpoint returns the currently authenticated user! + srv.use(gitAuth.validatePath, (req, res) => { + res.write(JSON.stringify(ghUser)) + res.end() + }) + srv.use(gitAuth.installationsPath, (req, res) => { + res.write(JSON.stringify(ghInstall)) + res.end() + }) + srv.use(gitAuth.tokenPath, (req, res) => { + res.write(JSON.stringify({ access_token: "hello-world" })) + res.end() + }) + + await page.goto(`/gitauth/${gitAuth.webProvider}`, { + waitUntil: "networkidle", + }) + await page.getByText("1 organization authorized").isVisible() +}) + +const ghUser: Endpoints["GET /user"]["response"]["data"] = { + login: "kylecarbs", + id: 7122116, + node_id: "MDQ6VXNlcjcxMjIxMTY=", + avatar_url: "https://avatars.githubusercontent.com/u/7122116?v=4", + gravatar_id: "", + url: "https://api.github.com/users/kylecarbs", + html_url: "https://github.com/kylecarbs", + followers_url: "https://api.github.com/users/kylecarbs/followers", + following_url: + "https://api.github.com/users/kylecarbs/following{/other_user}", + gists_url: "https://api.github.com/users/kylecarbs/gists{/gist_id}", + starred_url: "https://api.github.com/users/kylecarbs/starred{/owner}{/repo}", + subscriptions_url: "https://api.github.com/users/kylecarbs/subscriptions", + organizations_url: "https://api.github.com/users/kylecarbs/orgs", + repos_url: "https://api.github.com/users/kylecarbs/repos", + events_url: "https://api.github.com/users/kylecarbs/events{/privacy}", + received_events_url: "https://api.github.com/users/kylecarbs/received_events", + type: "User", + site_admin: false, + name: "Kyle Carberry", + company: "@coder", + blog: "https://carberry.com", + location: "Austin, TX", + email: "kyle@carberry.com", + hireable: null, + bio: "hey there", + twitter_username: "kylecarbs", + public_repos: 52, + public_gists: 9, + followers: 208, + following: 31, + created_at: "2014-04-01T02:24:41Z", + updated_at: "2023-06-26T13:03:09Z", +} + +const ghInstall: Endpoints["GET /user/installations"]["response"]["data"] = { + installations: [ + { + id: 1, + access_tokens_url: "", + account: ghUser, + app_id: 1, + app_slug: "coder", + created_at: "2014-04-01T02:24:41Z", + events: [], + html_url: "", + permissions: {}, + repositories_url: "", + repository_selection: "all", + single_file_name: "", + suspended_at: null, + suspended_by: null, + target_id: 1, + target_type: "", + updated_at: "2023-06-26T13:03:09Z", + }, + ], + total_count: 1, +} diff --git a/site/package.json b/site/package.json index 32e3681340bdc..92e5ace8ceece 100644 --- a/site/package.json +++ b/site/package.json @@ -99,6 +99,7 @@ "yup": "0.32.11" }, "devDependencies": { + "@octokit/types": "10.0.0", "@playwright/test": "1.35.1", "@storybook/addon-actions": "7.0.4", "@storybook/addon-essentials": "7.0.4", diff --git a/site/src/pages/GitAuthPage/GitAuthPageView.tsx b/site/src/pages/GitAuthPage/GitAuthPageView.tsx index 3b8155a0dfcfc..8b32ac8db5642 100644 --- a/site/src/pages/GitAuthPage/GitAuthPageView.tsx +++ b/site/src/pages/GitAuthPage/GitAuthPageView.tsx @@ -119,10 +119,8 @@ const GitAuthPageView: FC = ({
{!hasInstallations && gitAuth.app_installable && ( - You must{" "} - {installTheApp}{" "} - to clone private repositories. Accounts will appear here once - authorized. + You must {installTheApp} to clone private repositories. Accounts + will appear here once authorized. )} diff --git a/site/yarn.lock b/site/yarn.lock index 89ad84ef3c25c..5e74cab92418e 100644 --- a/site/yarn.lock +++ b/site/yarn.lock @@ -1902,6 +1902,18 @@ "@nodelib/fs.scandir" "2.1.5" fastq "^1.6.0" +"@octokit/openapi-types@^18.0.0": + version "18.0.0" + resolved "https://registry.yarnpkg.com/@octokit/openapi-types/-/openapi-types-18.0.0.tgz#f43d765b3c7533fd6fb88f3f25df079c24fccf69" + integrity sha512-V8GImKs3TeQRxRtXFpG2wl19V7444NIOTDF24AWuIbmNaNYOQMWRbjcGDXV5B+0n887fgDcuMNOmlul+k+oJtw== + +"@octokit/types@10.0.0": + version "10.0.0" + resolved "https://registry.yarnpkg.com/@octokit/types/-/types-10.0.0.tgz#7ee19c464ea4ada306c43f1a45d444000f419a4a" + integrity sha512-Vm8IddVmhCgU1fxC1eyinpwqzXPEYu0NrYzD3YZjlGjyftdLBTeqNblRC0jmJmgxbJIsQlyogVeGnrNaaMVzIg== + dependencies: + "@octokit/openapi-types" "^18.0.0" + "@open-draft/until@^1.0.3": version "1.0.3" resolved "https://registry.yarnpkg.com/@open-draft/until/-/until-1.0.3.tgz#db9cc719191a62e7d9200f6e7bab21c5b848adca" From d7cecd3ccc6086af9cfa72b54163e681f9f72128 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Wed, 28 Jun 2023 18:09:43 +0000 Subject: [PATCH 10/17] Fix route documentation --- coderd/gitauth.go | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/coderd/gitauth.go b/coderd/gitauth.go index 2a2ffd55e0852..c3f6694b73d17 100644 --- a/coderd/gitauth.go +++ b/coderd/gitauth.go @@ -15,7 +15,14 @@ import ( "github.com/coder/coder/codersdk" ) -// gitAuthByID returns the git auth status for the given git auth config ID. +// @Summary Get git auth by ID +// @ID get-git-auth-by-id +// @Security CoderSessionToken +// @Produce json +// @Tags Git +// @Param gitauth path string true "Git Provider ID" format(string) +// @Success 200 {object} codersdk.GitAuth +// @Router /gitauth/{gitauth} [get] func (api *API) gitAuthByID(w http.ResponseWriter, r *http.Request) { config := httpmw.GitAuthParam(r) apiKey := httpmw.APIKey(r) @@ -59,6 +66,13 @@ func (api *API) gitAuthByID(w http.ResponseWriter, r *http.Request) { httpapi.Write(ctx, w, http.StatusOK, res) } +// @Summary Post git auth device by ID +// @ID post-git-auth-device-by-id +// @Security CoderSessionToken +// @Tags Git +// @Param gitauth path string true "Git Provider ID" format(string) +// @Success 204 +// @Router /gitauth/{gitauth}/device [post] func (api *API) postGitAuthDeviceByID(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() apiKey := httpmw.APIKey(r) @@ -134,7 +148,14 @@ func (api *API) postGitAuthDeviceByID(rw http.ResponseWriter, r *http.Request) { httpapi.Write(ctx, rw, http.StatusNoContent, nil) } -// gitAuthDeviceByID issues a new device auth code for the given git auth config ID. +// @Summary Get git auth device by ID. +// @ID get-git-auth-device-by-id +// @Security CoderSessionToken +// @Produce json +// @Tags Git +// @Param gitauth path string true "Git Provider ID" format(string) +// @Success 200 {object} codersdk.GitAuthDevice +// @Router /gitauth/{gitauth}/device [get] func (*API) gitAuthDeviceByID(rw http.ResponseWriter, r *http.Request) { config := httpmw.GitAuthParam(r) ctx := r.Context() From 189d7fc2a0e0d2be965e9fb50524e204beb58a1c Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Wed, 28 Jun 2023 18:11:23 +0000 Subject: [PATCH 11/17] Fix imports --- coderd/gitauth/config_test.go | 7 ------- coderd/gitauth_test.go | 16 ++++++++++------ 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/coderd/gitauth/config_test.go b/coderd/gitauth/config_test.go index 82595bbbe34e5..31d6392341426 100644 --- a/coderd/gitauth/config_test.go +++ b/coderd/gitauth/config_test.go @@ -143,13 +143,6 @@ func TestConvertYAML(t *testing.T) { Type: string(codersdk.GitProviderGitHub), }}, Error: "client_id must be provided", - }, { - Name: "NoClientSecret", - Input: []codersdk.GitAuthConfig{{ - Type: string(codersdk.GitProviderGitHub), - ClientID: "example", - }}, - Error: "client_secret must be provided", }, { Name: "DuplicateType", Input: []codersdk.GitAuthConfig{{ diff --git a/coderd/gitauth_test.go b/coderd/gitauth_test.go index 406866ba5709c..969396cfc918b 100644 --- a/coderd/gitauth_test.go +++ b/coderd/gitauth_test.go @@ -96,12 +96,16 @@ func TestGitAuthByID(t *testing.T) { AvatarURL: github.String("https://avatars.githubusercontent.com/u/12345678?v=4"), }) case "/installs": - httpapi.Write(r.Context(), w, http.StatusOK, []github.Installation{{ - ID: github.Int64(12345678), - Account: &github.User{ - Login: github.String("coder"), - }, - }}) + httpapi.Write(r.Context(), w, http.StatusOK, struct { + Installations []github.Installation `json:"installations"` + }{ + Installations: []github.Installation{{ + ID: github.Int64(12345678), + Account: &github.User{ + Login: github.String("coder"), + }, + }}, + }) } })) defer srv.Close() From de5632c6d30ae90b6640274a72e300ef30213ed6 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Wed, 28 Jun 2023 18:13:15 +0000 Subject: [PATCH 12/17] Remove unused imports --- site/src/pages/GitAuthPage/GitAuthPage.tsx | 2 +- site/src/pages/GitAuthPage/GitAuthPageView.tsx | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/site/src/pages/GitAuthPage/GitAuthPage.tsx b/site/src/pages/GitAuthPage/GitAuthPage.tsx index 5a7f16f0aa066..97013b7b13fb4 100644 --- a/site/src/pages/GitAuthPage/GitAuthPage.tsx +++ b/site/src/pages/GitAuthPage/GitAuthPage.tsx @@ -6,7 +6,7 @@ import { } from "api/api" import { usePermissions } from "hooks" import { FC, useEffect } from "react" -import { redirect, useParams } from "react-router-dom" +import { useParams } from "react-router-dom" import { REFRESH_GITAUTH_BROADCAST_CHANNEL } from "xServices/createWorkspace/createWorkspaceXService" import GitAuthPageView from "./GitAuthPageView" import { ApiErrorResponse } from "api/errors" diff --git a/site/src/pages/GitAuthPage/GitAuthPageView.tsx b/site/src/pages/GitAuthPage/GitAuthPageView.tsx index 8b32ac8db5642..963f89d4c3265 100644 --- a/site/src/pages/GitAuthPage/GitAuthPageView.tsx +++ b/site/src/pages/GitAuthPage/GitAuthPageView.tsx @@ -1,6 +1,5 @@ import OpenInNewIcon from "@mui/icons-material/OpenInNew" import RefreshIcon from "@mui/icons-material/Refresh" -import Button from "@mui/material/Button" import CircularProgress from "@mui/material/CircularProgress" import Link from "@mui/material/Link" import Tooltip from "@mui/material/Tooltip" From 847b08818790923fb056e0ddd79c65c4d75de8d7 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Wed, 28 Jun 2023 18:33:50 +0000 Subject: [PATCH 13/17] Fix E2E web test --- coderd/apidoc/docs.go | 187 ++++++++++++++++++++++++++++++++- coderd/apidoc/swagger.json | 177 ++++++++++++++++++++++++++++++- coderd/gitauth/config.go | 7 +- coderd/gitauth_test.go | 9 +- docs/api/general.md | 2 +- docs/api/git.md | 127 ++++++++++++++++++++++ docs/api/schemas.md | 117 ++++++++++++++++++++- docs/manifest.json | 4 + site/e2e/constants.ts | 1 + site/e2e/playwright.config.ts | 1 + site/e2e/tests/gitAuth.spec.ts | 25 +++-- site/src/api/typesGenerated.ts | 2 +- 12 files changed, 635 insertions(+), 24 deletions(-) create mode 100644 docs/api/git.md diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 36c2095581d9c..0985371115763 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -658,6 +658,103 @@ const docTemplate = `{ } } }, + "/gitauth/{gitauth}": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": [ + "application/json" + ], + "tags": [ + "Git" + ], + "summary": "Get git auth by ID", + "operationId": "get-git-auth-by-id", + "parameters": [ + { + "type": "string", + "format": "string", + "description": "Git Provider ID", + "name": "gitauth", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/codersdk.GitAuth" + } + } + } + } + }, + "/gitauth/{gitauth}/device": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": [ + "application/json" + ], + "tags": [ + "Git" + ], + "summary": "Get git auth device by ID.", + "operationId": "get-git-auth-device-by-id", + "parameters": [ + { + "type": "string", + "format": "string", + "description": "Git Provider ID", + "name": "gitauth", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/codersdk.GitAuthDevice" + } + } + } + }, + "post": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "tags": [ + "Git" + ], + "summary": "Post git auth device by ID", + "operationId": "post-git-auth-device-by-id", + "parameters": [ + { + "type": "string", + "format": "string", + "description": "Git Provider ID", + "name": "gitauth", + "in": "path", + "required": true + } + ], + "responses": { + "204": { + "description": "No Content" + } + } + } + }, "/groups/{group}": { "get": { "security": [ @@ -7440,6 +7537,57 @@ const docTemplate = `{ } } }, + "codersdk.GitAuth": { + "type": "object", + "properties": { + "app_install_url": { + "description": "AppInstallURL is the URL to install the app.", + "type": "string" + }, + "app_installable": { + "description": "AppInstallable is true if the request for app installs was successful.", + "type": "boolean" + }, + "authenticated": { + "type": "boolean" + }, + "device": { + "type": "boolean" + }, + "installations": { + "description": "AppInstallations are the installations that the user has access to.", + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.GitAuthAppInstallation" + } + }, + "type": { + "type": "string" + }, + "user": { + "description": "User is the user that authenticated with the provider.", + "allOf": [ + { + "$ref": "#/definitions/codersdk.GitAuthUser" + } + ] + } + } + }, + "codersdk.GitAuthAppInstallation": { + "type": "object", + "properties": { + "account": { + "$ref": "#/definitions/codersdk.GitAuthUser" + }, + "configure_url": { + "type": "string" + }, + "id": { + "type": "integer" + } + } + }, "codersdk.GitAuthConfig": { "type": "object", "properties": { @@ -7455,7 +7603,7 @@ const docTemplate = `{ "client_id": { "type": "string" }, - "device_auth_url": { + "device_code_url": { "type": "string" }, "device_flow": { @@ -7487,6 +7635,43 @@ const docTemplate = `{ } } }, + "codersdk.GitAuthDevice": { + "type": "object", + "properties": { + "device_code": { + "type": "string" + }, + "expires_in": { + "type": "integer" + }, + "interval": { + "type": "integer" + }, + "user_code": { + "type": "string" + }, + "verification_uri": { + "type": "string" + } + } + }, + "codersdk.GitAuthUser": { + "type": "object", + "properties": { + "avatar_url": { + "type": "string" + }, + "login": { + "type": "string" + }, + "name": { + "type": "string" + }, + "profile_url": { + "type": "string" + } + } + }, "codersdk.GitProvider": { "type": "string", "enum": [ diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 0b6323fce65d4..b3598dc3e6c86 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -562,6 +562,93 @@ } } }, + "/gitauth/{gitauth}": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": ["application/json"], + "tags": ["Git"], + "summary": "Get git auth by ID", + "operationId": "get-git-auth-by-id", + "parameters": [ + { + "type": "string", + "format": "string", + "description": "Git Provider ID", + "name": "gitauth", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/codersdk.GitAuth" + } + } + } + } + }, + "/gitauth/{gitauth}/device": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": ["application/json"], + "tags": ["Git"], + "summary": "Get git auth device by ID.", + "operationId": "get-git-auth-device-by-id", + "parameters": [ + { + "type": "string", + "format": "string", + "description": "Git Provider ID", + "name": "gitauth", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/codersdk.GitAuthDevice" + } + } + } + }, + "post": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "tags": ["Git"], + "summary": "Post git auth device by ID", + "operationId": "post-git-auth-device-by-id", + "parameters": [ + { + "type": "string", + "format": "string", + "description": "Git Provider ID", + "name": "gitauth", + "in": "path", + "required": true + } + ], + "responses": { + "204": { + "description": "No Content" + } + } + } + }, "/groups/{group}": { "get": { "security": [ @@ -6661,6 +6748,57 @@ } } }, + "codersdk.GitAuth": { + "type": "object", + "properties": { + "app_install_url": { + "description": "AppInstallURL is the URL to install the app.", + "type": "string" + }, + "app_installable": { + "description": "AppInstallable is true if the request for app installs was successful.", + "type": "boolean" + }, + "authenticated": { + "type": "boolean" + }, + "device": { + "type": "boolean" + }, + "installations": { + "description": "AppInstallations are the installations that the user has access to.", + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.GitAuthAppInstallation" + } + }, + "type": { + "type": "string" + }, + "user": { + "description": "User is the user that authenticated with the provider.", + "allOf": [ + { + "$ref": "#/definitions/codersdk.GitAuthUser" + } + ] + } + } + }, + "codersdk.GitAuthAppInstallation": { + "type": "object", + "properties": { + "account": { + "$ref": "#/definitions/codersdk.GitAuthUser" + }, + "configure_url": { + "type": "string" + }, + "id": { + "type": "integer" + } + } + }, "codersdk.GitAuthConfig": { "type": "object", "properties": { @@ -6676,7 +6814,7 @@ "client_id": { "type": "string" }, - "device_auth_url": { + "device_code_url": { "type": "string" }, "device_flow": { @@ -6708,6 +6846,43 @@ } } }, + "codersdk.GitAuthDevice": { + "type": "object", + "properties": { + "device_code": { + "type": "string" + }, + "expires_in": { + "type": "integer" + }, + "interval": { + "type": "integer" + }, + "user_code": { + "type": "string" + }, + "verification_uri": { + "type": "string" + } + } + }, + "codersdk.GitAuthUser": { + "type": "object", + "properties": { + "avatar_url": { + "type": "string" + }, + "login": { + "type": "string" + }, + "name": { + "type": "string" + }, + "profile_url": { + "type": "string" + } + } + }, "codersdk.GitProvider": { "type": "string", "enum": ["azure-devops", "github", "gitlab", "bitbucket"], diff --git a/coderd/gitauth/config.go b/coderd/gitauth/config.go index 24635cd790fcf..29d4804dcd538 100644 --- a/coderd/gitauth/config.go +++ b/coderd/gitauth/config.go @@ -169,7 +169,11 @@ func (c *Config) AppInstallations(ctx context.Context, token string) ([]codersdk return nil, false, err } defer res.Body.Close() - + // It's possible the installation URL is misconfigured, so we don't + // want to return an error here. + if res.StatusCode != http.StatusOK { + return nil, false, nil + } installs := []codersdk.GitAuthAppInstallation{} if c.Type == codersdk.GitProviderGitHub { var ghInstalls struct { @@ -180,7 +184,6 @@ func (c *Config) AppInstallations(ctx context.Context, token string) ([]codersdk return nil, false, err } for _, installation := range ghInstalls.Installations { - account := installation.GetAccount() if account == nil { continue diff --git a/coderd/gitauth_test.go b/coderd/gitauth_test.go index 969396cfc918b..9d102aa2150f8 100644 --- a/coderd/gitauth_test.go +++ b/coderd/gitauth_test.go @@ -56,7 +56,8 @@ func TestGitAuthByID(t *testing.T) { }}, }) coderdtest.CreateFirstUser(t, client) - coderdtest.RequestGitAuthCallback(t, "test", client) + resp := coderdtest.RequestGitAuthCallback(t, "test", client) + _ = resp.Body.Close() auth, err := client.GitAuthByID(context.Background(), "test") require.NoError(t, err) require.True(t, auth.Authenticated) @@ -79,7 +80,8 @@ func TestGitAuthByID(t *testing.T) { }}, }) coderdtest.CreateFirstUser(t, client) - coderdtest.RequestGitAuthCallback(t, "test", client) + resp := coderdtest.RequestGitAuthCallback(t, "test", client) + _ = resp.Body.Close() auth, err := client.GitAuthByID(context.Background(), "test") require.NoError(t, err) require.True(t, auth.Authenticated) @@ -119,7 +121,8 @@ func TestGitAuthByID(t *testing.T) { }}, }) coderdtest.CreateFirstUser(t, client) - coderdtest.RequestGitAuthCallback(t, "test", client) + resp := coderdtest.RequestGitAuthCallback(t, "test", client) + _ = resp.Body.Close() auth, err := client.GitAuthByID(context.Background(), "test") require.NoError(t, err) require.True(t, auth.Authenticated) diff --git a/docs/api/general.md b/docs/api/general.md index f0d6d7f2e1c9f..2a2e3d4da4631 100644 --- a/docs/api/general.md +++ b/docs/api/general.md @@ -204,7 +204,7 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \ "app_installations_url": "string", "auth_url": "string", "client_id": "string", - "device_auth_url": "string", + "device_code_url": "string", "device_flow": true, "id": "string", "no_refresh": true, diff --git a/docs/api/git.md b/docs/api/git.md new file mode 100644 index 0000000000000..0f55fdfba909a --- /dev/null +++ b/docs/api/git.md @@ -0,0 +1,127 @@ +# Git + +## Get git auth by ID + +### Code samples + +```shell +# Example request using curl +curl -X GET http://coder-server:8080/api/v2/gitauth/{gitauth} \ + -H 'Accept: application/json' \ + -H 'Coder-Session-Token: API_KEY' +``` + +`GET /gitauth/{gitauth}` + +### Parameters + +| Name | In | Type | Required | Description | +| --------- | ---- | -------------- | -------- | --------------- | +| `gitauth` | path | string(string) | true | Git Provider ID | + +### Example responses + +> 200 Response + +```json +{ + "app_install_url": "string", + "app_installable": true, + "authenticated": true, + "device": true, + "installations": [ + { + "account": { + "avatar_url": "string", + "login": "string", + "name": "string", + "profile_url": "string" + }, + "configure_url": "string", + "id": 0 + } + ], + "type": "string", + "user": { + "avatar_url": "string", + "login": "string", + "name": "string", + "profile_url": "string" + } +} +``` + +### Responses + +| Status | Meaning | Description | Schema | +| ------ | ------------------------------------------------------- | ----------- | ---------------------------------------------- | +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [codersdk.GitAuth](schemas.md#codersdkgitauth) | + +To perform this operation, you must be authenticated. [Learn more](authentication.md). + +## Get git auth device by ID. + +### Code samples + +```shell +# Example request using curl +curl -X GET http://coder-server:8080/api/v2/gitauth/{gitauth}/device \ + -H 'Accept: application/json' \ + -H 'Coder-Session-Token: API_KEY' +``` + +`GET /gitauth/{gitauth}/device` + +### Parameters + +| Name | In | Type | Required | Description | +| --------- | ---- | -------------- | -------- | --------------- | +| `gitauth` | path | string(string) | true | Git Provider ID | + +### Example responses + +> 200 Response + +```json +{ + "device_code": "string", + "expires_in": 0, + "interval": 0, + "user_code": "string", + "verification_uri": "string" +} +``` + +### Responses + +| Status | Meaning | Description | Schema | +| ------ | ------------------------------------------------------- | ----------- | ---------------------------------------------------------- | +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [codersdk.GitAuthDevice](schemas.md#codersdkgitauthdevice) | + +To perform this operation, you must be authenticated. [Learn more](authentication.md). + +## Post git auth device by ID + +### Code samples + +```shell +# Example request using curl +curl -X POST http://coder-server:8080/api/v2/gitauth/{gitauth}/device \ + -H 'Coder-Session-Token: API_KEY' +``` + +`POST /gitauth/{gitauth}/device` + +### Parameters + +| Name | In | Type | Required | Description | +| --------- | ---- | -------------- | -------- | --------------- | +| `gitauth` | path | string(string) | true | Git Provider ID | + +### Responses + +| Status | Meaning | Description | Schema | +| ------ | --------------------------------------------------------------- | ----------- | ------ | +| 204 | [No Content](https://tools.ietf.org/html/rfc7231#section-6.3.5) | No Content | | + +To perform this operation, you must be authenticated. [Learn more](authentication.md). diff --git a/docs/api/schemas.md b/docs/api/schemas.md index 890a0cd91b65c..c4c054f684881 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -597,7 +597,7 @@ "app_installations_url": "string", "auth_url": "string", "client_id": "string", - "device_auth_url": "string", + "device_code_url": "string", "device_flow": true, "id": "string", "no_refresh": true, @@ -1885,7 +1885,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "app_installations_url": "string", "auth_url": "string", "client_id": "string", - "device_auth_url": "string", + "device_code_url": "string", "device_flow": true, "id": "string", "no_refresh": true, @@ -2220,7 +2220,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "app_installations_url": "string", "auth_url": "string", "client_id": "string", - "device_auth_url": "string", + "device_code_url": "string", "device_flow": true, "id": "string", "no_refresh": true, @@ -2583,6 +2583,71 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in | `count` | integer | false | | | | `users` | array of [codersdk.User](#codersdkuser) | false | | | +## codersdk.GitAuth + +```json +{ + "app_install_url": "string", + "app_installable": true, + "authenticated": true, + "device": true, + "installations": [ + { + "account": { + "avatar_url": "string", + "login": "string", + "name": "string", + "profile_url": "string" + }, + "configure_url": "string", + "id": 0 + } + ], + "type": "string", + "user": { + "avatar_url": "string", + "login": "string", + "name": "string", + "profile_url": "string" + } +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +| ----------------- | --------------------------------------------------------------------------- | -------- | ------------ | ----------------------------------------------------------------------- | +| `app_install_url` | string | false | | App install URL is the URL to install the app. | +| `app_installable` | boolean | false | | App installable is true if the request for app installs was successful. | +| `authenticated` | boolean | false | | | +| `device` | boolean | false | | | +| `installations` | array of [codersdk.GitAuthAppInstallation](#codersdkgitauthappinstallation) | false | | Installations are the installations that the user has access to. | +| `type` | string | false | | | +| `user` | [codersdk.GitAuthUser](#codersdkgitauthuser) | false | | User is the user that authenticated with the provider. | + +## codersdk.GitAuthAppInstallation + +```json +{ + "account": { + "avatar_url": "string", + "login": "string", + "name": "string", + "profile_url": "string" + }, + "configure_url": "string", + "id": 0 +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +| --------------- | -------------------------------------------- | -------- | ------------ | ----------- | +| `account` | [codersdk.GitAuthUser](#codersdkgitauthuser) | false | | | +| `configure_url` | string | false | | | +| `id` | integer | false | | | + ## codersdk.GitAuthConfig ```json @@ -2591,7 +2656,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "app_installations_url": "string", "auth_url": "string", "client_id": "string", - "device_auth_url": "string", + "device_code_url": "string", "device_flow": true, "id": "string", "no_refresh": true, @@ -2611,7 +2676,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in | `app_installations_url` | string | false | | | | `auth_url` | string | false | | | | `client_id` | string | false | | | -| `device_auth_url` | string | false | | | +| `device_code_url` | string | false | | | | `device_flow` | boolean | false | | | | `id` | string | false | | | | `no_refresh` | boolean | false | | | @@ -2621,6 +2686,48 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in | `type` | string | false | | | | `validate_url` | string | false | | | +## codersdk.GitAuthDevice + +```json +{ + "device_code": "string", + "expires_in": 0, + "interval": 0, + "user_code": "string", + "verification_uri": "string" +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +| ------------------ | ------- | -------- | ------------ | ----------- | +| `device_code` | string | false | | | +| `expires_in` | integer | false | | | +| `interval` | integer | false | | | +| `user_code` | string | false | | | +| `verification_uri` | string | false | | | + +## codersdk.GitAuthUser + +```json +{ + "avatar_url": "string", + "login": "string", + "name": "string", + "profile_url": "string" +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +| ------------- | ------ | -------- | ------------ | ----------- | +| `avatar_url` | string | false | | | +| `login` | string | false | | | +| `name` | string | false | | | +| `profile_url` | string | false | | | + ## codersdk.GitProvider ```json diff --git a/docs/manifest.json b/docs/manifest.json index 00574def08266..c7e56a3a23014 100644 --- a/docs/manifest.json +++ b/docs/manifest.json @@ -451,6 +451,10 @@ "title": "Files", "path": "./api/files.md" }, + { + "title": "Git", + "path": "./api/git.md" + }, { "title": "Insights", "path": "./api/insights.md" diff --git a/site/e2e/constants.ts b/site/e2e/constants.ts index b141f637db62f..98036d6fe73d5 100644 --- a/site/e2e/constants.ts +++ b/site/e2e/constants.ts @@ -15,6 +15,7 @@ export const gitAuth = { devicePort: 50515, webPort: 50516, + authPath: "/auth", tokenPath: "/token", codePath: "/code", validatePath: "/validate", diff --git a/site/e2e/playwright.config.ts b/site/e2e/playwright.config.ts index 8dbd45e388f87..0ff9a6fe74622 100644 --- a/site/e2e/playwright.config.ts +++ b/site/e2e/playwright.config.ts @@ -74,6 +74,7 @@ export default defineConfig({ CODER_GITAUTH_1_TYPE: "github", CODER_GITAUTH_1_CLIENT_ID: "client", CODER_GITAUTH_1_CLIENT_SECRET: "secret", + CODER_GITAUTH_1_AUTH_URL: localURL(gitAuth.webPort, gitAuth.authPath), CODER_GITAUTH_1_TOKEN_URL: localURL(gitAuth.webPort, gitAuth.tokenPath), CODER_GITAUTH_1_DEVICE_CODE_URL: localURL( gitAuth.webPort, diff --git a/site/e2e/tests/gitAuth.spec.ts b/site/e2e/tests/gitAuth.spec.ts index 1a99aa3ed0f0b..ab5659f92103b 100644 --- a/site/e2e/tests/gitAuth.spec.ts +++ b/site/e2e/tests/gitAuth.spec.ts @@ -4,6 +4,7 @@ import { Endpoints } from "@octokit/types" import { GitAuthDevice } from "api/typesGenerated" import { Awaiter, createServer } from "../helpers" +// Ensures that a Git auth provider with the device flow functions and completes! test("git auth device", async ({ page }) => { const device: GitAuthDevice = { device_code: "1234", @@ -13,8 +14,8 @@ test("git auth device", async ({ page }) => { verification_uri: "", } + // Start a server to mock the GitHub API. const srv = await createServer(gitAuth.devicePort) - // The GitHub validate endpoint returns the currently authenticated user! srv.use(gitAuth.validatePath, (req, res) => { res.write(JSON.stringify(ghUser)) res.end() @@ -33,7 +34,8 @@ test("git auth device", async ({ page }) => { error: "authorization_pending", error_description: "", } - + // First we send a result from the API that the token hasn't been + // authorized yet to ensure the UI reacts properly. const sentPending = new Awaiter() srv.use(gitAuth.tokenPath, (req, res) => { res.write(JSON.stringify(token)) @@ -46,31 +48,34 @@ test("git auth device", async ({ page }) => { }) await page.getByText(device.user_code).isVisible() await sentPending.wait() + // Update the token to be valid and ensure the UI updates! token.error = "" token.access_token = "hello-world" - await page.getByText("1 organization authorized").isVisible() + await page.waitForSelector("text=1 organization authorized") }) -test("git auth web", async ({ page }) => { +test("git auth web", async ({ baseURL, page }) => { const srv = await createServer(gitAuth.webPort) // The GitHub validate endpoint returns the currently authenticated user! srv.use(gitAuth.validatePath, (req, res) => { res.write(JSON.stringify(ghUser)) res.end() }) - srv.use(gitAuth.installationsPath, (req, res) => { - res.write(JSON.stringify(ghInstall)) - res.end() - }) srv.use(gitAuth.tokenPath, (req, res) => { res.write(JSON.stringify({ access_token: "hello-world" })) res.end() }) - + srv.use(gitAuth.authPath, (req, res) => { + res.redirect( + `${baseURL}/gitauth/${gitAuth.webProvider}/callback?code=1234&state=` + + req.query.state, + ) + }) await page.goto(`/gitauth/${gitAuth.webProvider}`, { waitUntil: "networkidle", }) - await page.getByText("1 organization authorized").isVisible() + // This endpoint doesn't have the installations URL set intentionally! + await page.waitForSelector("text=You've authenticated with GitHub!") }) const ghUser: Endpoints["GET /user"]["response"]["data"] = { diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 22e9393dfdbfe..2a8d50aada33a 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -446,7 +446,7 @@ export interface GitAuthConfig { readonly no_refresh: boolean readonly scopes: string[] readonly device_flow: boolean - readonly device_auth_url: string + readonly device_code_url: string } // From codersdk/gitauth.go From bdbb1db1963f5af7d525c8e353310fdf1a28c96e Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Wed, 28 Jun 2023 18:36:42 +0000 Subject: [PATCH 14/17] Fix friendly message appearance --- site/src/pages/GitAuthPage/GitAuthPageView.tsx | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/site/src/pages/GitAuthPage/GitAuthPageView.tsx b/site/src/pages/GitAuthPage/GitAuthPageView.tsx index 963f89d4c3265..4bf1acded55ba 100644 --- a/site/src/pages/GitAuthPage/GitAuthPageView.tsx +++ b/site/src/pages/GitAuthPage/GitAuthPageView.tsx @@ -66,12 +66,7 @@ const GitAuthPageView: FC = ({ let installTheApp: JSX.Element = <>{`install the ${gitAuth.type} App`} if (gitAuth.app_install_url) { installTheApp = ( - + {installTheApp} ) @@ -81,8 +76,9 @@ const GitAuthPageView: FC = ({

- Hey @{gitAuth.user?.login} 👋! You are now authenticated with Git. Feel - free to close this window! + Hey @{gitAuth.user?.login} 👋!{" "} + {(!gitAuth.app_installable || gitAuth.installations.length > 0) && + "You are now authenticated with Git. Feel free to close this window!"}

{gitAuth.installations.length > 0 && ( From a4fcf4b475f26d42970c559e2e5e9e3676f543bd Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Wed, 28 Jun 2023 18:50:43 +0000 Subject: [PATCH 15/17] Fix layout shifting for full-screen sign-in --- site/src/components/Margins/Margins.tsx | 1 - site/src/components/SignInLayout/SignInLayout.tsx | 7 +++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/site/src/components/Margins/Margins.tsx b/site/src/components/Margins/Margins.tsx index acfa252bb3815..0e3af4db9440b 100644 --- a/site/src/components/Margins/Margins.tsx +++ b/site/src/components/Margins/Margins.tsx @@ -20,7 +20,6 @@ const useStyles = makeStyles(() => ({ margin: "0 auto", maxWidth: ({ maxWidth }: { maxWidth: number }) => maxWidth, padding: `0 ${sidePadding}px`, - flex: 1, width: "100%", }, })) diff --git a/site/src/components/SignInLayout/SignInLayout.tsx b/site/src/components/SignInLayout/SignInLayout.tsx index 080274ca00c5e..f085d9891e0f8 100644 --- a/site/src/components/SignInLayout/SignInLayout.tsx +++ b/site/src/components/SignInLayout/SignInLayout.tsx @@ -2,8 +2,15 @@ import { makeStyles } from "@mui/styles" import { FC, ReactNode } from "react" export const useStyles = makeStyles((theme) => ({ + "@global": { + // Necessary for when this is on lonely pages! + "#root": { + height: "100vh", + }, + }, root: { flex: 1, + height: "-webkit-fill-available", display: "flex", justifyContent: "center", alignItems: "center", From 1ef7c5408e29f6d9a72a96670ea20e3842fb0e69 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Wed, 28 Jun 2023 19:04:43 +0000 Subject: [PATCH 16/17] Fix height going to 100% --- site/src/components/SignInLayout/SignInLayout.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/components/SignInLayout/SignInLayout.tsx b/site/src/components/SignInLayout/SignInLayout.tsx index f085d9891e0f8..cf223d7d8f53e 100644 --- a/site/src/components/SignInLayout/SignInLayout.tsx +++ b/site/src/components/SignInLayout/SignInLayout.tsx @@ -4,7 +4,7 @@ import { FC, ReactNode } from "react" export const useStyles = makeStyles((theme) => ({ "@global": { // Necessary for when this is on lonely pages! - "#root": { + "html, body, #root, #storybook-root": { height: "100vh", }, }, From d43bd717bb792e4ee938d61b2d90d579e08069e7 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Thu, 29 Jun 2023 18:31:55 +0000 Subject: [PATCH 17/17] Fix comments --- coderd/gitauth.go | 47 +++++++++++++++++++------------- coderd/gitauth/oauth.go | 59 +++++++++++++++++++---------------------- 2 files changed, 56 insertions(+), 50 deletions(-) diff --git a/coderd/gitauth.go b/coderd/gitauth.go index c3f6694b73d17..537f0b8794e32 100644 --- a/coderd/gitauth.go +++ b/coderd/gitauth.go @@ -29,40 +29,49 @@ func (api *API) gitAuthByID(w http.ResponseWriter, r *http.Request) { ctx := r.Context() res := codersdk.GitAuth{ - Authenticated: false, - Device: config.DeviceAuth != nil, - AppInstallURL: config.AppInstallURL, - Type: config.Type.Pretty(), + Authenticated: false, + Device: config.DeviceAuth != nil, + AppInstallURL: config.AppInstallURL, + Type: config.Type.Pretty(), + AppInstallations: []codersdk.GitAuthAppInstallation{}, } link, err := api.Database.GetGitAuthLink(ctx, database.GetGitAuthLinkParams{ ProviderID: config.ID, UserID: apiKey.UserID, }) - if err == nil { - var eg errgroup.Group - eg.Go(func() (err error) { - res.Authenticated, res.User, err = config.ValidateToken(ctx, link.OAuthAccessToken) - return err - }) - eg.Go(func() (err error) { - res.AppInstallations, res.AppInstallable, err = config.AppInstallations(ctx, link.OAuthAccessToken) - return err - }) - err = eg.Wait() - if err != nil { + if err != nil { + if !errors.Is(err, sql.ErrNoRows) { httpapi.Write(ctx, w, http.StatusInternalServerError, codersdk.Response{ - Message: "Failed to validate token.", + Message: "Failed to get git auth link.", Detail: err.Error(), }) return } - } + httpapi.Write(ctx, w, http.StatusOK, res) + return + } + var eg errgroup.Group + eg.Go(func() (err error) { + res.Authenticated, res.User, err = config.ValidateToken(ctx, link.OAuthAccessToken) + return err + }) + eg.Go(func() (err error) { + res.AppInstallations, res.AppInstallable, err = config.AppInstallations(ctx, link.OAuthAccessToken) + return err + }) + err = eg.Wait() + if err != nil { + httpapi.Write(ctx, w, http.StatusInternalServerError, codersdk.Response{ + Message: "Failed to validate token.", + Detail: err.Error(), + }) + return + } if res.AppInstallations == nil { res.AppInstallations = []codersdk.GitAuthAppInstallation{} } - httpapi.Write(ctx, w, http.StatusOK, res) } diff --git a/coderd/gitauth/oauth.go b/coderd/gitauth/oauth.go index 6756517347539..1e0748da91fc2 100644 --- a/coderd/gitauth/oauth.go +++ b/coderd/gitauth/oauth.go @@ -1,13 +1,11 @@ package gitauth import ( - "bytes" "context" "encoding/json" "net/http" "net/url" "regexp" - "strings" "golang.org/x/oauth2" "golang.org/x/oauth2/github" @@ -111,7 +109,11 @@ func (c *DeviceAuth) AuthorizeDevice(ctx context.Context) (*codersdk.GitAuthDevi if c.CodeURL == "" { return nil, xerrors.New("oauth2: device code URL not set") } - req, err := http.NewRequestWithContext(ctx, http.MethodPost, c.formatDeviceCodeURL(), nil) + codeURL, err := c.formatDeviceCodeURL() + if err != nil { + return nil, err + } + req, err := http.NewRequestWithContext(ctx, http.MethodPost, codeURL, nil) if err != nil { return nil, err } @@ -130,7 +132,7 @@ func (c *DeviceAuth) AuthorizeDevice(ctx context.Context) (*codersdk.GitAuthDevi return nil, err } if r.ErrorDescription != "" { - return nil, xerrors.Errorf("%s", r.ErrorDescription) + return nil, xerrors.New(r.ErrorDescription) } return &r.GitAuthDevice, nil } @@ -148,7 +150,11 @@ func (c *DeviceAuth) ExchangeDeviceCode(ctx context.Context, deviceCode string) if c.TokenURL == "" { return nil, xerrors.New("oauth2: token URL not set") } - req, err := http.NewRequestWithContext(ctx, http.MethodPost, c.formatDeviceTokenURL(deviceCode), nil) + tokenURL, err := c.formatDeviceTokenURL(deviceCode) + if err != nil { + return nil, err + } + req, err := http.NewRequestWithContext(ctx, http.MethodPost, tokenURL, nil) if err != nil { return nil, err } @@ -167,41 +173,32 @@ func (c *DeviceAuth) ExchangeDeviceCode(ctx context.Context, deviceCode string) return nil, err } if body.Error != "" { - return nil, xerrors.Errorf("%s", body.Error) + return nil, xerrors.New(body.Error) } return body.Token, nil } -func (c *DeviceAuth) formatDeviceTokenURL(deviceCode string) string { - var buf bytes.Buffer - _, _ = buf.WriteString(c.TokenURL) - v := url.Values{ +func (c *DeviceAuth) formatDeviceTokenURL(deviceCode string) (string, error) { + tok, err := url.Parse(c.TokenURL) + if err != nil { + return "", err + } + tok.RawQuery = url.Values{ "client_id": {c.ClientID}, "device_code": {deviceCode}, "grant_type": {"urn:ietf:params:oauth:grant-type:device_code"}, - } - if strings.Contains(c.TokenURL, "?") { - _ = buf.WriteByte('&') - } else { - _ = buf.WriteByte('?') - } - _, _ = buf.WriteString(v.Encode()) - return buf.String() + }.Encode() + return tok.String(), nil } -func (c *DeviceAuth) formatDeviceCodeURL() string { - var buf bytes.Buffer - _, _ = buf.WriteString(c.CodeURL) - - v := url.Values{ +func (c *DeviceAuth) formatDeviceCodeURL() (string, error) { + cod, err := url.Parse(c.CodeURL) + if err != nil { + return "", err + } + cod.RawQuery = url.Values{ "client_id": {c.ClientID}, "scope": c.Scopes, - } - if strings.Contains(c.CodeURL, "?") { - _ = buf.WriteByte('&') - } else { - _ = buf.WriteByte('?') - } - _, _ = buf.WriteString(v.Encode()) - return buf.String() + }.Encode() + return cod.String(), nil }