Skip to content

fix: Update OAuth authentication URL host to www domain & Bump version to 0.2.2 #30

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.

Sign up for GitHub

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

Merged
chyroc merged 8 commits into from
Jan 23, 2025
Merged

fix: Update OAuth authentication URL host to www domain & Bump version to 0.2.2 #30

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8bc92ed
fix: callback url
hanzeINGH Jan 23, 2025
d87e548
fix: callback url
hanzeINGH Jan 23, 2025
9f8a0c8
Update OAuthClient.java
hanzeINGH Jan 23, 2025
8b4efe4
Update pom.xml
hanzeINGH Jan 23, 2025
c8c1a1f
fix: callback url
hanzeINGH Jan 23, 2025
1274441
fix: callback url
hanzeINGH Jan 23, 2025
37c1ddc
fix
hanzeINGH Jan 23, 2025
4b113ee
fix
hanzeINGH Jan 23, 2025
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
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ jobs:
- name: Set up JDK ${{ matrix.java-version }}
uses: actions/setup-java@v4
with:
distribution: 'temurin'
distribution: 'adopt'
java-version: ${{ matrix.java-version }}
cache: 'maven'
- name: Code style check
Expand Down
2 changes: 1 addition & 1 deletion api/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
</parent>

<artifactId>coze-api</artifactId>
<version>0.2.1</version>
<version>0.2.2</version>

<scm>
<connection>scm:git:git://github.com/coze-dev/coze-java.git</connection>
Expand Down
17 changes: 14 additions & 3 deletions api/src/main/java/com/coze/openapi/service/auth/OAuthClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ public abstract class OAuthClient {
protected final String clientSecret;
protected final String clientID;
protected final String baseURL;
protected final String wwwURL;
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jan 23, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Validate wwwURL construction

While the URL domain separation is correct, the current implementation using simple string replacement could be fragile.

Consider adding validation and using URL parsing:

-    this.wwwURL = Strings.replace(baseURL, "api.", "www.");
+    try {
+        java.net.URL url = new java.net.URL(baseURL);
+        String host = url.getHost().replace("api.", "www.");
+        this.wwwURL = new java.net.URL(url.getProtocol(), host, url.getPort(), "").toString();
+    } catch (java.net.MalformedURLException e) {
+        throw new IllegalArgumentException("Invalid base URL: " + baseURL, e);
+    }

Also applies to: 59-59

protected final CozeAuthAPI api;
protected final ExecutorService executorService;
protected final String hostName;
Expand All @@ -54,6 +55,11 @@ protected OAuthClient(OAuthBuilder<?> builder) {
this.clientSecret = builder.clientSecret;
this.clientID = builder.clientID;
this.baseURL = builder.baseURL;
if (builder.wwwURL != null) {
this.wwwURL = builder.wwwURL;
} else {
this.wwwURL = builder.baseURL.replace("api.", "www.");
}
if (this.baseURL != null && !this.baseURL.isEmpty()) {
try {
java.net.URL url = new java.net.URL(this.baseURL);
Expand Down Expand Up @@ -121,10 +127,9 @@ private String _getOAuthURL(
params.put("code_challenge_method", codeChallengeMethod);
}

String uri = baseURL + "/api/permission/oauth2/authorize";
String uri = wwwURL + "/api/permission/oauth2/authorize";
if (workspaceID != null) {
uri =
baseURL + String.format("/api/permission/oauth2/workspace_id/%s/authorize", workspaceID);
uri = wwwURL + String.format("/api/permission/oauth2/workspace_id/%s/authorize", workspaceID);
}

String queryString =
Expand Down Expand Up @@ -251,6 +256,7 @@ public abstract static class OAuthBuilder<T extends OAuthBuilder<T>> {
protected String clientID;
protected String clientSecret;
protected String baseURL;
protected String wwwURL;
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jan 23, 2025

Choose a reason for hiding this comment

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

💡 Codebase verification

Add test coverage for wwwURL builder methods

The codebase analysis confirms that wwwURL builder functionality lacks test coverage. Existing OAuth client tests demonstrate thorough testing patterns for other builder properties, but wwwURL-specific tests are missing. Please add test cases to verify:

  • Setting custom wwwURL
  • Fallback to baseURL conversion
  • Invalid URL handling
🔗 Analysis chain

Add test coverage for wwwURL builder methods.

The new builder methods for wwwURL are not covered by tests. Please add test cases to verify:

  • Setting custom wwwURL
  • Fallback to baseURL conversion
  • Invalid URL handling

Also applies to: 285-289

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for existing test coverage of wwwURL functionality

# Search for test files that might contain wwwURL-related tests
echo "Searching for existing test files..."
fd -e java -e kt test

# Search for wwwURL usage in test files
echo -e "\nSearching for wwwURL test coverage..."
rg -l "wwwURL" $(fd -e java -e kt test)

Length of output: 4911

Script:

#!/bin/bash
# Check for builder pattern usage in auth tests
echo "Checking builder usage in auth tests..."
rg -A 5 -B 5 "\.builder\(\)" api/src/test/java/com/coze/openapi/service/auth/

# Check for any indirect wwwURL testing through setters
echo -e "\nChecking for indirect wwwURL testing..."
rg -A 5 -B 5 "\.setWww" api/src/test/java/com/coze/openapi/service/auth/

Length of output: 338

Script:

#!/bin/bash
# Check test structure and initialization patterns
echo "Analyzing OAuthClient test implementations..."
rg -l "OAuthClient" api/src/test/java/com/coze/openapi/service/auth/ | xargs cat

# Broader search for builder-related patterns
echo -e "\nChecking for builder patterns..."
rg "new\s+.*Builder|builder\s*\(\)" api/src/test/java/com/coze/openapi/service/auth/

Length of output: 30942

protected int readTimeout;
protected int connectTimeout;
protected OkHttpClient client;
Expand All @@ -276,6 +282,11 @@ public T baseURL(String baseURL) {
return self();
}

public T wwwURL(String wwwURL) {
this.wwwURL = wwwURL;
return self();
}

public T readTimeout(int readTimeout) {
this.readTimeout = readTimeout;
return self();
Expand Down
2 changes: 1 addition & 1 deletion api/src/main/java/com/coze/openapi/service/utils/UserAgentInterceptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public Response intercept(Chain chain) throws IOException {
return chain.proceed(request);
}

public static final String VERSION = "0.2.1";
public static final String VERSION = "0.2.2";
private static final ObjectMapper objectMapper = new ObjectMapper();

/** 获取操作系统版本 */
Expand Down
2 changes: 1 addition & 1 deletion example/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
<dependency>
<groupId>com.coze</groupId>
<artifactId>coze-api</artifactId>
<version>0.2.1</version>
<version>0.2.2</version>
</dependency>
</dependencies>
</project>
2 changes: 1 addition & 1 deletion example/src/main/java/example/auth/PKCEOAuthExample.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public static void main(String[] args) {
to select the code_challenge_method.
* */
GetPKCEAuthURLResp oauthURL =
oauth.genOAuthURL(redirectURI, "state", PKCEOAuthClient.CodeChallengeMethod.S256);
oauth.genOAuthURL(redirectURI, "states", PKCEOAuthClient.CodeChallengeMethod.S256);
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jan 23, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Critical: Incorrect OAuth state parameter name breaks CSRF protection

The state parameter has been changed from "state" to "states", which deviates from the OAuth 2.0 specification. This is incorrect and could break the CSRF protection mechanism, as OAuth providers expect the standard parameter name "state".

Apply this fix to align with the OAuth 2.0 specification:

-        oauth.genOAuthURL(redirectURI, "states", PKCEOAuthClient.CodeChallengeMethod.S256);
+        oauth.genOAuthURL(redirectURI, "state", PKCEOAuthClient.CodeChallengeMethod.S256);

This is also consistent with the example shown in the comments at line 56.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
oauth.genOAuthURL(redirectURI, "states", PKCEOAuthClient.CodeChallengeMethod.S256);
oauth.genOAuthURL(redirectURI, "state", PKCEOAuthClient.CodeChallengeMethod.S256);

System.out.println(oauthURL);

/*
Expand Down
4 changes: 2 additions & 2 deletions example/src/main/java/example/auth/WebOAuthExample.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,13 @@ public static void main(String[] args) {
.build();

// Generate the authorization link and direct the user to open it.
String oauthURL = oauth.getOAuthURL(redirectURI, null);
String oauthURL = oauth.getOAuthURL(redirectURI, "state");
System.out.println(oauthURL);

/*
* The space permissions for which the Access Token is granted can be specified. As following codes:
* */
oauthURL = oauth.getOAuthURL(redirectURI, null, "workspaceID");
oauthURL = oauth.getOAuthURL(redirectURI, "state", "workspaceID");
System.out.println(oauthURL);

/*
Expand Down