I noticed a case where this check may not be sufficient (which is the problem in this version as it was in the original):
My keycloak server has a redirect uri set to exactly: https://example.com?param=value. As keycloak expects me to use callback url exactly as configured, I do so in bruno, but after I provide correct credentials, I get redirected to https://example.com/?param=value&code=..., whis does not pass the check url.includes(callbackUrl) (extra / breaks this).
Perhaps the string comparison using includes should be done on URL.href's rather than plain string values, to have a level of normalization?

let strurl1 = 'https://example.com?par=val'
let strurl2 = 'https://example.com/?par=val'
strurl1 == strurl2 // ==>false
new URL(strurl1) == new URL(strurl2); // ==> false
new URL(strurl1).href == new URL(strurl2).href; // ==> true
new URL(strurl1).href // => https://example.com/?par=val 
new URL(strurl2).href // => https://example.com/?par=val 

@helloanoop @lohit-1 Do we have any unit tests on this behaviour? I was not able to find any when creating the PR but it seems like there are several edge cases here so I would like to add some if they are not there

Hey @dakshin-k, there aren't any unit tests in place for this behaviour currently.
Please feel free to add them. Thanks

@lohit-1 Have added some test cases, please review.

I couldn't write a test for the entire component, including mocking all the window events, so I just extracted that check into a separate function and wrote tests for it.

Nope: my callback url is pointing to the same server that is auth url. It will pass this check and fail on missing 'code' even before the first redirect.

Oh man, there are lots of edge cases here 😅

Comparing the href as you suggested earlier will not work as the href includes any query params in the URL, i.e. while it solves the trailing / problem it breaks for the below case:

callbackUrl: https://callback.url/endpoint
testUrl       : https://callback.url/endpoint?code=abcd

// testUrl.href = https://callback.url/endpoint?code=abcd

This is the reason I compared against hostname instead of href. I currently don't have a better idea to implement this, will think about it and come back later.

Ideas are welcome if I'm missing something 😇

The function could be something similar to this @dakshin-k

const matchesCallbackUrl = (url, callbackUrl) => {
  let { origin: urlOrigin, pathname: urlPathname } = new URL(url);
  let { origin: callbackUrlOrigin, pathname: callbackUrlPathname } = new URL(callbackUrl);

  urlPathname = urlPathname?.at(-1) == '/' ? urlPathname?.slice(0, -1) : urlPathname;
  callbackUrlPathname = callbackUrlPathname?.at(-1) == '/' ? callbackUrlPathname?.slice(0, -1) : callbackUrlPathname;

  const urlWithoutQueryParams = `${urlOrigin}${urlPathname}`;
  const callbackUrlWithoutQueryParams = `${callbackUrlOrigin}${callbackUrlPathname}`;

  return url ? urlWithoutQueryParams == callbackUrlWithoutQueryParams : false;
};

Edit:
Not recommended, seems like too much of an overkill
The below solution should be enough

Wouldn't this suffice?
testUrl.href.startswith(callbackUrl.href)

The spec mandates the code param is appended the the query component, so it should be at the end. Also luckily the fragment component (the url part after #) is not allowed, so it does not have to be handled.

Thanks, that worked! I have pushed the change, would someone mind testing it out once?

I faced a weird problem testing locally, not sure if I introduced a bug or found an existing one 😅

I'm able to generate auth tokens successfully now, but I can't make any requests - When I click the send request button, it just shows me the response from my OAuth server with the access token, it doesn't actually make a request using that token or display that response:

I wouldn't mind that. That's actually #1999

Ah okay, thanks!

So I guess that's it on this PR? Let me know if there's anything further to do!