Skip to content

Commit

Permalink
Fix an issue auto-provisioned PostgreSQL user may keep old roles inde…
Browse files Browse the repository at this point in the history
…finitely (#33969)

* Fix an issue auto-provisioned PostgreSQL user may keep old roles indefinitely

* upper case the sql scripts
  • Loading branch information
greedy52 committed Nov 1, 2023
1 parent 0110879 commit a0bd616
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 36 deletions.
49 changes: 28 additions & 21 deletions lib/srv/db/postgres/activate-user.sql
Original file line number Diff line number Diff line change
@@ -1,28 +1,35 @@
create or replace procedure teleport_activate_user(username varchar, roles varchar[])
language plpgsql
as $$
declare
CREATE OR REPLACE PROCEDURE teleport_activate_user(username varchar, roles varchar[])
LANGUAGE plpgsql
AS $$
DECLARE
role_ varchar;
begin
cur_roles varchar[];
BEGIN
-- If the user already exists and was provisioned by Teleport, reactivate
-- it, otherwise provision a new one.
if exists (select * from pg_auth_members where roleid = (select oid from pg_roles where rolname = 'teleport-auto-user') and member = (select oid from pg_roles where rolname = username)) then
-- If the user has active connections, just use it to avoid messing up
-- its existing roles.
if exists (select usename from pg_stat_activity where usename = username) then
return;
end if;
IF EXISTS (SELECT * FROM pg_auth_members WHERE roleid = (SELECT oid FROM pg_roles WHERE rolname = 'teleport-auto-user') and member = (SELECT oid FROM pg_roles WHERE rolname = username)) THEN
-- If the user has active connections, make sure the provided roles
-- match what the user currently has.
IF EXISTS (SELECT usename FROM pg_stat_activity WHERE usename = username) THEN
SELECT CAST(array_agg(rolname) as varchar[]) INTO cur_roles FROM pg_auth_members JOIN pg_roles ON roleid = oid WHERE member=(SELECT oid FROM pg_roles WHERE rolname = username) AND rolname != 'teleport-auto-user';
-- "a <@ b" checks if all unique elements in "a" are contained by
-- "b". Using length check plus "contains" check to avoid sorting.
IF ARRAY_LENGTH(roles, 1) = ARRAY_LENGTH(cur_roles, 1) AND roles <@ cur_roles THEN
RETURN;
END IF;
RAISE EXCEPTION SQLSTATE 'TP002' USING MESSAGE = 'TP002: User has active connections and roles have changed';
END IF;
-- Otherwise reactivate the user, but first strip if of all roles to
-- account for scenarios with left-over roles if database agent crashed
-- and failed to cleanup upon session termination.
call teleport_deactivate_user(username);
execute format('alter user %I with login', username);
else
execute format('create user %I in role "teleport-auto-user"', username);
end if;
CALL teleport_deactivate_user(username);
EXECUTE FORMAT('ALTER USER %I WITH LOGIN', username);
ELSE
EXECUTE FORMAT('CREATE USER %I IN ROLE "teleport-auto-user"', username);
END IF;
-- Assign all roles to the created/activated user.
foreach role_ in array roles
loop
execute format('grant %I to %I', role_, username);
end loop;
end;$$;
FOREACH role_ IN ARRAY roles
LOOP
EXECUTE FORMAT('GRANT %I TO %I', role_, username);
END LOOP;
END;$$;
30 changes: 15 additions & 15 deletions lib/srv/db/postgres/deactivate-user.sql
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
create or replace procedure teleport_deactivate_user(username varchar)
language plpgsql
as $$
declare
CREATE OR REPLACE PROCEDURE teleport_deactivate_user(username varchar)
LANGUAGE plpgsql
AS $$
DECLARE
role_ varchar;
begin
BEGIN
-- Only deactivate if the user doesn't have other active sessions.
if exists (select usename from pg_stat_activity where usename = username) then
raise notice 'User has active connections';
else
IF EXISTS (SELECT usename FROM pg_stat_activity WHERE usename = username) THEN
RAISE NOTICE 'User has active connections';
ELSE
-- Revoke all role memberships except teleport-auto-user group.
for role_ in select a.rolname from pg_roles a where pg_has_role(username, a.oid, 'member') and a.rolname not in (username, 'teleport-auto-user')
loop
execute format('revoke %I from %I', role_, username);
end loop;
FOR role_ IN SELECT a.rolname FROM pg_roles a WHERE pg_has_role(username, a.oid, 'member') AND a.rolname NOT IN (username, 'teleport-auto-user')
LOOP
EXECUTE FORMAT('REVOKE %I FROM %I', role_, username);
END LOOP;
-- Disable ability to login for the user.
execute format('alter user %I with nologin', username);
end if;
end;$$;
EXECUTE FORMAT('ALTER USER %I WITH NOLOGIN', username);
END IF;
END;$$;
3 changes: 3 additions & 0 deletions lib/srv/db/postgres/users.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ func (e *Engine) ActivateUser(ctx context.Context, sessionCtx *common.Session) e
if strings.Contains(err.Error(), "already exists") {
return trace.AlreadyExists("user %q already exists in this PostgreSQL database and is not managed by Teleport", sessionCtx.DatabaseUser)
}
if strings.Contains(err.Error(), "TP002: User has active connections and roles have changed") {
return trace.CompareFailed("roles for user %q has changed. Please quit all active connections and try again.", sessionCtx.DatabaseUser)
}
return trace.Wrap(err)
}

Expand Down

0 comments on commit a0bd616

Please sign in to comment.