Skip to content

Commit 4bf5ab9

Browse files
authored
[ZEPPELIN-6626] Fix JWT expiration validation security vulnerability
### What is this PR for? This PR fixes a critical security vulnerability in JWT token validation where tokens without expiration time were incorrectly accepted as valid, potentially allowing indefinite unauthorized access. The fix ensures all JWT tokens must have a valid expiration time and adds comprehensive unit tests to prevent regression. ### What type of PR is it? Bug Fix ### Todos * [x] - Fix JWT expiration validation logic to reject null expiration tokens * [x] - Add security warning logs for rejected tokens * [x] - Create comprehensive unit tests for JWT validation scenarios * [x] - Verify backwards compatibility with existing valid tokens ### What is the Jira issue? * [ZEPPELIN-6266](https://issues.apache.org/jira/browse/ZEPPELIN-6266) Fix JWT expiration validation security vulnerability ### How should this be tested? * **Automated Unit Tests Added**: - New test file: `KnoxJwtRealmTest.java` with 3 comprehensive test scenarios - Run: `mvn test -Dtest=KnoxJwtRealmTest -pl zeppelin-server` - All tests pass: `Tests run: 3, Failures: 0, Errors: 0, Skipped: 0` * **Manual Testing Steps**: 1. Create JWT token without expiration time → Should be rejected with security warning 2. Create JWT token with valid future expiration → Should be accepted 3. Create JWT token with past expiration → Should be rejected 4. Check server logs for security warnings: "JWT token has no expiration time - rejecting token for security" * **Security Validation**: - Verify that tokens without expiration are properly rejected - Confirm existing valid tokens continue to work - Check security event logging ### Screenshots (if appropriate) N/A - Security fix with no UI changes ### Questions: * **Does the license files need to update?** No - only modified existing files and added test files with standard Apache license headers * **Is there breaking changes for older versions?** No - fully backwards compatible. Only affects tokens with missing expiration (which should not exist in proper JWT implementations) * **Does this needs documentation?** No - internal security fix that doesn't change public APIs or configuration Closes #5007 from chadongmin/ZEPPELIN-6266. Signed-off-by: Jongyoul Lee <jongyoul@gmail.com>
1 parent ab52456 commit 4bf5ab9

File tree

2 files changed

+108
-4
lines changed

2 files changed

+108
-4
lines changed

zeppelin-server/src/main/java/org/apache/zeppelin/realm/jwt/KnoxJwtRealm.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -192,10 +192,12 @@ protected boolean validateExpiration(SignedJWT jwtToken) {
192192
boolean valid = false;
193193
try {
194194
Date expires = jwtToken.getJWTClaimsSet().getExpirationTime();
195-
if (expires == null || new Date().before(expires)) {
196-
if (LOGGER.isDebugEnabled()) {
197-
LOGGER.debug("SSO token expiration date has been " + "successfully validated");
198-
}
195+
if (expires == null) {
196+
LOGGER.warn("JWT token has no expiration time - rejecting token for security");
197+
return false;
198+
}
199+
if (new Date().before(expires)) {
200+
LOGGER.debug("SSO token expiration date has been successfully validated");
199201
valid = true;
200202
} else {
201203
LOGGER.warn("SSO expiration date validation failed.");
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.zeppelin.realm.jwt;
18+
19+
import com.nimbusds.jose.JWSAlgorithm;
20+
import com.nimbusds.jose.JWSHeader;
21+
import com.nimbusds.jose.JWSSigner;
22+
import com.nimbusds.jose.crypto.RSASSASigner;
23+
import com.nimbusds.jwt.JWTClaimsSet;
24+
import com.nimbusds.jwt.SignedJWT;
25+
import org.junit.jupiter.api.BeforeEach;
26+
import org.junit.jupiter.api.Test;
27+
28+
import java.util.Date;
29+
30+
import static org.junit.jupiter.api.Assertions.*;
31+
32+
public class KnoxJwtRealmTest {
33+
34+
private KnoxJwtRealm knoxJwtRealm;
35+
36+
@BeforeEach
37+
void setUp() throws Exception {
38+
knoxJwtRealm = new KnoxJwtRealm();
39+
}
40+
41+
private SignedJWT createJWT(String subject, Date expiration) throws Exception {
42+
JWTClaimsSet.Builder claimsBuilder = new JWTClaimsSet.Builder()
43+
.subject(subject)
44+
.issuer("KNOXSSO")
45+
.issueTime(new Date());
46+
47+
if (expiration != null) {
48+
claimsBuilder.expirationTime(expiration);
49+
}
50+
51+
JWTClaimsSet claimsSet = claimsBuilder.build();
52+
SignedJWT signedJWT = new SignedJWT(
53+
new JWSHeader.Builder(JWSAlgorithm.RS256).build(),
54+
claimsSet
55+
);
56+
57+
// Note: We don't sign the JWT for these tests as we're only testing expiration validation
58+
return signedJWT;
59+
}
60+
61+
@Test
62+
void testValidateExpiration_WithNullExpiration_ShouldReturnFalse() throws Exception {
63+
// Given: JWT token without expiration time
64+
SignedJWT jwtWithoutExpiration = createJWT("testuser", null);
65+
66+
// When: validating expiration
67+
boolean result = knoxJwtRealm.validateExpiration(jwtWithoutExpiration);
68+
69+
// Then: should return false
70+
assertFalse(result, "JWT token without expiration time should be rejected");
71+
}
72+
73+
@Test
74+
void testValidateExpiration_WithValidFutureExpiration_ShouldReturnTrue() throws Exception {
75+
// Given: JWT token with future expiration
76+
Date futureDate = new Date(System.currentTimeMillis() + 3600000); // 1 hour from now
77+
SignedJWT jwtWithValidExpiration = createJWT("testuser", futureDate);
78+
79+
// When: validating expiration
80+
boolean result = knoxJwtRealm.validateExpiration(jwtWithValidExpiration);
81+
82+
// Then: should return true
83+
assertTrue(result, "JWT token with valid future expiration should be accepted");
84+
}
85+
86+
@Test
87+
void testValidateExpiration_WithPastExpiration_ShouldReturnFalse() throws Exception {
88+
// Given: JWT token with past expiration
89+
Date pastDate = new Date(System.currentTimeMillis() - 3600000); // 1 hour ago
90+
SignedJWT jwtWithPastExpiration = createJWT("testuser", pastDate);
91+
92+
// When: validating expiration
93+
boolean result = knoxJwtRealm.validateExpiration(jwtWithPastExpiration);
94+
95+
// Then: should return false
96+
assertFalse(result, "JWT token with past expiration should be rejected");
97+
}
98+
99+
// Note: Full token validation tests are omitted as they require complex setup
100+
// including certificate files and signature validation. The core expiration
101+
// validation logic is tested above through direct method calls.
102+
}

0 commit comments

Comments
 (0)