Skip to content

Commit 5712df1

Browse files
setchybmulholland
setchy
and
bmulholland
authored
feat(auth): use system browser for GitHub SSO / OAuth authentication (#1781)
* wip Signed-off-by: Adam Setch <[email protected]> * feat: use system browser for oauth flow Signed-off-by: Adam Setch <[email protected]> * feat: use system browser for oauth flow Signed-off-by: Adam Setch <[email protected]> * fix tests Signed-off-by: Adam Setch <[email protected]> * feat: use system browser for oauth flow Signed-off-by: Adam Setch <[email protected]> * feat: use system browser for oauth flow Signed-off-by: Adam Setch <[email protected]> * Update config/webpack.config.renderer.base.ts Co-authored-by: Brendan Mulholland <[email protected]> * feat: use system browser for oauth flow Signed-off-by: Adam Setch <[email protected]> * feat: use system browser for oauth flow Signed-off-by: Adam Setch <[email protected]> * feat: use system browser for oauth flow Signed-off-by: Adam Setch <[email protected]> * fix add account error Signed-off-by: Adam Setch <[email protected]> * feat: use system browser for oauth flow Signed-off-by: Adam Setch <[email protected]> * feat: use system browser for oauth flow Signed-off-by: Adam Setch <[email protected]> --------- Signed-off-by: Adam Setch <[email protected]> Co-authored-by: Brendan Mulholland <[email protected]>
1 parent 73b2030 commit 5712df1

File tree

11 files changed

+244
-215
lines changed

11 files changed

+244
-215
lines changed

‎config/webpack.config.renderer.base.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,10 @@ const configuration: webpack.Configuration = {
4040
},
4141

4242
plugins: [
43-
// Development Keys - See README.md
43+
// Development Keys - See CONTRIBUTING.md
4444
new webpack.EnvironmentPlugin({
45-
OAUTH_CLIENT_ID: '3fef4433a29c6ad8f22c',
46-
OAUTH_CLIENT_SECRET: '9670de733096c15322183ff17ed0fc8704050379',
45+
OAUTH_CLIENT_ID: 'Ov23liQIkFs5ehQLNzHF',
46+
OAUTH_CLIENT_SECRET: '404b80632292e18419dbd2a6ed25976856e95255',
4747
}),
4848

4949
// Extract CSS into a separate file

‎package.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,12 @@
8080
"package.json"
8181
],
8282
"electronLanguages": ["en"],
83+
"protocols": [
84+
{
85+
"name": "Gitify",
86+
"schemes": ["gitify"]
87+
}
88+
],
8389
"mac": {
8490
"category": "public.app-category.developer-tools",
8591
"icon": "assets/images/app-icon.icns",

‎src/main/main.ts

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ const browserWindowOpts = {
2121
skipTaskbar: true, // Hide the app from the Windows taskbar
2222
// TODO ideally we would disable this as use a preload script with a context bridge
2323
webPreferences: {
24-
enableRemoteModule: true,
2524
nodeIntegration: true,
2625
contextIsolation: false,
2726
},
@@ -38,11 +37,14 @@ const mb = menubar({
3837
const menuBuilder = new MenuBuilder(mb);
3938
const contextMenu = menuBuilder.buildMenu();
4039

41-
/**
42-
* Electron Auto Updater only supports macOS and Windows
43-
* https://github.com/electron/update-electron-app
44-
*/
40+
// Register your app as the handler for a custom protocol
41+
app.setAsDefaultProtocolClient('gitify');
42+
4543
if (isMacOS() || isWindows()) {
44+
/**
45+
* Electron Auto Updater only supports macOS and Windows
46+
* https://github.com/electron/update-electron-app
47+
*/
4648
const updater = new Updater(mb, menuBuilder);
4749
updater.initialize();
4850
}
@@ -55,13 +57,6 @@ app.whenReady().then(async () => {
5557
mb.on('ready', () => {
5658
mb.app.setAppUserModelId(APPLICATION.ID);
5759

58-
/**
59-
* TODO: Remove @electron/remote use - see #650
60-
* GitHub OAuth 2 Login Flows - Enable Remote Browser Window Launch
61-
*/
62-
require('@electron/remote/main').initialize();
63-
require('@electron/remote/main').enable(mb.window.webContents);
64-
6560
// Tray configuration
6661
mb.tray.setToolTip(APPLICATION.NAME);
6762
mb.tray.setIgnoreDoubleClickEvents(true);
@@ -173,3 +168,9 @@ app.whenReady().then(async () => {
173168
app.setLoginItemSettings(settings);
174169
});
175170
});
171+
172+
// Handle gitify:// custom protocol URL events for OAuth 2.0 callback
173+
app.on('open-url', (event, url) => {
174+
event.preventDefault();
175+
mb.window.webContents.send(namespacedEvent('auth-callback'), url);
176+
});

‎src/renderer/components/settings/AppearanceSettings.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -262,8 +262,8 @@ export const AppearanceSettings: FC = () => {
262262
<Checkbox
263263
name="showAccountHeader"
264264
label="Show account header"
265-
checked={settings.showAccountHeader || hasMultipleAccounts(auth)}
266-
disabled={hasMultipleAccounts(auth)}
265+
checked={settings.showAccountHeader}
266+
visible={!hasMultipleAccounts(auth)}
267267
onChange={(evt) =>
268268
updateSetting('showAccountHeader', evt.target.checked)
269269
}

‎src/renderer/components/settings/NotificationSettings.tsx

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { type FC, type MouseEvent, useContext } from 'react';
22

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

66
import { APPLICATION } from '../../../shared/constants';
77
import { AppContext } from '../../context/App';
@@ -61,9 +61,8 @@ export const NotificationSettings: FC = () => {
6161
tooltip={
6262
<Text>
6363
See{' '}
64-
<button
65-
type="button"
66-
className="text-gitify-link"
64+
<Box
65+
className="text-gitify-link cursor-pointer"
6766
title="Open GitHub documentation for participating and watching notifications"
6867
onClick={(event: MouseEvent<HTMLElement>) => {
6968
// Don't trigger onClick of parent element.
@@ -72,7 +71,7 @@ export const NotificationSettings: FC = () => {
7271
}}
7372
>
7473
official docs
75-
</button>{' '}
74+
</Box>{' '}
7675
for more details.
7776
</Text>
7877
}

‎src/renderer/routes/Accounts.tsx

Lines changed: 37 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,10 @@ export const AccountsRoute: FC = () => {
4949
useContext(AppContext);
5050
const navigate = useNavigate();
5151

52+
const [loadingStates, setLoadingStates] = useState<Record<string, boolean>>(
53+
{},
54+
);
55+
5256
const logoutAccount = useCallback(
5357
(account: Account) => {
5458
logoutFromAccount(account);
@@ -65,6 +69,29 @@ export const AccountsRoute: FC = () => {
6569
navigate('/accounts', { replace: true });
6670
}, []);
6771

72+
const handleRefresh = useCallback(async (account: Account) => {
73+
const accountUUID = getAccountUUID(account);
74+
75+
setLoadingStates((prev) => ({
76+
...prev,
77+
[accountUUID]: true,
78+
}));
79+
80+
await refreshAccount(account);
81+
navigate('/accounts', { replace: true });
82+
83+
/**
84+
* Typically the above refresh API call completes very quickly,
85+
* so we add an brief artificial delay to allow the icon to spin a few times
86+
*/
87+
setTimeout(() => {
88+
setLoadingStates((prev) => ({
89+
...prev,
90+
[accountUUID]: false,
91+
}));
92+
}, 500);
93+
}, []);
94+
6895
const loginWithGitHub = useCallback(async () => {
6996
try {
7097
await loginWithGitHubApp();
@@ -89,11 +116,11 @@ export const AccountsRoute: FC = () => {
89116
{auth.accounts.map((account, i) => {
90117
const AuthMethodIcon = getAuthMethodIcon(account.method);
91118
const PlatformIcon = getPlatformIcon(account.platform);
92-
const [isRefreshingAccount, setIsRefreshingAccount] = useState(false);
119+
const accountUUID = getAccountUUID(account);
93120

94121
return (
95122
<Box
96-
key={getAccountUUID(account)}
123+
key={accountUUID}
97124
className="rounded-md p-2 mb-4 bg-gitify-accounts"
98125
>
99126
<Stack
@@ -107,7 +134,6 @@ export const AccountsRoute: FC = () => {
107134
title="Open account profile"
108135
onClick={() => openAccountProfile(account)}
109136
data-testid="account-profile"
110-
className="pb-2"
111137
>
112138
<AvatarWithFallback
113139
src={account.user.avatar}
@@ -130,10 +156,10 @@ export const AccountsRoute: FC = () => {
130156
</Stack>
131157
</Box>
132158

133-
<button
159+
<Box
134160
title="Open host"
135-
type="button"
136161
onClick={() => openHost(account.hostname)}
162+
className="cursor-pointer"
137163
data-testid="account-host"
138164
>
139165
<Stack
@@ -144,12 +170,12 @@ export const AccountsRoute: FC = () => {
144170
<PlatformIcon />
145171
<Text>{account.hostname}</Text>
146172
</Stack>
147-
</button>
173+
</Box>
148174

149-
<button
175+
<Box
150176
title="Open developer settings"
151-
type="button"
152177
onClick={() => openDeveloperSettings(account)}
178+
className="cursor-pointer"
153179
data-testid="account-developer-settings"
154180
>
155181
<Stack
@@ -160,7 +186,7 @@ export const AccountsRoute: FC = () => {
160186
<AuthMethodIcon />
161187
<Text>{account.method}</Text>
162188
</Stack>
163-
</button>
189+
</Box>
164190
</Stack>
165191
</Box>
166192
</Stack>
@@ -192,22 +218,9 @@ export const AccountsRoute: FC = () => {
192218
<IconButton
193219
icon={SyncIcon}
194220
aria-label={`Refresh ${account.user.login}`}
195-
onClick={async () => {
196-
setIsRefreshingAccount(true);
197-
198-
await refreshAccount(account);
199-
navigate('/accounts', { replace: true });
200-
201-
/**
202-
* Typically the above refresh API call completes very quickly,
203-
* so we add an brief artificial delay to allow the icon to spin a few times
204-
*/
205-
setTimeout(() => {
206-
setIsRefreshingAccount(false);
207-
}, 500);
208-
}}
221+
onClick={() => handleRefresh(account)}
209222
size="small"
210-
loading={isRefreshingAccount}
223+
loading={loadingStates[accountUUID] || false}
211224
data-testid="account-refresh"
212225
/>
213226

0 commit comments

Comments
 (0)