From b5f042ad15bc29d6af78368635a70d2add16377c Mon Sep 17 00:00:00 2001
From: Konstantin Schaper <konstantin.schaper@cloudogu.com>
Date: Wed, 4 Nov 2020 09:38:29 +0100
Subject: [PATCH] use preexisiting scope by default but prevent overriding of
 builder scope and update unit tests

---
 .../sonia/scm/security/JwtAccessTokenBuilder.java    | 12 +++++++-----
 .../scm/security/JwtAccessTokenBuilderTest.java      |  8 +++++++-
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java
index 5e6bda9000..257aa10467 100644
--- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java
+++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java
@@ -40,9 +40,7 @@ import java.time.Clock;
 import java.time.Instant;
 import java.util.Date;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.Map;
-import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
 /**
@@ -71,7 +69,6 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder {
   private Instant refreshExpiration;
   private String parentKeyId;
   private Scope scope = Scope.empty();
-  private Set<String> groups = new HashSet<>();
 
   private final Map<String,Object> custom = Maps.newHashMap();
 
@@ -155,8 +152,13 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder {
 
   @Override
   public JwtAccessToken build() {
-    if (SecurityUtils.getSubject().getPrincipals().getRealmNames().contains(ApiKeyRealm.NAME)) {
-      scope = Scope.valueOf(SecurityUtils.getSubject().getPrincipals().oneByType(Scope.class));
+    final Scope principalScope = SecurityUtils.getSubject().getPrincipals().oneByType(Scope.class);
+    if (principalScope != null) {
+      if (scope != null && !scope.isEmpty()) {
+        throw new AuthorizationException(String.format("cannot merge builder scope (%s) with principal scope (%s)", scope, principalScope));
+      }
+      LOG.debug("using existing scope for new access token: {}", principalScope);
+      scope = principalScope;
     }
     String id = keyGenerator.createKey();
 
diff --git a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenBuilderTest.java b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenBuilderTest.java
index 071ca6df48..1888bac08c 100644
--- a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenBuilderTest.java
+++ b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenBuilderTest.java
@@ -146,13 +146,19 @@ class JwtAccessTokenBuilderTest {
     }
 
     @Test
-    void testSimpleRequest() {
+    void shouldCreateJwtAndUsePreviousScope() {
       JwtAccessTokenBuilder builder = factory.create().subject("dent");
       final JwtAccessToken accessToken = builder.build();
       assertThat(accessToken).isNotNull();
       assertThat(accessToken.getSubject()).isEqualTo("dent");
       assertThat((Collection<String>) accessToken.getCustom("scope").get()).containsExactly("dummy:scope:*");
     }
+
+    @Test
+    void shouldThrowExceptionWhenScopeAlreadyDefinedInBuilder() {
+      JwtAccessTokenBuilder builder = factory.create().scope(Scope.valueOf("an:incompatible:scope")).subject("dent");
+      assertThrows(AuthorizationException.class, builder::build);
+    }
   }
 
   @Nested