Skip to content

Improve window handling in Authorization Code flow - #37

Merged
langsamu merged 2 commits into
mainfrom
improve-authorization-window
Aug 26, 2026
Merged

Improve window handling in Authorization Code flow#37
langsamu merged 2 commits into
mainfrom
improve-authorization-window

Conversation

@langsamu

@langsamu langsamu commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Supersedes #13 and aims to replicate its functionality but eliminates its code duplication.

This change modifies the functionality of the

  • Authorization Code provider and the
  • DPoP token provider that relies on it

so the popup window used for the Authorization Code flow is closed only after the code was received, potentially including after an unsuccessful silent attempt (prompt=none).

Closing the window is now the job of the caller, not the code provider:
image

For this reason I had converted the code provider from a callback lambda to an interface in 8c32223.

@langsamu langsamu left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Explanation

this.ownerDocument.defaultView?.removeEventListener("message", onMessage)
signal.removeEventListener("abort", onAbort)
this.#switchModal.close()
this.#authorizationWindow?.close()

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

First substantial change is not closing the authorization window in the provider.

Comment thread src/DPoPTokenProvider.ts
}
}

this.#codeProvider.cleanup()

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

And this is the counterpart: Closing the authorization window from the caller, crucially after a potential fallback in the try above.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Following https://github.com/solid-contrib/reactive-authentication/pull/37/changes#r3853649238 - this would be a good place to make use of using

Comment on lines +221 to +223
cleanup(): void {
this.#authorizationWindow?.close()
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Consider making the AuthorizationCodeFlow class disposable and invoking this in the [Symbol.dispose] function.

This also enables the new using sugar to be used.

This also applies to any other instances of cleanup methods.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Forgot to commit this change yesterday:

image

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

#38

Comment thread src/DPoPTokenProvider.ts
}
}

this.#codeProvider.cleanup()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Following https://github.com/solid-contrib/reactive-authentication/pull/37/changes#r3853649238 - this would be a good place to make use of using

@langsamu langsamu changed the title improve authorization window Improve authorization window Aug 26, 2026
@langsamu langsamu changed the title Improve authorization window Improve window handling in Authorization Code flow Aug 26, 2026
@langsamu
langsamu merged commit 99d87be into main Aug 26, 2026
5 checks passed
@langsamu
langsamu deleted the improve-authorization-window branch August 26, 2026 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants