fix: #6034 Authorization Code ( base and pkce flow ) - get right authorization code on each attempt

[davidanrod]

Description

PKCE Flow Issue: ( authorization code not being properly refreshed )

In the OAuth2 PKCE low, a client application requests an authorization code from the authorization server, then exchanges it for an access token. Each authorization attempt must use a fresh code, because authorization codes are single-use and that's not happening because the authorization code is being renewed only when the client closes the authorization popup.

Motivation and Context

This change ensures that on every PKCE authorization attempt, a fresh and correct authorization code is used. Any stale auth.code from previouss attempts is deleted before starting a new auth flow preventing reuse.
Fixes #6034

How Has This Been Tested?

I start by running swagger ui locally

1. git clone https://github.com/swagger-api/swagger-ui.git
2. cd swagger-ui
3. npm install
4. npm run dev

Then i set up a dummy server to simulate IdP for testing purposes.

// Step 1: /authorize
app.get("/authorize", (req, res) => {
  const { client_id, code_challenge, code_challenge_method, redirect_uri } = req.query;
  if (!client_id || !code_challenge) return res.status(400).send("ERROR");

  const code = crypto.randomBytes(8).toString("hex");
  codes[code] = { code_challenge, code_challenge_method, createdAt: Date.now() };

  console.log("code:::", code);
  const uri = new URL(redirect_uri);
  uri.searchParams.set("code", code);
  res.redirect(uri.toString());
});

app.post("/token", (req, res) => {
  const { grant_type, code, code_verifier } = req.body;
  if (grant_type !== "authorization_code") return res.status(400).json({ error: "ERROR" });

  const stored = codes[code];
  if (!stored) return res.status(400).json({ error: "invalid_grant" });

  // Validate PKCE
  const hash = crypto.createHash("sha256").update(code_verifier).digest();
  const base64url = hash.toString("base64").replace(/\+/g, "-").replace(/\//g, "_").replace(/=+$/, "");

  if (stored.code_challenge !== base64url) return res.status(400).json({ error: "invalid_grant", detail: "PKCE mismatch" });
  console.log("returning success:::");
  res.json({
    access_token: "__FOO__BAR__",
    token_type: "Bearer",
    expires_in: 36000
  });
});

So, it is clear that if we don’t close the popup after the first attempt, the authorization code is never refreshed. After this change, auth.code is always set correctly, regardless of whether the authorization popup is closed.

With this change, the authorization can be performed multiple times without closing the popup.

All tests passed:
npm run test:unit -- ./test/unit/core/oauth2-authorize.js --silent=false
npm run test

Checklist

My PR contains...

• No code changes (src/ is unmodified: changes to documentation, CI, metadata, etc.)
• Dependency changes (any modification to dependencies in package.json)
• Bug fixes (non-breaking change which fixes an issue)
• Improvements (misc. changes to existing features)
• Features (non-breaking change which adds functionality)

My changes...

• are breaking changes to a public API (config options, System API, major UI change, etc).
• are breaking changes to a private API (Redux, component props, utility functions, etc.).
• are breaking changes to a developer API (npm script behavior changes, new dev system dependencies, etc).
• are not breaking changes.

Documentation

• My changes do not require a change to the project documentation.
• My changes require a change to the project documentation.
• If yes to above: I have updated the documentation accordingly.

Automated tests

• My changes can not or do not need to be tested.
• My changes can and should be tested by unit and/or integration tests.
• If yes to above: I have added tests to cover my changes.
• If yes to above: I have taken care to cover edge cases in my tests.
• All new and existing tests passed.

[davidanrod]

Also:
Fixes #6959
Fixes #10229

[smitsjelle]

I experience the same issue for non-PKCE authorization code flows (as also risen in this discussion). Does the fix also work there?

[davidanrod]

Hi @smitsjelle, no it does not.. give me some hours, and I’ll update this PR to handle that too. Thank you

[davidanrod]

Hi @robert-hebel-sb can you help in some way with this being reviewed?

[dhsy6z]

This will be a huge improvement for my project. I appreciate you doing the work to resolve this. Hopefully they can get it reviewed and merged soon.

[cka121]

Hi @davidanrod, Thank you for identifying this issue and providing a fix.
However, I'd like to suggest a slightly improved approach before we go ahead with this. Here's a guide to make the changes:

What Needs to Change
Instead of using delete auth.code (which directly mutates the input object), let's use a non-mutating approach scoped specifically to the PKCE flow. This is safer and more predictable.

[davidanrod]

Hi @cka121, thank you for your comment.. actually that was my first approach but i don't know why it didn't work at the time :), because i made a mistake. Anyway, i'm no longer mutating the original object. All tests OK.

It would be great to get this merged into the main branch, because it's really annoying for end users.

David.