Skip to content

feat(auth): use system browser for GitHub SSO / OAuth authentication #1781

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

Merged
merged 17 commits into from
Jan 29, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions config/webpack.config.renderer.base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ const configuration: webpack.Configuration = {
},

plugins: [
// Development Keys - See README.md
// Development Keys - See CONTRIBUTING.md
new webpack.EnvironmentPlugin({
OAUTH_CLIENT_ID: '3fef4433a29c6ad8f22c',
OAUTH_CLIENT_SECRET: '9670de733096c15322183ff17ed0fc8704050379',
OAUTH_CLIENT_ID: 'Ov23liQIkFs5ehQLNzHF',
OAUTH_CLIENT_SECRET: '404b80632292e18419dbd2a6ed25976856e95255',
}),

// Extract CSS into a separate file
Expand Down
6 changes: 6 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,12 @@
"package.json"
],
"electronLanguages": ["en"],
"protocols": [
{
"name": "Gitify",
"schemes": ["gitify"]
}
],
"mac": {
"category": "public.app-category.developer-tools",
"icon": "assets/images/app-icon.icns",
Expand Down
25 changes: 13 additions & 12 deletions src/main/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ const browserWindowOpts = {
resizable: false,
skipTaskbar: true, // Hide the app from the Windows taskbar
webPreferences: {
enableRemoteModule: true,
nodeIntegration: true,
contextIsolation: false,
},
Expand All @@ -37,11 +36,14 @@ const mb = menubar({
const menuBuilder = new MenuBuilder(mb);
const contextMenu = menuBuilder.buildMenu();

/**
* Electron Auto Updater only supports macOS and Windows
* https://github.com/electron/update-electron-app
*/
// Register your app as the handler for a custom protocol
app.setAsDefaultProtocolClient('gitify');

if (isMacOS() || isWindows()) {
/**
* Electron Auto Updater only supports macOS and Windows
* https://github.com/electron/update-electron-app
*/
const updater = new Updater(mb, menuBuilder);
updater.initialize();
}
Expand All @@ -54,13 +56,6 @@ app.whenReady().then(async () => {
mb.on('ready', () => {
mb.app.setAppUserModelId(APPLICATION.ID);

/**
* TODO: Remove @electron/remote use - see #650
* GitHub OAuth 2 Login Flows - Enable Remote Browser Window Launch
*/
require('@electron/remote/main').initialize();
require('@electron/remote/main').enable(mb.window.webContents);

// Tray configuration
mb.tray.setToolTip(APPLICATION.NAME);
mb.tray.setIgnoreDoubleClickEvents(true);
Expand Down Expand Up @@ -172,3 +167,9 @@ app.whenReady().then(async () => {
app.setLoginItemSettings(settings);
});
});

// Handle gitify:// custom protocol URL events for OAuth 2.0 callback
app.on('open-url', (event, url) => {
event.preventDefault();
mb.window.webContents.send(namespacedEvent('auth-callback'), url);
});
4 changes: 2 additions & 2 deletions src/renderer/components/settings/AppearanceSettings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,8 @@ export const AppearanceSettings: FC = () => {
<Checkbox
name="showAccountHeader"
label="Show account header"
checked={settings.showAccountHeader || hasMultipleAccounts(auth)}
disabled={hasMultipleAccounts(auth)}
checked={settings.showAccountHeader}
visible={!hasMultipleAccounts(auth)}
onChange={(evt) =>
updateSetting('showAccountHeader', evt.target.checked)
}
Expand Down
9 changes: 4 additions & 5 deletions src/renderer/components/settings/NotificationSettings.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { type FC, type MouseEvent, useContext } from 'react';

import { BellIcon } from '@primer/octicons-react';
import { Stack, Text } from '@primer/react';
import { Box, Stack, Text } from '@primer/react';

import { APPLICATION } from '../../../shared/constants';
import { AppContext } from '../../context/App';
Expand Down Expand Up @@ -61,9 +61,8 @@ export const NotificationSettings: FC = () => {
tooltip={
<Text>
See{' '}
<button
type="button"
className="text-gitify-link"
<Box
className="text-gitify-link cursor-pointer"
title="Open GitHub documentation for participating and watching notifications"
onClick={(event: MouseEvent<HTMLElement>) => {
// Don't trigger onClick of parent element.
Expand All @@ -72,7 +71,7 @@ export const NotificationSettings: FC = () => {
}}
>
official docs
</button>{' '}
</Box>{' '}
for more details.
</Text>
}
Expand Down
61 changes: 37 additions & 24 deletions src/renderer/routes/Accounts.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ export const AccountsRoute: FC = () => {
useContext(AppContext);
const navigate = useNavigate();

const [loadingStates, setLoadingStates] = useState<Record<string, boolean>>(
{},
);

const logoutAccount = useCallback(
(account: Account) => {
logoutFromAccount(account);
Expand All @@ -65,6 +69,29 @@ export const AccountsRoute: FC = () => {
navigate('/accounts', { replace: true });
}, []);

const handleRefresh = useCallback(async (account: Account) => {
const accountUUID = getAccountUUID(account);

setLoadingStates((prev) => ({
...prev,
[accountUUID]: true,
}));

await refreshAccount(account);
navigate('/accounts', { replace: true });

/**
* Typically the above refresh API call completes very quickly,
* so we add an brief artificial delay to allow the icon to spin a few times
*/
setTimeout(() => {
setLoadingStates((prev) => ({
...prev,
[accountUUID]: false,
}));
}, 500);
}, []);

const loginWithGitHub = useCallback(async () => {
try {
await loginWithGitHubApp();
Expand All @@ -89,11 +116,11 @@ export const AccountsRoute: FC = () => {
{auth.accounts.map((account, i) => {
const AuthMethodIcon = getAuthMethodIcon(account.method);
const PlatformIcon = getPlatformIcon(account.platform);
const [isRefreshingAccount, setIsRefreshingAccount] = useState(false);
const accountUUID = getAccountUUID(account);

return (
<Box
key={getAccountUUID(account)}
key={accountUUID}
className="rounded-md p-2 mb-4 bg-gitify-accounts"
>
<Stack
Expand All @@ -107,7 +134,6 @@ export const AccountsRoute: FC = () => {
title="Open account profile"
onClick={() => openAccountProfile(account)}
data-testid="account-profile"
className="pb-2"
>
<AvatarWithFallback
src={account.user.avatar}
Expand All @@ -130,10 +156,10 @@ export const AccountsRoute: FC = () => {
</Stack>
</Box>

<button
<Box
title="Open host"
type="button"
onClick={() => openHost(account.hostname)}
className="cursor-pointer"
data-testid="account-host"
>
<Stack
Expand All @@ -144,12 +170,12 @@ export const AccountsRoute: FC = () => {
<PlatformIcon />
<Text>{account.hostname}</Text>
</Stack>
</button>
</Box>

<button
<Box
title="Open developer settings"
type="button"
onClick={() => openDeveloperSettings(account)}
className="cursor-pointer"
data-testid="account-developer-settings"
>
<Stack
Expand All @@ -160,7 +186,7 @@ export const AccountsRoute: FC = () => {
<AuthMethodIcon />
<Text>{account.method}</Text>
</Stack>
</button>
</Box>
</Stack>
</Box>
</Stack>
Expand Down Expand Up @@ -192,22 +218,9 @@ export const AccountsRoute: FC = () => {
<IconButton
icon={SyncIcon}
aria-label={`Refresh ${account.user.login}`}
onClick={async () => {
setIsRefreshingAccount(true);

await refreshAccount(account);
navigate('/accounts', { replace: true });

/**
* Typically the above refresh API call completes very quickly,
* so we add an brief artificial delay to allow the icon to spin a few times
*/
setTimeout(() => {
setIsRefreshingAccount(false);
}, 500);
}}
onClick={() => handleRefresh(account)}
size="small"
loading={isRefreshingAccount}
loading={loadingStates[accountUUID] || false}
data-testid="account-refresh"
/>

Expand Down
Loading