SDK-2771: Add new maven module for supporting the creation of Yoti au… by MrBurtyyy · Pull Request #505 · getyoti/yoti-java-sdk

MrBurtyyy · 2025-12-11T12:29:20Z

…thentication tokens via API

pn-santos

I'm not that familiar with the current practices in the SDK nor any stylistic conventions in place so apologies in advance if any of the comments are out-of-sync with how stuff is done in the SDKs.

Also, given I'm not really involved with the SDKs don't take the comments as blockers. I'm not going to explicitly approve, I'll leave that to someone more attuned to the SDK code base to take that responsibility, but I don't see anything functional that would be a reason to hold it up.

pn-santos · 2025-12-15T13:07:44Z

+    private final SignedJwtGenerator signedJwtGenerator;
+
+    private final String authApiUrl;
+    private final ObjectMapper objectMapper;


Why is the ObjectMapper being injected/exposed?

The CreateAuthenticationTokenResponse is defined by this module, shouldn't the ObjectMapper (and it's config) also be owned/defined by this module? What would be the point of the client providing their own instance config if they do not control the annotations/layout of CreateAuthenticationTokenResponse?

We don't expose the ObjectMapper to the Relying Business, but it's done this way (and in other places in the SDK) to make testing with mocks easier in the AuthenticationTokenGenerator

pn-santos · 2025-12-15T13:11:48Z

+            ObjectMapper objectMapper = new ObjectMapper()
+                    .setPropertyNamingStrategy(PropertyNamingStrategies.SNAKE_CASE)
+                    .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);


If we do not expose the ObjectMapper then there's no point in making it an instance var. It should be declared as a static final var of AuthenticationTokenGenerator

pn-santos · 2025-12-15T14:34:05Z

+        this.signedJwtGenerator = signedJwtGenerator;
+        this.objectMapper = objectMapper;
+
+        authApiUrl = System.getProperty(Properties.PROPERTY_YOTI_AUTH_URL, Properties.DEFAULT_YOTI_AUTH_URL);


This class is the sole user of this Properties class... why not simply declare those values as constants here? i.e.

private static final String YOTI_AUTH_URL_KEY = "yoti.auth.url"; (...) authApiUrl = System.getProperty(YOTI_AUTH_URL_KEY, "https://auth.api.yoti.com/v1/oauth/token");

and get rid of Properties altogether?

We have public properties in here, so the YOTI_AUTH_URL_KEY property can be accessed if we want to override the URL without knowing the actual key needed (like we do in the other modules).

Not sure if we want to change this, @bucky-boy what do you think?

irotech · 2025-12-15T15:55:12Z

+        String jwts = signedJwtGenerator.create(sdkId, keyPair, jwtIdSupplier, authApiUrl);
+
+        Map<String, String> formParams = new HashMap<>(DEFAULT_FORM_VALUES);
+        formParams.put("scope", String.join(" ", scopes));


Should this list be checked for null or empty string?

irotech · 2025-12-15T16:22:08Z

+     * @throws IOException
+     */
+    public CreateAuthenticationTokenResponse generate(List<String> scopes) throws ResourceException, IOException {
+        String jwts = signedJwtGenerator.create(sdkId, keyPair, jwtIdSupplier, authApiUrl);


can't this be inlined?

pn-santos · 2025-12-16T12:27:09Z

+                    try {
+                        return encode(entry.getKey()) + "=" + encode(entry.getValue());
+                    } catch (UnsupportedEncodingException e) {
+                        throw new RuntimeException(e);
+                    }


If doing the try/catch to turn it into a runtime exception it would probably make more sense to do it within the encode method.

irotech

only not blocking minor comments

irotech · 2025-12-16T12:42:17Z

+        formParams.put("grant_type", "client_credentials");
+        formParams.put("client_assertion_type", "urn:ietf:params:oauth:client-assertion-type:jwt-bearer");
+        formParams.put("scope", String.join(" ", scopes));
+        formParams.put("client_assertion", jwts);


why having a var if used once?

Suggested change

formParams.put("client_assertion", jwts);

formParams.put("client_assertion", createSignedJwt(sdkId, keyPair, jwtIdSupplier, authApiUrl));

irotech · 2025-12-16T12:47:31Z

+        connection.setRequestMethod(method);
+        connection.setRequestProperty("Content-Type", "application/x-www-form-urlencoded");
+        connection.setRequestProperty("Content-Length", Integer.toString(postData.length));
+        connection.setRequestProperty("charset", "utf-8");


Suggested change

connection.setRequestProperty("charset", "utf-8");

connection.setRequestProperty("charset", StandardCharsets.UTF_8);

irotech · 2025-12-16T12:49:09Z

+            + "vd67J8cdh1v6d3m0xrT639BIh7FIZjVIg3B8ERBmIH1oFn05BQFYlCEUG7Cl1KV2\n"
+            + "/VIBAoGAYin6eHFtBXpPRMddTt4lLOFtpBb9gGp9DxKGAsH0pdfQYgZHz0xKtxrq\n"
+            + "6l+d2M9sRhtisxw8YevPFWEVtcUWxz+qESM+wzUMrmWSuwwxXyVgjR4rC8untfPM\n"
+            + "laDBX8dDBwBQ3i0FhvuGR/LEjHWr9hj00faKROHOLFim6wbRIkg=\n" //


is this empty comment, "//", in there for any reason?

…thentication tokens via API

sonarqubecloud · 2025-12-16T14:30:30Z

Quality Gate passed

Issues
10 New issues
0 Accepted issues

Measures
0 Security Hotspots
47.5% Coverage on New Code
2.2% Duplication on New Code

See analysis details on SonarQube Cloud

MrBurtyyy requested review from bucky-boy, irotech and pn-santos December 11, 2025 12:32

MrBurtyyy force-pushed the SDK-2771 branch 2 times, most recently from 9d1586b to d994ecd Compare December 11, 2025 13:58

bucky-boy approved these changes Dec 11, 2025

View reviewed changes

pn-santos reviewed Dec 15, 2025

View reviewed changes

irotech reviewed Dec 15, 2025

View reviewed changes

MrBurtyyy requested review from irotech and pn-santos December 16, 2025 12:16

pn-santos approved these changes Dec 16, 2025

View reviewed changes

irotech approved these changes Dec 16, 2025

View reviewed changes

SDK-2771: Add new maven module for supporting the creation of Yoti au…

567520b

…thentication tokens via API

MrBurtyyy force-pushed the SDK-2771 branch from 9a2856f to 567520b Compare December 16, 2025 14:27

MrBurtyyy merged commit 428ae21 into DEVELOPMENT Dec 16, 2025
9 checks passed

MrBurtyyy deleted the SDK-2771 branch December 16, 2025 15:10

	formParams.put("client_assertion", jwts);
	formParams.put("client_assertion", createSignedJwt(sdkId, keyPair, jwtIdSupplier, authApiUrl));

	connection.setRequestProperty("charset", "utf-8");
	connection.setRequestProperty("charset", StandardCharsets.UTF_8);

Conversation

MrBurtyyy commented Dec 11, 2025

Uh oh!

pn-santos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

irotech left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Dec 16, 2025

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants