From 62056cab14e52017dd3a4c9a81ccc5a36508da65 Mon Sep 17 00:00:00 2001 From: Pravesh Sharma Date: Mon, 10 Jul 2023 10:36:15 +0530 Subject: [PATCH] Fixed sonaqube security smells and bugs 1. Delete unreachable code or refactor the code to make it reachable. 2. Unexpected var, use let or const instead. 3. Remove useless assignment to variable. 4. Define a constant instead of duplicating the literal 5. Remove commented out code --- runtime/src/js/pgadmin.js | 8 ++--- web/pgadmin/__init__.py | 9 ++--- web/pgadmin/authenticate/mfa/authenticator.py | 33 +++---------------- .../databases/casts/static/js/cast.ui.js | 12 ++++--- .../databases/publications/__init__.py | 7 ++-- .../schemas/domains/static/js/domain.ui.js | 5 +-- .../schemas/synonyms/static/js/synonym.ui.js | 5 +-- .../server_groups/servers/static/js/server.js | 2 -- web/pgadmin/static/js/pgadmin.js | 1 - .../SecurityPages/ForgotPasswordPage.spec.js | 1 - .../SecurityPages/LoginPage.spec.js | 1 - .../SecurityPages/MfaRegisterPage.spec.js | 1 - .../SecurityPages/MfaValidatePage.spec.js | 1 - .../SecurityPages/PasswordResetPage.spec.js | 1 - 14 files changed, 27 insertions(+), 60 deletions(-) diff --git a/runtime/src/js/pgadmin.js b/runtime/src/js/pgadmin.js index e4c710a07..8f7e62336 100644 --- a/runtime/src/js/pgadmin.js +++ b/runtime/src/js/pgadmin.js @@ -368,7 +368,7 @@ function addCommonMenus(menu) { let _menu = new gui.Menu(); menu.menuItems.forEach((menuItem) => { - var submenu = getSubMenu(menuItem); + let submenu = getSubMenu(menuItem); let _menuItem = new gui.MenuItem({ label: menuItem.label, @@ -417,8 +417,6 @@ function addCommonMenus(menu) { } function getRuntimeMenu() { - let controlKey = platform() === 'darwin' ? 'cmd' : 'ctrl'; - let fullScreenKey = platform() === 'darwin' ? 'F' : 'F10'; let subMenus = new gui.Menu(); let rtmenudt = pgAdminMainScreen.window.pgAdmin.Browser.RUNTIME_MENUS_OPTIONS['runtime'] let runtimeSubMenus = pgAdminMainScreen.window.pgAdmin.Browser.RUNTIME_MENUS_OPTIONS['runtime']['submenus'] @@ -558,7 +556,7 @@ function getSubMenu(menuItem) { function addMacMenu(menu) { if (menu.name == 'file' && platform() === 'darwin') { - var rootMenu = nativeMenu.items[0].submenu; + let rootMenu = nativeMenu.items[0].submenu; let indx = 0; menu.menuItems.forEach((menuItem) => { let submenu = getSubMenu(menuItem); @@ -652,7 +650,7 @@ function refreshMenuItems(menu) { } menu.menuItems.forEach((item) => { - var submenu = new gui.Menu(); + let submenu = new gui.Menu(); if (item.menu_items) { item.menu_items.forEach((subItem) => { submenu.append(new gui.MenuItem({ diff --git a/web/pgadmin/__init__.py b/web/pgadmin/__init__.py index 4e7267180..14b319a1b 100644 --- a/web/pgadmin/__init__.py +++ b/web/pgadmin/__init__.py @@ -68,6 +68,8 @@ socketio = SocketIO(manage_session=False, async_mode='threading', logger=False, engineio_logger=False, debug=False, ping_interval=25, ping_timeout=120) +_INDEX_PATH = 'browser.index' + class PgAdmin(Flask): def __init__(self, *args, **kwargs): @@ -126,8 +128,8 @@ class PgAdmin(Flask): # into endpoints ############################################################# wsgi_root_path = '' - if url_for('browser.index') != '/browser/': - wsgi_root_path = url_for('browser.index').replace( + if url_for(_INDEX_PATH) != '/browser/': + wsgi_root_path = url_for(_INDEX_PATH).replace( '/browser/', '' ) @@ -540,7 +542,7 @@ def create_app(app_name=None): # Make the Session more secure against XSS & CSRF when running in web mode if config.SERVER_MODE and config.ENHANCED_COOKIE_PROTECTION: paranoid = Paranoid(app) - paranoid.redirect_view = 'browser.index' + paranoid.redirect_view = _INDEX_PATH ########################################################################## # Load all available server drivers @@ -717,7 +719,6 @@ def create_app(app_name=None): except Exception as e: print(str(e)) db.session.rollback() - pass @user_logged_in.connect_via(app) @user_logged_out.connect_via(app) diff --git a/web/pgadmin/authenticate/mfa/authenticator.py b/web/pgadmin/authenticate/mfa/authenticator.py index dcc2c40d0..0722fff37 100644 --- a/web/pgadmin/authenticate/mfa/authenticator.py +++ b/web/pgadmin/authenticate/mfa/authenticator.py @@ -29,6 +29,7 @@ from pgadmin.utils.constants import MessageType _TOTP_AUTH_METHOD = "authenticator" _TOTP_AUTHENTICATOR = _("Authenticator App") +_OTP_PLACEHOLDER = _("Enter code") class TOTPAuthenticator(BaseMFAuth): @@ -113,7 +114,7 @@ class TOTPAuthenticator(BaseMFAuth): if totp.verify(code) is False: raise ValidationException("Invalid Code") - def validation_view(self) -> str: + def validation_view(self) -> dict: """ Generate the portion of the view to render on the authentication page @@ -125,10 +126,10 @@ class TOTPAuthenticator(BaseMFAuth): "Enter the code shown in your authenticator application for " "TOTP (Time-based One-Time Password)" ), - otp_placeholder=_("Enter code"), + otp_placeholder=_OTP_PLACEHOLDER, ) - def _registration_view(self) -> str: + def _registration_view(self) -> dict: """ Internal function to generate a view for the registration page. @@ -164,31 +165,7 @@ class TOTPAuthenticator(BaseMFAuth): auth_description=_( "Scan the QR code and the enter the code from the " "TOTP Authenticator application" - ), otp_placeholder=_("Enter code") - ) - - return "".join([ - "
{auth_title}
", - "", - "", - "{qrcode_alt_text}", - "
{auth_description}
", - "
", - "", - "
", - ]).format( - auth_title=_(_TOTP_AUTHENTICATOR), - auth_method=_TOTP_AUTH_METHOD, - image=img_base64.decode("utf-8"), - qrcode_alt_text=_("TOTP Authenticator QRCode"), - auth_description=_( - "Scan the QR code and the enter the code from the " - "TOTP Authenticator application" - ), otp_placeholder=_("Enter code") + ), otp_placeholder=_OTP_PLACEHOLDER ) def registration_view(self, form_data) -> Union[str, None]: diff --git a/web/pgadmin/browser/server_groups/servers/databases/casts/static/js/cast.ui.js b/web/pgadmin/browser/server_groups/servers/databases/casts/static/js/cast.ui.js index 861dc1ec1..deddb032c 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/casts/static/js/cast.ui.js +++ b/web/pgadmin/browser/server_groups/servers/databases/casts/static/js/cast.ui.js @@ -33,10 +33,14 @@ export default class CastSchema extends BaseUISchema { let srctype = state.srctyp; let trgtype = state.trgtyp; if(srctype != undefined && srctype != '' && - trgtype != undefined && trgtype != '') - return state.name = srctype+'->'+trgtype; - else - return state.name = ''; + trgtype != undefined && trgtype != '') { + state.name = srctype+'->'+trgtype; + return state.name; + } + else { + state.name = ''; + return state.name; + } } get baseFields() { diff --git a/web/pgadmin/browser/server_groups/servers/databases/publications/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/publications/__init__.py index 7d2cc37b3..ece195e87 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/publications/__init__.py +++ b/web/pgadmin/browser/server_groups/servers/databases/publications/__init__.py @@ -169,6 +169,7 @@ class PublicationView(PGChildNodeView, SchemaDiffObjectCompare): gettext("Could not find the publication information.") node_type = blueprint.node_type BASE_TEMPLATE_PATH = 'publications/{0}/#{1}#/sql' + GET_PUB_SCHEMAS_SQL = 'get_pub_schemas.sql' parent_ids = [ {'type': 'int', 'id': 'gid'}, @@ -380,7 +381,7 @@ class PublicationView(PGChildNodeView, SchemaDiffObjectCompare): if not res['rows'][0]['all_table']: if self.manager.version >= 150000: schema_name_sql = render_template( - "/".join([self.template_path, 'get_pub_schemas.sql']), + "/".join([self.template_path, self.GET_PUB_SCHEMAS_SQL]), pbid=pbid ) status, snames_list_res = self.conn.execute_dict( @@ -736,7 +737,7 @@ class PublicationView(PGChildNodeView, SchemaDiffObjectCompare): if self.manager.version >= 150000: schema_name_sql = render_template( - "/".join([self.template_path, 'get_pub_schemas.sql']), + "/".join([self.template_path, self.GET_PUB_SCHEMAS_SQL]), pbid=pbid ) status, snames_list_res = self.conn.execute_dict( @@ -949,7 +950,7 @@ class PublicationView(PGChildNodeView, SchemaDiffObjectCompare): if self.manager.version >= 150000: schema_name_sql = render_template( - "/".join([self.template_path, 'get_pub_schemas.sql']), + "/".join([self.template_path, self.GET_PUB_SCHEMAS_SQL]), pbid=pbid ) status, snames_list_res = self.conn.execute_dict( diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/domains/static/js/domain.ui.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/domains/static/js/domain.ui.js index 67fa1ee5a..14a466260 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/domains/static/js/domain.ui.js +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/domains/static/js/domain.ui.js @@ -40,10 +40,7 @@ export class DomainConstSchema extends BaseUISchema { type: 'checkbox', readonly: function(state) { let currCon = _.find(obj.top.origData.constraints, (con)=>con.conoid == state.conoid); - if (!obj.isNew(state) && currCon.convalidated) { - return true; - } - return false; + return !obj.isNew(state) && currCon.convalidated ? true : false; }, } ]; diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/synonyms/static/js/synonym.ui.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/synonyms/static/js/synonym.ui.js index cb48a7e08..90849ab9c 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/synonyms/static/js/synonym.ui.js +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/synonyms/static/js/synonym.ui.js @@ -97,10 +97,7 @@ export default class SynonymSchema extends BaseUISchema { }; }, readonly: function() { - if(!obj.inCatalog()) { - return false; - } - return true; + return !obj.inCatalog() ? false : true; } }, { id: 'is_sys_obj', label: gettext('System synonym?'), diff --git a/web/pgadmin/browser/server_groups/servers/static/js/server.js b/web/pgadmin/browser/server_groups/servers/static/js/server.js index a1d56a61e..5e5f65484 100644 --- a/web/pgadmin/browser/server_groups/servers/static/js/server.js +++ b/web/pgadmin/browser/server_groups/servers/static/js/server.js @@ -666,8 +666,6 @@ define('pgadmin.node.server', [ // Check the database server against supported version. checkSupportedVersion(_data.version, res.info); - // obj.trigger('connected', obj, _item, _data); - // Generate the event that server is connected pgBrowser.Events.trigger( 'pgadmin:server:connected', _data._id, _item, _data diff --git a/web/pgadmin/static/js/pgadmin.js b/web/pgadmin/static/js/pgadmin.js index ecede395b..6f4224e1e 100644 --- a/web/pgadmin/static/js/pgadmin.js +++ b/web/pgadmin/static/js/pgadmin.js @@ -26,7 +26,6 @@ define([], function() { _t = i; } _t = 'fontSize' === _r ? +t.parentNode || _t : _t; - // _t = _f ? _t : 'rem' === _c ? i : 'fontSize' === _r ? +t.parentNode || _t : _t; _f = _f || parseFloat(a(_t, 'fontSize')); _m = parseFloat(_e) * _f; } diff --git a/web/regression/javascript/SecurityPages/ForgotPasswordPage.spec.js b/web/regression/javascript/SecurityPages/ForgotPasswordPage.spec.js index 35f90e101..770eeeac7 100644 --- a/web/regression/javascript/SecurityPages/ForgotPasswordPage.spec.js +++ b/web/regression/javascript/SecurityPages/ForgotPasswordPage.spec.js @@ -21,7 +21,6 @@ describe('ForgotPasswordPage', ()=>{ /* https://material-ui.com/guides/testing/#api */ beforeAll(()=>{ mount = createMount(); - // spyOn(Notify, 'alert'); }); afterAll(() => { diff --git a/web/regression/javascript/SecurityPages/LoginPage.spec.js b/web/regression/javascript/SecurityPages/LoginPage.spec.js index ba22ca111..5772e5d0d 100644 --- a/web/regression/javascript/SecurityPages/LoginPage.spec.js +++ b/web/regression/javascript/SecurityPages/LoginPage.spec.js @@ -21,7 +21,6 @@ describe('LoginPage', ()=>{ /* https://material-ui.com/guides/testing/#api */ beforeAll(()=>{ mount = createMount(); - // spyOn(Notify, 'alert'); }); afterAll(() => { diff --git a/web/regression/javascript/SecurityPages/MfaRegisterPage.spec.js b/web/regression/javascript/SecurityPages/MfaRegisterPage.spec.js index 12462333b..13053a8df 100644 --- a/web/regression/javascript/SecurityPages/MfaRegisterPage.spec.js +++ b/web/regression/javascript/SecurityPages/MfaRegisterPage.spec.js @@ -21,7 +21,6 @@ describe('MfaRegisterPage', ()=>{ /* https://material-ui.com/guides/testing/#api */ beforeAll(()=>{ mount = createMount(); - // spyOn(Notify, 'alert'); }); afterAll(() => { diff --git a/web/regression/javascript/SecurityPages/MfaValidatePage.spec.js b/web/regression/javascript/SecurityPages/MfaValidatePage.spec.js index 4fe00b1ba..ced7b4d5a 100644 --- a/web/regression/javascript/SecurityPages/MfaValidatePage.spec.js +++ b/web/regression/javascript/SecurityPages/MfaValidatePage.spec.js @@ -21,7 +21,6 @@ describe('MfaValidatePage', ()=>{ /* https://material-ui.com/guides/testing/#api */ beforeAll(()=>{ mount = createMount(); - // spyOn(Notify, 'alert'); }); afterAll(() => { diff --git a/web/regression/javascript/SecurityPages/PasswordResetPage.spec.js b/web/regression/javascript/SecurityPages/PasswordResetPage.spec.js index 14411e8cb..e9cbf6a96 100644 --- a/web/regression/javascript/SecurityPages/PasswordResetPage.spec.js +++ b/web/regression/javascript/SecurityPages/PasswordResetPage.spec.js @@ -21,7 +21,6 @@ describe('PasswordResetPage', ()=>{ /* https://material-ui.com/guides/testing/#api */ beforeAll(()=>{ mount = createMount(); - // spyOn(Notify, 'alert'); }); afterAll(() => {