From 7461f61373a8764cd4d4271ad89297dd035127af Mon Sep 17 00:00:00 2001 From: Zaz Brown Date: Tue, 16 May 2023 22:24:30 +0000 Subject: [PATCH 1/7] add test for parsing URL of self-managed GitLab instance #868 --- .../gitlab_sync_backend_client.test.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/sync_backend_clients/gitlab_sync_backend_client.test.js b/src/sync_backend_clients/gitlab_sync_backend_client.test.js index 24d285916..10192fd13 100644 --- a/src/sync_backend_clients/gitlab_sync_backend_client.test.js +++ b/src/sync_backend_clients/gitlab_sync_backend_client.test.js @@ -17,6 +17,21 @@ test('Parses GitLab project from URL', () => { }); }); +test('Parses GitLab project from URL of a self-managed GitLab instance', () => { + [ + ['https://mygitlab.com/user/foo', 'user%2Ffoo'], + ['https://mygitlab.com/group/subgroup/project', 'group%2Fsubgroup%2Fproject'], + ['mygitlab.com/foo/bar', 'foo%2Fbar'], + ['mygitlab.com/user-but-no-project', undefined], + ['https://www.mygitlab.com/user/foo', 'user%2Ffoo'], + ['https://www.mygitlab.com/group/subgroup/project', 'group%2Fsubgroup%2Fproject'], + ['www.mygitlab.com/foo/bar', 'foo%2Fbar'], + ['www.mygitlab.com/user-but-no-project', undefined], + ].forEach(([input, expected]) => { + expect(gitLabProjectIdFromURL(input)).toEqual(expected); + }); +}); + test('Parses Link pagination header', () => { [ [null, {}], From 1f4d111c08499852e5b0844dbeaf207daf498db0 Mon Sep 17 00:00:00 2001 From: Zaz Brown Date: Tue, 16 May 2023 22:30:36 +0000 Subject: [PATCH 2/7] WiP: allow use of self-hosted GitLab instance #868 --- src/App.js | 3 ++- src/components/SyncServiceSignIn/index.js | 4 ++-- .../gitlab_sync_backend_client.js | 12 +++++++----- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/App.js b/src/App.js index b3de033b0..9cd7632d0 100644 --- a/src/App.js +++ b/src/App.js @@ -88,7 +88,8 @@ export default class App extends PureComponent { }); break; case 'GitLab': - const gitlabOAuth = createGitlabOAuth(); + # should be createGitlabOAuth(url), where url is the GitLab instance + const gitlabOAuth = createGitlabOAuth(); # FIXME if (gitlabOAuth.isAuthorized()) { client = createGitLabSyncBackendClient(gitlabOAuth); initialState.syncBackend = Map({ diff --git a/src/components/SyncServiceSignIn/index.js b/src/components/SyncServiceSignIn/index.js index 487466a94..a8728479e 100644 --- a/src/components/SyncServiceSignIn/index.js +++ b/src/components/SyncServiceSignIn/index.js @@ -117,10 +117,10 @@ function GitLab() { if (projectId) { persistField('authenticatedSyncService', 'GitLab'); persistField('gitLabProject', projectId); - createGitlabOAuth().fetchAuthorizationCode(); + createGitlabOAuth(project).fetchAuthorizationCode(); } else { evt.preventDefault(); - alert('Project does not appear to be a valid gitlab.com URL'); + alert('Project does not appear to be a valid GitLab URL'); } }; diff --git a/src/sync_backend_clients/gitlab_sync_backend_client.js b/src/sync_backend_clients/gitlab_sync_backend_client.js index 92fefc7ee..84d83b858 100644 --- a/src/sync_backend_clients/gitlab_sync_backend_client.js +++ b/src/sync_backend_clients/gitlab_sync_backend_client.js @@ -5,15 +5,16 @@ import { getPersistedField } from '../util/settings_persister'; import { fromJS, Map } from 'immutable'; -export const createGitlabOAuth = () => { +export const createGitlabOAuth = (url = 'https://gitlab.com') => { // Use promises as mutex to prevent concurrent token refresh attempts, which causes problems. // More info: https://github.com/BitySA/oauth2-auth-code-pkce/issues/29 // TODO: remove this workaround if/when oauth2-auth-code-pkce fixes the issue. let expiryPromise; let invalidGrantPromise; + url = new URL(url) return new OAuth2AuthCodePKCE({ - authorizationUrl: 'https://gitlab.com/oauth/authorize', - tokenUrl: 'https://gitlab.com/oauth/token', + authorizationUrl: url.origin + '/oauth/authorize', + tokenUrl: url.origin + '/oauth/token', clientId: process.env.REACT_APP_GITLAB_CLIENT_ID, redirectUrl: window.location.origin, scopes: ['api'], @@ -64,7 +65,7 @@ export const gitLabProjectIdFromURL = (projectURL) => { // to a project. Reminder: a project path is not necessarily // /user/project because it may be under one or more groups such // as /user/group/subgroup/project. - if (url.hostname === 'gitlab.com' && path.split('/').length > 1) { + if (path.split('/').length > 1) { return encodeURIComponent(path); } else { return undefined; @@ -130,7 +131,8 @@ export const treeToDirectoryListing = (tree) => { ); }; -const API_URL = 'https://gitlab.com/api/v4'; +# should use API URL of GitLab instance +const API_URL = 'https://gitlab.com/api/v4'; # FIXME /** * GitLab sync backend, implemented using their REST API. From 68a62d81661f327bc86ef240f12c4fcf72c15e2d Mon Sep 17 00:00:00 2001 From: Zaz Brown Date: Tue, 16 May 2023 22:54:34 +0000 Subject: [PATCH 3/7] store full GitLab URL in addition to just the project #868 --- src/App.js | 4 ++-- src/components/SyncServiceSignIn/index.js | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/App.js b/src/App.js index 9cd7632d0..6311f81a2 100644 --- a/src/App.js +++ b/src/App.js @@ -88,8 +88,8 @@ export default class App extends PureComponent { }); break; case 'GitLab': - # should be createGitlabOAuth(url), where url is the GitLab instance - const gitlabOAuth = createGitlabOAuth(); # FIXME + project = getPersistedField('gitLabURL'), + const gitlabOAuth = createGitlabOAuth(project); if (gitlabOAuth.isAuthorized()) { client = createGitLabSyncBackendClient(gitlabOAuth); initialState.syncBackend = Map({ diff --git a/src/components/SyncServiceSignIn/index.js b/src/components/SyncServiceSignIn/index.js index a8728479e..10585dc38 100644 --- a/src/components/SyncServiceSignIn/index.js +++ b/src/components/SyncServiceSignIn/index.js @@ -116,7 +116,8 @@ function GitLab() { const projectId = gitLabProjectIdFromURL(project); if (projectId) { persistField('authenticatedSyncService', 'GitLab'); - persistField('gitLabProject', projectId); + persistField('gitLabURL', project); + persistField('gitLabProject', projectId); # TODO: remove redundant field createGitlabOAuth(project).fetchAuthorizationCode(); } else { evt.preventDefault(); From 69fc8e18408c77d71df9802121560031f5d1d9b1 Mon Sep 17 00:00:00 2001 From: Zaz Brown Date: Tue, 16 May 2023 23:07:59 +0000 Subject: [PATCH 4/7] use the specific GitLab instace's API URL #868 --- .../gitlab_sync_backend_client.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/sync_backend_clients/gitlab_sync_backend_client.js b/src/sync_backend_clients/gitlab_sync_backend_client.js index 84d83b858..c3f1b3e08 100644 --- a/src/sync_backend_clients/gitlab_sync_backend_client.js +++ b/src/sync_backend_clients/gitlab_sync_backend_client.js @@ -131,8 +131,7 @@ export const treeToDirectoryListing = (tree) => { ); }; -# should use API URL of GitLab instance -const API_URL = 'https://gitlab.com/api/v4'; # FIXME +const API_PATH = '/api/v4/' /** * GitLab sync backend, implemented using their REST API. @@ -143,7 +142,12 @@ const API_URL = 'https://gitlab.com/api/v4'; # FIXME export default (oauthClient) => { const decoratedFetch = oauthClient.decorateFetchHTTPClient(fetch); - const getProjectApi = () => `${API_URL}/projects/${getPersistedField('gitLabProject')}`; + const getApi = () => { + url = new URL(getPersistedField('gitLabURL')) + return url.origin + API_PATH + } + + const getProjectApi = () => `${getApi()}projects/${getPersistedField('gitLabProject')}`; const isSignedIn = async () => { if (!oauthClient.isAuthorized()) { @@ -176,7 +180,7 @@ export default (oauthClient) => { // commit. const [userResponse, membersResponse] = await Promise.all([ // https://docs.gitlab.com/ee/api/users.html#list-current-user-for-normal-users - decoratedFetch(`${API_URL}/user`), + decoratedFetch(getApi() + 'user'), // https://docs.gitlab.com/ee/api/members.html#list-all-members-of-a-group-or-project decoratedFetch(`${getProjectApi()}/members`), ]); From 3a0ff1803842d81dd47acdd0d304afd87263db56 Mon Sep 17 00:00:00 2001 From: Zaz Brown Date: Tue, 16 May 2023 23:13:33 +0000 Subject: [PATCH 5/7] add `let` --- src/App.js | 2 +- src/sync_backend_clients/gitlab_sync_backend_client.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/App.js b/src/App.js index 6311f81a2..0a3fc8a5e 100644 --- a/src/App.js +++ b/src/App.js @@ -88,7 +88,7 @@ export default class App extends PureComponent { }); break; case 'GitLab': - project = getPersistedField('gitLabURL'), + let project = getPersistedField('gitLabURL'), const gitlabOAuth = createGitlabOAuth(project); if (gitlabOAuth.isAuthorized()) { client = createGitLabSyncBackendClient(gitlabOAuth); diff --git a/src/sync_backend_clients/gitlab_sync_backend_client.js b/src/sync_backend_clients/gitlab_sync_backend_client.js index c3f1b3e08..3a6d19363 100644 --- a/src/sync_backend_clients/gitlab_sync_backend_client.js +++ b/src/sync_backend_clients/gitlab_sync_backend_client.js @@ -143,7 +143,7 @@ export default (oauthClient) => { const decoratedFetch = oauthClient.decorateFetchHTTPClient(fetch); const getApi = () => { - url = new URL(getPersistedField('gitLabURL')) + let url = new URL(getPersistedField('gitLabURL')) return url.origin + API_PATH } From 65505ac76121085bb74d210843d49ab011fe0957 Mon Sep 17 00:00:00 2001 From: Zaz Brown Date: Tue, 16 May 2023 23:25:08 +0000 Subject: [PATCH 6/7] refactor: don't store GitLab project this information is included in the URL --- src/components/SyncServiceSignIn/index.js | 3 --- src/sync_backend_clients/gitlab_sync_backend_client.js | 4 +++- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/components/SyncServiceSignIn/index.js b/src/components/SyncServiceSignIn/index.js index 10585dc38..7ae0185d6 100644 --- a/src/components/SyncServiceSignIn/index.js +++ b/src/components/SyncServiceSignIn/index.js @@ -10,7 +10,6 @@ import GitLabLogo from './gitlab.svg'; import { persistField } from '../../util/settings_persister'; import { createGitlabOAuth, - gitLabProjectIdFromURL, } from '../../sync_backend_clients/gitlab_sync_backend_client'; import { DropboxAuth } from 'dropbox'; @@ -113,11 +112,9 @@ function GitLab() { const defaultProject = 'https://gitlab.com/your/project'; const [project, setProject] = useState(defaultProject); const handleSubmit = (evt) => { - const projectId = gitLabProjectIdFromURL(project); if (projectId) { persistField('authenticatedSyncService', 'GitLab'); persistField('gitLabURL', project); - persistField('gitLabProject', projectId); # TODO: remove redundant field createGitlabOAuth(project).fetchAuthorizationCode(); } else { evt.preventDefault(); diff --git a/src/sync_backend_clients/gitlab_sync_backend_client.js b/src/sync_backend_clients/gitlab_sync_backend_client.js index 3a6d19363..b50a13f3b 100644 --- a/src/sync_backend_clients/gitlab_sync_backend_client.js +++ b/src/sync_backend_clients/gitlab_sync_backend_client.js @@ -147,7 +147,9 @@ export default (oauthClient) => { return url.origin + API_PATH } - const getProjectApi = () => `${getApi()}projects/${getPersistedField('gitLabProject')}`; + const getProject = () => gitLabProjectIdFromURL(getPersistedField('gitLabURL')) + + const getProjectApi = () => getApi() + 'projects/' + getProject() const isSignedIn = async () => { if (!oauthClient.isAuthorized()) { From dae7568d222abdb343a87a8af3e7c2dd75213ff6 Mon Sep 17 00:00:00 2001 From: Zaz Brown Date: Sat, 3 Jun 2023 20:12:35 +0000 Subject: [PATCH 7/7] fixes --- src/App.js | 2 +- src/components/SyncServiceSignIn/index.js | 12 ++++-------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/App.js b/src/App.js index 0a3fc8a5e..dcb7ba8f9 100644 --- a/src/App.js +++ b/src/App.js @@ -88,7 +88,7 @@ export default class App extends PureComponent { }); break; case 'GitLab': - let project = getPersistedField('gitLabURL'), + let project = getPersistedField('gitLabURL'); const gitlabOAuth = createGitlabOAuth(project); if (gitlabOAuth.isAuthorized()) { client = createGitLabSyncBackendClient(gitlabOAuth); diff --git a/src/components/SyncServiceSignIn/index.js b/src/components/SyncServiceSignIn/index.js index 7ae0185d6..ee80d6a5c 100644 --- a/src/components/SyncServiceSignIn/index.js +++ b/src/components/SyncServiceSignIn/index.js @@ -112,14 +112,10 @@ function GitLab() { const defaultProject = 'https://gitlab.com/your/project'; const [project, setProject] = useState(defaultProject); const handleSubmit = (evt) => { - if (projectId) { - persistField('authenticatedSyncService', 'GitLab'); - persistField('gitLabURL', project); - createGitlabOAuth(project).fetchAuthorizationCode(); - } else { - evt.preventDefault(); - alert('Project does not appear to be a valid GitLab URL'); - } + persistField('authenticatedSyncService', 'GitLab'); + persistField('gitLabURL', project); + // TODO handle incorrect URLs, possibly with try ... catch ... + createGitlabOAuth(project).fetchAuthorizationCode(); }; return (