Skip to content

Refactor handling of OAuth client tokens - #24684 - #24726

Open
NobodysNightmare wants to merge 5 commits into
release/17.8from
oauth-client-tokens
Open

Refactor handling of OAuth client tokens - #24684#24726
NobodysNightmare wants to merge 5 commits into
release/17.8from
oauth-client-tokens

Conversation

@NobodysNightmare

Copy link
Copy Markdown
Contributor

Previously the fetching and updating of OAuthClientToken was left as an implementation detail of specific integrations, even though the concept of OAuthClient and OAuthClientToken were defined in the core and even their creation is already handled through a core class (the ConnectionManager).

This PR introduces the OAuthClients::TokenFetcher that serves as the new main interface to obtain the access token of an OAuthClientToken. A relevant difference compared to the previous approach is that we refresh access tokens based on their time-to-live (i.e. expires_in), not based on receiving an error response and then retrying.

Ticket

https://community.openproject.org/wp/SI-236

This class provides generic helpers to perform OAuth 2.0 token requests,
which are not only useful in the context of OpenID Connect, but also for other
kinds of OAuth clients.
Previously the fetching and updating of `OAuthClientToken` was left as an
implementation detail of specific integrations, even though the concept of
`OAuthClient` and `OAuthClientToken` were defined in the core and even their
creation is already handled through a core class (the `ConnectionManager`).

This commit introduces the `OAuthClients::TokenFetcher` that serves as
the new main interface to obtain the access token of an `OAuthClientToken`.
The previous cassette contained a refresh of an OAuth token that was performed
through the previous "try-and-error"-based approach for authentication.

I.e. the first request failed with an authentication error, then there was an additional
request included to refresh the OAuth token and the second request to the first endpoint
succeeded. This VCR cassette is incompatible with the new TTL-based authentication approach,
where a token is refreshed shortly before expiry, but not based on negative request outcomes.
Introducing a core class for logging that's inspired by the
Storages::TaggedLogging module. Though it aims to natively
support key-value-pairs during logging.

What still needs to be added is a way to temporarily add tags,
i.e. `with_log_tags { ... }`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant