Do not create web tokens for api keys

This fixes a way for privilege escalation with api keys.

CHANGELOG.md

@@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 - Handling of snapshot plugin dependencies ([#1384](https://github.com/scm-manager/scm-manager/pull/1384))
 - SyntaxHighlighting for GoLang ([#1386](https://github.com/scm-manager/scm-manager/pull/1386))
+- Privilege escalation for api keys ([#1388](https://github.com/scm-manager/scm-manager/pull/1388))
 
 ## [2.6.3] - 2020-10-16
 ### Fixed

scm-webapp/src/main/java/sonia/scm/security/ApiKeyRealm.java

@@ -46,6 +46,8 @@ import static com.google.common.base.Preconditions.checkArgument;
 @Extension
 public class ApiKeyRealm extends AuthenticatingRealm {
 
+  public static final String API_TOKEN_REALM_NAME = "ApiTokenRealm";
+
   private static final Logger LOG = LoggerFactory.getLogger(ApiKeyRealm.class);
 
   private final ApiKeyService apiKeyService;
@@ -55,7 +57,7 @@ public class ApiKeyRealm extends AuthenticatingRealm {
   @Inject
   public ApiKeyRealm(ApiKeyService apiKeyService, DAORealmHelperFactory helperFactory, RepositoryRoleManager repositoryRoleManager) {
     this.apiKeyService = apiKeyService;
-    this.helper = helperFactory.create("ApiTokenRealm");
+    this.helper = helperFactory.create(API_TOKEN_REALM_NAME);
     this.repositoryRoleManager = repositoryRoleManager;
     setAuthenticationTokenClass(BearerToken.class);
     setCredentialsMatcher(new AllowAllCredentialsMatcher());

scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.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.base.Preconditions;
@@ -31,7 +31,9 @@ import io.jsonwebtoken.Claims;
 import io.jsonwebtoken.Jwts;
 import io.jsonwebtoken.SignatureAlgorithm;
 import org.apache.shiro.SecurityUtils;
+import org.apache.shiro.authz.AuthorizationException;
 import org.apache.shiro.subject.Subject;
+import org.apache.shiro.util.ThreadContext;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -154,6 +156,9 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder {
 
   @Override
   public JwtAccessToken build() {
+    if (ThreadContext.getSubject().getPrincipals().getRealmNames().contains(ApiKeyRealm.API_TOKEN_REALM_NAME)) {
+      throw new AuthorizationException("Cannot create access token for api keys");
+    }
     String id = keyGenerator.createKey();
 
     String sub = getSubject();

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

@@ -21,122 +21,98 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-    
+
 package sonia.scm.security;
 
-import com.github.sdorra.shiro.ShiroRule;
-import com.github.sdorra.shiro.SubjectAware;
 import com.google.common.collect.Sets;
 import io.jsonwebtoken.Claims;
 import io.jsonwebtoken.Jwts;
+import org.apache.shiro.authz.AuthorizationException;
+import org.apache.shiro.subject.PrincipalCollection;
+import org.apache.shiro.subject.Subject;
 import org.apache.shiro.util.ThreadContext;
-import org.junit.Before;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.runner.RunWith;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.Mock;
-import org.mockito.junit.MockitoJUnitRunner;
+import org.mockito.junit.jupiter.MockitoExtension;
 
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
-import static org.hamcrest.Matchers.isEmptyOrNullString;
-import static org.hamcrest.Matchers.not;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.assertTrue;
+import static java.util.Collections.singleton;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.assertThrows;
 import static org.mockito.Mockito.anyString;
-import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.lenient;
 import static sonia.scm.security.SecureKeyTestUtil.createSecureKey;
 
 /**
  * Unit test for {@link JwtAccessTokenBuilder}.
- * 
+ *
  * @author Sebastian Sdorra
  */
-@RunWith(MockitoJUnitRunner.class)
-@SubjectAware(
-  configuration = "classpath:sonia/scm/shiro-001.ini",
-  username = "trillian",
-  password = "secret"
-)
-public class JwtAccessTokenBuilderTest {
-  
-  {
-    ThreadContext.unbindSubject();
-  }
+@ExtendWith(MockitoExtension.class)
+class JwtAccessTokenBuilderTest {
 
   @Mock
   private KeyGenerator keyGenerator;
-  
+
   @Mock
   private SecureKeyResolver secureKeyResolver;
-  
+
   private Set<AccessTokenEnricher> enrichers;
-  
+
   private JwtAccessTokenBuilderFactory factory;
-  
-  @Rule
-  public ShiroRule shiro = new ShiroRule();
-  
+
+  @Mock
+  private Subject subject;
+  @Mock
+  private PrincipalCollection principalCollection;
+
+  @BeforeEach
+  void bindSubject() {
+    lenient().when(subject.getPrincipal()).thenReturn("trillian");
+    lenient().when(subject.getPrincipals()).thenReturn(principalCollection);
+    ThreadContext.bind(subject);
+  }
+
+  @AfterEach
+  void unbindSubject() {
+    ThreadContext.unbindSubject();
+  }
+
   /**
    * Prepare mocks and set up object under test.
    */
-  @Before
-  public void setUpObjectUnderTest() {
-    when(keyGenerator.createKey()).thenReturn("42");
-    when(secureKeyResolver.getSecureKey(anyString())).thenReturn(createSecureKey());
+  @BeforeEach
+  void setUpObjectUnderTest() {
+    lenient().when(keyGenerator.createKey()).thenReturn("42");
+    lenient().when(secureKeyResolver.getSecureKey(anyString())).thenReturn(createSecureKey());
     enrichers = Sets.newHashSet();
     factory = new JwtAccessTokenBuilderFactory(keyGenerator, secureKeyResolver, enrichers);
   }
-  
-  /**
-   * Tests {@link JwtAccessTokenBuilder#build()} with subject from shiro context.
-   */
-  @Test
-  public void testBuildWithoutSubject() {
-    JwtAccessToken token = factory.create().build();
-    assertEquals("trillian", token.getSubject());
-  }
-  
-  /**
-   * Tests {@link JwtAccessTokenBuilder#build()} with explicit subject.
-   */
-  @Test
-  public void testBuildWithSubject() {
-    JwtAccessToken token = factory.create().subject("dent").build();
-    assertEquals("dent", token.getSubject());
-  }
-  
-  /**
-   * Tests {@link JwtAccessTokenBuilder#build()} with enricher.
-   */
-  @Test
-  public void testBuildWithEnricher() {
-    enrichers.add((b) -> b.custom("c", "d"));
-    JwtAccessToken token = factory.create().subject("dent").build();
-    assertEquals("d", token.getCustom("c").get());
-  }
-  
+
   /**
    * Tests {@link JwtAccessTokenBuilder#build()}.
    */
   @Test
-  public void testBuild(){
+  void testBuild() {
     JwtAccessToken token = factory.create().subject("dent")
       .issuer("https://www.scm-manager.org")
       .expiresIn(5, TimeUnit.SECONDS)
       .custom("a", "b")
       .scope(Scope.valueOf("repo:*"))
       .build();
-    
+
     // assert claims
     assertClaims(token);
-    
+
     // reparse and assert again
     String compact = token.compact();
-    assertThat(compact, not(isEmptyOrNullString()));
+    assertThat(compact).isNotEmpty();
     Claims claims = Jwts.parser()
       .setSigningKey(secureKeyResolver.getSecureKey("dent").getBytes())
       .parseClaimsJws(compact)
@@ -144,15 +120,67 @@ public class JwtAccessTokenBuilderTest {
     assertClaims(new JwtAccessToken(claims, compact));
   }
 
-  private void assertClaims(JwtAccessToken token){
-    assertThat(token.getId(), not(isEmptyOrNullString()));
-    assertNotNull( token.getIssuedAt() );
-    assertNotNull( token.getExpiration());
-    assertTrue(token.getExpiration().getTime() > token.getIssuedAt().getTime());
-    assertEquals("dent", token.getSubject());
-    assertTrue(token.getIssuer().isPresent());
-    assertEquals(token.getIssuer().get(), "https://www.scm-manager.org");
-    assertEquals("b", token.getCustom("a").get());
-    assertEquals("[\"repo:*\"]", token.getScope().toString());
+  private void assertClaims(JwtAccessToken token) {
+    assertThat(token.getId()).isNotEmpty();
+    assertThat(token.getIssuedAt()).isNotNull();
+    assertThat(token.getExpiration()).isNotNull();
+    assertThat(token.getExpiration().getTime() > token.getIssuedAt().getTime()).isTrue();
+    assertThat(token.getSubject()).isEqualTo("dent");
+    assertThat(token.getIssuer()).isNotEmpty();
+    assertThat(token.getIssuer()).get().isEqualTo("https://www.scm-manager.org");
+    assertThat(token.getCustom("a")).get().isEqualTo("b");
+    assertThat(token.getScope()).hasToString("[\"repo:*\"]");
+  }
+
+  @Nested
+  class FromApiKeyRealm {
+
+    @BeforeEach
+    void mockApiKeyRealm() {
+      lenient().when(principalCollection.getRealmNames()).thenReturn(singleton("ApiTokenRealm"));
+    }
+
+    @Test
+    void testRejectedRequest() {
+      JwtAccessTokenBuilder builder = factory.create().subject("dent");
+      assertThrows(AuthorizationException.class, builder::build);
+    }
+  }
+
+  @Nested
+  class FromDefaultRealm {
+
+    @BeforeEach
+    void mockDefaultRealm() {
+      lenient().when(principalCollection.getRealmNames()).thenReturn(singleton("DefaultRealm"));
+    }
+
+    /**
+     * Tests {@link JwtAccessTokenBuilder#build()} with subject from shiro context.
+     */
+    @Test
+    void testBuildWithoutSubject() {
+      JwtAccessToken token = factory.create().build();
+      assertThat(token.getSubject()).isEqualTo("trillian");
+    }
+
+    /**
+     * Tests {@link JwtAccessTokenBuilder#build()} with explicit subject.
+     */
+    @Test
+    void testBuildWithSubject() {
+      JwtAccessToken token = factory.create().subject("dent").build();
+      assertThat(token.getSubject()).isEqualTo("dent");
+    }
+
+    /**
+     * Tests {@link JwtAccessTokenBuilder#build()} with enricher.
+     */
+    @Test
+    void testBuildWithEnricher() {
+      enrichers.add((b) -> b.custom("c", "d"));
+      JwtAccessToken token = factory.create().subject("dent").build();
+      assertThat(token.getCustom("c")).get().isEqualTo("d");
+    }
   }
 }