pr comments

coder · sreya · Aug 17, 2022 · Aug 9, 2022 · Aug 9, 2022 · Aug 9, 2022
commit 083d256c65640d68ae848f2201390f05c9263b0e
diff --git a/coderd/database/migrations/000034_linked_user_id.up.sql b/coderd/database/migrations/000034_linked_user_id.up.sql
@@ -7,8 +7,8 @@ CREATE TABLE IF NOT EXISTS user_links (
 	oauth_access_token text DEFAULT ''::text NOT NULL,
 	oauth_refresh_token text DEFAULT ''::text NOT NULL,
 	oauth_expiry timestamp with time zone DEFAULT '0001-01-01 00:00:00+00'::timestamp with time zone NOT NULL,
-	UNIQUE(user_id, login_type),
-  FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE
+	PRIMARY KEY(user_id, login_type),
+	FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE
 );
 
 -- This migrates columns on api_keys to the new user_links table.
@@ -18,34 +18,34 @@ CREATE TABLE IF NOT EXISTS user_links (
 -- A user should at most have a row for an OIDC account and a Github account.
 -- 'password' login types are ignored.
 
-INSERT INTO user_links 
-	( 
+INSERT INTO user_links
+	(
 		user_id,
 		login_type,
 		linked_id,
 		oauth_access_token,
 		oauth_refresh_token,
 		oauth_expiry
 	)
-SELECT 
-	keys.user_id, 
+SELECT
+	keys.user_id,
 	keys.login_type,
 	'',
 	keys.oauth_access_token,
 	keys.oauth_refresh_token,
-	keys.oauth_expiry 
-FROM 
-	( 
-		SELECT 
-			row_number() OVER (partition by user_id, login_type ORDER BY last_used DESC) AS x, 
+	keys.oauth_expiry
+FROM
+	(
+		SELECT
+			row_number() OVER (partition by user_id, login_type ORDER BY last_used DESC) AS x,
 			api_keys.* FROM api_keys
 	) as keys
- WHERE x=1 AND keys.login_type != 'password';
+WHERE x=1 AND keys.login_type != 'password';
 
 -- Drop columns that have been migrated to user_links.
 -- It appears the 'oauth_id_token' was unused and so it has
 -- been dropped here as well to avoid future confusion.
-ALTER TABLE api_keys 
+ALTER TABLE api_keys
 	DROP COLUMN oauth_access_token,
 	DROP COLUMN oauth_refresh_token,
 	DROP COLUMN oauth_id_token,
@@ -54,18 +54,18 @@ ALTER TABLE api_keys
 ALTER TABLE users ADD COLUMN login_type login_type NOT NULL DEFAULT 'password';
 
 UPDATE
-  users
+	users
 SET
-  login_type =  (
-    SELECT
-      login_type
-    FROM
-      user_links
-    WHERE
-      user_links.user_id = users.id
-    ORDER BY oauth_expiry DESC
-    LIMIT 1
-  )
+	login_type =  (
+		SELECT
+			login_type
+		FROM
+			user_links
+		WHERE
+			user_links.user_id = users.id
+		ORDER BY oauth_expiry DESC
+		LIMIT 1
+	)
 FROM
 	user_links
 WHERE

diff --git a/coderd/database/queries/user_links.sql b/coderd/database/queries/user_links.sql
@@ -1,46 +1,46 @@
 -- name: GetUserLinkByLinkedID :one
 SELECT
-  *
+	*
 FROM
-  user_links
+	user_links
 WHERE
-  linked_id = $1;
+	linked_id = $1;
 
 -- name: GetUserLinkByUserIDLoginType :one
 SELECT
-  *
+	*
 FROM
-  user_links
+	user_links
 WHERE
-  user_id = $1 AND login_type = $2; 
+	user_id = $1 AND login_type = $2;
 
 -- name: InsertUserLink :one
 INSERT INTO
-  user_links (
-    user_id,
-    login_type,
-    linked_id,
-    oauth_access_token,
-    oauth_refresh_token,
-    oauth_expiry
-  )
+	user_links (
+		user_id,
+		login_type,
+		linked_id,
+		oauth_access_token,
+		oauth_refresh_token,
+		oauth_expiry
+	)
 VALUES
-  ( $1, $2, $3, $4, $5, $6 ) RETURNING *;
+	( $1, $2, $3, $4, $5, $6 ) RETURNING *;
 
 -- name: UpdateUserLinkedID :one
 UPDATE
-  user_links
+	user_links
 SET
-  linked_id = $1
+	linked_id = $1
 WHERE
-  user_id = $2 AND login_type = $3 RETURNING *;
+	user_id = $2 AND login_type = $3 RETURNING *;
 
 -- name: UpdateUserLink :one
 UPDATE
-  user_links
+	user_links
 SET
-  oauth_access_token = $1,
-  oauth_refresh_token = $2,
-  oauth_expiry = $3
+	oauth_access_token = $1,
+	oauth_refresh_token = $2,
+	oauth_expiry = $3
 WHERE
-  user_id = $4 AND login_type = $5 RETURNING *;
+	user_id = $4 AND login_type = $5 RETURNING *;
diff --git a/coderd/database/queries/users.sql b/coderd/database/queries/users.sql
@@ -38,7 +38,7 @@ INSERT INTO
 		created_at,
 		updated_at,
 		rbac_roles,
-    login_type
+		login_type
 	)
 VALUES
 	($1, $2, $3, $4, $5, $6, $7, $8) RETURNING *;
@@ -55,12 +55,12 @@ WHERE
 
 -- name: UpdateUserRoles :one
 UPDATE
-    users
+	users
 SET
 	-- Remove all duplicates from the roles.
 	rbac_roles = ARRAY(SELECT DISTINCT UNNEST(@granted_roles :: text[]))
 WHERE
- 	id = @id
+	id = @id
 RETURNING *;
 
 -- name: UpdateUserHashedPassword :exec
@@ -123,8 +123,8 @@ WHERE
 	END
 	-- End of filters
 ORDER BY
-    -- Deterministic and consistent ordering of all users, even if they share
-    -- a timestamp. This is to ensure consistent pagination.
+	-- Deterministic and consistent ordering of all users, even if they share
+	-- a timestamp. This is to ensure consistent pagination.
 	(created_at, id) ASC OFFSET @offset_opt
 LIMIT
 	-- A null limit means "no limit", so 0 means return all
@@ -153,10 +153,10 @@ SELECT
 			array_append(users.rbac_roles, 'member'),
 		-- All org_members get the org-member role for their orgs
 			array_append(organization_members.roles, 'organization-member:'||organization_members.organization_id::text)) :: text[]
-	    AS roles
+		AS roles
 FROM
 	users
 LEFT JOIN organization_members
 	ON id = user_id
 WHERE
-    id = @user_id;
+	id = @user_id;
diff --git a/coderd/userauth.go b/coderd/userauth.go
@@ -148,7 +148,7 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
 
 	if user.ID != uuid.Nil && user.LoginType != database.LoginTypeGithub {
 		httpapi.Write(rw, http.StatusForbidden, codersdk.Response{
-			Message: fmt.Sprintf("Incorrect login type, attempting to use %q but user is of login type %q", database.LoginTypeOIDC, user.LoginType),
+			Message: fmt.Sprintf("Incorrect login type, attempting to use %q but user is of login type %q", database.LoginTypeGithub, user.LoginType),
 		})
 		return
 	}
@@ -215,7 +215,7 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
 		if err != nil {
 			httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{
 				Message: "A database error occurred.",
-				Detail:  xerrors.Errorf("insert user link: %w", err.Error).Error(),
+				Detail:  fmt.Sprintf("insert user link: %s", err.Error()),
 			})
 			return
 		}
@@ -358,6 +358,8 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// This can happen if a user is a built-in user but is signing in
+	// with OIDC for the first time.
 	if user.ID == uuid.Nil {
 		var organizationID uuid.UUID
 		organizations, _ := api.Database.GetOrganizations(ctx)
@@ -404,7 +406,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
 		if err != nil {
 			httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{
 				Message: "A database error occurred.",
-				Detail:  xerrors.Errorf("insert user link: %w", err.Error).Error(),
+				Detail:  fmt.Sprintf("insert user link: %s", err.Error()),
 			})
 			return
 		}