Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

fix(coderd): ensure that user API keys are deleted when a user is#7270

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
johnstcn merged 1 commit intomainfromcj/invalidate_user_api_token
Apr 24, 2023
Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletioncoderd/apikey_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -195,7 +195,7 @@ func TestSessionExpiry(t *testing.T) {
}
}

funcTestAPIKey(t *testing.T) {
funcTestAPIKey_OK(t *testing.T) {
t.Parallel()
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
Expand All@@ -206,3 +206,20 @@ func TestAPIKey(t *testing.T) {
require.NoError(t, err)
require.Greater(t, len(res.Key), 2)
}

func TestAPIKey_Deleted(t *testing.T) {
t.Parallel()
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
user := coderdtest.CreateFirstUser(t, client)
_, anotherUser := coderdtest.CreateAnotherUser(t, client, user.OrganizationID)
require.NoError(t, client.DeleteUser(context.Background(), anotherUser.ID))

// Attempt to create an API key for the deleted user. This should fail.
_, err := client.CreateAPIKey(ctx, anotherUser.Username)
require.Error(t, err)
var apiErr *codersdk.Error
require.ErrorAs(t, err, &apiErr)
require.Equal(t, http.StatusBadRequest, apiErr.StatusCode())
}
13 changes: 13 additions & 0 deletionscoderd/database/dbfake/databasefake.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -931,6 +931,13 @@ func (q *fakeQuerier) UpdateUserDeletedByID(_ context.Context, params database.U
if u.ID == params.ID {
u.Deleted = params.Deleted
q.users[i] = u
// NOTE: In the real world, this is done by a trigger.
for i, k := range q.apiKeys {
if k.UserID == u.ID {
q.apiKeys[i] = q.apiKeys[len(q.apiKeys)-1]
q.apiKeys = q.apiKeys[:len(q.apiKeys)-1]
}
}
return nil
}
}
Expand DownExpand Up@@ -2768,6 +2775,12 @@ func (q *fakeQuerier) InsertAPIKey(_ context.Context, arg database.InsertAPIKeyP
arg.LifetimeSeconds = 86400
}

for _, u := range q.users {
if u.ID == arg.UserID && u.Deleted {
return database.APIKey{}, xerrors.Errorf("refusing to create APIKey for deleted user")
}
}

//nolint:gosimple
key := database.APIKey{
ID: arg.ID,
Expand Down
32 changes: 32 additions & 0 deletionscoderd/database/dump.sql
View file
Open in desktop

Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.

View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
BEGIN;

DROP TRIGGER IF EXISTS trigger_update_users ON users;
DROP FUNCTION IF EXISTS delete_deleted_user_api_keys;

DROP TRIGGER IF EXISTS trigger_insert_apikeys ON api_keys;
DROP FUNCTION IF EXISTS insert_apikey_fail_if_user_deleted;

COMMIT;
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
BEGIN;

-- We need to delete all existing API keys for soft-deleted users.
DELETE FROM
api_keys
WHERE
user_id
IN (
SELECT id FROM users WHERE deleted
);


-- When we soft-delete a user, we also want to delete their API key.
CREATE FUNCTION delete_deleted_user_api_keys() RETURNS trigger
LANGUAGE plpgsql
AS $$
DECLARE
BEGIN
IF (NEW.deleted) THEN
DELETE FROM api_keys
WHERE user_id = OLD.id;
END IF;
RETURN NEW;
END;
$$;


CREATE TRIGGER trigger_update_users
AFTER INSERT OR UPDATE ON users
FOR EACH ROW
WHEN (NEW.deleted = true)
EXECUTE PROCEDURE delete_deleted_user_api_keys();

-- When we insert a new api key, we want to fail if the user is soft-deleted.
CREATE FUNCTION insert_apikey_fail_if_user_deleted() RETURNS trigger
LANGUAGE plpgsql
AS $$

DECLARE
BEGIN
IF (NEW.user_id IS NOT NULL) THEN
IF (SELECT deleted FROM users WHERE id = NEW.user_id LIMIT 1) THEN
RAISE EXCEPTION 'Cannot create API key for deleted user';
END IF;
END IF;
RETURN NEW;
END;
$$;

CREATE TRIGGER trigger_insert_apikeys
BEFORE INSERT ON api_keys
FOR EACH ROW
EXECUTE PROCEDURE insert_apikey_fail_if_user_deleted();

COMMIT;
30 changes: 30 additions & 0 deletionscoderd/userauth_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -28,6 +28,36 @@ import (
"github.com/coder/coder/testutil"
)

func TestUserLogin(t *testing.T) {
t.Parallel()
t.Run("OK", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, nil)
user := coderdtest.CreateFirstUser(t, client)
anotherClient, anotherUser := coderdtest.CreateAnotherUser(t, client, user.OrganizationID)
_, err := anotherClient.LoginWithPassword(context.Background(), codersdk.LoginWithPasswordRequest{
Email: anotherUser.Email,
Password: "SomeSecurePassword!",
})
require.NoError(t, err)
})
t.Run("UserDeleted", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, nil)
user := coderdtest.CreateFirstUser(t, client)
anotherClient, anotherUser := coderdtest.CreateAnotherUser(t, client, user.OrganizationID)
client.DeleteUser(context.Background(), anotherUser.ID)
_, err := anotherClient.LoginWithPassword(context.Background(), codersdk.LoginWithPasswordRequest{
Email: anotherUser.Email,
Password: "SomeSecurePassword!",
})
require.Error(t, err)
var apiErr *codersdk.Error
require.ErrorAs(t, err, &apiErr)
require.Equal(t, http.StatusUnauthorized, apiErr.StatusCode())
})
}

func TestUserAuthMethods(t *testing.T) {
t.Parallel()
t.Run("Password", func(t *testing.T) {
Expand Down
9 changes: 8 additions & 1 deletioncoderd/users_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -285,7 +285,7 @@ func TestDeleteUser(t *testing.T) {
user := coderdtest.CreateFirstUser(t, client)
authz := coderdtest.AssertRBAC(t, api, client)

_, another := coderdtest.CreateAnotherUser(t, client, user.OrganizationID)
anotherClient, another := coderdtest.CreateAnotherUser(t, client, user.OrganizationID)
err := client.DeleteUser(context.Background(), another.ID)
require.NoError(t, err)
// Attempt to create a user with the same email and username, and delete them again.
Expand All@@ -299,6 +299,13 @@ func TestDeleteUser(t *testing.T) {
err = client.DeleteUser(context.Background(), another.ID)
require.NoError(t, err)

// IMPORTANT: assert that the deleted user's session is no longer valid.
_, err = anotherClient.User(context.Background(), codersdk.Me)
require.Error(t, err)
var apiErr *codersdk.Error
require.ErrorAs(t, err, &apiErr)
require.Equal(t, http.StatusUnauthorized, apiErr.StatusCode())

// RBAC checks
authz.AssertChecked(t, rbac.ActionCreate, rbac.ResourceUser)
authz.AssertChecked(t, rbac.ActionDelete, another)
Expand Down
[8]ページ先頭
©2009-2025 Movatter.jp