Skip to content

Commit

Permalink
Fix password change in enterprise auth manager
Browse files Browse the repository at this point in the history
Authentication with ShiroAuthManager is throwing ExpiredCredentialsException
(if a password change is required) before an identifying principal is assigned.
In this case we have to assign a principal identity with the username to allow
the user to change her password through the changePassword procedure.
At this point we know the username is valid.
  • Loading branch information
henriknyman committed May 26, 2016
1 parent 705ccb4 commit 63c6163
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 2 deletions.
Expand Up @@ -25,6 +25,8 @@
import org.apache.shiro.cache.ehcache.EhCacheManager;
import org.apache.shiro.mgt.DefaultSecurityManager;
import org.apache.shiro.mgt.SecurityManager;
import org.apache.shiro.subject.PrincipalCollection;
import org.apache.shiro.subject.SimplePrincipalCollection;
import org.apache.shiro.subject.Subject;

import java.io.IOException;
Expand Down Expand Up @@ -130,7 +132,8 @@ public AuthSubject login( String username, String password )
{
assertAuthEnabled();

Subject subject = new Subject.Builder( securityManager ).buildSubject();
// Start with an anonymous subject
Subject subject = buildSubject( null );

UsernamePasswordToken token = new UsernamePasswordToken( username, password );
AuthenticationResult result = AuthenticationResult.SUCCESS;
Expand All @@ -148,6 +151,10 @@ public AuthSubject login( String username, String password )
catch ( ExpiredCredentialsException e )
{
result = AuthenticationResult.PASSWORD_CHANGE_REQUIRED;

// We have to build an identity with the given username to allow the user to change password
// At this point we know that the username is valid
subject = buildSubject( username );
}
catch ( AuthenticationException e )
{
Expand Down Expand Up @@ -195,4 +202,16 @@ public boolean deleteUser( String username ) throws IOException
return realm.deleteUser( username );
}

private Subject buildSubject( String username )
{
Subject.Builder subjectBuilder = new Subject.Builder( securityManager );

if ( username != null )
{
PrincipalCollection identity = new SimplePrincipalCollection( username, realm.getName() );
subjectBuilder = subjectBuilder.principals( identity );
}

return subjectBuilder.buildSubject();
}
}
Expand Up @@ -74,7 +74,8 @@ public void setPassword( String password ) throws IOException, IllegalCredential

public boolean doesUsernameMatch( String username )
{
return subject.getPrincipal().equals( username );
Object principal = subject.getPrincipal();
return principal != null && username.equals( principal );
}

@Override
Expand Down
Expand Up @@ -214,6 +214,30 @@ public void shouldSetPassword() throws Throwable
assertThat( users.findByName( "jake" ), equalTo( user ) );
}

@Test
public void shouldSetPasswordThroughAuthSubject() throws Throwable
{
// Given
users.create( new User( "neo", Credential.forPassword( "abc123" ), true ) );
manager.start();
when( authStrategy.isAuthenticationPermitted( "neo" )).thenReturn( true );

// When
AuthSubject authSubject = manager.login( "neo", "abc123" );
assertThat( authSubject.getAuthenticationResult(), equalTo( AuthenticationResult.PASSWORD_CHANGE_REQUIRED ) );

authSubject.setPassword( "hello, world!" );

// Then
User user = manager.getUser( "neo" );
assertTrue( user.credentials().matchesPassword( "hello, world!" ) );
assertThat( users.findByName( "neo" ), equalTo( user ) );

authSubject.logout();
authSubject = manager.login( "neo", "hello, world!" );
assertThat( authSubject.getAuthenticationResult(), equalTo( AuthenticationResult.SUCCESS ) );
}

@Test
public void shouldReturnNullWhenSettingPasswordForUnknownUser() throws Throwable
{
Expand Down

0 comments on commit 63c6163

Please sign in to comment.