Skip to content

Commit 949e687

Browse files
EmyrkEmyrk
committed
PR feedback
1 parent e670557 commit 949e687

File tree

8 files changed

+75
-15
lines changed

8 files changed

+75
-15
lines changed

‎coderd/database/dbfake/dbfake.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4887,7 +4887,10 @@ func (q *fakeQuerier) UpdateUserLoginType(_ context.Context, arg database.Update
48874887

48884888
for i, u := range q.users {
48894889
if u.ID == arg.UserID {
4890-
u.LoginType = arg.LoginType
4890+
u.LoginType = arg.NewLoginType
4891+
if arg.NewLoginType != database.LoginTypePassword {
4892+
u.HashedPassword = []byte{}
4893+
}
48914894
q.users[i] = u
48924895
return u, nil
48934896
}

‎coderd/database/dbgen/dbgen.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ func User(t testing.TB, db database.Store, orig database.User) database.User {
199199
ID: takeFirst(orig.ID, uuid.New()),
200200
Email: takeFirst(orig.Email, namesgenerator.GetRandomName(1)),
201201
Username: takeFirst(orig.Username, namesgenerator.GetRandomName(1)),
202-
HashedPassword: takeFirstSlice(orig.HashedPassword, []byte{}),
202+
HashedPassword: takeFirstSlice(orig.HashedPassword, []byte(must(cryptorand.String(32)))),
203203
CreatedAt: takeFirst(orig.CreatedAt, database.Now()),
204204
UpdatedAt: takeFirst(orig.UpdatedAt, database.Now()),
205205
RBACRoles: takeFirstSlice(orig.RBACRoles, []string{}),

‎coderd/database/querier_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,51 @@ func TestUserLastSeenFilter(t *testing.T) {
441441
})
442442
}
443443

444+
func TestUserChangeLoginType(t *testing.T) {
445+
t.Parallel()
446+
if testing.Short() {
447+
t.SkipNow()
448+
}
449+
450+
sqlDB := testSQLDB(t)
451+
err := migrations.Up(sqlDB)
452+
require.NoError(t, err)
453+
db := database.New(sqlDB)
454+
ctx := context.Background()
455+
456+
alice := dbgen.User(t, db, database.User{
457+
LoginType: database.LoginTypePassword,
458+
})
459+
bob := dbgen.User(t, db, database.User{
460+
LoginType: database.LoginTypePassword,
461+
})
462+
bobExpPass := bob.HashedPassword
463+
require.NotEmpty(t, alice.HashedPassword, "hashed password should not start empty")
464+
require.NotEmpty(t, bob.HashedPassword, "hashed password should not start empty")
465+
466+
alice, err = db.UpdateUserLoginType(ctx, database.UpdateUserLoginTypeParams{
467+
NewLoginType: database.LoginTypeOIDC,
468+
UserID: alice.ID,
469+
})
470+
471+
require.NoError(t, err)
472+
require.Empty(t, alice.HashedPassword, "hashed password should be empty")
473+
474+
// First check other users are not affected
475+
bob, err = db.GetUserByID(ctx, bob.ID)
476+
require.NoError(t, err)
477+
require.Equal(t, bobExpPass, bob.HashedPassword, "hashed password should not change")
478+
479+
// Then check password -> password is a noop
480+
bob, err = db.UpdateUserLoginType(ctx, database.UpdateUserLoginTypeParams{
481+
NewLoginType: database.LoginTypePassword,
482+
UserID: bob.ID,
483+
})
484+
bob, err = db.GetUserByID(ctx, bob.ID)
485+
require.NoError(t, err)
486+
require.Equal(t, bobExpPass, bob.HashedPassword, "hashed password should not change")
487+
}
488+
444489
func requireUsersMatch(t testing.TB, expected []database.User, found []database.GetUsersRow, msg string) {
445490
t.Helper()
446491
require.ElementsMatch(t, expected, database.ConvertUserRows(found), msg)

‎coderd/database/queries.sql.go

Lines changed: 11 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/siteconfig.sql

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ SELECT
1717
COALESCE((SELECT value FROM site_configs WHERE key = 'default_proxy_icon_url'), '/emojis/1f3e1.png') :: text AS icon_url
1818
;
1919

20-
2120
-- name: InsertDeploymentID :exec
2221
INSERT INTO site_configs (key, value) VALUES ('deployment_id', $1);
2322

@@ -58,7 +57,6 @@ SELECT value FROM site_configs WHERE key = 'app_signing_key';
5857
INSERT INTO site_configs (key, value) VALUES ('app_signing_key', $1)
5958
ON CONFLICT (key) DO UPDATE set value = $1 WHERE site_configs.key = 'app_signing_key';
6059

61-
6260
-- name: GetOauthSigningKey :one
6361
SELECT value FROM site_configs WHERE key = 'oauth_signing_key';
6462

‎coderd/database/queries/users.sql

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,14 @@
22
UPDATE
33
users
44
SET
5-
login_type = @login_type
5+
login_type = @new_login_type,
6+
hashed_password = CASE WHEN @new_login_type = 'password' :: login_type THEN
7+
users.hashed_password
8+
ELSE
9+
-- If the login type is not password, then the password should be
10+
-- cleared.
11+
'':: bytea
12+
END
613
WHERE
714
id = @user_id RETURNING *;
815

‎coderd/database/types.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
package database
22

33
import (
4-
"database/sql/driver"
54
"encoding/json"
65
"time"
76

8-
"golang.org/x/xerrors"
7+
"database/sql/driver"
98

109
"github.com/google/uuid"
10+
"golang.org/x/xerrors"
1111

1212
"github.com/coder/coder/coderd/rbac"
1313
)

‎coderd/userauth.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -637,8 +637,8 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
637637
aReq.New = key
638638
aReq.UserID = key.UserID
639639

640-
for i := range cookies {
641-
http.SetCookie(rw, cookies[i])
640+
for _, cookie := range cookies {
641+
http.SetCookie(rw, cookie)
642642
}
643643

644644
redirect := state.Redirect
@@ -1392,8 +1392,8 @@ func (api *API) convertUserToOauth(ctx context.Context, r *http.Request, db data
13921392
// will be converted.
13931393
// nolint:gocritic // system query to update user login type
13941394
user, err = db.UpdateUserLoginType(dbauthz.AsSystemRestricted(ctx), database.UpdateUserLoginTypeParams{
1395-
LoginType: params.LoginType,
1396-
UserID: user.ID,
1395+
NewLoginType: params.LoginType,
1396+
UserID: user.ID,
13971397
})
13981398
if err != nil {
13991399
return database.User{}, httpError{

0 commit comments

Comments
 (0)