-
Notifications
You must be signed in to change notification settings - Fork 453
feat(js): add clerk.oauthApplication.getConsentInfo #8275
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
Changes from 2 commits
0c712f0
46c893c
f54b79d
8fc1f9d
8d6e852
fdf2b9f
3372795
e570b28
f6785c8
d623ef1
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 |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@clerk/clerk-js': patch | ||
| --- | ||
|
|
||
| Add OAuthApplication resource and fetchConsentInfo method | ||
|
jfoshee marked this conversation as resolved.
Outdated
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| import { ClerkRuntimeError } from '@clerk/shared/error'; | ||
| import type { | ||
| ClerkResourceJSON, | ||
| FetchOAuthConsentInfoParams, | ||
| OAuthConsentInfo, | ||
| OAuthConsentInfoJSON, | ||
| } from '@clerk/shared/types'; | ||
|
|
||
| import { BaseResource } from './internal'; | ||
|
|
||
| export class OAuthApplication extends BaseResource { | ||
| pathRoot = ''; | ||
|
|
||
| protected fromJSON(_data: ClerkResourceJSON | null): this { | ||
| return this; | ||
| } | ||
|
|
||
| static async fetchConsentInfo(params: FetchOAuthConsentInfoParams): Promise<OAuthConsentInfo> { | ||
|
Member
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. Small nit: we don't use a fetch* prefix elsewhere in resources (closest precedents for non-standard verbs are Not blocking, just wondering if
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. Oh cool, good catch. Looks like |
||
| const { oauthClientId, scope } = params; | ||
| const json = await BaseResource._fetch<OAuthConsentInfoJSON>( | ||
| { | ||
| method: 'GET', | ||
| path: `/me/oauth/consent/${encodeURIComponent(oauthClientId)}`, | ||
| search: scope !== undefined ? { scope } : undefined, | ||
| }, | ||
| { skipUpdateClient: true }, | ||
| ); | ||
|
|
||
| if (!json) { | ||
| throw new ClerkRuntimeError('Network request failed while offline', { code: 'network_error' }); | ||
| } | ||
|
|
||
| const envelope = json; | ||
| const data = envelope.response ?? json; | ||
|
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. Why do we coalesce here?
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. Seems to be the common pattern that callers to |
||
| return { | ||
| oauth_application_name: data.oauth_application_name, | ||
|
Member
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. I think this is deviating from the standard convention of having camelCase property being returned for SDK consumers
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. ah yes, thanks @jacekradko |
||
| oauth_application_logo_url: data.oauth_application_logo_url, | ||
| oauth_application_url: data.oauth_application_url, | ||
| client_id: data.client_id, | ||
| state: data.state, | ||
| scopes: data.scopes ?? [], | ||
| }; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,157 @@ | ||
| import { ClerkAPIResponseError } from '@clerk/shared/error'; | ||
| import type { InstanceType, OAuthConsentInfoJSON } from '@clerk/shared/types'; | ||
| import { afterEach, describe, expect, it, type Mock, vi } from 'vitest'; | ||
|
|
||
| import { mockFetch } from '@/test/core-fixtures'; | ||
|
|
||
| import { SUPPORTED_FAPI_VERSION } from '../../constants'; | ||
| import { createFapiClient } from '../../fapiClient'; | ||
| import { BaseResource } from '../internal'; | ||
| import { OAuthApplication } from '../OAuthApplication'; | ||
|
|
||
| const consentPayload: OAuthConsentInfoJSON = { | ||
| object: 'oauth_consent_info', | ||
| id: 'client_abc', | ||
| oauth_application_name: 'My App', | ||
| oauth_application_logo_url: 'https://img.example/logo.png', | ||
| oauth_application_url: 'https://app.example', | ||
| client_id: 'client_abc', | ||
| state: 'st', | ||
| scopes: [{ scope: 'openid', description: 'OpenID', requires_consent: true }], | ||
| }; | ||
|
|
||
| describe('OAuthApplication.fetchConsentInfo', () => { | ||
| afterEach(() => { | ||
| (global.fetch as Mock)?.mockClear?.(); | ||
| BaseResource.clerk = null as any; | ||
| vi.restoreAllMocks(); | ||
| }); | ||
|
|
||
| it('throws ClerkRuntimeError when there is no active session', async () => { | ||
| const fetchSpy = vi.spyOn(BaseResource, '_fetch'); | ||
|
|
||
| BaseResource.clerk = { | ||
| session: undefined, | ||
| } as any; | ||
|
|
||
| await expect(OAuthApplication.fetchConsentInfo({ oauthClientId: 'cid' })).rejects.toMatchObject({ | ||
| code: 'cannot_fetch_oauth_consent_no_session', | ||
| }); | ||
| expect(fetchSpy).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('calls BaseResource._fetch with GET, encoded path, sessionId, optional scope, and skipUpdateClient', async () => { | ||
| const fetchSpy = vi.spyOn(BaseResource, '_fetch').mockResolvedValue({ | ||
| response: consentPayload, | ||
| } as any); | ||
|
|
||
| BaseResource.clerk = { | ||
| session: { id: 'sess_test' }, | ||
| } as any; | ||
|
|
||
| await OAuthApplication.fetchConsentInfo({ oauthClientId: 'my/client id', scope: 'openid email' }); | ||
|
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. I thought this was an odd client id but I think it's actually good to verify that we url escape this to avoid path traversal issues. |
||
|
|
||
| expect(fetchSpy).toHaveBeenCalledWith( | ||
| { | ||
| method: 'GET', | ||
| path: '/me/oauth/consent/my%2Fclient%20id', | ||
| search: { scope: 'openid email' }, | ||
| sessionId: 'sess_test', | ||
| }, | ||
| { skipUpdateClient: true }, | ||
| ); | ||
| }); | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| it('omits search when scope is undefined', async () => { | ||
| const fetchSpy = vi.spyOn(BaseResource, '_fetch').mockResolvedValue({ | ||
| response: consentPayload, | ||
| } as any); | ||
|
|
||
| BaseResource.clerk = { | ||
| session: { id: 'sess_test' }, | ||
| } as any; | ||
|
|
||
| await OAuthApplication.fetchConsentInfo({ oauthClientId: 'cid' }); | ||
|
|
||
| expect(fetchSpy).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| search: undefined, | ||
| }), | ||
| { skipUpdateClient: true }, | ||
| ); | ||
| }); | ||
|
|
||
| it('returns OAuthConsentInfo from the FAPI response envelope', async () => { | ||
| vi.spyOn(BaseResource, '_fetch').mockResolvedValue({ | ||
| response: consentPayload, | ||
| } as any); | ||
|
|
||
| BaseResource.clerk = { | ||
| session: { id: 'sess_test' }, | ||
| } as any; | ||
|
|
||
| const info = await OAuthApplication.fetchConsentInfo({ oauthClientId: 'client_abc' }); | ||
|
|
||
| expect(info).toEqual({ | ||
| oauth_application_name: 'My App', | ||
| oauth_application_logo_url: 'https://img.example/logo.png', | ||
| oauth_application_url: 'https://app.example', | ||
| client_id: 'client_abc', | ||
| state: 'st', | ||
| scopes: [{ scope: 'openid', description: 'OpenID', requires_consent: true }], | ||
| }); | ||
| }); | ||
|
|
||
| it('defaults scopes to an empty array when absent', async () => { | ||
| vi.spyOn(BaseResource, '_fetch').mockResolvedValue({ | ||
| response: { ...consentPayload, scopes: undefined }, | ||
| } as any); | ||
|
|
||
| BaseResource.clerk = { | ||
| session: { id: 'sess_test' }, | ||
| } as any; | ||
|
|
||
| const info = await OAuthApplication.fetchConsentInfo({ oauthClientId: 'client_abc' }); | ||
| expect(info.scopes).toEqual([]); | ||
| }); | ||
|
|
||
| it('maps ClerkAPIResponseError from FAPI on non-2xx', async () => { | ||
| mockFetch(false, 422, { | ||
| errors: [{ code: 'oauth_consent_error', long_message: 'Consent metadata unavailable' }], | ||
| }); | ||
|
|
||
| BaseResource.clerk = { | ||
| session: { id: 'sess_1' }, | ||
| getFapiClient: () => | ||
| createFapiClient({ | ||
| frontendApi: 'clerk.example.com', | ||
| getSessionId: () => 'sess_1', | ||
| instanceType: 'development' as InstanceType, | ||
| }), | ||
| __internal_setCountry: vi.fn(), | ||
| handleUnauthenticated: vi.fn(), | ||
| __internal_handleUnauthenticatedDevBrowser: vi.fn(), | ||
| } as any; | ||
|
|
||
| await expect(OAuthApplication.fetchConsentInfo({ oauthClientId: 'cid' })).rejects.toSatisfy( | ||
| (err: unknown) => err instanceof ClerkAPIResponseError && err.message === 'Consent metadata unavailable', | ||
| ); | ||
|
|
||
| expect(global.fetch).toHaveBeenCalledTimes(1); | ||
| const [url] = (global.fetch as Mock).mock.calls[0]; | ||
| expect(url.toString()).toContain(`/v1/me/oauth/consent/cid`); | ||
| expect(url.toString()).toContain(`__clerk_api_version=${SUPPORTED_FAPI_VERSION}`); | ||
| }); | ||
|
|
||
| it('throws ClerkRuntimeError when _fetch returns null (offline)', async () => { | ||
| vi.spyOn(BaseResource, '_fetch').mockResolvedValue(null); | ||
|
|
||
| BaseResource.clerk = { | ||
| session: { id: 'sess_test' }, | ||
| } as any; | ||
|
|
||
| await expect(OAuthApplication.fetchConsentInfo({ oauthClientId: 'cid' })).rejects.toMatchObject({ | ||
| code: 'network_error', | ||
| }); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| import type { ClerkResourceJSON } from './json'; | ||
|
|
||
| /** | ||
| * A single OAuth scope row returned by the Frontend API consent metadata endpoint. | ||
| */ | ||
| export type OAuthConsentScopeJSON = { | ||
| scope: string; | ||
| description: string | null; | ||
| requires_consent: boolean; | ||
| }; | ||
|
|
||
| /** | ||
| * OAuth consent screen metadata from `GET /v1/me/oauth/consent/{oauthClientId}`. | ||
| * Field names match the Frontend API JSON (snake_case). | ||
| */ | ||
| export type OAuthConsentInfo = { | ||
| oauth_application_name: string; | ||
| oauth_application_logo_url: string; | ||
| oauth_application_url: string; | ||
| client_id: string; | ||
| state: string; | ||
| scopes: OAuthConsentScopeJSON[]; | ||
| }; | ||
|
|
||
| /** | ||
| * @internal | ||
| */ | ||
| export interface OAuthConsentInfoJSON extends ClerkResourceJSON { | ||
| object: 'oauth_consent_info'; | ||
| oauth_application_name: string; | ||
| oauth_application_logo_url: string; | ||
| oauth_application_url: string; | ||
| client_id: string; | ||
| state: string; | ||
| scopes: OAuthConsentScopeJSON[]; | ||
| } | ||
|
|
||
| export type FetchOAuthConsentInfoParams = { | ||
| /** OAuth `client_id` from the authorize request. */ | ||
| oauthClientId: string; | ||
| /** Optional normalized scope string (e.g. space-delimited). */ | ||
|
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. "e.g." means "for example". Is space-delimited just an example of a scope string or is this something we should enforce?
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. 😆 good catch |
||
| scope?: string; | ||
| }; | ||
|
|
||
| /** | ||
| * Namespace exposed on `Clerk` for OAuth application / consent helpers. | ||
| */ | ||
| export interface OAuthApplicationNamespace { | ||
| /** | ||
| * Loads consent metadata for the given OAuth client for the signed-in user. | ||
| * Uses the Frontend API session (cookies, `_clerk_session_id`, dev browser, etc.) like other `/me` requests. | ||
| */ | ||
| fetchConsentInfo: (params: FetchOAuthConsentInfoParams) => Promise<OAuthConsentInfo>; | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.