Skip to content

Commit 3efe41b

Browse files
anilsahoo20anilsahoo20
committed
Fix coderabbit review comments
1 parent acae3f2 commit 3efe41b

File tree

8 files changed

+52
-27
lines changed

8 files changed

+52
-27
lines changed

‎web/pgadmin/browser/server_groups/servers/roles/__init__.py‎

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -824,18 +824,22 @@ def _set_rolemembers(self, row):
824824
rolmembers = []
825825
for role in row['rolmembers']:
826826
if self.manager.version < 160000:
827-
role = re.search(r'([01])(.+)', role)
827+
m = re.match(r'^([01])(.+)$', role or '')
828+
if not m:
829+
continue
828830
rolmembers.append({
829-
'role': role.group(2),
830-
'admin': True if role.group(1) == '1' else False
831+
'role': m.group(2),
832+
'admin': m.group(1) == '1'
831833
})
832834
else:
833-
role = re.search(r'([01])([01])([01])(.+)', role)
835+
m = re.match(r'^([01])([01])([01])(.+)$', role or '')
836+
if not m:
837+
continue
834838
rolmembers.append({
835-
'role': role.group(4),
836-
'admin': True if role.group(1) == '1' else False,
837-
'inherit': True if role.group(2) == '1' else False,
838-
'set': True if role.group(3) == '1' else False
839+
'role': m.group(4),
840+
'admin': m.group(1) == '1',
841+
'inherit': m.group(2) == '1',
842+
'set': m.group(3) == '1'
839843
})
840844
row['rolmembers'] = rolmembers
841845

@@ -846,18 +850,22 @@ def transform(self, rset):
846850
row['rolpassword'] = ''
847851
for role in roles:
848852
if self.manager.version < 160000:
849-
role = re.search(r'([01])(.+)', role)
853+
m = re.match(r'^([01])(.+)$', role or '')
854+
if not m:
855+
continue
850856
res.append({
851-
'role': role.group(2),
852-
'admin': True if role.group(1) == '1' else False
857+
'role': m.group(2),
858+
'admin': m.group(1) == '1'
853859
})
854860
else:
855-
role = re.search(r'([01])([01])([01])(.+)', role)
861+
m = re.match(r'^([01])([01])([01])(.+)$', role or '')
862+
if not m:
863+
continue
856864
res.append({
857-
'role': role.group(4),
858-
'admin': True if role.group(1) == '1' else False,
859-
'inherit': True if role.group(2) == '1' else False,
860-
'set': True if role.group(3) == '1' else False
865+
'role': m.group(4),
866+
'admin': m.group(1) == '1',
867+
'inherit': m.group(2) == '1',
868+
'set': m.group(3) == '1'
861869
})
862870
row['rolmembership'] = res
863871
self._set_seclabels(row)

