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.

Sign up for GitHub

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

Jump to bottom

Add timeout option for updateToken in the js adapter #10516

Closed
wants to merge 2 commits into from
Closed

Add timeout option for updateToken in the js adapter #10516

wants to merge 2 commits into from

Conversation

panuhorsmalahti
Copy link

@panuhorsmalahti panuhorsmalahti commented Mar 1, 2022 •
edited
Loading

Closes keycloak/keycloak-js#25

I'll make a PR to the documentation repo once this change is approved.

@stianst stianst requested a review from jonkoops March 2, 2022 07:13
@panuhorsmalahti
Copy link
Author

Should create a PR to add documentation?

jonkoops
jonkoops requested changes Jun 13, 2022
View reviewed changes
Copy link
Contributor

@jonkoops jonkoops left a comment

Choose a reason for hiding this comment

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

Please also update the TypeScript definitions for this option with some documentation.

@jonkoops jonkoops added kind/feature Categorizes a PR related to a new feature area/adapter/javascript labels Jul 18, 2022
@jonkoops
Copy link
Contributor

@panuhorsmalahti Are you still interested in landing this change?

@panuhorsmalahti
Copy link
Author

@jonkoops yes I'd like to get this merged.

I added documentation to the TypeScript definition.

@jonkoops jonkoops requested a review from abstractj July 18, 2022 14:14
@jonkoops
Copy link
Contributor

Awesome, thanks for your contribution @panuhorsmalahti!

abstractj
abstractj reviewed Jul 18, 2022
View reviewed changes
Copy link
Contributor

@abstractj abstractj left a comment

Choose a reason for hiding this comment

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

I'd also suggest including tests for this new option. You can find some examples here: https://github.com/keycloak/keycloak/blob/main/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/javascript/JavascriptAdapterTest.java

@jonkoops
Copy link
Contributor

jonkoops commented Apr 4, 2023

@panuhorsmalahti could you rebase your PR on main? We've made some changes to the directory structure so the adapter is now located in js/libs/keycloak-js. Also, the Keycloak documentation has now been moved into this repository so those changes can also be included here.

@jonkoops
Copy link
Contributor

I am closing this PR as we first want to do a refactor of Keycloak JS itself, which will not be happening soon-ish. This is definitely a useful addition that we want to land, but the bandwidth isn't there at the moment.

I am keeping the issue around so we can get back to this change when we are in a better condition.

@jonkoops jonkoops closed this Apr 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abstractj abstractj abstractj left review comments

@jonkoops jonkoops jonkoops approved these changes

Assignees
No one assigned
Labels
area/adapter/javascript kind/feature Categorizes a PR related to a new feature team/ui
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Javascript adapter: updateToken timeout configuration
3 participants
@panuhorsmalahti @jonkoops @abstractj