Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Audience single string claim data type retention when copying #891

Merged
merged 1 commit into from
Jan 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
## Release Notes

### 0.12.4

This patch release:

* Ensures Android environments and older `org.json` library usages can parse JSON from a `JwtBuilder`-provided
`java.io.Reader` instance. [Issue 882](https://github.com/jwtk/jjwt/issues/882).
* Ensures a single string `aud` (Audience) claim is retained (without converting it to a `Set`) when copying/applying a
source Claims instance to a destination Claims builder. [Issue 890](https://github.com/jwtk/jjwt/issues/890).

### 0.12.3

This patch release:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import io.jsonwebtoken.lang.Strings;

import java.util.Date;
import java.util.Map;
import java.util.Set;

/**
Expand All @@ -46,6 +47,31 @@ <F> T put(Parameter<F> param, F value) {
return self();
}

@Override
public Object put(String key, Object value) {
if (AUDIENCE_STRING.getId().equals(key)) { // https://github.com/jwtk/jjwt/issues/890
if (value instanceof String) {
Object existing = get(key);
//noinspection deprecation
audience().single((String) value);
return existing;
}
// otherwise ensure that the Parameter type is the RFC-default data type (JSON Array of Strings):
getAudience();
}
// otherwise retain expected behavior:
return super.put(key, value);
}

@Override
public void putAll(Map<? extends String, ?> m) {
if (m == null) return;
for (Map.Entry<? extends String, ?> entry : m.entrySet()) {
String s = entry.getKey();
put(s, entry.getValue()); // ensure local put is called per https://github.com/jwtk/jjwt/issues/890
}
}

<F> F get(Parameter<F> param) {
return this.DELEGATE.get(param);
}
Expand Down
31 changes: 31 additions & 0 deletions impl/src/test/groovy/io/jsonwebtoken/JwtsTest.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import io.jsonwebtoken.impl.lang.Bytes
import io.jsonwebtoken.impl.lang.Services
import io.jsonwebtoken.impl.security.*
import io.jsonwebtoken.io.Decoders
import io.jsonwebtoken.io.Deserializer
import io.jsonwebtoken.io.Encoders
import io.jsonwebtoken.io.Serializer
import io.jsonwebtoken.lang.Strings
Expand Down Expand Up @@ -1167,6 +1168,36 @@ class JwtsTest {
.build().parseSignedClaims(jws)
}

/**
* Asserts that if a {@link Jwts#claims()} builder is used to set a single string Audience value, and the
* resulting constructed {@link Claims} instance is used on a {@link Jwts#builder()}, that the resulting JWT
* retains a single-string Audience value (and it is not automatically coerced to a {@code Set<String>}).
*
* @since 0.12.4
* @see <a href="https://github.com/jwtk/jjwt/issues/890">JJWT Issue 890</a>
*/
@Test
void testClaimsBuilderSingleStringAudienceThenJwtBuilder() {

def key = TestKeys.HS256
def aud = 'foo'
def claims = Jwts.claims().audience().single(aud).build()
def jws = Jwts.builder().claims(claims).signWith(key).compact()

// we can't use a JwtParser here because that will automatically normalize a single String value as a
// Set<String> for app developer convenience. So we assert that the JWT looks as expected by simple
// json parsing and map inspection

int i = jws.indexOf('.')
int j = jws.lastIndexOf('.')
def b64 = jws.substring(i, j)
def json = Strings.utf8(Decoders.BASE64URL.decode(b64))
def deser = Services.loadFirst(Deserializer)
def m = deser.deserialize(new StringReader(json)) as Map<String,?>

assertEquals aud, m.get('aud') // single string value
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@devdanylo and @arvindkrishnakumar-okta

Please feel free to take a look at this PR and the test case immediately above, especially lines 1184, 1185 and 1198.

Tests pass locally, I'm just waiting for CI to verify.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the quick response!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you  🤝

//Asserts correct/expected behavior discussed in https://github.com/jwtk/jjwt/issues/20
@Test
void testForgedTokenWithSwappedHeaderUsingNoneAlgorithm() {
Expand Down