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

Issue #6554 - create the DefaultIdentityService even if no realmName is provided #6569

Merged
merged 3 commits into from Aug 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -62,7 +62,7 @@ public Authenticator getAuthenticator(Server server, ServletContext context, Aut
String auth = configuration.getAuthMethod();
Authenticator authenticator = null;

if (auth == null || Constraint.__BASIC_AUTH.equalsIgnoreCase(auth))
if (Constraint.__BASIC_AUTH.equalsIgnoreCase(auth))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a test for this case?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lachlan-roberts I agree that we need at least some simple test that checks a DIS is created if there is no realm. Ie not necessarily testing this specific line, but testing the PR in general.

authenticator = new BasicAuthenticator();
else if (Constraint.__DIGEST_AUTH.equalsIgnoreCase(auth))
authenticator = new DigestAuthenticator();
Expand Down
Expand Up @@ -293,9 +293,6 @@ protected IdentityService findIdentityService()
return getServer().getBean(IdentityService.class);
}

/**
*
*/
@Override
protected void doStart()
throws Exception
Expand Down Expand Up @@ -334,11 +331,8 @@ protected void doStart()

if (_identityService == null)
{
if (_realmName != null)
{
setIdentityService(new DefaultIdentityService());
manage(_identityService);
}
setIdentityService(new DefaultIdentityService());
manage(_identityService);
}
else
unmanage(_identityService);
Expand All @@ -352,7 +346,7 @@ else if (_loginService.getIdentityService() != _identityService)
throw new IllegalStateException("LoginService has different IdentityService to " + this);
}

if (_authenticator == null && _identityService != null)
if (_authenticator == null)
{
// If someone has set an authenticator factory only use that, otherwise try the list of discovered factories.
if (_authenticatorFactory != null)
Expand Down Expand Up @@ -399,7 +393,6 @@ else if (_realmName != null)
}

@Override

protected void doStop() throws Exception
{
//if we discovered the services (rather than had them explicitly configured), remove them.
Expand Down
@@ -0,0 +1,89 @@
//
// ========================================================================
// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others.
//
// This program and the accompanying materials are made available under the
// terms of the Eclipse Public License v. 2.0 which is available at
// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
// which is available at https://www.apache.org/licenses/LICENSE-2.0.
//
// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
// ========================================================================
//

package org.eclipse.jetty.security;

import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;

import org.eclipse.jetty.server.Authentication;
import org.eclipse.jetty.server.Server;
import org.junit.jupiter.api.Test;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.instanceOf;

public class DefaultIdentityServiceTest
{
@Test
public void testDefaultIdentityService() throws Exception
{
Server server = new Server();
ConstraintSecurityHandler securityHandler = new ConstraintSecurityHandler();
TestAuthenticator authenticator = new TestAuthenticator();
securityHandler.setAuthenticator(authenticator);

try
{
server.setHandler(securityHandler);
server.start();

// The DefaultIdentityService should have been created by default.
assertThat(securityHandler.getIdentityService(), instanceOf(DefaultIdentityService.class));
assertThat(authenticator.getIdentityService(), instanceOf(DefaultIdentityService.class));
}
finally
{
server.stop();
}
}

public static class TestAuthenticator implements Authenticator
{
private IdentityService _identityService;

public IdentityService getIdentityService()
{
return _identityService;
}

@Override
public void setConfiguration(AuthConfiguration configuration)
{
_identityService = configuration.getIdentityService();
}

@Override
public String getAuthMethod()
{
return getClass().getSimpleName();
}

@Override
public void prepareRequest(ServletRequest request)
{
}

@Override
public Authentication validateRequest(ServletRequest request, ServletResponse response, boolean mandatory) throws ServerAuthException
{
return null;
}

@Override
public boolean secureResponse(ServletRequest request, ServletResponse response, boolean mandatory, Authentication.User validatedUser) throws ServerAuthException
{
return false;
}
}
}