[Fix] - Return 401 on any authentication error
Fixes a bug where the wrong exception and wrong error status code (500) were returned when the user sent the wrong username/password. This fixes this beahviour to return an `AuhthenticationException` with a 401 status code. Fixes elastic/elasticsearch#271 Original commit: elastic/x-pack-elasticsearch@0a120caeae
This commit is contained in:
parent
1f540dbc50
commit
3ab8f57f34
|
@ -10,6 +10,7 @@ import org.elasticsearch.common.cache.CacheBuilder;
|
|||
import org.elasticsearch.common.component.AbstractComponent;
|
||||
import org.elasticsearch.common.settings.Settings;
|
||||
import org.elasticsearch.common.unit.TimeValue;
|
||||
import org.elasticsearch.common.util.concurrent.UncheckedExecutionException;
|
||||
import org.elasticsearch.rest.RestRequest;
|
||||
import org.elasticsearch.shield.User;
|
||||
import org.elasticsearch.shield.authc.AuthenticationException;
|
||||
|
@ -92,7 +93,7 @@ public abstract class CachingUsernamePasswordRealm extends AbstractComponent imp
|
|||
public UserWithHash call() throws Exception {
|
||||
User user = doAuthenticate(token);
|
||||
if (user == null) {
|
||||
throw new AuthenticationException("Could not authenticate ['" + token.principal() + "]");
|
||||
throw new AuthenticationException("Could not authenticate [" + token.principal() + "]");
|
||||
}
|
||||
return new UserWithHash(user, token.credentials(), hasher);
|
||||
}
|
||||
|
@ -108,8 +109,8 @@ public abstract class CachingUsernamePasswordRealm extends AbstractComponent imp
|
|||
userWithHash = cache.get(token.principal(), callback);
|
||||
return userWithHash.user;
|
||||
|
||||
} catch (ExecutionException ee) {
|
||||
logger.warn("Could not authenticate ['" + token.principal() + "]", ee);
|
||||
} catch (ExecutionException | UncheckedExecutionException ee) {
|
||||
logger.warn("Could not authenticate [" + token.principal() + "]", ee);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
|
|
@ -6,29 +6,18 @@
|
|||
package org.elasticsearch.shield.authc.support;
|
||||
|
||||
import org.elasticsearch.common.settings.ImmutableSettings;
|
||||
import org.elasticsearch.common.settings.Settings;
|
||||
import org.elasticsearch.shield.User;
|
||||
import org.elasticsearch.shield.authc.Realm;
|
||||
import org.elasticsearch.test.ElasticsearchTestCase;
|
||||
import org.junit.Test;
|
||||
|
||||
import java.util.concurrent.atomic.AtomicInteger;
|
||||
|
||||
import static org.hamcrest.Matchers.is;
|
||||
import static org.hamcrest.Matchers.*;
|
||||
|
||||
public class CachingUsernamePasswordRealmTests extends ElasticsearchTestCase {
|
||||
|
||||
static class AlwaysAuthenticateCachingRealm extends CachingUsernamePasswordRealm {
|
||||
public AlwaysAuthenticateCachingRealm() {
|
||||
super(ImmutableSettings.EMPTY);
|
||||
}
|
||||
public final AtomicInteger INVOCATION_COUNTER = new AtomicInteger(0);
|
||||
@Override protected User doAuthenticate(UsernamePasswordToken token) {
|
||||
INVOCATION_COUNTER.incrementAndGet();
|
||||
return new User.Simple(token.principal(), "testRole1", "testRole2");
|
||||
}
|
||||
|
||||
@Override public String type() { return "test"; }
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testCache(){
|
||||
AlwaysAuthenticateCachingRealm realm = new AlwaysAuthenticateCachingRealm();
|
||||
|
@ -63,4 +52,69 @@ public class CachingUsernamePasswordRealmTests extends ElasticsearchTestCase {
|
|||
|
||||
assertThat(realm.INVOCATION_COUNTER.intValue(), is(2));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testAutheticateContract() throws Exception {
|
||||
Realm<UsernamePasswordToken> realm = new FailingAuthenticationRealm(ImmutableSettings.EMPTY);
|
||||
User user = realm.authenticate(new UsernamePasswordToken("user", SecuredStringTests.build("pass")));
|
||||
assertThat(user , nullValue());
|
||||
|
||||
realm = new ThrowingAuthenticationRealm(ImmutableSettings.EMPTY);
|
||||
user = realm.authenticate(new UsernamePasswordToken("user", SecuredStringTests.build("pass")));
|
||||
assertThat(user , nullValue());
|
||||
}
|
||||
|
||||
static class FailingAuthenticationRealm extends CachingUsernamePasswordRealm {
|
||||
|
||||
FailingAuthenticationRealm(Settings settings) {
|
||||
super(settings);
|
||||
}
|
||||
|
||||
@Override
|
||||
protected User doAuthenticate(UsernamePasswordToken token) {
|
||||
return null;
|
||||
}
|
||||
|
||||
@Override
|
||||
public String type() {
|
||||
return "failing";
|
||||
}
|
||||
}
|
||||
|
||||
static class ThrowingAuthenticationRealm extends CachingUsernamePasswordRealm {
|
||||
|
||||
ThrowingAuthenticationRealm(Settings settings) {
|
||||
super(settings);
|
||||
}
|
||||
|
||||
@Override
|
||||
protected User doAuthenticate(UsernamePasswordToken token) {
|
||||
throw new RuntimeException("whatever exception");
|
||||
}
|
||||
|
||||
@Override
|
||||
public String type() {
|
||||
return "throwing";
|
||||
}
|
||||
}
|
||||
|
||||
static class AlwaysAuthenticateCachingRealm extends CachingUsernamePasswordRealm {
|
||||
|
||||
public final AtomicInteger INVOCATION_COUNTER = new AtomicInteger(0);
|
||||
|
||||
AlwaysAuthenticateCachingRealm() {
|
||||
super(ImmutableSettings.EMPTY);
|
||||
}
|
||||
|
||||
@Override
|
||||
protected User doAuthenticate(UsernamePasswordToken token) {
|
||||
INVOCATION_COUNTER.incrementAndGet();
|
||||
return new User.Simple(token.principal(), "testRole1", "testRole2");
|
||||
}
|
||||
|
||||
@Override
|
||||
public String type() {
|
||||
return "always";
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue