chore: PR comments

coder · deansheather · Mar 7, 2023 · Feb 10, 2023 · Feb 10, 2023 · Feb 11, 2023
commit 189eecf2aa53f94c01b8a8dd98d585434d2ae694
diff --git a/cli/server.go b/cli/server.go
@@ -592,7 +592,7 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co
 			err = options.Database.InTx(func(tx database.Store) error {
 				// This will block until the lock is acquired, and will be
 				// automatically released when the transaction ends.
-				err := tx.AcquireLock(ctx, database.LockID("deployment_startup"))
+				err := tx.AcquireLock(ctx, database.LockIDDeploymentSetup)
 				if err != nil {
 					return xerrors.Errorf("acquire lock: %w", err)
 				}

diff --git a/coderd/database/db.go b/coderd/database/db.go
@@ -12,7 +12,6 @@ import (
 	"context"
 	"database/sql"
 	"errors"
-	"hash/fnv"
 	"time"
 
 	"github.com/jmoiron/sqlx"
@@ -104,10 +103,3 @@ func (q *sqlQuerier) InTx(function func(Store) error, txOpts *sql.TxOptions) err
 	}
 	return nil
 }
-
-// LockID hashes the given string into an int64 to use with lock functions.
-func LockID(name string) int64 {
-	hash := fnv.New64()
-	_, _ = hash.Write([]byte(name))
-	return int64(hash.Sum64())
-}
diff --git a/coderd/database/lock.go b/coderd/database/lock.go
@@ -0,0 +1,8 @@
+package database
+
+// Well-known lock IDs for lock functions in the database. These should not
+// change. If locks are deprecated, they should be kept to avoid reusing the
+// same ID.
+const (
+	LockIDDeploymentSetup = iota + 1
+)
diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go
@@ -110,10 +110,6 @@ func (api *API) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request)
 				},
 				RedirectToLogin:             true,
 				DisableSessionExpiryRefresh: api.DeploymentConfig.DisableSessionExpiryRefresh.Value,
-				// Optional is true to allow for public apps. If an
-				// authorization check fails and the user is not authenticated,
-				// they will be redirected to the login page below.
-				Optional: false,
 			}),
 			httpmw.ExtractUserParam(api.Database, true),
 		})

diff --git a/coderd/workspaceapps/auth.go b/coderd/workspaceapps/auth.go
@@ -21,7 +21,7 @@ import (
 )
 
 const (
-	// TODO: configurable expiry
+	// TODO(@deansheather): configurable expiry
 	TicketExpiry = time.Minute
 
 	// RedirectURIQueryParam is the query param for the app URL to be passed
@@ -90,14 +90,15 @@ func (p *Provider) ResolveRequest(rw http.ResponseWriter, r *http.Request, appRe
 		ticketOK = false
 	)
 	httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{
-		DB:            p.Database,
-		OAuth2Configs: p.OAuth2Configs,
-		// Optional is true to allow for public apps. If an authorization check
-		// fails and the user is not authenticated, they will be redirected to
-		// the login page below.
+		DB:                          p.Database,
+		OAuth2Configs:               p.OAuth2Configs,
 		RedirectToLogin:             false,
 		DisableSessionExpiryRefresh: p.DeploymentConfig.DisableSessionExpiryRefresh.Value,
-		Optional:                    true,
+		// Optional is true to allow for public apps. If an authorization check
+		// fails and the user is not authenticated, they will be redirected to
+		// the login page using code below (not the redirect from the
+		// middleware itself).
+		Optional: true,
 	})(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		// Get user.
 		var (
@@ -486,7 +487,7 @@ func (p *Provider) writeWorkspaceApp500(rw http.ResponseWriter, r *http.Request,
 			slog.F("app_name_or_port", appReq.AppSlugOrPort),
 		)
 	}
-	p.Logger.Warn(r.Context(),
+	p.Logger.Warn(ctx,
 		"workspace app auth server error: "+msg,
 		slog.Error(err),
 	)

diff --git a/coderd/workspaceapps/provider.go b/coderd/workspaceapps/provider.go
@@ -11,8 +11,8 @@ import (
 )
 
 // Provider provides authentication and authorization for workspace apps.
-// TODO: also provide workspace apps as a whole to remove all app code from
-// coderd.
+// TODO(@deansheather): also provide workspace apps as a whole to remove all app
+// code from coderd.
 type Provider struct {
 	Logger slog.Logger