fix: Check OAuth2 redirect URL for matching callback URL and authorization code in query parameters (#2148)

* Check OAuth2 redirect URL for matching callback URL and authorization code in query parameters

In an Authorization code flow, there may be multiple intermediate redirects before reaching the final one which matches the callback URL and has a code in the query params.

We should wait until we see a redirect URI that matches both the conditions. This fixes the issue where, when a redirect contains `code` as a query param but is not the final one (i.e., is not to the callback URL) an error is thrown saying the callback URL is invalid.

Fixes #2147

* Add test cases for callback URL check

* Update check to cover URLs with same host but different endpoints
This commit is contained in:
Dakshin K 2024-05-31 15:41:31 +05:30 committed by GitHub
parent 32b1ba1c92
commit 46df2e967f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 29 additions and 6 deletions

View File

@ -1,6 +1,10 @@
const { BrowserWindow } = require('electron');
const { preferencesUtil } = require('../../store/preferences');
const matchesCallbackUrl = (url, callbackUrl) => {
return url ? url.href.startsWith(callbackUrl.href) : false;
};
const authorizeUserInWindow = ({ authorizeUrl, callbackUrl, session }) => {
return new Promise(async (resolve, reject) => {
let finalUrl = null;
@ -30,12 +34,12 @@ const authorizeUserInWindow = ({ authorizeUrl, callbackUrl, session }) => {
});
function onWindowRedirect(url) {
// check if the url contains an authorization code
if (new URL(url).searchParams.has('code')) {
finalUrl = url;
if (!url || !finalUrl.includes(callbackUrl)) {
reject(new Error('Invalid Callback Url'));
// check if the redirect is to the callback URL and if it contains an authorization code
if (matchesCallbackUrl(new URL(url), new URL(callbackUrl))) {
if (!new URL(url).searchParams.has('code')) {
reject(new Error('Invalid Callback URL: Does not contain an authorization code'));
}
finalUrl = url;
window.close();
}
if (url.match(/(error=).*/) || url.match(/(error_description=).*/) || url.match(/(error_uri=).*/)) {
@ -93,4 +97,4 @@ const authorizeUserInWindow = ({ authorizeUrl, callbackUrl, session }) => {
});
};
module.exports = { authorizeUserInWindow };
module.exports = { authorizeUserInWindow, matchesCallbackUrl };

View File

@ -0,0 +1,19 @@
const { matchesCallbackUrl } = require('../../src/ipc/network/authorize-user-in-window');
describe('matchesCallbackUrl', () => {
const testCases = [
{ url: 'https://random-url/endpoint', expected: false },
{ url: 'https://random-url/endpoint?code=abcd', expected: false },
{ url: 'https://callback.url/endpoint?code=abcd', expected: true },
{ url: 'https://callback.url/endpoint/?code=abcd', expected: true },
{ url: 'https://callback.url/random-endpoint/?code=abcd', expected: false }
];
it.each(testCases)('$url - should be $expected', ({ url, expected }) => {
let callBackUrl = 'https://callback.url/endpoint';
let actual = matchesCallbackUrl(new URL(url), new URL(callBackUrl));
expect(actual).toBe(expected);
});
});