🌱 Add auth token refresh to Binding #592

aufi · 2024-01-15T10:57:52Z

There is an issue with long-time running tests e.g. for analysis on Konveyor with enabled Auth feature. Keycloak allows to refresh auth token, however the token for refresh was not stored in binding.Client and binding.RichClient.Login method didn't return the Login struct with Refresh and Expiry fields.

Main changes in this PR

binding.client keeps api.Login struct instead of string token to allow use Refresh token to get a new one
binding.client.send method can catch 401 and try refresh auth token after each API call

For visibility @mguetta1

Fixes konveyor/go-konveyor-tests#71

There was no logic to refresh auth token needed for longer-time tasks (e.g. tests). Adding this method to RichCLient. To be able use Token Refresh method, it was needed keep whole api.Login struct as part of client to have refresh token. Related to konveyor/go-konveyor-tests#71 Signed-off-by: Marek Aufart <[email protected]>

Signed-off-by: Marek Aufart <[email protected]>

jortel · 2024-01-16T15:05:16Z

Lets leverage the existing retry mechanism and refresh the token on demand. In Client.send(), on 401 (not authorized) refresh the token when defined (!= "").
Probably best to add a Client.refresh() that encapsulates this complexity and just returns a boolean.
usage: if r.refresh() continue, else fall-through and report the original 401.
This should require minimal change.

Signed-off-by: Marek Aufart <[email protected]>

jortel · 2024-01-30T15:10:21Z

binding/client.go

