Skip to content

Commit

Permalink
Validate webhook after auth flow
Browse files Browse the repository at this point in the history
  • Loading branch information
vbihun committed Aug 2, 2023
1 parent c48c0b7 commit 47f6a03
Show file tree
Hide file tree
Showing 10 changed files with 28 additions and 24 deletions.
4 changes: 2 additions & 2 deletions src/client/gitlab.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ export const deleteGroupWebhook = async (groupId: number, hookId: number, groupT
try {
await callGitlab(`/api/v4/groups/${groupId}/hooks/${hookId}`, groupToken, { method: HttpMethod.DELETE });
} catch (e) {
if (e.message.includes('Not Found')) {
if (e.statusText.includes('Not Found')) {
return;
}
throw e;
Expand All @@ -154,7 +154,7 @@ export const getGroupWebhook = async (

return webhook;
} catch (e) {
if (e.message.includes('Not Found')) {
if (e.statusText.includes('Not Found')) {
return null;
}
throw e;
Expand Down
10 changes: 7 additions & 3 deletions src/resolvers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
ImportFailedError,
importProjects,
} from './services/import-projects';
import { setupWebhook } from './services/webhooks';
import { setupAndValidateWebhook } from './services/webhooks';
import { disconnectGroup } from './services/disconnect-group';
import { getForgeAppId } from './utils/get-forge-app-id';
import { getLastSyncTime } from './services/last-sync-time';
Expand Down Expand Up @@ -48,10 +48,14 @@ resolver.define('groups/disconnect', async (req): Promise<ResolverResponse> => {
}
});

resolver.define('groups', async (): Promise<ResolverResponse<GitlabAPIGroup[]>> => {
resolver.define('groups/connectedInfo', async (): Promise<ResolverResponse<GitlabAPIGroup[]>> => {
try {
const connectedGroups = await getConnectedGroups();

if (connectedGroups.length) {
await setupAndValidateWebhook(connectedGroups[0].id);
}

return { success: true, data: connectedGroups };
} catch (e) {
return {
Expand All @@ -69,7 +73,7 @@ resolver.define('groups/connect', async (req): Promise<ResolverResponse> => {
try {
const groupId = await connectGroup(groupToken, groupTokenName);

await setupWebhook(groupId);
await setupAndValidateWebhook(groupId);

await graphqlGateway.compass.asApp().synchronizeLinkAssociations({
cloudId,
Expand Down
8 changes: 4 additions & 4 deletions src/services/webhook.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { storage, mockForgeApi, webTrigger } from '../__tests__/helpers/forge-he
mockForgeApi();

import { getGroupWebhook, registerGroupWebhook } from '../client/gitlab';
import { setupWebhook } from './webhooks';
import { setupAndValidateWebhook } from './webhooks';
import { TEST_TOKEN } from '../__tests__/fixtures/gitlab-data';

jest.mock('../client/gitlab');
Expand All @@ -27,7 +27,7 @@ describe('webhook service', () => {
storage.getSecret = jest.fn().mockReturnValueOnce(TEST_TOKEN);
mockGetGroupWebhook.mockResolvedValue({ id: 456 });

const result = await setupWebhook(123);
const result = await setupAndValidateWebhook(123);

expect(storage.set).not.toHaveBeenCalled();
expect(result).toBe(MOCK_WEBHOOK_ID);
Expand All @@ -39,7 +39,7 @@ describe('webhook service', () => {
webTrigger.getUrl = jest.fn().mockReturnValue('https://example.com');
mockRegisterGroupWebhook.mockResolvedValue(MOCK_WEBHOOK_ID);

const result = await setupWebhook(MOCK_GROUP_ID);
const result = await setupAndValidateWebhook(MOCK_GROUP_ID);

expect(mockGetGroupWebhook).not.toHaveBeenCalled();
expect(storage.set).toHaveBeenNthCalledWith(1, MOCK_WEBHOOK_KEY, MOCK_WEBHOOK_ID);
Expand All @@ -54,7 +54,7 @@ describe('webhook service', () => {
webTrigger.getUrl = jest.fn().mockReturnValue('https://example.com');
mockRegisterGroupWebhook.mockResolvedValue(MOCK_WEBHOOK_ID);

const result = await setupWebhook(MOCK_GROUP_ID);
const result = await setupAndValidateWebhook(MOCK_GROUP_ID);

expect(storage.set).toHaveBeenNthCalledWith(1, MOCK_WEBHOOK_KEY, MOCK_WEBHOOK_ID);
expect(storage.set).toHaveBeenNthCalledWith(2, MOCK_WEBHOOK_SIGNATURE_KEY, expect.anything());
Expand Down
2 changes: 1 addition & 1 deletion src/services/webhooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { registerGroupWebhook, deleteGroupWebhook, getGroupWebhook } from '../cl
import { GITLAB_EVENT_WEBTRIGGER, STORAGE_KEYS, STORAGE_SECRETS } from '../constants';
import { generateSignature } from '../utils/generate-signature-utils';

export const setupWebhook = async (groupId: number): Promise<number> => {
export const setupAndValidateWebhook = async (groupId: number): Promise<number> => {
const [existingWebhook, groupToken] = await Promise.all([
storage.get(`${STORAGE_KEYS.WEBHOOK_KEY_PREFIX}${groupId}`),
storage.getSecret(`${STORAGE_SECRETS.GROUP_TOKEN_KEY_PREFIX}${groupId}`),
Expand Down
4 changes: 2 additions & 2 deletions ui/src/components/ConnectedPage/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export const ConnectedPage = () => {
const [groups, setGroups] = useState<GitlabAPIGroup[]>();

const navigate = useNavigate();
const { getGroups, clearGroup } = useAppContext();
const { getConnectedInfo, clearGroup } = useAppContext();
const { isImportInProgress } = useImportContext();

const handleDisconnectGroup = async (id: number) => {
Expand All @@ -45,7 +45,7 @@ export const ConnectedPage = () => {
};

useEffect(() => {
getGroups().then(setGroups);
getConnectedInfo().then(setGroups);
}, []);

if (errorType) {
Expand Down
4 changes: 2 additions & 2 deletions ui/src/components/SelectImportPage/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const DEFAULT_GROUP_ID = 0;
export const SelectImportPage = () => {
const navigate = useNavigate();

const { getGroups, features } = useAppContext();
const { getConnectedInfo, features } = useAppContext();
const { setTotalSelectedRepos, setIsImportInProgress, setImportedRepositories } = useImportContext();
const componentTypesResult = useComponentTypes();

Expand Down Expand Up @@ -99,7 +99,7 @@ export const SelectImportPage = () => {

useEffect(() => {
setIsProjectsLoading(true);
getGroups()
getConnectedInfo()
.then((value) => {
if (value) {
setLocationGroupId(value[0].id);
Expand Down
12 changes: 6 additions & 6 deletions ui/src/context/AppContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { view } from '@forge/bridge';
import { CenterWrapper } from '../components/styles';
import { AuthErrorTypes, ErrorTypes, FeaturesList, GitlabAPIGroup } from '../resolverTypes';
import { ApplicationState } from '../routes';
import { getForgeAppId, listConnectedGroups } from '../services/invokes';
import { getForgeAppId, connectedInfo } from '../services/invokes';
import { DefaultErrorState } from '../components/DefaultErrorState';
import { useFeatures } from '../hooks/useFeatures';

Expand All @@ -16,7 +16,7 @@ type AppContextProviderProps = {

export type AppContextProps = {
initialRoute?: ApplicationState;
getGroups: () => Promise<GitlabAPIGroup[] | undefined>;
getConnectedInfo: () => Promise<GitlabAPIGroup[] | undefined>;
clearGroup: (groupId: number) => void;
features: FeaturesList;
moduleKey: string;
Expand Down Expand Up @@ -63,7 +63,7 @@ export const AppContextProvider: FunctionComponent<AppContextProviderProps> = ({
setErrorType(AuthErrorTypes.UNEXPECTED_ERROR);
});

listConnectedGroups()
connectedInfo()
.then(({ data, success, errors }) => {
setGroupsLoading(false);

Expand All @@ -85,13 +85,13 @@ export const AppContextProvider: FunctionComponent<AppContextProviderProps> = ({
});
}, []);

const getGroups = async (): Promise<GitlabAPIGroup[] | undefined> => {
const getConnectedInfo = async (): Promise<GitlabAPIGroup[] | undefined> => {
if (groups && groups?.length > 0) {
return groups;
}

try {
const { data, success, errors } = await listConnectedGroups();
const { data, success, errors } = await connectedInfo();

if (success && data && data.length > 0) {
setGroups(data);
Expand Down Expand Up @@ -126,7 +126,7 @@ export const AppContextProvider: FunctionComponent<AppContextProviderProps> = ({
}

return (
<AppContext.Provider value={{ initialRoute, getGroups, clearGroup, features, moduleKey, appId }}>
<AppContext.Provider value={{ initialRoute, getConnectedInfo, clearGroup, features, moduleKey, appId }}>
{children}
</AppContext.Provider>
);
Expand Down
2 changes: 1 addition & 1 deletion ui/src/context/__mocks__/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ export const filledMocks: {
[key: string]: unknown;
} = {
...defaultMocks,
groups: {
'groups/connectedInfo': {
success: true,
data: [
{
Expand Down
2 changes: 1 addition & 1 deletion ui/src/helpers/mockHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const getContext: jest.Mock = view.getContext as jest.Mock;
export const defaultMocks: {
[key: string]: unknown;
} = {
groups: {
'groups/connectedInfo': {
success: true,
data: [],
},
Expand Down
4 changes: 2 additions & 2 deletions ui/src/services/invokes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ export const disconnectGroup = (id: number): Promise<ResolverResponse> => {
});
};

export const listConnectedGroups = (): Promise<ResolverResponse<GitlabAPIGroup[]>> => {
return invoke<ResolverResponse<GitlabAPIGroup[]>>('groups');
export const connectedInfo = (): Promise<ResolverResponse<GitlabAPIGroup[]>> => {
return invoke<ResolverResponse<GitlabAPIGroup[]>>('groups/connectedInfo');
};

export const getAllExistingGroups = (): Promise<ResolverResponse<GitlabAPIGroup[]>> => {
Expand Down

0 comments on commit 47f6a03

Please sign in to comment.