diff --git a/composer.json b/composer.json index 1f1c8ad0c..824dfe707 100644 --- a/composer.json +++ b/composer.json @@ -8,7 +8,8 @@ "helmich/phpunit-json-assert": "^3.4", "vimeo/psalm": "5.23.1", "guzzlehttp/guzzle": "^7.9", - "behat/gherkin": "v4.9.0" + "behat/gherkin": "v4.9.0", + "php-mock/php-mock-phpunit": "^2.10" }, "scripts": { "cs:fix": "php-cs-fixer fix", diff --git a/composer.lock b/composer.lock index 5aa70c328..05696a9de 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "cfc2220dcb88b41bd62c394bf8d8e00b", + "content-hash": "011878127c7a8342d89525c1ca85fc9f", "packages": [], "packages-dev": [ { @@ -2099,6 +2099,213 @@ }, "time": "2022-02-21T01:04:05+00:00" }, + { + "name": "php-mock/php-mock", + "version": "2.5.1", + "source": { + "type": "git", + "url": "https://github.com/php-mock/php-mock.git", + "reference": "8f58972dce4de5a804dc0459383a11bc651416cf" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/php-mock/php-mock/zipball/8f58972dce4de5a804dc0459383a11bc651416cf", + "reference": "8f58972dce4de5a804dc0459383a11bc651416cf", + "shasum": "" + }, + "require": { + "php": "^5.6 || ^7.0 || ^8.0", + "phpunit/php-text-template": "^1 || ^2 || ^3 || ^4" + }, + "replace": { + "malkusch/php-mock": "*" + }, + "require-dev": { + "phpunit/phpunit": "^5.7 || ^6.5 || ^7.5 || ^8.0 || ^9.0 || ^10.0 || ^11.0", + "squizlabs/php_codesniffer": "^3.8" + }, + "suggest": { + "php-mock/php-mock-phpunit": "Allows integration into PHPUnit testcase with the trait PHPMock." + }, + "type": "library", + "autoload": { + "files": [ + "autoload.php" + ], + "psr-4": { + "phpmock\\": [ + "classes/", + "tests/" + ] + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "WTFPL" + ], + "authors": [ + { + "name": "Markus Malkusch", + "email": "markus@malkusch.de", + "homepage": "http://markus.malkusch.de", + "role": "Developer" + } + ], + "description": "PHP-Mock can mock built-in PHP functions (e.g. time()). PHP-Mock relies on PHP's namespace fallback policy. No further extension is needed.", + "homepage": "https://github.com/php-mock/php-mock", + "keywords": [ + "BDD", + "TDD", + "function", + "mock", + "stub", + "test", + "test double", + "testing" + ], + "support": { + "issues": "https://github.com/php-mock/php-mock/issues", + "source": "https://github.com/php-mock/php-mock/tree/2.5.1" + }, + "funding": [ + { + "url": "https://github.com/michalbundyra", + "type": "github" + } + ], + "time": "2024-12-07T20:52:37+00:00" + }, + { + "name": "php-mock/php-mock-integration", + "version": "2.3.0", + "source": { + "type": "git", + "url": "https://github.com/php-mock/php-mock-integration.git", + "reference": "ec6a00a8129d50ed0f07907c91e3274ca4ade877" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/php-mock/php-mock-integration/zipball/ec6a00a8129d50ed0f07907c91e3274ca4ade877", + "reference": "ec6a00a8129d50ed0f07907c91e3274ca4ade877", + "shasum": "" + }, + "require": { + "php": ">=5.6", + "php-mock/php-mock": "^2.5", + "phpunit/php-text-template": "^1 || ^2 || ^3 || ^4" + }, + "require-dev": { + "phpunit/phpunit": "^5.7.27 || ^6 || ^7 || ^8 || ^9 || ^10 || ^11" + }, + "type": "library", + "autoload": { + "psr-4": { + "phpmock\\integration\\": "classes/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "WTFPL" + ], + "authors": [ + { + "name": "Markus Malkusch", + "email": "markus@malkusch.de", + "homepage": "http://markus.malkusch.de", + "role": "Developer" + } + ], + "description": "Integration package for PHP-Mock", + "homepage": "https://github.com/php-mock/php-mock-integration", + "keywords": [ + "BDD", + "TDD", + "function", + "mock", + "stub", + "test", + "test double" + ], + "support": { + "issues": "https://github.com/php-mock/php-mock-integration/issues", + "source": "https://github.com/php-mock/php-mock-integration/tree/2.3.0" + }, + "funding": [ + { + "url": "https://github.com/michalbundyra", + "type": "github" + } + ], + "time": "2024-02-10T21:37:25+00:00" + }, + { + "name": "php-mock/php-mock-phpunit", + "version": "2.10.0", + "source": { + "type": "git", + "url": "https://github.com/php-mock/php-mock-phpunit.git", + "reference": "e1f7e795990b00937376e345883ea68ca3bda7e0" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/php-mock/php-mock-phpunit/zipball/e1f7e795990b00937376e345883ea68ca3bda7e0", + "reference": "e1f7e795990b00937376e345883ea68ca3bda7e0", + "shasum": "" + }, + "require": { + "php": ">=7", + "php-mock/php-mock-integration": "^2.3", + "phpunit/phpunit": "^6 || ^7 || ^8 || ^9 || ^10.0.17 || ^11" + }, + "require-dev": { + "mockery/mockery": "^1.3.6" + }, + "type": "library", + "autoload": { + "files": [ + "autoload.php" + ], + "psr-4": { + "phpmock\\phpunit\\": "classes/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "WTFPL" + ], + "authors": [ + { + "name": "Markus Malkusch", + "email": "markus@malkusch.de", + "homepage": "http://markus.malkusch.de", + "role": "Developer" + } + ], + "description": "Mock built-in PHP functions (e.g. time()) with PHPUnit. This package relies on PHP's namespace fallback policy. No further extension is needed.", + "homepage": "https://github.com/php-mock/php-mock-phpunit", + "keywords": [ + "BDD", + "TDD", + "function", + "mock", + "phpunit", + "stub", + "test", + "test double", + "testing" + ], + "support": { + "issues": "https://github.com/php-mock/php-mock-phpunit/issues", + "source": "https://github.com/php-mock/php-mock-phpunit/tree/2.10.0" + }, + "funding": [ + { + "url": "https://github.com/michalbundyra", + "type": "github" + } + ], + "time": "2024-02-11T07:24:16+00:00" + }, { "name": "phpdocumentor/reflection-common", "version": "2.2.0", @@ -6715,5 +6922,5 @@ "prefer-lowest": false, "platform": [], "platform-dev": [], - "plugin-api-version": "2.6.0" + "plugin-api-version": "2.3.0" } diff --git a/lib/Service/OpenProjectAPIService.php b/lib/Service/OpenProjectAPIService.php index 8fc803f54..e4fe18e23 100644 --- a/lib/Service/OpenProjectAPIService.php +++ b/lib/Service/OpenProjectAPIService.php @@ -35,6 +35,7 @@ use OCA\TermsOfService\Db\Mapper\TermsMapper; use OCA\UserOIDC\Db\ProviderMapper; use OCA\UserOIDC\Exception\TokenExchangeFailedException; +use OCA\UserOIDC\User\Backend as OIDCBackend; use OCP\App\IAppManager; use OCP\AppFramework\Http; use OCP\Encryption\IManager; @@ -55,7 +56,9 @@ use OCP\IGroupManager; use OCP\IL10N; use OCP\IURLGenerator; +use OCP\IUser; use OCP\IUserManager; +use OCP\IUserSession; use OCP\Log\ILogFactory; use OCP\PreConditionNotMetException; use OCP\Security\ISecureRandom; @@ -128,6 +131,11 @@ class OpenProjectAPIService { private ILogFactory $logFactory; private IManager $encryptionManager; + /** + * @var IUserSession + */ + private $userSession; + /** * Service to make requests to OpenProject v3 (JSON) API @@ -161,6 +169,7 @@ public function __construct( ILogFactory $logFactory, IManager $encryptionManager, ExchangedTokenRequestedEventHelper $exchangedTokenRequestedEventHelper, + IUserSession $userSession, ) { $this->appName = $appName; $this->avatarManager = $avatarManager; @@ -182,6 +191,7 @@ public function __construct( $this->logFactory = $logFactory; $this->encryptionManager = $encryptionManager; $this->exchangedTokenRequestedEventHelper = $exchangedTokenRequestedEventHelper; + $this->userSession = $userSession; } /** @@ -1647,7 +1657,6 @@ public function getOIDCToken(): ?string { return $token->getAccessToken(); } - /** * @param string $userId * @return void @@ -1682,4 +1691,19 @@ class_exists('\OCA\UserOIDC\Exception\TokenExchangeFailedException') && ) ); } + + /** + * @return bool + */ + public function isOIDCUser(): bool { + if (!class_exists(OIDCBackend::class)) { + return false; + } + + $user = $this->userSession->getUser(); + if ($user instanceof IUser && $user->getBackend() instanceof OIDCBackend) { + return true; + } + return false; + } } diff --git a/lib/Settings/Personal.php b/lib/Settings/Personal.php index 5835cdfa3..636483b17 100644 --- a/lib/Settings/Personal.php +++ b/lib/Settings/Personal.php @@ -6,7 +6,6 @@ use OCA\OpenProject\Service\OpenProjectAPIService; use OCP\AppFramework\Http\TemplateResponse; use OCP\AppFramework\Services\IInitialState; - use OCP\IConfig; use OCP\Settings\ISettings; @@ -35,7 +34,8 @@ public function __construct( IConfig $config, IInitialState $initialStateService, OpenProjectAPIService $openProjectAPIService, - ?string $userId) { + ?string $userId, + ) { $this->config = $config; $this->initialStateService = $initialStateService; $this->userId = $userId; @@ -78,10 +78,10 @@ public function getForm(): TemplateResponse { 'search_enabled' => ($searchEnabled === '1'), 'navigation_enabled' => ($navigationEnabled === '1'), 'user_name' => $userName, + 'admin_config_ok' => OpenProjectAPIService::isAdminConfigOk($this->config), + 'authorization_method' => $authorizationMethod, + 'oidc_user' => $this->openProjectAPIService->isOIDCUser(), ]; - - $userConfig['admin_config_ok'] = OpenProjectAPIService::isAdminConfigOk($this->config); - $userConfig['authorization_method'] = $authorizationMethod; $this->initialStateService->provideInitialState('user-config', $userConfig); $oauthConnectionResult = $this->config->getUserValue( @@ -103,7 +103,6 @@ public function getForm(): TemplateResponse { 'oauth-connection-error-message', $oauthConnectionErrorMessage ); - return new TemplateResponse(Application::APP_ID, 'personalSettings'); } diff --git a/psalm.xml b/psalm.xml index d188b313c..7a7a0cb81 100644 --- a/psalm.xml +++ b/psalm.xml @@ -36,6 +36,7 @@ + @@ -91,6 +92,7 @@ + diff --git a/src/components/ErrorLabel.vue b/src/components/ErrorLabel.vue new file mode 100644 index 000000000..df421a417 --- /dev/null +++ b/src/components/ErrorLabel.vue @@ -0,0 +1,23 @@ + + + + + diff --git a/src/components/PersonalSettings.vue b/src/components/PersonalSettings.vue index f946805f3..6306c646e 100644 --- a/src/components/PersonalSettings.vue +++ b/src/components/PersonalSettings.vue @@ -1,44 +1,47 @@ @@ -57,12 +60,20 @@ import { translate as t } from '@nextcloud/l10n' import { checkOauthConnectionResult, USER_SETTINGS, AUTH_METHOD } from '../utils.js' import { NcButton } from '@nextcloud/vue' import { error as errorMessages } from '../constants/messages.js' +import ErrorLabel from './ErrorLabel.vue' export default { name: 'PersonalSettings', components: { - SettingsTitle, OAuthConnectButton, NcButton, CloseIcon, CheckIcon, InformationVariant, CheckBox, + SettingsTitle, + OAuthConnectButton, + NcButton, + CloseIcon, + CheckIcon, + InformationVariant, + CheckBox, + ErrorLabel, }, data() { @@ -72,24 +83,25 @@ export default { oauthConnectionErrorMessage: loadState('integration_openproject', 'oauth-connection-error-message'), oauthConnectionResult: loadState('integration_openproject', 'oauth-connection-result'), userSettingDescription: USER_SETTINGS, - authMethods: AUTH_METHOD, errorMessages, } }, computed: { connected() { if (!this.state.admin_config_ok) return false - return this.state.token && this.state.token !== '' - && this.state.user_name && this.state.user_name !== '' + return !!this.state.token && !!this.state.user_name + }, + isOIDCAuthMethod() { + return this.state.authorization_method === AUTH_METHOD.OIDC }, - isNonOidcUserConnectedViaOidc() { - return !!(this.state.authorization_method === AUTH_METHOD.OIDC && this.state.admin_config_ok && !this.state.token) + isOauthAuthMethod() { + return this.state.authorization_method === AUTH_METHOD.OAUTH2 }, - showOAuthConnectButton() { - if (this.connected) { - return false + showConnectionSettings() { + if (this.isOIDCAuthMethod) { + return this.connected && this.state.oidc_user } - return !(this.state.admin_config_ok === true && this.state.authorization_method === this.authMethods.OIDC) + return this.connected }, }, watch: { @@ -106,7 +118,7 @@ export default { }, mounted() { - if (this.state.authorization_method === this.authMethods.OAUTH2) { + if (this.isOauthAuthMethod) { checkOauthConnectionResult(this.oauthConnectionResult, this.oauthConnectionErrorMessage) } }, @@ -153,6 +165,9 @@ export default { diff --git a/src/constants/messages.js b/src/constants/messages.js index 2a360d5ac..bb3d029f0 100644 --- a/src/constants/messages.js +++ b/src/constants/messages.js @@ -3,4 +3,5 @@ import APP_ID from './appID.js' export const error = { featureNotAvailable: t(APP_ID, 'This feature is not available for this user account'), + opConnectionUnauthorized: t(APP_ID, 'Unauthorized to connect to OpenProject'), } diff --git a/tests/jest/components/PersonalSettings.spec.js b/tests/jest/components/PersonalSettings.spec.js index 888a0f2dc..9a3230de7 100644 --- a/tests/jest/components/PersonalSettings.spec.js +++ b/tests/jest/components/PersonalSettings.spec.js @@ -31,29 +31,47 @@ jest.mock('@nextcloud/dialogs', () => ({ })) describe('PersonalSettings.vue', () => { - describe('oAuth', () => { - const oAuthButtonSelector = 'oauthconnectbutton-stub' - const oAuthDisconnectButtonSelector = '.openproject-prefs--disconnect' - const connectedAsLabelSelector = '.openproject-prefs--connected label' - const personalSettingsFormSelector = '.openproject-prefs--form' - const personalEnableNavigationSelector = '#openproject-prefs--link' - const personalEnableSearchSelector = '#openproject-prefs--u-search' - const userGuideIntegrationDocumentationLinkSelector = '.settings--documentation-info' - let wrapper + const oAuthButtonSelector = 'oauthconnectbutton-stub' + const oAuthDisconnectButtonSelector = '.openproject-prefs--disconnect' + const connectedInfoSelector = '.openproject-prefs--connected' + const connectedAsLabelSelector = `${connectedInfoSelector} label` + const personalSettingsFormSelector = '.openproject-prefs--form' + const personalEnableNavigationSelector = '#openproject-prefs--link' + const personalEnableSearchSelector = '#openproject-prefs--u-search' + const userGuideIntegrationDocumentationLinkSelector = '.settings--documentation-info' + const errorLabelSelector = 'errorlabel-stub' + let wrapper - beforeEach(() => { - wrapper = shallowMount(PersonalSettings, { - localVue, - mocks: { - t: (app, msg) => msg, - generateUrl() { - return '/' - }, + beforeEach(() => { + wrapper = shallowMount(PersonalSettings, { + localVue, + mocks: { + t: (app, msg) => msg, + generateUrl() { + return '/' }, - }) + }, + }) + }) + + it('should show show user guide documentation link', () => { + const wrapper = getMountedWrapper() + const userGuideIntegrationDocumentationLink = wrapper.find(userGuideIntegrationDocumentationLinkSelector) + expect(userGuideIntegrationDocumentationLink.text()).toBe('Learn how to get the most out of the OpenProject integration by visiting our {htmlLink}.') + }) + + describe('oAuth', () => { + // common state + const commonState = { authorization_method: AUTH_METHOD.OAUTH2 } + afterEach(() => { + delete commonState.admin_config_ok }) describe('when the admin config is okay', () => { + beforeEach(() => { + commonState.admin_config_ok = true + }) + describe.each([ { user_name: 'test', token: '' }, { user_name: 'test', token: null }, @@ -68,13 +86,14 @@ describe('PersonalSettings.vue', () => { beforeEach(async () => { await wrapper.setData({ state: { - admin_config_ok: true, + ...commonState, ...cases, }, }) }) it('oAuth connect button is displayed', () => { expect(wrapper.find(oAuthButtonSelector).exists()).toBeTruthy() + expect(wrapper.find(errorLabelSelector).exists()).toBeFalsy() }) it('personal settings form is not displayed', () => { expect(wrapper.find(personalSettingsFormSelector).exists()).toBeFalsy() @@ -82,16 +101,11 @@ describe('PersonalSettings.vue', () => { it('oAuth disconnect button is not displayed', () => { expect(wrapper.find(oAuthDisconnectButtonSelector).exists()).toBeFalsy() }) - it('should show show user guide documentation link', () => { - const wrapper = getMountedWrapper({ state: { admin_config_ok: true, cases } }) - const userGuideIntegrationDocumentationLink = wrapper.find(userGuideIntegrationDocumentationLinkSelector) - expect(userGuideIntegrationDocumentationLink.text()).toBe('Learn how to get the most out of the OpenProject integration by visiting our {htmlLink}.') - }) }) describe('when username and token are given', () => { beforeEach(async () => { await wrapper.setData({ - state: { user_name: 'test', token: '123', admin_config_ok: true, authorization_method: AUTH_METHOD.OAUTH2 }, + state: { ...commonState, user_name: 'test', token: '123' }, }) }) it('oAuth connect button is not displayed', () => { @@ -105,19 +119,16 @@ describe('PersonalSettings.vue', () => { }) it('personal settings form is displayed', () => { expect(wrapper.find(personalSettingsFormSelector).exists()).toBeTruthy() - }) - it('should show not show user guide documentation link', () => { - const wrapper = getMountedWrapper({ state: { user_name: 'test', token: '123', admin_config_ok: true } }) - const userGuideIntegrationDocumentationLink = wrapper.find(userGuideIntegrationDocumentationLinkSelector) - expect(userGuideIntegrationDocumentationLink.text()).toBe('Learn how to get the most out of the OpenProject integration by visiting our {htmlLink}.') + expect(wrapper.find(errorLabelSelector).exists()).toBeFalsy() }) }) }) describe('when the admin config is not okay', () => { beforeEach(async () => { + commonState.admin_config_ok = false await wrapper.setData({ - state: { user_name: 'test', token: '123', admin_config_ok: false }, + state: { ...commonState, user_name: 'test', token: '123' }, }) }) it('should set proper props to the oauth connect component', () => { @@ -125,103 +136,273 @@ describe('PersonalSettings.vue', () => { expect(wrapper.find(oAuthButtonSelector).props()).toMatchObject({ isAdminConfigOk: false, }) + expect(wrapper.find(errorLabelSelector).exists()).toBeFalsy() }) }) + }) - describe('user settings', () => { - it('should be enabled if the admin has enabled the settings', async () => { - await wrapper.setData({ - state: { user_name: 'test', token: '123', admin_config_ok: true, navigation_enabled: true, search_enabled: true, notification_enabled: true }, + describe('OIDC', () => { + // common state + const commonState = { authorization_method: AUTH_METHOD.OIDC } + afterEach(() => { + delete commonState.admin_config_ok + }) + + describe('when the admin config is okay', () => { + beforeEach(() => { + commonState.admin_config_ok = true + }) + afterEach(() => { + delete commonState.oidc_user + }) + + describe('OIDC user', () => { + beforeEach(() => { + commonState.oidc_user = true + }) + + describe.each([ + { user_name: 'test', token: '' }, + { user_name: 'test', token: null }, + { user_name: 'test', token: undefined }, + { user_name: '', token: '123' }, + { user_name: null, token: '123' }, + { user_name: undefined, token: '123' }, + { user_name: '', token: '' }, + { user_name: null, token: '' }, + { user_name: '', token: null }, + ])('when username or token not given', (cases) => { + beforeEach(async () => { + await wrapper.setData({ + state: { + ...commonState, + ...cases, + }, + }) + }) + it(`should show unauthorized error: ${JSON.stringify(cases)}`, () => { + const errorLabel = wrapper.find(errorLabelSelector) + expect(errorLabel.exists()).toBeTruthy() + expect(errorLabel.attributes('error')).toBe('Unauthorized to connect to OpenProject') + + expect(wrapper.find(connectedInfoSelector).exists()).toBeFalsy() + expect(wrapper.find(personalSettingsFormSelector).exists()).toBeFalsy() + expect(wrapper.find(oAuthButtonSelector).exists()).toBeFalsy() + expect(wrapper.find(oAuthDisconnectButtonSelector).exists()).toBeFalsy() + }) + }) + + describe('when username and token are given', () => { + beforeEach(async () => { + await wrapper.setData({ + state: { ...commonState, user_name: 'test', token: '123' }, + }) + }) + it('connected info and personal settings form are displayed', () => { + // see states + expect(wrapper.find(connectedAsLabelSelector).text()).toBe('Connected as {user}') + expect(wrapper.find(personalSettingsFormSelector).exists()).toBeTruthy() + + expect(wrapper.find(errorLabelSelector).exists()).toBeFalsy() + expect(wrapper.find(oAuthButtonSelector).exists()).toBeFalsy() + expect(wrapper.find(oAuthDisconnectButtonSelector).exists()).toBeFalsy() + }) }) - expect(wrapper.find(personalSettingsFormSelector)).toMatchSnapshot() }) - it('should be disabled if the admin has not enabled the settings', async () => { - await wrapper.setData({ - state: { - user_name: 'test', - token: '123', - admin_config_ok: true, - navigation_enabled: false, - search_enabled: false, - notification_enabled: false, - }, + describe('non OIDC user', () => { + beforeEach(() => { + commonState.oidc_user = false }) - expect(wrapper.find(personalSettingsFormSelector)).toMatchSnapshot() - }) - - it('should send only one request if only one is enabled by the user', async () => { - dialogs.showSuccess.mockImplementationOnce() - const saveDefaultsSpy = jest.spyOn(axios, 'put') - .mockImplementationOnce(() => Promise.resolve({ data: [] })) - const wrapper = getMountedWrapper({ - state: { - user_name: 'test', - token: '123', - admin_config_ok: true, - navigation_enabled: false, - search_enabled: false, - notification_enabled: false, - }, + + describe.each([ + { user_name: 'test', token: '' }, + { user_name: 'test', token: null }, + { user_name: 'test', token: undefined }, + { user_name: '', token: '123' }, + { user_name: null, token: '123' }, + { user_name: undefined, token: '123' }, + { user_name: '', token: '' }, + { user_name: null, token: '' }, + { user_name: '', token: null }, + ])('when username or token not given', (cases) => { + beforeEach(async () => { + await wrapper.setData({ + state: { + ...commonState, + ...cases, + }, + }) + }) + it(`should show feature not available error: ${JSON.stringify(cases)}`, () => { + const errorLabel = wrapper.find(errorLabelSelector) + expect(errorLabel.exists()).toBeTruthy() + expect(errorLabel.attributes('error')).toBe('This feature is not available for this user account') + + expect(wrapper.find(connectedInfoSelector).exists()).toBeFalsy() + expect(wrapper.find(personalSettingsFormSelector).exists()).toBeFalsy() + expect(wrapper.find(oAuthButtonSelector).exists()).toBeFalsy() + expect(wrapper.find(oAuthDisconnectButtonSelector).exists()).toBeFalsy() + }) }) - let personalEnableNavigation = wrapper.find(personalEnableNavigationSelector) - await personalEnableNavigation.trigger('click') - personalEnableNavigation = wrapper.find(personalEnableNavigationSelector) - expect(saveDefaultsSpy).toBeCalledTimes(1) - expect(saveDefaultsSpy).toBeCalledWith( - 'http://localhost/apps/integration_openproject/config', - { - values: { - navigation_enabled: '1', - }, - }, - ) - expect(dialogs.showSuccess).toBeCalledTimes(1) - expect(dialogs.showSuccess).toBeCalledWith('OpenProject options saved') - jest.clearAllMocks() - }) - - it('admin and user should be able to enable and disable the setting simultaneously', async () => { - dialogs.showSuccess.mockImplementationOnce() - const saveDefaultsSpy = jest.spyOn(axios, 'put') - .mockImplementationOnce(() => Promise.resolve({ data: [] })) - const wrapper = getMountedWrapper({ - state: { - user_name: 'test', - token: '123', - admin_config_ok: true, - navigation_enabled: true, - search_enabled: false, - notification_enabled: true, - }, + describe('when username and token are given', () => { + beforeEach(async () => { + await wrapper.setData({ + state: { ...commonState, user_name: 'test', token: '123' }, + }) + }) + it('should show feature not available error', () => { + const errorLabel = wrapper.find(errorLabelSelector) + expect(errorLabel.exists()).toBeTruthy() + expect(errorLabel.attributes('error')).toBe('This feature is not available for this user account') + + expect(wrapper.find(connectedInfoSelector).exists()).toBeFalsy() + expect(wrapper.find(personalSettingsFormSelector).exists()).toBeFalsy() + expect(wrapper.find(oAuthButtonSelector).exists()).toBeFalsy() + expect(wrapper.find(oAuthDisconnectButtonSelector).exists()).toBeFalsy() + }) }) + }) + }) - let personalEnableSearch = wrapper.find(personalEnableSearchSelector) - await personalEnableSearch.trigger('click') - personalEnableSearch = wrapper.find(personalEnableSearchSelector) - expect(saveDefaultsSpy).toBeCalledTimes(1) - expect(saveDefaultsSpy).toBeCalledWith( - 'http://localhost/apps/integration_openproject/config', - { - values: { - search_enabled: '1', - }, - }, - ) - expect(dialogs.showSuccess).toBeCalledTimes(1) - expect(dialogs.showSuccess).toBeCalledWith('OpenProject options saved') - expect(wrapper.find(personalSettingsFormSelector)).toMatchSnapshot() + describe.each([{ oidc_user: false }, { oidc_user: true }])('when the admin config is not okay', (states) => { + beforeEach(async () => { + commonState.admin_config_ok = false await wrapper.setData({ - state: { - navigation_enabled: false, - search_enabled: true, - notification_enabled: false, - }, + state: { ...commonState, user_name: 'test', token: '123', ...states }, }) - expect(wrapper.find(personalSettingsFormSelector)).toMatchSnapshot() - jest.clearAllMocks() }) + it(`should show error message: ${JSON.stringify(states)}`, () => { + let error = 'This feature is not available for this user account' + if (states.oidc_user) { + error = 'Unauthorized to connect to OpenProject' + } + + const errorLabel = wrapper.find(errorLabelSelector) + expect(errorLabel.exists()).toBeTruthy() + expect(errorLabel.attributes('error')).toBe(error) + + expect(wrapper.find(connectedInfoSelector).exists()).toBeFalsy() + expect(wrapper.find(personalSettingsFormSelector).exists()).toBeFalsy() + expect(wrapper.find(oAuthButtonSelector).exists()).toBeFalsy() + expect(wrapper.find(oAuthDisconnectButtonSelector).exists()).toBeFalsy() + }) + }) + }) + + describe.each([{ authorization_method: AUTH_METHOD.OAUTH2, oidc_user: false }, { authorization_method: AUTH_METHOD.OIDC, oidc_user: true }])('user settings', (states) => { + let saveDefaultsSpy, showSuccessSpy + beforeEach(() => { + showSuccessSpy = dialogs.showSuccess.mockImplementationOnce() + saveDefaultsSpy = jest.spyOn(axios, 'put').mockImplementationOnce(() => Promise.resolve({ data: [] })) + }) + + afterEach(() => { + saveDefaultsSpy.mockClear() + saveDefaultsSpy.mockReset() + saveDefaultsSpy.mockRestore() + showSuccessSpy.mockClear() + showSuccessSpy.mockReset() + showSuccessSpy.mockRestore() + }) + + it(`should be enabled if the admin has enabled the settings: ${states.authorization_method}`, async () => { + await wrapper.setData({ + state: { + user_name: 'test', + token: '123', + admin_config_ok: true, + navigation_enabled: true, + search_enabled: true, + notification_enabled: true, + ...states, + }, + }) + expect(wrapper.find(personalSettingsFormSelector).element).toMatchSnapshot() + }) + + it(`should be disabled if the admin has not enabled the settings: ${states.authorization_method}`, async () => { + await wrapper.setData({ + state: { + user_name: 'test', + token: '123', + admin_config_ok: true, + navigation_enabled: false, + search_enabled: false, + notification_enabled: false, + ...states, + }, + }) + expect(wrapper.find(personalSettingsFormSelector).element).toMatchSnapshot() + }) + + it(`should send only one request if only one is enabled by the user: ${states.authorization_method}`, async () => { + const wrapper = getMountedWrapper({ + state: { + user_name: 'test', + token: '123', + admin_config_ok: true, + navigation_enabled: false, + search_enabled: false, + notification_enabled: false, + ...states, + }, + }) + + let personalEnableNavigation = wrapper.find(personalEnableNavigationSelector) + await personalEnableNavigation.trigger('click') + personalEnableNavigation = wrapper.find(personalEnableNavigationSelector) + expect(saveDefaultsSpy).toBeCalledTimes(1) + expect(saveDefaultsSpy).toBeCalledWith( + 'http://localhost/apps/integration_openproject/config', + { + values: { + navigation_enabled: '1', + }, + }, + ) + expect(showSuccessSpy).toBeCalledTimes(1) + expect(showSuccessSpy).toBeCalledWith('OpenProject options saved') + }) + + it(`admin and user should be able to enable and disable the setting simultaneously: ${states.authorization_method}`, async () => { + const wrapper = getMountedWrapper({ + state: { + user_name: 'test', + token: '123', + admin_config_ok: true, + navigation_enabled: true, + search_enabled: false, + notification_enabled: true, + ...states, + }, + }) + + let personalEnableSearch = wrapper.find(personalEnableSearchSelector) + await personalEnableSearch.trigger('click') + personalEnableSearch = wrapper.find(personalEnableSearchSelector) + expect(saveDefaultsSpy).toBeCalledTimes(1) + expect(saveDefaultsSpy).toBeCalledWith( + 'http://localhost/apps/integration_openproject/config', + { + values: { + search_enabled: '1', + }, + }, + ) + expect(showSuccessSpy).toBeCalledTimes(1) + expect(showSuccessSpy).toBeCalledWith('OpenProject options saved') + expect(wrapper.find(personalSettingsFormSelector).element).toMatchSnapshot() + await wrapper.setData({ + state: { + navigation_enabled: false, + search_enabled: true, + notification_enabled: false, + ...states, + }, + }) + expect(wrapper.find(personalSettingsFormSelector).element).toMatchSnapshot() }) }) }) diff --git a/tests/jest/components/__snapshots__/PersonalSettings.spec.js.snap b/tests/jest/components/__snapshots__/PersonalSettings.spec.js.snap index 9f8d13848..0862c0fff 100644 --- a/tests/jest/components/__snapshots__/PersonalSettings.spec.js.snap +++ b/tests/jest/components/__snapshots__/PersonalSettings.spec.js.snap @@ -1,25 +1,477 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`PersonalSettings.vue oAuth user settings admin and user should be able to enable and disable the setting simultaneously 1`] = ` -Wrapper { - "selector": ".openproject-prefs--form", -} +exports[`PersonalSettings.vue user settings admin and user should be able to enable and disable the setting simultaneously: oauth2 1`] = ` +
+
+ + + + +

+ Displays a link to your OpenProject instance in the Nextcloud header. +

+
+ +
+ + + + +

+ Allows you to search OpenProject work packages via the universal search bar in Nextcloud. +

+ +

+ + + Warning, everything you type in the search bar will be sent to your OpenProject instance. + +

+
+
`; -exports[`PersonalSettings.vue oAuth user settings admin and user should be able to enable and disable the setting simultaneously 2`] = ` -Wrapper { - "selector": ".openproject-prefs--form", -} +exports[`PersonalSettings.vue user settings admin and user should be able to enable and disable the setting simultaneously: oauth2 2`] = ` +
+
+ + + + +

+ Displays a link to your OpenProject instance in the Nextcloud header. +

+
+ +
+ + + + +

+ Allows you to search OpenProject work packages via the universal search bar in Nextcloud. +

+ +

+ + + Warning, everything you type in the search bar will be sent to your OpenProject instance. + +

+
+
`; -exports[`PersonalSettings.vue oAuth user settings should be disabled if the admin has not enabled the settings 1`] = ` -Wrapper { - "selector": ".openproject-prefs--form", -} +exports[`PersonalSettings.vue user settings admin and user should be able to enable and disable the setting simultaneously: oidc 1`] = ` +
+
+ + + + +

+ Displays a link to your OpenProject instance in the Nextcloud header. +

+
+ +
+ + + + +

+ Allows you to search OpenProject work packages via the universal search bar in Nextcloud. +

+ +

+ + + Warning, everything you type in the search bar will be sent to your OpenProject instance. + +

+
+
`; -exports[`PersonalSettings.vue oAuth user settings should be enabled if the admin has enabled the settings 1`] = ` -Wrapper { - "selector": ".openproject-prefs--form", -} +exports[`PersonalSettings.vue user settings admin and user should be able to enable and disable the setting simultaneously: oidc 2`] = ` +
+
+ + + + +

+ Displays a link to your OpenProject instance in the Nextcloud header. +

+
+ +
+ + + + +

+ Allows you to search OpenProject work packages via the universal search bar in Nextcloud. +

+ +

+ + + Warning, everything you type in the search bar will be sent to your OpenProject instance. + +

+
+
+`; + +exports[`PersonalSettings.vue user settings should be disabled if the admin has not enabled the settings: oauth2 1`] = ` +
+ +

+ Displays a link to your OpenProject instance in the Nextcloud header. +

+
+ + +

+ Allows you to search OpenProject work packages via the universal search bar in Nextcloud. +

+ +
+
+
+`; + +exports[`PersonalSettings.vue user settings should be disabled if the admin has not enabled the settings: oidc 1`] = ` +
+ +

+ Displays a link to your OpenProject instance in the Nextcloud header. +

+
+ + +

+ Allows you to search OpenProject work packages via the universal search bar in Nextcloud. +

+ +
+
+
+`; + +exports[`PersonalSettings.vue user settings should be enabled if the admin has enabled the settings: oauth2 1`] = ` +
+ +

+ Displays a link to your OpenProject instance in the Nextcloud header. +

+
+ + +

+ Allows you to search OpenProject work packages via the universal search bar in Nextcloud. +

+ +

+ + + Warning, everything you type in the search bar will be sent to your OpenProject instance. + +

+
+
+`; + +exports[`PersonalSettings.vue user settings should be enabled if the admin has enabled the settings: oidc 1`] = ` +
+ +

+ Displays a link to your OpenProject instance in the Nextcloud header. +

+
+ + +

+ Allows you to search OpenProject work packages via the universal search bar in Nextcloud. +

+ +

+ + + Warning, everything you type in the search bar will be sent to your OpenProject instance. + +

+
+
`; diff --git a/tests/lib/Service/OpenProjectAPIServiceTest.php b/tests/lib/Service/OpenProjectAPIServiceTest.php index 3860719c8..281577cfc 100644 --- a/tests/lib/Service/OpenProjectAPIServiceTest.php +++ b/tests/lib/Service/OpenProjectAPIServiceTest.php @@ -29,6 +29,7 @@ use OCA\UserOIDC\Event\ExchangedTokenRequestedEvent; use OCA\UserOIDC\Exception\TokenExchangeFailedException; use OCA\UserOIDC\Model\Token; +use OCA\UserOIDC\User\Backend as OIDCBackend; use OCP\App\IAppManager; use OCP\AppFramework\Http; use OCP\Encryption\IManager; @@ -49,9 +50,11 @@ use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserManager; +use OCP\IUserSession; use OCP\Log\ILogFactory; use OCP\Security\IRemoteHostValidator; use OCP\Security\ISecureRandom; +use phpmock\phpunit\PHPMock; use PhpPact\Consumer\InteractionBuilder; use PhpPact\Consumer\Model\Body\Binary; use PhpPact\Consumer\Model\ConsumerRequest; @@ -61,19 +64,9 @@ use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; -/** - * overriding the class_exists method, so that the unit tests always pass, - * no matter if the activity app is enabled or not - */ -function class_exists(string $className): bool { - if ($className === '\OCA\GroupFolders\Folder\FolderManager') { - return true; - } else { - return \class_exists($className); - } -} - class OpenProjectAPIServiceTest extends TestCase { + use PHPMock; + /** * @var InteractionBuilder */ @@ -662,6 +655,7 @@ private function getOpenProjectAPIServiceConstructArgs(array $constructParams = 'logFactory' => $this->createMock(ILogFactory::class), 'manager' => $this->createMock(IManager::class), 'exchangedTokenRequestedEventHelper' => $this->createMock(ExchangedTokenRequestedEventHelper::class), + 'userSession' => $this->createMock(IUserSession::class), ]; // replace default mocks with manually passed in mocks @@ -4280,4 +4274,49 @@ public function testGetOIDCTokenNoToken(): void { $result = $service->getOIDCToken(); $this->assertEquals(null, $result); } + + public function dataProviderForIsOIDCUser(): array { + $backendMock = $this->getMockBuilder(OIDCBackend::class)->disableOriginalConstructor()->getMock(); + return [ + 'has Backend class and OIDC user' => [ + 'backend' => true, + 'oidcUser' => $backendMock, + 'expected' => true, + ], + 'has Backend class but not OIDC user' => [ + 'backend' => true, + 'oidcUser' => new \stdClass(), + 'expected' => false, + ], + 'missing Backend class' => [ + 'backend' => false, + 'oidcUser' => $backendMock, + 'expected' => false, + ], + ]; + } + + /** + * @dataProvider dataProviderForIsOIDCUser + * + * @return void + */ + public function testIsOIDCUser($backend, $oidcUser, $expected): void { + $mock = $this->getFunctionMock(__NAMESPACE__, "class_exists"); + $mock->expects($this->once())->willReturn($backend); + + $userSessionMock = $this->createMock(IUserSession::class); + $userMock = $this->createMock(IUser::class); + + $userMock->method('getBackend')->willReturn($oidcUser); + $userSessionMock->method('getUser')->willReturn($userMock); + + $service = $this->getOpenProjectAPIServiceMock( + [], + ['userSession' => $userSessionMock], + ); + + $result = $service->isOIDCUser(); + $this->assertEquals($expected, $result); + } } diff --git a/tests/lib/Settings/PersonalTest.php b/tests/lib/Settings/PersonalTest.php index 02ef9a28a..6a7a489f9 100644 --- a/tests/lib/Settings/PersonalTest.php +++ b/tests/lib/Settings/PersonalTest.php @@ -136,7 +136,8 @@ public function testGetForm( 'search_enabled' => false, 'navigation_enabled' => false, 'admin_config_ok' => $adminConfigStatus, - 'authorization_method' => OpenProjectAPIService::AUTH_METHOD_OAUTH + 'authorization_method' => OpenProjectAPIService::AUTH_METHOD_OAUTH, + 'oidc_user' => false, ] ], ['oauth-connection-result'], @@ -198,7 +199,8 @@ public function testNoPersonalSettingsShouldUseValueFromTheDefaults() { 'search_enabled' => true, 'navigation_enabled' => true, 'admin_config_ok' => true, - 'authorization_method' => OpenProjectAPIService::AUTH_METHOD_OAUTH + 'authorization_method' => OpenProjectAPIService::AUTH_METHOD_OAUTH, + 'oidc_user' => false, ] ], ['oauth-connection-result'],