Skip to content

Commit

Permalink
[PDI-12814] - Need to prohibit empty names when adding new users or r…
Browse files Browse the repository at this point in the history
…oles

- refactor UserRoleDelegate: replace manual duplication by introducing a common method
- prohibit creating users and roles with same names for DB and EE repositories
- for KettleDatabaseRepositorySecurityProvider prohibit outright to call saveUserInfo() for updating data
- add tests
  • Loading branch information
Andrey Khayrutdinov committed Oct 7, 2015
1 parent af1ae19 commit 80a3a8a
Show file tree
Hide file tree
Showing 5 changed files with 409 additions and 188 deletions.
Expand Up @@ -42,5 +42,5 @@ public interface RepositorySecurityUserValidator extends RepositorySecurityManag
* *
* @param user user's info * @param user user's info
*/ */
void normalizeUserInfo(IUser user); void normalizeUserInfo( IUser user );
} }
Expand Up @@ -91,11 +91,33 @@ public IUser loadUserInfo( String login ) throws KettleException {
return userDelegate.loadUserInfo( new UserInfo(), login ); return userDelegate.loadUserInfo( new UserInfo(), login );
} }


/**
* This method creates new user after all validations have been done. For updating user's data please use {@linkplain
* #updateUser(IUser)}.
*
* @param userInfo user's info
* @throws KettleException
* @throws IllegalArgumentException if {@code userInfo.getObjectId() != null}
*/
public void saveUserInfo( IUser userInfo ) throws KettleException { public void saveUserInfo( IUser userInfo ) throws KettleException {
normalizeUserInfo( userInfo ); normalizeUserInfo( userInfo );
if ( !validateUserInfo( userInfo ) ) { if ( !validateUserInfo( userInfo ) ) {
throw new KettleException( "Empty name is not allowed" ); throw new KettleException( "Empty name is not allowed" );
} }

if ( userInfo.getObjectId() != null ) {
throw new IllegalArgumentException( "Use updateUser() for updating" );
}

String userLogin = userInfo.getLogin();
ObjectId exactMatch = userDelegate.getUserID( userLogin );
if ( exactMatch != null ) {
// found the corresponding record in db, prohibit creation!
throw new KettleException(
"Cannot create a user with name [" + userLogin + "] because another one already exists: [" + userLogin
+ "]. Please try different name." );
}

userDelegate.saveUserInfo( userInfo ); userDelegate.saveUserInfo( userInfo );
} }


Expand Down
Expand Up @@ -28,12 +28,12 @@
import org.mockito.ArgumentCaptor; import org.mockito.ArgumentCaptor;
import org.pentaho.di.core.KettleEnvironment; import org.pentaho.di.core.KettleEnvironment;
import org.pentaho.di.core.exception.KettleException; import org.pentaho.di.core.exception.KettleException;
import org.pentaho.di.repository.StringObjectId;
import org.pentaho.di.repository.UserInfo; import org.pentaho.di.repository.UserInfo;
import org.pentaho.di.repository.kdr.delegates.KettleDatabaseRepositoryUserDelegate; import org.pentaho.di.repository.kdr.delegates.KettleDatabaseRepositoryUserDelegate;


import static org.junit.Assert.*; import static org.junit.Assert.*;
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.*;
import static org.mockito.Mockito.verify;


/** /**
* @author Andrey Khayrutdinov * @author Andrey Khayrutdinov
Expand All @@ -57,21 +57,45 @@ public void setUp() {
new KettleDatabaseRepositorySecurityProvider( repository, new KettleDatabaseRepositoryMeta(), new UserInfo() ); new KettleDatabaseRepositorySecurityProvider( repository, new KettleDatabaseRepositoryMeta(), new UserInfo() );
} }



@Test( expected = KettleException.class )
public void saveUserInfo_NormalizesInfo_FailsIfStillBreaches() throws Exception {
provider.saveUserInfo( new UserInfo( " " ) );
}

@Test( expected = KettleException.class )
public void saveUserInfo_CheckDuplication_FailsIfFoundSame() throws Exception {
testSaveUserInfo_Passes( "login", "login", "login" );
}


@Test
public void saveUserInfo_CheckDuplication_PassesIfFoundDifferenceInCase() throws Exception {
testSaveUserInfo_Passes( "login", "login", "LOGIN" );
}

@Test @Test
public void saveUserInfo_NormalizesInfo_PassesIfNoViolations() throws Exception { public void saveUserInfo_NormalizesInfo_PassesIfNoViolations() throws Exception {
UserInfo info = new UserInfo( "login " ); testSaveUserInfo_Passes( "login ", "login" );
}


ArgumentCaptor<UserInfo> captor = ArgumentCaptor.forClass( UserInfo.class ); @Test
provider.saveUserInfo( info ); public void saveUserInfo_CheckDuplication_PassesIfFoundNothing() throws Exception {
verify( repository.userDelegate ).saveUserInfo( captor.capture() ); testSaveUserInfo_Passes( "login", "login" );
}


info = captor.getValue(); private void testSaveUserInfo_Passes( String login, String expectedLogin ) throws Exception {
assertEquals( "Spaces should be trimmed", "login", info.getLogin() ); testSaveUserInfo_Passes( login, expectedLogin, "prefix_" + login );
} }


@Test( expected = KettleException.class ) private void testSaveUserInfo_Passes( String login, String expectedLogin, String existing ) throws Exception {
public void saveUserInfo_NormalizesInfo_FailsIfStillBreaches() throws Exception { doReturn( new StringObjectId( existing ) ).when( repository.userDelegate ).getUserID( eq( existing ) );
UserInfo info = new UserInfo( " " );
provider.saveUserInfo( info ); provider.saveUserInfo( new UserInfo( login ) );

ArgumentCaptor<UserInfo> captor = ArgumentCaptor.forClass( UserInfo.class );
verify( repository.userDelegate ).saveUserInfo( captor.capture() );

assertEquals( "UserInfo should be passed", expectedLogin, captor.getValue().getLogin() );
} }
} }

0 comments on commit 80a3a8a

Please sign in to comment.