From 0664303854c3d5fa55a47d0b1e58a08eea0e09ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Thu, 29 Nov 2018 08:01:02 +0100 Subject: [PATCH 01/20] Use new JWT library --- scm-webapp/pom.xml | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/scm-webapp/pom.xml b/scm-webapp/pom.xml index 4dfb749690..4503274adb 100644 --- a/scm-webapp/pom.xml +++ b/scm-webapp/pom.xml @@ -72,8 +72,18 @@ io.jsonwebtoken - jjwt - 0.4 + jjwt-impl + 0.10.5 + + + io.jsonwebtoken + jjwt-api + 0.10.5 + + + io.jsonwebtoken + jjwt-jackson + 0.10.5 From c85c0229c14a7be61b92e678a270fa51e5de47a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Thu, 29 Nov 2018 08:01:25 +0100 Subject: [PATCH 02/20] First steps for JWT refresh --- .../java/sonia/scm/security/AccessToken.java | 46 ++++++---- .../scm/security/AccessTokenBuilder.java | 12 ++- .../sonia/scm/security/JwtAccessToken.java | 14 ++- .../scm/security/JwtAccessTokenBuilder.java | 71 +++++++++------ .../JwtAccessTokenRefreshStrategy.java | 5 ++ .../scm/security/JwtAccessTokenRefresher.java | 53 ++++++++++++ .../security/JwtAccessTokenRefresherTest.java | 86 +++++++++++++++++++ .../resources/sonia/scm/repository/shiro.ini | 2 + 8 files changed, 241 insertions(+), 48 deletions(-) create mode 100644 scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefreshStrategy.java create mode 100644 scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java create mode 100644 scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java diff --git a/scm-core/src/main/java/sonia/scm/security/AccessToken.java b/scm-core/src/main/java/sonia/scm/security/AccessToken.java index 714b09eff8..ec448ad235 100644 --- a/scm-core/src/main/java/sonia/scm/security/AccessToken.java +++ b/scm-core/src/main/java/sonia/scm/security/AccessToken.java @@ -31,6 +31,7 @@ package sonia.scm.security; import java.util.Date; +import java.util.Map; import java.util.Optional; /** @@ -38,70 +39,77 @@ import java.util.Optional; * be issued from a restful webservice endpoint by providing credentials. After the token was issued, the token must be * send along with every request. The token should be send in its compact representation as bearer authorization header * or as cookie. - * + * * @author Sebastian Sdorra * @since 2.0.0 */ public interface AccessToken { - + /** * Returns unique id of the access token. - * + * * @return unique id */ String getId(); - + /** * Returns name of subject which identifies the principal. - * + * * @return name of subject */ String getSubject(); - + /** * Returns optional issuer. The issuer identifies the principal that issued the token. - * + * * @return optional issuer */ Optional getIssuer(); - + /** * Returns time at which the token was issued. - * + * * @return time at which the token was issued */ Date getIssuedAt(); - + /** * Returns the expiration time of token. - * + * * @return expiration time */ Date getExpiration(); - + + Date getRefreshExpiration(); + /** - * Returns the scope of the token. The scope is able to reduce the permissions of the subject in the context of this + * Returns the scope of the token. The scope is able to reduce the permissions of the subject in the context of this * token. For example we could issue a token which can only be used to read a single repository. for more informations * please have a look at {@link Scope}. - * + * * @return scope of token. */ Scope getScope(); - + /** * Returns an optional value of a custom token field. - * + * * @param type of field * @param key key of token field - * + * * @return optional value of custom field */ Optional getCustom(String key); - + /** * Returns compact representation of token. - * + * * @return compact representation */ String compact(); + + /** + * Returns read only map of all claim keys with their values. + */ + Map getClaims(); } diff --git a/scm-core/src/main/java/sonia/scm/security/AccessTokenBuilder.java b/scm-core/src/main/java/sonia/scm/security/AccessTokenBuilder.java index dd7986c22a..5e36ba468f 100644 --- a/scm-core/src/main/java/sonia/scm/security/AccessTokenBuilder.java +++ b/scm-core/src/main/java/sonia/scm/security/AccessTokenBuilder.java @@ -74,11 +74,21 @@ public interface AccessTokenBuilder { * Sets the expiration for the token. * * @param count expiration count - * @param unit expirtation unit + * @param unit expiration unit * * @return {@code this} */ AccessTokenBuilder expiresIn(long count, TimeUnit unit); + + /** + * Sets the time how long this token may be refreshed. Set this to 0 (zero) to disable automatic refresh. + * + * @param count Time unit count. If set to 0, automatic refresh is disabled. + * @param unit time unit + * + * @return {@code this} + */ + AccessTokenBuilder refreshableFor(long count, TimeUnit unit); /** * Reduces the permissions of the token by providing a scope. diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java index 46f4c68e74..35013e4fec 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java @@ -31,7 +31,10 @@ package sonia.scm.security; import io.jsonwebtoken.Claims; + +import java.util.Collections; import java.util.Date; +import java.util.Map; import java.util.Optional; /** @@ -75,6 +78,11 @@ public final class JwtAccessToken implements AccessToken { return claims.getExpiration(); } + @Override + public Date getRefreshExpiration() { + return claims.get("scm-manager.refreshableUntil", Date.class); + } + @Override public Scope getScope() { return Scopes.fromClaims(claims); @@ -90,5 +98,9 @@ public final class JwtAccessToken implements AccessToken { public String compact() { return compact; } - + + @Override + public Map getClaims() { + return Collections.unmodifiableMap(claims); + } } 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 ece96e2954..1207b252e4 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java @@ -39,7 +39,6 @@ import io.jsonwebtoken.SignatureAlgorithm; import java.util.Date; import java.util.HashMap; import java.util.Map; -import java.util.Set; import java.util.concurrent.TimeUnit; import org.apache.shiro.SecurityUtils; import org.apache.shiro.subject.Subject; @@ -48,7 +47,7 @@ import org.slf4j.LoggerFactory; /** * Jwt implementation of {@link AccessTokenBuilder}. - * + * * @author Sebastian Sdorra * @since 2.0.0 */ @@ -58,18 +57,20 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { * the logger for JwtAccessTokenBuilder */ private static final Logger LOG = LoggerFactory.getLogger(JwtAccessTokenBuilder.class); - - private final KeyGenerator keyGenerator; - private final SecureKeyResolver keyResolver; - + + private final KeyGenerator keyGenerator; + private final SecureKeyResolver keyResolver; + private String subject; private String issuer; - private long expiresIn = 60l; - private TimeUnit expiresInUnit = TimeUnit.MINUTES; + private long expiresIn = 1; + private TimeUnit expiresInUnit = TimeUnit.HOURS; + private long refreshableFor = 12; + private TimeUnit refreshableForUnit = TimeUnit.HOURS; private Scope scope = Scope.empty(); - + private final Map custom = Maps.newHashMap(); - + JwtAccessTokenBuilder(KeyGenerator keyGenerator, SecureKeyResolver keyResolver) { this.keyGenerator = keyGenerator; this.keyResolver = keyResolver; @@ -81,7 +82,7 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { this.subject = subject; return this; } - + @Override public JwtAccessTokenBuilder custom(String key, Object value) { Preconditions.checkArgument(!Strings.isNullOrEmpty(key), "null or empty value not allowed"); @@ -92,11 +93,11 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { @Override public JwtAccessTokenBuilder scope(Scope scope) { - Preconditions.checkArgument(scope != null, "scope can not be null"); + Preconditions.checkArgument(scope != null, "scope cannot be null"); this.scope = scope; return this; } - + @Override public JwtAccessTokenBuilder issuer(String issuer) { Preconditions.checkArgument(!Strings.isNullOrEmpty(issuer), "null or empty value not allowed"); @@ -106,15 +107,26 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { @Override public JwtAccessTokenBuilder expiresIn(long count, TimeUnit unit) { - Preconditions.checkArgument(count > 0, "expires in must be greater than 0"); - Preconditions.checkArgument(unit != null, "unit can not be null"); - + Preconditions.checkArgument(count > 0, "count must be greater than 0"); + Preconditions.checkArgument(unit != null, "unit cannot be null"); + this.expiresIn = count; this.expiresInUnit = unit; - + return this; } - + + @Override + public JwtAccessTokenBuilder refreshableFor(long count, TimeUnit unit) { + Preconditions.checkArgument(count >= 0, "count must be greater or equal to 0"); + Preconditions.checkArgument(unit != null, "unit cannot be null"); + + this.refreshableFor = count; + this.refreshableForUnit = unit; + + return this; + } + private String getSubject(){ if (subject == null) { Subject currentSubject = SecurityUtils.getSubject(); @@ -130,35 +142,40 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { String id = keyGenerator.createKey(); String sub = getSubject(); - + LOG.trace("create new token {} for user {}", id, subject); SecureKey key = keyResolver.getSecureKey(sub); - + Map customClaims = new HashMap<>(custom); - + // add scope to custom claims Scopes.toClaims(customClaims, scope); - + Date now = new Date(); long expiration = expiresInUnit.toMillis(expiresIn); - + Claims claims = Jwts.claims(customClaims) .setSubject(sub) .setId(id) .setIssuedAt(now) .setExpiration(new Date(now.getTime() + expiration)); - + + if (refreshableFor > 0) { + long refreshExpiration = refreshableForUnit.toMillis(refreshableFor); + claims.put("scm-manager.refreshableUntil", new Date(now.getTime() + refreshExpiration).getTime() / 1000); + } + if ( issuer != null ) { claims.setIssuer(issuer); } - + // sign token and create compact version String compact = Jwts.builder() .setClaims(claims) .signWith(SignatureAlgorithm.HS256, key.getBytes()) .compact(); - + return new JwtAccessToken(claims, compact); } - + } diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefreshStrategy.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefreshStrategy.java new file mode 100644 index 0000000000..47d6a09285 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefreshStrategy.java @@ -0,0 +1,5 @@ +package sonia.scm.security; + +public interface JwtAccessTokenRefreshStrategy { + boolean shouldBeRefreshed(JwtAccessToken oldToken); +} diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java new file mode 100644 index 0000000000..cb26d1f010 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java @@ -0,0 +1,53 @@ +package sonia.scm.security; + +import java.time.Instant; +import java.util.Date; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.TimeUnit; + +public class JwtAccessTokenRefresher { + + private final JwtAccessTokenBuilderFactory builderFactory; + private final JwtAccessTokenRefreshStrategy refreshStrategy; + + public JwtAccessTokenRefresher(JwtAccessTokenBuilderFactory builderFactory, JwtAccessTokenRefreshStrategy refreshStrategy) { + this.builderFactory = builderFactory; + this.refreshStrategy = refreshStrategy; + } + + public Optional refresh(JwtAccessToken oldToken) { + JwtAccessTokenBuilder builder = builderFactory.create(); + Map claims = oldToken.getClaims(); + claims.forEach(builder::custom); + + if (canBeRefreshed(oldToken) && shouldBeRefreshed(oldToken)) { + builder.expiresIn(1, TimeUnit.HOURS); +// builder.custom("scm-manager.parentTokenId") + return Optional.of(builder.build()); + } else { + return Optional.empty(); + } + } + + private boolean canBeRefreshed(JwtAccessToken oldToken) { + return tokenIsValid(oldToken) || tokenCanBeRefreshed(oldToken); + } + + private boolean shouldBeRefreshed(JwtAccessToken oldToken) { + return refreshStrategy.shouldBeRefreshed(oldToken); + } + + private boolean tokenCanBeRefreshed(JwtAccessToken oldToken) { + Date refreshExpiration = oldToken.getRefreshExpiration(); + return refreshExpiration != null && isBeforeNow(refreshExpiration); + } + + private boolean tokenIsValid(JwtAccessToken oldToken) { + return isBeforeNow(oldToken.getExpiration()); + } + + private boolean isBeforeNow(Date expiration) { + return expiration.toInstant().isBefore(Instant.now()); + } +} diff --git a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java new file mode 100644 index 0000000000..1464bfeade --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java @@ -0,0 +1,86 @@ +package sonia.scm.security; + +import com.github.sdorra.shiro.ShiroRule; +import com.github.sdorra.shiro.SubjectAware; +import org.assertj.core.api.Assertions; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import java.util.Collections; +import java.util.Optional; +import java.util.Random; +import java.util.concurrent.TimeUnit; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.when; + +@SubjectAware( + username = "user", + password = "secret", + configuration = "classpath:sonia/scm/repository/shiro.ini" +) +@RunWith(MockitoJUnitRunner.class) +public class JwtAccessTokenRefresherTest { + + @Rule + public ShiroRule shiro = new ShiroRule(); + + @Mock + private SecureKeyResolver keyResolver; + @Mock + private JwtAccessTokenRefreshStrategy refreshStrategy; + private JwtAccessTokenBuilderFactory builderFactory; + private JwtAccessTokenRefresher refresher; + private JwtAccessTokenBuilder tokenBuilder; + + @Before + public void initKeyResolver() { + byte[] bytes = new byte[256]; + new Random().nextBytes(bytes); + SecureKey secureKey = new SecureKey(bytes, System.currentTimeMillis()); + when(keyResolver.getSecureKey(any())).thenReturn(secureKey); + + builderFactory = new JwtAccessTokenBuilderFactory(new DefaultKeyGenerator(), keyResolver, Collections.emptySet()); + refresher = new JwtAccessTokenRefresher(builderFactory, refreshStrategy); + tokenBuilder = builderFactory.create(); + } + + @Test + public void shouldNotRefreshTokenWithDisabledRefresh() { + JwtAccessToken oldToken = tokenBuilder + .refreshableFor(0, TimeUnit.MINUTES) + .build(); + + Optional refreshedToken = refresher.refresh(oldToken); + + Assertions.assertThat(refreshedToken).isEmpty(); + } + + @Test + public void shouldNotRefreshTokenWhenStrategyDoesNotSaySo() { + JwtAccessToken oldToken = tokenBuilder + .refreshableFor(10, TimeUnit.MINUTES) + .build(); + when(refreshStrategy.shouldBeRefreshed(oldToken)).thenReturn(false); + + Optional refreshedToken = refresher.refresh(oldToken); + + Assertions.assertThat(refreshedToken).isEmpty(); + } + + @Test + public void shouldRefreshTokenWithEnabledRefresh() { + JwtAccessToken oldToken = tokenBuilder + .refreshableFor(1, TimeUnit.MINUTES) + .build(); + when(refreshStrategy.shouldBeRefreshed(oldToken)).thenReturn(true); + + Optional refreshedToken = refresher.refresh(oldToken); + + Assertions.assertThat(refreshedToken).isNotEmpty(); + } +} diff --git a/scm-webapp/src/test/resources/sonia/scm/repository/shiro.ini b/scm-webapp/src/test/resources/sonia/scm/repository/shiro.ini index 9a39a2d46c..500325faf3 100644 --- a/scm-webapp/src/test/resources/sonia/scm/repository/shiro.ini +++ b/scm-webapp/src/test/resources/sonia/scm/repository/shiro.ini @@ -4,6 +4,7 @@ dent = secret, creator, heartOfGold, puzzle42 unpriv = secret crato = secret, creator community = secret, oss +user = secret, user [roles] admin = * @@ -11,3 +12,4 @@ creator = repository:create heartOfGold = "repository:read,modify,delete:hof" puzzle42 = "repository:read,write:p42" oss = "repository:pull" +user = * From 0b1edaab08081972a9e56779fe1cc53847bbe0c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Thu, 29 Nov 2018 17:04:38 +0100 Subject: [PATCH 03/20] Fix time computations --- .../scm/security/JwtAccessTokenBuilder.java | 2 +- .../scm/security/JwtAccessTokenRefresher.java | 20 +++++-- .../security/JwtAccessTokenRefresherTest.java | 58 +++++++++++++++---- 3 files changed, 62 insertions(+), 18 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 1207b252e4..8e5de9c283 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java @@ -162,7 +162,7 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { if (refreshableFor > 0) { long refreshExpiration = refreshableForUnit.toMillis(refreshableFor); - claims.put("scm-manager.refreshableUntil", new Date(now.getTime() + refreshExpiration).getTime() / 1000); + claims.put("scm-manager.refreshableUntil", new Date(now.getTime() + refreshExpiration).getTime()); } if ( issuer != null ) { diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java index cb26d1f010..16370209b6 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java @@ -1,6 +1,7 @@ package sonia.scm.security; -import java.time.Instant; +import javax.inject.Inject; +import java.time.Clock; import java.util.Date; import java.util.Map; import java.util.Optional; @@ -10,10 +11,17 @@ public class JwtAccessTokenRefresher { private final JwtAccessTokenBuilderFactory builderFactory; private final JwtAccessTokenRefreshStrategy refreshStrategy; + private final Clock clock; + @Inject public JwtAccessTokenRefresher(JwtAccessTokenBuilderFactory builderFactory, JwtAccessTokenRefreshStrategy refreshStrategy) { + this(builderFactory, refreshStrategy, Clock.systemDefaultZone()); + } + + JwtAccessTokenRefresher(JwtAccessTokenBuilderFactory builderFactory, JwtAccessTokenRefreshStrategy refreshStrategy, Clock clock) { this.builderFactory = builderFactory; this.refreshStrategy = refreshStrategy; + this.clock = clock; } public Optional refresh(JwtAccessToken oldToken) { @@ -31,7 +39,7 @@ public class JwtAccessTokenRefresher { } private boolean canBeRefreshed(JwtAccessToken oldToken) { - return tokenIsValid(oldToken) || tokenCanBeRefreshed(oldToken); + return tokenIsValid(oldToken) && tokenCanBeRefreshed(oldToken); } private boolean shouldBeRefreshed(JwtAccessToken oldToken) { @@ -40,14 +48,14 @@ public class JwtAccessTokenRefresher { private boolean tokenCanBeRefreshed(JwtAccessToken oldToken) { Date refreshExpiration = oldToken.getRefreshExpiration(); - return refreshExpiration != null && isBeforeNow(refreshExpiration); + return refreshExpiration != null && isAfterNow(refreshExpiration); } private boolean tokenIsValid(JwtAccessToken oldToken) { - return isBeforeNow(oldToken.getExpiration()); + return isAfterNow(oldToken.getExpiration()); } - private boolean isBeforeNow(Date expiration) { - return expiration.toInstant().isBefore(Instant.now()); + private boolean isAfterNow(Date expiration) { + return expiration.toInstant().isAfter(clock.instant()); } } diff --git a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java index 1464bfeade..7c7a8c68cf 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java @@ -2,7 +2,6 @@ package sonia.scm.security; import com.github.sdorra.shiro.ShiroRule; import com.github.sdorra.shiro.SubjectAware; -import org.assertj.core.api.Assertions; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -10,11 +9,15 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; +import java.time.Clock; +import java.time.Instant; import java.util.Collections; import java.util.Optional; import java.util.Random; -import java.util.concurrent.TimeUnit; +import static java.time.Duration.ofMinutes; +import static java.util.concurrent.TimeUnit.MINUTES; +import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.when; @@ -33,7 +36,9 @@ public class JwtAccessTokenRefresherTest { private SecureKeyResolver keyResolver; @Mock private JwtAccessTokenRefreshStrategy refreshStrategy; - private JwtAccessTokenBuilderFactory builderFactory; + @Mock + private Clock clock; + private JwtAccessTokenRefresher refresher; private JwtAccessTokenBuilder tokenBuilder; @@ -44,43 +49,74 @@ public class JwtAccessTokenRefresherTest { SecureKey secureKey = new SecureKey(bytes, System.currentTimeMillis()); when(keyResolver.getSecureKey(any())).thenReturn(secureKey); - builderFactory = new JwtAccessTokenBuilderFactory(new DefaultKeyGenerator(), keyResolver, Collections.emptySet()); - refresher = new JwtAccessTokenRefresher(builderFactory, refreshStrategy); + JwtAccessTokenBuilderFactory builderFactory = new JwtAccessTokenBuilderFactory(new DefaultKeyGenerator(), keyResolver, Collections.emptySet()); + refresher = new JwtAccessTokenRefresher(builderFactory, refreshStrategy, clock); tokenBuilder = builderFactory.create(); + when(clock.instant()).thenAnswer(invocationOnMock -> Instant.now()); + when(refreshStrategy.shouldBeRefreshed(any())).thenReturn(true); } @Test public void shouldNotRefreshTokenWithDisabledRefresh() { JwtAccessToken oldToken = tokenBuilder - .refreshableFor(0, TimeUnit.MINUTES) + .refreshableFor(0, MINUTES) .build(); Optional refreshedToken = refresher.refresh(oldToken); - Assertions.assertThat(refreshedToken).isEmpty(); + assertThat(refreshedToken).isEmpty(); + } + + @Test + public void shouldNotRefreshTokenWhenTokenExpired() { + Instant oneMinuteAgo = Instant.now().plus(ofMinutes(2)); + when(clock.instant()).thenReturn(oneMinuteAgo); + JwtAccessToken oldToken = tokenBuilder + .expiresIn(1, MINUTES) + .refreshableFor(5, MINUTES) + .build(); + + Optional refreshedToken = refresher.refresh(oldToken); + + assertThat(refreshedToken).isEmpty(); + } + + @Test + public void shouldNotRefreshTokenWhenRefreshExpired() { + Instant oneMinuteAgo = Instant.now().plus(ofMinutes(2)); + when(clock.instant()).thenReturn(oneMinuteAgo); + JwtAccessToken oldToken = tokenBuilder + .expiresIn(5, MINUTES) + .refreshableFor(1, MINUTES) + .build(); + + Optional refreshedToken = refresher.refresh(oldToken); + + assertThat(refreshedToken).isEmpty(); } @Test public void shouldNotRefreshTokenWhenStrategyDoesNotSaySo() { JwtAccessToken oldToken = tokenBuilder - .refreshableFor(10, TimeUnit.MINUTES) + .refreshableFor(10, MINUTES) .build(); when(refreshStrategy.shouldBeRefreshed(oldToken)).thenReturn(false); Optional refreshedToken = refresher.refresh(oldToken); - Assertions.assertThat(refreshedToken).isEmpty(); + assertThat(refreshedToken).isEmpty(); } @Test public void shouldRefreshTokenWithEnabledRefresh() { JwtAccessToken oldToken = tokenBuilder - .refreshableFor(1, TimeUnit.MINUTES) + .expiresIn(1, MINUTES) + .refreshableFor(1, MINUTES) .build(); when(refreshStrategy.shouldBeRefreshed(oldToken)).thenReturn(true); Optional refreshedToken = refresher.refresh(oldToken); - Assertions.assertThat(refreshedToken).isNotEmpty(); + assertThat(refreshedToken).isNotEmpty(); } } From 2adcbe5d990141d655bacd865351b8fd50741128 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 30 Nov 2018 09:22:02 +0100 Subject: [PATCH 04/20] Set parent token id --- .../java/sonia/scm/security/AccessToken.java | 2 +- .../sonia/scm/security/JwtAccessToken.java | 10 ++++-- .../scm/security/JwtAccessTokenBuilder.java | 13 ++++++- .../scm/security/JwtAccessTokenRefresher.java | 15 ++++++-- .../security/JwtAccessTokenRefresherTest.java | 34 +++++++++---------- 5 files changed, 49 insertions(+), 25 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/security/AccessToken.java b/scm-core/src/main/java/sonia/scm/security/AccessToken.java index ec448ad235..c2a5f4b747 100644 --- a/scm-core/src/main/java/sonia/scm/security/AccessToken.java +++ b/scm-core/src/main/java/sonia/scm/security/AccessToken.java @@ -80,7 +80,7 @@ public interface AccessToken { */ Date getExpiration(); - Date getRefreshExpiration(); + Optional getRefreshExpiration(); /** * Returns the scope of the token. The scope is able to reduce the permissions of the subject in the context of this diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java index 35013e4fec..e6cfb8c957 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java @@ -37,6 +37,8 @@ import java.util.Date; import java.util.Map; import java.util.Optional; +import static java.util.Optional.ofNullable; + /** * Jwt implementation of {@link AccessToken}. * @@ -44,7 +46,9 @@ import java.util.Optional; * @since 2.0.0 */ public final class JwtAccessToken implements AccessToken { - + + public static final String REFRESHABLE_UNTIL_CLAIM_KEY = "scm-manager.refreshableUntil"; + public static final String PARENT_TOKEN_ID_CLAIM_KEY = "scm-manager.parentTokenId"; private final Claims claims; private final String compact; @@ -79,8 +83,8 @@ public final class JwtAccessToken implements AccessToken { } @Override - public Date getRefreshExpiration() { - return claims.get("scm-manager.refreshableUntil", Date.class); + public Optional getRefreshExpiration() { + return ofNullable(claims.get(REFRESHABLE_UNTIL_CLAIM_KEY, Date.class)); } @Override 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 8e5de9c283..b6b293f525 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java @@ -67,6 +67,7 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { private TimeUnit expiresInUnit = TimeUnit.HOURS; private long refreshableFor = 12; private TimeUnit refreshableForUnit = TimeUnit.HOURS; + private String parentKeyId; private Scope scope = Scope.empty(); private final Map custom = Maps.newHashMap(); @@ -127,6 +128,11 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { return this; } + public JwtAccessTokenBuilder parentKey(String parentKeyId) { + this.parentKeyId = parentKeyId; + return this; + } + private String getSubject(){ if (subject == null) { Subject currentSubject = SecurityUtils.getSubject(); @@ -162,7 +168,12 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { if (refreshableFor > 0) { long refreshExpiration = refreshableForUnit.toMillis(refreshableFor); - claims.put("scm-manager.refreshableUntil", new Date(now.getTime() + refreshExpiration).getTime()); + claims.put(JwtAccessToken.REFRESHABLE_UNTIL_CLAIM_KEY, new Date(now.getTime() + refreshExpiration).getTime()); + } + if (parentKeyId == null) { + claims.put(JwtAccessToken.PARENT_TOKEN_ID_CLAIM_KEY, id); + } else { + claims.put(JwtAccessToken.PARENT_TOKEN_ID_CLAIM_KEY, parentKeyId); } if ( issuer != null ) { diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java index 16370209b6..f3f3d032fc 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java @@ -1,5 +1,8 @@ package sonia.scm.security; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import javax.inject.Inject; import java.time.Clock; import java.util.Date; @@ -9,6 +12,8 @@ import java.util.concurrent.TimeUnit; public class JwtAccessTokenRefresher { + private static final Logger log = LoggerFactory.getLogger(JwtAccessTokenRefresher.class); + private final JwtAccessTokenBuilderFactory builderFactory; private final JwtAccessTokenRefreshStrategy refreshStrategy; private final Clock clock; @@ -30,8 +35,13 @@ public class JwtAccessTokenRefresher { claims.forEach(builder::custom); if (canBeRefreshed(oldToken) && shouldBeRefreshed(oldToken)) { + Optional parentTokenId = oldToken.getCustom("scm-manager.parentTokenId"); + if (!parentTokenId.isPresent()) { + log.warn("no parent token id found in token; could not refresh"); + return Optional.empty(); + } builder.expiresIn(1, TimeUnit.HOURS); -// builder.custom("scm-manager.parentTokenId") + builder.parentKey(parentTokenId.get().toString()); return Optional.of(builder.build()); } else { return Optional.empty(); @@ -47,8 +57,7 @@ public class JwtAccessTokenRefresher { } private boolean tokenCanBeRefreshed(JwtAccessToken oldToken) { - Date refreshExpiration = oldToken.getRefreshExpiration(); - return refreshExpiration != null && isAfterNow(refreshExpiration); + return oldToken.getRefreshExpiration().map(this::isAfterNow).orElse(false); } private boolean tokenIsValid(JwtAccessToken oldToken) { diff --git a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java index 7c7a8c68cf..06daf87fcb 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java @@ -39,6 +39,8 @@ public class JwtAccessTokenRefresherTest { @Mock private Clock clock; + private KeyGenerator keyGenerator = () -> "key"; + private JwtAccessTokenRefresher refresher; private JwtAccessTokenBuilder tokenBuilder; @@ -49,11 +51,16 @@ public class JwtAccessTokenRefresherTest { SecureKey secureKey = new SecureKey(bytes, System.currentTimeMillis()); when(keyResolver.getSecureKey(any())).thenReturn(secureKey); - JwtAccessTokenBuilderFactory builderFactory = new JwtAccessTokenBuilderFactory(new DefaultKeyGenerator(), keyResolver, Collections.emptySet()); + JwtAccessTokenBuilderFactory builderFactory = new JwtAccessTokenBuilderFactory(keyGenerator, keyResolver, Collections.emptySet()); refresher = new JwtAccessTokenRefresher(builderFactory, refreshStrategy, clock); tokenBuilder = builderFactory.create(); when(clock.instant()).thenAnswer(invocationOnMock -> Instant.now()); when(refreshStrategy.shouldBeRefreshed(any())).thenReturn(true); + + // set default expiration values + tokenBuilder + .expiresIn(5, MINUTES) + .refreshableFor(10, MINUTES); } @Test @@ -69,12 +76,9 @@ public class JwtAccessTokenRefresherTest { @Test public void shouldNotRefreshTokenWhenTokenExpired() { - Instant oneMinuteAgo = Instant.now().plus(ofMinutes(2)); - when(clock.instant()).thenReturn(oneMinuteAgo); - JwtAccessToken oldToken = tokenBuilder - .expiresIn(1, MINUTES) - .refreshableFor(5, MINUTES) - .build(); + Instant afterNormalExpiration = Instant.now().plus(ofMinutes(6)); + when(clock.instant()).thenReturn(afterNormalExpiration); + JwtAccessToken oldToken = tokenBuilder.build(); Optional refreshedToken = refresher.refresh(oldToken); @@ -83,10 +87,9 @@ public class JwtAccessTokenRefresherTest { @Test public void shouldNotRefreshTokenWhenRefreshExpired() { - Instant oneMinuteAgo = Instant.now().plus(ofMinutes(2)); - when(clock.instant()).thenReturn(oneMinuteAgo); + Instant afterRefreshExpiration = Instant.now().plus(ofMinutes(2)); + when(clock.instant()).thenReturn(afterRefreshExpiration); JwtAccessToken oldToken = tokenBuilder - .expiresIn(5, MINUTES) .refreshableFor(1, MINUTES) .build(); @@ -97,9 +100,7 @@ public class JwtAccessTokenRefresherTest { @Test public void shouldNotRefreshTokenWhenStrategyDoesNotSaySo() { - JwtAccessToken oldToken = tokenBuilder - .refreshableFor(10, MINUTES) - .build(); + JwtAccessToken oldToken = tokenBuilder.build(); when(refreshStrategy.shouldBeRefreshed(oldToken)).thenReturn(false); Optional refreshedToken = refresher.refresh(oldToken); @@ -109,14 +110,13 @@ public class JwtAccessTokenRefresherTest { @Test public void shouldRefreshTokenWithEnabledRefresh() { - JwtAccessToken oldToken = tokenBuilder - .expiresIn(1, MINUTES) - .refreshableFor(1, MINUTES) - .build(); + JwtAccessToken oldToken = tokenBuilder.build(); when(refreshStrategy.shouldBeRefreshed(oldToken)).thenReturn(true); Optional refreshedToken = refresher.refresh(oldToken); assertThat(refreshedToken).isNotEmpty(); + assertThat(refreshedToken.get().getClaims()) + .containsEntry(JwtAccessToken.PARENT_TOKEN_ID_CLAIM_KEY, "key"); } } From 0f6b9ba891b4c3d0cf84c60ac318fb732fbe1532 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 30 Nov 2018 09:43:13 +0100 Subject: [PATCH 05/20] Inject clocks for tests --- .../sonia/scm/security/JwtAccessToken.java | 4 +++ .../scm/security/JwtAccessTokenBuilder.java | 16 ++++++---- .../JwtAccessTokenBuilderFactory.java | 13 ++++++-- .../security/JwtAccessTokenRefresherTest.java | 30 +++++++++++-------- 4 files changed, 43 insertions(+), 20 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java index e6cfb8c957..7832a463a3 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java @@ -87,6 +87,10 @@ public final class JwtAccessToken implements AccessToken { return ofNullable(claims.get(REFRESHABLE_UNTIL_CLAIM_KEY, Date.class)); } + public Optional getParentKey() { + return ofNullable(claims.get(PARENT_TOKEN_ID_CLAIM_KEY).toString()); + } + @Override public Scope getScope() { return Scopes.fromClaims(claims); 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 b6b293f525..a261c9a303 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java @@ -36,6 +36,9 @@ import com.google.common.collect.Maps; import io.jsonwebtoken.Claims; import io.jsonwebtoken.Jwts; import io.jsonwebtoken.SignatureAlgorithm; + +import java.time.Clock; +import java.time.Instant; import java.util.Date; import java.util.HashMap; import java.util.Map; @@ -60,6 +63,7 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { private final KeyGenerator keyGenerator; private final SecureKeyResolver keyResolver; + private final Clock clock; private String subject; private String issuer; @@ -72,9 +76,10 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { private final Map custom = Maps.newHashMap(); - JwtAccessTokenBuilder(KeyGenerator keyGenerator, SecureKeyResolver keyResolver) { + JwtAccessTokenBuilder(KeyGenerator keyGenerator, SecureKeyResolver keyResolver, Clock clock) { this.keyGenerator = keyGenerator; this.keyResolver = keyResolver; + this.clock = clock; } @Override @@ -157,18 +162,19 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { // add scope to custom claims Scopes.toClaims(customClaims, scope); - Date now = new Date(); + Instant now = clock.instant(); long expiration = expiresInUnit.toMillis(expiresIn); Claims claims = Jwts.claims(customClaims) .setSubject(sub) .setId(id) - .setIssuedAt(now) - .setExpiration(new Date(now.getTime() + expiration)); + .setIssuedAt(Date.from(now)) + .setExpiration(new Date(now.toEpochMilli() + expiration)); + if (refreshableFor > 0) { long refreshExpiration = refreshableForUnit.toMillis(refreshableFor); - claims.put(JwtAccessToken.REFRESHABLE_UNTIL_CLAIM_KEY, new Date(now.getTime() + refreshExpiration).getTime()); + claims.put(JwtAccessToken.REFRESHABLE_UNTIL_CLAIM_KEY, new Date(now.toEpochMilli() + refreshExpiration).getTime()); } if (parentKeyId == null) { claims.put(JwtAccessToken.PARENT_TOKEN_ID_CLAIM_KEY, id); diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilderFactory.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilderFactory.java index 63c4ea981c..a5704b0e82 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilderFactory.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilderFactory.java @@ -30,6 +30,7 @@ */ package sonia.scm.security; +import java.time.Clock; import java.util.Set; import javax.inject.Inject; import sonia.scm.plugin.Extension; @@ -46,19 +47,25 @@ public final class JwtAccessTokenBuilderFactory implements AccessTokenBuilderFac private final KeyGenerator keyGenerator; private final SecureKeyResolver keyResolver; private final Set enrichers; + private final Clock clock; @Inject public JwtAccessTokenBuilderFactory( - KeyGenerator keyGenerator, SecureKeyResolver keyResolver, Set enrichers - ) { + KeyGenerator keyGenerator, SecureKeyResolver keyResolver, Set enrichers) { + this(keyGenerator, keyResolver, enrichers, Clock.systemDefaultZone()); + } + + JwtAccessTokenBuilderFactory( + KeyGenerator keyGenerator, SecureKeyResolver keyResolver, Set enrichers, Clock clock) { this.keyGenerator = keyGenerator; this.keyResolver = keyResolver; this.enrichers = enrichers; + this.clock = clock; } @Override public JwtAccessTokenBuilder create() { - JwtAccessTokenBuilder builder = new JwtAccessTokenBuilder(keyGenerator, keyResolver); + JwtAccessTokenBuilder builder = new JwtAccessTokenBuilder(keyGenerator, keyResolver, clock); // enrich access token builder enrichers.forEach((enricher) -> { diff --git a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java index 06daf87fcb..88a402841d 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java @@ -29,6 +29,9 @@ import static org.mockito.Mockito.when; @RunWith(MockitoJUnitRunner.class) public class JwtAccessTokenRefresherTest { + private static final Instant NOW = Instant.now(); + private static final Instant TOKEN_CREATION = NOW.minus(ofMinutes(1)); + @Rule public ShiroRule shiro = new ShiroRule(); @@ -37,7 +40,9 @@ public class JwtAccessTokenRefresherTest { @Mock private JwtAccessTokenRefreshStrategy refreshStrategy; @Mock - private Clock clock; + private Clock refreshClock; + @Mock + private Clock creationClock; private KeyGenerator keyGenerator = () -> "key"; @@ -51,10 +56,11 @@ public class JwtAccessTokenRefresherTest { SecureKey secureKey = new SecureKey(bytes, System.currentTimeMillis()); when(keyResolver.getSecureKey(any())).thenReturn(secureKey); - JwtAccessTokenBuilderFactory builderFactory = new JwtAccessTokenBuilderFactory(keyGenerator, keyResolver, Collections.emptySet()); - refresher = new JwtAccessTokenRefresher(builderFactory, refreshStrategy, clock); + JwtAccessTokenBuilderFactory builderFactory = new JwtAccessTokenBuilderFactory(keyGenerator, keyResolver, Collections.emptySet(), creationClock); + refresher = new JwtAccessTokenRefresher(builderFactory, refreshStrategy, refreshClock); tokenBuilder = builderFactory.create(); - when(clock.instant()).thenAnswer(invocationOnMock -> Instant.now()); + when(creationClock.instant()).thenReturn(TOKEN_CREATION); + when(refreshClock.instant()).thenReturn(NOW); when(refreshStrategy.shouldBeRefreshed(any())).thenReturn(true); // set default expiration values @@ -76,8 +82,8 @@ public class JwtAccessTokenRefresherTest { @Test public void shouldNotRefreshTokenWhenTokenExpired() { - Instant afterNormalExpiration = Instant.now().plus(ofMinutes(6)); - when(clock.instant()).thenReturn(afterNormalExpiration); + Instant afterNormalExpiration = NOW.plus(ofMinutes(6)); + when(refreshClock.instant()).thenReturn(afterNormalExpiration); JwtAccessToken oldToken = tokenBuilder.build(); Optional refreshedToken = refresher.refresh(oldToken); @@ -88,7 +94,7 @@ public class JwtAccessTokenRefresherTest { @Test public void shouldNotRefreshTokenWhenRefreshExpired() { Instant afterRefreshExpiration = Instant.now().plus(ofMinutes(2)); - when(clock.instant()).thenReturn(afterRefreshExpiration); + when(refreshClock.instant()).thenReturn(afterRefreshExpiration); JwtAccessToken oldToken = tokenBuilder .refreshableFor(1, MINUTES) .build(); @@ -109,14 +115,14 @@ public class JwtAccessTokenRefresherTest { } @Test - public void shouldRefreshTokenWithEnabledRefresh() { + public void shouldRefreshTokenWithCorrectClaims() { JwtAccessToken oldToken = tokenBuilder.build(); when(refreshStrategy.shouldBeRefreshed(oldToken)).thenReturn(true); - Optional refreshedToken = refresher.refresh(oldToken); + Optional refreshedTokenResult = refresher.refresh(oldToken); - assertThat(refreshedToken).isNotEmpty(); - assertThat(refreshedToken.get().getClaims()) - .containsEntry(JwtAccessToken.PARENT_TOKEN_ID_CLAIM_KEY, "key"); + assertThat(refreshedTokenResult).isNotEmpty(); + JwtAccessToken refreshedToken = refreshedTokenResult.get(); + assertThat(refreshedToken.getParentKey()).get().isEqualTo("key"); } } From 46f94730838997814988d627b4de39dba3bc0c02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 30 Nov 2018 10:05:43 +0100 Subject: [PATCH 06/20] Compute new expiration from old expiration --- .../scm/security/JwtAccessTokenRefresher.java | 6 +++- .../security/JwtAccessTokenRefresherTest.java | 31 ++++++++++++++----- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java index f3f3d032fc..f219262626 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java @@ -40,7 +40,7 @@ public class JwtAccessTokenRefresher { log.warn("no parent token id found in token; could not refresh"); return Optional.empty(); } - builder.expiresIn(1, TimeUnit.HOURS); + builder.expiresIn(computeOldExpirationInMillis(oldToken), TimeUnit.MILLISECONDS); builder.parentKey(parentTokenId.get().toString()); return Optional.of(builder.build()); } else { @@ -48,6 +48,10 @@ public class JwtAccessTokenRefresher { } } + private long computeOldExpirationInMillis(JwtAccessToken oldToken) { + return oldToken.getExpiration().getTime() - oldToken.getIssuedAt().getTime(); + } + private boolean canBeRefreshed(JwtAccessToken oldToken) { return tokenIsValid(oldToken) && tokenCanBeRefreshed(oldToken); } diff --git a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java index 88a402841d..92c6bb98e9 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java @@ -9,16 +9,21 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; +import java.sql.Date; import java.time.Clock; import java.time.Instant; +import java.time.temporal.ChronoUnit; import java.util.Collections; import java.util.Optional; import java.util.Random; +import static java.time.Duration.ofHours; import static java.time.Duration.ofMinutes; +import static java.time.temporal.ChronoUnit.SECONDS; import static java.util.concurrent.TimeUnit.MINUTES; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @SubjectAware( @@ -29,7 +34,7 @@ import static org.mockito.Mockito.when; @RunWith(MockitoJUnitRunner.class) public class JwtAccessTokenRefresherTest { - private static final Instant NOW = Instant.now(); + private static final Instant NOW = Instant.now().truncatedTo(SECONDS); private static final Instant TOKEN_CREATION = NOW.minus(ofMinutes(1)); @Rule @@ -41,8 +46,6 @@ public class JwtAccessTokenRefresherTest { private JwtAccessTokenRefreshStrategy refreshStrategy; @Mock private Clock refreshClock; - @Mock - private Clock creationClock; private KeyGenerator keyGenerator = () -> "key"; @@ -56,10 +59,12 @@ public class JwtAccessTokenRefresherTest { SecureKey secureKey = new SecureKey(bytes, System.currentTimeMillis()); when(keyResolver.getSecureKey(any())).thenReturn(secureKey); - JwtAccessTokenBuilderFactory builderFactory = new JwtAccessTokenBuilderFactory(keyGenerator, keyResolver, Collections.emptySet(), creationClock); - refresher = new JwtAccessTokenRefresher(builderFactory, refreshStrategy, refreshClock); - tokenBuilder = builderFactory.create(); + Clock creationClock = mock(Clock.class); when(creationClock.instant()).thenReturn(TOKEN_CREATION); + tokenBuilder = new JwtAccessTokenBuilderFactory(keyGenerator, keyResolver, Collections.emptySet(), creationClock).create(); + + JwtAccessTokenBuilderFactory refreshBuilderFactory = new JwtAccessTokenBuilderFactory(keyGenerator, keyResolver, Collections.emptySet(), refreshClock); + refresher = new JwtAccessTokenRefresher(refreshBuilderFactory, refreshStrategy, refreshClock); when(refreshClock.instant()).thenReturn(NOW); when(refreshStrategy.shouldBeRefreshed(any())).thenReturn(true); @@ -115,7 +120,7 @@ public class JwtAccessTokenRefresherTest { } @Test - public void shouldRefreshTokenWithCorrectClaims() { + public void shouldRefreshTokenWithParentId() { JwtAccessToken oldToken = tokenBuilder.build(); when(refreshStrategy.shouldBeRefreshed(oldToken)).thenReturn(true); @@ -125,4 +130,16 @@ public class JwtAccessTokenRefresherTest { JwtAccessToken refreshedToken = refreshedTokenResult.get(); assertThat(refreshedToken.getParentKey()).get().isEqualTo("key"); } + + @Test + public void shouldRefreshTokenWithSameExpiration() { + JwtAccessToken oldToken = tokenBuilder.build(); + when(refreshStrategy.shouldBeRefreshed(oldToken)).thenReturn(true); + + Optional refreshedTokenResult = refresher.refresh(oldToken); + + assertThat(refreshedTokenResult).isNotEmpty(); + JwtAccessToken refreshedToken = refreshedTokenResult.get(); + assertThat(refreshedToken.getExpiration()).isEqualTo(Date.from(NOW.plus(ofMinutes(5)))); + } } From e8672bbeff45f7054defbfc4b1c494714e01f311 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 30 Nov 2018 10:15:12 +0100 Subject: [PATCH 07/20] Keep refresh expiration --- .../main/java/sonia/scm/security/JwtAccessToken.java | 2 +- .../sonia/scm/security/JwtAccessTokenBuilder.java | 9 +++++++++ .../sonia/scm/security/JwtAccessTokenRefresher.java | 3 ++- .../scm/security/JwtAccessTokenRefresherTest.java | 12 ++++++++++++ 4 files changed, 24 insertions(+), 2 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java index 7832a463a3..8fb5929188 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java @@ -47,7 +47,7 @@ import static java.util.Optional.ofNullable; */ public final class JwtAccessToken implements AccessToken { - public static final String REFRESHABLE_UNTIL_CLAIM_KEY = "scm-manager.refreshableUntil"; + public static final String REFRESHABLE_UNTIL_CLAIM_KEY = "scm-manager.refreshExpiration"; public static final String PARENT_TOKEN_ID_CLAIM_KEY = "scm-manager.parentTokenId"; private final Claims claims; private final String compact; 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 a261c9a303..66db720125 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java @@ -71,6 +71,7 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { private TimeUnit expiresInUnit = TimeUnit.HOURS; private long refreshableFor = 12; private TimeUnit refreshableForUnit = TimeUnit.HOURS; + private Instant refreshExpiration; private String parentKeyId; private Scope scope = Scope.empty(); @@ -133,6 +134,12 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { return this; } + JwtAccessTokenBuilder refreshExpiration(Instant refreshExpiration) { + this.refreshExpiration = refreshExpiration; + this.refreshableFor = 0; + return this; + } + public JwtAccessTokenBuilder parentKey(String parentKeyId) { this.parentKeyId = parentKeyId; return this; @@ -175,6 +182,8 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { if (refreshableFor > 0) { long refreshExpiration = refreshableForUnit.toMillis(refreshableFor); claims.put(JwtAccessToken.REFRESHABLE_UNTIL_CLAIM_KEY, new Date(now.toEpochMilli() + refreshExpiration).getTime()); + } else if (refreshExpiration != null) { + claims.put(JwtAccessToken.REFRESHABLE_UNTIL_CLAIM_KEY, Date.from(refreshExpiration)); } if (parentKeyId == null) { claims.put(JwtAccessToken.PARENT_TOKEN_ID_CLAIM_KEY, id); diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java index f219262626..5efc01a096 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java @@ -29,7 +29,7 @@ public class JwtAccessTokenRefresher { this.clock = clock; } - public Optional refresh(JwtAccessToken oldToken) { + Optional refresh(JwtAccessToken oldToken) { JwtAccessTokenBuilder builder = builderFactory.create(); Map claims = oldToken.getClaims(); claims.forEach(builder::custom); @@ -42,6 +42,7 @@ public class JwtAccessTokenRefresher { } builder.expiresIn(computeOldExpirationInMillis(oldToken), TimeUnit.MILLISECONDS); builder.parentKey(parentTokenId.get().toString()); + builder.refreshExpiration(oldToken.getRefreshExpiration().get().toInstant()); return Optional.of(builder.build()); } else { return Optional.empty(); diff --git a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java index 92c6bb98e9..83f528092c 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java @@ -142,4 +142,16 @@ public class JwtAccessTokenRefresherTest { JwtAccessToken refreshedToken = refreshedTokenResult.get(); assertThat(refreshedToken.getExpiration()).isEqualTo(Date.from(NOW.plus(ofMinutes(5)))); } + + @Test + public void shouldRefreshTokenWithSameRefreshExpiration() { + JwtAccessToken oldToken = tokenBuilder.build(); + when(refreshStrategy.shouldBeRefreshed(oldToken)).thenReturn(true); + + Optional refreshedTokenResult = refresher.refresh(oldToken); + + assertThat(refreshedTokenResult).isNotEmpty(); + JwtAccessToken refreshedToken = refreshedTokenResult.get(); + assertThat(refreshedToken.getRefreshExpiration()).get().isEqualTo(Date.from(TOKEN_CREATION.plus(ofMinutes(10)))); + } } From 2e092b36cf38ce99dd543da5b969e2b2379ae00d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 30 Nov 2018 10:20:12 +0100 Subject: [PATCH 08/20] Suppress warning --- .../main/java/sonia/scm/security/JwtAccessTokenRefresher.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java index 5efc01a096..ebcf88b346 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java @@ -29,6 +29,8 @@ public class JwtAccessTokenRefresher { this.clock = clock; } + @SuppressWarnings("squid:S3655") // the refresh expiration cannot be null at the time building the new token, because + // we checked this before in tokenCanBeRefreshed Optional refresh(JwtAccessToken oldToken) { JwtAccessTokenBuilder builder = builderFactory.create(); Map claims = oldToken.getClaims(); From 176d121aa0f169d8cd92a33ebdd92a6452b7f06e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 30 Nov 2018 10:29:08 +0100 Subject: [PATCH 09/20] Adapt tests to new version of jjwt --- .../src/test/java/sonia/scm/security/BearerRealmTest.java | 2 +- .../java/sonia/scm/security/JwtAccessTokenResolverTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 e6061e61a1..d223e271a6 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/BearerRealmTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/BearerRealmTest.java @@ -272,7 +272,7 @@ private String createCompactToken(String subject, SecureKey key) { .thenReturn( new SecretKeySpec( key.getBytes(), - SignatureAlgorithm.HS256.getValue() + SignatureAlgorithm.HS256.getJcaName() ) ); } diff --git a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenResolverTest.java b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenResolverTest.java index 689fc4bb35..cd7e6475aa 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenResolverTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenResolverTest.java @@ -230,7 +230,7 @@ public class JwtAccessTokenResolverTest { .thenReturn( new SecretKeySpec( key.getBytes(), - SignatureAlgorithm.HS256.getValue() + SignatureAlgorithm.HS256.getJcaName() ) ); } From 205ca42e09a7bcb49d8699e5a086f0430822899b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 30 Nov 2018 11:18:37 +0100 Subject: [PATCH 10/20] Introduce simple refresh strategy --- ...rcentageJwtAccessTokenRefreshStrategy.java | 25 +++++++ .../security/JwtAccessTokenRefresherTest.java | 2 - ...tageJwtAccessTokenRefreshStrategyTest.java | 70 +++++++++++++++++++ 3 files changed, 95 insertions(+), 2 deletions(-) create mode 100644 scm-webapp/src/main/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategy.java create mode 100644 scm-webapp/src/test/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategyTest.java diff --git a/scm-webapp/src/main/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategy.java b/scm-webapp/src/main/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategy.java new file mode 100644 index 0000000000..2d5c67c7b6 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategy.java @@ -0,0 +1,25 @@ +package sonia.scm.security; + +import java.time.Clock; + +public class PercentageJwtAccessTokenRefreshStrategy implements JwtAccessTokenRefreshStrategy { + + private final Clock clock; + private final float refreshPercentage; + + public PercentageJwtAccessTokenRefreshStrategy(float refreshPercentage) { + this(Clock.systemDefaultZone(), refreshPercentage); + } + + PercentageJwtAccessTokenRefreshStrategy(Clock clock, float refreshPercentage) { + this.clock = clock; + this.refreshPercentage = refreshPercentage; + } + + @Override + public boolean shouldBeRefreshed(JwtAccessToken oldToken) { + long liveSpan = oldToken.getExpiration().getTime() - oldToken.getIssuedAt().getTime(); + long age = clock.instant().toEpochMilli() - oldToken.getIssuedAt().getTime(); + return age/liveSpan > refreshPercentage; + } +} diff --git a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java index 83f528092c..cd902fb0a8 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java @@ -12,12 +12,10 @@ import org.mockito.junit.MockitoJUnitRunner; import java.sql.Date; import java.time.Clock; import java.time.Instant; -import java.time.temporal.ChronoUnit; import java.util.Collections; import java.util.Optional; import java.util.Random; -import static java.time.Duration.ofHours; import static java.time.Duration.ofMinutes; import static java.time.temporal.ChronoUnit.SECONDS; import static java.util.concurrent.TimeUnit.MINUTES; diff --git a/scm-webapp/src/test/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategyTest.java b/scm-webapp/src/test/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategyTest.java new file mode 100644 index 0000000000..d2c684d4a0 --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategyTest.java @@ -0,0 +1,70 @@ +package sonia.scm.security; + +import com.github.sdorra.shiro.ShiroRule; +import com.github.sdorra.shiro.SubjectAware; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; + +import java.time.Clock; +import java.time.Instant; +import java.util.Collections; +import java.util.Random; + +import static java.time.temporal.ChronoUnit.MINUTES; +import static java.time.temporal.ChronoUnit.SECONDS; +import static java.util.concurrent.TimeUnit.HOURS; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +@SubjectAware( + username = "user", + password = "secret", + configuration = "classpath:sonia/scm/repository/shiro.ini" +) +public class PercentageJwtAccessTokenRefreshStrategyTest { + + private static final Instant TOKEN_CREATION = Instant.now().truncatedTo(SECONDS); + + @Rule + public ShiroRule shiro = new ShiroRule(); + + private KeyGenerator keyGenerator = () -> "key"; + + private Clock refreshClock = mock(Clock.class); + + private JwtAccessTokenBuilder tokenBuilder; + private PercentageJwtAccessTokenRefreshStrategy refreshStrategy; + + @Before + public void initToken() { + SecureKeyResolver keyResolver = mock(SecureKeyResolver.class); + byte[] bytes = new byte[256]; + new Random().nextBytes(bytes); + SecureKey secureKey = new SecureKey(bytes, System.currentTimeMillis()); + when(keyResolver.getSecureKey(any())).thenReturn(secureKey); + + Clock creationClock = mock(Clock.class); + when(creationClock.instant()).thenReturn(TOKEN_CREATION); + + tokenBuilder = new JwtAccessTokenBuilderFactory(keyGenerator, keyResolver, Collections.emptySet(), creationClock).create(); + tokenBuilder + .refreshableFor(1, HOURS); + + refreshStrategy = new PercentageJwtAccessTokenRefreshStrategy(refreshClock, 0.5F); + } + + @Test + public void shouldNotRefreshWhenTokenIsYoung() { + when(refreshClock.instant()).thenReturn(TOKEN_CREATION.plus(1, MINUTES)); + assertThat(refreshStrategy.shouldBeRefreshed(tokenBuilder.build())).isFalse(); + } + + @Test + public void shouldRefreshWhenTokenIsOld() { + when(refreshClock.instant()).thenReturn(TOKEN_CREATION.plus(31, MINUTES)); + assertThat(refreshStrategy.shouldBeRefreshed(tokenBuilder.build())).isFalse(); + } +} From f7fc81b62660982be91242e55a69a760248af90e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 30 Nov 2018 11:26:23 +0100 Subject: [PATCH 11/20] Remove redundant key generation in tests --- .../test/java/sonia/scm/security/BearerRealmTest.java | 11 +---------- .../sonia/scm/security/JwtAccessTokenBuilderTest.java | 9 +-------- .../scm/security/JwtAccessTokenRefresherTest.java | 7 ++----- .../scm/security/JwtAccessTokenResolverTest.java | 8 ++------ .../PercentageJwtAccessTokenRefreshStrategyTest.java | 7 ++----- .../java/sonia/scm/security/SecureKeyTestUtil.java | 11 +++++++++++ 6 files changed, 19 insertions(+), 34 deletions(-) create mode 100644 scm-webapp/src/test/java/sonia/scm/security/SecureKeyTestUtil.java 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 d223e271a6..26dfcb2099 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/BearerRealmTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/BearerRealmTest.java @@ -61,7 +61,6 @@ import sonia.scm.user.UserDAO; import sonia.scm.user.UserTestData; import javax.crypto.spec.SecretKeySpec; -import java.security.SecureRandom; import java.util.Date; import java.util.Set; @@ -71,6 +70,7 @@ import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.any; import static org.mockito.Mockito.when; +import static sonia.scm.security.SecureKeyTestUtil.createSecureKey; /** * Unit tests for {@link BearerRealm}. @@ -256,12 +256,6 @@ private String createCompactToken(String subject, SecureKey key) { .compact(); } - private SecureKey createSecureKey() { - byte[] bytes = new byte[32]; - random.nextBytes(bytes); - return new SecureKey(bytes, System.currentTimeMillis()); - } - private void resolveKey(SecureKey key) { when( keyResolver.resolveSigningKey( @@ -279,9 +273,6 @@ private String createCompactToken(String subject, SecureKey key) { //~--- fields --------------------------------------------------------------- - /** Field description */ - private final SecureRandom random = new SecureRandom(); - @InjectMocks private DAORealmHelperFactory helperFactory; 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 6dda005019..c005e7d381 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenBuilderTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenBuilderTest.java @@ -44,7 +44,6 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; -import java.util.Random; import java.util.Set; import java.util.concurrent.TimeUnit; @@ -56,6 +55,7 @@ import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.anyString; import static org.mockito.Mockito.when; +import static sonia.scm.security.SecureKeyTestUtil.createSecureKey; /** * Unit test for {@link JwtAccessTokenBuilder}. @@ -162,11 +162,4 @@ public class JwtAccessTokenBuilderTest { assertEquals("b", token.getCustom("a").get()); assertEquals("[\"repo:*\"]", token.getScope().toString()); } - - private SecureKey createSecureKey() { - byte[] bytes = new byte[32]; - new Random().nextBytes(bytes); - return new SecureKey(bytes, System.currentTimeMillis()); - } - } diff --git a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java index cd902fb0a8..774677cde3 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenRefresherTest.java @@ -14,7 +14,6 @@ import java.time.Clock; import java.time.Instant; import java.util.Collections; import java.util.Optional; -import java.util.Random; import static java.time.Duration.ofMinutes; import static java.time.temporal.ChronoUnit.SECONDS; @@ -23,6 +22,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import static sonia.scm.security.SecureKeyTestUtil.createSecureKey; @SubjectAware( username = "user", @@ -52,10 +52,7 @@ public class JwtAccessTokenRefresherTest { @Before public void initKeyResolver() { - byte[] bytes = new byte[256]; - new Random().nextBytes(bytes); - SecureKey secureKey = new SecureKey(bytes, System.currentTimeMillis()); - when(keyResolver.getSecureKey(any())).thenReturn(secureKey); + when(keyResolver.getSecureKey(any())).thenReturn(createSecureKey()); Clock creationClock = mock(Clock.class); when(creationClock.instant()).thenReturn(TOKEN_CREATION); diff --git a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenResolverTest.java b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenResolverTest.java index cd7e6475aa..d4341f104e 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenResolverTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenResolverTest.java @@ -56,6 +56,8 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.Mockito; import static org.mockito.Mockito.*; +import static sonia.scm.security.SecureKeyTestUtil.createSecureKey; + import org.mockito.junit.MockitoJUnitRunner; /** @@ -214,12 +216,6 @@ public class JwtAccessTokenResolverTest { .compact(); } - private SecureKey createSecureKey() { - byte[] bytes = new byte[32]; - random.nextBytes(bytes); - return new SecureKey(bytes, System.currentTimeMillis()); - } - private void resolveKey(SecureKey key) { when( keyResolver.resolveSigningKey( diff --git a/scm-webapp/src/test/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategyTest.java b/scm-webapp/src/test/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategyTest.java index d2c684d4a0..e35823e445 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategyTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategyTest.java @@ -9,7 +9,6 @@ import org.junit.Test; import java.time.Clock; import java.time.Instant; import java.util.Collections; -import java.util.Random; import static java.time.temporal.ChronoUnit.MINUTES; import static java.time.temporal.ChronoUnit.SECONDS; @@ -18,6 +17,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import static sonia.scm.security.SecureKeyTestUtil.createSecureKey; @SubjectAware( username = "user", @@ -41,10 +41,7 @@ public class PercentageJwtAccessTokenRefreshStrategyTest { @Before public void initToken() { SecureKeyResolver keyResolver = mock(SecureKeyResolver.class); - byte[] bytes = new byte[256]; - new Random().nextBytes(bytes); - SecureKey secureKey = new SecureKey(bytes, System.currentTimeMillis()); - when(keyResolver.getSecureKey(any())).thenReturn(secureKey); + when(keyResolver.getSecureKey(any())).thenReturn(createSecureKey()); Clock creationClock = mock(Clock.class); when(creationClock.instant()).thenReturn(TOKEN_CREATION); diff --git a/scm-webapp/src/test/java/sonia/scm/security/SecureKeyTestUtil.java b/scm-webapp/src/test/java/sonia/scm/security/SecureKeyTestUtil.java new file mode 100644 index 0000000000..3b9c95fd17 --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/security/SecureKeyTestUtil.java @@ -0,0 +1,11 @@ +package sonia.scm.security; + +import java.security.SecureRandom; + +public class SecureKeyTestUtil { + public static SecureKey createSecureKey() { + byte[] bytes = new byte[32]; + new SecureRandom().nextBytes(bytes); + return new SecureKey(bytes, System.currentTimeMillis()); + } +} From 57753e4de05b027417df5e5adb72842a4870f984 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 30 Nov 2018 11:35:20 +0100 Subject: [PATCH 12/20] Add default refresh strategy --- .../security/DefaultJwtAccessTokenRefreshStrategy.java | 10 ++++++++++ .../scm/security/JwtAccessTokenRefreshStrategy.java | 3 +++ .../PercentageJwtAccessTokenRefreshStrategyTest.java | 5 ++--- 3 files changed, 15 insertions(+), 3 deletions(-) create mode 100644 scm-webapp/src/main/java/sonia/scm/security/DefaultJwtAccessTokenRefreshStrategy.java diff --git a/scm-webapp/src/main/java/sonia/scm/security/DefaultJwtAccessTokenRefreshStrategy.java b/scm-webapp/src/main/java/sonia/scm/security/DefaultJwtAccessTokenRefreshStrategy.java new file mode 100644 index 0000000000..266a327d44 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/security/DefaultJwtAccessTokenRefreshStrategy.java @@ -0,0 +1,10 @@ +package sonia.scm.security; + +import sonia.scm.plugin.Extension; + +@Extension +public class DefaultJwtAccessTokenRefreshStrategy extends PercentageJwtAccessTokenRefreshStrategy { + public DefaultJwtAccessTokenRefreshStrategy() { + super(0.5F); + } +} diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefreshStrategy.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefreshStrategy.java index 47d6a09285..f7f030d1f6 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefreshStrategy.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefreshStrategy.java @@ -1,5 +1,8 @@ package sonia.scm.security; +import sonia.scm.plugin.ExtensionPoint; + +@ExtensionPoint public interface JwtAccessTokenRefreshStrategy { boolean shouldBeRefreshed(JwtAccessToken oldToken); } diff --git a/scm-webapp/src/test/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategyTest.java b/scm-webapp/src/test/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategyTest.java index e35823e445..2e1fa44a76 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategyTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategyTest.java @@ -47,15 +47,14 @@ public class PercentageJwtAccessTokenRefreshStrategyTest { when(creationClock.instant()).thenReturn(TOKEN_CREATION); tokenBuilder = new JwtAccessTokenBuilderFactory(keyGenerator, keyResolver, Collections.emptySet(), creationClock).create(); - tokenBuilder - .refreshableFor(1, HOURS); + tokenBuilder.refreshableFor(1, HOURS); refreshStrategy = new PercentageJwtAccessTokenRefreshStrategy(refreshClock, 0.5F); } @Test public void shouldNotRefreshWhenTokenIsYoung() { - when(refreshClock.instant()).thenReturn(TOKEN_CREATION.plus(1, MINUTES)); + when(refreshClock.instant()).thenReturn(TOKEN_CREATION.plus(29, MINUTES)); assertThat(refreshStrategy.shouldBeRefreshed(tokenBuilder.build())).isFalse(); } From 043be9f47b672d790ce2564c92aaacd0410ff0a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 30 Nov 2018 12:55:48 +0100 Subject: [PATCH 13/20] Add filter for token refresh --- .../sonia/scm/web/security/TokenRefreshFilter.java | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 scm-webapp/src/main/java/sonia/scm/web/security/TokenRefreshFilter.java 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 new file mode 100644 index 0000000000..18980311e6 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/web/security/TokenRefreshFilter.java @@ -0,0 +1,11 @@ +package sonia.scm.web.security; + +import sonia.scm.Priority; +import sonia.scm.filter.Filters; +import sonia.scm.filter.WebElement; + +@Priority(Filters.PRIORITY_POST_AUTHENTICATION) +@WebElement(value = Filters.PATTERN_RESTAPI, + morePatterns = { Filters.PATTERN_DEBUG }) +public class TokenRefreshFilter { +} From aec5520e57404697625c7e1a49ecca3f4fd9bdb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 30 Nov 2018 16:57:04 +0100 Subject: [PATCH 14/20] Implement simple JWT refresh filter --- .../main/java/sonia/scm/ScmServletModule.java | 4 ++ .../scm/security/JwtAccessTokenRefresher.java | 2 +- .../scm/web/security/TokenRefreshFilter.java | 57 ++++++++++++++++++- 3 files changed, 61 insertions(+), 2 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/ScmServletModule.java b/scm-webapp/src/main/java/sonia/scm/ScmServletModule.java index 90764a7e00..7cbdc906b8 100644 --- a/scm-webapp/src/main/java/sonia/scm/ScmServletModule.java +++ b/scm-webapp/src/main/java/sonia/scm/ScmServletModule.java @@ -83,8 +83,10 @@ import sonia.scm.security.AuthorizationChangedEventProducer; import sonia.scm.security.CipherHandler; import sonia.scm.security.CipherUtil; import sonia.scm.security.ConfigurableLoginAttemptHandler; +import sonia.scm.security.DefaultJwtAccessTokenRefreshStrategy; import sonia.scm.security.DefaultKeyGenerator; import sonia.scm.security.DefaultSecuritySystem; +import sonia.scm.security.JwtAccessTokenRefreshStrategy; import sonia.scm.security.KeyGenerator; import sonia.scm.security.LoginAttemptHandler; import sonia.scm.security.SecuritySystem; @@ -319,6 +321,8 @@ public class ScmServletModule extends ServletModule // bind(LastModifiedUpdateListener.class); bind(PushStateDispatcher.class).toProvider(PushStateDispatcherProvider.class); + + bind(JwtAccessTokenRefreshStrategy.class).to(DefaultJwtAccessTokenRefreshStrategy.class); } diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java index ebcf88b346..6db01c904f 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefresher.java @@ -31,7 +31,7 @@ public class JwtAccessTokenRefresher { @SuppressWarnings("squid:S3655") // the refresh expiration cannot be null at the time building the new token, because // we checked this before in tokenCanBeRefreshed - Optional refresh(JwtAccessToken oldToken) { + public Optional refresh(JwtAccessToken oldToken) { JwtAccessTokenBuilder builder = builderFactory.create(); Map claims = oldToken.getClaims(); claims.forEach(builder::custom); 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 18980311e6..827aaafa6d 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 @@ -1,11 +1,66 @@ package sonia.scm.web.security; +import org.apache.shiro.authc.AuthenticationToken; import sonia.scm.Priority; import sonia.scm.filter.Filters; import sonia.scm.filter.WebElement; +import sonia.scm.security.AccessToken; +import sonia.scm.security.AccessTokenCookieIssuer; +import sonia.scm.security.AccessTokenResolver; +import sonia.scm.security.BearerToken; +import sonia.scm.security.JwtAccessToken; +import sonia.scm.security.JwtAccessTokenRefresher; +import sonia.scm.web.WebTokenGenerator; +import sonia.scm.web.filter.HttpFilter; + +import javax.inject.Inject; +import javax.servlet.FilterChain; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.util.Set; @Priority(Filters.PRIORITY_POST_AUTHENTICATION) @WebElement(value = Filters.PATTERN_RESTAPI, morePatterns = { Filters.PATTERN_DEBUG }) -public class TokenRefreshFilter { +public class TokenRefreshFilter extends HttpFilter { + + private final Set tokenGenerators; + private final AccessTokenCookieIssuer cookieIssuer; + private final JwtAccessTokenRefresher refresher; + private final AccessTokenResolver resolver; + private final AccessTokenCookieIssuer issuer; + + @Inject + public TokenRefreshFilter(Set tokenGenerators, AccessTokenCookieIssuer cookieIssuer, JwtAccessTokenRefresher refresher, AccessTokenResolver resolver, AccessTokenCookieIssuer issuer) { + this.tokenGenerators = tokenGenerators; + this.cookieIssuer = cookieIssuer; + this.refresher = refresher; + this.resolver = resolver; + this.issuer = issuer; + } + + @Override + protected void doFilter(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws IOException, ServletException { + AuthenticationToken token = createToken(request); + if (token != null && token instanceof BearerToken) { + AccessToken accessToken = resolver.resolve((BearerToken) token); + if (accessToken instanceof JwtAccessToken) { + refresher.refresh((JwtAccessToken) accessToken) + .ifPresent(jwtAccessToken -> issuer.authenticate(request, response, jwtAccessToken)); + } + } + chain.doFilter(request, response); + } + + private AuthenticationToken createToken(HttpServletRequest request) { + for (WebTokenGenerator generator : tokenGenerators) { + AuthenticationToken token = generator.createToken(request); + if (token != null) { + return token; + } + } + return null; + } } From 58268f88db70f9dc0bf55f68852d25402e4216be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 30 Nov 2018 17:19:59 +0100 Subject: [PATCH 15/20] Fix refresh strategy --- .../scm/security/PercentageJwtAccessTokenRefreshStrategy.java | 2 +- .../security/PercentageJwtAccessTokenRefreshStrategyTest.java | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategy.java b/scm-webapp/src/main/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategy.java index 2d5c67c7b6..c78654c389 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategy.java +++ b/scm-webapp/src/main/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategy.java @@ -20,6 +20,6 @@ public class PercentageJwtAccessTokenRefreshStrategy implements JwtAccessTokenRe public boolean shouldBeRefreshed(JwtAccessToken oldToken) { long liveSpan = oldToken.getExpiration().getTime() - oldToken.getIssuedAt().getTime(); long age = clock.instant().toEpochMilli() - oldToken.getIssuedAt().getTime(); - return age/liveSpan > refreshPercentage; + return (float)age/liveSpan > refreshPercentage; } } diff --git a/scm-webapp/src/test/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategyTest.java b/scm-webapp/src/test/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategyTest.java index 2e1fa44a76..122c1b5381 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategyTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/PercentageJwtAccessTokenRefreshStrategyTest.java @@ -47,6 +47,7 @@ public class PercentageJwtAccessTokenRefreshStrategyTest { when(creationClock.instant()).thenReturn(TOKEN_CREATION); tokenBuilder = new JwtAccessTokenBuilderFactory(keyGenerator, keyResolver, Collections.emptySet(), creationClock).create(); + tokenBuilder.expiresIn(1, HOURS); tokenBuilder.refreshableFor(1, HOURS); refreshStrategy = new PercentageJwtAccessTokenRefreshStrategy(refreshClock, 0.5F); @@ -61,6 +62,6 @@ public class PercentageJwtAccessTokenRefreshStrategyTest { @Test public void shouldRefreshWhenTokenIsOld() { when(refreshClock.instant()).thenReturn(TOKEN_CREATION.plus(31, MINUTES)); - assertThat(refreshStrategy.shouldBeRefreshed(tokenBuilder.build())).isFalse(); + assertThat(refreshStrategy.shouldBeRefreshed(tokenBuilder.build())).isTrue(); } } From 80ce5af12a7b11eed614ea942a0a979185db5930 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 30 Nov 2018 17:25:53 +0100 Subject: [PATCH 16/20] Log token refresh --- .../sonia/scm/web/security/TokenRefreshFilter.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) 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 827aaafa6d..6177b0251b 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 @@ -1,6 +1,8 @@ package sonia.scm.web.security; import org.apache.shiro.authc.AuthenticationToken; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import sonia.scm.Priority; import sonia.scm.filter.Filters; import sonia.scm.filter.WebElement; @@ -26,6 +28,8 @@ import java.util.Set; morePatterns = { Filters.PATTERN_DEBUG }) public class TokenRefreshFilter extends HttpFilter { + private static final Logger LOG = LoggerFactory.getLogger(TokenRefreshFilter.class); + private final Set tokenGenerators; private final AccessTokenCookieIssuer cookieIssuer; private final JwtAccessTokenRefresher refresher; @@ -48,12 +52,17 @@ public class TokenRefreshFilter extends HttpFilter { AccessToken accessToken = resolver.resolve((BearerToken) token); if (accessToken instanceof JwtAccessToken) { refresher.refresh((JwtAccessToken) accessToken) - .ifPresent(jwtAccessToken -> issuer.authenticate(request, response, jwtAccessToken)); + .ifPresent(jwtAccessToken -> refreshToken(request, response, jwtAccessToken)); } } chain.doFilter(request, response); } + private void refreshToken(HttpServletRequest request, HttpServletResponse response, JwtAccessToken jwtAccessToken) { + LOG.debug("refreshing authentication token"); + issuer.authenticate(request, response, jwtAccessToken); + } + private AuthenticationToken createToken(HttpServletRequest request) { for (WebTokenGenerator generator : tokenGenerators) { AuthenticationToken token = generator.createToken(request); From e30d32f1cdf29a9c75c80716567eb6a9b247eb1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Sat, 1 Dec 2018 20:46:05 +0100 Subject: [PATCH 17/20] Test things --- .../scm/web/security/TokenRefreshFilter.java | 43 +++---- .../web/security/TokenRefreshFilterTest.java | 107 ++++++++++++++++++ 2 files changed, 130 insertions(+), 20 deletions(-) create mode 100644 scm-webapp/src/test/java/sonia/scm/web/security/TokenRefreshFilterTest.java 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 6177b0251b..f85c0fbbbd 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,8 +21,12 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import java.io.IOException; +import java.util.Optional; import java.util.Set; +import static java.util.Optional.empty; +import static java.util.Optional.of; + @Priority(Filters.PRIORITY_POST_AUTHENTICATION) @WebElement(value = Filters.PATTERN_RESTAPI, morePatterns = { Filters.PATTERN_DEBUG }) @@ -31,15 +35,13 @@ public class TokenRefreshFilter extends HttpFilter { private static final Logger LOG = LoggerFactory.getLogger(TokenRefreshFilter.class); private final Set tokenGenerators; - private final AccessTokenCookieIssuer cookieIssuer; private final JwtAccessTokenRefresher refresher; private final AccessTokenResolver resolver; private final AccessTokenCookieIssuer issuer; @Inject - public TokenRefreshFilter(Set tokenGenerators, AccessTokenCookieIssuer cookieIssuer, JwtAccessTokenRefresher refresher, AccessTokenResolver resolver, AccessTokenCookieIssuer issuer) { + public TokenRefreshFilter(Set tokenGenerators, JwtAccessTokenRefresher refresher, AccessTokenResolver resolver, AccessTokenCookieIssuer issuer) { this.tokenGenerators = tokenGenerators; - this.cookieIssuer = cookieIssuer; this.refresher = refresher; this.resolver = resolver; this.issuer = issuer; @@ -47,29 +49,30 @@ public class TokenRefreshFilter extends HttpFilter { @Override protected void doFilter(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws IOException, ServletException { - AuthenticationToken token = createToken(request); - if (token != null && token instanceof BearerToken) { - AccessToken accessToken = resolver.resolve((BearerToken) token); - if (accessToken instanceof JwtAccessToken) { - refresher.refresh((JwtAccessToken) accessToken) - .ifPresent(jwtAccessToken -> refreshToken(request, response, jwtAccessToken)); + extractToken(request).ifPresent(token -> examineToken(request, response, token)); + chain.doFilter(request, response); + } + + private Optional extractToken(HttpServletRequest request) { + for (WebTokenGenerator generator : tokenGenerators) { + AuthenticationToken token = generator.createToken(request); + if (token instanceof BearerToken) { + return of((BearerToken) token); } } - chain.doFilter(request, response); + return empty(); + } + + private void examineToken(HttpServletRequest request, HttpServletResponse response, BearerToken token) { + AccessToken accessToken = resolver.resolve(token); + if (accessToken instanceof JwtAccessToken) { + refresher.refresh((JwtAccessToken) accessToken) + .ifPresent(jwtAccessToken -> refreshToken(request, response, jwtAccessToken)); + } } private void refreshToken(HttpServletRequest request, HttpServletResponse response, JwtAccessToken jwtAccessToken) { LOG.debug("refreshing authentication token"); issuer.authenticate(request, response, jwtAccessToken); } - - private AuthenticationToken createToken(HttpServletRequest request) { - for (WebTokenGenerator generator : tokenGenerators) { - AuthenticationToken token = generator.createToken(request); - if (token != null) { - return token; - } - } - return null; - } } 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 new file mode 100644 index 0000000000..945d8cf0d2 --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/web/security/TokenRefreshFilterTest.java @@ -0,0 +1,107 @@ +package sonia.scm.web.security; + +import org.apache.shiro.authc.AuthenticationToken; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import sonia.scm.security.AccessTokenCookieIssuer; +import sonia.scm.security.AccessTokenResolver; +import sonia.scm.security.BearerToken; +import sonia.scm.security.JwtAccessToken; +import sonia.scm.security.JwtAccessTokenRefresher; +import sonia.scm.web.WebTokenGenerator; + +import javax.servlet.FilterChain; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.util.Set; + +import static java.util.Collections.singleton; +import static java.util.Optional.of; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@ExtendWith({MockitoExtension.class}) +class TokenRefreshFilterTest { + + @Mock + private Set tokenGenerators; + @Mock + private WebTokenGenerator tokenGenerator; + @Mock + private JwtAccessTokenRefresher refresher; + @Mock + private AccessTokenResolver resolver; + @Mock + private AccessTokenCookieIssuer issuer; + + @InjectMocks + private TokenRefreshFilter filter; + + @Mock + private HttpServletRequest request; + @Mock + private HttpServletResponse response; + @Mock + private FilterChain filterChain; + + @BeforeEach + void initGenerators() { + when(tokenGenerators.iterator()).thenReturn(singleton(tokenGenerator).iterator()); + } + + @Test + void shouldContinueChain() throws IOException, ServletException { + filter.doFilter(request, response, filterChain); + + verify(filterChain).doFilter(request, response); + verify(issuer, never()).authenticate(any(), any(), any()); + } + + @Test + void shouldNotRefreshNonBearerToken() throws IOException, ServletException { + AuthenticationToken token = mock(AuthenticationToken.class); + when(tokenGenerator.createToken(request)).thenReturn(token); + + filter.doFilter(request, response, filterChain); + + verify(issuer, never()).authenticate(any(), any(), any()); + verify(filterChain).doFilter(request, response); + } + + @Test + void shouldNotRefreshNonJwtToken() throws IOException, ServletException { + BearerToken token = mock(BearerToken.class); + JwtAccessToken jwtToken = mock(JwtAccessToken.class); + when(tokenGenerator.createToken(request)).thenReturn(token); + when(resolver.resolve(token)).thenReturn(jwtToken); + + filter.doFilter(request, response, filterChain); + + verify(issuer, never()).authenticate(any(), any(), any()); + verify(filterChain).doFilter(request, response); + } + + @Test + void shouldRefreshIfRefreshable() throws IOException, ServletException { + BearerToken token = mock(BearerToken.class); + JwtAccessToken jwtToken = mock(JwtAccessToken.class); + JwtAccessToken newJwtToken = mock(JwtAccessToken.class); + when(tokenGenerator.createToken(request)).thenReturn(token); + when(resolver.resolve(token)).thenReturn(jwtToken); + when(refresher.refresh(jwtToken)).thenReturn(of(newJwtToken)); + + filter.doFilter(request, response, filterChain); + + verify(issuer).authenticate(request, response, newJwtToken); + verify(filterChain).doFilter(request, response); + } +} From 581e6a9bffb9f228ba66c073992cb044c3881bf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Mon, 3 Dec 2018 08:20:41 +0100 Subject: [PATCH 18/20] Fix extension point injection --- scm-webapp/src/main/java/sonia/scm/ScmServletModule.java | 2 -- .../java/sonia/scm/security/JwtAccessTokenRefreshStrategy.java | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/ScmServletModule.java b/scm-webapp/src/main/java/sonia/scm/ScmServletModule.java index 7cbdc906b8..9555ad66b5 100644 --- a/scm-webapp/src/main/java/sonia/scm/ScmServletModule.java +++ b/scm-webapp/src/main/java/sonia/scm/ScmServletModule.java @@ -321,8 +321,6 @@ public class ScmServletModule extends ServletModule // bind(LastModifiedUpdateListener.class); bind(PushStateDispatcher.class).toProvider(PushStateDispatcherProvider.class); - - bind(JwtAccessTokenRefreshStrategy.class).to(DefaultJwtAccessTokenRefreshStrategy.class); } diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefreshStrategy.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefreshStrategy.java index f7f030d1f6..9135a0e099 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefreshStrategy.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenRefreshStrategy.java @@ -2,7 +2,7 @@ package sonia.scm.security; import sonia.scm.plugin.ExtensionPoint; -@ExtensionPoint +@ExtensionPoint(multi = false) public interface JwtAccessTokenRefreshStrategy { boolean shouldBeRefreshed(JwtAccessToken oldToken); } From f0665663ec79aa2284cdcbfd0ef9f0714a7f3851 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 4 Dec 2018 16:45:21 +0100 Subject: [PATCH 19/20] Use property for jjwt version --- scm-webapp/pom.xml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/scm-webapp/pom.xml b/scm-webapp/pom.xml index 4503274adb..01f45cb54e 100644 --- a/scm-webapp/pom.xml +++ b/scm-webapp/pom.xml @@ -73,17 +73,17 @@ io.jsonwebtoken jjwt-impl - 0.10.5 + ${jjwt.version} io.jsonwebtoken jjwt-api - 0.10.5 + ${jjwt.version} io.jsonwebtoken jjwt-jackson - 0.10.5 + ${jjwt.version} @@ -550,6 +550,7 @@ DEVELOPMENT target/scm-it default + 0.10.5 2.53.1 1.0 0.8.17 From 8aeb6333377e977036e0e1917d18f89fbf307914 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Wed, 5 Dec 2018 09:14:16 +0000 Subject: [PATCH 20/20] Close branch feature/jwt_refresh