-
Notifications
You must be signed in to change notification settings - Fork 6
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
Modify auth flow for transkribus to include refresh token #68
Conversation
Added static methods for making accesstoken and refreshtoken requests
Removed accessToken and refreshToken arguments from TranskribusClient constructor Added env variables to phpunit.xml.dist
… and removed reduntant auth code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, works in my tests. Just a few errant variables that are no longer required, but otherwise good to go I think.
Replaced magic number with a constant in TranskribusClient and removed TranskribusClient services from services config
src/Engine/TranskribusEngine.php
Outdated
Intuition $intuition, | ||
string $projectDir, | ||
HttpClientInterface $httpClient | ||
) { | ||
parent::__construct($intuition, $projectDir, $httpClient); | ||
|
||
$this->transkribusClient = $transkribusClient; | ||
$this->transkribusClient->setTokens($accessToken, $refreshToken); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tokens are only injected here in order to be passed through to the client — would it be better to inject them into the client instead?
Moved token injection from TranskribusEngine to TranskribusClient Updated OcrControllerTest and services.yaml
Modify the authentication flow of the Transkribus engine to use refresh tokens in addition to access tokens
Bug: T330052