Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Permission-revamp-for-data-quality #19136

Closed
Closed
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions bootstrap/sql/migrations/native/1.6.2/mysql/schemaChanges.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
-- add timestamp index for test case result reindex performance
ALTER TABLE data_quality_data_time_series ADD INDEX `idx_timestamp_desc` (timestamp DESC);
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
-- add timestamp index for test case result reindex performance
CREATE INDEX idx_timestamp_desc ON data_quality_data_time_series (timestamp DESC);
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public final class CatalogExceptionMessage {
public static final String PASSWORD_INVALID_FORMAT =
"Password must be of minimum 8 characters, with one special, one Upper, one lower case character, and one Digit.";
public static final String MAX_FAILED_LOGIN_ATTEMPT =
"Failed Login Attempts Exceeded. Please try after some time.";
"Failed Login Attempts Exceeded. Use Forgot Password or retry after some time.";

public static final String INCORRECT_OLD_PASSWORD = "INCORRECT_OLD_PASSWORD";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import org.openmetadata.service.secrets.SecretsManager;
import org.openmetadata.service.secrets.SecretsManagerFactory;
import org.openmetadata.service.security.JwtFilter;
import org.openmetadata.service.security.auth.LoginAttemptCache;
import org.openmetadata.service.util.JsonUtils;
import org.openmetadata.service.util.OpenMetadataConnectionBuilder;
import org.openmetadata.service.util.RestUtil;
Expand Down Expand Up @@ -249,6 +250,10 @@ private void postUpdate(SettingsType settingsType) {
WorkflowHandler workflowHandler = WorkflowHandler.getInstance();
workflowHandler.initializeNewProcessEngine(workflowHandler.getProcessEngineConfiguration());
}

if (settingsType == SettingsType.LOGIN_CONFIGURATION) {
LoginAttemptCache.updateLoginConfiguration();
}
}

