-
Notifications
You must be signed in to change notification settings - Fork 1.8k
[OPIK-8252] [BE] feat: Report expires_at from MCP OAuth token introspection #8153
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -183,6 +183,7 @@ public Optional<ValidatedToken> validateAccessToken(@NonNull String token) { | |
| .workspaceId(row.workspaceId()) | ||
| .workspaceName(row.workspaceName()) | ||
| .resource(row.resource()) | ||
| .expiresAt(row.expiresAt()) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Persisted expiry propagation lacks integration coverageThe Want Baz to fix this for you? Activate Fixer You can also update your AI coding guidelines based on this comment by Other fix methodsPrompt for AI Agents
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Commit 5759852 addressed this comment by adding a real MySQL-backed integration test that mints a token, calls
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added |
||
| .build()); | ||
| }); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,9 @@ | |
| import org.junit.jupiter.params.provider.NullSource; | ||
| import org.junit.jupiter.params.provider.ValueSource; | ||
|
|
||
| import java.time.Duration; | ||
| import java.time.Instant; | ||
| import java.time.temporal.ChronoUnit; | ||
| import java.util.Optional; | ||
| import java.util.UUID; | ||
|
|
||
|
|
@@ -94,6 +97,7 @@ void acceptsValidToken(String schemePrefix) { | |
| .workspaceId(UUID.randomUUID().toString()) | ||
| .workspaceName(RandomStringUtils.secure().nextAlphanumeric(10)) | ||
| .resource("http://localhost/api/v1/mcp/%s".formatted(RandomStringUtils.secure().nextAlphanumeric(8))) | ||
| .expiresAt(Instant.now().plus(Duration.ofHours(1)).truncatedTo(ChronoUnit.MILLIS)) | ||
| .build(); | ||
| when(mcpOAuthService.validateAccessToken(ACCESS_TOKEN)).thenReturn(Optional.of(validated)); | ||
|
|
||
|
|
@@ -102,4 +106,25 @@ void acceptsValidToken(String schemePrefix) { | |
| assertThat(response.readEntity(ValidatedToken.class)).isEqualTo(validated); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("reports the token expiry as an ISO-8601 expires_at so resource servers can cache until then") | ||
| void reportsExpiresAtOnTheWire() { | ||
| // opik-mcp caches a "valid" answer until this instant (OPIK-8252); an | ||
| // epoch number or a camelCase key would silently fall back to its TTL. | ||
| var expiresAt = Instant.parse("2026-09-04T10:15:30.123Z"); | ||
| var validated = ValidatedToken.builder() | ||
| .userName(RandomStringUtils.secure().nextAlphanumeric(10)) | ||
| .workspaceId(UUID.randomUUID().toString()) | ||
| .workspaceName(RandomStringUtils.secure().nextAlphanumeric(10)) | ||
| .resource("http://localhost/api/v1/mcp") | ||
| .expiresAt(expiresAt) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. New fixture bypasses Podam conventionThe new wire-format and modified valid-token fixtures manually populate Want Baz to fix this for you? Activate Fixer You can also update your AI coding guidelines based on this comment by Other fix methodsPrompt for AI Agents
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Commit 5759852 addressed this comment by using
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Switched both |
||
| .build(); | ||
| when(mcpOAuthService.validateAccessToken(ACCESS_TOKEN)).thenReturn(Optional.of(validated)); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Token expiry mapping lacks regression coverageThe new wire-format test mocks Want Baz to fix this for you? Activate Fixer Other fix methodsPrompt for AI Agents
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Commit 5759852 addressed this comment by adding an integration regression test that exercises the real OAuth flow, reads the persisted token expiry, and asserts
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Covered by the new |
||
|
|
||
| try (Response response = validate("Bearer " + ACCESS_TOKEN)) { | ||
| assertThat(response.getStatus()).isEqualTo(Response.Status.OK.getStatusCode()); | ||
| assertThat(response.readEntity(String.class)).contains("\"expires_at\":\"2026-09-04T10:15:30.123Z\""); | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generated clients omit token expiry
expiresAtis now emitted in the backend response, but the checked-in OpenAPIValidatedTokenschema isn't updated, so Fern-generated models lackexpires_atand typed SDK consumers can't access it — should we update the API definition and regenerate the SDK artifacts as part of this change?Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commit 5759852 addressed this comment by adding
expires_atto both checked-in OpenAPI schemas as an ISO date-time field. Generated SDK artifact changes are not present in the diff.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added
expires_at(string, date-time) toValidatedTokenin both checked-in OpenAPI specs in 5759852. SDK regeneration is left to the scheduled Fern workflow, as with previous DTO changes in this package.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, noted—the specs now include
expires_at, and SDK regeneration follows the scheduled Fern workflow. I’ll save this context to memory once the PR is merged.