Skip to content

improve authorization window - #37

Open
langsamu wants to merge 2 commits into
mainfrom
improve-authorization-window
Open

improve authorization window#37
langsamu wants to merge 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.

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

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