Skip to content
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

Add state support to PKCE implementation #2114

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,12 @@ const authorizeUserInWindow = ({ authorizeUrl, callbackUrl, session }) => {
});

function onWindowRedirect(url) {
// The finalUrl should always be the callbackUrl.
const hasHitRedirectUrl = url.includes(callbackUrl);
Copy link
Collaborator

@lohxt1 lohxt1 May 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#2148 (comment)

@m-schrepel Could you please handle the scenario pointed out by @pietrygamat in the above comment.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary now considering #2275 ?

// check if the url contains an authorization code
if (new URL(url).searchParams.has('code')) {
if (hasHitRedirectUrl && new URL(url).searchParams.has('code')) {
finalUrl = url;
if (!url || !finalUrl.includes(callbackUrl)) {
if (!url) {
reject(new Error('Invalid Callback Url'));
}
window.close();
Expand Down
17 changes: 8 additions & 9 deletions packages/bruno-electron/src/ipc/network/oauth2-helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,17 @@ const generateCodeVerifier = () => {
return crypto.randomBytes(22).toString('hex');
};

const generateCodeChallenge = (codeVerifier) => {
const hash = crypto.createHash('sha256');
hash.update(codeVerifier);
const base64Hash = hash.digest('base64');
return base64Hash.replace(/\+/g, '-').replace(/\//g, '_').replace(/=/g, '');
const generateUniqueHash = (inputString) => {
return crypto.createHash('sha256').update(inputString).digest('base64url');
};

// AUTHORIZATION CODE

const resolveOAuth2AuthorizationCodeAccessToken = async (request, collectionUid) => {
let codeVerifier = generateCodeVerifier();
let codeChallenge = generateCodeChallenge(codeVerifier);

let codeChallenge = generateUniqueHash(codeVerifier);
let requestCopy = cloneDeep(request);

const { authorizationCode } = await getOAuth2AuthorizationCode(requestCopy, codeChallenge, collectionUid);
const oAuth = get(requestCopy, 'oauth2', {});
const { clientId, clientSecret, callbackUrl, scope, pkce } = oAuth;
Expand All @@ -47,21 +44,23 @@ const getOAuth2AuthorizationCode = (request, codeChallenge, collectionUid) => {
return new Promise(async (resolve, reject) => {
const { oauth2 } = request;
const { callbackUrl, clientId, authorizationUrl, scope, pkce } = oauth2;
const oauth2Store = new Oauth2Store();
const state = generateUniqueHash(oauth2Store.getSessionIdOfCollection(collectionUid));

let oauth2QueryParams =
(authorizationUrl.indexOf('?') > -1 ? '&' : '?') + `client_id=${clientId}&response_type=code`;

if (callbackUrl) {
oauth2QueryParams += `&redirect_uri=${callbackUrl}`;
}
if (scope) {
oauth2QueryParams += `&scope=${scope}`;
}
if (pkce) {
oauth2QueryParams += `&code_challenge=${codeChallenge}&code_challenge_method=S256`;
oauth2QueryParams += `&code_challenge=${codeChallenge}&code_challenge_method=S256&state=${state}`;
}
const authorizationUrlWithQueryParams = authorizationUrl + oauth2QueryParams;
try {
const oauth2Store = new Oauth2Store();
const { authorizationCode } = await authorizeUserInWindow({
authorizeUrl: authorizationUrlWithQueryParams,
callbackUrl,
Expand Down