Skip to content

Commit

Permalink
Restore the authorisation opt-out functionality (#1098)
Browse files Browse the repository at this point in the history
Always decode url on factory url accept.
Restore the skip oauth functionality.
  • Loading branch information
vinokurig authored Apr 22, 2024
1 parent ea7a3bc commit ad970c4
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,11 @@ export function registerFactoryAcceptanceRedirect(instance: FastifyInstance): vo
.replace(path, '')
.replace(/^\?/, '')
.replace(`${FACTORY_LINK_ATTR}%3D`, `${FACTORY_LINK_ATTR}=`);
if (!factoryLinkStr.includes('=')) {
factoryLinkStr = decodeURIComponent(factoryLinkStr);
}
factoryLinkStr = decodeURIComponent(factoryLinkStr);
const query = querystring.parse(factoryLinkStr);
if (query[FACTORY_LINK_ATTR] !== undefined) {
// restore the factory link from the query string as base64 encoded string
const factoryLinkBase64 = decodeURIComponent(query[FACTORY_LINK_ATTR] as string);
const factoryLinkBase64 = query[FACTORY_LINK_ATTR] as string;
// decode from base64
factoryLinkStr = Buffer.from(factoryLinkBase64, 'base64').toString('utf-8');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ describe('GitServicesList', () => {
],
isDisabled: false,
onRevokeServices: jest.fn(),
onClearService: jest.fn(),
providersWithToken: ['github', 'gitlab'],
skipOauthProviders: [],
};
Expand Down Expand Up @@ -178,6 +179,58 @@ describe('GitServicesList', () => {
]);
});

test('can clear opt-out (github)', () => {
props = {
gitOauth: [
{
name: 'github',
endpointUrl: 'https://github.com',
},
],
isDisabled: false,
onRevokeServices: jest.fn(),
onClearService: jest.fn(),
providersWithToken: [],
skipOauthProviders: ['github'],
};
renderComponent(props);

const rows = screen.getAllByRole('row');

// get the github row
const githubRow = rows[1];
expect(githubRow).toHaveTextContent('github');

const githubCheckbox = within(githubRow).getByRole('checkbox');
const githubKebab = within(githubRow).getByRole('button', { name: 'Actions' });

// the checkbox is disabled and unchecked
expect(githubCheckbox).toBeDisabled();
expect(githubCheckbox).not.toBeChecked();

// the kebab button is enabled
expect(githubKebab).toBeEnabled();

// Clear button is not present
{
const clearButton = within(githubRow).queryByRole('menuitem', { name: 'Clear' });
expect(clearButton).toBeNull();
}

// open kebab menu
userEvent.click(githubKebab);

// the Clear button is present
const clearButton = within(githubRow).queryByRole('menuitem', { name: 'Clear' });
expect(clearButton).not.toBeNull();

// click the Clear button
userEvent.click(clearButton!);

expect(props.onClearService).toHaveBeenCalledTimes(1);
expect(props.onClearService).toHaveBeenCalledWith('github');
});

test('toolbar', () => {
renderComponent(props);

Expand Down Expand Up @@ -248,6 +301,7 @@ function getComponent(props: Props): React.ReactElement<Props> {
gitOauth={props.gitOauth}
isDisabled={props.isDisabled}
onRevokeServices={props.onRevokeServices}
onClearService={props.onClearService}
providersWithToken={props.providersWithToken}
skipOauthProviders={props.skipOauthProviders}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export type Props = {
providersWithToken: api.GitOauthProvider[];
skipOauthProviders: api.GitOauthProvider[];
onRevokeServices: (services: IGitOauth[]) => void;
onClearService: (service: api.GitOauthProvider) => void;
};

type State = {
Expand Down Expand Up @@ -118,8 +119,10 @@ export class GitServicesList extends React.PureComponent<Props, State> {
this.hasOauthToken(service.name) === true;

const canRevoke = this.isRevokeEnabled(service.name) === true;
const canClear = this.hasSkipOauth(service.name) === true;
const hasToken = this.hasOauthToken(service.name) === true;
const rowDisabled = isDisabled || canRevoke === false || hasToken === false;
const kebabDisabled = (isDisabled || !canRevoke || !hasToken) && !canClear;

const actionItems = this.buildActionItems(service);

Expand Down Expand Up @@ -158,7 +161,7 @@ export class GitServicesList extends React.PureComponent<Props, State> {
/>
</Td>
<Td dataLabel="Git Service Actions" isActionCell={true}>
<ActionsColumn isDisabled={rowDisabled} items={actionItems} />
<ActionsColumn isDisabled={kebabDisabled} items={actionItems} />
</Td>
</Tr>
);
Expand All @@ -167,6 +170,14 @@ export class GitServicesList extends React.PureComponent<Props, State> {
}

private buildActionItems(service: IGitOauth): IAction[] {
if (this.hasSkipOauth(service.name)) {
return [
{
title: 'Clear',
onClick: () => this.handleClearService(service),
},
];
}
return [
{
title: 'Revoke',
Expand All @@ -179,6 +190,13 @@ export class GitServicesList extends React.PureComponent<Props, State> {
return CAN_REVOKE_FROM_DASHBOARD.includes(providerName) === true;
}

private hasSkipOauth(providerName: api.GitOauthProvider): boolean {
// Use includes filter to handle the bitbucket-server oauth 2 provider.
// The bitbucket server oauth2 provider name is 'bitbucket',
// but the corresponding 'skip oauth' item is 'bitbucket-server'.
return this.props.skipOauthProviders.some(s => s.includes(providerName));
}

private hasOauthToken(providerName: api.GitOauthProvider): boolean {
return this.props.providersWithToken.includes(providerName);
}
Expand All @@ -188,6 +206,17 @@ export class GitServicesList extends React.PureComponent<Props, State> {
this.deselectServices([service]);
}

private handleClearService(service: IGitOauth): void {
// Use includes filter to handle the bitbucket-server oauth 2 provider.
// The bitbucket server oauth2 provider name is 'bitbucket',
// but the corresponding 'skip oauth' item is 'bitbucket-server'.
const serviceToClear = this.props.skipOauthProviders.find(s => s.includes(service.name));
if (serviceToClear != undefined) {
this.props.onClearService(serviceToClear);
this.deselectServices([service]);
}
}

private async handleRevokeSelectedServices(): Promise<void> {
const { selectedItems } = this.state;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,10 @@ export class GitServices extends React.PureComponent<Props, State> {
});
}

private handleClearServices(selectedService: api.GitOauthProvider): void {
this.props.deleteSkipOauth(selectedService);
}

render(): React.ReactNode {
const { gitOauth, isLoading, providersWithToken, skipOauthProviders } = this.props;
const { isModalOpen, selectedServices } = this.state;
Expand All @@ -155,6 +159,7 @@ export class GitServices extends React.PureComponent<Props, State> {
providersWithToken={providersWithToken}
skipOauthProviders={skipOauthProviders}
onRevokeServices={services => this.handleRevokeServices(services)}
onClearService={service => this.handleClearServices(service)}
/>
)}
</PageSection>
Expand Down
17 changes: 12 additions & 5 deletions packages/dashboard-frontend/src/store/GitOauthConfig/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,11 +235,18 @@ export const actionCreators: ActionCreators = {
});
} catch (e) {
const errorMessage = common.helpers.errors.getMessage(e);
dispatch({
type: Type.RECEIVE_GIT_OAUTH_ERROR,
error: errorMessage,
});
throw e;
if (new RegExp('^OAuth token for user .* was not found$').test(errorMessage)) {
dispatch({
type: Type.DELETE_GIT_OAUTH_TOKEN,
provider: oauthProvider,
});
} else {
dispatch({
type: Type.RECEIVE_GIT_OAUTH_ERROR,
error: errorMessage,
});
throw e;
}
}
},

Expand Down

0 comments on commit ad970c4

Please sign in to comment.