Skip to content

Return HTTP OK on token drop for oauth2 authentication#10929

Merged
kokosing merged 2 commits intotrinodb:masterfrom
wendigo:serafin/fix-token-resolving
Feb 3, 2022
Merged

Return HTTP OK on token drop for oauth2 authentication#10929
kokosing merged 2 commits intotrinodb:masterfrom
wendigo:serafin/fix-token-resolving

Conversation

@wendigo
Copy link
Copy Markdown
Contributor

@wendigo wendigo commented Feb 3, 2022

HttpTokenPoller waits 4 seconds for token drop to be acknowledged (checks for HTTP OK). With this small change CLI/JDBC returns token almost immediately upon resolution.

HttpTokenPoller waits 4 seconds for token drop to be acknowledged. With this small change CLI/JDBC returns token almost immediately upon resolution.
@cla-bot cla-bot bot added the cla-signed label Feb 3, 2022
@wendigo wendigo requested review from kokosing and s2lomon February 3, 2022 10:15
Copy link
Copy Markdown
Member

@kokosing kokosing left a comment

Choose a reason for hiding this comment

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

Why it was waiting?

@wendigo
Copy link
Copy Markdown
Contributor Author

wendigo commented Feb 3, 2022

@kokosing because HttpTokenPoller waits for the ACK (HTTP OK) on HTTP DELETE on token endpoint with a timeout of 4s. And didn't return Response http status that was HTTP OK (I guess that it was NO CONTENT)

@kokosing
Copy link
Copy Markdown
Member

kokosing commented Feb 3, 2022

HttpTokenPoller waits for the ACK (HTTP OK)

Can we make HttpTokenPoller to wait for 2xx?

Are there any other places likes that?

@wendigo
Copy link
Copy Markdown
Contributor Author

wendigo commented Feb 3, 2022

@kokosing done

@wendigo wendigo force-pushed the serafin/fix-token-resolving branch from eaf2048 to 816cebc Compare February 3, 2022 10:57
@s2lomon s2lomon mentioned this pull request Feb 3, 2022
Copy link
Copy Markdown
Member

@s2lomon s2lomon left a comment

Choose a reason for hiding this comment

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

% 1 minor comment.

@wendigo wendigo requested a review from kokosing February 3, 2022 12:27
@wendigo
Copy link
Copy Markdown
Contributor Author

wendigo commented Feb 3, 2022

Ping @kokosing

@kokosing kokosing merged commit 932f153 into trinodb:master Feb 3, 2022
@kokosing kokosing mentioned this pull request Feb 3, 2022
@github-actions github-actions bot added this to the 370 milestone Feb 3, 2022
@wendigo wendigo deleted the serafin/fix-token-resolving branch February 5, 2022 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants