diff --git a/CHANGELOG.md b/CHANGELOG.md index e14e58db6a..de732d6da1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Null Pointer Exception on anonymous migration with deleted repositories ([#1371](https://github.com/scm-manager/scm-manager/pull/1371)) - Null Pointer Exception on parsing SVN properties ([#1373](https://github.com/scm-manager/scm-manager/pull/1373)) +### Changed +- Reduced logging for invalid JWT or api keys ([#1374](https://github.com/scm-manager/scm-manager/pull/1374)) + ## [2.7.0] - 2020-10-12 ### Added - Users can create API keys with limited permissions ([#1359](https://github.com/scm-manager/scm-manager/pull/1359)) diff --git a/scm-webapp/src/main/java/sonia/scm/security/ApiKeyRealm.java b/scm-webapp/src/main/java/sonia/scm/security/ApiKeyRealm.java index ed2954eb32..672f972dab 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/ApiKeyRealm.java +++ b/scm-webapp/src/main/java/sonia/scm/security/ApiKeyRealm.java @@ -30,6 +30,8 @@ import org.apache.shiro.authc.UsernamePasswordToken; import org.apache.shiro.authc.credential.AllowAllCredentialsMatcher; import org.apache.shiro.authz.AuthorizationException; import org.apache.shiro.realm.AuthenticatingRealm; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import sonia.scm.plugin.Extension; import sonia.scm.repository.RepositoryRole; import sonia.scm.repository.RepositoryRoleManager; @@ -43,6 +45,8 @@ import static com.google.common.base.Preconditions.checkArgument; @Extension public class ApiKeyRealm extends AuthenticatingRealm { + private static final Logger LOG = LoggerFactory.getLogger(ApiKeyRealm.class); + private final ApiKeyService apiKeyService; private final DAORealmHelper helper; private final RepositoryRoleManager repositoryRoleManager; @@ -58,7 +62,14 @@ public class ApiKeyRealm extends AuthenticatingRealm { @Override public boolean supports(AuthenticationToken token) { - return token instanceof UsernamePasswordToken || token instanceof BearerToken; + if (token instanceof UsernamePasswordToken || token instanceof BearerToken) { + boolean containsDot = getPassword(token).contains("."); + if (containsDot) { + LOG.debug("Ignoring token with at least one dot ('.'); this is probably a JWT token"); + } + return !containsDot; + } + return false; } @Override @@ -74,6 +85,7 @@ public class ApiKeyRealm extends AuthenticatingRealm { private AuthenticationInfo buildAuthenticationInfo(AuthenticationToken token, ApiKeyService.CheckResult check) { RepositoryRole repositoryRole = determineRole(check); Scope scope = createScope(repositoryRole); + LOG.debug("login for user {} with api key limited to role {}", check.getUser(), check.getPermissionRole()); return helper .authenticationInfoBuilder(check.getUser()) .withSessionId(getPrincipal(token)) diff --git a/scm-webapp/src/main/java/sonia/scm/security/BearerRealm.java b/scm-webapp/src/main/java/sonia/scm/security/BearerRealm.java index e037590db6..f554c92d04 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/BearerRealm.java +++ b/scm-webapp/src/main/java/sonia/scm/security/BearerRealm.java @@ -21,7 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.security; import com.google.common.annotations.VisibleForTesting; @@ -29,6 +29,8 @@ import org.apache.shiro.authc.AuthenticationInfo; import org.apache.shiro.authc.AuthenticationToken; import org.apache.shiro.authc.credential.AllowAllCredentialsMatcher; import org.apache.shiro.realm.AuthenticatingRealm; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import sonia.scm.group.GroupDAO; import sonia.scm.plugin.Extension; import sonia.scm.user.UserDAO; @@ -54,6 +56,7 @@ public class BearerRealm extends AuthenticatingRealm @VisibleForTesting static final String REALM = "BearerRealm"; + private static final Logger LOG = LoggerFactory.getLogger(BearerRealm.class); /** dao realm helper */ private final DAORealmHelper helper; @@ -76,7 +79,17 @@ public class BearerRealm extends AuthenticatingRealm setAuthenticationTokenClass(BearerToken.class); } - //~--- methods -------------------------------------------------------------- + @Override + public boolean supports(AuthenticationToken token) { + if (token instanceof BearerToken) { + boolean containsDot = ((BearerToken) token).getCredentials().contains("."); + if (!containsDot) { + LOG.debug("Ignoring token without a dot ('.'); this probably is an API key"); + } + return containsDot; + } + return false; + } /** * Validates the given bearer token and retrieves authentication data from diff --git a/scm-webapp/src/main/java/sonia/scm/web/security/TokenRefreshFilter.java b/scm-webapp/src/main/java/sonia/scm/web/security/TokenRefreshFilter.java index 47da2d5718..be691da1d8 100644 --- a/scm-webapp/src/main/java/sonia/scm/web/security/TokenRefreshFilter.java +++ b/scm-webapp/src/main/java/sonia/scm/web/security/TokenRefreshFilter.java @@ -21,7 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.web.security; import org.apache.shiro.authc.AuthenticationException; @@ -90,6 +90,10 @@ public class TokenRefreshFilter extends HttpFilter { private void examineToken(HttpServletRequest request, HttpServletResponse response, BearerToken token) { AccessToken accessToken; + if (!token.getCredentials().contains(".")) { + LOG.trace("Ignoring token without dot. This probably is an API key, no JWT"); + return; + } try { accessToken = resolver.resolve(token); } catch (AuthenticationException e) { diff --git a/scm-webapp/src/test/java/sonia/scm/security/ApiKeyRealmTest.java b/scm-webapp/src/test/java/sonia/scm/security/ApiKeyRealmTest.java index 48a014dfb2..0f4171b8d9 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/ApiKeyRealmTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/ApiKeyRealmTest.java @@ -96,6 +96,15 @@ class ApiKeyRealmTest { assertThrows(AuthorizationException.class, () -> realm.doGetAuthenticationInfo(token)); } + @Test + void shouldIgnoreTokensWithDots() { + BearerToken token = valueOf("this.is.no.api.token"); + + boolean supports = realm.supports(token); + + assertThat(supports).isFalse(); + } + void verifyScopeSet(String... permissions) { verify(authenticationInfoBuilder).withScope(argThat(scope -> { assertThat(scope).containsExactly(permissions); diff --git a/scm-webapp/src/test/java/sonia/scm/security/ApiKeyTokenHandlerTest.java b/scm-webapp/src/test/java/sonia/scm/security/ApiKeyTokenHandlerTest.java index 390d5e6ba2..ac0ee44040 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/ApiKeyTokenHandlerTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/ApiKeyTokenHandlerTest.java @@ -61,4 +61,11 @@ class ApiKeyTokenHandlerTest { assertThat(token).isEmpty(); } + + @Test + void shouldParseRealWorldExample() { + Optional token = handler.readToken("eyJhcGlLZXlJZCI6IkE2U0ROWmV0MjEiLCJ1c2VyIjoiaG9yc3QiLCJwYXNzcGhyYXNlIjoiWGNKQ01PMnZuZ1JaOEhVU21BSVoifQ"); + + assertThat(token).get().extracting("user").isEqualTo("horst"); + } } diff --git a/scm-webapp/src/test/java/sonia/scm/security/BearerRealmTest.java b/scm-webapp/src/test/java/sonia/scm/security/BearerRealmTest.java index c0ac82b7f6..c9d834cdbc 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/BearerRealmTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/BearerRealmTest.java @@ -40,6 +40,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import static sonia.scm.security.BearerToken.valueOf; /** * Unit tests for {@link BearerRealm}. @@ -96,4 +97,13 @@ class BearerRealmTest { void shouldThrowIllegalArgumentExceptionForWrongTypeOfToken() { assertThrows(IllegalArgumentException.class, () -> realm.doGetAuthenticationInfo(new UsernamePasswordToken())); } + + @Test + void shouldIgnoreTokensWithoutDot() { + BearerToken token = valueOf("this-is-no-jwt-token"); + + boolean supports = realm.supports(token); + + assertThat(supports).isFalse(); + } } diff --git a/scm-webapp/src/test/java/sonia/scm/web/security/TokenRefreshFilterTest.java b/scm-webapp/src/test/java/sonia/scm/web/security/TokenRefreshFilterTest.java index d74a5eef7f..410eb1f368 100644 --- a/scm-webapp/src/test/java/sonia/scm/web/security/TokenRefreshFilterTest.java +++ b/scm-webapp/src/test/java/sonia/scm/web/security/TokenRefreshFilterTest.java @@ -21,7 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.web.security; import org.apache.shiro.authc.AuthenticationToken; @@ -52,6 +52,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static sonia.scm.security.BearerToken.valueOf; @ExtendWith({MockitoExtension.class}) class TokenRefreshFilterTest { @@ -103,7 +104,7 @@ class TokenRefreshFilterTest { @Test void shouldNotRefreshNonJwtToken() throws IOException, ServletException { - BearerToken token = mock(BearerToken.class); + BearerToken token = createValidToken(); JwtAccessToken jwtToken = mock(JwtAccessToken.class); when(tokenGenerator.createToken(request)).thenReturn(token); when(resolver.resolve(token)).thenReturn(jwtToken); @@ -116,7 +117,7 @@ class TokenRefreshFilterTest { @Test void shouldRefreshIfRefreshable() throws IOException, ServletException { - BearerToken token = mock(BearerToken.class); + BearerToken token = createValidToken(); JwtAccessToken jwtToken = mock(JwtAccessToken.class); JwtAccessToken newJwtToken = mock(JwtAccessToken.class); when(tokenGenerator.createToken(request)).thenReturn(token); @@ -128,4 +129,8 @@ class TokenRefreshFilterTest { verify(issuer).authenticate(request, response, newJwtToken); verify(filterChain).doFilter(request, response); } + + BearerToken createValidToken() { + return valueOf("some.jwt.token"); + } }