public void updateSetting(Settings setting) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ private static void createDefaultConfiguration(OpenMetadataApplicationConfig app
.withConfigValue(
new LoginConfiguration()
.withMaxLoginFailAttempts(3)
.withAccessBlockTime(600)
.withAccessBlockTime(30)
.withJwtTokenExpiryTime(3600));
systemRepository.createNewSetting(setting);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ public class BasicAuthenticator implements AuthenticatorHandler {
private static final int HASHING_COST = 12;
private UserRepository userRepository;
private TokenRepository tokenRepository;
private LoginAttemptCache loginAttemptCache;
private AuthorizerConfiguration authorizerConfiguration;
private boolean isSelfSignUpAvailable;

Expand All @@ -111,7 +110,6 @@ public void init(OpenMetadataApplicationConfig config) {
this.userRepository = (UserRepository) Entity.getEntityRepository(Entity.USER);
this.tokenRepository = Entity.getTokenRepository();
this.authorizerConfiguration = config.getAuthorizerConfiguration();
this.loginAttemptCache = new LoginAttemptCache();
this.isSelfSignUpAvailable = config.getAuthenticationConfiguration().getEnableSelfSignup();
}

Expand Down Expand Up @@ -267,7 +265,7 @@ public void resetUserPasswordWithToken(UriInfo uriInfo, PasswordResetRequest req
LOG.error("Error in sending Password Change Mail to User. Reason : " + ex.getMessage(), ex);
throw new CustomExceptionMessage(424, FAILED_SEND_EMAIL, EMAIL_SENDING_ISSUE);
}
loginAttemptCache.recordSuccessfulLogin(request.getUsername());
LoginAttemptCache.getInstance().recordSuccessfulLogin(request.getUsername());
}

@Override
Expand Down Expand Up @@ -312,7 +310,7 @@ public void changeUserPwdWithOldPwd(
storedUser.getAuthenticationMechanism().setConfig(storedBasicAuthMechanism);
PutResponse<User> response = userRepository.createOrUpdate(uriInfo, storedUser);
// remove login/details from cache
loginAttemptCache.recordSuccessfulLogin(userName);
LoginAttemptCache.getInstance().recordSuccessfulLogin(userName);

// in case admin updates , send email to user
if (request.getRequestType() == USER && getSmtpSettings().getEnableSmtpServer()) {
Expand Down Expand Up @@ -476,23 +474,23 @@ public JwtResponse loginUser(LoginRequest loginRequest) throws IOException, Temp

@Override
public void checkIfLoginBlocked(String email) {
if (loginAttemptCache.isLoginBlocked(email)) {
if (LoginAttemptCache.getInstance().isLoginBlocked(email)) {
throw new AuthenticationException(MAX_FAILED_LOGIN_ATTEMPT);
}
}

@Override
public void recordFailedLoginAttempt(String email, String userName)
throws TemplateException, IOException {
loginAttemptCache.recordFailedLogin(email);
int failedLoginAttempt = loginAttemptCache.getUserFailedLoginCount(email);
LoginAttemptCache.getInstance().recordFailedLogin(email);
int failedLoginAttempt = LoginAttemptCache.getInstance().getUserFailedLoginCount(email);
if (failedLoginAttempt == SecurityUtil.getLoginConfiguration().getMaxLoginFailAttempts()) {
sendAccountStatus(
userName,
email,
"Multiple Failed Login Attempts.",
String.format(
"Someone is trying to access your account. Login is Blocked for %s minutes. Please change your password.",
"Someone is trying to access your account. Login is Blocked for %s seconds. Please change your password.",
SecurityUtil.getLoginConfiguration().getAccessBlockTime()));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ public class LdapAuthenticator implements AuthenticatorHandler {
private RoleRepository roleRepository;
private UserRepository userRepository;
private TokenRepository tokenRepository;
private LoginAttemptCache loginAttemptCache;
private LdapConfiguration ldapConfiguration;
private LDAPConnectionPool ldapLookupConnectionPool;
private boolean isSelfSignUpEnabled;
Expand All @@ -102,7 +101,6 @@ public void init(OpenMetadataApplicationConfig config) {
this.roleRepository = (RoleRepository) Entity.getEntityRepository(Entity.ROLE);
this.tokenRepository = Entity.getTokenRepository();
this.ldapConfiguration = config.getAuthenticationConfiguration().getLdapConfiguration();
this.loginAttemptCache = new LoginAttemptCache();
this.isSelfSignUpEnabled = config.getAuthenticationConfiguration().getEnableSelfSignup();
}

Expand Down Expand Up @@ -176,16 +174,16 @@ private User checkAndCreateUser(String userDn, String email, String userName) th

@Override
public void checkIfLoginBlocked(String email) {
if (loginAttemptCache.isLoginBlocked(email)) {
if (LoginAttemptCache.getInstance().isLoginBlocked(email)) {
throw new AuthenticationException(MAX_FAILED_LOGIN_ATTEMPT);
}
}

@Override
public void recordFailedLoginAttempt(String email, String userName)
throws TemplateException, IOException {
loginAttemptCache.recordFailedLogin(email);
int failedLoginAttempt = loginAttemptCache.getUserFailedLoginCount(email);
LoginAttemptCache.getInstance().recordFailedLogin(email);
int failedLoginAttempt = LoginAttemptCache.getInstance().getUserFailedLoginCount(email);
if (failedLoginAttempt == SecurityUtil.getLoginConfiguration().getMaxLoginFailAttempts()) {
EmailUtil.sendAccountStatus(
userName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import io.dropwizard.logback.shaded.guava.annotations.VisibleForTesting;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import lombok.NonNull;
Expand All @@ -12,10 +13,11 @@
import org.openmetadata.service.resources.settings.SettingsCache;

public class LoginAttemptCache {
private static LoginAttemptCache INSTANCE;
private int maxAttempt = 3;
private final LoadingCache<String, Integer> attemptsCache;

public LoginAttemptCache() {
private LoginAttemptCache() {
LoginConfiguration loginConfiguration =
SettingsCache.getSetting(SettingsType.LOGIN_CONFIGURATION, LoginConfiguration.class);
long accessBlockTime = 600;
Expand All @@ -35,6 +37,18 @@ public LoginAttemptCache() {
});
}

public static LoginAttemptCache getInstance() {
if (INSTANCE == null) {
INSTANCE = new LoginAttemptCache();
}
return INSTANCE;
}

public static void updateLoginConfiguration() {
INSTANCE = new LoginAttemptCache();
}

@VisibleForTesting
public LoginAttemptCache(int maxAttempt, int blockTimeInSec) {
this.maxAttempt = maxAttempt;
attemptsCache =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ void get_Login_Configuration_200_OK() throws IOException {
LoginConfiguration loginConfiguration =
TestUtils.get(target, LoginConfiguration.class, TEST_AUTH_HEADERS);
assertEquals(3, loginConfiguration.getMaxLoginFailAttempts());
assertEquals(600, loginConfiguration.getAccessBlockTime());
assertEquals(30, loginConfiguration.getAccessBlockTime());
assertEquals(3600, loginConfiguration.getJwtTokenExpiryTime());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ void testLoginConfigurationSettings() throws HttpResponseException {

// Assert default values
assertEquals(3, loginConfig.getMaxLoginFailAttempts());
assertEquals(600, loginConfig.getAccessBlockTime());
assertEquals(30, loginConfig.getAccessBlockTime());
assertEquals(3600, loginConfig.getJwtTokenExpiryTime());

// Update login configuration
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,31 +19,31 @@ import React, { useEffect, useMemo, useState } from 'react';
import { useTranslation } from 'react-i18next';
import { Link, useParams } from 'react-router-dom';
import { getEntityDetailsPath } from '../../../../constants/constants';
import { usePermissionProvider } from '../../../../context/PermissionProvider/PermissionProvider';
import { ResourceEntity } from '../../../../context/PermissionProvider/PermissionProvider.interface';
import { EntityTabs, EntityType } from '../../../../enums/entity.enum';
import { ThreadType } from '../../../../generated/api/feed/createThread';
import { CreateTestCaseResolutionStatus } from '../../../../generated/api/tests/createTestCaseResolutionStatus';
import {
Thread,
ThreadTaskStatus,
} from '../../../../generated/entity/feed/thread';
import { Operation } from '../../../../generated/entity/policies/policy';
import { EntityReference } from '../../../../generated/tests/testCase';
import {
Severities,
TestCaseResolutionStatus,
TestCaseResolutionStatusTypes,
} from '../../../../generated/tests/testCaseResolutionStatus';
import { useTestCaseStore } from '../../../../pages/IncidentManager/IncidentManagerDetailPage/useTestCase.store';
import {
getListTestCaseIncidentByStateId,
postTestCaseIncidentStatus,
updateTestCaseIncidentById,
} from '../../../../rest/incidentManagerAPI';
import { getNameFromFQN } from '../../../../utils/CommonUtils';
import { getEntityName } from '../../../../utils/EntityUtils';
import {
getColumnNameFromEntityLink,
getEntityName,
} from '../../../../utils/EntityUtils';
import { getEntityFQN } from '../../../../utils/FeedUtils';
import { checkPermission } from '../../../../utils/PermissionsUtils';
import { getDecodedFqn } from '../../../../utils/StringsUtils';
import { getTaskDetailPath } from '../../../../utils/TasksUtils';
import { showErrorToast } from '../../../../utils/ToastUtils';
Expand All @@ -56,14 +56,14 @@ import { IncidentManagerPageHeaderProps } from './IncidentManagerPageHeader.inte

const IncidentManagerPageHeader = ({
onOwnerUpdate,
testCaseData,
fetchTaskCount,
}: IncidentManagerPageHeaderProps) => {
const { t } = useTranslation();
const [activeTask, setActiveTask] = useState<Thread>();
const [testCaseStatusData, setTestCaseStatusData] =
useState<TestCaseResolutionStatus>();
const [isLoading, setIsLoading] = useState(true);
const { testCase: testCaseData, testCasePermission } = useTestCaseStore();

const { fqn } = useParams<{ fqn: string }>();
const decodedFqn = getDecodedFqn(fqn);
Expand All @@ -76,6 +76,17 @@ const IncidentManagerPageHeader = ({
initialAssignees,
} = useActivityFeedProvider();

const columnName = useMemo(() => {
const isColumn = testCaseData?.entityLink.includes('::columns::');
if (isColumn) {
const name = getColumnNameFromEntityLink(testCaseData?.entityLink ?? '');

return name;
}

return null;
}, [testCaseData]);

const tableFqn = useMemo(
() => getEntityFQN(testCaseData?.entityLink ?? ''),
[testCaseData]
Expand Down Expand Up @@ -198,14 +209,14 @@ const IncidentManagerPageHeader = ({
}
}, [testCaseData]);

const { permissions } = usePermissionProvider();
const hasEditPermission = useMemo(() => {
return checkPermission(
Operation.EditAll,
ResourceEntity.TEST_CASE,
permissions
);
}, [permissions]);
const { hasEditStatusPermission, hasEditOwnerPermission } = useMemo(() => {
return {
hasEditStatusPermission:
testCasePermission?.EditAll || testCasePermission?.EditStatus,
hasEditOwnerPermission:
testCasePermission?.EditAll || testCasePermission?.EditOwners,
};
}, []);

const statusDetails = useMemo(() => {
if (isLoading) {
Expand Down Expand Up @@ -243,7 +254,7 @@ const IncidentManagerPageHeader = ({
className="font-medium"
data-testid="table-name"
to={getTaskDetailPath(activeTask)}>
{`#${activeTask?.task?.id}` ?? '--'}
{`#${activeTask?.task?.id}`}
</Link>
</Typography.Text>
</>
Expand All @@ -267,7 +278,7 @@ const IncidentManagerPageHeader = ({
<span className="text-grey-muted">{`${t('label.assignee')}: `}</span>

<OwnerLabel
hasPermission={hasEditPermission}
hasPermission={hasEditStatusPermission}
multiple={{
user: false,
team: false,
Expand Down Expand Up @@ -298,7 +309,7 @@ const IncidentManagerPageHeader = ({
return (
<Space wrap align="center">
<OwnerLabel
hasPermission={hasEditPermission}
hasPermission={hasEditOwnerPermission}
owners={testCaseData?.owners}
onUpdate={onOwnerUpdate}
/>
Expand Down Expand Up @@ -327,6 +338,17 @@ const IncidentManagerPageHeader = ({
</Typography.Text>
</>
)}
{columnName && (
<>
<Divider className="self-center m-x-sm" type="vertical" />
<Typography.Text className="self-center text-xs whitespace-nowrap">
<span className="text-grey-muted">{`${t('label.column')}: `}</span>
<span className="font-medium" data-testid="test-column-name">
{columnName}
</span>
</Typography.Text>
</>
)}
<Divider className="self-center m-x-sm" type="vertical" />
<Typography.Text className="self-center text-xs whitespace-nowrap">
<span className="text-grey-muted">{`${t('label.test-type')}: `}</span>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,9 @@ jest.mock('../../../../utils/CommonUtils', () => ({

jest.mock('../../../../utils/EntityUtils', () => ({
getEntityName: jest.fn().mockReturnValue('getEntityName'),
getColumnNameFromEntityLink: jest
.fn()
.mockReturnValue('getColumnNameFromEntityLink'),
}));

jest.mock('../../../../utils/FeedUtils', () => ({
Expand Down Expand Up @@ -270,5 +273,8 @@ describe('Incident Manager Page Header component', () => {
// Test Type
expect(screen.getByText('label.test-type:')).toBeInTheDocument();
expect(screen.getByText('getEntityName')).toBeInTheDocument();
// If Column is present
expect(screen.getByText('label.column:')).toBeInTheDocument();
expect(screen.getByText('getColumnNameFromEntityLink')).toBeInTheDocument();
});
});
Loading
Loading