From 0b4eacd917404305e8ec6dcbf174352b8d3adbe9 Mon Sep 17 00:00:00 2001
From: Ammar Bandukwala <ammar@ammar.io>
Date: Mon, 1 May 2023 02:10:16 +0000
Subject: [PATCH] fix(coderd): don't hang on first gitauth clone

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 a token expiry date became the zero
date in the database.

Follow-up calls to `git clone` would succeed because the non-listen path doesn't
check expiry, perhaps by mistake.

In addition to fixing the zero date issue, this PR removes the 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 1b58c9f2c3c0c..e6ffa5d4d6ef9 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)
 	}
 }