diff --git a/java/yb-pgsql/src/test/java/org/yb/pgsql/TestYbRoleProfile.java b/java/yb-pgsql/src/test/java/org/yb/pgsql/TestYbRoleProfile.java index fd66dc1c5ac7..4e4708eca4c5 100644 --- a/java/yb-pgsql/src/test/java/org/yb/pgsql/TestYbRoleProfile.java +++ b/java/yb-pgsql/src/test/java/org/yb/pgsql/TestYbRoleProfile.java @@ -280,9 +280,12 @@ public void testLogin() throws Exception { } assertProfileStateForUser(USERNAME, PRF_1_FAILED_ATTEMPTS + 1, false); - /* Now even the correct password will not let us in */ + /* + * Now even the correct password will not let us in. + * Failed attempts above the limit + 1 are not counted. + */ attemptLogin(USERNAME, PASSWORD); attemptLogin(USERNAME, "wrong"); - assertProfileStateForUser(USERNAME, PRF_1_FAILED_ATTEMPTS + 3, false); + assertProfileStateForUser(USERNAME, PRF_1_FAILED_ATTEMPTS + 1, false); } } diff --git a/src/postgres/src/backend/commands/ybc_profile.c b/src/postgres/src/backend/commands/ybc_profile.c index 5a39caa08892..56db877b0ab0 100644 --- a/src/postgres/src/backend/commands/ybc_profile.c +++ b/src/postgres/src/backend/commands/ybc_profile.c @@ -152,7 +152,7 @@ get_profile_oid(const char *prfname, bool missing_ok) heap_close(rel, AccessShareLock); if (!OidIsValid(result) && !missing_ok) - ereport(ERROR, + ereport(ERROR, (errcode(ERRCODE_UNDEFINED_OBJECT), errmsg("profile \"%s\" does not exist", prfname))); @@ -252,7 +252,7 @@ RemoveProfileById(Oid prfid) if (!HeapTupleIsValid(tuple)) { ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_OBJECT), + (errcode(ERRCODE_UNDEFINED_OBJECT), errmsg("profile with oid %u does not exist", prfid))); } @@ -373,12 +373,6 @@ get_role_oid_from_role_profile(Oid roleprfid) return roleid; } -/* - * This function does not check that the profile tables exist. It is either - * called before the database is initalized, or as a helper for another - * function that should do this verification. In either case, it is up to the - * caller to verify that this function can do the right thing. - */ HeapTuple get_role_profile_tuple(Oid roleid) { @@ -411,7 +405,7 @@ get_role_profile_tuple(Oid roleid) /* - * get_role_profile_oid - given a role oid, return the oid of the row in + * get_role_profile_oid - given a role oid, return the oid of the row in * pg_yb_role_profile for that role. * * If missing_ok is false, throw an error if role profile is not found. @@ -501,7 +495,6 @@ update_role_profile(Oid roleid, const char *rolename, Datum *new_record, * rolename: Name of the role. Required for error messages * prfname: Name of the profile. */ - void CreateRoleProfile(Oid roleid, const char *rolename, const char *prfname) { @@ -578,7 +571,7 @@ CreateRoleProfile(Oid roleid, const char *rolename, const char *prfname) * * roleid - the oid of the role * rolename - Name of the role. Used in the error message - * isEenabled - bool value + * isEnabled - bool value */ void EnableRoleProfile(Oid roleid, const char *rolename, bool is_enabled) @@ -690,9 +683,6 @@ IncFailedAttemptsAndMaybeDisableProfile(Oid roleid, const char *rolename) /* * YBCIncFailedAttemptsAndMaybeDisableProfile - increment failed_attempts * counter and disable if it exceeds limit - * This function does not check that the table exists. Since it is called - * before the database is initialized, it expects its caller to verify that - * the profile tables exist. * * roleid - the oid of the role */ @@ -703,8 +693,9 @@ YBCIncFailedAttemptsAndMaybeDisableProfile(Oid roleid) Form_pg_yb_role_profile rolprfform; Form_pg_yb_profile prfform; HeapTuple prftuple; - int failed_attempts; int failed_attempts_limit; + int current_failed_attempts; + int new_failed_attempts; bool rolisenabled; rolprftuple = get_role_profile_tuple(roleid); @@ -723,14 +714,18 @@ YBCIncFailedAttemptsAndMaybeDisableProfile(Oid roleid) prfform = (Form_pg_yb_profile) GETSTRUCT(prftuple); - failed_attempts = DatumGetInt16(rolprfform->rolfailedloginattempts) + 1; + current_failed_attempts = DatumGetInt16(rolprfform->rolfailedloginattempts); failed_attempts_limit = DatumGetInt16(prfform->prffailedloginattempts); + new_failed_attempts = current_failed_attempts < failed_attempts_limit + ? current_failed_attempts + 1 + : failed_attempts_limit + 1; + // Keep role enabled IFF role is enabled AND failed attempts < limit rolisenabled = rolprfform->rolisenabled && - (failed_attempts <= failed_attempts_limit); + (new_failed_attempts <= failed_attempts_limit); - YBCExecuteUpdateLoginAttempts(roleid, failed_attempts, rolisenabled); + YBCExecuteUpdateLoginAttempts(roleid, new_failed_attempts, rolisenabled); CommitTransactionCommand(); } diff --git a/src/postgres/src/backend/libpq/auth.c b/src/postgres/src/backend/libpq/auth.c index e3489be1a10e..94f3dc14f6d7 100644 --- a/src/postgres/src/backend/libpq/auth.c +++ b/src/postgres/src/backend/libpq/auth.c @@ -654,8 +654,8 @@ ClientAuthentication(Port *port) profileisdisabled = true; } } + ReleaseSysCache(roleTup); } - ReleaseSysCache(roleTup); if (status == STATUS_OK && !profileisdisabled) { @@ -667,7 +667,8 @@ ClientAuthentication(Port *port) } else { - if (roleid != InvalidOid) + /* Do not increment login attempts if no password was supplied */ + if (roleid != InvalidOid && status != STATUS_EOF) { YBCIncFailedAttemptsAndMaybeDisableProfile(roleid); } diff --git a/src/postgres/src/backend/utils/cache/relcache.c b/src/postgres/src/backend/utils/cache/relcache.c index ca757c9b1c96..e6035963e7ff 100644 --- a/src/postgres/src/backend/utils/cache/relcache.c +++ b/src/postgres/src/backend/utils/cache/relcache.c @@ -1942,7 +1942,7 @@ YBPreloadRelCache() YbRegisterSysTableForPrefetching(NamespaceRelationId); // pg_namespace YbRegisterSysTableForPrefetching(AuthIdRelationId); // pg_authid - if (YbLoginProfileCatalogsExist) + if (*YBCGetGFlags()->ysql_enable_profile && YbLoginProfileCatalogsExist) { YbRegisterSysTableForPrefetching(YbProfileRelationId); // yb_pg_profile YbRegisterSysTableForPrefetching(YbRoleProfileRelationId); // yb_pg_role_profile @@ -4133,8 +4133,8 @@ RelationBuildLocalRelation(const char *relname, * to the set of shared relations. */ if (shared_relation != IsSharedRelation(relid) && !yb_test_system_catalogs_creation) - elog(ERROR, "shared_relation flag for \"%s\" (%s) does not match IsSharedRelation(%u) (%s)", - relname, shared_relation ? "true" : "false", relid, IsSharedRelation(relid) ? "true" : "false"); + elog(ERROR, "shared_relation flag for \"%s\" does not match IsSharedRelation(%u)", + relname, relid); /* (Non-YB) shared relations had better be mapped, too */ Assert(IsYugaByteEnabled() || @@ -4519,7 +4519,7 @@ RelationCacheInitializePhase2(void) false, Natts_pg_shseclabel, Desc_pg_shseclabel); formrdesc("pg_subscription", SubscriptionRelation_Rowtype_Id, true, true, Natts_pg_subscription, Desc_pg_subscription); - if (YbLoginProfileCatalogsExist) + if (*YBCGetGFlags()->ysql_enable_profile && YbLoginProfileCatalogsExist) { formrdesc("pg_yb_profile", YbProfileRelation_Rowtype_Id, true, true, Natts_pg_yb_profile, Desc_pg_yb_profile); @@ -4527,7 +4527,7 @@ RelationCacheInitializePhase2(void) true, Natts_pg_yb_role_profile, Desc_pg_yb_role_profile); } -#define NUM_CRITICAL_SHARED_RELS (YbLoginProfileCatalogsExist ? 7 : 5) /* fix if you change list above */ +#define NUM_CRITICAL_SHARED_RELS (*YBCGetGFlags()->ysql_enable_profile && YbLoginProfileCatalogsExist ? 7 : 5) /* fix if you change list above */ } MemoryContextSwitchTo(oldcxt); diff --git a/src/postgres/src/backend/utils/init/postinit.c b/src/postgres/src/backend/utils/init/postinit.c index 8db901fab7e0..916faf75e204 100644 --- a/src/postgres/src/backend/utils/init/postinit.c +++ b/src/postgres/src/backend/utils/init/postinit.c @@ -704,7 +704,7 @@ InitPostgresImpl(const char *in_dbname, Oid dboid, const char *username, YbRegisterSysTableForPrefetching( AuthMemRelationId); // pg_auth_members - if (YbLoginProfileCatalogsExist) + if (*YBCGetGFlags()->ysql_enable_profile && YbLoginProfileCatalogsExist) { YbRegisterSysTableForPrefetching( YbProfileRelationId); // pg_yb_profile