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.

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

Add a button to clear oauth2 from the auth options #2953

Merged

Conversation

gravyboat
Copy link
Contributor

Closes #2902

This uses the same initNewOAuthSession as the preferences menu to ensure the functionality is identical.

@rafaelrenanpacheco is this along the lines of what you were thinking?

oauth2-clear-session

Copy link
Contributor

@develohpanda develohpanda left a comment

Choose a reason for hiding this comment

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

Very cool! 😎

@rafaelrenanpacheco
Copy link
Contributor

Exactly like that @gravyboat, very nice!

Thanks for this PR 👍

Copy link
Contributor

@nijikokun nijikokun left a comment

Choose a reason for hiding this comment

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

Like the placement, and the functionality.

Makes me think we should look into something like profiles/session-store. So users can save sessions, and swap between them. Seems like issues surrounding clearing and changing consistently crop up and would be solved by a more robust profile/session-store style solution.

Secondly a minor nitpick, it looks like this button is always available, seems like it should be disabled unless a session exists.

@gravyboat
Copy link
Contributor Author

gravyboat commented Dec 18, 2020

Secondly a minor nitpick, it looks like this button is always available, seems like it should be disabled unless a session exists.

This is correct, I left it that way since that's how it operates in the preferences menu currently as well.

Edit: If LOCALSTORAGE_KEY_SESSION_ID was exported the check could be changed to look like:

showAdvanced && window.localStorage.getItem(LOCALSTORAGE_KEY_SESSION_ID)

It could also be updated to check that const in the preferences menu but that seems like it should probably be a second PR.

@develohpanda
Copy link
Contributor

I agree with you @gravyboat, my preference will be a separate PR as it is a separate request.

I have noticed that the action does not have any kind of feedback either, so that would be something to also consider. ☺

@develohpanda develohpanda merged commit 6d5b8dd into Kong:develop Dec 19, 2020
@gravyboat gravyboat deleted the feature/2902-clear-oauth2-session-button branch December 19, 2020 04:47
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.

Move or replicate "Clear OAuth 2 session" button location
4 participants