Merge branch 'develop' into bugfix/svn_props_null_check

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))

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))

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

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) {

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);

scm-webapp/src/test/java/sonia/scm/security/ApiKeyTokenHandlerTest.java

@@ -61,4 +61,11 @@ class ApiKeyTokenHandlerTest {
 
     assertThat(token).isEmpty();
   }
+
+  @Test
+  void shouldParseRealWorldExample() {
+    Optional<ApiKeyTokenHandler.Token> token = handler.readToken("eyJhcGlLZXlJZCI6IkE2U0ROWmV0MjEiLCJ1c2VyIjoiaG9yc3QiLCJwYXNzcGhyYXNlIjoiWGNKQ01PMnZuZ1JaOEhVU21BSVoifQ");
+
+    assertThat(token).get().extracting("user").isEqualTo("horst");
+  }
 }

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();
+  }
 }

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");
+  }
 }