-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: Implement OAuthClientsLimitExceeded dialog #900
Merged
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
00cab96
feat: Implement OAuthClientsLimitExceeded dialog
Ldoppea 208334d
feat: Handle OAuthClientsLimitExceeded outgoing navigation
Ldoppea f9a7443
feat: Stop waiting for cozy-app when opening OAuthClientsLimitExceeded
Ldoppea 34fe243
feat: Open redirect url from OAuthClientsLimitExceeded when in focus
Ldoppea 83eabbc
feat: Don't open redirect url if targeting cozy-home
Ldoppea cfa3891
feat: Provide redirect URL to OAuthClientsLimit WebView
Ldoppea 696ca1a
fix: Secure checkOauthClientsLimit
Ldoppea 1beaebc
feat: Use limitExceeded from stack's answer when checking OAuth limit
Ldoppea 1cf43f0
test: Add unit tests for OAuth clients limit
Ldoppea 6275c7a
fix: Prevent OAuthClientsLimitExceeded popup url to change when open
Ldoppea 04eff67
fix: Secure OauthClientsLimitService
Ldoppea bda093a
feat: Add loading overlay on OauthClientsLimitExceeded popup
Ldoppea d5219cb
feat: Prevent checking OauthClientsLimit if flag is not set on the Cozy
Ldoppea File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,264 @@ | ||
import { NavigationProp, ParamListBase } from '@react-navigation/native' | ||
import type { | ||
WebViewOpenWindowEvent, | ||
WebViewNavigation | ||
} from 'react-native-webview/lib/WebViewTypes' | ||
|
||
import CozyClient from 'cozy-client' | ||
|
||
import { | ||
interceptNavigation, | ||
interceptOpenWindow | ||
} from '/app/domain/limits/OauthClientsLimitService' | ||
import { routes } from '/constants/routes' | ||
import { navigateToApp } from '/libs/functions/openApp' | ||
import { navigationRef } from '/libs/RootNavigation' | ||
|
||
jest.mock('/libs/functions/openApp') | ||
|
||
const mockNavigationProp = {} as NavigationProp<ParamListBase> | ||
|
||
const mockClient = (): CozyClient => { | ||
return { | ||
getStackClient: (): { uri: string } => ({ | ||
uri: 'http://claude.mycozy.cloud' | ||
}), | ||
capabilities: { flat_subdomains: true } | ||
} as CozyClient | ||
} | ||
|
||
const mockWebViewNavigationRequest = (url: string): WebViewNavigation => { | ||
return { | ||
url: url | ||
} as WebViewNavigation | ||
} | ||
|
||
const mockOpenWindowRequest = (url: string): WebViewOpenWindowEvent => { | ||
return { | ||
nativeEvent: { | ||
targetUrl: url | ||
} | ||
} as WebViewOpenWindowEvent | ||
} | ||
|
||
const mockCurrentRouteName = (currentRouteName: string): void => { | ||
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment | ||
navigationRef.current = { | ||
getCurrentRoute: jest.fn().mockReturnValue({ name: currentRouteName }) | ||
// eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
} as any | ||
} | ||
|
||
describe('interceptNavigation', () => { | ||
it('should do nothing if client is null', () => { | ||
const initialUrl = 'SOME_URL' | ||
const destinationUrl = 'SOME_DESTINATION_URL' | ||
|
||
const client = null | ||
const closePopup = jest.fn() | ||
const navigationRequest = mockWebViewNavigationRequest(destinationUrl) | ||
|
||
const allowNavigation = interceptNavigation( | ||
initialUrl, | ||
closePopup, | ||
client, | ||
mockNavigationProp | ||
)(navigationRequest) | ||
|
||
expect(allowNavigation).toBe(false) | ||
expect(closePopup).not.toHaveBeenCalled() | ||
expect(navigateToApp).not.toHaveBeenCalled() | ||
}) | ||
|
||
it('should close the OAuthClientLimit popup when redirected to Home', () => { | ||
const initialUrl = | ||
'http://claude.mycozy.cloud/settings/clients/limit-exceeded?redirect=http%3A%2F%2Fclaude-home.mycozy.cloud%2F' | ||
const destinationUrl = 'http://claude-home.mycozy.cloud/' | ||
|
||
const client = mockClient() | ||
const closePopup = jest.fn() | ||
const navigationRequest = mockWebViewNavigationRequest(destinationUrl) | ||
|
||
mockCurrentRouteName(routes.default) | ||
|
||
const allowNavigation = interceptNavigation( | ||
initialUrl, | ||
closePopup, | ||
client, | ||
mockNavigationProp | ||
)(navigationRequest) | ||
|
||
expect(allowNavigation).toBe(false) | ||
expect(closePopup).toHaveBeenCalled() | ||
expect(navigateToApp).not.toHaveBeenCalled() | ||
}) | ||
|
||
it('should close the OAuthClientLimit popup and navigate to drive when redirected to a cozy-drive url', () => { | ||
const initialUrl = | ||
'http://claude.mycozy.cloud/settings/clients/limit-exceeded?redirect=http%3A%2F%2Fclaude-drive.mycozy.cloud%2F' | ||
const destinationUrl = 'http://claude-drive.mycozy.cloud/' | ||
|
||
const client = mockClient() | ||
const closePopup = jest.fn() | ||
const navigationRequest = mockWebViewNavigationRequest(destinationUrl) | ||
|
||
mockCurrentRouteName(routes.default) | ||
|
||
const allowNavigation = interceptNavigation( | ||
initialUrl, | ||
closePopup, | ||
client, | ||
mockNavigationProp | ||
)(navigationRequest) | ||
|
||
expect(allowNavigation).toBe(false) | ||
expect(closePopup).toHaveBeenCalled() | ||
expect(navigateToApp).toHaveBeenCalledWith({ | ||
href: 'http://claude-drive.mycozy.cloud/', | ||
slug: 'drive', | ||
navigation: mockNavigationProp | ||
}) | ||
}) | ||
|
||
it('should close the OAuthClientLimit popup but not navigate to drive when redirected to a cozy-drive url while in cozy-settings', () => { | ||
const initialUrl = | ||
'http://claude.mycozy.cloud/settings/clients/limit-exceeded?redirect=http%3A%2F%2Fclaude-drive.mycozy.cloud%2F' | ||
const destinationUrl = 'http://claude-drive.mycozy.cloud/' | ||
|
||
const client = mockClient() | ||
const closePopup = jest.fn() | ||
const navigationRequest = mockWebViewNavigationRequest(destinationUrl) | ||
|
||
mockCurrentRouteName(routes.cozyapp) | ||
|
||
const allowNavigation = interceptNavigation( | ||
initialUrl, | ||
closePopup, | ||
client, | ||
mockNavigationProp | ||
)(navigationRequest) | ||
|
||
expect(allowNavigation).toBe(false) | ||
expect(closePopup).toHaveBeenCalled() | ||
expect(navigateToApp).not.toHaveBeenCalled() | ||
}) | ||
|
||
it('should allow navigation in case of refresh', () => { | ||
const initialUrl = | ||
'http://claude.mycozy.cloud/settings/clients/limit-exceeded?redirect=http%3A%2F%2Fclaude-drive.mycozy.cloud%2F' | ||
const destinationUrl = initialUrl | ||
|
||
const client = mockClient() | ||
const closePopup = jest.fn() | ||
const navigationRequest = mockWebViewNavigationRequest(destinationUrl) | ||
|
||
mockCurrentRouteName(routes.default) | ||
|
||
const allowNavigation = interceptNavigation( | ||
initialUrl, | ||
closePopup, | ||
client, | ||
mockNavigationProp | ||
)(navigationRequest) | ||
|
||
expect(allowNavigation).toBe(true) | ||
expect(closePopup).not.toHaveBeenCalled() | ||
expect(navigateToApp).not.toHaveBeenCalled() | ||
}) | ||
|
||
it('should allow navigation in case of refresh (resilient to trailing hash)', () => { | ||
const initialUrl = | ||
'http://claude.mycozy.cloud/settings/clients/limit-exceeded?redirect=http%3A%2F%2Fclaude-drive.mycozy.cloud%2F' | ||
const destinationUrl = `${initialUrl}#` | ||
|
||
const client = mockClient() | ||
const closePopup = jest.fn() | ||
const navigationRequest = mockWebViewNavigationRequest(destinationUrl) | ||
|
||
mockCurrentRouteName(routes.default) | ||
|
||
const allowNavigation = interceptNavigation( | ||
initialUrl, | ||
closePopup, | ||
client, | ||
mockNavigationProp | ||
)(navigationRequest) | ||
|
||
expect(allowNavigation).toBe(true) | ||
expect(closePopup).not.toHaveBeenCalled() | ||
expect(navigateToApp).not.toHaveBeenCalled() | ||
}) | ||
|
||
it('should be error safe and interrupt navigation if any', () => { | ||
const initialUrl = | ||
'http://claude.mycozy.cloud/settings/clients/limit-exceeded?redirect=http%3A%2F%2Fclaude-drive.mycozy.cloud%2F' | ||
const destinationUrl = 'http://claude-drive.mycozy.cloud/' | ||
|
||
const client = mockClient() | ||
const closePopup = jest.fn() | ||
const navigationRequest = mockWebViewNavigationRequest(destinationUrl) | ||
|
||
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment | ||
navigationRef.current = { | ||
getCurrentRoute: jest.fn().mockImplementation(() => { | ||
throw new Error('SOME_ERROR') | ||
}) | ||
// eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
} as any | ||
|
||
const allowNavigation = interceptNavigation( | ||
initialUrl, | ||
closePopup, | ||
client, | ||
mockNavigationProp | ||
)(navigationRequest) | ||
|
||
expect(allowNavigation).toBe(false) | ||
expect(closePopup).not.toHaveBeenCalled() | ||
expect(navigateToApp).not.toHaveBeenCalled() | ||
}) | ||
}) | ||
|
||
describe('interceptOpenWindow', () => { | ||
it('should do nothing if client is null', () => { | ||
const targetUrl = 'SOME_DESTINATION_URL' | ||
|
||
const client = null | ||
const openWindowRequest = mockOpenWindowRequest(targetUrl) | ||
|
||
interceptOpenWindow(client, mockNavigationProp)(openWindowRequest) | ||
|
||
expect(navigateToApp).not.toHaveBeenCalled() | ||
}) | ||
|
||
it('should open cozy-settings when called with corresponding url', () => { | ||
const targetUrl = 'http://claude-settings.mycozy.cloud/#/connectedDevices' | ||
|
||
const client = mockClient() | ||
const openWindowRequest = mockOpenWindowRequest(targetUrl) | ||
|
||
interceptOpenWindow(client, mockNavigationProp)(openWindowRequest) | ||
|
||
expect(navigateToApp).toHaveBeenCalledWith({ | ||
href: 'http://claude-settings.mycozy.cloud/#/connectedDevices', | ||
slug: 'settings', | ||
navigation: mockNavigationProp | ||
}) | ||
}) | ||
|
||
it('should be error safe', () => { | ||
const targetUrl = 'http://claude-settings.mycozy.cloud/#/connectedDevices' | ||
|
||
const client = mockClient() | ||
const openWindowRequest = mockOpenWindowRequest(targetUrl) | ||
|
||
// eslint-disable-next-line @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any | ||
;(navigateToApp as any).mockImplementation(() => { | ||
throw new Error('SOME_ERROR') | ||
}) | ||
|
||
interceptOpenWindow(client, mockNavigationProp)(openWindowRequest) | ||
|
||
expect(true).toBe(true) | ||
}) | ||
}) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,133 @@ | ||
import { EventEmitter } from 'events' | ||
|
||
import { NavigationProp, ParamListBase } from '@react-navigation/native' | ||
import type { | ||
WebViewOpenWindowEvent, | ||
WebViewNavigation | ||
} from 'react-native-webview/lib/WebViewTypes' | ||
|
||
import type CozyClient from 'cozy-client' | ||
import { deconstructCozyWebLinkWithSlug } from 'cozy-client' | ||
import Minilog from 'cozy-minilog' | ||
|
||
import { routes } from '/constants/routes' | ||
import { navigateToApp } from '/libs/functions/openApp' | ||
import { getErrorMessage } from '/libs/functions/getErrorMessage' | ||
import { navigate, navigationRef } from '/libs/RootNavigation' | ||
|
||
const log = Minilog('⛔ OAuth Clients Limit Service') | ||
|
||
export const OAUTH_CLIENTS_LIMIT_EXCEEDED = 'OAUTH_CLIENTS_LIMIT_EXCEEDED' | ||
|
||
export const oauthClientLimitEventHandler = new EventEmitter() | ||
|
||
export const showOauthClientsLimitExceeded = (href: string): void => { | ||
navigate('home') | ||
oauthClientLimitEventHandler.emit(OAUTH_CLIENTS_LIMIT_EXCEEDED, href) | ||
} | ||
|
||
export const interceptNavigation = | ||
( | ||
initialUrl: string, | ||
closePopup: () => void, | ||
client: CozyClient | null, | ||
navigation: NavigationProp<ParamListBase> | ||
) => | ||
(request: WebViewNavigation): boolean => { | ||
try { | ||
if (client === null) { | ||
log.error('Client is null, should not happen') | ||
return false | ||
} | ||
|
||
const destinationUrl = cleanUrl(request.url) | ||
const subdomainType = client.capabilities.flat_subdomains | ||
? 'flat' | ||
: 'nested' | ||
|
||
if (destinationUrl === initialUrl) { | ||
return true | ||
} | ||
|
||
const destinationUrlData = deconstructCozyWebLinkWithSlug( | ||
destinationUrl, | ||
subdomainType | ||
) | ||
|
||
if (!destinationUrlData.slug) { | ||
return false | ||
} | ||
|
||
const currentRouteName = | ||
navigationRef.current?.getCurrentRoute()?.name ?? '' | ||
|
||
if (destinationUrlData.slug === 'home') { | ||
log.debug( | ||
`Destination URL is Home which should already be rendered in background, only close popup to reveal the HomeView` | ||
) | ||
closePopup() | ||
} else if (currentRouteName !== routes.default) { | ||
log.debug( | ||
`Current route is not Home but ${currentRouteName}, only close popup and don't interrupt user` | ||
) | ||
closePopup() | ||
} else { | ||
log.debug( | ||
`Current route is Home, close popup and navigate to ${destinationUrl}` | ||
) | ||
closePopup() | ||
void navigateToApp({ | ||
navigation, | ||
href: destinationUrl, | ||
slug: destinationUrlData.slug | ||
}) | ||
} | ||
|
||
return false | ||
} catch (error) { | ||
const errorMessage = getErrorMessage(error) | ||
log.error( | ||
`Error while analysing WebView navigation. Intercept it anyway to prevent unexpected behavior: ${errorMessage}` | ||
) | ||
|
||
return false | ||
} | ||
} | ||
|
||
export const interceptOpenWindow = | ||
(client: CozyClient | null, navigation: NavigationProp<ParamListBase>) => | ||
(syntheticEvent: WebViewOpenWindowEvent): void => { | ||
try { | ||
if (client === null) { | ||
log.error('Client is null, should not happen') | ||
return | ||
} | ||
|
||
const { nativeEvent } = syntheticEvent | ||
const destinationUrl = nativeEvent.targetUrl | ||
|
||
const subdomainType = client.capabilities.flat_subdomains | ||
? 'flat' | ||
: 'nested' | ||
|
||
const destinationUrlData = deconstructCozyWebLinkWithSlug( | ||
destinationUrl, | ||
subdomainType | ||
) | ||
|
||
if (destinationUrlData.slug) { | ||
void navigateToApp({ | ||
navigation, | ||
href: destinationUrl, | ||
slug: destinationUrlData.slug | ||
}) | ||
} | ||
} catch (error) { | ||
const errorMessage = getErrorMessage(error) | ||
log.error(`Error while intercepting WebView openWindow: ${errorMessage}`) | ||
} | ||
} | ||
|
||
const cleanUrl = (url: string): string => { | ||
return url.endsWith('#') ? url.replace('#', '') : url | ||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I've thought about this recently and I've come to the conclusion a lot of issues we have with navigation come from react-decoupled calls, that don't fit well with the view lifecycles. I started to avoid as much as possible calling them from outside components in the sharing feature. What do you think?
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.
I tried to use the
navigation
param only, but the problem is thatnavigation.getCurrentRoute()
will always return thehome
route here, even if cozy-settings is on focus. Seems like thenavigation
prop's context is bind to where the component is displayed and not where the app is.When using
navigationRef.getCurrentRoute()
, then I correctly get thecozyapp
route when cozy-settings is displayed.