From 4b9621f9ae2ba547e2a297373a5fb7b5be20d151 Mon Sep 17 00:00:00 2001
From: Ammar Bandukwala <ammar@ammar.io>
Date: Mon, 1 May 2023 14:19:41 -0500
Subject: [PATCH] fix(coderd): don't hang on first gitauth clone (#7331)

Previously, the `coder git ssh` command would hang on the API, which was endlessly polling the database for oauth tokens that expire in the future.

Some oAuth implementations (including GitHub by default) will not give back a token expiry date, and the absence of such a date was represented as a zero data in the database as opposed to a null value.

Follow-up calls to `git clone` would succeed because this hot path doesn't check expiry, perhaps originally by mistake.

In addition to fixing the zero date issue, this PR removes all gitauth PubSub, which added too much complexity when the polling interval is 1 second.
---
 coderd/workspaceagents.go | 47 ++++++++-------------------------------
 1 file changed, 9 insertions(+), 38 deletions(-)

diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go
index 1b58c9f2c3..e6ffa5d4d6 100644
--- a/coderd/workspaceagents.go
+++ b/coderd/workspaceagents.go
@@ -1737,32 +1737,8 @@ func (api *API) workspaceAgentsGitAuth(rw http.ResponseWriter, r *http.Request)
 	}
 
 	if listen {
-		// If listening we await a new token...
-		authChan := make(chan struct{}, 1)
-		cancelFunc, err := api.Pubsub.Subscribe("gitauth", func(ctx context.Context, message []byte) {
-			ids := strings.Split(string(message), "|")
-			if len(ids) != 2 {
-				return
-			}
-			if ids[0] != gitAuthConfig.ID {
-				return
-			}
-			if ids[1] != workspace.OwnerID.String() {
-				return
-			}
-			select {
-			case authChan <- struct{}{}:
-			default:
-			}
-		})
-		if err != nil {
-			httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
-				Message: "Failed to listen for git auth token.",
-				Detail:  err.Error(),
-			})
-			return
-		}
-		defer cancelFunc()
+		// Since we're ticking frequently and this sign-in operation is rare,
+		// we are OK with polling to avoid the complexity of pubsub.
 		ticker := time.NewTicker(time.Second)
 		defer ticker.Stop()
 		for {
@@ -1770,7 +1746,6 @@ func (api *API) workspaceAgentsGitAuth(rw http.ResponseWriter, r *http.Request)
 			case <-ctx.Done():
 				return
 			case <-ticker.C:
-			case <-authChan:
 			}
 			gitAuthLink, err := api.Database.GetGitAuthLink(ctx, database.GetGitAuthLinkParams{
 				ProviderID: gitAuthConfig.ID,
@@ -1786,7 +1761,12 @@ func (api *API) workspaceAgentsGitAuth(rw http.ResponseWriter, r *http.Request)
 				})
 				return
 			}
-			if gitAuthLink.OAuthExpiry.Before(database.Now()) {
+
+			// Expiry may be unset if the application doesn't configure tokens
+			// to expire.
+			// See
+			// https://docs.github.com/en/apps/creating-github-apps/authenticating-with-a-github-app/generating-a-user-access-token-for-a-github-app.
+			if gitAuthLink.OAuthExpiry.Before(database.Now()) && !gitAuthLink.OAuthExpiry.IsZero() {
 				continue
 			}
 			if gitAuthConfig.ValidateURL != "" {
@@ -1932,20 +1912,11 @@ func (api *API) gitAuthCallback(gitAuthConfig *gitauth.Config) http.HandlerFunc
 			}
 		}
 
-		err = api.Pubsub.Publish("gitauth", []byte(fmt.Sprintf("%s|%s", gitAuthConfig.ID, apiKey.UserID)))
-		if err != nil {
-			httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
-				Message: "Failed to publish auth update.",
-				Detail:  err.Error(),
-			})
-			return
-		}
-
 		redirect := state.Redirect
 		if redirect == "" {
+			// This is a nicely rendered screen on the frontend
 			redirect = "/gitauth"
 		}
-		// This is a nicely rendered screen on the frontend
 		http.Redirect(rw, r, redirect, http.StatusTemporaryRedirect)
 	}
 }