@@ -83,7 +83,7 @@ type Client struct {
 	// baseURL for the nub.
 	baseURL string
 	// addon API token
-	token string


Why does the client need the Login?

It needs the Refresh token to perform the auth token refresh API call. api.Login contains all data returned from login (auth token, refresh token, expiration time).

Calling token refresh with "normal" expired token returns 401, so it is needed to use Refresh token (originally returned by Login call).

Example success with Refresh token

... # 29 === RUN TestTaskCRUD/Test_windup_task#29 time=2024-02-06T15:47:17+01:00 level=info msg=[binding] |201| POST /hub/tasks time=2024-02-06T15:47:17+01:00 level=info msg=[binding] |200| GET /hub/tasks/1 time=2024-02-06T15:47:17+01:00 level=info msg=[binding] |204| PUT /hub/tasks/1 time=2024-02-06T15:47:17+01:00 level=info msg=[binding] |200| GET /hub/tasks/1 time=2024-02-06T15:47:17+01:00 level=info msg=[binding] |204| DELETE /hub/tasks/1 time=2024-02-06T15:47:17+01:00 level=info msg=[binding] |404| GET /hub/tasks/1 # 30 === RUN TestTaskCRUD/Test_windup_task#30 time=2024-02-06T15:47:27+01:00 level=info msg=[binding] |401| POST /hub/tasks time=2024-02-06T15:47:27+01:00 level=info msg=[binding] |201| POST /hub/auth/refresh time=2024-02-06T15:47:27+01:00 level=info msg=[binding] |201| POST /hub/tasks time=2024-02-06T15:47:27+01:00 level=info msg=[binding] |200| GET /hub/tasks/1 time=2024-02-06T15:47:27+01:00 level=info msg=[binding] |204| PUT /hub/tasks/1 time=2024-02-06T15:47:27+01:00 level=info msg=[binding] |200| GET /hub/tasks/1 time=2024-02-06T15:47:27+01:00 level=info msg=[binding] |204| DELETE /hub/tasks/1 time=2024-02-06T15:47:27+01:00 level=info msg=[binding] |404| GET /hub/tasks/1 # 31 === RUN TestTaskCRUD/Test_windup_task#31 time=2024-02-06T15:47:37+01:00 level=info msg=[binding] |201| POST /hub/tasks time=2024-02-06T15:47:37+01:00 level=info msg=[binding] |200| GET /hub/tasks/1 time=2024-02-06T15:47:37+01:00 level=info msg=[binding] |204| PUT /hub/tasks/1 time=2024-02-06T15:47:37+01:00 level=info msg=[binding] |200| GET /hub/tasks/1 time=2024-02-06T15:47:37+01:00 level=info msg=[binding] |204| DELETE /hub/tasks/1 time=2024-02-06T15:47:37+01:00 level=info msg=[binding] |404| GET /hub/tasks/1

Failure with just "normal" expired token

... # 29 === RUN TestTaskCRUD/Test_windup_task#29 time=2024-02-06T15:41:37+01:00 level=info msg=[binding] |201| POST /hub/tasks time=2024-02-06T15:41:37+01:00 level=info msg=[binding] |200| GET /hub/tasks/1 time=2024-02-06T15:41:37+01:00 level=info msg=[binding] |204| PUT /hub/tasks/1 time=2024-02-06T15:41:37+01:00 level=info msg=[binding] |200| GET /hub/tasks/1 time=2024-02-06T15:41:37+01:00 level=info msg=[binding] |204| DELETE /hub/tasks/1 time=2024-02-06T15:41:37+01:00 level=info msg=[binding] |404| GET /hub/tasks/1 # 30 === RUN TestTaskCRUD/Test_windup_task#30 time=2024-02-06T15:41:47+01:00 level=info msg=[binding] |401| POST /hub/tasks time=2024-02-06T15:41:47+01:00 level=info msg=[binding] |401| POST /hub/auth/refresh api_test.go:20: POST /hub/tasks failed: 401(Unauthorized) time=2024-02-06T15:41:47+01:00 level=info msg=[binding] |401| GET /hub/tasks/0 time=2024-02-06T15:41:47+01:00 level=info msg=[binding] |401| POST /hub/auth/refresh api_test.go:26: GET /hub/tasks/0 failed: 401(Unauthorized) time=2024-02-06T15:41:47+01:00 level=info msg=[binding] |401| PUT /hub/tasks/0 time=2024-02-06T15:41:47+01:00 level=info msg=[binding] |401| POST /hub/auth/refresh api_test.go:36: PUT /hub/tasks/0 failed: 401(Unauthorized) time=2024-02-06T15:41:47+01:00 level=info msg=[binding] |401| GET /hub/tasks/0 time=2024-02-06T15:41:47+01:00 level=info msg=[binding] |401| POST /hub/auth/refresh api_test.go:41: GET /hub/tasks/0 failed: 401(Unauthorized) api_test.go:44: Different response error. Got , expected Updated Test windup task time=2024-02-06T15:41:47+01:00 level=info msg=[binding] |401| DELETE /hub/tasks/0 time=2024-02-06T15:41:47+01:00 level=info msg=[binding] |401| POST /hub/auth/refresh api_test.go:50: DELETE /hub/tasks/0 failed: 401(Unauthorized) time=2024-02-06T15:41:47+01:00 level=info msg=[binding] |401| GET /hub/tasks/0 time=2024-02-06T15:41:47+01:00 level=info msg=[binding] |401| POST /hub/auth/refresh # 31 === RUN TestTaskCRUD/Test_windup_task#31 time=2024-02-06T15:41:57+01:00 level=info msg=[binding] |401| POST /hub/tasks time=2024-02-06T15:41:57+01:00 level=info msg=[binding] |401| POST /hub/auth/refresh api_test.go:20: POST /hub/tasks failed: 401(Unauthorized) time=2024-02-06T15:41:57+01:00 level=info msg=[binding] |401| GET /hub/tasks/0 time=2024-02-06T15:41:57+01:00 level=info msg=[binding] |401| POST /hub/auth/refresh api_test.go:26: GET /hub/tasks/0 failed: 401(Unauthorized) ...

Thanks for the explanation.

binding/client.go

Signed-off-by: Marek Aufart <[email protected]>

aufi · 2024-02-07T13:24:35Z

@jortel Thanks for your review&inputs. The PR was updated with changes you suggested.

One open thing is the storage of the refresh token together with "normal" auth token. I can understand that using api.Login{} directly in client might not look the best, but I think we do need store these data within the client and api.Login struct allows set the login action response directly to the client. But I'm open to creating an own struct within binding client to keep tokens or a separate field for refresh token if needed.

jortel

LGTM

Updating hub dependency to latest version that provides automatic auth token refresh logic added in konveyor/tackle2-hub#592 Fixes: konveyor#71 Signed-off-by: Marek Aufart <[email protected]>

* Update Hub for auth token refresh Updating hub dependency to latest version that provides automatic auth token refresh logic added in konveyor/tackle2-hub#592 Fixes: #71 Signed-off-by: Marek Aufart <[email protected]> * Add missing go.sum entry for urn Signed-off-by: Marek Aufart <[email protected]> --------- Signed-off-by: Marek Aufart <[email protected]>

* Update Hub for auth token refresh Updating hub dependency to latest version that provides automatic auth token refresh logic added in konveyor/tackle2-hub#592 Fixes: #71 Signed-off-by: Marek Aufart <[email protected]> * Add missing go.sum entry for urn Signed-off-by: Marek Aufart <[email protected]> --------- Signed-off-by: Marek Aufart <[email protected]> Signed-off-by: Cherry Picker <[email protected]>

* Update Hub for auth token refresh Updating hub dependency to latest version that provides automatic auth token refresh logic added in konveyor/tackle2-hub#592 Fixes: #71 * Add missing go.sum entry for urn --------- Signed-off-by: Marek Aufart <[email protected]> Signed-off-by: Cherry Picker <[email protected]> Co-authored-by: Marek Aufart <[email protected]>

aufi added 2 commits January 12, 2024 15:43

Update token refresh loop

c104568

Signed-off-by: Marek Aufart <[email protected]>

aufi changed the title ~~Add auth token refresh to Binding~~ 🌱 Add auth token refresh to Binding Jan 16, 2024

Move refresh logic to client.send()

4e32878

Signed-off-by: Marek Aufart <[email protected]>

aufi force-pushed the binding-token-refresh branch from 3136f14 to 335fd9b Compare January 25, 2024 10:10

Update token refresh logging

dab0f9e

Signed-off-by: Marek Aufart <[email protected]>

aufi force-pushed the binding-token-refresh branch from 42da159 to dab0f9e Compare January 25, 2024 10:23

aufi marked this pull request as ready for review January 25, 2024 10:30

aufi requested review from jortel and mansam January 25, 2024 16:46

jortel reviewed Jan 30, 2024

View reviewed changes

binding/client.go Outdated Show resolved Hide resolved

jortel reviewed Jan 30, 2024

View reviewed changes

binding/client.go Outdated Show resolved Hide resolved

Update refresh token call in client

4860b42

Signed-off-by: Marek Aufart <[email protected]>

aufi force-pushed the binding-token-refresh branch from acf33df to 4860b42 Compare February 6, 2024 14:50

Merge branch 'main' into binding-token-refresh

f698ae8

Signed-off-by: Marek Aufart <[email protected]>

aufi requested a review from jortel February 8, 2024 16:17

jortel approved these changes Feb 9, 2024

View reviewed changes

aufi merged commit ae2295a into konveyor:main Feb 12, 2024
11 checks passed

aufi mentioned this pull request Feb 12, 2024

Update Hub for auth token refresh konveyor/go-konveyor-tests#93

Merged

github-actions bot mentioned this pull request Feb 13, 2024

Update Hub for auth token refresh (#93) konveyor/go-konveyor-tests#96

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🌱 Add auth token refresh to Binding #592

🌱 Add auth token refresh to Binding #592

aufi commented Jan 15, 2024 •

edited

Loading

jortel commented Jan 16, 2024 •

edited

Loading

jortel Jan 30, 2024 •

edited

Loading

aufi Jan 31, 2024

aufi Feb 6, 2024 •

edited

Loading

jortel Feb 9, 2024

aufi commented Feb 7, 2024 •

edited

Loading

jortel left a comment

🌱 Add auth token refresh to Binding #592

🌱 Add auth token refresh to Binding #592

Conversation

aufi commented Jan 15, 2024 • edited Loading

jortel commented Jan 16, 2024 • edited Loading

jortel Jan 30, 2024 • edited Loading

Choose a reason for hiding this comment

aufi Jan 31, 2024

Choose a reason for hiding this comment

aufi Feb 6, 2024 • edited Loading

Choose a reason for hiding this comment

Example success with Refresh token

Failure with just "normal" expired token

jortel Feb 9, 2024

Choose a reason for hiding this comment

aufi commented Feb 7, 2024 • edited Loading

jortel left a comment

Choose a reason for hiding this comment

aufi commented Jan 15, 2024 •

edited

Loading

jortel commented Jan 16, 2024 •

edited

Loading

jortel Jan 30, 2024 •

edited

Loading

aufi Feb 6, 2024 •

edited

Loading

aufi commented Feb 7, 2024 •

edited

Loading