fix well known endpoint. #136
Conversation
WalkthroughThis pull request modifies the URL construction for OAuth well-known endpoints by adding a forward slash (/) before the “.well-known” segment in two methods within the internal resource. Additionally, test files are updated: a hardcoded callback URL is replaced with a placeholder and a test method is renamed while updating its endpoint reference. No changes were made to exported or public declarations. Changes
Assessment against linked issues
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
auth/src/jvmTest/kotlin/work/socialhub/kbsky/auth/OAuthTest.kt (1)
53-53: Add null safety checks for required URL parameters.The code assumes the 'code' parameter will always be present and uses a non-null assertion (!!) which could cause runtime failures.
Add proper null checking:
- val code = Url(callback).parameters["code"].also { println(it) } + val code = Url(callback).parameters["code"] + ?: throw IllegalArgumentException("Missing required 'code' parameter in callback URL") + println(code) val response = AuthFactory .instance(BSKY_SOCIAL.uri) .oauth() .authorizationCodeTokenRequest(oAuthContext, OAuthAuthorizationCodeTokenRequest().also { - it.code = code!! + it.code = code } )Also applies to: 60-60
🧹 Nitpick comments (2)
auth/src/jvmTest/kotlin/work/socialhub/kbsky/auth/OAuthTest.kt (1)
47-47: Document the expected callback URL format and required parameters.The placeholder
{{CALLBACK_URL}}makes the test more flexible, but it needs documentation to ensure proper usage.Add a comment explaining:
- The expected format of the callback URL
- Required URL parameters (especially 'code' which is used in the test)
- Example of a valid callback URL
+ /** + * Example callback URL format: + * http://example.com/callback?code=auth_code&state=state_value&iss=issuer + * Required parameters: + * - code: OAuth authorization code + * Optional parameters: + * - iss: Issuer (currently commented out) + * - state: State parameter (currently commented out) + */ val callback = """ {{CALLBACK_URL}} """auth/src/commonMain/kotlin/work/socialhub/kbsky/auth/internal/_WellKnownResource.kt (1)
40-40: LGTM! Consider extracting the well-known path segment to a constant.The URL path is now correctly formatted. However, since both methods use the
.well-knownpath segment, consider extracting it to avoid duplication.class _WellKnownResource( private val config: AuthConfig ) : WellKnownResource { + companion object { + private const val WELL_KNOWN_PATH = "/.well-known" + private const val OAUTH_PROTECTED_RESOURCE_PATH = "$WELL_KNOWN_PATH/oauth-protected-resource" + private const val OAUTH_AUTHORIZATION_SERVER_PATH = "$WELL_KNOWN_PATH/oauth-authorization-server" + } override fun oAuthProtectedResource() : Response<WellKnownOAuthProtectedResourceResponse> { return proceed<WellKnownOAuthProtectedResourceResponse> { runBlocking { HttpRequest() - .url("${config.pdsServer}/.well-known/oauth-protected-resource") + .url("${config.pdsServer}$OAUTH_PROTECTED_RESOURCE_PATH") .accept(MediaType.JSON) .get() } } } override fun oAuthAuthorizationServer() : Response<WellKnownOAuthAuthorizationServerResponse> { return proceed<WellKnownOAuthAuthorizationServerResponse> { runBlocking { HttpRequest() - .url("${config.authorizationServer}/.well-known/oauth-authorization-server") + .url("${config.authorizationServer}$OAUTH_AUTHORIZATION_SERVER_PATH") .accept(MediaType.JSON) .get() } } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
auth/src/commonMain/kotlin/work/socialhub/kbsky/auth/internal/_WellKnownResource.kt(2 hunks)auth/src/jvmTest/kotlin/work/socialhub/kbsky/auth/OAuthTest.kt(1 hunks)auth/src/jvmTest/kotlin/work/socialhub/kbsky/auth/WellKnownTest.kt(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (5)
auth/src/jvmTest/kotlin/work/socialhub/kbsky/auth/WellKnownTest.kt (3)
4-4: LGTM!The import statement for OYSTER_US_EAST follows the existing pattern and is required for the test method update.
12-12: Verify the endpoint change rationale.The test now uses OYSTER_US_EAST.uri instead of BSKY_SOCIAL.uri. While this change might be intentional for testing against different endpoints, please confirm if this is part of the fix for the well-known endpoint issue (#135).
19-19: LGTM! Great improvement in method naming.The renamed method
testWellKnownOAuthAuthorizationServeris more descriptive and clearly indicates its purpose, improving code readability.auth/src/jvmTest/kotlin/work/socialhub/kbsky/auth/OAuthTest.kt (1)
46-50: Verify test functionality with the placeholder.The test now uses a placeholder for the callback URL. Ensure there's a mechanism in place to replace this placeholder with actual values during test execution.
Please confirm:
- How is the placeholder replaced with actual values during testing?
- Are there any test setup methods or test data providers that handle this?
- Does this change affect CI/CD pipeline execution?
auth/src/commonMain/kotlin/work/socialhub/kbsky/auth/internal/_WellKnownResource.kt (1)
22-22: LGTM! The URL path now correctly follows RFC 8615.The addition of the forward slash before
.well-knownensures proper URL construction and compliance with the well-known URI standard.✅ Verification successful
🌐 Web query:
What is the correct path format for well-known URIs according to RFC 8615?💡 Result:
The correct path format for well-known URIs as defined in **RFC 8615** begins with the reserved prefix `/.well-known/`, followed by a registered suffix specific to the application or protocol. For example: ``` http://example.com/.well-known/<registered-suffix> ``` Key details of this format include: 1. **Path prefix**: All well-known URIs **must** start with `/.well-known/` in their path component[1][5][7]. This standardized prefix ensures consistency across implementations. 2. **Supported URI schemes**: The specification applies to HTTP, HTTPS, WebSocket (WS), and Secure WebSocket (WSS) URIs by default[5][7]. Other schemes may adopt it only if explicitly defined[7]. 3. **Registered suffixes**: Suffixes like `acme-challenge` (for Let's Encrypt domain validation) or `security.txt` (for security policies) must be formally registered with IETF[2][7]. Example: ``` https://example.com/.well-known/acme-challenge/xyz123 ``` 4. **Collision avoidance**: The registry system prevents conflicts between different uses of the same suffix[2][7]. Developers proposing new suffixes must submit them for review via IETF’s process[2]. RFC 8615 obsoletes RFC 5785 and updates earlier standards like HTTP/1.1 (RFC 7230) to reserve this URI space[1][5]. Well-known URIs are widely used for domain validation, security policies, and service discovery[2][10]. Citations: - 1: https://www.rfc-editor.org/info/rfc8615 - 2: https://www.keycdn.com/support/well-known - 3: https://tex2e.github.io/rfc-translater/html/rfc8615.html - 4: https://dl.acm.org/doi/10.17487/RFC8615 - 5: https://www.rfc-editor.org/rfc/rfc8615.html - 6: https://datatracker.ietf.org/doc/html/rfc5785 - 7: https://datatracker.ietf.org/doc/html/rfc8615 - 8: https://en.wikipedia.org/wiki/Well-known_URI - 9: https://en.wikipedia.org/wiki/Uniform_Resource_Locator - 10: https://blog.devgenius.io/well-known-locations-a-reserved-uri-path-prefix-5277b2f37db7?gi=8d00a1ca3654
LGTM – The URL construction correctly uses the required "/.well-known/" prefix per RFC 8615.
The updated code now ensures that the pdsServer value is appropriately combined with the reserved URI path, maintaining proper format regardless of trailing slashes. This change aligns with the specification detailed in RFC 8615.
fix #135
Summary by CodeRabbit