‎web/pgadmin/browser/server_groups/servers/roles/static/js/role.ui.js‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,17 @@ export default class RoleSchema extends BaseUISchema {
5858
memberDataFormatter(rawData) {
5959
let members = '';
6060
if(_.isObject(rawData)) {
61-
const serverVersion = this.nodeInfo && this.nodeInfo.server && this.nodeInfo.server.version || 0;
61+
const serverVersion = this.nodeInfo?.server?.version || 0;
6262
rawData.forEach(member => {
63-
let badges = serverVersion >= 160000 ? ` [WITH ADMIN ${member.admin.toString().toUpperCase()}, INHERIT ${member.inherit.toString().toUpperCase()}, SET ${member.set.toString().toUpperCase()}]` : member.admin ? ' [WITH ADMIN OPTION]' : '';
63+
let badges = '';
64+
if (serverVersion >= 160000) {
65+
const admin = (member.admin ?? false).toString().toUpperCase();
66+
const inherit = (member.inherit ?? false).toString().toUpperCase();
67+
const set = (member.set ?? true).toString().toUpperCase();
68+
badges = ` [WITH ADMIN ${admin}, INHERIT ${inherit}, SET ${set}]`;
69+
} else {
70+
badges = member.admin ? ' [WITH ADMIN OPTION]' : '';
71+
}
6472

6573
if (members.length > 0) { members += ', '; }
6674
members = members + (member.role + badges);

‎web/pgadmin/browser/server_groups/servers/roles/templates/roles/sql/16_plus/create.sql‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,11 @@ CREATE ROLE {{ conn|qtIdent(data.rolname) }} WITH{% if data.rolcanlogin and data
3737
PASSWORD {% if data.rolpassword is none %}NULL{% else %}{% if dummy %}'xxxxxx'{% else %} {{ data.rolpassword|qtLiteral(conn) }}{% endif %}{% endif %}{% endif %};
3838
{% if data.rolmembership_list and data.rolmembership_list|length > 0 %}
3939
{% for item in data.rolmembership_list %}
40+
{% set admin_opt = item.admin|default(false) %}
41+
{% set inherit_opt = item.inherit|default(false) %}
42+
{% set set_opt = item.set|default(true) %}
4043

41-
GRANT {{ conn|qtIdent(item.role) }} TO {{ conn|qtIdent(data.rolname) }}{% if 'admin' in item or 'inherit' in item or 'set' in item %} WITH ADMIN {{ item.admin }}, INHERIT {{ item.inherit }}, SET {{ item.set }}{% endif %};
44+
GRANT {{ conn|qtIdent(item.role) }} TO {{ conn|qtIdent(data.rolname) }}{% if 'admin' in item or 'inherit' in item or 'set' in item %} WITH ADMIN {{ admin_opt }}, INHERIT {{ inherit_opt }}, SET {{ set_opt }}{% endif %};
4245
{% endfor %}
4346
{% endif %}
4447
{% if data.seclabels and data.seclabels|length > 0 %}
@@ -56,7 +59,10 @@ COMMENT ON ROLE {{ conn|qtIdent(data.rolname) }} IS {{ data.description|qtLitera
5659
{% endif %}
5760
{% if data.rol_members_list and data.rol_members_list|length > 0 %}
5861
{% for item in data.rol_members_list %}
62+
{% set admin_opt = item.admin|default(false) %}
63+
{% set inherit_opt = item.inherit|default(false) %}
64+
{% set set_opt = item.set|default(true) %}
5965

60-
GRANT {{ conn|qtIdent(data.rolname) }} TO {{ conn|qtIdent(item.role) }}{% if 'admin' in item or 'inherit' in item or 'set' in item %} WITH ADMIN {{ item.admin }}, INHERIT {{ item.inherit }}, SET {{ item.set }}{% endif %};
66+
GRANT {{ conn|qtIdent(data.rolname) }} TO {{ conn|qtIdent(item.role) }}{% if 'admin' in item or 'inherit' in item or 'set' in item %} WITH ADMIN {{ admin_opt }}, INHERIT {{ inherit_opt }}, SET {{ set_opt }}{% endif %};
6167
{% endfor %}
6268
{% endif %}

‎web/pgadmin/browser/server_groups/servers/roles/templates/roles/sql/16_plus/properties.sql‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,11 @@ SELECT
2525
pg_auth_members.admin_option AS admin_option,
2626
pg_auth_members.inherit_option AS inherit_option,
2727
pg_auth_members.set_option AS set_option
28-
FROM pg_roles
29-
JOIN pg_auth_members ON pg_roles.oid=pg_auth_members.member AND pg_auth_members.roleid={{ rid|qtLiteral(conn) }}::oid) pg
28+
FROM pg_catalog.pg_roles
29+
JOIN pg_catalog.pg_auth_members
30+
ON pg_roles.oid = pg_auth_members.member
31+
AND pg_auth_members.roleid = {{ rid|qtLiteral(conn) }}::oid) pg
32+
ORDER BY pg.usename
3033
) rolmembers
3134
{% endif %}
3235
FROM

‎web/pgadmin/browser/server_groups/servers/roles/templates/roles/sql/16_plus/sql.sql‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ FROM
1515
-- PostgreSQL >= 9.1
1616
CASE WHEN rolreplication THEN 'REPLICATION' ELSE 'NOREPLICATION' END || E'\n ' ||
1717
CASE WHEN rolbypassrls THEN 'BYPASSRLS' ELSE 'NOBYPASSRLS' END ||
18-
CASE WHEN rolconnlimit > 0 THEN E'\n CONNECTION LIMIT ' || rolconnlimit ELSE '' END ||
18+
CASE WHEN rolconnlimit != -1 THEN E'\n CONNECTION LIMIT ' || rolconnlimit ELSE '' END ||
1919
{% if show_password %}
2020
(SELECT CASE
2121
WHEN (rolpassword LIKE 'md5%%' or rolpassword LIKE 'SCRAM%%') THEN E'\n ENCRYPTED PASSWORD ''' || rolpassword || ''''

‎web/pgadmin/browser/server_groups/servers/roles/templates/roles/sql/16_plus/update.sql‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ ALTER ROLE {{ conn|qtIdent(rolname) }}{% if 'rolcanlogin' in data %}
4242
CONNECTION LIMIT {{ data.rolconnlimit }}
4343
{% endif %}{% if 'rolvaliduntil' in data %}
4444

45-
VALID UNTIL {% if data.rolvaliduntil %}{{ data.rolvaliduntil|qtLiteral(conn) }}{% else %}'infinity'
45+
VALID UNTIL {% if data.rolvaliduntil and data.rolvaliduntil is not none %}{{ data.rolvaliduntil|qtLiteral(conn) }}{% else %}'infinity'
4646
{% endif %}{% endif %}{% if 'rolpassword' in data %}
4747

4848
PASSWORD{% if data.rolpassword is none %} NULL{% else %}{% if dummy %} 'xxxxxx'{% else %} {{ data.rolpassword|qtLiteral(conn) }}{% endif %}{% endif %}{% endif %};{% endif %}
@@ -116,4 +116,4 @@ REVOKE {{ conn|qtIdent(rolname) }} FROM {{ conn|qtIdent(item.role) }};
116116

117117
GRANT {{ conn|qtIdent(rolname) }} TO {{ conn|qtIdent(item.role) }} {% if 'admin' in item or 'inherit' in item or 'set' in item %} WITH ADMIN {{ item.admin }}, INHERIT {{ item.inherit }}, SET {{ item.set }}{% endif %};
118118
{% endfor %}
119-
{% endif %}
119+
{% endif %}

‎web/pgadmin/browser/server_groups/servers/roles/templates/roles/sql/default/sql.sql‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ FROM
1515
-- PostgreSQL >= 9.1
1616
CASE WHEN rolreplication THEN 'REPLICATION' ELSE 'NOREPLICATION' END || E'\n ' ||
1717
CASE WHEN rolbypassrls THEN 'BYPASSRLS' ELSE 'NOBYPASSRLS' END ||
18-
CASE WHEN rolconnlimit > 0 THEN E'\n CONNECTION LIMIT ' || rolconnlimit ELSE '' END ||
18+
CASE WHEN rolconnlimit != -1 THEN E'\n CONNECTION LIMIT ' || rolconnlimit ELSE '' END ||
1919
{% if show_password %}
2020
(SELECT CASE
2121
WHEN (rolpassword LIKE 'md5%%' or rolpassword LIKE 'SCRAM%%') THEN E'\n ENCRYPTED PASSWORD ''' || rolpassword || ''''

‎web/pgadmin/browser/server_groups/servers/roles/templates/roles/sql/default/update.sql‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ ALTER ROLE {{ conn|qtIdent(rolname) }}{% if 'rolcanlogin' in data %}
4242
CONNECTION LIMIT {{ data.rolconnlimit }}
4343
{% endif %}{% if 'rolvaliduntil' in data %}
4444

45-
VALID UNTIL {% if data.rolvaliduntil %}{{ data.rolvaliduntil|qtLiteral(conn) }}{% else %}'infinity'
45+
VALID UNTIL {% if data.rolvaliduntil and data.rolvaliduntil is not none %}{{ data.rolvaliduntil|qtLiteral(conn) }}{% else %}'infinity'
4646
{% endif %}{% endif %}{% if 'rolpassword' in data %}
4747

4848
PASSWORD{% if data.rolpassword is none %} NULL{% else %}{% if dummy %} 'xxxxxx'{% else %} {{ data.rolpassword|qtLiteral(conn) }}{% endif %}{% endif %}{% endif %};{% endif %}

0 commit comments

Comments
 (0)