Feb 16, 2024 · Feb 14, 2024 · Feb 15, 2024 · Feb 15, 2024 · Feb 16, 2024 · Feb 16, 2024
diff --git a/coderd/userauth.go b/coderd/userauth.go
 // with OIDC for the first time.
 if user.ID == uuid.Nil {
 var organizationID uuid.UUID
 // Ignoring this error is a product of our unit tests. In prod this should never
 // happen. Unit tests use this as a shortcut to making a new organization. We
 // should really fix our unit tests and remove this.
 //nolint:gocritic
 organizations, _ := tx.GetOrganizations(dbauthz.AsSystemRestricted(ctx))
 if len(organizations) > 0 {
 // Add the user to the first organization. Once multi-organization
 // support is added, we should enable a configuration map of user
 // email to organization.
 organizationID = organizations[0].ID
 }
 organization, _ := tx.GetDefaultOrganization(dbauthz.AsSystemRestricted(ctx))

 // Add the user to the default organization.
 // Once multi-organization we should check some configuration to see
 // if we should add the user to a different organization.
 organizationID = organization.ID

 //nolint:gocritic
 _, err := tx.GetUserByEmailOrUsername(dbauthz.AsSystemRestricted(ctx), database.GetUserByEmailOrUsernameParams{
 // All of the userauth tests depend on this being able to create
 // the first organization. It shouldn't be possible in normal
 // operation.
 CreateOrganization:len(organizations) ==0,
 CreateOrganization:organizationID ==uuid.Nil,
 LoginType:          params.LoginType,
 })
 if err != nil {
Original file line number	Diff line number	Diff line change
Expand Up		@@ -1342,14 +1342,16 @@ func (api API) oauthLogin(r http.Request, params oauthLoginParams) ([]http.C
		// with OIDC for the first time.
		if user.ID == uuid.Nil {
		var organizationID uuid.UUID
		// Ignoring this error is a product of our unit tests. In prod this should never
		// happen. Unit tests use this as a shortcut to making a new organization. We
		// should really fix our unit tests and remove this.
		//nolint:gocritic
		organizations, _ := tx.GetOrganizations(dbauthz.AsSystemRestricted(ctx))
		if len(organizations) > 0 {
		// Add the user to the first organization. Once multi-organization
		// support is added, we should enable a configuration map of user
		// email to organization.
		organizationID = organizations[0].ID
		}
		organization, _ := tx.GetDefaultOrganization(dbauthz.AsSystemRestricted(ctx))

		// Add the user to the default organization.
		// Once multi-organization we should check some configuration to see
		// if we should add the user to a different organization.
		organizationID = organization.ID
Comment on lines +1349 to +1354 Copy link MemberAuthor EmyrkFeb 15, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. This is annoyingly allowed to be`uuid.Nil` in unit tests. I am not going to change the behavior here, but we should really fix that so we don't have an edge case living for the sake of tests. mafredri reacted with thumbs up emoji Copy link Member mafredriFeb 16, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Fine not to fix here, but can we add a comment for the ignored err above (`tx.GetDefaultOrganization`) about this? Comment is just a band-aid though, we definitely want to nip this in the bud before too long. Emyrk reacted with thumbs up emoji

		//nolint:gocritic
		_, err := tx.GetUserByEmailOrUsername(dbauthz.AsSystemRestricted(ctx), database.GetUserByEmailOrUsernameParams{
Expand DownExpand Up		@@ -1395,7 +1397,7 @@ func (api API) oauthLogin(r http.Request, params oauthLoginParams) ([]http.C
		// All of the userauth tests depend on this being able to create
		// the first organization. It shouldn't be possible in normal
		// operation.
		CreateOrganization:len(organizations) ==0,
		CreateOrganization:organizationID ==uuid.Nil,
		LoginType: params.LoginType,
		})
		if err != nil {
Expand Down